Skip to content

fix(markdown): preserve bounded inline and list-fence ownership - #950

Merged
rng1995 merged 2 commits into
NVIDIA:mainfrom
kshitijrajes:fix/markdown-list-fence-ownership
Nov 2, 2026
Merged

rng1995 merged 2 commits into
NVIDIA:mainfrom
kshitijrajes:fix/markdown-list-fence-ownership

Conversation

@kshitijrajes

@kshitijrajes kshitijrajes commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

Problem

Two valid Markdown ownership cases could exhaust static parsing:

  • complete inline-code spans before prose possessives were treated as unresolved shell quoting
  • fenced code nested at valid list-container-relative indentation was not recognized

Fix

  • reuse bounded inline-delimiter pairing for same-line prose classification
  • accept fences only at CommonMark's 0-3 columns relative to a proven list container (top-level and nested, keeping the typed-fence label and literal-apostrophe state)
  • keep inline and fenced payloads visible to security rules
  • keep incomplete spans and malformed deep indentation fail-closed
  • runtime-command guard: an inert list marker (-, +, 1., 1)) owns only the initial command position. Any unquoted control operator before the span, between spans, or inside the span retains the guard. A leading * is a glob and never owns it.

Verification

  • The four analyzer test modules pass (552 tests), including regressions for the wrapped $CMD case, in-span and outside-span control operators, multiple spans, the * marker, the 3/4-column fence boundary, and nested typed fences.
  • Differential check against current main over 3,220 generated list-item variants: no case moves from COMPLETED to PARTIAL in the wrong direction, and every variant with a control operator stays PARTIAL / STATIC_PARSE_LIMIT.
  • CLI-based tests need click < 8.2 output semantics locally; the remaining local failures are the same stderr-in-output failures present on main.
  • Ruff check and format pass.
  • Rebased onto current main, including fix(static): stop backticks/apostrophes in comments and prose from tripping static_parse_limit #902 and fix(scanner): tighten bounded parser ownership #947.

@rng1995 rng1995 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[SkillSpector Review]

Value and readiness

The list-container fence handling and inline-code ownership changes address real Markdown false positives. Blanking complete inline spans for prose classification creates a new gap in the runtime-command guard, so this head needs a focused correction.

Material findings

Blocker — src/skillspector/nodes/analyzers/static_patterns_tool_misuse.py:5311: the new prose view removes all bytes inside a complete inline span, including the shell syntax that previously prevented apostrophe masking. Adapt the existing negative control test_possessive_cannot_hide_runtime_command_operands by putting backticks around its $CMD:

`$CMD` reads each reference's
probe' -rf /

Follow this with the same _OBSERVATIONS tail used by that test. The shell projection keeps $CMD while dropping its backtick delimiters. The classification view now lacks $, schedules the possessive apostrophe for masking, and the final ownership pass only parses the $CMD word to its following space, so it does not protect that later apostrophe. Removing the apostrophe makes the first newline terminate the runtime command; the later -rf / no longer belongs to it, and the prose retry can clear the incomplete-coverage result. The original negative control retains this signal. Keep shell-syntax protection for inline content that can still supply a runtime command, or establish command/operand ownership before changing its later quotes. Add this wrapped version of the existing regression. The new positive inline-command test keeps the destructive operands inside the span and does not cover this split.

PIC tradeoffs

Prose punctuation should not create spurious shell quotes, but the classification and executable projections must agree about runtime syntax before a quote can be removed. Complete Markdown delimiter pairing alone does not prove that the remainder is independent prose.

Verification and gaps

Reviewed exact head 092b33f09622e4f72372780199b828b98c2b4667: full three-file diff, pinned surrounding implementation/tests, description and available review/comment history. Read the new list/fence controls and the existing runtime-command possessive control; independently corroborated the full source trace through span blanking, apostrophe collection, ownership parsing, operand tokenization and the retry return. No hosted checks are available on this exact head. Author-reported baseline and focused test results were read without rerunning them. Static review only: no contributor code, tests, imports, builds, scanner invocations, supplied snippets or regex probes were executed.

@kshitijrajes
kshitijrajes force-pushed the fix/markdown-list-fence-ownership branch from 092b33f to 5bcfc50 Compare October 30, 2026 18:47

