From f79c9758423a2a5db23d9ff607d1ad1c805121e3 Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Thu, 25 Jun 2026 18:28:06 +0000 Subject: [PATCH] fix(#4928): redact tool args at all render/copy sites now that content args are full-length Codex gate: raising content args from 120->4000 chars newly surfaces secrets on non-first lines / past char 120 in render paths that print tc.args directly and bypass the #4926 label redaction. Apply _redactToolTargetLabel to arg values at every such site: the transparent Full-tab args render (_transparentToolDetailHtml), the non-transparent tool-card args block (buildToolCard visibleArgs), the clipboard copy path (_copyEventToClipboard), and the _toolArgsSnapshot itself. + node regression proving a secret past char 120 in a long command is masked in the Full tab while the non-secret command structure is retained. --- static/ui.js | 39 ++++++++++++-- tests/test_issue4928_tool_arg_content_cap.py | 54 ++++++++++++++++++++ 2 files changed, 89 insertions(+), 4 deletions(-) diff --git a/static/ui.js b/static/ui.js index 1eff12b5..744f1584 100644 --- a/static/ui.js +++ b/static/ui.js @@ -8451,7 +8451,21 @@ function _copyEventToClipboard(row){ const fallbackName=row.getAttribute('data-event-name')||row.getAttribute('data-tool-name')||'tool'; label=`tool ${tc.name||fallbackName}`; const parts=[`tool: ${tc.name||fallbackName}`]; - if(tc.args&&Object.keys(tc.args).length) parts.push('args: '+JSON.stringify(tc.args,null,2)); + if(tc.args&&Object.keys(tc.args).length){ + // Redact secret-bearing arg values before copying to clipboard, mirroring + // the Full-tab render — content args can be long commands with secrets + // past the first line (#4928 gate). + let argsForCopy=tc.args; + if(typeof _redactToolTargetLabel==='function'){ + try{ + argsForCopy={}; + Object.entries(tc.args).forEach(([k,v])=>{ + argsForCopy[k]=typeof v==='string'?_redactToolTargetLabel(v):v; + }); + }catch(e){ argsForCopy=tc.args; } + } + parts.push('args: '+JSON.stringify(argsForCopy,null,2)); + } if(tc.snippet) parts.push('output:\n'+String(tc.snippet)); if(parts.length===1){ const argsText=Array.from(row.querySelectorAll('.tool-card-args .tool-arg-pair')) @@ -8586,7 +8600,14 @@ function _transparentToolDetailHtml(tc, status){ const meta=[]; if(tc&&tc.duration!==undefined&&tc.duration!==null) meta.push(['duration', String(tc.duration)]); const preview=String((tc&&(tc.snippet||tc.preview||tc.result||tc.output))||'').trim(); - const argHtml=[...meta,...argEntries].map(([k,v])=>`
${esc(String(k))}${esc(typeof v==='string'?v:JSON.stringify(v,null,2))}
`).join(''); + const argHtml=[...meta,...argEntries].map(([k,v])=>{ + let sv=typeof v==='string'?v:JSON.stringify(v,null,2); + // Redact secret-bearing arg values before rendering the transparent Full + // tab — content args can be long multi-line commands (#4928) whose later + // lines may carry secrets the short label never showed (#4928 gate). + if(typeof _redactToolTargetLabel==='function'){ try{ sv=_redactToolTargetLabel(sv); }catch(e){} } + return `
${esc(String(k))}${esc(sv)}
`; + }).join(''); return `
FullOutput
${argHtml}
${preview?`
${esc(preview)}
`:''}
`; } function _syncTransparentEventControls(turn){ @@ -10974,7 +10995,13 @@ function _toolArgsSnapshot(args, limit){ keys.forEach(k=>{ const v=String(args[k]); const cap=contentKeys.has(String(k).toLowerCase())?CONTENT_CAP:120; - out[k]=v.slice(0,cap)+(v.length>cap?'...':''); + let val=v.slice(0,cap)+(v.length>cap?'...':''); + // Now that content args are retained up to 4000 chars (#4928), a secret on + // a non-first line / past char 120 would otherwise reach the args block, + // the Full tab, and clipboard copy unredacted. Redact at the snapshot so + // every downstream renderer receives already-masked args (#4928 gate). + if(typeof _redactToolTargetLabel==='function'){ try{ val=_redactToolTargetLabel(val); }catch(e){} } + out[k]=val; }); return out; } @@ -13032,7 +13059,11 @@ function buildToolCard(tc){ ${hasDetail?`
${detailLead} ${visibleArgs.length?`
${ - visibleArgs.map(([k,v])=>`
${esc(k)}${esc(String(v))}
`).join('') + visibleArgs.map(([k,v])=>{ + let sv=String(v); + if(typeof _redactToolTargetLabel==='function'){ try{ sv=_redactToolTargetLabel(sv); }catch(e){} } + return `
${esc(k)}${esc(sv)}
`; + }).join('') }
`:''} ${displaySnippet?`
${tc.is_diff||_snippetLooksLikeDiff(displaySnippet)?`${_colorDiffLines(displaySnippet)}`:esc(displaySnippet)}
diff --git a/tests/test_issue4928_tool_arg_content_cap.py b/tests/test_issue4928_tool_arg_content_cap.py index 85b15eeb..0c830e1a 100644 --- a/tests/test_issue4928_tool_arg_content_cap.py +++ b/tests/test_issue4928_tool_arg_content_cap.py @@ -133,3 +133,57 @@ def test_frontend_keeps_patch_args(fe_driver): def test_frontend_incidental_arg_still_capped(fe_driver): out = _fe_snapshot(fe_driver, {"label": "z" * 300}) assert out["label"] == "z" * 120 + "..." + + +# ---------- frontend: transparent Full-tab args render must redact ---------- + +_DETAIL_DRIVER = r""" +const fs = require('fs'); +const src = fs.readFileSync(process.argv[2], 'utf8'); +function grab(name){ + const re = new RegExp('function ' + name + '\\([^]*?\\n}', 'm'); + const m = src.match(re); + if (!m) throw new Error('not found: ' + name); + return m[0]; +} +global.esc = (s) => String(s); // identity so we can scan the raw value +global._decodeToolLabelEntities = (s) => s; +eval(grab('_redactToolTargetLabel')); +eval(grab('_transparentToolDetailHtml')); +let buf = ''; +process.stdin.on('data', c => { buf += c; }); +process.stdin.on('end', () => { + const payload = JSON.parse(buf || '{}'); + process.stdout.write(_transparentToolDetailHtml(payload.tc || {}, 'done')); +}); +""" + + +@pytest.fixture(scope="module") +def detail_driver(tmp_path_factory): + if NODE is None: + pytest.skip("node not on PATH") + p = tmp_path_factory.mktemp("detail_driver") / "driver.js" + p.write_text(_DETAIL_DRIVER, encoding="utf-8") + return str(p) + + +@pytest.mark.skipif(NODE is None, reason="node not on PATH") +def test_transparent_full_tab_redacts_secret_in_long_command(detail_driver): + """#4928 gate: now that content args are retained to 4000 chars, the Full-tab + args render must redact secrets past char 120 (it renders tc.args directly).""" + cmd = "echo start\n" + ("x" * 130) + "\nexport OPENAI_API_KEY=sk_LEAKsecret123\necho end" + result = subprocess.run( + [NODE, detail_driver, str(UI_JS_PATH)], + input=json.dumps({"tc": {"name": "shell", "args": {"command": cmd}}}), + capture_output=True, + text=True, + timeout=30, + ) + if result.returncode != 0: + raise RuntimeError(result.stderr) + html = result.stdout + assert "sk_LEAKsecret123" not in html, f"secret leaked into Full-tab args: {html[:600]}" + assert "[redacted]" in html + # The non-secret structure should still be present (full command retained). + assert "echo end" in html