Conversation
CloudFormation stack for the cli-artifacts-prod account: a public-read bucket for slim native artifacts (anonymous GetObject only, TLS required, no listing, versioning with 30-day noncurrent expiry) and a slim-artifacts-publisher role trusted for develop with s3:PutObject on that bucket's objects and nothing else. The workflow lints the template on pull requests and deploys it from develop with the account's github-deploy role; the deploy job is skipped until CLI_ARTIFACTS_AWS_ACCOUNT_ID is set. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PeXSyQtKtGuwrdNW7uVKom
Contributor
There was a problem hiding this comment.
🤖 AI Review
Both independent reviews completed. Their 11 reports merge into 10 unique findings, all confirmed after reading the cited code; the dotted-bucket finding was raised by both reviewers. No critical or major issues were found.
Findings
| Severity | Location | Category | Sources | Claim |
|---|---|---|---|---|
| 🟡 MINOR | .github/workflows/cli-artifacts-infra.yml:69 |
infrastructure |
claude | The deployment automation cannot apply a changed TrustedSubject value to an existing stack because it supplies no parameter overrides. |
| 🟡 MINOR | infra/cli-artifacts/template.yaml:86 |
security |
claude | TrustedSubject is unrestricted and evaluated with wildcard-aware StringLike even though the documented value is intended to identify one exact branch. |
| 🟡 MINOR | infra/cli-artifacts/template.yaml:18 |
infrastructure |
claude | The artifact bucket lacks retention policies, so stack deletion or bucket replacement can fail when published objects remain. |
| 🟡 MINOR | infra/cli-artifacts/template.yaml:88 |
infrastructure |
claude | The publisher cannot abort failed multipart uploads because its object policy grants only s3:PutObject. |
| 🟡 MINOR | .github/workflows/cli-artifacts-infra.yml:23 |
ci |
claude | All pull-request lint runs and deployments share one concurrency group, allowing a newly queued run to cancel another pull request's pending check. |
| 🟡 MINOR | infra/cli-artifacts/template.yaml:54 |
cost |
claude | The stack exposes every object directly for anonymous S3 downloads without adding monitoring or mitigation for attacker-driven request and egress costs. |
| 🟡 MINOR | infra/cli-artifacts/template.yaml:11 |
input-validation |
claude+codex | BucketName accepts dotted names that are incompatible with the template's virtual-hosted HTTPS URL and also accepts some S3-invalid dot/hyphen combinations. |
| 🟡 MINOR | .github/CODEOWNERS:6 |
security |
claude | The new account-admin deployment workflow remains ownerless under CODEOWNERS and therefore is not specifically gated on CLI-team review. |
| ⚪ NIT | .github/workflows/cli-artifacts-infra.yml:75 |
ci |
claude | The Show stack outputs multiline shell block omits the explicit strict-mode declaration used by the workflow's other multiline shell blocks. |
| ⚪ NIT | infra/cli-artifacts/README.md:31 |
documentation |
claude | The README incorrectly says linting runs on every pull request even though the workflow is path-filtered. |
Findings outside the diff
- 🟡 MINOR
.github/CODEOWNERS:6— The new account-admin deployment workflow remains ownerless under CODEOWNERS and therefore is not specifically gated on CLI-team review.
Stats
Claude findings: 10 · Codex findings: 1 · Confirmed: 10 · Refuted: 0 · Uncertain: 0
Models: claude-opus-5 + gpt-5.6-sol · Trigger: auto · Workflow run
This review runs once per PR. A maintainer can request another with a /ai-review comment.
Use the partition pseudo-parameter in the publisher role trust principal, delete a stack left in ROLLBACK_COMPLETE by a failed initial create before deploying (that state cannot be updated and holds no resources), and print the failed stack events when a deploy fails. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PeXSyQtKtGuwrdNW7uVKom
Match the publisher trust subject exactly and constrain both template parameters, retain the bucket on stack delete or replace, let the publisher abort its own failed multipart uploads, pass the parameters explicitly on every deploy so a change actually reaches the live stack, scope the workflow concurrency per pull request with a separate serial group for deploys, and put the deploy workflow under CLI maintainer ownership. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PeXSyQtKtGuwrdNW7uVKom
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PeXSyQtKtGuwrdNW7uVKom
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Infrastructure for the public-read S3 mirror of slim native artifacts, in the dedicated
cli-artifacts-prodAWS account (created insupabase/aws-org-root). The CLI will fall back to this host where registry blob CDNs and GitHub release assets are blocked, for example in agent sandboxes that allow*.amazonaws.com. This PR is infrastructure only; the upload job in the mirror workflow and the CLI-side candidate follow in separate PRs.infra/cli-artifacts/template.yaml(CloudFormation):supabase-cli-artifacts: anonymousGetObjectthrough the bucket policy, TLS required, noListBucket, ACL-based public access blocked, SSE-S3, versioning with 30-day noncurrent expiry and abort of stale multipart uploads. Retained on stack delete or replace.slim-artifacts-publisher: trusts the account's GitHub OIDC provider for exactlyrepo:supabase/cli:ref:refs/heads/developand can do nothing buts3:PutObjectands3:AbortMultipartUploadon the bucket's objects..github/workflows/cli-artifacts-infra.yml: runscfn-linton pull requests that touch this directory or the workflow, and deploys the stack on pushes todevelopwith the account'sgithub-deployrole, passing the bucket name and trusted subject explicitly. Pull requests never touch AWS, because onlydevelopcan assume that role. The deploy job is skipped until theCLI_ARTIFACTS_AWS_ACCOUNT_IDrepository variable exists, clears a stack left inROLLBACK_COMPLETEby a failed first create, and prints failed stack events on error. The workflow file is placed under CLI maintainer ownership in CODEOWNERS.Object layout, kept identical to the GitHub release asset names so checksum files match without rewriting:
Linked issue
Tracked in Linear (CLI-2451); no GitHub issue.
open-for-contributionlabel (or I'm a Supabase maintainer).Checklist
fix(cli): …).cfn-lintin the new workflow.pnpm check:allpasses; relevant package tests pass for every touched workspace, andpnpm types:checkpasses for each touched TypeScript workspace (or workspace declaring it). No TypeScript workspace is touched.🤖 Generated with Claude Code
https://claude.ai/code/session_01PeXSyQtKtGuwrdNW7uVKom