@rng1995 rng1995 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review of head 5bcfc50 (static read only; I did not run the tests). The earlier blocker (wrapped $CMD) looks fixed by inline_runtime_guard plus its regression test. One test gap remains before I'd approve; not approving yet.

@kshitijrajes
kshitijrajes force-pushed the fix/markdown-list-fence-ownership branch from 5bcfc50 to 7db6766 Compare October 31, 2026 10:13

@rng1995 rng1995 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rechecked head 7db6766c23f031c120b53bb73d01722ba86ab6cc. The requested list-item regression and 3/4-column fence boundary cases are present. The plain list-item COMPLETED rationale holds for that exact test, and independent nested-list boundary probes preserve the payload.

One blocker remains in the same guard exemption: an inline shell control operator can put $CMD back in command position after the Markdown marker. The attached finding was independently reproduced through the public static-runner interface against base and head: base reports PARTIAL / STATIC_PARSE_LIMIT; this head reports COMPLETED with no findings.

Validation: both changed test modules pass (435 tests); changed-file Ruff check/format pass; all six hosted checks pass on this exact head. The passing tests omit the reported control-operator variant. This review does not establish a combined scanner verdict or a full local repository-suite result.

@kshitijrajes
kshitijrajes force-pushed the fix/markdown-list-fence-ownership branch from 7db6766 to 9a07020 Compare October 31, 2026 14:01

@rng1995 rng1995 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[SkillSpector Review]

Value and readiness

The wrapped $CMD regression is now covered, and the new guard preserves runtime-command ownership for the reported control operators inside inline code. The list-fence change retains payloads and has the requested three/four-column boundary controls. The same ownership gap remains when the operator is immediately outside the inline span, so this head still needs a focused correction.

Material findings

Blocker — src/skillspector/nodes/analyzers/static_patterns_tool_misuse.py:5189-5201: account for the clause before each inline span. The list-marker exemption considers _shell_clause_starts(body) only. Move the semicolon from the last reported reproducer to immediately before the inline span:

- ; `$CMD` reads each reference's
probe' -rf /

Append the existing _OBSERVATIONS tail from test_prose_shell_context.py. The span body is only $CMD, so it has no additional clause; list_marker_owns_initial_command=True suppresses the guard despite the preceding semicolon. The classification view blanks $CMD, and its remaining semicolon is permitted by _MARKDOWN_PROSE_SHELL_SYNTAX_RE, allowing the possessive apostrophe to be masked at lines 5428-5455. The security projection retains the semicolon and $CMD, which is now a separate command after the list marker. Before masking, the paired apostrophes join the second line and its destructive operands to that runtime command. After masking, _bounded_shell_tokens ends it at the first newline; the quote following probe is not a new command-word candidate because it follows an ordinary letter. Consequently the prose retry can clear the original incomplete-coverage result. This is the same loss as the prior blocker with only the Markdown delimiter boundary moved; the new parameterized controls keep every operator inside the span and miss it. Determine ownership using the projected clause prefix, or conservatively retain the guard if an earlier unquoted operator can put this span in command position. Add the outside-span variant and a multiple-span variant as regressions requiring PARTIAL / STATIC_PARSE_LIMIT.

PIC tradeoffs

The proven list marker can justify treating the first inline command-looking word as an argument, but that proof ends at a shell control operator regardless of its Markdown ownership. Keep the precision improvement while preserving coverage when executable clauses cross inline boundaries.

Verification and gaps

Reviewed pinned head 9a07020d9842b9881bf5419dd02d6674f93be0e6: complete three-file diff, surrounding delimiter/list/fence projection, shell-clause and word parsing, operand tokenization, retry logic, and relevant positive/negative tests. Rechecked the original wrapped-command blocker, the later list-marker control-operator blocker, all review threads and author replies, and both resolved test requests. All six hosted checks succeeded on this head. The hosted unit log reports 16,278 passed, 24 skipped, 258 deselected and 4 expected failures, including both changed test modules; the outside-span case is absent. The finding above is established by static source tracing, without a runtime reproduction or a claim about a combined scanner verdict. No contributor code, tests, imports, builds, scanner invocations or supplied snippets were executed.

