Skip to content

feat: replace the hand-rolled walker with the ignore crate (#22) - #69

Merged
hermanngeorge15 merged 1 commit into
mainfrom
feat/ignore-crate-walker-22
Aug 24, 2026
Merged

feat: replace the hand-rolled walker with the ignore crate (#22)#69
hermanngeorge15 merged 1 commit into
mainfrom
feat/ignore-crate-walker-22

Conversation

@hermanngeorge15

Copy link
Copy Markdown
Contributor

Closes #22.

main::walkdir was a recursive read_dir with a five-extension allowlist and nothing else: it descended into .git, target and node_modules, ignored .gitignore entirely, read files of any size fully into memory, and used is_dir() — which follows symlinks — with no loop guard.

Measured, release vs release

this repo ecosystem tree (8 repos)
before 100 ms 1.46 s
after 10 ms 0.31 s

The findings that disappeared are the point

On this repo the count goes 159 → 129. All thirty came from four gitignored scratch files — review reports quoting attack strings, which nobody would act on.

$ injection-scanner check . --no-ignore
159 finding(s): 108 critical, 51 high

Exactly 159 back. That's the check that this removed noise rather than coverage, and it's the assertion I'd want a reviewer to push on hardest.

What's in it

  • ignore crate (the ripgrep walker) — .gitignore support and parallel traversal.
  • Flags: --exclude, --include, --no-ignore, --max-file-size, --follow-symlinks, --jobs.
  • Unconditional deny-list for build output, not lifted by --no-ignore and not reachable by --include. "Don't trust this repo's ignore rules" is a reasonable thing to want on a checkout you didn't write; "scan 40,000 files of build output" is not. Both pinned by tests.
  • require_git(false) — otherwise the same directory gives different answers scanned as a checkout vs. as an extracted tarball. Coverage that depends on whether .git happens to be present is coverage you can't reason about.
  • Sorted results — parallel traversal finishes in thread order, and the JSON output is an array consumers diff.

Nothing is skipped silently

Files the walker reached and declined are counted with their reason. Ignore rules prune whole subtrees before the walker sees them, so they can't be itemised without giving up the speed — those are disclosed as having been applied instead:

note: .gitignore rules were applied — paths they exclude were not scanned and are
      not counted above. Use --no-ignore to include them.
note: 27 file(s) not scanned — not a scanned file type. Use --include <glob> to add them.

A skills pack shipping a .gitignore containing * would otherwise scan nothing and report a clean bill of health — the same failure mode this milestone has been closing everywhere else.

Extensions are deliberately unchanged

Widening them is #23. 209 of the 215 files it would newly reach on this repo are under target/, which is why it needed this first — as the issue predicted. Doing both at once would also make it impossible to tell a walker regression from a file-type regression.

Also fixes the #19 harness guard

It flagged two of the new tests. It matched target/debug as a substring, so a fixture directory named that — created to prove the walker refuses to descend into one — read as a test mis-targeting the binary.

Now it requires the binary name on the same line, and adds the direct check the original lacked: a file that spawns Command::new must mention CARGO_BIN_EXE somewhere in it. File-level, because going through a local binary_path() helper is the correct shape — a per-line rule would have condemned the very fix the guard exists to protect. Both branches mutation-checked against deliberate offenders.

Tests

11 new, one per defect the old walker had: deny-list, deny-list under --no-ignore, .gitignore round-trip, size cap, symlink loop, unscanned-type accounting, --include, --include vs deny-list, --exclude, deterministic ordering, unreadable subdirectory. Full suite: 18 binaries green, clippy clean.

Merge order

Touches src/main.rs's Check args, so this conflicts with #65 (which adds --strict / --min-confidence there). Trivial to resolve either way — happy to rebase this one on top once #65 lands.

`main::walkdir` was a recursive `read_dir` with a five-extension allowlist and
nothing else. It descended into `.git`, `target` and `node_modules`, ignored
`.gitignore` entirely, read files of any size fully into memory, and used
`is_dir()` — which follows symlinks — with no loop guard.

Release build, measured:

              this repo    ecosystem tree (8 repos)
  before        100ms        1.46s
  after          10ms        0.31s

The findings that disappeared are the point. On this repo the count goes 159 ->
129, and all thirty came from four gitignored scratch files nobody would act on.
`--no-ignore` returns exactly 159, which is the check that this removed noise
rather than coverage.

  - `ignore` crate (the ripgrep walker): .gitignore support and parallelism.
  - `--exclude`, `--include`, `--no-ignore`, `--max-file-size`,
    `--follow-symlinks`, `--jobs`.
  - Unconditional deny-list for build output, NOT lifted by `--no-ignore` and
    not reachable by `--include`. "Do not trust this repo's ignore rules" is a
    reasonable thing to want; "scan 40,000 files of build output" is not.
  - `require_git(false)`, so a directory scanned as a checkout and as an
    extracted tarball give the same answer. Coverage that depends on whether
    `.git` happens to be present is coverage you cannot reason about.
  - Sorted results. Parallel traversal finishes in thread order, and the JSON
    output is an array consumers diff.

Nothing is skipped silently. Files reached and declined are counted with their
reason; ignore rules prune whole subtrees before the walker sees them, so they
cannot be itemised and are instead disclosed as having been applied. A skills
pack shipping a `.gitignore` containing `*` would otherwise scan nothing and
report clean — the same failure this milestone has been closing everywhere else.

Extensions are deliberately unchanged. Widening them is #23, and 209 of the 215
files it would newly reach on this repo are under `target/` — which is why it
needed this first, as the issue predicted.

Also fixes the harness contract guard from #19, which flagged two of the new
tests. It matched `target/debug` as a substring, so a fixture directory named
that — created to prove the walker refuses to descend into one — read as a test
mis-targeting the binary. Now it requires the binary name on the same line, and
adds the direct check the original lacked: a file that spawns `Command::new`
must mention `CARGO_BIN_EXE` somewhere. Both branches mutation-checked.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@hermanngeorge15
hermanngeorge15 force-pushed the feat/ignore-crate-walker-22 branch from d33d17a to 5fda3aa Compare August 24, 2026 18:35
@hermanngeorge15
hermanngeorge15 merged commit c04e0cb into main Aug 24, 2026
1 check passed
@hermanngeorge15
hermanngeorge15 deleted the feat/ignore-crate-walker-22 branch August 24, 2026 18:40
hermanngeorge15 added a commit that referenced this pull request Aug 25, 2026
It claimed the v0.0.3 tag was still pending — it shipped on 2026-08-23 — and
that there were no open pull requests, while four were open and three of those
were a stack. Phase 3 has been in progress since #65 and #69 merged.

Also records what v0.0.3 actually shipped, and that main has since moved past it
without a tag: no consumer is affected, because spec-ci-plugin pins v0.0.3, so a
v0.0.4 is a choice rather than a debt. That distinction was not written down
anywhere.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.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.

feat: replace hand-rolled walkdir with the ignore crate — excludes, .gitignore, size caps

1 participant