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.
This commit is contained in:
1 parent
bd6e57fb42
commit
f79c975842
2 files changed
+89
-4
No files matched your search
+35
-4
@@ -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])=>`<div class="tool-arg-pair"><span class="tool-arg-key">${esc(String(k))}</span><span class="tool-arg-val">${esc(typeof v==='string'?v:JSON.stringify(v,null,2))}</span></div>`).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 `<div class="tool-arg-pair"><span class="tool-arg-key">${esc(String(k))}</span><span class="tool-arg-val">${esc(sv)}</span></div>`;
|
||||
}).join('');
|
||||
return `<div class="tool-card-detail" data-transparent-detail-mode="full"><div class="transparent-detail-modes" role="tablist"><span class="transparent-detail-mode active" role="tab" tabindex="0" data-mode="full" onclick="_setTransparentDetailMode(this,'full')">Full</span><span class="transparent-detail-mode" role="tab" tabindex="0" data-mode="output" onclick="_setTransparentDetailMode(this,'output')">Output</span></div><div class="tool-card-args">${argHtml}</div>${preview?`<div class="tool-card-result"><pre>${esc(preview)}</pre></div>`:''}</div>`;
|
||||
}
|
||||
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?`<div class="tool-card-detail">
|
||||
${detailLead}
|
||||
${visibleArgs.length?`<div class="tool-card-args">${
|
||||
visibleArgs.map(([k,v])=>`<div class="tool-arg-pair"><span class="tool-arg-key">${esc(k)}</span><span class="tool-arg-val">${esc(String(v))}</span></div>`).join('')
|
||||
visibleArgs.map(([k,v])=>{
|
||||
let sv=String(v);
|
||||
if(typeof _redactToolTargetLabel==='function'){ try{ sv=_redactToolTargetLabel(sv); }catch(e){} }
|
||||
return `<div class="tool-arg-pair"><span class="tool-arg-key">${esc(k)}</span><span class="tool-arg-val">${esc(sv)}</span></div>`;
|
||||
}).join('')
|
||||
}</div>`:''}
|
||||
${displaySnippet?`<div class="tool-card-result">
|
||||
<pre>${tc.is_diff||_snippetLooksLikeDiff(displaySnippet)?`<code class="diff-block" data-highlighted="1">${_colorDiffLines(displaySnippet)}</code>`:esc(displaySnippet)}</pre>
|
||||
|
||||
@@ -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
|
||||
Reference in new issue
Block a user