Repository navigation
fix(cli): a printed hint must survive a real SHELL, not just shlex.split - #889
Conversation
Round 2 of card t_c9e1a012. Round 1 (#879) fixed the argparse half and the whitespace half, but decided "safe to print bare" with `shlex.split(value) == [value]`. `shlex.split` is a tokenizer: it models quoting and whitespace and nothing else. Measured against a real /bin/bash on fork/main f8b6d54, 12 of 36 legal repository-key / path tokens still produced an unsafe hint: '$HOME' --thing $HOME -> expanded to /Users/alexgierczyk '`id`' --thing `id` -> EXECUTED id '$(id)' --thing $(id) -> EXECUTED id 'semi;colon' --thing semi;colon -> ran `colon` 'amp&sand' --thing amp&sand -> backgrounded 'amp' 'pipe|line' --thing pipe|line -> piped into `line` 'paren(th)' --thing paren(th) -> bash syntax error 'a&&b' --thing a&&b -> ran `b` 'brace{a,b}' --thing brace{a,b} -> two words '~' --thing ~ -> expanded to $HOME 'a$b','${x}' -> truncated / emptied The printed remedy executing code is strictly worse than the loud `expected one argument` the card was filed for, and the glob shapes (`star*glob`, `q?mark`, `brack[et]`) bind a DIFFERENT value silently, only when a matching name exists in the operator's CWD. The discriminator is now `shlex.quote(value) == value` -- the exact inverse of the question, so it is a shell model rather than a tokenizer. Measured: 0 of 36 unsafe, and plain/with-slash/repo@v2/a+b/under_score keep their readable bare spelling. `~scratch` and `unicode` now quote; `~name` genuinely expands (`~root` -> /var/root, measured), so that is correct rather than conservative. Adds `hint_value()` for a bare positional (`cd <path>`), which `hint_arg` could not express. CLASS SWEEP (AST, not grep; sweeper kept out of the diff). Discriminator: an f-string interpolating a runtime value at an ARGUMENT position of a copy-pasteable command line, where the value can hold a shell metacharacter (a path or a user-named key). 9 live sites fixed, all through the choke point: hermes_cli/kanban_survivor.py repo key (the card, already via hint_arg) hermes_cli/sessions_cmd.py backup path (already via hint_arg) hermes_cli/update_cmd.py:1400 git -C <cwd> status / checkout <branch> hermes_cli/update_cmd.py:8200 cd <PROJECT_ROOT> && git reset --hard [1] hermes_cli/update_cmd.py:8206 cd <PROJECT_ROOT> && git reflog [1] hermes_cli/doctor.py:2679/81 cd <npm_dir> && npm audit fix hermes_cli/setup.py:3350 cp <backup> <config> hermes_cli/main.py:9264/12184 cd <PROJECT_ROOT> tools/self_repo_guard.py:732 git clone --shared <root> <scratch>/<task> cli.py:17983 + main.py:2003 hermes -c "<title>" [2] [1] the rollback-FAILED recovery path, flagged by Argus. Extracted as `_manual_rollback_remedy` out of the ~2800-line `_cmd_update_impl` so the string can be driven by a real shell in a test (AGENTS.md: extract, don't regex around a god-file). [2] a DIFFERENT spelling of the same class: hand-rolled double quotes. They stop word splitting but not expansion, so a session titled `$HOME` or `` `id` `` printed a remedy that expanded or executed. TEST ORACLE REBUILT. Round 1's `_paste()` was `shlex.split` while the implementation also decided safety with `shlex.split` -- the same function on both sides, a tautology that could not fail for any of the above. The paste simulator is now a real `/bin/bash`, run in a CWD seeded with names that MATCH the glob tokens, so the CWD-dependent silent-mangle case is reachable. VERIFIED (.venv/bin/python 3.11.15; module __file__ printed to prove the worktree, not the live tree, was under test): - tests/hermes_cli/test_cli_hint.py 117 passed - the four survivor files (card baseline 118) 118 passed - combined 235 passed - ruff 0.15.20 clean, whole tree - 10 mutations, each restored byte-identical (cmp) and re-run green: M1 revert discriminator to shlex.split -> 26 failed <- the F2 capability M2 hint_arg always bare -> 46 failed M3 hint_arg always quoted (over-fix) -> 7 failed <- readability guard M4 hint_value pass-through -> 24 failed M5 unwire update_cmd rollback -> 2 failed M6 unwire survivor site -> 21 failed M7 unwire sessions site -> 1 failed M9 unwire self_repo_guard clone remedy -> 1 failed M10 unwire main.py resume title -> 2 failed - adjacency re-measured in a clean-room worktree at fork/main f8b6d54: -k "update_cmd or sessions" 6 failed both sides, identical names -k doctor/self_repo_guard 315 passed, 0 failed tests/hermes_cli -k setup/profile/resume/exit_summary 7 failed + 2 errors both sides, identical One test relaxed, not deleted: test_kanban_survivor_stale_bases.py:1056 pinned the literal spelling `--survivor-pr gone1=`. The `owner/repo#N` template contains `#`, which `shlex.quote` escapes, so that assertion pinned a spelling rather than the invariant. Replaced with a round-trip through a real argparse + the module's own `_split_qualifier`. KNOWN GAP, stated rather than papered over: cli.py:17983 is the structural twin of main.py:2003 and is fixed identically, but it sits inside a mixin method that needs a live CLI object, so mutating it alone does NOT turn the suite red. main.py's twin IS gated (M10). Driving the cli.py site would need a CLI harness that does not exist here.
…this PR removed tests/cli/test_exit_summary_resume_hint.py expected `hermes -c "My Cool Session" -p dev` — the hand-rolled DOUBLE-quoted spelling this PR deliberately replaced at cli.py:17984 with `hint_value(session_title)`. shlex.quote emits POSIX single quotes, so the site now prints `hermes -c 'My Cool Session' -p dev` and the stale assertion went red on this PR's own CI (slice 16/16). Updated the expected spelling; the site is untouched — reverting it would re-introduce the $VAR / `cmd` / $(cmd) expansion bug this PR closes. This test is also the GATE the round-2 review believed was missing. The review's M8 mutation (unwire cli.py:17984) re-ran only tests/hermes_cli/test_cli_hint.py, which stayed green at 117 — a different pytest slice. Measured here at 7327d22: fixed, tests/cli/test_exit_summary_resume_hint.py -> 5 passed fixed + test_cli_hint.py -> 122 passed M8a site reverted to f' hermes -c "{title}"...' -> 1 failed, 4 passed M8b site fully unwired to bare {session_title} -> 1 failed, 4 passed M8a, tests/hermes_cli/test_cli_hint.py -> 117 passed (why M8 missed it) cli.py restored, cmp vs pre-mutation copy -> byte-identical Sibling assertions in the file target `hermes --resume <id>`, which hint_value does not touch; the other four tests pass unchanged. Swept the PR's other hint sites' suites (self_repo_guard + 6 survivor files) -> 250 passed. ruff 0.15.10 clean on the changed file. tests/cli full dir: 2 failed on the clean base at 7327d22 -> 1 failed with this diff. The survivor, test_resume_quiet_stderr.py::test_session_not_found_goes_to_stdout_in_full_mode, is INHERITED and order-dependent (passes 4/4 in isolation on both trees, fails in the full-dir run on both).
|
🤖 merged-by: apollo · lane: landing-stack · gate: BYPASS: FleetReview is DISABLED by operator (Ace: systemctl disable --now fleetreview-router 09:30 PT; 'Fleet reviews currently paused'), so no FR record can exist for any head. Landing on kanban APPROVAL + fully green CI + Apollo pre-flight (no open review runs). Ace 18:17 PT: 'regarding the landing stack, 888 and 891, can we just handle that right now'. · why: t_c9e1a012 APPROVED r2 EXECUTION (7327d22) then t_0c5ac29a APPROVED r1 ARTIFACT on the CURRENT head fda1503 (fixed the inherited stale test assertion; DISPROVED the parent's '17984 is ungated' finding by measurement). _qualified_hint printed a --survivor-pr hint argparse rejected as pasted, for repo keys starting with '-'. CI 38/0 CLEAN. Base main; #891 retargets onto main after this. |
|
🤖 merged-by: apollo · lane: fr-pause-0922 · gate: BYPASS: FR PAUSED by Ace ruling 2026-09-22 (state/fleetreview-pause-20260922.md); t_c9e1a012 approved · why: argus APPROVED (printed hint survives real shell); CI green; Apollo merge pass 2026-09-22 |
…sted
hermes_state printed a pasteable recovery command with the DB path
interpolated bare at three sites on the DB-corruption path:
... salvage with `sqlite3 {db_path} ".recover"`.
For a HERMES_HOME holding a space (Google Drive's "My Drive", or the Windows
LOCALAPPDATA default under C:/Users/<First Last>/, where a space is the NORM)
the printed remedy splits:
printed : sqlite3 /Users/x/My Drive/hermes/state.db ".recover"
bash argv: ['sqlite3', '/Users/x/My', 'Drive/hermes/state.db', '.recover']
sqlite3 : exit=1 Error: near "Drive": syntax error
Unreachable remedy on the one path where the operator has least slack. Routed
all three through hermes_cli.cli_hint.hint_value, the choke point #889 landed;
no import cycle (hermes_state.py already hard-imports hermes_cli.sqlite_runtime
at module level, and cli_hint's only dependency is shlex).
hermes_state.py:2521 _persistent_repair_exhausted_error
hermes_state.py:2721 _backup_db_file, low-disk guard
hermes_state.py:2736 _backup_db_file, disk-space-unknown guard
Found by Argus as FINDING F3 in the round-2 review of PR #889 (card
t_b649d6d1), non-blocking and deliberately carded as an orthogonal file.
VERIFIED
- Reproduced first, E2E through the real `hermes sessions repair` path with a
real damaged DB under a spaced HERMES_HOME: broken before, one word after.
- 7 new tests drive the REAL print path (sessions_cmd.cmd_sessions), extract
the backticked span from CAPTURED stdout and hand it to a REAL /bin/bash.
No shlex simulation of the paste — that was the F2 tautology round 1 of the
parent card was blocked for. 7 passed.
- Mutation battery, each applied by a mutator that REFUSES to report a no-op,
each reverted and cmp-verified byte-identical:
M1 revert all 3 sites -> 5 failed
M2 revert site 1 only -> 3 failed
M3 revert site 2 only -> 2 failed
M4 revert site 3 only -> 2 failed
M5 hint_value -> passthrough -> 4 failed
Each single-site mutation reds exactly its own test plus the class guard.
- Adjacency, same 8 files on BOTH trees: 210 passed at PR-889 base, 210 passed
here. Identical.
- ruff 0.15.10 clean; git diff --check clean.
Implementation diff is 4 lines plus one import.
…hell Residual of the #889/#891 unreachable-remedy class. Eight sites across six subsystems interpolated a HERMES_HOME-derived path -- or an interpreter or executable path resolved under one -- bare into a backticked span the operator is told to paste. For a home holding a space (Google Drive's "My Drive", or the Windows C:/Users/<First Last> default where a space is the NORM) the printed remedy does something other than what the message says. Measured before the fix, each through its REAL printer under a real spaced HERMES_HOME, with the printed span handed to a REAL /bin/bash: self_repo_guard git clone --shared '<root>' <scratch>/t_123 -> ['git','clone','--shared','<root>', '/tmp/.../My','Drive/hermes/scratch/t_123'] #889 fixed {root} and left {scratch} bare in the SAME span. whatsapp adapter cd <bridge> && <npm> install -> bash rc=127: /tmp/.../My: No such file or directory (refuses outright; the remedy cannot even start) runtime-parity git -C <TREE> log --oneline HEAD...fork/main -> ['git','-C','/tmp/.../My','Drive/.../hermes-agent', ...] doctor / run.py <sys.executable> -m pip install ... -> ['/tmp/.../My','Drive/venv/bin/python','-m','pip', ...] run.py skills hermes skills install official/My Category/demo-skill -> [...,'official/My','Category/demo-skill'] plugins_cmd x2 hermes plugins install <source> --force --ref <sha> -> [...,'file:///tmp/.../My','Drive/.../demo-plugin', ...] Routed each through hermes_cli.cli_hint.hint_value, the choke point #889 established. Import direction was checked per file; five import hermes_cli directly. staging/scripts/runtime-parity-check.py cannot: it deploys STANDALONE to ~/.hermes/scripts/ and runs under /usr/bin/python3, where hermes_cli is not importable (verified), so it carries a verbatim local copy with a test pinning it to the shared implementation over six token shapes. TESTS drive the REAL print path and judge with a REAL shell; no assertion is on the string's shape, and the paste is never simulated with shlex (the tautology the grandparent card was blocked for). The whatsapp span is a COMPOUND command (`cd X && Y install`), not a word list, so printf would execute rather than lex it -- that oracle instead RUNS the span and has the callee report the argv and cwd bash actually handed it. Verified: tests/test_residual_cli_hint_pasteable.py 12 passed mutation: 10 mutants, 10 KILLED -- each of the 8 sites reverted individually reds its OWN assertion; M10 (always-quote) reds the three readable-form guards, proving the over-fix control bites adjacency: 468 passed across test_self_repo_guard, test_terminal_task_cwd, test_plugins_cmd, test_plugin_install_ref, test_doctor, test_voice_command, test_cli_hint, test_whatsapp_send_message_media staging/tests/test_pending_restart_redrive.py green under /usr/bin/python3, confirming the standalone shim does not break the real deploy path ruff 0.15.20 clean; git diff --check clean Stacked on #889 (hint_value lands there); do not merge before it. Card: t_e587758d
…hell Residual of the #889/#891 unreachable-remedy class. Eight sites across six subsystems interpolated a HERMES_HOME-derived path -- or an interpreter or executable path resolved under one -- bare into a backticked span the operator is told to paste. For a home holding a space (Google Drive's "My Drive", or the Windows C:/Users/<First Last> default where a space is the NORM) the printed remedy does something other than what the message says. Measured before the fix, each through its REAL printer under a real spaced HERMES_HOME, with the printed span handed to a REAL /bin/bash: self_repo_guard git clone --shared '<root>' <scratch>/t_123 -> ['git','clone','--shared','<root>', '/tmp/.../My','Drive/hermes/scratch/t_123'] #889 fixed {root} and left {scratch} bare in the SAME span. whatsapp adapter cd <bridge> && <npm> install -> bash rc=127: /tmp/.../My: No such file or directory (refuses outright; the remedy cannot even start) runtime-parity git -C <TREE> log --oneline HEAD...fork/main -> ['git','-C','/tmp/.../My','Drive/.../hermes-agent', ...] doctor / run.py <sys.executable> -m pip install ... -> ['/tmp/.../My','Drive/venv/bin/python','-m','pip', ...] run.py skills hermes skills install official/My Category/demo-skill -> [...,'official/My','Category/demo-skill'] plugins_cmd x2 hermes plugins install <source> --force --ref <sha> -> [...,'file:///tmp/.../My','Drive/.../demo-plugin', ...] Routed each through hermes_cli.cli_hint.hint_value, the choke point #889 established. Import direction was checked per file; five import hermes_cli directly. staging/scripts/runtime-parity-check.py cannot: it deploys STANDALONE to ~/.hermes/scripts/ and runs under /usr/bin/python3, where hermes_cli is not importable (verified), so it carries a verbatim local copy with a test pinning it to the shared implementation over six token shapes. TESTS drive the REAL print path and judge with a REAL shell; no assertion is on the string's shape, and the paste is never simulated with shlex (the tautology the grandparent card was blocked for). The whatsapp span is a COMPOUND command (`cd X && Y install`), not a word list, so printf would execute rather than lex it -- that oracle instead RUNS the span and has the callee report the argv and cwd bash actually handed it. Verified: tests/test_residual_cli_hint_pasteable.py 12 passed mutation: 10 mutants, 10 KILLED -- each of the 8 sites reverted individually reds its OWN assertion; M10 (always-quote) reds the three readable-form guards, proving the over-fix control bites adjacency: 468 passed across test_self_repo_guard, test_terminal_task_cwd, test_plugins_cmd, test_plugin_install_ref, test_doctor, test_voice_command, test_cli_hint, test_whatsapp_send_message_media staging/tests/test_pending_restart_redrive.py green under /usr/bin/python3, confirming the standalone shim does not break the real deploy path ruff 0.15.20 clean; git diff --check clean Stacked on #889 (hint_value lands there); do not merge before it. Card: t_e587758d
tests/cli/test_exit_summary_resume_hint.py gates cli.py:17984, and its
only fixture title was "My Cool Session" — no apostrophe, so
shlex.quote(t) and a naive hand-rolled f"'{t}'" emit BYTE-IDENTICAL
output for it. The test therefore could not tell correct quoting from
naive quoting, and mutant M9 (naive single quotes at that site) survived
the whole file. M9 is a real defect: real bash REFUSES
`hermes -c 'don't'` with "unexpected EOF while looking for matching `''".
The class is already gated at the twin site (main.py:2003) by
tests/hermes_cli/test_cli_hint.py's `quo'te` parametrization — this was a
missing discriminator, not a missing gate.
Replaced the literal-spelling assertion with a ROUND-TRIP oracle
mirroring the twin's: drive the real print site, hand the printed line to
a real /bin/bash (NUL-delimited), and require the argv back to equal
["hermes", "-c", <title>, "-p", "dev"]. Parametrized over 12 titles
including the apostrophe discriminator, expansion/substitution, control
operators, and globbing (with a hostile_cwd so the globs can actually
hit). A round trip cannot go stale when the site's quoting style changes
— which is exactly what caused t_0c5ac29a.
VERIFIED (worktree at fda1503; .venv/bin/python -P 3.11.15,
PYTHONPATH=<worktree>, HERMES_HOME=/tmp/te484-home, -p no:randomly;
every mutant ast.parse'd VALID-PYTHON, every restore cmp'd
byte-identical):
M0 control -> 16 passed GREEN
M9 naive single -> 1 failed, 15 passed KILLED [quo'te]
M8a pre-#889 double -> 4 failed, 12 passed KILLED
M8b bare/unwired -> 12 failed, 4 passed KILLED
cli.py untouched (git status: test file only). ruff 0.15.10 clean.
tests/cli + tests/hermes_cli/test_cli_hint.py: 1 failed, 1561 passed —
the failure is INHERITED (test_resume_quiet_stderr, 1 failed/1433 passed
on the same worktree stashed clean; passes 4/4 in isolation).
Stacked on #889 (daedalus-opus/t_c9e1a012-hint-shell): hint_value at
cli.py:17984 exists only on that branch.
Reverts e1de835. Fork-PR audit UNRESOLVED ruling (t_63023f77, #1186): the trigger is whitespace in a home/interpreter/skill-category/plugin path; measured 0 occurrences on both fleet hosts, so the hints never needed quoting here. The gateway/run.py hunk conflicted in all 3 upstream syncs. Kept: #889's hermes_cli.cli_hint.hint_value and its callers (KEEP row). gateway/run.py conflict resolved by keeping current main's _skill_slug_index loop and dropping only the hint_value() wrap. Card: t_ceb111f9
Round 2 of card t_c9e1a012. Round 1 (#879) fixed the argparse half and the
whitespace half, but decided "safe to print bare" with
shlex.split(value) == [value].shlex.splitis a tokenizer: it models quoting and whitespace andnothing else. Measured against a real /bin/bash on fork/main f8b6d54,
12 of 36 legal repository-key / path tokens still produced an unsafe hint:
The printed remedy executing code is strictly worse than the loud
expected one argumentthe card was filed for, and the glob shapes(
star*glob,q?mark,brack[et]) bind a DIFFERENT value silently, onlywhen a matching name exists in the operator's CWD.
The discriminator is now
shlex.quote(value) == value-- the exact inverse ofthe question, so it is a shell model rather than a tokenizer. Measured: 0 of 36
unsafe, and plain/with-slash/repo@v2/a+b/under_score keep their readable bare
spelling.
~scratchandunicodenow quote;~namegenuinely expands(
~root-> /var/root, measured), so that is correct rather than conservative.Adds
hint_value()for a bare positional (cd <path>), whichhint_argcould not express.
CLASS SWEEP (AST, not grep; sweeper kept out of the diff). Discriminator: an
f-string interpolating a runtime value at an ARGUMENT position of a
copy-pasteable command line, where the value can hold a shell metacharacter
(a path or a user-named key). 9 live sites fixed, all through the choke point:
hermes_cli/kanban_survivor.py repo key (the card, already via hint_arg)
hermes_cli/sessions_cmd.py backup path (already via hint_arg)
hermes_cli/update_cmd.py:1400 git -C status / checkout
hermes_cli/update_cmd.py:8200 cd <PROJECT_ROOT> && git reset --hard [1]
hermes_cli/update_cmd.py:8206 cd <PROJECT_ROOT> && git reflog [1]
hermes_cli/doctor.py:2679/81 cd <npm_dir> && npm audit fix
hermes_cli/setup.py:3350 cp
hermes_cli/main.py:9264/12184 cd <PROJECT_ROOT>
tools/self_repo_guard.py:732 git clone --shared /
cli.py:17983 + main.py:2003 hermes -c "<title>" [2]
[1] the rollback-FAILED recovery path, flagged by Argus. Extracted as
_manual_rollback_remedyout of the ~2800-line_cmd_update_implsothe string can be driven by a real shell in a test (AGENTS.md: extract,
don't regex around a god-file).
[2] a DIFFERENT spelling of the same class: hand-rolled double quotes. They
stop word splitting but not expansion, so a session titled
$HOMEor`id`printed a remedy that expanded or executed.TEST ORACLE REBUILT. Round 1's
_paste()wasshlex.splitwhile theimplementation also decided safety with
shlex.split-- the same function onboth sides, a tautology that could not fail for any of the above. The paste
simulator is now a real
/bin/bash, run in a CWD seeded with names that MATCHthe glob tokens, so the CWD-dependent silent-mangle case is reachable.
VERIFIED (.venv/bin/python 3.11.15; module file printed to prove the
worktree, not the live tree, was under test):
M1 revert discriminator to shlex.split -> 26 failed <- the F2 capability
M2 hint_arg always bare -> 46 failed
M3 hint_arg always quoted (over-fix) -> 7 failed <- readability guard
M4 hint_value pass-through -> 24 failed
M5 unwire update_cmd rollback -> 2 failed
M6 unwire survivor site -> 21 failed
M7 unwire sessions site -> 1 failed
M9 unwire self_repo_guard clone remedy -> 1 failed
M10 unwire main.py resume title -> 2 failed
-k "update_cmd or sessions" 6 failed both sides, identical names
-k doctor/self_repo_guard 315 passed, 0 failed
tests/hermes_cli -k setup/profile/resume/exit_summary
7 failed + 2 errors both sides, identical
One test relaxed, not deleted: test_kanban_survivor_stale_bases.py:1056
pinned the literal spelling
--survivor-pr gone1=. Theowner/repo#Ntemplate contains
#, whichshlex.quoteescapes, so that assertion pinned aspelling rather than the invariant. Replaced with a round-trip through a real
argparse + the module's own
_split_qualifier.KNOWN GAP, stated rather than papered over: cli.py:17983 is the structural
twin of main.py:2003 and is fixed identically, but it sits inside a mixin
method that needs a live CLI object, so mutating it alone does NOT turn the
suite red. main.py's twin IS gated (M10). Driving the cli.py site would need a
CLI harness that does not exist here.
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.