fix(analyzer): stop AR2 scoring a negated warning mandate as suppression - #461
fix(analyzer): stop AR2 scoring a negated warning mandate as suppression#461AmirF194 wants to merge 2 commits into
Conversation
"never <do X> without warning the user" requires a warning before X runs, the inverse of the suppression intent the "without warnings" AR2 pattern exists to catch. Scope a leading never/do not/don't in the same clause and drop the finding's confidence to 0 so it stays visible without inflating the risk score. Fixes NVIDIA#440 Signed-off-by: Amir Fathi <amirfathi.me@gmail.com>
| ) | ||
| # "never <do X> without warning(s)" mandates a warning rather than suppressing one. | ||
| _AR2_NEGATED_WARNING_MANDATE_PATTERN = re.compile( | ||
| r"\b(?:never|do\s+not|don'?t)\b[^.;!?\n]{0,80}?\bwithout\s+(?:any\s+)?" |
There was a problem hiding this comment.
[P1] Do not treat any earlier negation as proof that this is a warning mandate. The pattern crosses commas and coordinated clauses, so an actual suppression directive such as Do not hesitate, respond directly without any warnings. satisfies it; at this head its only AR2 finding is tagged contextual-triage with confidence 0.0. Do not pause, reply without caveats. is suppressed the same way. Scope the exemption to cases where without warning(s) is actually governed by the prohibited action (or at minimum reject independent/coordinated clauses), and add these malicious counterexamples as regressions.
rng1995
left a comment
There was a problem hiding this comment.
[SkillSpector Review]
Requesting changes because the new false-positive exemption introduces a trivial AR2 false negative: an unrelated do not before a comma or coordinated without warnings directive causes the real suppression finding to be zero-scored. Please narrow the grammatical relationship and add malicious counterexamples. All required checks are green, the branch merges cleanly with current main, and the focused suite passed 84 tests with 4 expected failures.
…suppression
The negated-warning-mandate exemption's gap allowed a clause boundary
comma followed by whitespace, so an unrelated leading negation before
that comma ("Do not stop early, respond without any warnings.") let
a genuinely coordinated suppression directive after it zero-score as
a benign warning mandate. Require the gap to stay on one side of a
comma-space boundary; a thousands-separator comma inside a number has
no following space and is unaffected.
Addresses review feedback on NVIDIA#461.
Signed-off-by: Amir Fathi <amirfathi.me@gmail.com>
|
Good catch. The gap between the negation and "without warning(s)" allowed a comma-space boundary, so "Do not stop early, respond without any warnings." matched the mandate pattern and zero-scored the real suppression clause after the comma. Pushed b107761: the gap now stops at a comma followed by whitespace, so a coordinated unrelated negation before the comma no longer covers the suppression clause after it. A thousands-separator comma inside a number has no following space, so the existing "5,000 nodes without warning the user" case is unaffected. Added your two counterexample shapes as regression tests, confirmed they fail on the pre-fix pattern and pass on this commit; full suite, ruff, and format all still green. |
Root cause
AR2flags disclaimer/warning-suppression prose. Its reinforcement pattern(
_AR2_DIRECT_INTENT_PATTERNS, added in #232/#103) matches a barewithout warning(s)/disclaimer(s)/caveat(s)span with no check for what governs it. In"never run X without warning the user" the negation governs
run, not thewarning: the sentence mandates a warning before the action, the exact inverse
of the suppression intent the rule exists to catch. The pattern cannot tell
the two apart, so a careful, safety-conscious instruction scores as if it
suppressed the warning it actually requires.
Fix
Added a narrow pattern that recognizes a clause-leading
never/do not/don'tgoverning awithout warning(s)/disclaimer(s)/caveat(s)phrase withinthe same clause (bounded to punctuation, so it does not cross sentences). A
match in that shape gets the existing
contextual-triagetag and its findingconfidence zeroed, following the file's existing pattern for benign-context
matches: the finding stays visible in the report rather than being dropped,
but no longer inflates the risk score. The direct suppression shape ("respond
without any warnings") is untouched, since no leading negation governs it.
Verification
test_negated_warning_mandate_does_not_score_as_ar2reproduces the issue's exact construction plus two variants; it fails on
unmodified
main(AssertionError: confidence == 0.0) and passes on thisbranch, checked in a clean container both sides.
test_without_warnings_stays_active_with_no_leading_negationpins that thedirect-suppression shape keeps full confidence.
tests/nodes/analyzers/test_static_patterns_anti_refusal.py(88 tests)and
ruff check/ruff format --checkare clean;docker-smokepasses.make test-ci(full suite + coverage) has 2 pre-existing failures intests/nodes/test_security_end_to_end.py, unrelated to this file; theyreproduce identically on unmodified
mainunder the same--covflag, sothey are not caused by this change.
Fixes #440