@rng1995 rng1995 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed head 9a07020 (static read + green CI). Both of my comments are addressed: the list-marker guard exemption now has positive and control-operator negative tests, and the 3/4-column fence boundary is covered. LGTM. Note the branch currently shows merge conflicts with main and needs a rebase before merging.

Complete inline-code spans before prose possessives were misread as
unresolved shell quotes, producing static_parse_limit. Fenced blocks
indented relative to a proven list container were also left unowned
despite being valid Markdown.

Reuse bounded inline-delimiter pairing for same-line prose classification
and recognize fences only within CommonMark's 0-3 container-relative
indentation, for top-level and nested list items alike, keeping the
typed-fence label and literal-apostrophe state.

Preserve runtime-command ownership when inline payloads can reach
destructive operands. An inert list marker (-, +, 1., 1)) owns only the
initial command position, and only when no unquoted shell control operator
precedes the inline span, sits between spans, or appears inside the span;
any such operator retains the runtime guard for the following clause. A
leading * is a shell glob, so it never owns the command position.

Keep inline and fenced payloads visible to security rules. Incomplete
spans, runtime-selected clauses, and malformed four-column relative fence
indentation remain fail-closed.

Verification after rebasing onto current upstream main (including NVIDIA#902
and NVIDIA#947): the four analyzer test modules pass (552 tests). A 3,220-variant
differential against main finds no case that moves from COMPLETED to
PARTIAL and none where a control operator clears the parse-limit result.
CLI-based tests only pass locally with click < 8.2 output semantics; the
remaining local failures are the same stderr-in-output failures present on
main. Ruff check and format pass.

Signed-off-by: kshitijrajes <kshitijrajes@nvidia.com>
@kshitijrajes
kshitijrajes force-pushed the fix/markdown-list-fence-ownership branch from 9a07020 to c462a4e Compare November 1, 2026 06:13
@kshitijrajes

Copy link
Copy Markdown
Contributor Author

Addressed in the force-push, which also rebases onto current main (including #902 and #947):

  • Clause ownership is now checked before each inline span and between spans, not only inside the span body. - ; \$CMD`, - `echo ready`; `$CMD`, and the pipeline variants all stay PARTIAL / STATIC_PARSE_LIMIT`; each is a regression.
  • A benign earlier span without an operator (- \echo ready` `$CMD``) still owns the command position through the list marker and completes.
  • A leading * is a shell glob, so it no longer owns the command position; it keeps the guard.
  • The rebase exposed that nested list fences did not carry main's new typed-fence label. They now do, with a regression for nested tcl/verilog fences.
  • Reconciled the apostrophe check with fix(static): stop backticks/apostrophes in comments and prose from tripping static_parse_limit #902: the prose view still blanks complete spans, then applies _blank_line_local_paired_backticks, so behavior outside inline spans matches main.

Since this changes the head after your approval, a re-check of the ownership change is welcome.

@rng1995 rng1995 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[SkillSpector Review]

Value and readiness

The previous wrapped-command and inside/outside/multiple-span operator blockers are now addressed, including the requested list-marker and fence-boundary controls. A remaining bounded-lookahead error can still clear the scanner’s explicit span-limit result.

Material findings

All earlier exact examples resolved. The guard now checks both the prefix before each span and the span body, preserves control operators across span boundaries, and excludes a leading glob marker from inert command ownership. Nested typed fences preserve their language and literal-apostrophe state.

Required — src/skillspector/nodes/analyzers/static_patterns_tool_misuse.py:5499: absence of operand evidence in the first 8,192 characters is treated as proof that the runtime-command guard is unnecessary. Extend the existing wrapped $CMD regression by inserting 2,300 repetitions of pad after its first newline, before probe' -rf /, and keep its _OBSERVATIONS tail. The opening line remains:

`$CMD` reads each reference's

The later closing apostrophe and destructive operands now lie beyond the guard’s lookahead. In the lexical reading, the apostrophes join the padding and operands to $CMD; _bounded_shell_tokens reaches its 8,192-character operand bound. _has_runtime_command_operand_exhaustion explicitly preserves that exhaustion even when real operands occur beyond the bound. The new guard sees no flags in its clipped slice, however, so the prose reading masks the first apostrophe. $CMD then ends at the first newline, while the apostrophe following probe is not a new command-word candidate, allowing the retry to clear the original coverage signal. Treat an exhausted evidence window as unproved, or use a bounded/cached whole-source evidence summary that preserves the existing span contract. Add a regression with the closing quote and operands beyond the bound. This is a source-level incomplete-coverage finding, not a runtime reproduction or combined SAFE-verdict claim.

PIC tradeoffs

Bounded work is appropriate, but a truncated search cannot prove operand absence. Preserve the precision improvement for proven inert list positions while retaining uncertainty for a runtime-selected command whose quote-joined arguments exceed the parser’s bound.

Verification and gaps

Reviewed exact head c462a4ed11c6a6097e9729124b39421317985573: complete diff, pinned surrounding implementation/tests, description, every available prior review/comment/thread, and relevant linked issue intent. Read the full current three-file patch, pinned projection/word/clause/token/retry code and all added controls, every prior finding/reply, and the static-runner path. The example fits within a single 256,000-character raw security window, so fragment/window handling does not recover its cleared signal. Independently corroborated the span-contract mismatch with the root reviewer. All six exact-head hosted checks are successful; the new far-operand control is absent from the tests. No contributor code, tests, imports, builds, scanner invocations, supplied snippets or runtime probes were executed.

The inline runtime-command guard searched only the first 8,192 characters
after a span for destructive operands and treated absence there as proof
that the guard was unnecessary. With `$CMD` in an inline span before a
prose possessive and the closing apostrophe plus `-rf /` placed beyond
that bound, the prose reading masked the apostrophe and cleared the
static_parse_limit result that _bounded_shell_tokens had preserved.

Treat a clipped evidence window as unproved: when the window ends before
the content does, the guard is retained. Windows reaching the end of the
content are judged exactly as before, and proven inert list positions keep
their precision. The guard only runs with complete context, where content
is the whole artifact, so window fragments cannot shorten the bound.

Add a regression with 2,300 lines or words of padding before
`probe' -rf /`, plus a list-marker control that stays COMPLETED.

Verification:
- New regression fails on c462a4e and passes with this change.
- Full suite: the same 583 tests fail with and without this change
  (local click >= 8.2 stderr-in-output and environment failures; CI is
  green on c462a4e); 4 additional tests pass, all of them new.
- 10,080-variant differential (list markers, inline spans, possessives,
  padding around the bound, destructive and benign tails) against
  c462a4e: 1,180 COMPLETED -> PARTIAL, none PARTIAL -> COMPLETED, no
  finding gained or lost. Against 6d0b7c8 (pre-PR): no finding gained or
  lost.
- A real `rm -rf /` (bare, list item, or fenced) still yields TM1. A
  fenced `rm -rf /` beyond the bound after a non-list `$CMD` span is
  PARTIAL again, as on 6d0b7c8.
- Ruff check and format pass; mypy output is unchanged from c462a4e.

Signed-off-by: kshitijrajes <kshitijrajes@nvidia.com>

@rng1995 rng1995 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[SkillSpector Review]

Value and readiness

The remaining bounded-lookahead blocker is resolved. A clipped evidence window now preserves the runtime-command guard instead of proving that later operands are absent. The scoped ownership change is ready for maintainer/PIC final review.

Material findings

All previously required corrections are resolved. The guard now remains active whenever evidence_end is before the source end, so the far closing quote and destructive operands cannot be detached by the prose retry. Both prose branches continue to require an inactive guard. Four new regression cases cover line/word padding beyond the bound for the runtime command and the inert-list precision control. Earlier wrapped-command, outside/inside/multiple-span operator, glob-marker, typed-fence and relative-indentation controls remain. No further required changes found.

PIC tradeoffs

The bounded evidence search remains unchanged in size. A truncated search keeps uncertainty; a complete search reaching EOF can still establish operand absence. The conservative runtime guard does not expand the inert list-marker exemption.

Verification and gaps

Read the complete current three-file patch, the54-line increment against the previously reviewed source, pinned guard/tokenizer/prose retry and runner context, all added/adjacent tests and prior review/discussion/thread history. A second reviewer independently confirmed that the exact far-operand probe retains STATIC_PARSE_LIMIT and that the existing precision controls are preserved. All six exact-head hosted checks succeeded. No contributor modules, tests, imports, builds, scanner invocations or runtime probes were executed. This approval is based on source/test inspection and hosted CI.

Reviewed head: 8ccb18da6db5040db1a14241b38724d3281ddfa6.

@kshitijrajes

Copy link
Copy Markdown
Contributor Author

[SkillSpector Review]

Value and readiness

The previous wrapped-command and inside/outside/multiple-span operator blockers are now addressed, including the requested list-marker and fence-boundary controls. A remaining bounded-lookahead error can still clear the scanner’s explicit span-limit result.

Material findings

All earlier exact examples resolved. The guard now checks both the prefix before each span and the span body, preserves control operators across span boundaries, and excludes a leading glob marker from inert command ownership. Nested typed fences preserve their language and literal-apostrophe state.

Required — src/skillspector/nodes/analyzers/static_patterns_tool_misuse.py:5499: absence of operand evidence in the first 8,192 characters is treated as proof that the runtime-command guard is unnecessary. Extend the existing wrapped $CMD regression by inserting 2,300 repetitions of pad after its first newline, before probe' -rf /, and keep its _OBSERVATIONS tail. The opening line remains:

`$CMD` reads each reference's

The later closing apostrophe and destructive operands now lie beyond the guard’s lookahead. In the lexical reading, the apostrophes join the padding and operands to $CMD; _bounded_shell_tokens reaches its 8,192-character operand bound. _has_runtime_command_operand_exhaustion explicitly preserves that exhaustion even when real operands occur beyond the bound. The new guard sees no flags in its clipped slice, however, so the prose reading masks the first apostrophe. $CMD then ends at the first newline, while the apostrophe following probe is not a new command-word candidate, allowing the retry to clear the original coverage signal. Treat an exhausted evidence window as unproved, or use a bounded/cached whole-source evidence summary that preserves the existing span contract. Add a regression with the closing quote and operands beyond the bound. This is a source-level incomplete-coverage finding, not a runtime reproduction or combined SAFE-verdict claim.

PIC tradeoffs

Bounded work is appropriate, but a truncated search cannot prove operand absence. Preserve the precision improvement for proven inert list positions while retaining uncertainty for a runtime-selected command whose quote-joined arguments exceed the parser’s bound.

Verification and gaps

Reviewed exact head c462a4ed11c6a6097e9729124b39421317985573: complete diff, pinned surrounding implementation/tests, description, every available prior review/comment/thread, and relevant linked issue intent. Read the full current three-file patch, pinned projection/word/clause/token/retry code and all added controls, every prior finding/reply, and the static-runner path. The example fits within a single 256,000-character raw security window, so fragment/window handling does not recover its cleared signal. Independently corroborated the span-contract mismatch with the root reviewer. All six exact-head hosted checks are successful; the new far-operand control is absent from the tests. No contributor code, tests, imports, builds, scanner invocations, supplied snippets or runtime probes were executed.

Addressed in 8ccb18d.
Fix: treat a clipped evidence window as unproved. In prose_without_complete_inline_code, when remainder_start + _ROOT_GLOB_COMMAND_CHARS falls before the end of the content, the runtime-command guard is retained instead of relying on _may_have_destructive_outer_operands over the truncated slice. Windows that reach the end of the content are judged exactly as before, and proven inert list positions (-, +, 1., 1)) keep their precision. The guard only runs with complete context, where content is the whole artifact, so window fragments cannot shorten the bound.
Regression: test_inline_runtime_command_operands_beyond_evidence_bound extends the wrapped $CMD example with 2,300 repetitions of pad (and of pad\n) after the first newline, before probe' -rf /, keeping the _OBSERVATIONS tail. It asserts -rf / lies beyond the bound and the result stays PARTIAL/STATIC_PARSE_LIMIT. The same input with a - list marker remains COMPLETED. It fails on c462a4e and passes on 8ccb18d.
Verification:

  • Full suite: identical failure set with and without the change (local-environment CLI failures only); the 4 new tests are the only difference.
  • 10,080-variant differential (list markers, inline spans, possessives, padding around the bound, destructive and benign tails) against c462a4e: 1,180 COMPLETED → PARTIAL, none PARTIAL → COMPLETED, no finding gained or lost. Against 6d0b7c8 (pre-PR): no finding gained or lost.
  • A real rm -rf / (bare, list item, or fenced) still yields TM1.
  • Ruff check and format pass; mypy output unchanged from c462a4e.

@kshitijrajes kshitijrajes reopened this Nov 2, 2026
@kshitijrajes

Copy link
Copy Markdown
Contributor Author

[SkillSpector Review]

Value and readiness

The previous wrapped-command and inside/outside/multiple-span operator blockers are now addressed, including the requested list-marker and fence-boundary controls. A remaining bounded-lookahead error can still clear the scanner’s explicit span-limit result.

Material findings

All earlier exact examples resolved. The guard now checks both the prefix before each span and the span body, preserves control operators across span boundaries, and excludes a leading glob marker from inert command ownership. Nested typed fences preserve their language and literal-apostrophe state.
Required — src/skillspector/nodes/analyzers/static_patterns_tool_misuse.py:5499: absence of operand evidence in the first 8,192 characters is treated as proof that the runtime-command guard is unnecessary. Extend the existing wrapped $CMD regression by inserting 2,300 repetitions of pad after its first newline, before probe' -rf /, and keep its _OBSERVATIONS tail. The opening line remains:

`$CMD` reads each reference's

The later closing apostrophe and destructive operands now lie beyond the guard’s lookahead. In the lexical reading, the apostrophes join the padding and operands to $CMD; _bounded_shell_tokens reaches its 8,192-character operand bound. _has_runtime_command_operand_exhaustion explicitly preserves that exhaustion even when real operands occur beyond the bound. The new guard sees no flags in its clipped slice, however, so the prose reading masks the first apostrophe. $CMD then ends at the first newline, while the apostrophe following probe is not a new command-word candidate, allowing the retry to clear the original coverage signal. Treat an exhausted evidence window as unproved, or use a bounded/cached whole-source evidence summary that preserves the existing span contract. Add a regression with the closing quote and operands beyond the bound. This is a source-level incomplete-coverage finding, not a runtime reproduction or combined SAFE-verdict claim.

PIC tradeoffs

Bounded work is appropriate, but a truncated search cannot prove operand absence. Preserve the precision improvement for proven inert list positions while retaining uncertainty for a runtime-selected command whose quote-joined arguments exceed the parser’s bound.

Verification and gaps

Reviewed exact head c462a4ed11c6a6097e9729124b39421317985573: complete diff, pinned surrounding implementation/tests, description, every available prior review/comment/thread, and relevant linked issue intent. Read the full current three-file patch, pinned projection/word/clause/token/retry code and all added controls, every prior finding/reply, and the static-runner path. The example fits within a single 256,000-character raw security window, so fragment/window handling does not recover its cleared signal. Independently corroborated the span-contract mismatch with the root reviewer. All six exact-head hosted checks are successful; the new far-operand control is absent from the tests. No contributor code, tests, imports, builds, scanner invocations, supplied snippets or runtime probes were executed.

Addressed in 8ccb18d. Fix: treat a clipped evidence window as unproved. In prose_without_complete_inline_code, when remainder_start + _ROOT_GLOB_COMMAND_CHARS falls before the end of the content, the runtime-command guard is retained instead of relying on _may_have_destructive_outer_operands over the truncated slice. Windows that reach the end of the content are judged exactly as before, and proven inert list positions (-, +, 1., 1)) keep their precision. The guard only runs with complete context, where content is the whole artifact, so window fragments cannot shorten the bound. Regression: test_inline_runtime_command_operands_beyond_evidence_bound extends the wrapped $CMD example with 2,300 repetitions of pad (and of pad\n) after the first newline, before probe' -rf /, keeping the _OBSERVATIONS tail. It asserts -rf / lies beyond the bound and the result stays PARTIAL/STATIC_PARSE_LIMIT. The same input with a - list marker remains COMPLETED. It fails on c462a4e and passes on 8ccb18d. Verification:

  • Full suite: identical failure set with and without the change (local-environment CLI failures only); the 4 new tests are the only difference.
  • 10,080-variant differential (list markers, inline spans, possessives, padding around the bound, destructive and benign tails) against c462a4e: 1,180 COMPLETED → PARTIAL, none PARTIAL → COMPLETED, no finding gained or lost. Against 6d0b7c8 (pre-PR): no finding gained or lost.
  • A real rm -rf / (bare, list item, or fenced) still yields TM1.
  • Ruff check and format pass; mypy output unchanged from c462a4e.

