scan a named file; find config files below the root; --only '' fails closed - #114
Conversation
…closed Three defects found by the fresh-user release test, all pre-existing, all the same shape as the ones this release already fixes: a command reporting success while doing less than it said. scan <file> accepted a file path, checked it existed, then walked it as a directory and found nothing - so it printed 'No hardcoded credentials found' and exited 0 over a live credential, while scanning the parent directory found it. A named file is now scanned as that file, and because naming a path is explicit rather than a walk, the ignore list and test-file heuristics do not apply to it. Config files were matched only against the scan root while source files recursed, so a monorepo, or anything under deploy/ or infra/, got a false clean bill of health from the first command anyone runs. They are now found at any depth, including the hidden directories where tool configs live. Generated trees are still never walked and .secretlessignore still applies. run --only '' read as 'flag not supplied', so the filter vanished and the command ran with the whole store, exit 0 - while --only ,, refused. Both now fail closed. One test was rebuilt after mutation testing: the node_modules assertion could not fail, because the default-ignore list covers the same directories as the walker's own skip list. The new one runs with ignore:false, where the walker is the only thing left.
There was a problem hiding this comment.
Claude Code Review
VERDICT: APPROVE
SUMMARY: This PR fixes three critical defects in the scan and run commands that caused false negatives — commands reporting success while doing less than advertised. The fixes are: (1) scan <file> now actually scans named files instead of treating them as empty directories, (2) config files are now found recursively instead of only at the scan root, and (3) --only '' now fails closed instead of being treated as "flag absent". All three fixes include comprehensive test coverage proving the old behavior was broken and the new behavior is correct. The changes are well-scoped, the new walkConfigFiles function mirrors the existing walkers' safety patterns, and the single-file scan path applies the same credential detection logic as the directory scan.
No security, correctness, or quality findings after verification against the full source files.
Reviewed 5 files changed (20688 bytes)
Three defects found by the fresh-user release test for 0.21.1. All pre-existing (present in the published 0.20.0 and 0.21.0), and all the same shape as the ones 0.21.1 already fixes: a command reporting success while doing less than it said.
Shipping 0.21.1 without these would be incoherent, since that is precisely what the release is about.
P1:
scan <file>reported clean over a live credentialA file path was accepted, checked for existence (a nonexistent path correctly errors), then walked as if it were a directory — which found nothing.
secretless-ai scan src/config.tsin a CI step was a green pass over a live credential.A named file is now scanned as that file. Because naming a path is an explicit instruction rather than a directory walk, the ignore list and the test-file heuristics do not apply to it — those exist to keep a walk from being noisy, and there is no walk. So
scan test/fixture.test.jsscans it.P1: config files were only found at the scan root
Source files recursed; config files (
config.json,docker-compose.yml,.mcp.json,terraform.tfvars, …) were matched only against the top directory. A monorepo, or anything underdeploy/orinfra/, got a false clean bill of health from the first command anyone runs.Config files are now found at any depth, including the hidden directories where tool configs actually live (
.cursor/,.claude/,.vscode/) — those are skipped for source files but are exactly where these configs are. Generated trees (node_modules/,dist/,build/) are still never walked,.secretlessignorestill applies, and a file covered by the config pass is not re-reported by the source pass.P2:
run --only ''ran the command and exited 0An empty value read as "flag not supplied", so the filter vanished. Semantically identical input, opposite fail direction. A CI step computing
--only "$KEYS"with an empty$KEYSran unprotected and reported success. Both forms now fail closed, at bothrunandenvparse sites.Verification
node_modulesassertion could not fail, because the default-ignore list covers the same directories as the walker's own skip list, so removing the walker's check left it green. The replacement runs withignore: false, where the walker is the only thing left.