feat: harden GuardScan release distribution infrastructure - #36
feat: harden GuardScan release distribution infrastructure#36ntanwir10 wants to merge 23 commits into
Conversation
|
Warning Review limit reachedNext included review available in 51 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (96)
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 review |
|
@codex review |
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
GuardScan/.github/workflows/release-train.yml
Lines 856 to 860 in 7db43b8
Fail when the publication-ledger commit fails
In the publication-recording section, git commit ... || exit 0 treats every commit failure as the expected no-op case. If the commit fails because of a hook, repository corruption, identity/configuration error, or another unexpected Git error, the subshell exits successfully before the push, so the workflow can report success after publishing public artifacts without persisting their ledger events or active-train entry. Test whether the index is empty explicitly and allow only that case to skip the commit; propagate all actual commit failures.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (resolved.length === expectedCount) { | ||
| const types = ['winget', 'chocolatey'].includes(channel) | ||
| ? ['channel_accepted', 'channel_verified'] | ||
| : ['channel_verified']; |
There was a problem hiding this comment.
After the first successful hourly canary, each required channel is already verified, but the next run emits another run-specific channel_verified event here (and moderated channels emit channel_accepted first). materializeReleaseState does not allow verified -> verified or verified -> accepted, so the second scheduled RC canary makes the ledger unmaterializable; subsequent reconciliation and the required 24-hour promotion can no longer complete. Check the materialized channel status and append status transitions only when they actually advance or recover the channel.
Useful? React with 👍 / 👎.
| timestamp: existing?.timestamp || timestamp, | ||
| type: 'channel_published', | ||
| channel, | ||
| idempotencyKey, |
There was a problem hiding this comment.
When the first catalog reconciliation occurs after a successful Homebrew/Scoop canary, those channels are already verified, yet this step appends channel_published. The state machine permits published -> verified but not verified -> published, so this timing corrupts the ledger and the promotion step fails while materializing it. Reconciliation should avoid appending this publication transition once the channel has reached verified or another later terminal state.
Useful? React with 👍 / 👎.
| channel, | ||
| idempotencyKey: `${type}:${suffix}`, | ||
| payload: { | ||
| remoteIdentity: `${report.channel}:${version}`, | ||
| }, | ||
| payload: {}, |
There was a problem hiding this comment.
On the first successful public WinGet or Chocolatey canary, this block emits channel_accepted with an empty payload. validateModerationEvent requires every moderated channel_accepted event to contain the artifact IDs, remote identity/digest, and canonical submission evidence, so appendEvent rejects the event before channel_verified can be recorded. As a result, a stable train cannot complete either moderated channel even when the package is publicly installable; reuse the provider-bound submission evidence already stored in the channel state when constructing this event.
Useful? React with 👍 / 👎.
| for (const {file, report} of versionReports) { | ||
| if (report.status === 'pending') continue; | ||
| const suffix = `${process.env.GITHUB_RUN_ID}:${process.env.GITHUB_RUN_ATTEMPT}:${report.channel}:${report.target}:${path.basename(path.dirname(file))}`; | ||
| appendEvent(ledger, { |
There was a problem hiding this comment.
The report timestamp is captured in the matrix job, before the serialized ledger-writing job starts. If a scheduled reconciliation or another protected-ledger writer appends an event while the canary matrix is running or waiting for the release-ledger concurrency lock, this timestamp can precede the ledger's latest event; validateEvent then rejects it because release-event timestamps may not move backward. Use a timestamp at append time (or clamp it to the previous ledger timestamp) and retain the original check time separately if needed.
Useful? React with 👍 / 👎.
| for (const [channel, channelState] of Object.entries(state.channels)) { | ||
| if (!['github', 'pnpm', 'yarn', 'bun', 'homebrew', 'scoop'].includes(channel) | ||
| || !['published', 'submitted', 'accepted', 'verified'].includes(channelState.status)) { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
When rollback is initiated after a canary has marked a repository-owned channel such as GitHub, Homebrew, or Scoop failed, this filter skips that channel even though the state machine explicitly permits failed -> superseded. The workflow then records repository recovery as complete and removes the defective train from active-versions.json, leaving the failed public channel non-terminal and no longer monitored. Include failed in the statuses that receive the rollback terminal event.
Useful? React with 👍 / 👎.
1 participant
Stack 3 of 4 for #32; depends on the preceding catalog PR. Review boundary: 3335331..7db43b8 (97 changed files). Adds dependency-ordered release distribution infrastructure, CI and package-channel safeguards, documentation, and path normalization. Automation and publication remain disabled.