Wire gitleaks / shellcheck / script self-test CI gates — PR #177 Major

[jgruberf5]

Addresses the CI-hygiene Major from @bonnyr-f5's #177 review: .gitleaks.toml was config nothing ran, make shellcheck had no caller, and compute_version_bump.sh's SELF_TEST was never invoked — so the release-critical scripts were statically unchecked and the secret-scanning gate was unwired for a public repo.

Three new P1 jobs, all wired into the CI gate:

• Secret Scan (gitleaks) — --no-git over the tracked source. Extended .gitleaks.toml's allowlist to cover generated build artifacts (.pyc, frontend-v2/dist/) alongside the existing synthetic-key entries. Verified clean locally even with artifacts present (the 7 raw hits are all build artifacts; git-tracked source is clean).
• ShellCheck — make shellcheck over the whole corpus. Fixed the only two pre-existing findings that would have blocked the gate: a missing shebang in get_dpu_pwd.sh (SC2148) and unused for i counters in ibm_cloud_bnk_forge.sh (SC2034 → for _). Corpus clean at --severity=warning.
• Script Self-Tests — SELF_TEST=1 compute_version_bump.sh, which exits non-zero on failure (that behaviour ships with Fix non-deterministic version derivation (SIGPIPE race) — PR #177 Blocker 1 #179), so a regression in the version-deciding logic fails CI.

The release-critical scripts (compute_version_bump, extract-breaking-changes, publish-signed-images) already pass shellcheck cleanly. YAML validates.

Depends on #179 for the SELF_TEST non-zero exit to actually gate.

https://claude.ai/code/session_01UpRYiFserdBE5ESHn759N4

[mwiget]

The three jobs are real and I confirmed they do work rather than pass vacuously. One change to the allowlist needs to come out before this lands.

Verified working

• gitleaks genuinely scans: scanned ~28577099 bytes (28.58 MB) in 8.67s / no leaks found. Not an empty-scan pass.
• ShellCheck job is green and the corpus really is covered -- make shellcheck globs upgrade.sh scripts/*.sh vm-bnk-forge/*.sh vm-bnk-forge/lib/*.sh, and all four expand here (17 tracked scripts/*.sh, plus vm-bnk-forge/{make-vm,destroy-vm,render-cloud-init}.sh and lib/render.sh). I checked because an unexpanded glob would have made the new gate fail on arrival.
• Gate wiring matches the existing convention, actions/checkout@v6 is consistent, and the for i -> for _ / missing-shebang fixes are correct.

Blocking

frontend-v2/dist/.* should not go in the global allowlist. Three reasons, in order of weight:

1. .gitleaks.toml's own header is explicit: "Do NOT add blanket directory allowlists — that is how a real credential hides. Scope each entry to a rule + a specific path." These three entries are exactly blanket directory allowlists, and they went into the top-level [allowlist], which suppresses every rule -- the default private-key and cloud-credential detectors included -- not just generic-api-key.
2. A built frontend bundle is the single highest-value place to scan, not to skip: VITE_*-style build-time injection is the way a secret ends up in plaintext in a JS bundle. Allowlisting the directory permanently blinds the scanner to that whole class.
3. It buys nothing in CI. The job runs docker run ... immediately after actions/checkout with no build step, and frontend-v2/dist/, __pycache__/ and *.pyc are all gitignored with zero tracked files -- so none of those paths exist in the scanned tree. I confirmed against the branch. The entries only affect a developer scanning a dirty working tree.

If the goal is a clean local run (which is a fair goal), that belongs in the local invocation rather than in the committed config that also governs the CI gate. .pyc/__pycache__ I'd argue are defensible on their own terms -- compiled bytecode of sources that are scanned anyway -- but frontend-v2/dist/ I'd want gone, and whatever stays should carry the same "reviewed, here's why" framing the existing entries have.

Non-blocking

• ghcr.io/gitleaks/gitleaks:latest is unpinned for a security gate. Pin a tag.
• script-selftests currently cannot fail (staging's SELF_TEST exits 0 regardless -- you flagged the #179 dependency). Worth knowing it stays partly toothless even after #179: see my review there, the SIGPIPE case in that suite passes with the fix fully reverted, so this job would go green on the regression it exists to catch.
• This conflicts with #180 -- both insert into the same needs: list and gate loop in ci.yml. Whichever lands second needs a rebase.

Everything else here I'd approve as-is.

[mwiget]

This is the one thing I'd like removed before merge.

The header of this file says, twelve lines up:

Do NOT add blanket directory allowlists — that is how a real credential hides. Scope each entry to a rule + a specific path.

These three are blanket directory allowlists, and they are in the top-level [allowlist] rather than [rules.allowlist], so they suppress every rule -- the default private-key, AWS, GCP and token detectors included -- not just generic-api-key. Every existing entry in this block is a single named file with a written-out reason; these are the first that aren't.

frontend-v2/dist/ is the one that actually matters. A built JS bundle is where a build-time-injected secret physically lands in plaintext -- a VITE_* variable baked in at build is the classic case -- so it is the last directory you'd want permanently exempt.

And in CI they do nothing at all. The job runs docker run straight after actions/checkout, with no build step; frontend-v2/dist/, __pycache__/ and *.pyc are gitignored (.gitignore:128 for dist) with zero tracked files between them, so none of those paths exist in the tree being scanned. I checked against this branch. They only change the result of a local scan over a dirty working tree.

If the intent is "make gitleaks is clean on my laptop after a build" -- fair -- that belongs in the local invocation, not in the committed config the CI gate reads from. .pyc/__pycache__ I could live with on their own merits (bytecode of sources that are scanned anyway), and if they stay they should carry the same reviewed-and-why framing as the entries above them. frontend-v2/dist/ I'd drop outright.

[mwiget]

Non-blocking: :latest on the gate that decides whether a secret reaches a public repo. A gitleaks release can add, change or drop rules, so the same commit can scan differently tomorrow -- either breaking CI out of nowhere or quietly narrowing coverage, which is the worse direction and the one nobody notices. Pin a tag (ghcr.io/gitleaks/gitleaks:v8.x.y) and bump it deliberately.

Two small ones while you're here: the mount can be read-only (-v "$PWD:/repo:ro") since detect only reads, and with --redact set, a finding prints a redacted value plus the file and line -- which is what you want, so no change needed there, just confirming it's deliberate.

For the record, this job is doing real work: scanned ~28577099 bytes (28.58 MB) in 8.67s / no leaks found.

[mwiget]

Worth stating plainly since this job is the reason #179 is a dependency: it currently cannot fail. On staging's script a FAIL: still exits 0, so this is a green light regardless of outcome until #179 lands. The run on this PR shows 6 PASS and exit 0 -- fine, but it would have shown exit 0 with six FAILs too.

The part that isn't fixed by #179 landing: I reverted the SIGPIPE fix entirely at #179's head and ran that suite -- 7/7 PASS, exit 0. The one case meant to pin the version-derivation bug uses a single-line body, and grep can't match a line before reading all of it, so no SIGPIPE ever occurs and the case passes on the bug. Until that test changes shape, this job gates the presence of a self-test rather than the behaviour it's named for. Details and a verified replacement are on #179.

No change needed here -- just don't count this as covering the release-critical path yet.

[jgruberf5]

Fixed — frontend-v2/dist/.* is out of the allowlist. You're right on all three counts: a built bundle is where VITE_* injection lands so it's the last thing to skip, a top-level [allowlist] path suppresses every rule, and it bought nothing in CI (dist is gitignored, absent from a fresh checkout). The .pyc/__pycache__ entries stay with the "reviewed, here's why" framing. Also pinned the gitleaks image to v8.21.2.

On the toothless note: once #179 lands, its rebuilt Test 7 catches the SIGPIPE regression on CI's GNU grep, so this job stops being a no-op. And yes — this and #180 both edit the gate needs:/loop, so whichever merges second rebases.

[mwiget]

The allowlist change is exactly right and I'm glad the reasoning went into the comment rather than the PR description. The pin, though, has broken the gate -- and for an interesting reason worth knowing before you pick a replacement.

Fixed

frontend-v2/dist/.* is gone, and the surviving .pyc/__pycache__ entries now carry the same reviewed-and-why framing as the entries above them, including the point that they're gitignored so a fresh checkout has none. That's the right shape.

Blocking: the pinned version is the problem, not the config

P1 · Secret Scan (gitleaks) is red: 13 findings, all RuleID: private-key. I reproduced it locally against d48648dc and then isolated the cause:

	with your .gitleaks.toml	with no config at all
v8.21.2 (your pin)	13 findings	13 findings
v8.30.1 (:latest today)	0 findings	0 findings

The config is irrelevant -- it's the private-key rule itself, which was tightened somewhere in the nine minors between those versions. All 13 are PEM headers wrapped around a literal ..., e.g.

placeholder={'-----BEGIN OPENSSH PRIVATE KEY-----\n...\n-----END OPENSSH PRIVATE KEY-----'}

svc.validate_private_key("-----BEGIN RSA PRIVATE KEY-----\n...\n-----END RSA PRIVATE KEY-----")

-- UI placeholder text and test literals, no key material anywhere. The newer rule is simply better at that, so you've pinned to a version whose accuracy regressed.

Pin v8.30.1. I confirmed it's clean on this tree, and -- since "newer rule finds fewer things" deserves a second look -- I checked it hasn't just gone blind: a throwaway ssh-keygen -t ed25519 key dropped into a scratch directory is caught by both versions (1 finding each). So v8.30.1 is more precise, not weaker. (The canary was shredded immediately; nothing of it exists.)

Pinning was still the right call, and this is a good demonstration of why: the same tree scanned two ways gave 0 and 13. It just needs to name the version you actually validated against.

Still standing from the last review

• script-selftests gates the presence of a self-test rather than its behaviour -- though #179 at 0f8fe8fd fixes that side: I verified its Test 7 now fails when the SIGPIPE fix is reverted, so once #179 lands this job earns its keep.
• Conflicts with #180 (now approved) -- same needs: list and gate loop. Whichever lands second needs a rebase.

[mwiget]

This is what turned the gate red -- 13 findings, all RuleID: private-key.

I reproduced locally against d48648dc and isolated it. The config isn't involved:

	with .gitleaks.toml	with no config
v8.21.2	13	13
v8.30.1 (:latest today)	0	0

It's the private-key rule, tightened somewhere across those nine minors. Every one of the 13 is a PEM header around a literal ...:

frontend-v2/src/components/settings/SSHCredentials.tsx:430
  placeholder={'-----BEGIN OPENSSH PRIVATE KEY-----\n...\n-----END OPENSSH PRIVATE KEY-----'}

backend/tests/component/test_ssh_service.py:26
  svc.validate_private_key("-----BEGIN RSA PRIVATE KEY-----\n...\n-----END RSA PRIVATE KEY-----")

Six are production .tsx (textarea placeholders), seven are test literals. No key material in any of them.

docker run --rm -v "$PWD:/repo:ro" -w /repo ghcr.io/gitleaks/gitleaks:v8.30.1 detect \
  --source=/repo --config=/repo/.gitleaks.toml --no-git --redact --verbose

Verified clean on this tree. And because "newer version finds fewer leaks" is the kind of thing that ought to make you suspicious, I checked it hasn't gone blind: a throwaway ssh-keygen -t ed25519 key in a scratch directory is flagged by both v8.21.2 and v8.30.1, one finding each. v8.30.1 is more precise, not weaker. (Canary shredded straight after.)

The :ro above is the other small thing from last time -- detect only reads.

[mwiget]

This is the right outcome, and the comment does the important part: it says what was considered and rejected, so nobody re-adds frontend-v2/dist/ in six months thinking it was an oversight. Noting the gitignored/zero-tracked-files reasoning inline is what makes the two survivors defensible rather than just tolerated.

[mwiget]

This makes the gate green, and every one of the 13 really is a false positive -- I checked them all last round. But the remedy has a cost I don't think is worth paying, and it's a cost the alternative doesn't have at all.

What the new entries actually do

They're in the top-level [allowlist] paths, so they suppress every rule in those files, not just private-key. Six of the twelve are production source, and they are the SSH-key-handling UI. I measured it: identical real ssh-keygen -t ed25519 key, two files, one allowlisted and one not.

$ gitleaks detect --config=.gitleaks.toml --no-git --redact
v8.21.2  -> leaks found: 1     # only the non-allowlisted copy
v8.30.1  -> leaks found: 1     # only the non-allowlisted copy

A live private key committed to frontend-v2/src/components/settings/SSHCredentials.tsx is invisible to this gate. The identical key one directory over is caught. Same for an AWS-shaped key -- the exemption isn't rule-scoped, so it's all of them. (Canaries were shredded immediately; nothing persisted anywhere.)

That's the file whose entire job is handling private keys. It's the last place I'd want permanently exempt, for the same reason frontend-v2/dist/ was.

And none of it is needed

The findings only exist because the pin is nine minors behind. On d48648dc -- your tree before these twelve entries -- I ran both:

	findings
v8.21.2	13
v8.30.1	0

Zero, with no allowlist entries at all. The newer private-key rule already distinguishes a PEM header wrapped around ... from real key material, which is exactly the judgement these twelve entries are hand-encoding. And it hasn't gone blind doing it: a throwaway ed25519 key is caught by both versions, as above.

Pin v8.30.1 and drop the twelve entries. One line instead of twelve, no exemptions, and the gate keeps watching the files that matter most.

If there's a reason to stay on v8.21.2 that I'm not seeing, then at minimum these need to move out of the global [allowlist] into a private-key-scoped [rules.allowlist], so a hardcoded token in SSHCredentials.tsx is still caught even if a key isn't. But that's the worse branch of the fork -- it still leaves private keys invisible in the private-key UI, and it still carries twelve entries that v8.30.1 makes unnecessary.

The catalogue comment itself is good work and the per-file scoping is the right instinct -- I'd just rather not need it.

[mwiget]

These are in the top-level [allowlist], so they suppress every rule in these files -- not just private-key. Six are production source, and they're the SSH-key-handling UI.

Measured, identical real ssh-keygen -t ed25519 key written into two files, one allowlisted (SSHCredentials.tsx) and one not:

v8.21.2  -> leaks found: 1     # only the non-allowlisted copy
v8.30.1  -> leaks found: 1     # only the non-allowlisted copy

Both versions miss the allowlisted copy. A live deploy key committed to SSHCredentials.tsx, or an AKIA... in DpuFormDialog.tsx, now reaches a public repo with a green gate. (Canaries shredded immediately.)

The part that makes this avoidable: these twelve exist only because the pin is nine minors behind. On d48648dc -- this branch one commit ago, with none of these entries -- v8.30.1 reported 0 findings. The newer private-key rule already tells a PEM header around ... apart from real key material, which is precisely the judgement being hand-encoded here.

ghcr.io/gitleaks/gitleaks:v8.30.1

and delete lines 57-71. One line instead of twelve, and the key-handling files stay under the scanner.

If v8.21.2 has to stay for a reason I can't see, these should at least move into a private-key-scoped [rules.allowlist] rather than the global one, so a hardcoded token in these files is still caught. That's strictly worse than pinning forward, but it's better than a blanket exemption on the SSH UI.

[jgruberf5]

Reworked the gitleaks fix using your version data — thank you, that table was the key.

Re-pinned :v8.21.2 → :v8.30.1. You showed v8.21.2 flags 13 private-key false positives on these PEM-header placeholders while v8.30.1 flags 0 — the rule was tightened across those minors. So rather than catalogue false positives against an old scanner (I'd briefly added 13 allowlist entries — you'd have rightly objected, since they suppressed all rules on production key-input components), I dropped them entirely and pinned the scanner that gets it right. Verified: clean tracked-only checkout scans no leaks found on v8.30.1 with no new allowlist entries.

Also from your nits:

• frontend-v2/dist/.* allowlist — removed (your blocker); a built bundle is where VITE_* injection lands.
• Mount is now read-only (-v "$PWD:/repo:ro") — detect only reads.
• --redact is deliberate (redacted value + file + line), confirmed, no change.
• The .pyc/__pycache__ entries stay with the reviewed-and-why framing.

On script-selftests being toothless: fixed on #179 — its rebuilt Test 7 (early marker + multi-line tail) now FAILs against the reverted loop on GNU grep, so this job stops passing on the bug once #179 lands.

[mwiget]

Approving at 5741c345. All 28 checks green, including P1 · Secret Scan (gitleaks), P1 · ShellCheck, P1 · Script Self-Tests and the CI Gate.

The pin moved to v8.30.1, the twelve allowlist entries are gone, and the mount is :ro. .gitleaks.toml is now byte-identical to d48648dc, the tree I'd already measured, and I re-ran the exact command CI runs against this head:

INF scanned ~28577103 bytes (28.58 MB) in 1.23s
INF no leaks found

Zero findings, zero exemptions on the SSH-key-handling files. That's the outcome I was after: the gate stays pointed at the code where a leaked key would actually matter.

Worth recording what this exercise established, because it's the argument for pinning in the first place: the same tree scanned 0 under v8.30.1 and 13 under v8.21.2, and every one of the 13 was a PEM header wrapped around a literal .... Pinning is right; the version just has to be one you validated against, and it wants re-validating when it's bumped rather than bumped on faith.

Standing notes, neither blocking:

• script-selftests earns its keep now that #179 is approved -- I verified its Test 7 fails when the SIGPIPE fix is reverted, so this job gates behaviour rather than the mere presence of a self-test. Before #179 lands it's still a green light regardless of outcome.
• Conflicts with #180 (also approved now) in ci.yml -- same needs: list and gate loop. Whichever merges second needs a trivial rebase.

[bonnyr-f5]

Review: REVISE

Reviewed under the review-discipline pipeline (invariant sweep + context-isolated cold audit at 5741c34, base 4a52ed4). Two of the three new gates were confirmed to actually fail on planted defects, which is the right bar. Three things to fix.

Verified genuinely blocking

ShellCheck gate. make shellcheck → shellcheck --severity=warning upgrade.sh scripts/*.sh vm-bnk-forge/*.sh vm-bnk-forge/lib/*.sh (Makefile:464-467, no || true). Exit 0 at this ref; planted SC2034 in a scripts/*.sh file → make: *** [shellcheck] Error 1. Failure propagates.

Also worth noting the get_dpu_pwd.sh shebang is a real finding, not cosmetic — base emits SC2148 (error), above the warning threshold, so the gate would have failed without it. Confirmed the base tree is red (SC2148 + SC2034) and this branch is the only one of the five that makes it green. The for i → for _ renames are safe (no $i references remain).

gitleaks gate. Ran the exact CI invocation: no leaks found, 28.58 MB, exit 0. Falsification tests all pass — planted AWS-shaped key in a .py → detected, exit 1; planted Slack token in a .md → detected, exit 1; invalid TOML → FTL unable to load gitleaks config, exit 1; missing config → exit 1.

Allowlist scoping proven by test rather than by reading: planting five secret shapes in both backend/tests/unit/ and a neutral directory shows [rules.allowlist] correctly attaches to the generic-api-key rule only — private-key and slack-bot-token still fire inside backend/tests/**. The scanner is not blinded. The new __pycache__ / .pyc entries do land in the global [allowlist] as intended, and your "zero tracked files" claim is true (git ls-tree -r | grep -cE '__pycache__|\.pyc$' → 0).

Blocker (merge-order) — script-selftests cannot fail unless #179 lands first

SELF_TEST=1 bash scripts/compute_version_bump.sh prints FAIL: but the base harness has no counter and no exit 1, and run_test swallows the recursive invocation's status with || true. Injecting a wrong expectation into the base script:

  FAIL: feat commit → minor
        expected bump=major ver=9.9.9
        got     bump=minor ver=1.3.0
=== END SELF-TEST ===
EXIT=0

ci-gate treats only failure/cancelled as bad, so a broken version-derivation regression would merge green — on the very logic this gate is meant to protect. #179 adds exactly the missing plumbing (its commit is titled "make SELF_TEST exit non-zero on failure so CI can gate it"); against that harness the same injection exits 1. Land #179 first and this resolves itself with no change here. Flagging it because in the other order the job is decorative from birth.

Major — the secret-scan gate never runs on doc-only changes

ci.yml:24-49 — both pull_request and push carry paths-ignore: ['**.md', 'docs/**', '.agent/**', '.opencode/**', 'LICENSE', '.gitignore', '.trivyignore', 'USER_GUIDE.md']. When every changed file matches, the whole workflow is skipped, so secret-scan never runs. A token pasted into a .md or anywhere under docs/ merges into a public repo with zero scanning — and markdown is precisely where illustrative-looking-but-real tokens land. The pre-existing config already allowlists cli/README.md and docs/archive/OPERATOR_ARCHITECTURE.md as known false positives, which confirms docs here do carry token-shaped content. Consider giving secret-scan its own workflow without paths-ignore, or adding a pull_request trigger that ignores nothing for this job.

Minor

• Coverage is narrower than the job names imply. 13 tracked .sh files plus the two extensionless .githooks/* are outside the ShellCheck globs — notably dist/install.sh (the customer installer) and all four bin/local/*.sh. I ran --severity=warning over all of them: zero findings, so this is a latent scope gap, not a live defect. Similarly script-selftests self-tests exactly one script, while Fix non-deterministic version derivation (SIGPIPE race) — PR #177 Blocker 1 #179 also changes extract-breaking-changes.sh and Sync Helm chart tag / appVersion / package.json with VERSION — PR #177 Blocker 2 #180 adds sync-version-artifacts.sh — neither is exercised. Per INV-9's own rule, a gate narrower than its claim can't back the claim: either widen the globs or rename the job to compute_version_bump self-test.
• Neither gate covers workflow run: blocks. Release workflow: republish recovery path + RC/loop-guard robustness — PR #177 Blocker 3 #181 adds ~40 lines of new release shell carrying an explicit "must NOT set -o pipefail" hazard, linted by nothing. An actionlint step would close this; it reports no new findings vs base today, so it can be added green.
• ShellCheck is installed unpinned (apt-get install -y shellcheck) while gitleaks is deliberately pinned to v8.30.1. The runner's version therefore drifts independently of the repo, and local ≠ CI. I re-ran the merged tree under 0.9.0 and 0.10.0 as well as 0.11.0 — all exit 0 — so nothing is broken now, but please pin it for the same reason you pinned gitleaks.
• detect --no-git scans the working tree only. Combined with actions/checkout@v6's default fetch-depth: 1, a secret introduced and later removed in an earlier commit is invisible. For a public repo the history is the exposed surface.

Nits

• .gitleaks.toml:3-5 says "Do NOT add blanket directory allowlists… Scope each entry to a rule + a specific path." The new .*__pycache__/.* is a global, un-rule-scoped wildcard directory allowlist. Defensible for bytecode, but it contradicts the file's stated policy without amending it.
• needs: changes on all three new jobs is inert — none consume needs.changes.outputs.* and all carry if: always(), so it only serialises them behind changes.
• scripts/get_dpu_pwd.sh gained a shebang but stays mode 100644.
• The gate wiring itself is correct: all three jobs appear in both ci-gate.needs and the decision loop, YAML parses (27 jobs), no duplicate keys, no undefined needs: references, and injecting failure for each new job blocks the gate. The blocker above is purely the script's exit code, not the plumbing.

Registry note

INV-9's recorded Sweep/Freeze does not match this repo — the registry claims CI runs shellcheck … bin/*.sh bin/lib/*.sh bin/hooks/*.sh tests/*.sh scripts/test/*.sh, and none of those paths exist here. This PR is the right place to make the registry entry name the real gate.

Merge order

Land fourth — after #179 (so script-selftests is real), and before #180. This PR conflicts with #180 in ci.yml (4 hunks); see my note there. It carries the fixes for the two pre-existing ShellCheck findings, so it is self-consistent in any position.

[mwiget]

Re-approving at ff93ab39 — 28/28 green. Both changes verified.

Dropping the workflow-level paths-ignore is correct, and the per-job filters really do keep the cost down. I checked the three jobs that must see every change: shellcheck, secret-scan, and script-selftests all sit on needs: changes + if: always() with no needs.changes.outputs.* condition, while every expensive job (unit/component/integration/docker/migration) still carries one. So a doc-only PR now runs three cheap jobs plus changes and the gate, and skips the rest — which is what the comment claims. CI Gate treats skipped as acceptable, so the doc-only shape passes rather than hanging.

There's a bonus this earns that the commit message doesn't claim: with the trigger-level ignore gone, a doc-only PR now produces a CI Gate conclusion instead of producing no checks at all. Under branch protection a skipped-by-paths workflow leaves the required check permanently pending — the classic unmergeable-docs-PR trap. Moot today (#185: staging has no protection) but it stops that from becoming a second bug the moment protection is turned on.

The self-test guard works, and it's genuinely needed on this branch. scripts/compute_version_bump.sh here prints FAIL: $desc at line 199 and has no failure counter and no exit 1 — I confirmed the harness exits 0 regardless. So before this change the job could not fail on a broken self-test, exactly as the comment says. Ran the real thing plus the false-positive cases:

$ SELF_TEST=1 bash scripts/compute_version_bump.sh   → rc=0, 6× "PASS:", grep clean ✅
'  FAIL: some test desc'    → matches ✅ (leading whitespace)
'FAIL: at line start'       → matches ✅
'PASS: FAILS: nope'         → no match ✅
'PASS: 6 passed, 0 failed'  → no match ✅

The (^|[[:space:]]) prefix is what keeps FAILS:/FAILED from tripping it. And this stays useful after #179 lands its exit-code fix — the two are independent checks on the same harness, which is the point.

One non-blocking follow-up, cross-PR: release.yml still carries the comment "MUST stay in sync with ci.yml's push-trigger paths-ignore list — a divergence lets a push trigger Release without a matching CI run". After this PR the lists diverge by design, and the comment now instructs the next editor to re-add exactly what you just removed. The divergence itself is safe — the direction is CI ⊇ Release, so a doc-only push runs CI and no release, never the reverse — but the comment should say that instead. #181 is already editing release.yml; whichever merges second could reword it.

Also worth folding into the same script-selftests job: #179 adds extract-breaking-changes.sh --self-test, which nothing currently invokes.

[jgruberf5]

Rebased this branch onto #180 (fix/version-artifact-consistency) to defuse the ci.yml merge conflict bonnyrf5 flagged (#180↔#182, "shaped to resolve silently wrong"). The conflict is now resolved once, correctly, in the branch: ci.yml carries both version-consistency (from #180) and shellcheck / secret-scan / script-selftests (from this PR) as separate jobs, and the ci-gate needs: list and its result-check loop list all four — no job is silently dropped.

Merge order: this PR now sits on top of #180, so merge #180 first, then this one (its diff collapses to just its own changes once #180 lands). YAML validates and actionlint is clean.

[jgruberf5]

@bonnyr-f5 — BLOCKER fixed in d357e40. You were right on every point: the gate ran the container but asserted nothing, so a git error produced identical green output to a real clean scan. I reproduced both failure modes in throwaway git fixtures before touching the code.

Reproduction (gitleaks v8.30.1, real containers)

Bad revision range — --log-opts=deadbeef..cafebabe:

ERR [git] fatal: ambiguous argument 'deadbeef..cafebabe': unknown revision ...
INF 0 commits scanned.
INF no leaks found
docker rc=0

Dubious ownership — root container over a checkout owned by another uid, no safe.directory (-u 0:0 -e HOME=/nonexistent):

ERR [git] fatal: detected dubious ownership in repository at '/repo'
INF 0 commits scanned.
INF no leaks found
docker rc=0

Both green, both scanning nothing — 0 commits scanned indistinguishable from 11 commits scanned, exactly as you described.

The fix (your class fix)

1. Root-cause the ownership path so it can't short-circuit in the first place. Whitelist /repo via GIT_CONFIG_COUNT=1 / GIT_CONFIG_KEY_0=safe.directory / GIT_CONFIG_VALUE_0=/repo env forwarded into the container. I used env rather than git config --global deliberately: it needs no writable HOME, so it holds even under the hostile HOME=/nonexistent mount. Verified — the same root-mount that printed dubious ownership above now prints 2 commits scanned.

2. Keep the assertion regardless, so a future breakage is caught rather than masked. Capture the output and fail on: any ERR [git] line; a missing commits scanned line (silence must not pass — same doctrine as script-selftests' PASS/END markers); 0 commits for a non-empty range; and a non-zero gitleaks exit (leaks found).

Verification of the shipped step body (exact block, real containers)

Case	Result
bad range	FAIL (git error)
dubious-ownership, hostile root mount	PASS, 2 commits scanned (safe.directory fix)
valid range	PASS, 2 commits scanned
full history (empty range)	PASS, 3 commits scanned
planted OpenSSH private key	FAIL, leaks found: 1, rc=1

Defense-in-depth check — with the safe.directory env removed (fix regressed) under the hostile root mount, the assertion still FAILs on ERR [git] rather than passing blind. So the gate can distinguish clean-scan from didn't-run in every case.

Validation: yaml.safe_load OK; bash -n on the extracted run block OK; actionlint finding count unchanged (9 pre-existing, all at line 1132 in an unrelated job — zero new from this step).

The Major/Minor/Nit items (archive depth, scheduled baseline, needs.changes.result, local make targets, stale release.yml comments, extractor self-test wiring, digest pin) are acknowledged and out of scope for this blocker commit.

[jgruberf5]

@bonnyr-f5 — round-3 follow-up: every remaining item is now addressed in cf64faf0. Each fix was reproduced and mutation-tested (broke what it guards, confirmed red), then verified green in real CI on this head.

Major — tracked archives never scanned

--max-archive-depth 2 added to the scan. Synthetic fixture (private key inside a .tar.gz):

depth 0 (default):  1 commits scanned / no leaks found        <- invisible
depth 2 (fixed):    1 commits scanned / leaks found: 1        (secrets.tar.gz!id_rsa)

Major — no baseline / full-history scan

New workflow .github/workflows/secret-baseline.yml: schedule (weekly cron) + workflow_dispatch, runs gitleaks over full history (RANGE="") with the same assertion backstop. Ran the baseline manually against a normal clone: 1006 commits scanned / no leaks found — history is clean today, and now stays monitored.

Major — CI Gate never checked needs.changes.result

The gate now fails when change-detection did not succeed. Mutation-tested by rendering the real gate script with changes=failure and the path-gated test jobs skipped:

OLD gate (guard removed):  rc=0  -> CI Gate PASSED   <- the bug you described
NEW gate (with guard):     rc=1  -> CI Gate FAILED   (change-detection did not succeed)

Major — make pre-push != CI (#166)

The scan + its entire assertion backstop now live in scripts/secret-scan.sh, called byte-identically by the CI job, the baseline workflow, and make secret-scan. Added make targets version-check / secret-scan / commit-lint / script-selftests, aggregated as ci-gates, and pre-push now depends on it. make -n ci-gates shows all five gate commands; make version-check and make script-selftests pass locally.

Minor -> enforcement — the skip-CI marker rule

scripts/lint-commit-markers.sh + a commit-lint CI gate (wired into the aggregator's needs/loop/always-run set) + a .githooks/pre-push step. Fails any commit range carrying a CI-control marker, or a line-start prose form that would spuriously major-bump; a genuine BREAKING CHANGE: footer still passes. Eight mutation cases (marker in body/subject case-insensitive, bold heading, colon-less line, multi-marker → FAIL; clean commit, real footer, mid-line prose → PASS). Real CI: commit-lint scanned 14 commits → OK.

One correction with evidence: a blanket line-start ban on BREAKING CHANGE would reject both (a) the documented+self-tested conventional footer used for intended majors (compute_version_bump.sh:124, self-test 5), and (b) this branch's own commit 83982af4 ("assert the BREAKING CHANGE detector...") which mentions it as mid-line prose. So the gate anchors to line-start AND exempts plain footers — it passes this PR's 13-commit range while still catching the accidental forms. The greedier mid-line prose match is a detector issue owned by the version-tooling PRs, out of scope here.

Nit — pin the gitleaks image by digest

Pinned ghcr.io/gitleaks/gitleaks@sha256:c00b6bd0aeb3071cbcb79009cb16a60dd9e0a7c60e2be9ab65d25e6bc8abbb7f (your c00b6bd0…), v8.30.1 kept in a comment.

release.yml stale comments — both confirmed and corrected (text only, no logic change)

• :23-26 — correct: the comment referenced a ci.yml paths-ignore that no longer exists (ci.yml runs CI on every push to main/staging). Reworded to explain ci.yml is now a strict superset, so any release-triggering push still has a CI run for the preflight to find.
• :150-157 — correct, and the more serious one: it claimed cancel-in-progress: true, but ci.yml:48 sets it false for exactly main/staging. Reworded; the exit 1 (a cancelled run is stale) stays.

Real-CI verification on cf64faf0

P1 · Secret Scan (gitleaks)   success   14 commits scanned / no leaks found / secret-scan: OK
P1 · Commit Message Lint      success   scanned 14 commit(s) / commit-lint: OK
P1 · Version Consistency      success   (make version-check)
P1 · Script Self-Tests        success
P1 · ShellCheck               success   (both new scripts + the hook are shellcheck-clean)

secret-scan: 14 commits scanned (not a blind pass — the safe.directory env resolves the root-vs-runner ownership path on the real checkout). YAML parses; actionlint finding counts unchanged (ci.yml 9, release.yml 4 — all pre-existing, none from these steps); secret-baseline.yml is clean.

Cross-PR (left to merge order, as you noted)

The sync-version-artifacts.sh second-tag: reproduction is fixed in #180's head; compute_version_bump.sh's exit-0-on-fail in #179's head. Merge #180 first, merge-commit rather than squash.

[jgruberf5]

Follow-up (46d4ec78) to @bonnyr-f5's #179 r4 nit: #179 factors detection into _is_breaking_subject + _is_breaking_body functions (the body one is a paragraph-aware awk, not a single grep), so the regex-extraction parity check couldn't see the awk detector — and you were right that compute had no function to diff.

The parity step is now adaptive: when both scripts define _is_breaking_body it diffs the full bodies of both detector functions (guarding the awk too); on the pre-#179 tree (inline greps) it falls back to the regex extraction, so the gate stays meaningful on this PR before the stack merges. Verified: old tree → regex fallback passes; new scripts → function-diff passes + runs the extractor self-test (17 assertions); a one-char mutation of _is_breaking_body → exit 1 naming the differing function.

[bonnyr-f5]

Review: REVISE

Round 4, cold re-audit of cf64faf against origin/staging 4a52ed4 — whole diff (15 files,
+614/-48, 14 commits), fresh context. I ran real gitleaks v8.30.1 (the pinned digest, verified)
and shellcheck against self-contained sandbox clones with planted secrets, mutated the version
detector, and simulated the CI Gate aggregator under four failure topologies.

BLOCKER (latent) — the new commit-lint gate deadlocks the release promotion (INV-4)

scripts/lint-commit-markers.sh:39 has no exemption for the release bot's own commits. The bot
commits release: vX [skip ci]; the staging→main promotion range therefore contains a commit
that fails commit-lint → the CI Gate goes red → the release preflight refuses that SHA → main
never releases again. The gate failure is execution-proven (rc=1 on a release-bot commit
in-range). The promotion topology is inferred from release.yml + the documented branch layout,
and there are 0 release commits in history today, so it is latent rather than firing now — which is
why this is the one finding holding the PR at REVISE rather than BLOCK. It needs an exemption for the
machine identity before the first automated release, or the first promotion after this lands wedges.

Major findings (all execution-proven)

• ci.yml:244-282 — the paired-detector "INV-15" gate compares regex text, not behaviour. The
  two detectors already diverge in input (subject+body vs body only) and compute has a trigger
  extract lacks, so feat: BREAKING CHANGE … and feat!: … both bump major with empty notes
  while CI prints INV-15 OK. The positive-fixture half is a ::warning:: skip.
• scripts/lint-commit-markers.sh:30-36,69 — unresolvable range → silent fallback to 1 commit,
  prints the range it never scanned, rc=0 with a violation inside. Sibling secret-scan.sh fails on
  the identical input (asymmetry proven).
• Makefile:506-513 (claim at :477-482) — local gate diverges from CI on the anti-vacuity
  logic. A genuinely broken version detector makes make script-selftests/ci-gates/pre-push
  print FAIL: and exit 0; the CI step exits 1. (The "local gate ≠ CI command" escape class,
  again.)
• scripts/lint-commit-markers.sh:39 — misses GitHub's documented skip-checks: true trailer,
  which suppresses all gates while commit-lint passes.
• scripts/lint-commit-markers.sh:58-64 — the rejecter catches 2 of 8 marker shapes. Bullet /
  indented / two-space / blockquote BREAKING CHANGE: all pass lint and ship a spurious major
  publish (8-row table proven).
• scripts/sync-version-artifacts.sh — add/add conflict with Sync Helm chart tag / appVersion / package.json with VERSION — PR #177 Blocker 2 #180 under squash. Both PRs add
  the file; the repo allows and uses squash; a careless resolution reverts Sync Helm chart tag / appVersion / package.json with VERSION — PR #177 Blocker 2 #180's round-3 blocker
  fixes and operator-chart coverage (merge-tree both strategies + gh api settings proven).

Genuinely fixed, and verified by execution

secret-scan.sh fail-closed on 5/5 harness breaks (bad range, no docker, malformed/missing config,
0-commit range); INV-22 closed (add-then-remove caught, no --no-git left anywhere); --redact
effective so a failing public scan log doesn't leak the secret; --max-archive-depth 2 really finds
keys inside tarballs; full-history baseline clean (74 commits); ShellCheck now covers 41 files (was
23/37); INV-14 closed. The diff adds no credential literals; permissions: contents: read; repo
mounted :ro.

Cross-PR

Besides the #180 add/add above, #182 and #181 both edit release.yml and conflict
(merge-tree → CONFLICT). Resolve on an integration branch; don't land independently.

Review Assessment

• Verdict: REVISE
• Audit SHA: cf64faf
• Cold Audit Performed: Yes
• Invariants Verified: INV-4 VIOLATED (B1); INV-15, INV-16 (mixed), INV-21, INV-23 VIOLATED; INV-8, INV-9, INV-17, INV-20, INV-22 UPHELD. Proposed new class INV-28 (a gate that forbids a token must exempt the machine identity told to emit it).
• Git & Harness Cleanliness: Clean

Findings & Action Items

• Major (Blockers):
  • lint-commit-markers.sh:39: no release-bot exemption → CI Gate red on promotion → main never releases (latent; fix before first automated release)
• Minor (Non-blocking): ci.yml:244-282 INV-15 gate compares text not behaviour; :30-36,69 silent 1-commit fallback; Makefile:506-513 local≠CI; :39 skip-checks trailer; :58-64 2/8 shapes; sync-version-artifacts add/add with Sync Helm chart tag / appVersion / package.json with VERSION — PR #177 Blocker 2 #180
• Nits: mcp-tests off the gate; PR body documents removed --no-git; two new scripts disagree on the default range (17 vs 14 commits); BSD sed -i -E leaves *-E files

[jgruberf5]

@bonnyr-f5 — round-4 REVISE addressed. Head is now fb1e7933. Two files changed: scripts/lint-commit-markers.sh and Makefile.

BLOCKER (INV-4) — release-bot exemption: fixed

Deadlock reproduced first, against the pre-fix script, using the exact promotion topology you describe — a staging→main promotion range that contains the release bot's own VERSION-bump commit:

RANGE=<base>..<head>   # release-bot commit in-range
::error::commit <sha> (release: v1.2.3 [skip ci]): message contains CI-control marker "[skip ci]" ...
::error::commit-lint failed -- see markers above.
--- exit code: 1  (rc=1 => commit-lint FAILS => CI Gate red => preflight refuses SHA => main never releases) ---

Fix (lint-commit-markers.sh): commits whose subject matches ^release: (the machine identity release.yml emits) are exempt from the whole gate, while a human quoting a marker in any other commit is still caught. Same reproduction against the fixed script:

commit-lint: commit <sha> (release: v1.2.3 [skip ci]) is a release-bot commit -- exempt
commit-lint: OK
--- exit code: 0 ---

Mutation-test (the two you named, plus the full matrix below):

• release: v1.2.3 [skip ci] → PASS (exempt)
• human commit body/subject carrying the same marker → FAIL

Valid same-surface Minor findings, also fixed

• :30-36,69 silent 1-commit fallback — an explicitly-set but unresolvable RANGE now fails closed (rc=1, "the scan did not run"), matching secret-scan.sh, instead of silently linting the tip while printing a range it never scanned. A valid-but-empty range (base..base) still passes with scanned 0.
• :39 skip-checks: true trailer — now caught (^\s*skip-checks:\s*true), case-insensitive; the bracketed fixed-string list alone missed this documented commit-check trailer.
• :58-64 2/8 BREAKING CHANGE shapes — the opener detector now peels a leading run of markdown/quote/whitespace/bold before judging the line, so bold, bulleted (-), block-quoted (>), indented, and multi-separator shapes all FAIL, aligned to the version detector's BREAKING[[:space:] -]+CHANGE. The canonical column-0 footer (BREAKING CHANGE: / BREAKING-CHANGE:) stays ALLOWED, and lowercase breaking-change stays a valid escape.
• Makefile:506-513 local≠CI anti-vacuity — make script-selftests now mirrors ci.yml's step: it fails on a printed FAIL: line, not only on a non-zero exit. Proven: a deliberately broken detector makes the target exit non-zero (rc=2) instead of 0.

Mutation matrix — 21/21 green

release-bot exempt · human [skip ci] body/subject FAIL · [ci skip]/[skip actions] FAIL · skip-checks: true (incl. mixed-case) FAIL · bare/bold/bullet/blockquote/indented/two-space/hyphen-space BREAKING CHANGE FAIL · canonical BREAKING CHANGE:/BREAKING-CHANGE: footers PASS · lowercase escape PASS · mid-line prose mention PASS (out of scope, unchanged) · plain feat PASS · unresolvable RANGE FAIL · empty valid range PASS.

Validation: bash -n clean; shellcheck -S style clean; the actual PR range (4a52ed4..HEAD, 15 commits) still lints OK; my own commit lints OK (message uses only hyphenated / mid-sentence marker references).

Deferred with rationale (not fixed here)

• ci.yml:244-282 INV-15 (text-not-behaviour) — the compared detector token lives in compute_version_bump.sh / extract-breaking-changes.sh, which are owned by Fix non-deterministic version derivation (SIGPIPE race) — PR #177 Blocker 1 #179/Sync Helm chart tag / appVersion / package.json with VERSION — PR #177 Blocker 2 #180; the subject-vs-body input difference is by design (compute decides the bump incl. subject !; extract pulls notes from the body footer). This is a version-tooling contract, out of scope for a CI-hygiene PR — flagging here rather than editing another PR's scripts.
• sync-version-artifacts.sh add/add with Sync Helm chart tag / appVersion / package.json with VERSION — PR #177 Blocker 2 #180, and the Release workflow: republish recovery path + RC/loop-guard robustness — PR #177 Blocker 3 #181 release.yml conflict — merge-time cross-PR concerns, not code defects in this diff; agreed these resolve on an integration branch, not by landing independently.
• Nit: default-range wording (@{upstream}..HEAD vs merge-base..HEAD) — intentional: commit-lint should only judge commits unique to the branch.

CI is running on fb1e7933 (CI workflow in_progress). Not merging.

[bonnyr-f5]

Review: BLOCK

Round 5, cold re-audit of fb1e793 against origin/staging 4a52ed4. Auditor had no prior-round
context. The blocker is re-verified by me below.

Merges standalone: it merges, but it must not. fb1e793 is a linear descendant of 4a52ed4 with
no conflict, and every gate is green over its own PR range (make shellcheck rc=0, version-check
rc=0, secret-scan 16 commits rc=0, commit-lint 16 commits rc=0, script-selftests rc=0). Its
simulated GitHub squash-merge message also passes commit-lint. The problem is what the gate does to
the pipeline after the merge.

Round-4 B1 is fixed — and the same class survives one identity over

The ^release: bot exemption works; I confirmed it. But INV-28's class is "a gate that forbids a
token must exempt the machine identity told to emit it", and there is a second machine identity on
the promotion path: GitHub's own squash-commit composer.

BLOCKER-1 · commit-lint rejects the base commit — the current tip of staging

$ git log -1 --format='committer=%cn <%ce>  parents=%p' 4a52ed4
committer=GitHub <noreply@github.com>  parents=009840a

$ RANGE="009840a..4a52ed4" bash scripts/lint-commit-markers.sh
::error::commit 4a52ed4556... line "- BREAKING CHANGE footer in the BODY of a fix-subject commit → major (non-"
  starts a BREAKING CHANGE declaration that is not a plain Conventional Commits footer
commit-lint: scanned 1 commit(s) in range '009840a..4a52ed4'
::error::commit-lint failed -- see markers above.
rc=1

4a52ed4 is a real GitHub squash commit, it is the current tip of staging, it is the base of
all seven PRs in this series, and it is an ancestor of main. It is also unamendable — the body
was composed by GitHub from the PR description.

The chain: commit-lint fails → ci-gate red → release.yml:150-170 requires the ci.yml run
conclusion to be success → exit 1 → that SHA never releases. The auditor's sweep of the
repo's own 82-commit history found 2 rejected commits (4a52ed4, 7ece9b04); I confirmed the first
directly, and did not independently re-run the full-history sweep.

The gate needs to judge the footer position of human-authored commits, not any line in a
machine-composed body — or exempt commits whose committer is GitHub <noreply@github.com> with a
single parent.

Major-1 · the gate blocks sibling #181 outright

Two of #181's commits carry a literal [skip ci] in the body → rc=1, executed. And if #181 were
squash-merged anyway, the squash body carries the marker → GitHub suppresses all workflows for that
push → no CI run, no Release run, and the release is silently starved. That is the #179/#181
incident recreated one layer up, in the place the gate cannot see.

Major-2 · this PR adds a second, older copy of sync-version-artifacts.sh

Both #180 and #182 add scripts/sync-version-artifacts.sh with different content, and #182's is
the pre-fix variant carrying INV-19 verbatim — grep -m1 readers against a global sed writer,
post-verification re-reading with the same first-match helpers, no empty-VERSION guard, no vacuity
floor, no operator chart, no --list. Latent today (one match per pattern), but merge-hazardous:
merge-base(pr182, pr180) = 99a286f, merge-commit is clean, and squash gives an add/add conflict on
that file plus content conflicts in ci.yml / release.yml / AGENTS.md. This repo allows squash.
#182 ↔ #181 also conflicts on release.yml under both strategies.

Simplest resolution: drop this file from #182 and let #180 own it.

Major-3 · make script-selftests implements 2 of 4 anti-vacuity assertions

Contradicting its own comment. Breaking the harness rather than the code:

harness break	CI job	make
silenced guard	RED	GREEN
early exit 0	RED	GREEN
deleted END marker	RED	GREEN

It also never runs ci.yml's parity step. INV-16: the local target is not the gate it claims to be.

Major-4 · make pre-push is red on a clean staging checkout

Same root cause as BLOCKER-1 — the base commit fails the gate, so the documented local
pre-push command fails on an untouched tree.

Minors

• Parity gate green while feat!: → major 2.0.0 with empty release notes (executed) — the
  INV-15 class this gate claims to freeze.
• ^release: is a subject allowlist, not an identity check: a human spoofing that subject is
  exempt from every rule.
• commit-lint passes a 0-commit range while secret-scan.sh correctly fails it — inconsistent
  vacuity floors between two gates in the same PR.
• .githooks/pre-push lints @{upstream}..HEAD and so misses non-tip commits on a first push.
• sed -i -E litters *-E files on BSD.

Verified correct — executed, and I could not break these

The secret scan does catch add-then-remove in-branch (rc=1), so INV-22 is upheld. The private-key
allowlist is rule-scoped: a PAT planted in the allowlisted fixture file is still caught, a key
outside it is caught, the fixture key is suppressed. --max-archive-depth 2 works in git mode
(docs/bundle.tar.gz!inner/id_rsa). Both blindness assertions fire (ERR [git], 0 commits). Image
digest really is v8.30.1. ShellCheck coverage widened to 41/41 tracked shell files — green on this
ref, red on base, so the two SC2148/SC2034 findings are pre-existing on staging and fixed here.
The CI script-selftests job is red in all four harness-break modes. INV-14 satisfied in the right
direction (ci ignore-set ∅ ⊆ release's; per-SHA groups; cancel-in-progress false only on
main/staging). The CI Gate aggregator's needs-vs-loop is complete, and a non-success changes
now fails the gate.

This is good gate work — the secret-scan and shellcheck halves are the strongest verified pieces in
the series. BLOCKER-1 is a one-condition fix, but it has to land before this merges, because the
moment it does the pipeline stops releasing.

[bonnyr-f5]

Cross-PR merge-order constraints for this series are now tracked in #192.

Relevant here: two items. (1) This PR adds scripts/sync-version-artifacts.sh, which #180 also adds with different, newer content — simplest fix is to drop the file here and let #180 own it. (2) It conflicts with #181 on release.yml under both strategies, and its commit-lint gate rejects two of #181's commits outright.

[jgruberf5]

@/tmp/claude-1000/-mnt-d-project-bnk-forge/01a289e5-e845-497f-99d9-622be0bf9a98/scratchpad/comment.md

[jgruberf5]

Superseded by the consolidated integration PR #193 (branch pr177-integration), which merges this and the other six #177 follow-up PRs in the dependency order from #192 and passes full CI (CI Gate green). Per #192, these seven share the credential and release/CI surfaces and could not merge in arbitrary order, so they land together via #193.

Closing unmerged, not abandoning: the branch is retained and all review history stays on this page for reference. See #193 for the integrated, validated result and #192 for the cross-PR conflict analysis.