From ed21bd20cd58aa2c7caa8378cdbfb716534524ce Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Thu, 25 Jun 2026 17:52:41 +0000 Subject: [PATCH] fix(#4926): broaden _redactToolTargetLabel to mask env/flag/header secrets in the full command Codex gate: exposing the full multi-line command in the expanded shell card newly surfaces secrets on non-first lines (the collapsed header only ever showed line 1). The old redactor masked only password-shaped fields. Broaden it to mask common secret forms across the WHOLE command: env/colon assignments for *TOKEN/*API_KEY/*SECRET/*ACCESS_KEY/*PRIVATE_KEY/*CREDENTIAL/*CLIENT_SECRET/ *SESSION_KEY and *AUTH*=, --token/--api-key/--secret style flags, Authorization: Bearer/Bot/Token headers, and secret-looking URL query params. Test now drives the REAL redactor (not an identity stub) and asserts non-first-line secrets are masked while the collapsed header stays first-line. --- static/ui.js | 18 ++++++++- tests/test_issue4926_shell_full_command.py | 44 +++++++++++++++++++--- 2 files changed, 56 insertions(+), 6 deletions(-) diff --git a/static/ui.js b/static/ui.js index 90916dd7..0eed0295 100644 --- a/static/ui.js +++ b/static/ui.js @@ -12520,7 +12520,23 @@ function _redactToolTargetLabel(value){ return String(value||'') .replace(/\bsshpass\s+-p\s+(?:"[^"]*"|'[^']*'|\S+)/gi,'sshpass -p "[redacted]"') .replace(/(--password(?:=|\s+))(?:"[^"]*"|'[^']*'|\S+)/gi,'$1[redacted]') - .replace(/(password(?:=|\s+))(?:"[^"]*"|'[^']*'|\S+)/gi,'$1[redacted]'); + .replace(/(password(?:=|\s+))(?:"[^"]*"|'[^']*'|\S+)/gi,'$1[redacted]') + // Env-assignment / flag secrets, masked across the full (multi-line) text so + // the expanded shell card can't leak a key on a non-first line (#4926). Keys + // matched case-insensitively: *(TOKEN|API_KEY|APIKEY|SECRET|PASSWD|PASSWORD| + // ACCESS_KEY|PRIVATE_KEY|AUTH|CREDENTIAL|SESSION_KEY|CLIENT_SECRET)*. + .replace(/(\b[A-Za-z0-9_]*(?:TOKEN|API[_-]?KEY|SECRET|PASSWD|ACCESS[_-]?KEY|PRIVATE[_-]?KEY|CREDENTIALS?|CLIENT[_-]?SECRET|SESSION[_-]?KEY)[A-Za-z0-9_]*\s*=\s*)(?:"[^"]*"|'[^']*'|\S+)/gi,'$1[redacted]') + // AUTH-family env assignment, but only the `=` form (the `Authorization:` + // header colon form is handled separately below, and must not be eaten here). + .replace(/(\b[A-Za-z0-9_]*AUTH[A-Za-z0-9_]*\s*=\s*)(?:"[^"]*"|'[^']*'|\S+)/gi,'$1[redacted]') + // --token / --api-key / --secret style flags. + .replace(/(--(?:token|api[_-]?key|secret|access[_-]?key|client[_-]?secret|auth[_-]?token)(?:=|\s+))(?:"[^"]*"|'[^']*'|\S+)/gi,'$1[redacted]') + // Authorization: Bearer/Bot/Token (header or curl -H form): + // redact everything after the scheme keyword up to the closing quote/space. + .replace(/(authorization\s*:?\s*(?:bearer|bot|token)\s+)(?:"[^"]*"|'[^']*'|[^\s'"]+)/gi,'$1[redacted]') + .replace(/((?:authorization|x-api-key)\s*:\s+)(?:"[^"]*"|'[^']*'|[^\s'"]{12,})/gi,'$1[redacted]') + // Secret-looking URL query params (?token=... &api_key=... &access_token=...). + .replace(/([?&](?:token|api[_-]?key|access[_-]?token|secret|sig|signature|key)=)(?:[^&\s"']+)/gi,'$1[redacted]'); } function _shortToolLabel(value, limit){ const text=String(value||'').replace(/\s+/g,' ').trim(); diff --git a/tests/test_issue4926_shell_full_command.py b/tests/test_issue4926_shell_full_command.py index cbdb938e..0b7b7809 100644 --- a/tests/test_issue4926_shell_full_command.py +++ b/tests/test_issue4926_shell_full_command.py @@ -26,9 +26,9 @@ NODE = shutil.which("node") pytestmark = pytest.mark.skipif(NODE is None, reason="node not on PATH") -# Extract the three helper functions by name and eval them with light stubs for -# their dependencies (redaction / entity-decode / action-kind), so the test is -# resilient to the rest of ui.js not being importable headlessly. +# Extract the helper functions by name and eval them. We use the REAL +# _redactToolTargetLabel (not a stub) so the redaction invariant is actually +# exercised; only _decodeToolLabelEntities / _toolActionKind are stubbed. _DRIVER_SRC = r""" const fs = require('fs'); const src = fs.readFileSync(process.argv[2], 'utf8'); @@ -38,10 +38,9 @@ function grab(name){ if (!m) throw new Error('function not found: ' + name); return m[0]; } -// Stubs for dependencies of the grabbed functions. -global._redactToolTargetLabel = (s) => s; global._decodeToolLabelEntities = (s) => s; global._toolActionKind = (tc) => 'shell'; +eval(grab('_redactToolTargetLabel')); // REAL redactor eval(grab('_toolTargetLabel')); eval(grab('_toolFullCommandLabel')); eval(grab('_toolDetailLeadText')); @@ -118,3 +117,38 @@ def test_command_from_top_level_field(driver_path): out = _run(driver_path, {"command": "echo one\necho two"}) assert "echo one" in out["lead"] and "echo two" in out["lead"] assert out["lead"].count("\n") == 1 + + +def test_secret_on_non_first_line_is_redacted(driver_path): + """#4926 security: exposing the full command must NOT leak a secret that + sits on a non-first line (the expanded card previously showed line 1 only, + so non-first-line secrets were never rendered). The real redactor masks + common env / flag / header secret forms across the whole command.""" + cmd = ( + "cd /app\n" + "export OPENAI_API_KEY=skSECRETvalue12345\n" + "export AUTH_TOKEN=tokSECRET987654\n" + "curl -H 'Authorization: Bearer bearerSECRETxyz' https://api.example.com\n" + "node app.js --client-secret hunterSECRET2" + ) + out = _run(driver_path, {"args": {"command": cmd}}) + lead = out["lead"] + # Structural lines preserved (it's still the full multi-line command). + assert "cd /app" in lead and "node app.js" in lead + # Secret VALUES must be gone (none of these should survive). + for secret in ( + "skSECRETvalue12345", + "tokSECRET987654", + "bearerSECRETxyz", + "hunterSECRET2", + ): + assert secret not in lead, f"secret leaked into expanded shell lead: {secret!r}\n{lead}" + assert "[redacted]" in lead + + +def test_password_redaction_still_works(driver_path): + """The pre-existing password/sshpass redaction is unchanged.""" + out = _run(driver_path, {"args": {"command": "sshpass -p hunter2 ssh host\nmysql --password=topsecret"}}) + assert "hunter2" not in out["lead"] + assert "topsecret" not in out["lead"] + assert "[redacted]" in out["lead"]