fix(ci): guard PR title/body — the one leak path no local hook can see - #418
Open
runyourempire wants to merge 1 commit into
Open
fix(ci): guard PR title/body — the one leak path no local hook can see#418runyourempire wants to merge 1 commit into
runyourempire wants to merge 1 commit into
Conversation
`.husky/commit-msg` guards locally authored messages; `.husky/pre-push` guards outgoing commits. Neither runs when GitHub performs a SQUASH MERGE, because that commit message is composed server-side from the PR title and body. That gap has already fired. Two commits on public `main` — `d5df9e34` (#409) and `053e5813` (#362) — carry the private external-verifier name in their MESSAGES. A scan of all 1,948 tracked files at `origin/main` returns zero flagged files, so the 2026-07-13 content scrub held; only this path regressed. Removing those two messages now needs a public-history rewrite. This stops the next one, which is the part still in our control. The check runs on `pull_request`, where the title/body are known BEFORE the merge button, and scans title + body + every commit message in the range via the existing hashed detector in scripts/private-asset-guard.cjs (no literal name is added anywhere). Wired into `validate-success`'s `needs`, so it gates through the existing required check — no branch-protection change required. Two deliberate choices: * FAIL-CLOSED. `.husky/pre-push` fails OPEN on tool error so a broken guard can never brick the fleet's ability to push. CI is the opposite trade: it blocks a merge, not a local loop, the failure is visible on the PR, and a re-run is cheap. A guard that silently cannot run is not a guard — and silent failure is the exact bug class this check exists to stop. * Title and body reach the script through the ENVIRONMENT, never `${{ }}` inlined into the `run:` block. PR text is attacker-controlled; inlining it is a script-injection primitive. Verified end-to-end against the real detector: * range containing `d5df9e34` -> BLOCKED, naming the offending commit (exit 1) * clean range (`adfb68ff`) -> passes (exit 0) * 7/7 unit tests (injected predicate, so no private literal in a tracked test) * workflow YAML parses; `validate-success.needs` includes `pr-metadata` Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WR8YdADiaw1p8pD8CddncD
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This was referenced Aug 13, 2026
runyourempire
added a commit
that referenced
this pull request
Aug 14, 2026
Today's outage made this concrete. #423 was a Rust-only PR, so the path filter skipped its Frontend job — and `scripts/check-file-sizes.cjs --ci` ran ONLY inside that job, even though it scans BOTH trees (SCAN_DIRS = ['src', 'src-tauri/src']). A 1032-line src-tauri/src/analysis_rerank.rs merged past the 1000-line hard error threshold with the gate never executing. check-file-sizes then exited 1 on main, and because .husky/pre-commit treats that as blocking, every developer in the fleet was unable to commit until #430 landed. A gate that guards Rust files must not be reachable only through a filter that excludes Rust. Adds a `repo-guards` job with NO path filter and NO `needs:`, so it runs on every pull request and dispatch whatever changed. It carries check-file-sizes, check-no-window-spawns, check-release-channel and the guard self-tests, which are removed from `frontend` — they were never frontend-specific. It needs no pnpm install (all three guards use only node builtins) and runs hosted in ~40s. It is in `validate-success`'s needs, so it actually gates the merge: since it never skips, it is the only leg guaranteed to have run. Also folds in `pnpm run test:scripts` — 53 tests across 6 files that verify the guards still detect what they claim, and which previously executed in no hook and no workflow. package.json is deliberately untouched: a peer holds a claim on it and #418 edits the same line, and wiring it here achieves the same goal. Rust job timeout 30 -> 45: that job now compiles the integration test targets, and Swatinem's cache is only saved from main, so until this lands every PR run pays a cold link for 5 extra binaries. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AUeKTKwNmdow8yUk3q8RB2
runyourempire
added a commit
that referenced
this pull request
Aug 14, 2026
…ncovered paths (#429) ## What this fixes Four CI gates reported success without doing the work they claim. Each was verified against live run data before anything was changed. ### 1. The integration tests had never run in CI — on any workflow Every `cargo test` in `.github/workflows/` was `--lib`-scoped (`validate.yml:303/308`, `hermetic.yml:167/176`). No `--tests`, no `--all-targets`. So **all 154 real integration tests in `src-tauri/tests/` had never executed in CI**, including all 12 migration tests and the repo's only forward-migration coverage — against a `TARGET_VERSION = 103` migration chain that has no checksums and no downgrade path. **I measured before changing anything.** Full `cargo test --tests` run on this branch's base (`a6ece843`), isolated data dir, exit code 0: | Target | Result | |---|---| | lib unittests | **4,290 passed**, 0 failed, 10 ignored | | `4da` (cli bin) | 13 passed, 0 failed | | `fourda` / `fourda-engine` bins | 0 tests each | | `migration_tests` | **12 passed**, 0 failed | | `pipeline_integration` | **13 passed**, 0 failed | | `source_resilience` | **5 passed**, 0 failed | | `stack_simulation` | **124 passed**, 0 failed | | `victauri_dogfood` | 157 passed, 3 ignored (self-skips without `VICTAURI_E2E=1`) | | **Total** | **4,614 passed across 9 binaries, 0 failed** | **Nothing was broken and nothing had to be excluded.** They are also hermetic by construction, which I verified rather than assumed: `pipeline_integration` uses an in-memory DB (`test_utils::test_db()` → `:memory:`) and `migration_tests` uses `tempfile::tempdir()`. Both workflows now run `--tests` (lib + bins + integration) under the same throwaway-data-dir isolation `hermetic.yml` already used. Two follow-on fixes were required to avoid landing a red gate: - The count floor took `tail -1` of the `test result:` lines. With `--tests` there are **9** test binaries, so it would have read the *last* binary's total (157) and tripped the 2000 floor on every run. It now **sums** all binaries. - A new assertion fails if fewer than 5 test binaries report — so if this is ever re-scoped to `--lib`, it fails loudly instead of silently dropping the integration suite again. >⚠️ **Non-obvious trap for reviewers:** the isolation directory name must keep containing the substring `data`. `src/state.rs::test_get_db_path_points_to_data_dir` asserts the resolved DB path contains `"data"`. Pointing `FOURDA_DATA_DIR` at e.g. `/tmp/4da-hermetic` makes that lib test fail; `…/4da-hermetic-data` passes. I hit this during measurement. Both call sites are commented. ### 2. The hermetic fresh-clone canary never ran outside PRs `fresh-clone` needs the PR-only `changes` job. GitHub skips any job whose `needs` was skipped **unless its `if:` contains a status function** — and `hermetic.yml:94` had none. So push-to-main, the nightly cron and manual dispatch all skipped the clone and reported success: | Trigger | Duration | Result | |---|---|---| | push → main (08-14 03:52) | **9s** | "success" | | push → main (08-13 16:42) | **7s** | "success" | | schedule (08-13 08:21) | **7s** | "success" | | pull_request (real work) | ~19min | success | The file's own comment at `:56-60` claimed these paths "ALWAYS run the full canary". **The nightly cron had never built a single fresh clone.** Fixed with `!cancelled()` — not `always()`, so a cancelled run doesn't spawn a 45-minute cold build. The **identical defect** silently disabled `workflow_dispatch` for Frontend, MCP Server and the entire Rust matrix in `validate.yml`: the `github.event_name == 'workflow_dispatch'` clause on those three jobs had never once fired, while `Validate Success` (`if: always()`) still went green. Same fix. ### 3. A path-filter hole took the whole fleet down today This stopped being hypothetical while this PR was being written: - **#423 was a Rust-only PR.** Its `Frontend` job was skipped by the path filter. - `scripts/check-file-sizes.cjs --ci` ran **only inside the Frontend job** — but it scans `SCAN_DIRS = ['src', 'src-tauri/src']`, i.e. it guards Rust files too. - So #423 merged a **1032-line `src-tauri/src/analysis_rerank.rs`** past the 1000-line hard error threshold, with the gate never executing. - `check-file-sizes.cjs` then exited 1 on `main`, and `.husky/pre-commit:38-41` treats that as blocking — **every developer in the fleet was unable to commit.** A gate that guards Rust files must not be reachable only through a filter that excludes Rust. This PR adds a **`repo-guards` job with no path filter and no `needs:`** — it runs on every PR and dispatch, and carries `check-file-sizes`, `check-no-window-spawns`, `check-release-channel` and the guard self-tests. They are removed from `Frontend` (they were never frontend-specific). Hosted, ~40s, no `pnpm install` needed: all three guards use only node builtins. It is also in `validate-success`'s `needs`, so it actually gates the merge — `repo-guards` is now the only leg guaranteed to have run. **Additionally, "no filter matched" now means RUN, not PASS.** `site/`, `paddle-webhook/`, `mcp-memory-server/`, `editors/vscode/` and `.husky/` matched no filter, while `Validate Success` is the only required check and auto-merge is enabled repo-wide — so a Dependabot bump into the payment webhook, or a PR weakening `.husky/` itself, could merge with nothing run. Added `.github/**` and `.husky/**` explicitly, plus an `uncovered` fail-safe filter that catches anything unrecognised **including directories added in future**. > The `uncovered` filter uses `predicate-quantifier: 'every'`, which is **required** — the default `'some'` ORs the patterns, and a list of negations OR'd together matches every file. Verified against the action's source at the pinned SHA (`src/filter.ts:110-113` → `patterns.every(...)`; `MatchOptions = {dot: true}`, so `.husky/**` matches). ### 4. The guards' own self-tests ran nowhere `pnpm run test:scripts` (53 tests across 6 files) executed in **no hook and no workflow** — nothing verified the guards still detect what they claim. `pnpm run validate` isn't run by CI either (the Frontend job runs its steps individually), so wiring it into `package.json` alone would not have gated it. It is now a step in `repo-guards`. ## Deliberately NOT done - **No branch-protection or ruleset change.** Making `Hermetic Success` required is the correct end state — currently `Validate Success` is the *only* required check in the active `main-protection` ruleset (verified via the API; classic branch protection is disabled). But with ~30 open PRs and Hermetic historically failing on #421, flipping it now would block the fleet. **Recommended as an explicit follow-up** once this lands and Hermetic is observed green on push-to-main for a few days — which, note, is the first time that signal will ever have existed. - **`package.json` untouched.** `test:scripts` was going to be added to the `validate` chain, but a peer worktree agent holds a claim on that file and #418 also edits that exact line. Wiring it into `repo-guards` achieves the real goal (it now runs in CI) without touching the claimed file. - **`analysis_rerank.rs` / `check-file-sizes.cjs` untouched** — a separate agent owns the immediate unblock. This PR fixes the structural cause only. - **No per-package jobs for `site/`, `paddle-webhook/`, `mcp-memory-server/`, `editors/vscode/`.** The `uncovered` fail-safe means they now trigger the generic gate instead of passing silently, but that gate does not *build* them. Dedicated jobs are the right follow-up and belong in their own PR. - **Rust job timeout raised 30 → 45 min.** Not cosmetic: this job now compiles the integration test targets, and Swatinem's cache is only saved from `main`, so until this lands there every PR run pays a cold link for 5 extra binaries. 30 was too tight for that first window, and a timeout on a required gate is a red gate. ## Conflicts with open PRs Checked `gh pr diff --name-only` on every PR touching these files: | PR | Overlap | Notes | |---|---|---| | #387, #350 | none | Dependabot `actions/checkout` SHA pins only — different lines | | #388 | none | `taiki-e/install-action` SHA pin only | | #424 | none | Adds 3 matrix legs at `validate.yml:236-255`; my edits are at 301+ and inside `steps:`. I deliberately did **not** add a matrix key — an integration-floor key would have had to be added to its new legs. Verified `cargo test --tests --features experimental` compiles clean, so its `test-floor: 0` compile-gate legs are unaffected. | | #418 | **1 line** | Both edit `validate-success`'s `needs:`. #418 adds `pr-metadata`, this adds `repo-guards`. Resolution is a union of the two lists — whoever merges second takes both. Flagged rather than pre-empted. | `uncovered` was also deliberately placed *before* the main filter step, to stay clear of the end-of-job boundary #418 inserts a job into. ## Live CI evidence from this PR's own run The first run of this branch already proves the fix, on both platforms: | Check | Result | |---|---| | Fresh clone (ubuntu-22.04) | **pass**, 12m22s | | Fresh clone (windows-latest) | **pass**, 18m21s | | Hermetic Success | **pass** | | Rust (default) | **pass**, 12m41s | | Rust (experimental) | **pass**, 12m02s | The hermetic job log shows **all 9 test binaries executing on both legs** — `migration_tests` 12, `pipeline_integration` 13, `source_resilience` 5, `stack_simulation` 124, `victauri_dogfood` 157, plus lib (4,290 windows / 4,284 ubuntu — a 6-test platform delta, far above the 2000 floor) and the 3 bin targets. **Zero failures.** That is the first time any of those integration tests has run in CI. Rust finished in ~12min against the old 30min cap, so the 45min bump is headroom for the first cold-cache window rather than a response to an observed timeout. ## Verification - Both workflows parse as YAML; all `if:` expressions and filter blocks inspected post-rebase. - `cargo test --tests` measured green in full **before** any workflow edit, and **re-run green after** rebasing onto current `main` (table above) — #421 removed ~54k lines and #423 changed pipeline code between those two runs. - `dorny/paths-filter` negation + `every` semantics confirmed from source at the pinned SHA, not assumed. - `check-no-window-spawns`, `check-release-channel`, `test:scripts` all verified exit 0 locally; `check-file-sizes` correctly exits 1 (the live outage above). - Rebased onto latest `main`; only the two workflow files differ. The `analysis_rerank.rs` unblock (#430) has landed, so `repo-guards` passes; this branch is rebased on top of it. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.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
1 participant
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.
.husky/commit-msgguards locally authored commit messages..husky/pre-pushguards outgoing commits. Neither runs when GitHub performs a squash merge — that commit message is composed server-side from the PR title and body.That gap has already fired. Two commits on public
main—d5df9e34(#409) and053e5813(#362) — carry the private external-verifier name in their messages. A scan of all 1,948 tracked files atorigin/mainreturns zero flagged files, so the 2026-07-13 content scrub held; only this path regressed. Removing those two messages now needs a public-history rewrite. This PR stops the next one, which is the part still in our control.What it does
Runs on
pull_request, where the title and body are known before the merge button, and scans title + body + every commit message in the range using the existing hashed detector inscripts/private-asset-guard.cjs. No literal name is added anywhere in this change.Wired into
validate-success'sneeds, so it gates through the existing required check — no branch-protection change needed.Two deliberate choices
Fail-closed.
.husky/pre-pushfails open on tool error, correctly — a broken guard must never brick the fleet's ability to push. CI is the opposite trade: it blocks a merge rather than a local loop, the failure is visible on the PR, and a re-run is cheap. A guard that silently cannot run is not a guard — and silent failure is precisely the bug class this check exists to stop.Title/body reach the script through the environment, never
${{ }}inlined into therun:block. PR text is attacker-controlled; inlining it is a script-injection primitive.Verification
Against the real detector, not a mock:
scripts/check-pr-metadata.test.cjs, added totest:scripts). They inject their own predicate, so no private literal lands in a tracked test file.validate-success.needsconfirmed to includepr-metadata.Scope
Deliberately limited to the private-asset marker — the demonstrated leak, and the one with a proper shared module. Extending the same job to the secret-pattern set is a reasonable follow-up; those patterns currently live inline in the husky hook rather than in a shared module.
🤖 Generated with Claude Code
https://claude.ai/code/session_01WR8YdADiaw1p8pD8CddncD