ci(cryptify): publish the cryptify image from this repo - #280
Conversation
Starts moving cryptify's image delivery here now that the crate lives here (#255). Adds build-cryptify, scan-cryptify and finalize-cryptify, mirroring the pg-pkg jobs, plus a cryptify_version output on release-plz-release so a release tags the image the way pg-pkg's does. Two deliberate choices. The image keeps the name the cryptify repo publishes today, so nothing that pulls it has to be repointed. That needs the existing GHCR package to grant this repository push access, which is a package setting no workflow can set, so it cannot happen in this PR. Until it does, all three jobs are gated on the repo variable PUBLISH_CRYPTIFY_IMAGE. Unset, they skip and nothing about today's delivery changes; merging this is inert. Without that gate the push would fail and turn delivery red on every push to main. Purely additive: 165 insertions, no deletions, and the pg-pkg jobs are untouched. `actionlint` reports no workflow or expression errors. Refs #255
|
/dobby review |
There was a problem hiding this comment.
Reviewed the diff plus a rule-compliance pass. Nothing here blocks merge — the PUBLISH_CRYPTIFY_IMAGE gate really does keep all three jobs inert, and I verified the build side works: cargo build --profile edge --bin cryptify --locked succeeds (2m07s), the resulting binary launches Rocket cleanly on conf/config.toml, cargo test --manifest-path cryptify/Cargo.toml --locked passes 157/157, and actionlint is clean on both the branch and the main baseline. The imagetools create shell logic reproduces correctly against simulated digests. All 30 CI checks are green with nothing in flight.
Two things I'd fix before merge, both cheap:
1. The "redundant but not harmful" premise is false, and it changes the checklist ordering. I confirmed this rather than inferring it:
encryption4all/cryptifyis not archived (last push 2026-07-29) and itsci.ymlstill runs onpush: main+pull_request, pushing the same three tag types to the sameghcr.io/encryption4all/cryptify.- That repo passes no
build-args, so it builds with the Dockerfile's defaultCARGO_PROFILE=release(LTO on) against its ownCargo.lock. This job passesCARGO_PROFILE=edge(LTO off) against the workspace lockfile. Different binaries, same mutable tag. - Both trees are at
0.1.27, that repo has an openchore: release v0.1.28PR, and this repo's ownrelease-plz.toml:28-29says this workspace cutscryptify-v0.1.28onward. So both pipelines would push:0.1.28from different trees.:edgeand:pr-Nflip depending on which repo pushed last, and PR numbers are per-repo, so postguard #N and cryptify #N collide on:pr-N.
So enablement step 4 (retire the cryptify repo's build) has to land before or together with step 2, not after it. This sentence lives in two places — the PR body's "both build the same source from the same lockfile" and the checklist's "Step 4 is deliberately not here" — and the body is not in the diff, so please fix it there too, not just in the plan.
2. The new comment block landed inside the pg-ffi section (inline below). Moving lines 244-398 above line 239 fixes both ends.
The other three are optional nits, flagged inline. One rule note worth stating explicitly since it affects how you'd fix the cost nit: if you add a path filter to build-cryptify, scope it with a step (dorny/paths-filter + per-step if), not with on: paths: — a job skipped by an on: paths: filter reports no status at all, so any of these that is or becomes a required check would leave unrelated PRs pending forever. A job-level if: like the one you already have reports fine.
Also checked and clean: the PR title is valid conventional-commit (ci is an accepted type, and the Conventional Commit check passes); Refs #255 is the correct non-closing form here, since #255 is a closed decision issue this PR is follow-on work for, not something it resolves; env: at workflow level may use github.repository_owner, and env is available in every place you reference CRYPTIFY_IMAGE (step with: and run:); category: cryptify does avoid the SARIF collision; and no doc or runbook in the repo describes cryptify image publishing, so nothing outside the diff goes stale on merge.
| with: | ||
| images: ${{ env.CRYPTIFY_IMAGE }} | ||
| tags: | | ||
| type=edge,branch=main |
There was a problem hiding this comment.
Tag collision with the cryptify repo's still-live pipeline. Confirmed rather than inferred:
encryption4all/cryptifyis not archived (last push 2026-07-29) and its.github/workflows/ci.ymlstill runs onpush: mainandpull_request, pushing the same three tag types (type=edge,branch=main,type=ref,event=pr,type=raw,value=<version>) toghcr.io/encryption4all/cryptify.- Both trees are at
0.1.27; that repo has an openchore: release v0.1.28PR, and this repo'srelease-plz.toml:28-29says this workspace cutscryptify-v0.1.28onward. Both pipelines would push:0.1.28from different trees.
The PR body's "both build the same source from the same lockfile, redundant but not harmful" does not hold. The cryptify repo's build job passes no build-args, so it uses the Dockerfile's CARGO_PROFILE=release default (LTO on) against that repo's own Cargo.lock; this job passes CARGO_PROFILE=edge (LTO off) against the workspace lockfile. Different binaries under one mutable tag. :edge and :pr-N then flip depending on which repo pushed last, and PR numbers are per-repo, so postguard #N and cryptify #N write the same :pr-N.
The gate keeps this inert, so it is not a merge blocker — but step 4 of the enablement checklist (retire the cryptify repo's build) has to land before or together with step 2, not after it.
| @@ -232,6 +241,162 @@ jobs: | |||
| # --------------------------------------------------------------------------- | |||
|
|
|||
| # Build pg-ffi native libraries and upload as GitHub release assets. | |||
There was a problem hiding this comment.
The new block landed inside the pg-ffi section. Lines 239-241 are the pg-ffi: build native libraries for all platforms banner and line 243 is # Build pg-ffi native libraries and upload as GitHub release assets. — both now head the cryptify Docker block that starts at line 244, and build-ffi (line 400) is left with no comment at all, sitting under a banner that says cryptify. Moving lines 244-398 above line 239 fixes both ends.
| finalize-cryptify: | ||
| name: Finalize cryptify manifest | ||
| needs: [build-cryptify, scan-cryptify, release-plz-release] | ||
| if: always() && vars.PUBLISH_CRYPTIFY_IMAGE == 'true' && needs.build-cryptify.result == 'success' |
There was a problem hiding this comment.
nit — finalize-cryptify gates on needs.build-cryptify.result == 'success' only. scan-cryptify is in needs but its result is never checked, and always() drops the implicit success gate, so a critical CVE (the scan runs fail-build: true, severity-cutoff: critical) fails the scan job and the manifest is still pushed to :edge / :0.1.x. This is copied from finalize-docker (line 203), so it is pre-existing rather than introduced here — but adding && needs.scan-cryptify.result == 'success' to both is cheap while the file is open.
| platforms: ${{ matrix.platform }} | ||
| build-args: CARGO_PROFILE=edge | ||
| outputs: type=image,name=${{ env.CRYPTIFY_IMAGE }},push-by-digest=true,name-canonical=true,push=true | ||
| cache-from: type=gha,scope=cryptify-${{ matrix.name }} |
There was a problem hiding this comment.
nit — CI cost once the variable is on.
(a) Two more cache-to: type=gha,mode=max scopes on top of pg-pkg's two. mode=max stores every intermediate layer including cargo-chef's cooked target/, and GitHub's per-repo Actions cache is 10 GB with LRU eviction, so four full Rust dep caches (2 arches x 2 images) can evict each other and leave both images cache-cold. mode=min on the cryptify scopes would avoid that.
(b) build-cryptify has no path filter, so every postguard PR — docs-only included — runs two extra multi-arch cargo-chef builds and pushes two extra :pr-N tags, doubling PR-time Docker cost.
If you do add a filter for (b), put it in a step (dorny/paths-filter + per-step if), not in on: paths: — an on: paths:-skipped job reports no status at all, which permanently blocks any PR where one of these is a required check.
| - name: Resolve image reference | ||
| id: ref | ||
| run: | | ||
| DIGEST=$(ls /tmp/digests | head -1) |
There was a problem hiding this comment.
nit — this run: block has a pipe (ls | head -1) and no shell: key, so it executes under bash -e {0}, which has no pipefail; only an explicit shell: bash gets -eo pipefail. If ls fails, head still exits 0, the step goes green, and DIGEST is empty, so the next step emits ...cryptify@sha256: and the failure surfaces later as a confusing anchore pull error instead of here.
Honest caveat on the severity: this is byte-identical to scan-docker (line 173), so it is a copied pre-existing pattern, and the realistic failure mode (artifact downloaded but directory empty) leaves ls exiting 0 anyway, which pipefail would not catch either. Cheap to add shell: bash to both while the file is open; not worth a round on its own.
|
Done — I posted a consolidated review on this PR. It's a COMMENT, nothing blocks merge: the Two things worth fixing before merge though:
Also flagged a few optional nits inline (pre-existing patterns copied over, not new issues) — details in the review. |
Starts moving cryptify's image delivery into this repo now that the crate lives here. Refs #255.
Adds
build-cryptify,scan-cryptifyandfinalize-cryptifytodelivery.yml, mirroring the existing pg-pkg jobs, plus acryptify_versionoutput onrelease-plz-releaseso a release tags the image the way pg-pkg's does.Merging this changes nothing that runs today
All three jobs are gated on a repo variable:
Unset, they skip. That gate is load-bearing rather than decorative, see below.
The decision this PR does not make
The image keeps the name the cryptify repo publishes today:
so nothing that pulls
ghcr.io/encryption4all/cryptifyhas to be repointed. That requires granting this repository push access to the existing GHCR package — package settings → Manage Actions access → addencryption4all/postguardwith the Write role. No workflow can set that, so it cannot happen in this PR, and I could not verify the package's current settings either (this token lacksread:packages).Until that access exists the push would fail and turn delivery red on every push to main, which is why the jobs are gated rather than merged live.
The alternative is publishing under a new name. That needs no package settings change but moves the work to every deployment that pulls the image, which is privacybydesign/postguard-ops territory. Changing the one
env:line above is the whole cost of going that way instead.Turning it on
encryption4all/postguardWrite on thecryptifyGHCR package.PUBLISH_CRYPTIFY_IMAGE=true.:edge, and that a cryptify release also tags:0.1.x.Step 4 is deliberately not here. Until it happens both repos publish, which is redundant but not harmful, since both build the same source from the same lockfile.
Notes
build-cryptifyuses the repo root as build context withfile: cryptify/Dockerfile, because a workspace member cannot be built without the root manifest and its siblings.category: cryptifyso it does not collide with the pg-pkg scan's results.actionlintreports no workflow or expression errors; its shellcheck notes on the new jobs are the same ones the existing jobs already carry, kept for consistency.