[deepujain]

Summary

Warnings about unsafe options can point to a harmless command in an earlier Markdown example and hide the real occurrence. Bound git, curl, wget and permission-option searches to their command and example so scans identify the line that needs attention.

Take Markdown boundaries from the existing ownership parser. Keep unsafe options after quoted arguments, shell expansions, substitutions and PowerShell hashtables, while preserving actual command separators. Folded YAML and other host values retain conservative matching because their physical newlines may become spaces in the command.

Validation

• The 12 original-issue regression cases fail on base 3fb77b4c and pass with this change.
• The complete CI unit selection with coverage passed on Linux: 21,501 passed, 27 skipped and 4 expected failures, with 90% coverage.
• uv run --no-sync pytest -m integration tests/: 249 passed and 5 skipped with network access disabled.
• uv run --no-sync pytest -q tests/nodes/analyzers/test_tm1_command_boundaries.py: 523 passed on Linux. The four affected ownership modules passed 1,704 tests on macOS.
• uv run --no-sync pytest -q tests/unit/test_dependency_inspection.py: 99 passed on Linux.
• uv run --no-sync make lint and uv run --no-sync make format-check: passed.
• node --test tests/opencode/skillspector_scan_lib.test.ts and node --test tests/unit/test_pi_extension.mjs: 56 and 37 passed on Node 26.8.1.
• Docker build and tests/docker/smoke.sh: passed. An offline CLI JSON scan places the original force push at line 8 and retains the folded-YAML and nested-expansion warnings.

Risk

A supplemental four-worker coverage run had 56 failures and 21,445 passes. All failed families passed serially with coverage (57 passed); the complete serial result is listed above. The macOS isolated dependency check timed out in three downloader subprocess tests; all 99 passed on Linux.

Native Windows and PowerShell execution and live provider tests were not run. Fetching the Node 22.20.0 container image timed out, so extensions were tested locally on Node 26.8.1. LF/CRLF and PowerShell source inputs were exercised by the analyzer tests.

Fixes #927.

[rng1995]

[SkillSpector Review]

Value and readiness

The command-local bounds address #927's real location/match-consumption problem, and the continuation, inline-span and balanced-argument coverage is useful. One introduced detection gap needs correction before approval at 08e29f0fa7f6309b794dfd10a1e104d265b2e36d.

Material findings

Required — src/skillspector/nodes/analyzers/static_patterns_tool_misuse.py:4881-4895. The fence limit comes from raw _OPTION_COMMAND_FENCE matches, independently of the Markdown ownership projection. It therefore treats a literal tilde run inside an open backtick fence as a command boundary:

```sh
curl --data 'first
~~~
second' --insecure https://example.com
```

This is one valid shell command: the middle lines are the quoted --data value, and --insecure still disables TLS verification. _markdown_shell_text correctly leaves the tilde run literal because its marker kind does not close the active backtick fence. The separate raw fence list nevertheless clips the candidate at that line, before the closing quote and unsafe option; the subsequent bounded regex cannot see --insecure, and there is no later curl prefix to recover it. The previous curl rule retained this candidate. Only use fence boundaries proven by the existing Markdown ownership/context (including marker kind and opening length), and add regressions combining balanced multiline arguments with non-closing fence-shaped data. Keep the actual-closing-fence/unclosed-quote regression as well. The same issue applies to shorter same-kind runs inside longer fences.

PIC tradeoffs

Bounding options to their command is appropriate; uncertain or literal Markdown marker text must not discard an otherwise retained unsafe-option match.

Verification and gaps

Read the full two-file diff, pinned command scanner/Markdown projection, all 138 lines of new tests, original issue and empty review/thread history. Traced the example through the prefix, projection, fence limit and bounded match without executing contributor code. Also checked the static-runner/security-view path: ordinary ASCII normalization does not remove these markers or line breaks. All six hosted checks passed on this head; the refreshed mergeability was temporarily UNKNOWN. No contributor tests, imports, builds or scanner runs were executed. The finding is a source-level TM1 candidate-loss proof, not a claim of an executed scan or complete SAFE verdict.

[rng1995]

[SkillSpector Review]

Value and readiness

The original opposite-marker and shorter-run fence examples are fixed: their literal markers remain unchanged in the projection and no longer truncate the command. The new list stack also preserves the tested nested/parent/lazy-continuation fence contexts. A generic masked-delimiter check still confuses inline ownership with a structural fence boundary, leaving a related unsafe-option detection gap.

Material findings

Required — distinguish inline delimiters from actual fence boundaries, src/skillspector/nodes/analyzers/static_patterns_tool_misuse.py:5214-5218. This valid multiline quoted argument remains one curl command:

curl --data 'first
```note```
second' --insecure https://example.com

