Skip to content

fix(notify): allow staged rollout verification#122

Merged
psimaker merged 3 commits into
mainfrom
agent/helper-rollout-draft-visibility
Jul 14, 2026
Merged

fix(notify): allow staged rollout verification#122
psimaker merged 3 commits into
mainfrom
agent/helper-rollout-draft-visibility

Conversation

@psimaker

@psimaker psimaker commented Jul 14, 2026

Copy link
Copy Markdown
Owner

Summary

  • grant the owner-gated rollout job the contents permission required to see private draft releases
  • validate the exact release state and emit explicit resolution errors
  • statically reject release commands, GraphQL mutations, and mutating REST methods from the rollout job

Root cause

  • both publication attempts failed before any asset download because the contents:read job token could not resolve the private draft
  • GitHub documents draft listings as visible only with push access
  • staging with contents:write resolved the same draft successfully, while the rollout job failed twice with contents:read

Security and release truth

  • permission remains job-local and the exact permission map is policy-enforced
  • checkout credentials remain non-persistent
  • the job performs only validated GraphQL queries and REST GET asset downloads
  • private-draft verification and idempotent post-public recovery remain supported without release mutation
  • helper rollout, finalization, and public release have not yet run

Verification

  • ruby -c .github/scripts/notify-publish-safety.rb
  • ruby .github/scripts/notify-publish-safety.rb
  • jq -er missing/present release-ID exit-status regression
  • /tmp/actionlint-1.7.12/actionlint -shellcheck=/tmp/shellcheck-v0.10.0/shellcheck .github/workflows/docker.yml
  • /tmp/vaultsync-pr116-tools/zizmor/zizmor --min-severity medium .github/workflows/docker.yml
  • cd notify && go test ./... -count=1
  • git diff --check

Grant the owner-gated helper rollout job the contents permission GitHub requires to resolve and download draft release assets before finalization.

Keep the job read-only by policy, require the exact release to remain a draft, and surface draft-resolution failures explicitly.
@coderabbitai

coderabbitai Bot commented Jul 14, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The publish-safety policy now recognizes rollout-verify as a write-permission job while enforcing its permission shape and release read-only behavior. The workflow grants contents write access and adds explicit diagnostics for release resolution and draft-state validation failures.

Changes

Rollout safety enforcement

Layer / File(s) Summary
Publish-safety policy validation
.github/scripts/notify-publish-safety.rb
rollout-verify is added to the write-job policy, requires contents: write and packages: read, validates draft-state handling, and rejects release mutation commands.
Rollout workflow permissions and diagnostics
.github/workflows/docker.yml
rollout-verify receives contents write access, while release lookup, parsing, visibility, metadata, and draft-state failures emit explicit error diagnostics.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

Suggested reviewers: dependabot[bot]

Poem

Drafts are checked, mutations flee,
Permissions guard the release sea.
Errors glow where lookups fail,
Rollout sails on a safer trail.

🚥 Pre-merge checks | ✅ 7
✅ Passed checks (7 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
No Private Note Leakage ✅ Passed Changed code only adds generic draft-release errors and read-only GitHub release fetches; no logging or network calls include note contents, vault paths, or secrets.
Bounded Ios Background Work ✅ Passed Not applicable: the PR only changes Ruby policy and a GitHub workflow, with no Swift background-execution code touched.
Bridge Contract Compatibility ✅ Passed Only .github policy/workflow files changed; no Go bridge or Swift bridge-service contract code, JSON shapes, or tests were modified.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title is concise, conventional-commit style, and matches the rollout verification safety changes.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch agent/helper-rollout-draft-visibility

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@psimaker psimaker changed the title fix(notify): allow draft rollout verification fix(notify): allow staged rollout verification Jul 14, 2026
@psimaker
psimaker marked this pull request as ready for review July 14, 2026 16:25
Accept both validated draft and published release states during read-only rollout verification so a failed post-finalization verification can resume without mutating existing assets.

Keep invalid or missing release states fail-closed.
@psimaker

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 14, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
.github/workflows/docker.yml (1)

1203-1206: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider using jq -er to activate this error handling.

Because the // empty filter prevents jq from outputting null, jq -r will exit with a 0 status code even if databaseId is absent. Since the input $lookup was already verified as valid JSON on line 1199, this command will practically never fail, making the fallback block dead code.

A missing ID falls through and is safely caught by the regex check on line 1207. If you want this specific parsing error message to be emitted when the ID is absent, add the -e flag to make jq exit 1 on an empty stream.

♻️ Proposed refactor
-            release_id=$(jq -r '.data.repository.release.databaseId // empty' <<<"$lookup") || {
+            release_id=$(jq -er '.data.repository.release.databaseId // empty' <<<"$lookup") || {
               echo "::error::Draft release database ID could not be parsed." >&2
               return 1
             }
🤖 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 1203 - 1206, Update the jq
invocation assigning release_id in the release lookup flow to use the
exit-status behavior that treats an empty result as failure, while preserving
the existing .data.repository.release.databaseId filter and error handler so
missing IDs emit the intended parse error.
🤖 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/workflows/docker.yml:
- Around line 1203-1206: Update the jq invocation assigning release_id in the
release lookup flow to use the exit-status behavior that treats an empty result
as failure, while preserving the existing .data.repository.release.databaseId
filter and error handler so missing IDs emit the intended parse error.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 5bc8b46e-9abc-41a6-888b-613b277139f7

📥 Commits

Reviewing files that changed from the base of the PR and between 8a72ae8 and b334e50.

📒 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/scripts/notify-publish-safety.rb (1)

16-16: LGTM!

Also applies to: 203-203, 221-224, 573-587

.github/workflows/docker.yml (1)

1163-1166: LGTM!

Make jq return a failing status when the draft/public release lookup yields no database ID, so the explicit parse diagnostic is reachable.

Require this exit-status behavior in the publish-safety policy.
@psimaker
psimaker merged commit f652c15 into main Jul 14, 2026
32 checks passed
@psimaker
psimaker deleted the agent/helper-rollout-draft-visibility branch July 14, 2026 17:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant