Three proposals: surface rules that die, exclude test code, scope guards per account

[anthonyra]

Draft, opened for direction rather than for merge. Three changes that came
out of running radar as a gate on two Solana repos. Each argues a separate
point; I'd rather find out which ones you agree with before polishing any of
them. There are rough edges — the third one in particular overlaps work you did
independently, and I'd like to talk about that overlap rather than assume mine
wins.

---

1. A rule that dies reads exactly like a rule that found nothing

exit_on_none / exit_on_value signal "abandon this item" by raising
StopIteration, and templates catch it with a bare except:. That handler also
catches every genuine bug in a rule. A TypeError from a bad comparison, a
NameError from a builtin the sandbox doesn't expose — all of it is swallowed
and the item is skipped, so a broken rule reports zero findings and a scan comes
back clean.

I hit this twice while writing rules. Once by putting a dict in a set, once by
calling str(), which isn't in the sandbox's builtins. Both times the rule went
to zero findings on its own vulnerable mock and nothing said a word.

This introduces RuleSkip for that control-flow signal, and narrows bare
except: to it in the AST transform, so anything else propagates. The reporting
chain behind it already existed and was already correct — nothing could reach
it.

Swept all Rust rules across every mock variant afterwards: 0 rules raise.

2. Test code is excluded by default, --include-tests puts it back

Tests unwrap, hardcode keys, reuse a PDA across two domains and skip owner
checks, because doing it the production way would obscure what's being tested.
Reporting that is a fixed cost on every scan, and it trains people to skim the
report — which is how the real finding gets missed.

Two filters, because Rust puts tests in two places: paths (tests/,
test-named files) and #[cfg(test)] / #[test] items inside the module they
test, which no path filter can see.

Measured on two real repos:

repo	default	--include-tests
srsly rust/	78 files, 0 findings	113 files, 172 findings
antegen programs/	35 files, 15 findings	61 files, 25 findings

On srsly, test code was 100% of the findings. On antegen, 9 of the 10 dropped
findings are inside a #[cfg(test)] mod tests in
thread/src/state/config.rs — a production file by every naming convention, so
the item filter is doing work the path filter structurally cannot.

Two implementation notes that might matter to you:

• Pruning happens after span enrichment, deliberately. Enrichment hands out
  the nth textual occurrence of an identifier, so dropping nodes before it would
  shift the spans of nodes that remain. There's an E2E fixture with
  #[cfg(test)] mod tests placed above the production code, pinning a finding
  to the same line in both modes.
• is_test_path judges directories relative to the scan root. Judged
  absolutely, a checkout that happens to live in ~/work/tests/my-program gets
  every file classified as test code and returns an empty scan with nothing to
  explain it — the exact silent-clean failure this feature must not introduce.

Deliberately not excluded: benches/ and examples/. A benchmark isn't
asserting correctness and an example is code users are invited to copy, so
neither has the written-unsafely-on-purpose property that justifies skipping
tests.

3. Three rules that keyed on Anchor's spelling — and where this overlaps #43713b0

unchecked_arithmetics, account_data_matching and
invoke_signed_unvalidated_seeds were the three noisiest rules on already-fixed
code. Measured with one harness across old and new templates, findings on fixed
code went 81 → 19.

Causes, per rule: saturating_* and wrapping_* read as no overflow handling
at all; one expression reporting the same line once per operator; index and
?-operand arithmetic that cannot wrap silently; guards written as named
helpers (check_owner, assert_owned_by) rather than require!; and CPI
wrapper functions whose accounts and seeds are all parameters, so the function
chose nothing.

The overlap. You fixed account_data_matching independently in 43713b0 —
accepting an if-guard, not only require!. That's the same false positive I
was chasing and your fix lands it. Mine goes further in a way I think matters,
and this is the part I most want your read on:

Both your v0.1.1 and my first attempt scope the guard to the function. That
makes a validated account excuse an unvalidated sibling in the same handler —
unpack two accounts, check the owner of one, and the other stops being
reported. It's the same shape as the file-scope suppression problem from #23,
one level down. So v0.2.0 here scopes per account: it resolves one-hop let
bindings, attributes each guard to the account it names, and asks the question
per unpacked account rather than per function.

api/tests/detection_fixtures/account_data_matching__unchecked_sibling.rs is
the case that separates them — your v0.1.1 and my earlier function-scoped
version both go silent on it.

I'm not attached to my implementation. If you'd rather keep v0.1.1's shape and
add per-account scoping to it, that's a fine outcome and I'll rework it.

There's one documented fallback I'm not happy about: when the receiver chain of
&mut info.data.borrow_mut() is dropped during enrichment, only borrow_mut
survives and the rule can't tell which account was touched, so it falls back to
function scope. That looks like an AST bug rather than a rule bug, and fixing it
would let the fallback be deleted. Happy to take that separately if you agree
with the diagnosis.

---

What's deliberately not in this branch

Three more changes exist in the fork that I left out rather than pad the diff:

• Workspace-shaped mocks + whole-variant parsing. The template harness
  parses one file per AST while production parses the whole crate, so two-pass
  cross-file rules (pda_sharing, init_if_needed_reinitialization) have zero
  coverage of the pass that matters — they're structurally unable to fire in the
  harness while working fine in production. Fixing it means restructuring the
  mocks to {bad,good}/programs/<name>/src/, which conflicts heavily with the
  mock tree here. Worth its own PR if you want it.
• The accuracy suite in CI. The fast test job has no rust_syn, so Rust
  fixture generation fails under || true and every Rust accuracy test silently
  skips — a skip isn't a failure, so two drifted expectations sat green. Moving
  those tests to the job that has rust_syn catches it.
• An out-of-sample nightly scoring the rules against ScannerTruth's corpus-2
  with that pack's own scorer. Left out because it clones a third-party repo into
  CI and adds a baseline file someone would have to own. Mentioned only so the
  numbers above have a provenance.

Verification

pytest api/tests/ on this branch: 198 passed, 1 skipped, 6 failed. All six
failures are test_cli.py and are identical on stock upstream/main — they
shell out to the radar wrapper, which needs Docker inside the test container.

Also dropped from this branch after finding you'd already solved it, better: I
had a fix for [i] Results written to <path> printing on clean scans where no
file exists. copy_out() already gates that on the copy succeeding and warns on
failure, where mine stayed silent.