ci(notify): require owner-gated publication#111
Conversation
Make pull requests and main pushes run Notify tests and a local image build without registry authentication or publication. Move image and binary publication behind a manual repository-owner dispatch bound to a pre-existing notify-vX.Y.Z tag, matching input, exact confirmation, and origin/main ancestry. Keep default workflow permissions read-only, grant narrowly scoped write permissions only to the gated jobs, disable checkout credential persistence, and pin every action in the publication workflow. Add a merge-blocking policy test with negative event, ref, actor, tag, and confirmation cases. A plain revert would restore automatic main-push publication; disable the workflow or land a forward safety fix before any rollback. No image or binary was pushed, and no tag, release, deployment, archive, signing, or store action was performed.
📝 WalkthroughWalkthroughAdds a Ruby policy checker and CI invocation, restructures Docker workflow validation, and introduces owner-gated manual publication of multi-architecture images and cross-compiled release binaries. ChangesPublication safety workflow
Estimated code review effort: 4 (Complex) | ~60 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 7✅ Passed checks (7 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
.github/workflows/docker.yml (1)
213-219: 🔒 Security & Privacy | 🔵 TrivialPublish-then-scan: the image (and
latest) is public before Trivy can gate it.
push: trueon the preceding build ships the multi-arch image and moveslatestbefore this scan runs, so a HIGH/CRITICAL finding fails the job only after the artifact is already exposed on GHCR. This is largely inherent (multi-arch images can't beloaded for a pre-push local scan the way the validation job does), and the PR/main validation build already scans amd64, but arm64 layers and newly-disclosed CVEs are only caught after publication. If tightening this matters, consider building without push, scanning per-platform, then pushing on success — otherwise treatlatestreassignment as provisional until this step passes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/docker.yml around lines 213 - 219, Adjust the publish workflow so the multi-architecture image is not pushed or assigned to latest until Trivy scanning completes successfully. Update the preceding build/publish flow around the image digest and the “Scan published image” step to build without publication, scan each platform’s image (including arm64), and push the final manifest only after the scan passes; preserve the existing HIGH/CRITICAL and ignore-unfixed gating..github/scripts/notify-publish-safety.rb (1)
43-47: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win
flattened_step_textskipsenv, leaving a hole in the privileged-op scan.The forbidden-operation guard at Lines 128-136 (and the
secrets.GITHUB_TOKENcheck in particular) only seesuses/run/withvalues. A normal validation job could still expose the token via a stepenv:such asTOKEN: ${{ secrets.GITHUB_TOKEN }}and pass the policy unchallenged. Including stepenvvalues closes that gap without affecting the login/release/gate detections (their env blocks don't contain those markers).🔒️ Include step env in the scan
def flattened_step_text(job) steps(job).flat_map do |step| - [step["uses"], step["run"], step.fetch("with", {}).values] + [step["uses"], step["run"], step.fetch("with", {}).values, step.fetch("env", {}).values] end.flatten.compact.join("\n") endAs per path instructions, review CI for "secret scoping ... and unnecessary privilege escalation."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/scripts/notify-publish-safety.rb around lines 43 - 47, Update flattened_step_text to include values from each step’s env mapping alongside uses, run, and with values. Preserve the existing flattening, compaction, and newline-joining behavior so the privileged-operation and secrets.GITHUB_TOKEN scans inspect environment values as well.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In @.github/scripts/notify-publish-safety.rb:
- Around line 43-47: Update flattened_step_text to include values from each
step’s env mapping alongside uses, run, and with values. Preserve the existing
flattening, compaction, and newline-joining behavior so the privileged-operation
and secrets.GITHUB_TOKEN scans inspect environment values as well.
In @.github/workflows/docker.yml:
- Around line 213-219: Adjust the publish workflow so the multi-architecture
image is not pushed or assigned to latest until Trivy scanning completes
successfully. Update the preceding build/publish flow around the image digest
and the “Scan published image” step to build without publication, scan each
platform’s image (including arm64), and push the final manifest only after the
scan passes; preserve the existing HIGH/CRITICAL and ignore-unfixed gating.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a4433d09-e743-4242-a815-cd46f013a581
📒 Files selected for processing (3)
.github/scripts/notify-publish-safety.rb.github/workflows/ci.yml.github/workflows/docker.yml
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
.github/workflows/**/*.yml
⚙️ CodeRabbit configuration file
.github/workflows/**/*.yml: Review CI for correct Go test tags, Xcode/iOS simulator assumptions, secret scoping,
dependency integrity, permissions, Docker publishing safety, and unnecessary privilege escalation.
Files:
.github/workflows/ci.yml.github/workflows/docker.yml
🔇 Additional comments (4)
.github/scripts/notify-publish-safety.rb (1)
29-57: LGTM!Also applies to: 59-124, 138-197
.github/workflows/ci.yml (1)
83-85: LGTM!.github/workflows/docker.yml (2)
4-38: LGTM!Also applies to: 40-104, 106-151, 154-212, 220-277
152-153: 🩺 Stability & AvailabilityNo issue:
origin/mainis available in this checkout.actions/checkoutwithfetch-depth: 0fetches the remote-tracking branches and tags here, so the ancestry gate can readrefs/remotes/origin/mainas written.> Likely an incorrect or invalid review comment.
1 participant
What and why
A normal
mainpush touchingnotify/**previously received package, OIDC, and attestation write permissions, logged into GHCR, and pushed the mutablelatestimage. This change makes pull requests and normalmainpushes validation-only and reserves every image/binary publication for a separate explicit repository-owner action.Normal-event safety
mainpushes run the policy guard, Notify tests, a locallinux/amd64image build withpush: false, and a local Trivy scan.contents: read.persist-credentials: false.Owner publication gate
The two privileged jobs can run only after a manual
workflow_dispatchwhere all of the following hold:notify-vX.Y.Ztag;release_taginput exactly matches that ref;PUBLISH_NOTIFY_RELEASE;origin/main.The gate itself is read-only. Image publication receives only package/provenance scopes; binary publication receives only
contents: write. The workflow never creates a tag. No manual dispatch was triggered by this change.Automated policy
.github/scripts/notify-publish-safety.rbparses both workflows and enforces trigger, permission, checkout, immutable-action, dependency, login, build-push, release-command, and main-ancestry invariants. Its model rejects eight negative event/ref/tag/input/actor cases and permits only the exact owner/tag/confirmation case. The merge-blockingNotify Testsjob executes this policy on every pull request.Local validation
ruby -c .github/scripts/notify-publish-safety.rb— passedruby .github/scripts/notify-publish-safety.rb— 8 negative cases blocked; 1 exact positive model case passedactionlint v1.7.12with ShellCheck v0.10.0 — passed fordocker.ymlandci.ymlzizmor v1.26.1 --pedantic .github/workflows/docker.yml— no findingscd notify && go test ./... -count=1— passedcd notify && go vet ./...— passedgofmt -l notify— emptycd go && make patch && go test -tags noassets ./bridge -count=1— passed in 57.891 secondsgofmt -l go/bridge— emptynotify/tests/m4-docker/run.sh— passednotify/scripts/tests/install-dry-run-test.sh— passed without privileged or mutating commandsmake xcframework— passedxcodebuild build testwith code signing disabled — passed.github— no leaksgit diff --check origin/main...HEAD— passedCompatibility, privacy, and rollback
No app, helper, bridge, Relay, Trigger v1, APNs, StoreKit, wire, persisted-state, namespace, credential, pairing, cleanup, backup, versioning, conflict, or tombstone behavior changes. Existing users receive no runtime or migration change.
A plain repository revert would restore unsafe automatic
main-push publication. Safe rollback is to disable the Docker workflow through an explicit owner action or land a forward safety replacement before reverting this commit. No published artifact exists from this milestone.Release gate
VaultSync 2.0 remains NO-GO. The strongest implemented runtime proof remains a fresh successful local file
ItemFinishedapply after cursor, RFC3339-nanosecond time, and stable engine-generation baselines.No image or binary was pushed. No tag, release, deployment, rollout, archive, signing, notarization, TestFlight/App Store Connect mutation, submission, or publication was performed.