@rng1995 rng1995 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed head 8ccb18d (static read; no code run). The clipped-lookahead finding is fixed: the runtime-command guard now stays active when the 8,192-char evidence window ends before EOF, and the far-operand regression (lines/words padding, with a list-marker precision control) pins it. All earlier items are addressed and the review threads are resolved. LGTM; test-unit was still pending when I approved, so please wait for it to go green before merging.

@rng1995
rng1995 merged commit 6ce47f3 into NVIDIA:main Nov 2, 2026
13 checks passed
rng1995 pushed a commit that referenced this pull request Nov 2, 2026
* fix(markdown): preserve bounded inline and list-fence ownership

Complete inline-code spans before prose possessives were misread as
unresolved shell quotes, producing static_parse_limit. Fenced blocks
indented relative to a proven list container were also left unowned
despite being valid Markdown.

Reuse bounded inline-delimiter pairing for same-line prose classification
and recognize fences only within CommonMark's 0-3 container-relative
indentation, for top-level and nested list items alike, keeping the
typed-fence label and literal-apostrophe state.

Preserve runtime-command ownership when inline payloads can reach
destructive operands. An inert list marker (-, +, 1., 1)) owns only the
initial command position, and only when no unquoted shell control operator
precedes the inline span, sits between spans, or appears inside the span;
any such operator retains the runtime guard for the following clause. A
leading * is a shell glob, so it never owns the command position.