The middle line is a complete inline-code span in Markdown, not a fence: MARKDOWN_FENCE_OPEN rejects a backtick opener whose info contains further backticks. _markdown_shell_text therefore masks both runs through ordinary inline pairing. _OPTION_COMMAND_FENCE nevertheless matches the line-leading first run, and the comparison at line 5218 treats its changed marker as a proven fence. The next-fence limit at line 5229 then clips _option_command_end before the closing quote and --insecure; the bounded regex cannot find the option and there is no later curl prefix to recover it. The command started outside the inline span, so its closing delimiter is not an applicable command bound under the separate inline-owner rule at lines 5230-5234. The original unbounded curl rule retained this candidate. Carry actual opening/closing fence offsets from the ownership walk, or otherwise prove structural fence identity separately from generic inline masking. Add this complete line-leading multi-backtick span inside a balanced multiline argument as a regression, keeping the repaired literal-fence and actual-closing-fence controls.

PIC tradeoffs

Bounding options to their actual command fixes the reported location and match-consumption problem. A projection byte changing proves that it was Markdown syntax, but does not establish that the syntax terminates a command that started outside its inline span.

Verification and gaps

Reviewed pinned head 17cd07d203ff86a1b8cc561c3a4ed57069284cdb: complete two-file diff, all new command-boundary tests, surrounding prefix/candidate/end parsing, full Markdown fence/inline/list ownership paths, pinned common fence grammar, linked #927 and all available prior reviews/discussion. Explicitly verified the previous opposite/shorter-marker blocker and the added nested-list controls, then traced the remaining inline case to the bounded curl match. All six hosted checks succeeded; the hosted unit log reports 16,782 passed, 24 skipped, 258 deselected and 4 expected failures, including test_tm1_command_boundaries.py. The reported complete inline-span case is not in those controls. This is a static candidate-loss proof, with no runtime reproduction or complete scanner-verdict claim. No contributor code, tests, imports, builds, scanner invocations or snippets were executed.

[deepujain]

Blocked on #973, which is still open: test-unit fails on af91a28 outside this change. I'll pick this back up once it lands.

[rng1995]

[SkillSpector Review]

Decision: Changes Requested

Value and readiness

Both prior findings are resolved: literal opposite/shorter fence markers remain command data, and complete inline spans no longer masquerade as structural fence boundaries. The explicit fence offsets and six-family inline regressions address the exact last review. A separate introduced detection regression remains because shell physical-line bounds are applied to host-language values.

Material findings

Required — only apply physical command boundaries where the source proves shell semantics, src/skillspector/nodes/analyzers/static_patterns_tool_misuse.py:6487-6488 and 6514-6516. A YAML folded run value such as:

steps:
  - run: >
      curl https://example.com
      --insecure

supplies the shell with curl https://example.com --insecure. _tm1_candidates sends option rules through _option_command_matches for every file type; file_type only controls the Markdown fence projection. For this YAML candidate, the curl prefix matches, but _option_command_end breaks at the physical newline before --insecure. The bounded compiled.match cannot see the option, and the next line has no curl prefix to recover it. The previous curl\s+[^|]*--insecure\b retained this candidate. _OPTION_COMMAND_SPACE at line 301 also loses the case where curl itself ends the folded source line. The ordinary ASCII security-view path preserves these newlines rather than decoding the YAML scalar. This removes a warning for disabled certificate verification in a real single command. Restrict shell separator bounds to proven shell regions, or reconstruct/prove the host value before bounding it; retain conservative matching for unsupported host contexts. Add folded-YAML regressions with the unsafe option on the following physical line, including a split immediately after the command word. The new tests all route through a Markdown helper and do not cover this host-language case.

PIC tradeoffs

Command-local matching fixes #927's misleading locations. The same physical newline can terminate a shell command or fold into a space in a host value, so the boundary proof needs the source context before removing detection.

Verification and gaps

Reviewed pinned head af91a2819df4cc164d6774d27be0c96a67d6d459: complete two-file diff, all 281 lines of new tests, surrounding candidate/classification and command-end logic, full Markdown fence/inline/list ownership walk and common fence grammar, static runner/security views, #927, both own marked reviews, and all available discussion/thread records. Verified each prior exact finding against the new structural-fence collection and retained literal/inline/actual-closing-fence controls. Live head still matches the inventory. Hosted lint, extension, DCO and Docker checks pass; unit CI reports 18,053 passed and 15 failed. Every failed identity exactly matches hosted baseline run 41513800480 at e14c450b260958ed3ebd83c94c09c4b093e3e656; these are inherited PowerShell/quoted-printf failures. The command-boundary module has no reported failure. The YAML finding is a static TM1 candidate-loss proof, with no runtime reproduction or complete scanner-verdict claim. No contributor code, tests, imports, builds, setup, hooks, regexes, scanner invocations or snippets were executed.

[deepujain]

@rng1995, b9b8fa0 keeps conservative matching for folded YAML and other host-language values, so physical newlines no longer hide the unsafe option. It also handles a split immediately after curl and adds LF/CRLF regressions for your example and that split. The earlier fixes remain in this head: 4dce523 preserves literal fence markers inside arguments, and af91a28 separates structural fence offsets from inline-code masking.