Repository navigation
Conversation
Sign each pushed component digest with a pinned cosign key and verify against the committed public key before latest or channel tags move. Publication fails closed when signing secrets are missing.
Use imagetools inspect's Image JSON for publication timestamps, label legacy dashboard images for signing without updater metadata, and exercise the real timestamp reader in publication tests.
Install the production cosign public key, keep offline keys under test fixtures only, re-check the SHA tag digest after verification, and extend publish-image-test signing and redaction coverage.
Generate ephemeral cosign key pairs in a temp directory during publish-image-test.sh and remove committed fixture key material.
Refuse unsigned, invalid, expired, and untrusted-key release digests in discover_ref and files before any layer download. Boot repair falls back to the baked binary only when schemas fit and records update_boot_mismatch_v1. Bump launcher revision to 18 and extend tests for signature policy.
Validate signing secrets before any build, unset them from the process environment during npm/docker work, and scope private key material to the cosign sign subshell only. Verify and sign immutable digest references, defer version and unique nightly tags until after signing, and pass --yes for unattended cosign. Install cosign on quick CI lanes, bootstrap cosign in offline tests, and extend publication tests for trace, label, and digest guarantees.
|
@pullfrog review |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Changes are required before merging: production image builds lack the newly embedded key, published Cosign signatures cannot pass verification, and several boot-repair paths leave compatible applications stopped. Six findings are detailed inline.
Reviewed changes against the stacked signing branch, including the publisher contract from #379 and the boot-recovery requirements in #188.
- Signature admission: Verification before image discovery and file extraction, pinned-key handling, expiry checks, and permanent/transient error classification.
- Boot recovery: Installed-release restoration, schema-checked baked fallback, and durable mismatch recording and clearing across bot/API writer boundaries.
- Staging and compatibility: Temporary-file cleanup, launcher capability/minimum revision 18, publication assertions, and operator documentation.
- Regression evidence: New signed registry/controller fixtures and the existing updater suite, checked against the actual publisher format and production Docker inputs.
Validation: all 102 updater tests and bash scripts/publish-image-test.sh passed. Independent probes against the built production libraries reproduced the SPKI rejection, malformed-signature fallback failure, last-mismatch deletion failure, and read-only API rejection. cargo fmt --check failed; the current CI Rust job also reports clippy::redundant_locals at src/db/updates.rs:932. Full preflight, container builds, and live registry/provider tests were not run. The worktree is clean.
gpt-6.1-sol ｜ 𝕏
Stage images before promotion, verify and sign the digest from build metadata, persist signing material in scratch files for cosign only, and harden publish-image-test mocks for imagetools promotion retries.
Copy deploy signing keys into image builds, parse OCI .sig manifests, decode SPKI public keys for ring, forward boot-mismatch writes through ControlOp, bound clearing the mismatch meta row, and classify malformed signature parse failures for boot repair.
|
@pullfrog review |
There was a problem hiding this comment.
Caution
Published Cosign images still cannot pass verification, and boot recovery can discard the installed selection or leave a compatible API stopped. Changes are required before merging.
Reviewed changes across the complete diff against cursor/sign-release-images-3fd7, including the publisher contract and the boot-recovery requirements in #188.
- Signature admission: Manifest verification before discovery/extraction, Cosign artifact parsing, production SPKI decoding, expiry, trust rotation, and transport-error classification.
- Boot recovery: Schema-checked baked fallback, readiness reporting, durable installed identity, and mismatch recording/clearing across the read-only API writer boundary.
- Build and compatibility: Docker signing-key inputs, staging cleanup, launcher capability/minimum revision 18, and affected publication/startup tests.
- Verification: Registry tests, last-mismatch deletion, formatting, and the original boot-fallback test pass. Independent local probes reproduce the Cosign format rejection, installed-selection loss after fallback readiness, writer-unavailable API startup failure, premature mismatch clearing, and truncated-body misclassification. Production SPKI decoding and malformed-signature boot classification now pass their probes.
cargo test -p jmodguard-updater --all-targets fails the existing publisher integration test at main.rs:3234; bash scripts/publish-image-test.sh fails with missing launcher requirement launcher-revision=17. Current CI also reports the affected API launcher-gate failures. Full preflight, container builds, and live GHCR/Discord/Bunny tests were not run; the recovery probes used real local SQLite and supervisor processes with fixture executables, and the Cosign-shaped artifact was a local fixture checked against upstream source. Temporary repository probes were removed and the worktree is clean.
gpt-6.1-sol ｜ 𝕏
Stage signing credentials outside the publish step, keep them only in shell memory through WebUI and image builds, and pass a staging ref in the launcher publication contract test.
Update launcher revision integration tests for revision 18, satisfy abs_home_path dylint, defer signing key files until cosign, and pass a staging ref in the publication contract test.
Parse Cosign OCI signature manifests like the publisher emits, preserve installed selection during baked boot repair, defer mismatch receipts, and align launcher revision tests with REQUIRED_LAUNCHER_REVISION 18. Load rotated signing keys via JMODGUARD_REPO_ROOT in production images.
|
@pullfrog review |
… rollback Use cosign -a for not-after annotations, match runtime ENTRYPOINT checks to Dockerfiles, reject pre-build signing secret files on disk, and let promotion failures roll back before exiting.
There was a problem hiding this comment.
Caution
Changes are required before merging: the runtime image layout breaks updater extraction, baked fallback clears its mismatch while still running the wrong release, and existing revision-18 launchers cannot receive the new verifier. Four findings are detailed inline.
Reviewed changes across the complete diff against cursor/sign-release-images-3fd7, including the actual publisher contract and the boot-recovery requirements in #188.
- Signature admission: Cosign payload/annotation parsing, pinned-key verification, expiry, rotation, digest binding, and registry-error classification before discovery and extraction.
- Boot recovery: Schema-checked baked fallback, readiness receipts, installed identity, and durable mismatch recording/clearing across the bot/API writer boundary.
- Packaging and rollout: Docker signing-key placement, final-layer extraction, launcher capability/minimum requirements, staging cleanup, and affected publication/startup contracts.
- Verification:
cargo fmt --check, publisher contract tests, both registry tests, and the last-mismatch deletion regression pass. The updater suite failsboot_repair_starts_baked_when_signature_missing_and_schema_fitsatcontroller_tests.rs:8256withno entry found for key; current CI reproduces that failure. Two independent local signed-registry probes also reproduce final-layer extraction failure and rejection of a valid later signature. These used local HTTP fixtures; container builds, live GHCR/Discord/Bunny tests, and full preflight were not run. Temporary probes were removed and the worktree is clean.
gpt-6.1-sol ｜ 𝕏
Split CI into build/verify and sign/promote steps so signing secrets never enter the process that runs selected-source builds. Propagate build/sign failures through publish_component, exercise the real signer in tests, and pin the production cosign public key fingerprint.
Stop layer traversal once requested files resolve, keep installed release identity on baked boot fallback, align API fixture guild schema, and drive the bad-signature controller test through staging with a real install error.
|
@pullfrog review |
There was a problem hiding this comment.
Important
Three correctness issues remain: slow baked recovery is stopped on the selected-release readiness deadline, valid layered overwrites are rejected, and directory/opaque whiteouts can resurrect deleted executables. Details and reproduction evidence are inline.
Reviewed changes across the complete diff against cursor/sign-release-images-3fd7, refreshed through f31cd99, with #188 and the publisher contract checked as surrounding context.
- Signature admission: Cosign manifest-layer and raw-envelope handling, P-256 verification, repository/digest binding, expiry, trusted-key rotation, and transient-error classification before discovery and extraction.
- Boot recovery: Schema-checked baked fallback, installed identity, readiness reporting, and deferred mismatch record/clear operations across the API writer boundary.
- Packaging and rollout: Docker signing assets, multi-layer extraction, staging cleanup, launcher capability/minimum revision 19, documentation, and affected publication/startup fixtures.
- Verification: All 113 updater tests, both mismatch database regressions, updater Clippy with
-D warnings, formatting, whitespace checks, and publication contracts pass. An additional temporary supervisor regression reproduces baked-fallback termination after the readiness deadline; independent signed HTTP fixtures using productionRegistry::filesreproduce both whiteout acceptance and cross-layer duplicate rejection.
The probes use real local SQLite, fixture child executables, and local HTTP, not live Discord, GHCR, Bunny, or remote libSQL. Full preflight and container builds were not run. The reviewed head's CI passed check reports failure with several check groups cancelled; that result is not attributed to a code defect here. Temporary repository probes were removed and the worktree is clean.
gpt-6.1-sol ｜ 𝕏
Drop GITHUB_RUN_ATTEMPT from publish staging refs so a retried publish-sign job can sign the image the build job already pushed. Legacy WebUI sources write sign:false handoff metadata, and contract tests cover sign retry and historical WebUI skips.
|
@pullfrog review |
Keep baked fallback start without passing release into start() so readiness timeouts stay correct. Reject duplicate paths within one layer only; add regression test. Mark layer-resolution test helper cfg(test) for clippy.
There was a problem hiding this comment.
Important
Baked boot repair again discards the installed selection, and three extraction defects remain in whiteout and duplicate handling. Four findings are detailed inline; the latest formatting and Clippy fixes are verified.
Reviewed changes across the complete PR against cursor/sign-release-images-3fd7, including the commits that arrived during this review and the recovery requirements in #188.
- Signature admission: Cosign artifact/envelope parsing, P-256 trust keys, repository/digest binding, expiry, renewal, rotation, and permanent/transient error classification before discovery and extraction.
- Boot recovery: Installed-release restoration, schema-checked baked fallback, readiness semantics, and durable mismatch recording/clearing across the API writer boundary.
- Extraction and rollout: Multi-layer resolution, whiteouts, tar ambiguity checks, staging cleanup, Docker signing assets, launcher revision 19, and publication/startup fixtures.
- Verification: The existing boot-fallback regression fails in isolation with
installed=None, also reproduced by the updater suite. Both registry tests, both boot-mismatch database regressions, formatting, and updater Clippy pass. Independent signed local-registry probes reproduce all three extraction findings. Publisher contracts pass on rerun after one promotion-trace assertion failure.
The probes use generated trusted test keys, local HTTP, real local SQLite, and fixture executables, not live GHCR, Discord, Bunny, or remote libSQL. Full preflight and container builds were not run. Temporary repository probes were removed and the worktree is clean.
gpt-6.1-sol ｜ 𝕏
There was a problem hiding this comment.
Important
Changes are required before merging: baked boot recovery discards the installed selection, and three OCI extraction errors remain. Four findings are detailed inline.
Reviewed changes across the complete PR diff against cursor/sign-release-images-3fd7, including the commit that landed during this review and the boot-recovery requirements in #188.
- Signature admission: Cosign envelope/artifact parsing, P-256 verification, repository and digest binding, expiry, trusted-key rotation, and transport-error classification before discovery and extraction.
- Boot recovery: Installed-release restoration, schema-checked baked fallback, readiness reporting, and durable mismatch recording/clearing across the API writer boundary.
- Packaging and rollout: Docker signing assets, layered file extraction, staging cleanup, launcher capability/minimum revision 19, publication contracts, and startup fixtures.
- Verification: On
75bfdc3, 18 library tests and 96/97 controller tests pass; the installed-selection regression fails both in the suite and in isolation. Both registry tests, Clippy with-D warnings, formatting, and whitespace checks pass. The transactional mismatch regressions and publisher contracts also passed before the final two-file commit. Independent signed local HTTP probes reproduce ignored standard opaque markers, incorrect whiteout precedence, and accepted same-layer duplicates while confirming cross-layer overrides work.
Full preflight, container builds, and live GHCR/Discord/Bunny or remote-libSQL tests were not run. Recovery tests use real local SQLite and fixture executables; the extraction probes exercise production Registry::files, not a replacement extractor. Temporary repository probes were removed and the worktree is clean.
gpt-6.1-sol ｜ 𝕏
Pass the installed release when starting the baked binary so durable selection is not cleared, but skip replacement readiness timeout while the child path is the baked executable.
|
@pullfrog review |
There was a problem hiding this comment.
Important
The installed-selection loss during baked recovery is fixed on this head, but three previously reported OCI extraction defects remain. Changes are needed before merging; the findings below were reproduced against the current production registry library.
Reviewed changes across the complete PR diff against main, including the signing and boot-recovery requirements in #188.
- Signature admission: Cosign envelope/artifact parsing, P-256 verification, repository/digest binding, expiry, key rotation, and transport-error classification before discovery and extraction.
- Boot recovery: Schema-checked baked fallback, retained installed selection, readiness handling, and deferred transactional mismatch recording/clearing across the API writer boundary.
- Packaging and rollout: Docker signing-key inputs, layered file resolution, staging cleanup, launcher capability/minimum revision 19, publication contracts, deployment documentation, and startup fixtures.
- Verification: All 117 updater tests, both boot-mismatch database regressions, updater Clippy with
-D warnings, formatting, whitespace checks, and publisher contracts pass. Current CI is green. Six independent signed local-registry fixtures confirm split-layer reads and cross-layer overrides work, but reproduce ignored standard opaque markers, incorrect whiteout precedence, and accepted same-layer duplicates.
The probes exercise production Registry::files against local HTTP fixtures; recovery/database tests use real local SQLite and fixture executables. Full preflight, container builds, and live GHCR/Discord/Bunny or remote-libSQL tests were not run. No repository files were changed, and the worktree is clean.
gpt-6.1-sol ｜ 𝕏
Start the baked binary without a Running.release so replacement readiness deadlines do not apply, but record activation through running() with the pinned installed release instead of baked_running().
|
@pullfrog review |
There was a problem hiding this comment.
Important
Three previously reported OCI extraction bugs remain on this head: opaque deletions are ignored, whiteouts affect the wrong layers, and same-layer duplicates bypass validation. These can stage files absent from the image or reject valid releases despite green CI.
Reviewed changes across the complete 21-file diff against main, including the signing publisher contract and the boot-repair requirements in #188.
- Signature admission: Cosign artifact/envelope parsing, pinned-key verification, digest/repository binding, expiry, renewal, key rotation, and transient-error classification before discovery and extraction.
- Boot recovery: Schema-gated baked fallback, preserved installed selection, readiness reporting, and transactional/deferred mismatch records across the bot/API writer boundary.
- Extraction and rollout: Multi-layer file resolution, whiteouts, archive validation, temporary staging cleanup, Docker signing-key inputs, launcher revision 19, deployment docs, and publication/startup fixtures.
- Verification: All 117 updater tests, both boot-mismatch database regressions, updater Clippy with
-D warnings, formatting, whitespace checks, and publisher contract tests pass; current GitHub CI is green.
Independent probes against production Registry::files reproduce all three findings while split-layer and ordinary cross-layer-override controls succeed; the reproduction fixture uses locally signed images and a local HTTP registry, not live GHCR. Full local preflight, container builds, and live Discord/Bunny/provider acceptance were not run. No repository files were changed, and the worktree is clean.
gpt-6.1-sol ｜ 𝕏
Recognize the standard opaque-whiteout basename, keep same-layer and newer files, and reject a repeated path inside one layer before the resolved-path shortcut.
|
@pullfrog review |
1 similar comment
|
@pullfrog review |
There was a problem hiding this comment.
Important
A failed update during baked boot recovery can discard the recorded installed release. One reproduced regression is detailed inline.
Reviewed changes across the complete current diff against main, including the publisher contract and the boot-recovery requirements in #188.
- Signature admission: Cosign artifact parsing, P-256 verification, repository/digest binding, expiry, trusted-key rotation, and registry-error classification before discovery and extraction.
- OCI extraction: Newest-first multi-layer resolution, bounded archive processing, whiteouts restricted to lower layers, same-layer duplicate rejection, and unsafe-path checks.
- Boot recovery: Schema-checked baked fallback, installed identity, readiness reporting, deferred mismatch receipts, and transactional mismatch-map updates across the bot/API writer boundary.
- Packaging and compatibility: Docker signing-key inputs, temporary staging cleanup, launcher capability/minimum revision 19, publication/startup fixtures, and operator documentation.
Validation: the unchanged updater library, launcher, and registry suites pass, as do both boot-mismatch database regressions, updater Clippy with -D warnings, formatting, whitespace checks, publication contracts, and service boundaries. Current GitHub CI is green. A separate temporary supervisor regression reproduced the installed-selection loss at the intended assertion using real local SQLite, local signed HTTP fixtures, and fixture executables; the probe was removed and the worktree is clean. Full preflight, container builds, remote libSQL, and live GHCR/Discord/Bunny tests were not run.
gpt-6.1-sol ｜ 𝕏
There was a problem hiding this comment.
Important
A failed update attempted during baked boot repair can erase the retained installed release. One finding is detailed inline.
Reviewed changes across the complete current diff against main, including the publisher contract and the boot-recovery requirements in #188.
- Signature admission: Cosign artifact parsing, P-256 verification, repository/digest binding, expiry, trust rotation, and transient/permanent failure classification before discovery and extraction.
- Layer extraction: Newest-first resolution across image layers, lower-layer whiteouts, same-layer replacements, duplicate requested paths, and extraction bounds.
- Boot recovery: Schema-checked baked fallback, retained installed identity, readiness and rollback receipts, and durable mismatch record/clear retries across the API writer boundary.
- Packaging and rollout: Docker signing assets, staging cleanup, launcher capability/minimum revision 19, publication assertions, startup tests, and operator documentation.
- Verification: All 121 existing updater tests, both boot-mismatch database regressions, updater Clippy with
-D warnings, formatting, whitespace checks, publisher contracts, and service-boundary checks pass. An additional temporary supervisor regression reproduces installed-selection loss after a candidate readiness timeout during baked recovery; it uses real local SQLite and OS children with fixture executables and a local signed HTTP registry, not live providers. The probes were removed and the worktree is clean. Current CI is green; full local preflight, container builds, and live GHCR/Discord/Bunny tests were not run.
gpt-6.1-sol ｜ 𝕏
A transient registry failure during startup restore is retried and is not recorded as a signature rejection. A boot-mismatch clear waits while a record cannot be saved. A failed candidate during baked recovery keeps the installed selection and returns to the baked executable without treating that process as the installed release.
|
@pullfrog review |
There was a problem hiding this comment.
Caution
A registry outage during cold start leaves moderation stopped even when a compatible baked executable is available, contrary to the explicit boot-recovery requirement in #188. One finding is detailed inline.
Reviewed changes across the complete current diff against main, including the publication contract and owner requirements in #188.
- Signature admission: Cosign artifact parsing, P-256 verification, repository/digest binding, expiry, trusted-key rotation, and transient/permanent error classification before discovery and extraction.
- OCI extraction: Newest-first multi-layer resolution, lower-layer whiteouts, same-layer replacements, duplicate requested paths, unsafe-path checks, and extraction bounds.
- Boot recovery: Schema-checked baked fallback, retained installed identity through failed replacements and rollback, readiness reporting, and deferred mismatch receipts across the bot/API writer boundary.
- Persistence and rollout: Transactional mismatch-map writes, last-record deletion, staging cleanup, Docker signing-key inputs, launcher capability/minimum revision 19, documentation, and affected fixtures.
- Verification: All 125 updater tests, both boot-mismatch database regressions, updater Clippy with
-D warnings, formatting, whitespace checks, publication contracts, and service boundaries pass. Current GitHub CI is green. The new transient-restore regression explicitly confirms that an unavailable registry leaves no application child; it tests the behavior identified below rather than the issue's required fallback.
Tests use real local SQLite and supervisor child processes with fixture executables and local signed HTTP registries, not live providers. Full preflight, container builds, remote libSQL, and live GHCR/Discord/Bunny tests were not run. Temporary review code was removed and the worktree is clean.
gpt-6.1-sol ｜ 𝕏
| if registry_transient(&error) { | ||
| tracing::warn!( | ||
| component = %self.role, | ||
| digest = %release.digest, | ||
| "registry download stalled on transient error: {error:#}" | ||
| ); | ||
| return Err(error); |
There was a problem hiding this comment.
[P1] Preserve compatible baked startup during registry outages
On a cold start with an installed release recorded, a connection failure, HTTP 503, or HTTP 429 returns here without starting any application, even when the baked binary supports the database. The bot consequently remains unable to moderate for the entire registry outage, contrary to the owner's explicit fallback requirement, which requires stopping only when no available binary can open the database.
Technical details
# Transient restore failures must not gate compatible local startup
## Affected sites
- `crates/updater/src/main.rs:1988-1994` returns the transient staging error before the schema-checked baked fallback can run.
- `crates/updater/src/signature.rs:336-356` removes registry-outage classification from boot repair entirely.
- `crates/updater/src/controller_tests.rs:8581-8616` encodes the opposite of the accepted behavior by asserting that the application and mismatch state remain absent during an outage.
## Required outcome
- Start the trusted baked executable when its core/guild compatibility checks pass, even if restoring the selected image fails because the registry is unavailable.
- Preserve the installed selection for later restoration, log the outage, and retain an accurate durable mismatch reason without treating the outage as a bad signature.
- Continue refusing unverified downloaded executables; falling back to the container's baked binary must not bypass verification of registry content.
- Cover a cold-start registry outage with a compatible baked child and assert actual baked startup plus retained installed selection; also retain refusal coverage for incompatible schemas.
## Verification
The existing `transient_restore_does_not_boot_repair_or_record_mismatch` test passes against a real local SQLite database and an unreachable loopback registry. It reaches the transient-error assertion and verifies `running.is_none()`, confirming the availability failure directly. The normal supervisor propagates that restore error and retries without launching the compatible application.
Scope auditThis PR is item 3 of #188: verify the image signature before staging and during boot repair. It is 63 commits and roughly 3,200 lines. Its own branch was stacked on #379 and merged back into it about 10 times, so it carried about 25 publish and CI commits that belong to #379. It also added a multi-layer OCI extractor with whiteout rules, a boot-mismatch retry and control path, a launcher revision bump and registry-outage retry. Guideline for any redo:
No more fix pushes here until the owner decides whether to close and redo this PR. |
Summary
Addresses review feedback on image signature verification and boot-mismatch persistence.
deploy/into bot/api image builds so the embedded signing public key compiles..sigtags as either a raw simple-signing envelope or an OCI manifest whose layers hold the envelope; accept a valid later signature when Cosign appends renewals.files.sha256are found even when split across COPY steps.record_boot_mismatch/clear_boot_mismatchthroughControlOpwhen the API launcher is not the local writer, with deferred retry when receipts are not yet durable.execute_rowswhen deleting the last boot-mismatch meta row, with a regression test.bad image signaturefor boot repair classification.Merged
cursor/sign-release-images-3fd7publisher hardening for aligned CI scripts.Merged current main.
Part of #188
Testing
cargo fmt --all -- --checkgit diff --checkcargo clippy --locked --workspace --all-targets -- -D warningscargo clippy --locked -p jmodguard-api --no-default-features --all-targets -- -D warningscargo test --locked --workspacecargo test --locked -p jmodguard-api --no-default-features --libbash scripts/publish-image-test.shbash scripts/ci-remote-database.shSummary by Sourcery
Enforce signed OCI releases throughout staging and boot repair while preserving safe fallback and durable mismatch state.
New Features:
Bug Fixes:
Enhancements:
Build:
CI:
Documentation:
Tests:
Summary by Sourcery
Verify release image signatures throughout staging and boot recovery while hardening layered image handling and durable mismatch recovery.
New Features:
Bug Fixes:
Enhancements:
Build:
CI:
Documentation:
Tests: