Repository navigation
fix(analyzer): keep checking command lines inside a word that spans lines - #814
elliottwaves-20 wants to merge 2 commits into
Conversation
…ines
A quote in a heredoc body, a shell comment or Markdown prose can pair with
a quote on a later command line. The command-word scan then parsed one word
across those lines and skipped every candidate inside it, so a runtime
command such as
cat <<E
he said "hi
E
$CMD"" -rf / # "
was reported as completely inspected.
A word that spans a line break now opens a shadow region instead of moving
the parse frontier. Candidates in that region are still checked at command
positions (line start or after ; | & (), but they never own quotes, never
move the frontier, and an unresolved parse there is not charged to the
budget. Python sources keep their AST-proven frontier. Later candidates are
the same as on main, so the result is never less strict than main.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: elliottwaves-20 <pail1217@web.de>
rng1995
left a comment
There was a problem hiding this comment.
[SkillSpector Review]
@elliottwaves-20 thank you for the clear reproducers in #813, and for designing the shadow region so that it can only add checks. The "no file moves from partial to complete" property holds up when I read the code.
Value and readiness
This is a real scan-integrity bug. On main, a quote in a heredoc body, a comment or prose can pair across lines and hide $CMD"" -rf /, and the file is reported COMPLETED. I confirmed the #813 reproducers on main. The design is sound and monotonic:
- The spanning word now sets
shadow_throughinstead ofparsed_through. - Shadowed candidates never change ownership or the frontier.
- Candidates after the region are exactly the ones main checks.
However, the new command-position predicate still lets the same hiding happen whenever the runtime command follows a common prefix word, so the PR only partly achieves its stated outcome.
Material findings
-
Blocker:
src/skillspector/nodes/analyzers/static_patterns_tool_misuse.py:522(_is_shell_command_position). Inside a shadow region, only line starts and positions after;,|,&or(are checked.The candidates that matter here are
$-led runtime commands, and those often follow a prefix word:sudo $CMD …,then $CMD …,do/else $CMD …,{ $CMD …; },! $CMD,exec $CMD,time $CMD. Outside a region, main checks them, because_is_shell_command_word_startaccepts any candidate preceded by whitespace.On main, each of
sudo $CMD"" -rf / # ",if true; then $CMD"" -rf / # "and{ $CMD"" -rf / # "is partial on its own. Each becomes complete when preceded bycat <<E/he said "hi/E. In this PR, the$aftersudo,thenor{is not a command position, so the shadowed candidate is skipped and the result stays COMPLETE. Prefixing the #813 reproducer withsudobrings the hiding back.Expected: also count a position as a command position when the preceding word is one of:
- the clause prefixes this module already defines (
_SHELL_CLAUSE_PREFIX_WORDS), plusif/while/until {or a backtick- the wrappers it models (
sudo,env,command,nohup,nice,timeout,xargs,exec)
Alternatively, check
$-led shadowed candidates with_is_shell_command_word_start. Please add thesudo,thenand{variants totest_quote_spanning_lines_cannot_hide_a_later_command. - the clause prefixes this module already defines (
PIC tradeoffs
- The corpus run moves 4 Markdown files from complete to partial, where a prose quote pairs into a later block. That is the intended conservative cost. Widening the predicate per finding 1 may add a few more.
Verification and gaps
- Read the diff against
_has_shell_command_word_exhaustion,_is_shell_command_word_startand_parse_shell_command_wordon main. I checked that the candidates and owned positions after a region match main. Shadowed candidates can only return True (through the limited, printf or runtime-operand paths), so the change cannot make a file complete. - The baseline probes above ran on trusted main code only. I did not run the PR's code or tests.
static_patterns_tool_misuse.pyon main has not changed since this PR's base (5edc347), so the green CI (6/6) on 011c792 is current. The PR merges cleanly with main and with #779, #783, #790 and #801.- I checked the parse-work bound only by reading. The widened predicate should still keep
test_json_owned_runtime_bounds.pypassing.
| cursor = start - 1 | ||
| while cursor >= 0 and content[cursor] in " \t": | ||
| cursor -= 1 | ||
| return cursor < 0 or content[cursor] in "\n\r;|&(" |
There was a problem hiding this comment.
Inside a shadow region, a runtime command after a prefix word is still hidden: sudo $CMD"" -rf / # ", if true; then $CMD"" -rf / # " and { $CMD"" -rf / # " are each partial on main when standalone. Each becomes complete after cat <<E / he said "hi / E, and here the $ after sudo /then /{ is not a command position, so it stays complete. Please also accept positions after _SHELL_CLAUSE_PREFIX_WORDS, if/while/until, {, a backtick, and the wrappers this module models (sudo, env, command, nohup, nice, timeout, xargs, exec). Alternatively, check $-led shadowed candidates with _is_shell_command_word_start. Please add these variants to the test.
There was a problem hiding this comment.
Fixed in 8dd0d6a. Shadowed candidates that open a runtime word ($, "$, or a single backtick) keep main's word-start check, which also covers sudo -u root $CMD, env A=1 $CMD and xargs -0 $CMD. The variants are in test_quote_spanning_lines_cannot_hide_a_later_command. Details in the PR comment.
…gion
Inside a shadow region only command positions (line start, ;, |, &, ()
were checked, so a runtime command after a prefix word stayed hidden:
sudo $CMD"" -rf / # ", then $CMD, { $CMD, env A=1 $CMD, sudo -u root $CMD,
and the backtick form sudo `...`. Each is partial on main when it stands
alone and became complete after a heredoc whose prose quote pairs across
lines.
A shadowed candidate that opens a runtime word ($, "$, or a single
backtick, bare or after ") now keeps main's word-start check. A run of
backticks such as a Markdown fence never opens a substitution, so the
parse-work bound in test_json_owned_runtime_bounds.py still holds.
Refs NVIDIA#813
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: elliottwaves-20 <pail1217@web.de>
|
@rng1995 I reproduced the prefix-word cases on this branch and fixed them in 8dd0d6a. I took your second option. A candidate inside a shadow region that opens a runtime word now skips the command-position filter and keeps main's Backticks are included because
Verification (Windows):
🤖 Generated with Claude Code |
Fixes #813.
Problem
A double quote in a heredoc body, a shell comment or Markdown prose can pair with a quote on a later command line.
_has_shell_command_word_exhaustionthen parses one word across those lines and movesparsed_throughpast it. Every candidate inside the word is skipped, including a runtime-selected command such as$CMD"" -rf /, and the file is reported as completely inspected. The reproducers are in #813.Change
The fix does not try to prove shell quote state. It only stops a multi-line word from hiding later command lines.
shadow_through) instead of movingparsed_through.$…,"$…, or a single backtick, bare or after"; helper_opens_runtime_word) is checked wherevermainchecks it, because such a command can follow any prefix word:sudo -u root $CMD,then $CMD,{ $CMD,env A=1 $CMD.;,|,&or((helper_is_shell_command_position).test_json_owned_runtime_bounds.pystill passes.parsed_throughorshadow_through. An unresolved parse inside the region is not charged to the command-word budget, because the spanning word already closed that region's quotes.python_source is not None) keep the current frontier, because the AST already proves their string and comment bounds.The candidates after the region are therefore the same as on
main, and the region's candidates can only add checks. No file can move from partial to complete.Tests
tests/nodes/analyzers/test_multiline_word_shadow.py:test_quote_spanning_lines_cannot_hide_a_later_command: comment, heredoc with plain and quoted delimiter,${X},${X}" ",$()"e", a backtick variant, lone-CR line endings, and Markdown prose into a fence, and runtime commands after the prefix wordssudo,sudo -u root,then,{,!,env A=1andxargs -0(with$CMD,"$CMD"and backtick forms). Each runs short and padded. All fail onmain.test_heredoc_quote_cannot_hide_a_runtime_rm_at_scan_level: therun.shfrom Quote in a heredoc body, comment or prose hides a later command line from the command-word scan #813 throughrun_static_patterns_with_ledger, expected to be notCOMPLETED.echo "…$HOME"stays complete, a dynamic multi-line$(…)word behaves as onmain, and Python sources keep their frontier.Verification against main 5edc347
Full suite: the set of failing tests is identical on
mainand on this branch. Those failures are Windows/environment-specific on my machine (release, git and CLI tests).The corpus and fuzzing figures below were measured on the first head (011c792). The review fix only lets more shadowed candidates be checked, so it can only add partial results.
Corpus of about 3,600 skill files: no file moves from partial to complete, and no file loses a TM1 finding. 4 files move from complete to partial (3 distinct). All are Markdown where a prose quote pairs across lines into a later block, so
mainskipped those lines.Differential fuzzing, 100k generated shell snippets with heredoc, comment and adjacent-quote shapes: 0 cases where
mainis partial and this branch is complete.🤖 Generated with Claude Code