feat: detect-secrets as 20th scanner (Track B)#72
Merged
Conversation
f6a0fe1 to
ac9d628
Compare
Closed
theagenticguy
pushed a commit
that referenced
this pull request
May 10, 2026
## Track B — constraint-10 (detect-secrets as 20th scanner)
Closes the constraint-10 leg of v1-finalize per
`.erpaval/specs/006-v1-finalize/`.
Spec: [spec.md§Track B](.erpaval/specs/006-v1-finalize/spec.md).
PR-split:
[pr-split-analysis.md](.erpaval/specs/006-v1-finalize/pr-split-analysis.md)
— A → C → B → D ordering, this is leg 2 of 4.
### What changes
- **`DETECT_SECRETS_SPEC`** added to `packages/scanners/src/catalog.ts`
between BANDIT and BIOME. Polyglot P1, Apache-2.0, pinned at v1.5.0 with
a stale-since-2024 catalog comment. `ALL_SPECS.length` rises 19 → 20;
`P1_SPECS` rises 11 → 12.
- **`createDetectSecretsWrapper`**
(`packages/scanners/src/wrappers/detect-secrets.ts`) invokes
`detect-secrets scan . --all-files` and pipes stdout JSON through the
converter. Empty SARIF + `skipped` on missing binary; empty SARIF on
malformed stdout — `tool.driver.name` always preserved (E-B-2).
- **`detectSecretsJsonToSarif`**
(`packages/scanners/src/converters/detect-secrets-to-sarif.ts`) walks
the per-file `results` map, maps detect-secrets `type` → SARIF `ruleId`
via a 25-row table covering every v1.5.0 detector, with slug fallback
for future detectors. SHA-1 `hashed_secret` stamped on
`partialFingerprints.detect_secrets_sha1` — explicitly **not**
advertised as a cryptographic fingerprint per W-B-1. Overlapping
findings (KeywordDetector + AWSKeyDetector on the same line) pass
through without dedupe per W-B-2; OCH's downstream SARIF dedupe handles
merge. Every emitted log validated against `SarifLogSchema` before
return.
### AC summary (Track B — 2 of 2)
| AC | What |
|---|---|
| B-1 | `DETECT_SECRETS_SPEC` added to catalog (19→20); P1 stable-order
updated; cli `selectScanners` test extended for the new polyglot P1 |
| B-2 | wrapper + JSON→SARIF converter + 13 new tests (9 converter + 4
wrapper) |
### Validation
- **93 tests in `@opencodehub/scanners`** (was 80; +13 new), all green.
- `pnpm -r exec tsc --noEmit` clean.
- `bash scripts/check-banned-strings.sh` PASS.
- `mise run check` exits 0.
- SARIF emissions validated against `SarifLogSchema` at the conversion
boundary.
- W-B-1 honored: `partialFingerprints.detect_secrets_sha1` slot is
plugin-defined identifier per SARIF §3.27.18, not a crypto claim.
- W-B-2 honored: overlapping findings test asserts both pass through
with the same line number.
- E-B-2 honored: missing-binary path returns empty SARIF with
`tool.driver.name = "detect-secrets"` preserved.
### Compound lessons extracted
1 durable lesson written to `.erpaval/solutions/`:
- `best-practices/squash-merge-masks-pre-existing-debt.md` — first
action on a fresh branch from main (especially during a multi-PR
finalize like A → C → B → D) is `mise run check` BEFORE starting work;
per-commit U6 invariant inside a track-PR is not transitive across
squash boundaries when biome rule sets, transitive deps, or
cross-package test assertions drift between PRs.
### Opportunistic fixes (pre-existing main debt)
`mise run check` exited non-zero on a fresh cut from main (post Track A
squash) on 6 unrelated lint findings. Per the "fix problems you
encounter" tenet, swept inline:
- `packages/scip-ingest/src/derive.test.ts` — 4 `noNonNullAssertion` →
`assert.ok(edge)` + safe ref
- `packages/embedder/src/sagemaker-embedder.parity.test.ts` —
`noConsole` + suppress `noTemplateCurlyInString` on the deliberate
`${this.name}` fixture
- `packages/search/src/{bm25,hybrid}.test.ts` +
`packages/wiki/src/index.test.ts` — drop now-unused `biome-ignore
lint/correctness/useYield` suppressions on empty `async *` generators
- `packages/cli/src/commands/scan.test.ts` — `selectScanners` assertions
extended for the new polyglot P1 (cross-package coupling that
`scanners/catalog.test.ts` alone couldn't catch)
### Worktree cleanup
Pruned 14 orphan `.claude/worktrees/agent-*` worktrees from Track A's
parallel Act subagents — recurrence of
`worktree-isolation-pwd-pin-and-biome-exclusion`.
### Out of scope (still queued for follow-on PRs)
- Track C — debt sweep (parse-cache eviction, stringArrayField
asymmetry, SageMaker rebuild-on-switch refusal, SCIP REFERENCES +
TYPE_OF emission, 4 READMEs)
- Track D — dogfood polish (semgrep.yml, osv.yml split,
och-self-scan.yml, code-pack release asset, lefthook polish, mise
och:self-* tasks)
🤖 Squashed via
[bonk-ai](https://github.com/theagenticguy/ai-gateway/blob/main/scripts/bot-push.py).
Co-authored-by: bonk-ai[bot] <269762587+bonk-ai[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Track B — constraint-10 (detect-secrets as 20th scanner)
Closes the constraint-10 leg of v1-finalize per
.erpaval/specs/006-v1-finalize/.Spec: spec.md§Track B.
PR-split: pr-split-analysis.md — A → C → B → D ordering, this is leg 2 of 4.
What changes
DETECT_SECRETS_SPECadded topackages/scanners/src/catalog.tsbetween BANDIT and BIOME. Polyglot P1, Apache-2.0, pinned at v1.5.0 with a stale-since-2024 catalog comment.ALL_SPECS.lengthrises 19 → 20;P1_SPECSrises 11 → 12.createDetectSecretsWrapper(packages/scanners/src/wrappers/detect-secrets.ts) invokesdetect-secrets scan . --all-filesand pipes stdout JSON through the converter. Empty SARIF +skippedon missing binary; empty SARIF on malformed stdout —tool.driver.namealways preserved (E-B-2).detectSecretsJsonToSarif(packages/scanners/src/converters/detect-secrets-to-sarif.ts) walks the per-fileresultsmap, maps detect-secretstype→ SARIFruleIdvia a 25-row table covering every v1.5.0 detector, with slug fallback for future detectors. SHA-1hashed_secretstamped onpartialFingerprints.detect_secrets_sha1— explicitly not advertised as a cryptographic fingerprint per W-B-1. Overlapping findings (KeywordDetector + AWSKeyDetector on the same line) pass through without dedupe per W-B-2; OCH's downstream SARIF dedupe handles merge. Every emitted log validated againstSarifLogSchemabefore return.AC summary (Track B — 2 of 2)
DETECT_SECRETS_SPECadded to catalog (19→20); P1 stable-order updated; cliselectScannerstest extended for the new polyglot P1Validation
@opencodehub/scanners(was 80; +13 new), all green.pnpm -r exec tsc --noEmitclean.bash scripts/check-banned-strings.shPASS.mise run checkexits 0.SarifLogSchemaat the conversion boundary.partialFingerprints.detect_secrets_sha1slot is plugin-defined identifier per SARIF §3.27.18, not a crypto claim.tool.driver.name = "detect-secrets"preserved.Compound lessons extracted
1 durable lesson written to
.erpaval/solutions/:best-practices/squash-merge-masks-pre-existing-debt.md— first action on a fresh branch from main (especially during a multi-PR finalize like A → C → B → D) ismise run checkBEFORE starting work; per-commit U6 invariant inside a track-PR is not transitive across squash boundaries when biome rule sets, transitive deps, or cross-package test assertions drift between PRs.Opportunistic fixes (pre-existing main debt)
mise run checkexited non-zero on a fresh cut from main (post Track A squash) on 6 unrelated lint findings. Per the "fix problems you encounter" tenet, swept inline:packages/scip-ingest/src/derive.test.ts— 4noNonNullAssertion→assert.ok(edge)+ safe refpackages/embedder/src/sagemaker-embedder.parity.test.ts—noConsole+ suppressnoTemplateCurlyInStringon the deliberate${this.name}fixturepackages/search/src/{bm25,hybrid}.test.ts+packages/wiki/src/index.test.ts— drop now-unusedbiome-ignore lint/correctness/useYieldsuppressions on emptyasync *generatorspackages/cli/src/commands/scan.test.ts—selectScannersassertions extended for the new polyglot P1 (cross-package coupling thatscanners/catalog.test.tsalone couldn't catch)Worktree cleanup
Pruned 14 orphan
.claude/worktrees/agent-*worktrees from Track A's parallel Act subagents — recurrence ofworktree-isolation-pwd-pin-and-biome-exclusion.Out of scope (still queued for follow-on PRs)
🤖 Squashed via bonk-ai.