From 1aed605fb6d5314f334df84374106dd1cadd79c3 Mon Sep 17 00:00:00 2001 From: nesquena-hermes <[email protected]> Date: Sun, 31 May 2026 19:21:15 +0000 Subject: [PATCH] fix(#3267): harden collapsed tool-preview secret filter (Codex gate MUST-FIX) Codex regression gate found the exact-name hidden-key set leaked secret-shaped args (apiKey/access_token/clientSecret/Authorization/cookie/...) into the always-visible collapsed tool-card header. Replace with a normalized case-insensitive _toolArgPreviewKeyIsHidden() predicate matching secret-bearing substrings + camelCase variants. Adds 22 parametrized regression tests pinning the secret-key denial + a legit-key-still-shown guard. Co-authored-by preserved. --- static/ui.js | 19 +++++++-- ...test_issue1824_cli_patch_diff_rendering.py | 2 + tests/test_tool_card_preview_summary.py | 39 +++++++++++++++++++ 3 files changed, 57 insertions(+), 3 deletions(-) diff --git a/static/ui.js b/static/ui.js index 64ec33a5..e0b576bc 100644 --- a/static/ui.js +++ b/static/ui.js @@ -7030,17 +7030,30 @@ function _toolArgPreviewValue(value){ if(typeof value==='object') return 'object'; return String(value).replace(/\s+/g,' ').trim(); } +// Secret/sensitive-arg guard for collapsed tool-card previews. Exact-name hiding +// alone misses camelCase / variant spellings (apiKey, access_token, clientSecret, +// Authorization, …), so a normalized substring check runs first so secret-shaped +// argument names are never surfaced in the always-visible collapsed header (#3267). +function _toolArgPreviewKeyIsHidden(key){ + const k=String(key||'').toLowerCase().replace(/[^a-z0-9]/g,''); + // verbose-but-not-secret bodies we keep out of the compact preview + const verbose=['content','filecontent','newstring','oldstring','patch','text','message','prompt','code','script','cookies','headers']; + if(verbose.includes(k)) return true; + // secret-shaped substrings (covers api_key/apiKey, access_token/auth_token/bearer, + // client_secret, password, credential, private_key, authorization, etc.) + return /(apikey|token|secret|password|passwd|credential|authorization|\bauth\b|auth$|^auth|bearer|privatekey|accesskey|sessionkey|signingkey|cookie)/.test(k) + || k==='auth' || k==='key' || k==='pat'; +} function _formatToolArgPreview(args){ if(!args||typeof args!=='object') return ''; const preferred=['path','file_path','target','pattern','query','url','urls','name','ref','command','action','mode','schedule','workdir']; - const hidden=new Set(['content','file_content','new_string','old_string','patch','text','message','prompt','code','script','cookies','headers','auth','api_key','password','token','secret']); const keys=[]; for(const key of preferred){ - if(Object.prototype.hasOwnProperty.call(args,key)&&!hidden.has(key)) keys.push(key); + if(Object.prototype.hasOwnProperty.call(args,key)&&!_toolArgPreviewKeyIsHidden(key)) keys.push(key); } for(const key of Object.keys(args)){ if(keys.length>=3) break; - if(keys.includes(key)||hidden.has(key)) continue; + if(keys.includes(key)||_toolArgPreviewKeyIsHidden(key)) continue; keys.push(key); } const parts=[]; diff --git a/tests/test_issue1824_cli_patch_diff_rendering.py b/tests/test_issue1824_cli_patch_diff_rendering.py index 21b76d48..c93d0281 100644 --- a/tests/test_issue1824_cli_patch_diff_rendering.py +++ b/tests/test_issue1824_cli_patch_diff_rendering.py @@ -109,6 +109,8 @@ def test_rendered_apply_patch_tool_card_html_contains_diff_lines(): "_cliPatchSnippetFromArgs", "_cliToolCardSnippet", "_cliToolCardHasDiffSnippet", + "_toolArgPreviewValue", + "_toolArgPreviewKeyIsHidden", "_formatToolArgPreview", "_toolCardPreviewText", "buildToolCard", diff --git a/tests/test_tool_card_preview_summary.py b/tests/test_tool_card_preview_summary.py index 89e1ccad..b50b7664 100644 --- a/tests/test_tool_card_preview_summary.py +++ b/tests/test_tool_card_preview_summary.py @@ -36,6 +36,7 @@ function extractFunc(name) { return src.slice(start, i); } eval(extractFunc('_toolArgPreviewValue')); +eval(extractFunc('_toolArgPreviewKeyIsHidden')); eval(extractFunc('_formatToolArgPreview')); eval(extractFunc('_toolCardPreviewText')); let buf = ''; @@ -103,3 +104,41 @@ def test_explicit_progress_preview_still_wins(driver_path): "stdout", ) assert preview == "Running command" + + +@pytest.mark.parametrize( + "secret_key", + [ + "api_key", "apiKey", "API_KEY", "x-api-key", + "token", "access_token", "refresh_token", "auth_token", "bearer_token", + "authorization", "Authorization", + "secret", "secret_key", "client_secret", "clientSecret", + "password", "passwd", + "private_key", "credential", "cookie", "cookies", + ], +) +def test_collapsed_preview_never_exposes_secret_shaped_arg_keys(driver_path, secret_key): + """Regression: the collapsed-preview arg summary must never surface a + secret-shaped argument key/value, including camelCase / variant spellings. + + Codex regression gate found that exact-name hiding leaked apiKey / + access_token / clientSecret / Authorization etc. into the always-visible + collapsed header (v0.51.190). The normalized `_toolArgPreviewKeyIsHidden` + predicate closes this; this test pins it so it can't silently regress. + """ + preview = _preview( + driver_path, + {"name": "mcp_tool", "args": {secret_key: "SUPER-SECRET-VALUE-xyz", "path": "/visible/ok"}, "done": True}, + ) + assert "SUPER-SECRET-VALUE-xyz" not in preview, f"{secret_key} value leaked into preview: {preview!r}" + # the legit, non-secret key is still allowed to render + assert "/visible/ok" in preview + + +def test_collapsed_preview_still_shows_legit_keys(driver_path): + """The secret guard must not over-block ordinary tool args.""" + preview = _preview( + driver_path, + {"name": "search_files", "args": {"target": "content", "pattern": "foo", "workdir": "/repo"}, "done": True}, + ) + assert "target=" in preview and "pattern=" in preview