Skip to content

feat: detect-secrets as 20th scanner (Track B)#72

Merged
theagenticguy merged 1 commit into
mainfrom
feat/v1-finalize-track-b
May 9, 2026
Merged

feat: detect-secrets as 20th scanner (Track B)#72
theagenticguy merged 1 commit into
mainfrom
feat/v1-finalize-track-b

Conversation

@bonk-ai
Copy link
Copy Markdown
Contributor

@bonk-ai bonk-ai Bot commented May 9, 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.
PR-split: 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 noNonNullAssertionassert.ok(edge) + safe ref
  • packages/embedder/src/sagemaker-embedder.parity.test.tsnoConsole + 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.tsselectScanners 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.

@bonk-ai bonk-ai Bot force-pushed the feat/v1-finalize-track-b branch from f6a0fe1 to ac9d628 Compare May 9, 2026 20:11
Copy link
Copy Markdown
Owner

@theagenticguy theagenticguy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@theagenticguy theagenticguy merged commit 0f0018a into main May 9, 2026
17 checks passed
@theagenticguy theagenticguy deleted the feat/v1-finalize-track-b branch May 9, 2026 20:17
@github-actions github-actions Bot mentioned this pull request May 9, 2026
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>
@github-actions github-actions Bot mentioned this pull request May 11, 2026
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.

1 participant