diff --git a/CHANGELOG.md b/CHANGELOG.md index df8ce2ed..611f9d51 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,12 @@ ## [Unreleased] +## [v0.51.657] — 2026-06-25 — Release XM (expanded shell tool cards show the full command) + +### Fixed + +- **Expanding a shell/terminal tool card now shows the full multi-line command, not just the first line.** In the transparent activity stream, a shell card's collapsed header correctly shows only the first line (compact metadata), but the expanded card reused that same first-line-only value — so a multi-line script still rendered as line 1 when you expanded it. The expanded detail lead now shows the complete command. As part of this, command redaction was broadened to mask common secret forms (env/colon assignments for `*TOKEN`/`*API_KEY`/`*SECRET`/`*ACCESS_KEY`/`*PRIVATE_KEY`/`*CREDENTIAL`/`*CLIENT_SECRET`/`*SESSION_KEY`/`*AUTH*`, `--token`/`--api-key`/`--secret`-style flags, `Authorization:` headers, and secret-looking URL query params) across the whole command, so exposing lines beyond the first never leaks a key. (#4926, part of #4925) + ## [v0.51.656] — 2026-06-25 — Release XL (stream writeback timing diagnostics) ### Fixed diff --git a/static/ui.js b/static/ui.js index c7b9eec6..7402cb89 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(/(^|[\s;|(])([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$2[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(/(^|[\s;|(])([A-Za-z0-9_]*AUTH[A-Za-z0-9_]*\s*=\s*)(?:"[^"]*"|'[^']*'|\S+)/gi,'$1$2[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(); @@ -12586,6 +12602,15 @@ function _toolTargetLabel(tc){ else raw=a.cmd||a.command||a.path||a.file_path||a.file||a.uri||a.url||a.query||a.pattern||a.dir||a.task||a.name||''; return _redactToolTargetLabel(_decodeToolLabelEntities(String(raw).split('\n')[0].trim())); } +function _toolFullCommandLabel(tc){ + // Full (multi-line) shell command for the EXPANDED detail lead. Mirrors the + // shell raw-extraction in _toolTargetLabel but WITHOUT the .split('\n')[0] + // first-line collapse, so a multi-line script shows every line when the card + // is expanded (#4926). Redaction + entity-decode still applied to the whole. + const a=tc&&tc.args||{}; + const raw=a.cmd||a.command||tc.command||tc.raw_command||tc.original_command||tc.display_command||''; + return _redactToolTargetLabel(_decodeToolLabelEntities(String(raw).replace(/\s+$/,''))); +} function _toolVisibleTargetLabel(tc, opts){ opts=opts||{}; const target=_toolTargetLabel(tc); @@ -12931,8 +12956,14 @@ function _toolDetailLeadLabel(kind){ } function _toolDetailLeadText(kind, tc){ const target=_toolTargetLabel(tc); + if(kind==='shell'){ + // Expanded card shows the FULL multi-line command, not just the header's + // first line (#4926). Fall back to the first-line target if full is empty. + const full=_toolFullCommandLabel(tc); + const cmd=full||target; + return cmd?`$ ${cmd}`:''; + } if(!target) return ''; - if(kind==='shell') return `$ ${target}`; return target; } function buildToolCard(tc){ diff --git a/tests/test_issue4926_shell_full_command.py b/tests/test_issue4926_shell_full_command.py new file mode 100644 index 00000000..c213457f --- /dev/null +++ b/tests/test_issue4926_shell_full_command.py @@ -0,0 +1,173 @@ +"""Regression coverage for #4926 — the EXPANDED shell tool card must show the +full multi-line command, not just the first line. + +In the transparent activity stream, a shell/terminal tool card's collapsed +header correctly shows only the first line of the command (compact metadata), +but the EXPANDED card's detail lead (`$ `) was reusing that same +first-line-only value, so a multi-line script still rendered as line 1 when the +user expanded it. The full command is available frontend-side; the truncation +was purely in the label derivation. The fix keeps the collapsed header on the +first line but gives the expanded detail lead the full command. + +Drives the real `static/ui.js` helpers under node (no server). +""" +from __future__ import annotations + +import json +import shutil +import subprocess +from pathlib import Path + +import pytest + +REPO_ROOT = Path(__file__).parent.parent.resolve() +UI_JS_PATH = REPO_ROOT / "static" / "ui.js" +NODE = shutil.which("node") + +pytestmark = pytest.mark.skipif(NODE is None, reason="node not on PATH") + +# 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'); +function grab(name){ + const re = new RegExp('function ' + name + '\\([^]*?\\n}', 'm'); + const m = src.match(re); + if (!m) throw new Error('function not found: ' + name); + return m[0]; +} +global._decodeToolLabelEntities = (s) => s; +global._toolActionKind = (tc) => 'shell'; +eval(grab('_redactToolTargetLabel')); // REAL redactor +eval(grab('_toolTargetLabel')); +eval(grab('_toolFullCommandLabel')); +eval(grab('_toolDetailLeadText')); +let buf = ''; +process.stdin.on('data', c => { buf += c; }); +process.stdin.on('end', () => { + const payload = JSON.parse(buf || '{}'); + const tc = payload.tc || {}; + process.stdout.write(JSON.stringify({ + header: _toolTargetLabel(tc), + lead: _toolDetailLeadText('shell', tc), + })); +}); +""" + + +@pytest.fixture(scope="module") +def driver_path(tmp_path_factory): + p = tmp_path_factory.mktemp("shell_fullcmd_driver") / "driver.js" + p.write_text(_DRIVER_SRC, encoding="utf-8") + return str(p) + + +def _run(driver_path: str, tc: dict) -> dict: + assert NODE is not None + result = subprocess.run( + [NODE, driver_path, str(UI_JS_PATH)], + input=json.dumps({"tc": tc}), + capture_output=True, + text=True, + timeout=30, + ) + if result.returncode != 0: + raise RuntimeError(result.stderr) + return json.loads(result.stdout) + + +_MULTILINE = ( + "cd ~/hermes-webui-public\n" + "git fetch origin --tags -q\n" + "for t in a b c; do echo $t; done\n" + "echo done" +) + + +def test_expanded_shell_lead_shows_full_multiline_command(driver_path): + """#4926 — the expanded detail lead must contain every line of the command.""" + out = _run(driver_path, {"args": {"command": _MULTILINE}}) + lead = out["lead"] + assert lead.startswith("$ "), lead + # Every line of the original command must be present in the expanded lead. + for line in _MULTILINE.split("\n"): + assert line in lead, f"expanded lead dropped line {line!r}: {lead!r}" + # And it must actually be multi-line (the bug rendered it as one line). + assert lead.count("\n") == 3, f"expected 4 lines in expanded lead, got: {lead!r}" + + +def test_collapsed_header_stays_first_line_only(driver_path): + """The compact header should remain the first line (unchanged behavior).""" + out = _run(driver_path, {"args": {"command": _MULTILINE}}) + assert out["header"] == "cd ~/hermes-webui-public", out["header"] + assert "\n" not in out["header"] + + +def test_single_line_command_unchanged(driver_path): + """A single-line command is identical in header and (less the `$ `) lead.""" + out = _run(driver_path, {"args": {"command": "ls -la /tmp"}}) + assert out["header"] == "ls -la /tmp" + assert out["lead"] == "$ ls -la /tmp" + + +def test_command_from_top_level_field(driver_path): + """Full-command accessor also reads tc.command (not just tc.args.command).""" + 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"] + + +def test_url_query_redaction_preserves_command_tail(driver_path): + """#4926 gate: redacting a secret URL query param must NOT swallow the rest + of the URL (later params + closing quote). Regression for the env-assignment + regex over-matching `token=` inside a URL and eating the tail with \\S+.""" + out = _run(driver_path, {"args": {"command": "curl 'https://x.com/cb?token=tok_leak&mode=debug&signature=sig_leak'"}}) + lead = out["lead"] + assert "tok_leak" not in lead and "sig_leak" not in lead + # The non-secret param + closing quote must survive (no tail truncation). + assert "mode=debug" in lead + assert lead.rstrip().endswith("'") + + +def test_benign_assignment_not_over_redacted(driver_path): + """A non-secret assignment like PATH= must not be mangled.""" + out = _run(driver_path, {"args": {"command": "export PATH=/usr/bin:/bin\necho ok"}}) + assert "/usr/bin:/bin" in out["lead"] + assert "echo ok" in out["lead"]