Keep inline and fenced payloads visible to security rules. Incomplete
spans, runtime-selected clauses, and malformed four-column relative fence
indentation remain fail-closed.

Verification after rebasing onto current upstream main (including #902
and #947): the four analyzer test modules pass (552 tests). A 3,220-variant
differential against main finds no case that moves from COMPLETED to
PARTIAL and none where a control operator clears the parse-limit result.
CLI-based tests only pass locally with click < 8.2 output semantics; the
remaining local failures are the same stderr-in-output failures present on
main. Ruff check and format pass.

Signed-off-by: kshitijrajes <kshitijrajes@nvidia.com>

* fix(markdown): keep runtime guard when operand lookahead is clipped

The inline runtime-command guard searched only the first 8,192 characters
after a span for destructive operands and treated absence there as proof
that the guard was unnecessary. With `$CMD` in an inline span before a
prose possessive and the closing apostrophe plus `-rf /` placed beyond
that bound, the prose reading masked the apostrophe and cleared the
static_parse_limit result that _bounded_shell_tokens had preserved.

Treat a clipped evidence window as unproved: when the window ends before
the content does, the guard is retained. Windows reaching the end of the
content are judged exactly as before, and proven inert list positions keep
their precision. The guard only runs with complete context, where content
is the whole artifact, so window fragments cannot shorten the bound.

Add a regression with 2,300 lines or words of padding before
`probe' -rf /`, plus a list-marker control that stays COMPLETED.

Verification:
- New regression fails on c462a4e and passes with this change.
- Full suite: the same 583 tests fail with and without this change
  (local click >= 8.2 stderr-in-output and environment failures; CI is
  green on c462a4e); 4 additional tests pass, all of them new.
- 10,080-variant differential (list markers, inline spans, possessives,
  padding around the bound, destructive and benign tails) against
  c462a4e: 1,180 COMPLETED -> PARTIAL, none PARTIAL -> COMPLETED, no
  finding gained or lost. Against 6d0b7c8 (pre-PR): no finding gained or
  lost.
- A real `rm -rf /` (bare, list item, or fenced) still yields TM1. A
  fenced `rm -rf /` beyond the bound after a non-list `$CMD` span is
  PARTIAL again, as on 6d0b7c8.
- Ruff check and format pass; mypy output is unchanged from c462a4e.

Signed-off-by: kshitijrajes <kshitijrajes@nvidia.com>

---------

Signed-off-by: kshitijrajes <kshitijrajes@nvidia.com>
(cherry picked from commit 6ce47f3)
Signed-off-by: Narendran Raghavan <nraghavan@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants