Repository navigation
ci: finish checkout-credential and action-pin hardening across all workflows (#3313) - #3388
Merged
Merged
Conversation
…rkflows (#3313) #3282 set persist-credentials: false and SHA-pinned actions for security.yml and two frontend jobs in quality.yml. The rest of the tree was left as is: 30 of 33 checkouts left the GITHUB_TOKEN in .git/config on the runner, most of them on the shared self-hosted honeypot-ci executor that deploy.yml also holds the deploy SSH material on, and 80-odd third-party `uses:` were still tag-pinned. Finishes all three proposals across all 19 workflows. persist-credentials: false on every checkout. Audited for `git push` first: none exists outside Dependabot tooling, so no checkout needs the token left on disk. 35 checkouts, 0 without it. Full-SHA pins with a `# vX.Y.Z` comment on every third-party `uses:` (52 references). SHAs resolved with `git ls-remote --tags`, not copied from a release page. Dependabot's github-actions ecosystem already covers `/` with no ignore rules, so it keeps bumping these. Two pins that were already SHAs gained a version comment, and quality.yml's two setup-node pins said `# v7` where the tag is v7.0.0. permissions: {} at workflow level on the 13 workflows that granted a write scope there, with the grant moved to the job that spends it. actions: write existed only so the ci-target job could call the reusable ci-router.yml and dispatch the ci-heartbeat canary, so it stays on ci-target and no longer reaches the other 24 jobs in quality.yml; packages: write is now on containers.yml's build job alone, and security-events: write on security.yml's analyze job alone. Verified job by job that every job's effective envelope is equal or narrower than before: no job gained a scope, and the dropped ones each had a single consumer that keeps its grant. deploy.yml, diagnostics.yml, vps-start-blackhole.yml and ci-router.yml grant no write scope at the workflow level and are left alone. Enforced rather than asserted: #3314's zizmor step listed unpinned-uses, excessive-permissions and artipacked as advisory "until #3313 lands". They are dropped from ADVISORY, so a regression in any of them is now a blocking finding. The two rules left advisory are documented inline with why: the five `uses: ./.github/workflows/ci-router.yml` calls (zizmor wants the owner/repo/path@ref form; the local-path form is GitHub's own first-party syntax and is what lets the shared router change without a second SHA to bump), and main-health-watch.yml's workflow_run (it runs the default branch's copy of the script and reads no event payload -- every value it reports comes from a main-scoped API read, and changing the trigger would blind the #3324 alarm that exists to catch runs nobody started). zizmor on the tree: unpinned-uses 51 -> 0, excessive-permissions 7 -> 0, artipacked 27 -> 0, with the gate now passing. actionlint 1.7.7 clean, all 19 workflows parse, and tests/docs (444) plus scripts/tests (193 of 195; the two compose-drift-watch_sweep privileged-fallback failures are pre-existing on the base commit) pass.
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.OpenSSF Scorecard
Scanned Files
|
Xore
enabled auto-merge (squash)
September 27, 2026 02:42
Xore
added a commit
that referenced
this pull request
Sep 27, 2026
…3325) The rebase onto #3388 pulled in main's /livez and /readyz, which the contract test caught as registered-but-undocumented. Documented both, and widened the public-route invariant in the same test to name all four public routes instead of prefix-matching two. Zizmor: this branch introduces weekly-schemathesis.yml, which #3388 never saw, so it kept floating action tags and expanded a workflow_dispatch input inline into a run block. Pin both actions and route max_examples through env like every other value in that run block.
Xore
added a commit
that referenced
this pull request
Sep 27, 2026
…3325) The rebase onto #3388 pulled in main's /livez and /readyz, which the contract test caught as registered-but-undocumented. Documented both, and widened the public-route invariant in the same test to name all four public routes instead of prefix-matching two. Zizmor: this branch introduces weekly-schemathesis.yml, which #3388 never saw, so it kept floating action tags and expanded a workflow_dispatch input inline into a run block. Pin both actions and route max_examples through env like every other value in that run block.
Xore
added a commit
that referenced
this pull request
Sep 27, 2026
… module move broke (#3325) The rebase onto #3400 brought main's fixes in beside #3325's move of the handler modules and the route table out of `src/main.rs` and into `src/lib.rs`. Three things were red as a result. None of them is a behaviour regression; all three are places where a gate reads a file, and the move changed which file. `zizmor` reported a blocking `artipacked` on this branch's own `weekly-schemathesis.yml`: an `actions/checkout` without `persist-credentials: false`. #3388 added that to every checkout in the tree and #3400 finished the job, but this workflow was added after that work and never met it. Fixed the way every other one is -- the `with:` block, and the version comment normalised from `# v7` to the `# v7.0.1` that the other 38 checkouts in the tree spell. No rule was added to the ADVISORY allowlist; the gate now reports only the allowlisted `dangerous-triggers` finding, which is what it reported on main. `scripts/tests/test_3316_image_boot_smoke.py` asserted that each path the boot smoke probes is registered by looking for `.route("<path>"` in `src/main.rs`. Two moves broke that string, not one: the table moved to `src/lib.rs`, and then #3325's own commit replaced every `.route(path, method(handler))` with `.routes(utoipa_axum::routes!(handler))`, which takes the path off the handler's `#[utoipa::path]`. So there is no path literal in the table at all any more. It now reads the paths declared by the annotations, which is where they live, and keeps the literal form so a table written in plain axum is still checked rather than passing on an empty scan. The scan is guarded on `assertGreaterEqual(len(declared), 4)`, and both patterns require a leading `/`, because `openapi.rs`'s own `BYPASSING_ROUTES` holds the literal strings `".route("` and friends and a looser pattern reads that Rust string as a route registration. `scripts/tests/test_3315_image_revision.py` pinned the Rust normalizer to the shared revision corpus by asserting on `src/main.rs` too, and `normalize_revision`, `REVISION_UNKNOWN` and the test module that reads the corpus all moved to `src/lib.rs`. Repointed at the file they moved to. It reads one named file rather than scanning the tree: an assertion against all of `src/` dumps every module into the failure message, and if the next move relocates these the assertion should fail with the file it looked in. Both test repoints are verified to still fail for the right reason -- renaming the `/livez` annotation, dropping the `/readyz` annotation, and pointing the corpus `include_str!` at a PNG each fail with the intended message, so the gates are not weakened into passing.
Xore
added a commit
that referenced
this pull request
Sep 27, 2026
… it weekly (#3325) (#3382) * feat(backend-service): publish the OpenAPI contract for /api and fuzz it weekly (#3325) Adds a machine-readable OpenAPI 3.1 contract for backend-service's /api surface -- 130 registered paths, 139 operations -- and the two gates that keep it from drifting, plus the weekly fuzz job the issue asked for. The contract is hand-maintained in a new src/openapi.rs rather than derived with utoipa. Almost every handler returns Json<Value>, and the property the issue wants pinned lives in the route table and the extractor signatures, not the response bodies, so a #[utoipa::path] on 138 handlers would learn only that GET /api/v1/events takes a Query<EventsQuery> and answers 401 without a service token. Three gates, failing in different directions on purpose: - contract_covers_every_router_route reads main.rs and fails if the router and the contract disagree about which (path, method) pairs exist. A route added without a contract row is the direction that rots, since it drops a fuzz target silently. - checked_in_contract_is_current fails if the committed openapi.json is not what the module renders. - quality.yml runs the same generator as a diff -u in both backend jobs, because a test failure says "stale" while the diff says what changed. Response bodies stay free-form on purpose: 138 transcribed shapes would be 138 chances to assert something the code does not enforce. The document pins the auth tier, the request parameters (including the enums the handlers actually validate), the declared status codes, and every response's media type. weekly-schemathesis.yml runs Mondays 04:23 UTC and on workflow_dispatch, booting the service against a single-node Elasticsearch service container -- without a cluster every ES-backed route answers 502 and the run measures nothing. Only the auth gate can fail the job; both schemathesis passes are advisory, because this API validates aggressively enough that a hard gate would be red on its first run and train everyone to ignore it. That gate is scripts/check-api-auth-tier.py, not schemathesis's ignored_auth. ignored_auth skips any operation the contract declares public, so demoting a live route in the document turns the check green while the route keeps serving anonymous callers -- reproduced on this service, where marking /api/v1/events public gave a passing run and one warning. The script reads the same document for expected answers but never trusts it about which routes are secured; that direction is the Rust test, which asserts the public set is exactly /healthz and /metrics. One row does not follow the file's pattern and is worth flagging in review: GET /api/v1/webhook-delivery, added by #3373 on main during this branch's rebase, is the only ES-backed read that declares no 502. Its handler catches the cluster error and answers 200 with `available: false` and the reason in the body, so a stated empty card beats a page that fails to render. Every neighbouring diagnostics row 502s because its extractor turns a dead cluster into a status code; copying the pattern here would have been a guess the source does not support. Running the job before shipping it found eight real omissions in the first draft of the contract, all now fixed: 415 and 422 from axum's Json<T> rejection on all 25 body routes, 400 from its Query<T> rejection, four services routes answering JSON errors declared as text/plain, a text/plain export declared as JSON, 404 from the correlations and reporter-stats lookups, and 422/501 from the report renderer's own error mapping. None of them are reachable from the router's source -- nothing in main.rs mentions 415 -- so the builders now add the extractor-level statuses instead of leaving each row to remember. The passes report those two finding classes by name so a regression in the document is visible even though it cannot fail the job. Against a booted service the document is now clean: 136 secured operations refuse an anonymous caller, and the authenticated pass finds nothing against the contract itself. Refs #3325 * fix(contract): cover /livez and /readyz, and satisfy the zizmor gate (#3325) The rebase onto #3388 pulled in main's /livez and /readyz, which the contract test caught as registered-but-undocumented. Documented both, and widened the public-route invariant in the same test to name all four public routes instead of prefix-matching two. Zizmor: this branch introduces weekly-schemathesis.yml, which #3388 never saw, so it kept floating action tags and expanded a workflow_dispatch input inline into a run block. Pin both actions and route max_examples through env like every other value in that run block. * refactor(backend-service): move the handler modules and route table into the library being transcribed by hand, and that needs a shape the crate did not have: `#[utoipa::path]` annotations live on the handlers, and the second binary that prints the document cannot see a first binary's modules. So the modules move into `src/lib.rs`, which already existed for exactly this reason, and the route table moves with them so the document is generated from the same `Router` the process serves. The move is mechanical on purpose. No module, type or function is renamed or split, and the ~320 `crate::` references the handlers make resolve to the same items as before, because `AppState` and the probe handlers landed in the same place the crate root used to be. `main.rs` keeps what is genuinely a process: the environment, the #2183 boot gate, state construction, the listener. `allow_unauth_dev_from_env` becomes `pub` because `main.rs` now reaches it through the library rather than declaring it in the crate root. One test changes, and only in what it reads: `contract_covers_every_router_route` parsed `.route(...)` out of `src/main.rs`, which no longer holds the route table. It reads `src/lib.rs` now. Every assertion is unchanged -- including the `registered.len() > 100` guard that would have caught a stale path -- so the gate is exactly as strong, pointed at the file the code moved to. 544 passed, 0 failed, 1 ignored: same as before the move. `openapi.json` is byte-identical, which the checked_in_contract_is_current gate asserts. * refactor(backend-service): generate the OpenAPI contract with utoipa (#3325) Replaces the 1820-line hand-written table in src/openapi.rs with utoipa/utoipa-axum, so the contract is derived from the annotations and the route table rather than transcribed alongside them. The 128 /api paths and 4 health probes come out byte-identical, which the the_document_is_byte_stable gate asserts against the committed openapi.json. The route table is now an index, not a second copy. Every entry moved from `.route(path, method(handler))` to `.routes(utoipa_axum::routes!(handler))`, in the original order, one handler per call: `routes!` panics if two handlers in a single call share an HTTP method, and the table has methods that repeat across adjacent entries. Paths and methods now come from `#[utoipa::path]`, so they cannot disagree with the annotations the document is built from. `#[derive(OpenApi)]` is deliberately not used. It tags every operation with a module-path tag; utoipa-axum's OpenApiRouter does not, and the committed document's tag list is per-area, not per-module. `OpenApiRouter::default()` rather than `new()`, because `new()` fills info.contact and info.license from utoipa's own Cargo.toml. The `axum_extras` feature stays off so the annotations are authoritative rather than inferred from extractor types. 141 handlers annotated, `inline(...)` throughout -- no components/schemas and no $refs anywhere in the document. The two `Option<Json<T>>` bodies and the `Option<T>` parameters cannot express `pattern`/`maxLength`/`minimum`/ `maximum` or inline enums at container level through utoipa's derive, so 16 named parameter-shape types live in a new src/contract.rs behind a `contract_schema!` macro; STORE_NAMES moved there with them and is re-exported from openapi.rs for the handside allowlist test. A post-generation transform in render() reconciles the document with the committed one. Six steps, each documented at the point of implementation and counted in the module doc: merge the middleware 401, derive `operationId` and `tags` as pure functions of the path, read `Option` as an absence rather than a null (this is also what turns `x-optional-body` into `required: false`), strip the descriptions utoipa derives from doc comments, pin the document-level literals, and emit `parameters: []` on operations that take none. contract_covers_every_router_route is rewritten, not just repointed. It parsed `.route(...)` calls out of the source, and that form no longer exists. The two doors that remain open are now the two it closes: the BYPASSING_ROUTES list (`.route(`, `.route_service(`, `.nest_service(`) must not appear in src/lib.rs, because OpenApiRouter inherits all three as pass-throughs that serve a route with no OpenAPI operation; and every `#[utoipa::path]` in src/**/*.rs must be routed. Both directions were exercised rather than assumed -- injecting a bare `.route("/api/v1/sneaky")` and removing an existing `routes!` entry each fail with the intended message. One intentional line of difference in openapi.json. The serviceToken security-scheme description reads "lib.rs's require_service_token" where it used to read "main.rs's", because Stage 1 moved that middleware out of the crate root. Publishing the old string would state something untrue about where the check lives. That is the only byte that changed, in 296852. 546 passed, 0 failed, 1 ignored: the 544 baseline plus the endless-stream invariant and the new byte-stability test. `cargo build --release` succeeds (the Dockerfile copies src/ wholesale), and clippy --lib --all-targets is clean. * fix(ci): close the artipacked advisory, and repoint the two gates the module move broke (#3325) The rebase onto #3400 brought main's fixes in beside #3325's move of the handler modules and the route table out of `src/main.rs` and into `src/lib.rs`. Three things were red as a result. None of them is a behaviour regression; all three are places where a gate reads a file, and the move changed which file. `zizmor` reported a blocking `artipacked` on this branch's own `weekly-schemathesis.yml`: an `actions/checkout` without `persist-credentials: false`. #3388 added that to every checkout in the tree and #3400 finished the job, but this workflow was added after that work and never met it. Fixed the way every other one is -- the `with:` block, and the version comment normalised from `# v7` to the `# v7.0.1` that the other 38 checkouts in the tree spell. No rule was added to the ADVISORY allowlist; the gate now reports only the allowlisted `dangerous-triggers` finding, which is what it reported on main. `scripts/tests/test_3316_image_boot_smoke.py` asserted that each path the boot smoke probes is registered by looking for `.route("<path>"` in `src/main.rs`. Two moves broke that string, not one: the table moved to `src/lib.rs`, and then #3325's own commit replaced every `.route(path, method(handler))` with `.routes(utoipa_axum::routes!(handler))`, which takes the path off the handler's `#[utoipa::path]`. So there is no path literal in the table at all any more. It now reads the paths declared by the annotations, which is where they live, and keeps the literal form so a table written in plain axum is still checked rather than passing on an empty scan. The scan is guarded on `assertGreaterEqual(len(declared), 4)`, and both patterns require a leading `/`, because `openapi.rs`'s own `BYPASSING_ROUTES` holds the literal strings `".route("` and friends and a looser pattern reads that Rust string as a route registration. `scripts/tests/test_3315_image_revision.py` pinned the Rust normalizer to the shared revision corpus by asserting on `src/main.rs` too, and `normalize_revision`, `REVISION_UNKNOWN` and the test module that reads the corpus all moved to `src/lib.rs`. Repointed at the file they moved to. It reads one named file rather than scanning the tree: an assertion against all of `src/` dumps every module into the failure message, and if the next move relocates these the assertion should fail with the file it looked in. Both test repoints are verified to still fail for the right reason -- renaming the `/livez` annotation, dropping the `/readyz` annotation, and pointing the corpus `include_str!` at a PNG each fail with the intended message, so the gates are not weakened into passing. * docs(CI-CD): point the #3325 section at where the contract actually lives (#3325) The rebase's own conflict resolution made these three claims false, and the file is one this branch already owns, so they are corrected here rather than left for the next reader to trip over. "the source of truth is the operation table in src/openapi.rs" was true when the table was hand-written there and stopped being true when it was generated from the `#[utoipa::path]` annotations and the route table in src/lib.rs. "contract_covers_every_router_route reads src/main.rs" named a file that no longer holds a route table. "Fix those by editing the row in operations()" named a function that no longer exists -- the edit point is the annotation on the handler, or the shape it names in src/contract.rs. The counts were stale in the same way: the section claimed 129 registered /api paths and 138 operations, and the committed document has 132 paths and 141 operations -- 128 /api paths and 137 /api operations behind the token, plus the four public probes. Now stated as counted, with the public set named. No behaviour and no generated contract changes: `openapi.json` is byte-identical before and after, which `the_document_is_byte_stable` and `checked_in_contract_is_current` both assert.
3 tasks done
Xore
pushed a commit
that referenced
this pull request
Sep 27, 2026
…orm (#3313) #3388 landed the #3313 work and the tree already satisfies it: 39 of 39 checkouts carry persist-credentials: false, no `uses:` is tag-pinned, and every workflow-level `permissions:` is {} or read-only. The issue table it was written against is pre-#3388 and no longer describes main. What #3388 did not finish is the version comment on a pin. It claims "quality.yml's two setup-node pins said `# v7` where the tag is v7.0.0" -- two `# v7` comments survived that commit, and a third pin was labelled with a major it is not in: quality.yml:453,648 setup-node@820762786 # v7 -> # v7.0.0 weekly-schemathesis.yml:276 upload-artifact@ea165 # v6 -> # v4.6.2 All three verified with `git ls-remote`, not the release page: 820762786 is the commit behind both `v7` and `v7.0.0`, and ea165f8d is the commit behind both `v4` and `v4.6.2` -- `v6` is b7c566a, a different commit. The upload-artifact SHA is what all seven other upload-artifact pins in the tree use, so the comment is corrected to match the code; bumping the major to match the comment would be a build-logic change, not a pin fix. Enforced, because zizmor's unpinned-uses audit reads the SHA and ignores the comment -- that gap is exactly how a well-formed pin shipped claiming a version it does not run. The new check is in the existing #3314 lint gate, offline and deterministic, and asserts a full `# vX.Y.Z` on every SHA pin. It was proven against the pre-fix tree: it fails on all three lines above and on a pin with no comment at all, and passes on the fixed tree. zizmor --min-severity medium: 1 advisory (dangerous-triggers), 0 blocking, unchanged from the pre-change baseline. actionlint 1.7.7: clean.
Xore
pushed a commit
that referenced
this pull request
Sep 27, 2026
…orm (#3313) #3388 landed the #3313 work and the tree already satisfies it: 39 of 39 checkouts carry persist-credentials: false, no `uses:` is tag-pinned, and every workflow-level `permissions:` is {} or read-only. The issue table it was written against is pre-#3388 and no longer describes main. What #3388 did not finish is the version comment on a pin. It claims "quality.yml's two setup-node pins said `# v7` where the tag is v7.0.0" -- two `# v7` comments survived that commit, and a third pin was labelled with a major it is not in: quality.yml:453,648 setup-node@820762786 # v7 -> # v7.0.0 weekly-schemathesis.yml:276 upload-artifact@ea165 # v6 -> # v4.6.2 All three verified with `git ls-remote`, not the release page: 820762786 is the commit behind both `v7` and `v7.0.0`, and ea165f8d is the commit behind both `v4` and `v4.6.2` -- `v6` is b7c566a, a different commit. The upload-artifact SHA is what all seven other upload-artifact pins in the tree use, so the comment is corrected to match the code; bumping the major to match the comment would be a build-logic change, not a pin fix. Enforced, because zizmor's unpinned-uses audit reads the SHA and ignores the comment -- that gap is exactly how a well-formed pin shipped claiming a version it does not run. The new check is in the existing #3314 lint gate, offline and deterministic, and asserts a full `# vX.Y.Z` on every SHA pin. It was proven against the pre-fix tree: it fails on all three lines above and on a pin with no comment at all, and passes on the fixed tree. zizmor --min-severity medium: 1 advisory (dangerous-triggers), 0 blocking, unchanged from the pre-change baseline. actionlint 1.7.7: clean.
Xore
pushed a commit
that referenced
this pull request
Sep 27, 2026
…orm (#3313) #3388 landed the #3313 work and the tree already satisfies it: 39 of 39 checkouts carry persist-credentials: false, no `uses:` is tag-pinned, and every workflow-level `permissions:` is {} or read-only. The issue table it was written against is pre-#3388 and no longer describes main. What #3388 did not finish is the version comment on a pin. It claims "quality.yml's two setup-node pins said `# v7` where the tag is v7.0.0" -- two `# v7` comments survived that commit, and a third pin was labelled with a major it is not in: quality.yml:453,648 setup-node@820762786 # v7 -> # v7.0.0 weekly-schemathesis.yml:276 upload-artifact@ea165 # v6 -> # v4.6.2 All three verified with `git ls-remote`, not the release page: 820762786 is the commit behind both `v7` and `v7.0.0`, and ea165f8d is the commit behind both `v4` and `v4.6.2` -- `v6` is b7c566a, a different commit. The upload-artifact SHA is what all seven other upload-artifact pins in the tree use, so the comment is corrected to match the code; bumping the major to match the comment would be a build-logic change, not a pin fix. Enforced, because zizmor's unpinned-uses audit reads the SHA and ignores the comment -- that gap is exactly how a well-formed pin shipped claiming a version it does not run. The new check is in the existing #3314 lint gate, offline and deterministic, and asserts a full `# vX.Y.Z` on every SHA pin. It was proven against the pre-fix tree: it fails on all three lines above and on a pin with no comment at all, and passes on the fixed tree. zizmor --min-severity medium: 1 advisory (dangerous-triggers), 0 blocking, unchanged from the pre-change baseline. actionlint 1.7.7: clean.
Xore
added a commit
that referenced
this pull request
Sep 27, 2026
…orm (#3313) (#3407) * ci: correct three wrong action-pin version comments and enforce the form (#3313) #3388 landed the #3313 work and the tree already satisfies it: 39 of 39 checkouts carry persist-credentials: false, no `uses:` is tag-pinned, and every workflow-level `permissions:` is {} or read-only. The issue table it was written against is pre-#3388 and no longer describes main. What #3388 did not finish is the version comment on a pin. It claims "quality.yml's two setup-node pins said `# v7` where the tag is v7.0.0" -- two `# v7` comments survived that commit, and a third pin was labelled with a major it is not in: quality.yml:453,648 setup-node@820762786 # v7 -> # v7.0.0 weekly-schemathesis.yml:276 upload-artifact@ea165 # v6 -> # v4.6.2 All three verified with `git ls-remote`, not the release page: 820762786 is the commit behind both `v7` and `v7.0.0`, and ea165f8d is the commit behind both `v4` and `v4.6.2` -- `v6` is b7c566a, a different commit. The upload-artifact SHA is what all seven other upload-artifact pins in the tree use, so the comment is corrected to match the code; bumping the major to match the comment would be a build-logic change, not a pin fix. Enforced, because zizmor's unpinned-uses audit reads the SHA and ignores the comment -- that gap is exactly how a well-formed pin shipped claiming a version it does not run. The new check is in the existing #3314 lint gate, offline and deterministic, and asserts a full `# vX.Y.Z` on every SHA pin. It was proven against the pre-fix tree: it fails on all three lines above and on a pin with no comment at all, and passes on the fixed tree. zizmor --min-severity medium: 1 advisory (dangerous-triggers), 0 blocking, unchanged from the pre-change baseline. actionlint 1.7.7: clean. * fix(#3316): only resolve a boot-smoke image by digest when a push happened build-push-action emits steps.build.outputs.digest even with push: false, where it describes the image built locally and names nothing in the registry. The boot-smoke step tested IMAGE_DIGEST for non-empty to mean "we pushed this", so every pull_request run took the docker-pull path against a tag that was never pushed: three "manifest unknown" retries, then a hard exit. That is what turned every PR run of this workflow red, including the docs PRs, which changed no container at all. The comment above the input already stated the intended contract ("Empty exactly when nothing was pushed"); only the expression disagreed. Gate it on the event so a pull_request reaches the local-label branch that exists for exactly this case, and a push with no digest now fails with that branch's own message instead of a phantom registry lookup. The tag that was being pulled, sha-51ced49, is metadata-action's type=sha applied to the synthetic refs/pull/3398/merge commit -- which is why it names no SHA in the repository and why ghcr has no such tag. * fix(#3316): look up the boot-smoke image with --all, or the label filter never matches The image this step is looking for is the one it just built. On a pull_request the build uses the docker exporter, which drops the tag, so the image arrives dangling. `docker image ls --filter label=...` applies the label filter to tagged images only, so the query returned nothing and the step failed on a build that had loaded correctly -- the error even said "no local image carries label ...", which is false. Reproduced against a real buildx --load: the label is on the image (`docker image inspect` shows it) and `--filter dangling=true` finds it, but `--filter label=...` finds nothing until --all is passed. With --all the same query returns the image id. The comment three lines above the query already said "The docker exporter drops the tag" -- the code just did not account for what that implies for this particular filter. * fix(#3316): emit the build-row label from a shell, or the newline stays literal The boot-smoke step finds its image with `docker image ls --all --filter "label=apiary.ci.build-row=$BUILD_ROW"`, but apiary.ci.build-row was never applied as its own key. The labels input appended it with format('\napiary.ci.build-row=...'), and GitHub's format() does not interpret \n -- it emits a backslash and an 'n'. The text landed at the end of the previous label's value, which the run log shows as: "label:org.opencontainers.image.version": "sha-047a7a3\\napiary.ci.build-row=dashboard-next-36338870116-1" so the filter had no key to match and every pull_request run failed the "Resolve the image ID to boot-smoke" step. Only the push path looks for the label, which is why it went unnoticed. Build the list in a shell step instead, where printf can emit a real line break, and feed that to build-push-action. The list goes out through the `labels<<EOF` heredoc form rather than a bare printf of k=v lines: a bare key=value line becomes its own step output, so the list would arrive as outputs named org.opencontainers.* and the `labels` key the input reads would not exist at all. Gated on matrix.boot_smoke, so the other sixteen rows skip the step, get an empty output, and fall back to the metadata-action list byte for byte. They must not gain a CI run id in their manifest. Proven locally: an untagged buildx --load image carrying the label is found by the lookup above, and `image inspect` shows apiary.ci.build-row as its own key with org.opencontainers.image.version left intact. The pre-fix label shape was built too as a control, and that one is not found -- so the check discriminates rather than passing trivially. * ci: validate a PR train locally once, instead of per-PR on a saturated fleet The self-hosted fleet is seven CI runners and saturates on two concurrent runs, leaving the second queued with no signal. Waiting for each PR's CI after each sibling merge is O(N^2) runs on top of that. scripts/merge-train.sh merges every train member into a throwaway worktree cut from the base tip, runs the local gate suite ONCE on the result, and prints the evidence that authorizes the merges. It reads origin only: never pushes, never merges, never touches another worktree, never stashes. A PR that conflicts is ejected and the train continues. A named gate that is missing at the base is a failure, not a skip, so a renamed gate cannot quietly turn the train green. Adapted from diegosouzapw/OmniRoute scripts/release/merge-train.sh. Their --fast reduced-coverage mode is deliberately not ported: the doc gates are the point of what this repo is merging, so parity means the full suite. * fix(#3316): assert the label-append step, not the old inline gating The boot-smoke parity test asserted `matrix.boot_smoke` appears in the build step's `labels:` expression. The newline fix moved that gating into the `if:` of the label-append step, so the assertion no longer described where the gating lives and the test failed on a correct workflow. Assert the invariant instead: the labels input still falls back to the base labels, still prefers the appended build-row list, and the append step itself is gated on a boot-smoke row. Verified the test fails when the shell-step form is reverted to the inline expression, so it still bites. --------- Co-authored-by: Xore <xore@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
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.
What
#3282 set
persist-credentials: falseand SHA-pinned actions forsecurity.ymland two frontend jobs inquality.yml. The rest of the tree was left as is. This finishes all three proposals from #3313 across all 19 workflows.uses:on a mutable tagGITHUB_TOKENin.git/config1.
persist-credentials: falseon every checkoutAudited for
git pushfirst, as the issue asks: there is none anywhere in the tree outside Dependabot tooling, so no checkout needs the token left on disk. All 35 checkouts across 14 workflows now carry it, includingdeploy.yml's two — the ones that matter most, since that workflow also installs the VPS deploy SSH key on the same runner.2. Full-SHA pins on every third-party
uses:52 references pinned to a full SHA with a
# vX.Y.Zcomment, coveringactions/{checkout,setup-python,setup-node,setup-go,cache,upload-artifact,configure-pages,upload-pages-artifact,deploy-pages,dependency-review-action}anddocker/{setup-buildx-action,login-action,metadata-action,build-push-action}.SHAs were resolved with
git ls-remote --tags, not read off a release page. Dependabot'sgithub-actionsecosystem already covers/weekly with noignorerules, so it keeps bumping these — the pins do not become a maintenance burden.Two pins that were already SHAs gained the version comment they were missing, and
quality.yml's twosetup-nodepins said# v7where the tag isv7.0.0.3. Job-scoped
permissions: {}Thirteen workflows granted a write scope at workflow level. Each now defaults to
permissions: {}with the grant moved onto the job that spends it:actions: writeexisted only so theci-targetjob could call the reusableci-router.ymland dispatch theci-heartbeatcanary. It stays onci-targetand no longer reaches the other 24 jobs inquality.yml, nor the scan/build jobs inimage-security-scan.yml/pages.yml. The existing comment in each of those files already explained why the grant cannot simply be dropped (a called reusable workflow can never exceed the caller's envelope, so under-granting it fails the run asInvalid workflow file) — that reasoning is now scoped to the one job that needs it instead of the whole workflow.packages: writeis oncontainers.yml'sbuildjob alone; thecontainers-gatejob needs no token at all, since it only readsneeds.*.result.security-events: writeis onsecurity.yml'sanalyzejob alone — it is the SARIF upload, and nothing else in that workflow talks to code scanning.contents: write/pull-requests: writeindependabot-auto-merge.ymlandissues: writein the five*-watch.ymlsweeps moved to their single jobs.cache-cleanup.yml'sactions: write(the whole reason that workflow exists) anddependency-review.yml'spull-requests: writelikewise.Checked job by job with a script that computes each job's effective envelope (job block, else workflow block, else GitHub's implicit default) before and after: 27 jobs got strictly narrower, 0 got wider, 0 lost a scope they use. Every dropped scope had exactly one consumer, and that consumer keeps its grant.
deploy.yml,diagnostics.yml,vps-start-blackhole.ymlandci-router.ymlare deliberately untouched — they grant no write scope at workflow level, so the issue's scoping condition does not apply to them, and their current block already means "everything else: none".4. Enforced, not asserted
#3314's zizmor step already listed
unpinned-uses,excessive-permissionsandartipackedas advisory "until #3313 lands — then drop them from ADVISORY". They are dropped, so a regression in any of them is now a blocking finding.The two rules left advisory are documented inline with the specific reason they are not fixable here, rather than left as a bare list:
self-repository— the fiveuses: ./.github/workflows/ci-router.ymlcalls. zizmor wants the explicitowner/repo/path@refform; the local-path form is GitHub's own first-party-reusable-workflow syntax and is what lets the shared router change without a second SHA to bump. None of the five is triggered bypull_request_target, so the called workflow is always the default branch's own file.dangerous-triggers—main-health-watch.yml'sworkflow_run. It runs the default branch's copy of the script and reads no event payload: every value it reports comes from a main-scopedgh api/gh run listread, and the only untrusted-looking text it echoes ismain's own commit subjects. Changing the trigger would blind the ci: scheduled main-health sweep that opens and closes a single 'main is red' issue #3324 alarm that exists precisely to catch runs nobody started.Verification
The two
scripts/testsfailures aretest_compose_drift_watch_sweep.py's privileged-fallback cases; they fail identically on the base commit (git stash+ rerun) and are unrelated to this change.Refs #3313. Follows #3282 (partial) and #3314 (the lint gate that makes this non-regressable).