fix(notify): resume failed helper publication safely#117
Conversation
Resolve draft releases through GraphQL release IDs so staged assets remain visible without treating API failures as absence. Allow an owner-only main-ref recovery only when it binds to the failed tag workflow and source commit, while forbidding image and attestation writes and checking out the immutable source for every publication job. Keep normal and recovery dispatches serialized per release tag and extend the publication safety policy.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughChangesThe publish workflow now supports recovery dispatches from Recovery publication
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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.
Inline comments:
In @.github/scripts/notify-publish-safety.rb:
- Around line 219-221: Update the checkout policy assertion to use dig when
reading the nested with/ref values, so missing with or ref entries yield nil and
assert_policy reports the existing policy failure instead of raising KeyError.
Preserve the immutable release_sha comparison and message.
In @.github/workflows/docker.yml:
- Line 36: Update the workflow concurrency group expression near the release-tag
selection so manually dispatched runs use a run-scoped fallback until the owner
check passes, preventing untrusted dispatches from sharing or evicting the
release-tag group; preserve the existing release-tag grouping only for
authorized owner publications.
- Around line 294-295: Split the recovery and non-recovery paths for both
publish-image (.github/workflows/docker.yml#L294-L295) and release-binaries
(.github/workflows/docker.yml#L506-L507) into separate jobs so recovery runs do
not inherit id-token: write or attestations: write. Retain only each job’s
required publish/write permissions, and place attestation and OIDC-dependent
steps in the non-recovery jobs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 8d58d2de-345b-4c9c-a69d-974ecd785881
📒 Files selected for processing (2)
.github/scripts/notify-publish-safety.rb.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/docker.yml
🔇 Additional comments (2)
.github/workflows/docker.yml (2)
23-34: LGTM!Also applies to: 143-164, 168-206, 208-223, 260-279, 312-316, 332-336, 352-361, 375-380, 436-458, 471-495, 522-526, 578-650, 723-790, 794-817, 860-885, 899-923, 968-973, 1008-1016, 1020-1027, 1042-1046
207-207: 🎯 Functional CorrectnessNo change needed here. The workflow run
pathfield is just the workflow file path, so the exact match is fine.> Likely an incorrect or invalid review comment.
Keep unauthorized dispatches out of release-tag concurrency groups and make malformed checkout policy entries fail with the intended diagnostic. Split normal image and binary attestation work from read-only recovery so recovery jobs cannot receive OIDC or attestation write permissions. Preserve exact source, digest, draft, and public retry verification across both paths.
|
@coderabbitai review |
✅ Action performedReview finished.
|
Summary
mainonly when it binds to the failed tag-bound publication run, source tag, source commit, failed step, and skipped rollout/finalization jobsFailed publication state being recovered
notify-v2.0.0664a896c5c14b86a806f8fa47b0d900b32649bff29324314809Stage Release Binaries / Create or verify the exact draft release and assets353712477, but the REST release-by-tag endpoint returned 404 for that draftSafety invariants
origin/mainnotify/release.jsonmust be byte-identical between the recovery workflow commit and the source taglatesttag is read or moved by the workflowValidation
ruby -c .github/scripts/notify-publish-safety.rbruby .github/scripts/notify-publish-safety.rbcd notify && go test ./... -count=129324314809and rejected an unrelated successful run353712477and failed closed on a simulated API/repository failuresha256:9718196b97afb6aee43c4578c1fb130a7874c34115f04b22c690b7d243667fc7verified against existing provenance and SPDX attestationslatestremainedsha256:90d4a56b0e7d5bf90541157c13b0265198df6893accf8e64b228587996e41fa4Compatibility and scope
This changes only publication orchestration and its executable safety policy. Helper runtime behavior, wire formats, Decision 024, credentials, pairing, namespace handling, existing-user behavior, upload/download/roundtrip semantics, backup/versioning/conflict/tombstone behavior, and Relay/App runtime are unchanged.