Distillation

Date: Aug 29, 2026

• 🟡 (13:30) User provided src/modules/update-issue.js (77 lines) from /home/byk/Code/getsentry/publish. updateTargets({ octokit, version, publishRepo, issue_number }) reads ${process.env.GITHUB_WORKSPACE}/__repo__/.craft-publish-${version}.json only if it exists; concurrently fetches the issue and parses the Craft state JSON; then calls transformIssueBody(craftState, issue.body) and updates the issue body. updateIssue({ context, octokit, inputs }) concurrently calls updateTargets(...) and removes the accepted label via octokit.rest.issues.removeLabel.
• 🟡 (13:30) transformIssueBody(craftState, issueBody) in src/modules/update-issue.js:37-60 replaces TARGETS_SECTION_PARSER_REGEX; for each TARGETS_PARSER_REGEX target it preserves/list-checks the target as - [x] ${targetId} when craftState.published[targetId] is truthy, otherwise - [ ] ${targetId}; it appends previously undeclared keys from Object.keys(craftState.published) in the same checkbox format, followed by newline separators.
• 🟡 (13:31) Assistant security review reported the previously run targeted Vitest suite passed 33/33 tests across six approval-attestation/authorization test files, but concluded DO-NOT-MERGE due to five MUST-FIX findings.
• 🟡 (13:31) Assistant MUST-FIX finding: approval is not cryptographically bound to the actual release request. src/modules/approval-attestation.js:11-25 signs only actor, accepted-event ID, and title; src/publish/authorize-approval.js:111-136 reads only the issue title; and src/publish/validate-approval-attestation.js:63-99 validates only title, labels, events, and comments. The issue body and dry-run label remain mutable after approval.
• 🟡 (13:31) Assistant explained the release-request integrity attack path: ci-poller.yml:149-156 extracts CI SHA from the mutable issue body; src/modules/details-from-context.js:36-64 derives dry-run, merge target, and targets from mutable issue state; and src/publish/inputs.js:6-9 later uses a ci-ready event snapshot that can contain attacker-modified body/labels. A Publish-repository issue editor could alter the checked SHA to a known-green commit, targets, merge target, or remove dry-run after valid approval; the poller validates CI for the altered SHA and Craft publishes the altered request.
• 🟡 (13:31) Assistant recommended binding a canonical request digest—including relevant body-derived fields, dry-run state, and checked SHA—to both attestations and validating it at every transition. Existing tests cover title changes in validate-approval-attestation.js:58-95, but not issue/body/label mutation.
• 🟡 (13:31) Assistant MUST-FIX finding: the approval gate permits self-approval. src/modules/approval-authorizer.js:1,28-34 authorizes any target-repository collaborator with write, maintain, or admin; src/publish/authorize-approval.js:111-120 does not fetch issue author and therefore cannot reject the requester approving their own release request.
• 🟡 (13:31) Assistant stated that a user with Publish issue-label rights and target-repository write access can create a release issue and apply accepted themselves, violating the second-person approval boundary. Recommended fetching and binding requester identity, rejecting self-approval except on an explicit automatic path, and adopting a designated-approver policy rather than generic write. Existing src/modules/__tests__/approval-authorizer.js:44-65 explicitly accepts write and lacks requester/approver-separation coverage.
• 🟡 (13:31) Assistant MUST-FIX finding: auto-approval provenance is lost and the intended automatic path fails closed. auto-approve.yml:12-15 restricts issue creation to trusted bots, but auto-approve.yml:20-34 applies accepted with the sentry-internal-app token, making the issues.labeled actor sentry-internal-app[bot].
• 🟡 (13:31) Assistant noted publish.yml:61-67 passes the label actor as APPROVAL_ACTOR, while approval-authorizer.js:2,20-26 recognizes only getsantry[bot] as automatic; therefore normal auto-approved releases fail authorization and have accepted removed. approval-authorizer.test.js:8-24 models this wrong identity directly. Recommended preserving and validating trusted opener/auto-workflow provenance in an attestation tied to both issue and release request; explicitly warned not to trust every sentry-internal-app[bot] label because that would authorize every workflow able to mint that token.
• 🟡 (13:31) Assistant MUST-FIX finding: revocation can race validation. validate-approval-attestation.js:63-69 independently fetches issue state, events, and comments in parallel; currentLabeledEvent in approval-attestation.js:98-120 considers only labeled events, while live label presence is from a separate, non-atomic issue response.
• 🟡 (13:31) Assistant described a race where an actor removes accepted or ci-ready after the issue response but before validation finishes. publish.yml:196-219 validates only once before continuing to privileged Craft execution at publish.yml:265-381; the immutable event-payload condition in publish.yml:179-185 does not protect current live state. Recommended a fenced state transition plus revalidation of the live canonical request immediately before Craft, with an adversarial test for label removal after initial issue read; current tests cover only stable label-absent response in validate-approval-attestation.test.js:97-131.
• 🟡 (13:31) Assistant MUST-FIX finding: publish.yml:327 executes mutable image tag getsentry/craft:latest while supplying cross-repository tokens and publishing secrets at publish.yml:349-381. Recommended pinning Craft to an immutable digest and applying the same immutable-reference policy to privileged Actions dependencies.
• 🟡 (13:31) Assistant concern: invalid-approval cleanup can strand a request unretryably. The handler asks users to re-add accepted in publish.yml:93-111, but relies on one removeLabel API call; if removal fails, accepted remains without ci-pending, and re-adding creates no event. Similar one-shot cleanup occurs in ci-poller.yml:106-112 and later invalidation branches. This fails safely but may require manual intervention; recommended retryable cleanup or recovery that removes stale accepted before requesting re-approval.
• 🟡 (13:31) Assistant PASS finding: normal approvals are re-authorized against the target repository rather than trusting a forged accepted label. publish.yml:49-67 and approval-authorizer.js:28-34 check current label actor permissions using the release-bot token; authorize-approval.js:57-77 paginates issue events; authorize-approval.js:24-30 and validate-approval-attestation.js:40-44 fail closed on API errors.
• 🟡 (13:31) Assistant PASS finding: CI-ready provenance is strongly tied to the internal app. ci-poller.yml:273-338 records authenticated label-app identity before adding ci-ready; publishing requires a trusted github-actions[bot] comment binding current approval, title, and actual ci-ready-label actor through approval-attestation.js:146-166 and validate-approval-attestation.js:89-99. This rejects manually added ci-ready labels; regression coverage is in validate-approval-attestation.test.js:283-342.
• 🟡 (13:31) Assistant PASS finding: shell handling avoids direct injection from title/body fields because publish.yml:61-67 and ci-poller.yml:95-105 pass user-controlled title data through environment variables, details-from-context.js:18-23 constrains parsed fields, and shell command arguments are quoted. Assistant distinguished the remaining mutable issue-body risk as semantic integrity rather than shell injection.
• 🟡 (13:31) Assistant PASS finding: API failures block release progression. ci-poller.yml:204-216 skips CI promotion if cross-repository status or check-run API calls fail; publish.yml:208-219 removes approval rather than publishing on invalid/missing attestations; workflow_dispatch remains available for manual recovery in ci-poller.yml:3-24.