Skip to content

Read every secret prod needs in the move-day deploy, held by a test - #273

Merged
esthor merged 1 commit into
mainfrom
fix/move-day-deploy-secrets
Oct 11, 2026
Merged

esthor merged 1 commit into
mainfrom
fix/move-day-deploy-secrets

Conversation

@esthor

@esthor esthor commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

preview: ready coderabbit: approved findings: none checks: passing merge: ready

Preview · Change Stack · Checks · Files: +57 −0 in 2 files

  • infra/scripts/move-day-deploy.sh read six of the eight settings a prod
    deploy needs since the draft preview landed: without NEON_COLLAB_URL and
    COLLAB_ANSWERS_KEY its opening plan fails ("Expected string at
    COLLAB_ANSWERS_KEY"), checked against prod on 2026-10-11. It now reads,
    checks and exports both, from the same vault items as prod-deploy.
  • infra/tests/move-day-deploy.test.ts: every Config.String,
    Config.Redacted and requiredSecret name in alchemy.run.ts and src/ must
    be exported by the script, so a new setting can't break move day again.
    It fails on main with exactly the two names above.

Found while preparing the allthings.dev account move: bun run plan --stage prod --profile allthings with only the script's six reads fails on COLLAB_ANSWERS_KEY; with all eight it plans cleanly (3 updates, no delete/replace).

Summary by CodeRabbit

  • Bug Fixes
    • Production deployments now check that required collaboration settings are present before continuing and make those settings available to the deployed application. This helps prevent deployments from proceeding with incomplete configuration.

- infra/scripts/move-day-deploy.sh read six of the eight settings a prod
  deploy needs since the draft preview landed: without NEON_COLLAB_URL and
  COLLAB_ANSWERS_KEY its opening plan fails ("Expected string at
  COLLAB_ANSWERS_KEY"), checked against prod on 2026-10-11. It now reads,
  checks and exports both, from the same vault items as prod-deploy.
- infra/tests/move-day-deploy.test.ts: every Config.String,
  Config.Redacted and requiredSecret name in alchemy.run.ts and src/ must
  be exported by the script, so a new setting can't break move day again.
  It fails on main with exactly the two names above.
@coderabbitai

coderabbitai Bot commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 8b8150ed-f533-48fb-9654-e6a34ba11333

📥 Commits

Reviewing files that changed from the base of the PR and between b568c0d and b1f0228.


📒 Files selected for processing (2)
  • infra/scripts/move-day-deploy.sh
  • infra/tests/move-day-deploy.test.ts

Included review availability: This review used your included allowance. 3 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 4 reviews per hour.


📜 Recent review details
⏰ Context from checks skipped due to timeout. (3)
  • GitHub Check: 🧹 Lint
  • GitHub Check: 🚀 Deploy
  • GitHub Check: 🧪 Tests

🧰 Additional context used
📓 Path-based instructions (1)
infra declares allthings' Cloudflare resources with Alchemy v2 in alchemy.run.ts.

⚙️ CodeRabbit configuration file

Files:

  • infra/scripts/move-day-deploy.sh
  • infra/tests/move-day-deploy.test.ts


📝 Walkthrough

Walkthrough

The deployment script now reads, validates, and exports NEON_COLLAB_URL and COLLAB_ANSWERS_KEY. New tests compare settings used by the stack with settings exported by the script.

Changes

Deployment secrets

Layer / File(s) Summary
Read, validate, and export deployment secrets
infra/scripts/move-day-deploy.sh, infra/tests/move-day-deploy.test.ts
The script reads the collaboration secrets from 1Password, rejects empty values, and exports them. Tests check that stack settings include NEON_READER_URL and that every discovered stack setting is exported.

Priority: ⬆️ High

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix


Merge Risk | ⚪ Minimal · up to b1f02

Merge Risk: ⚪ Minimal · up to b1f02

The collaboration credentials are required before production planning and are exported for the stack. The new test checks that every current stack setting is exported; no actionable merge blocker is evident.

Security Architecture Review

Security architecture risk: 🔵 Low · up to b1f02

The change supplies existing collaboration resources with their required credentials while preserving database-role and request-authentication controls. No introduced security vulnerability was established. Risk is low because additional sensitive credentials enter the deployment process, and live permissions and deployment recovery guarantees were not independently verified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The added credentials are inherited by subsequent deployment and zone-polling subprocesses. Compromise of that execution context could expose the production collaboration database credential and the shared key used to open rounds sealed with it. Effective database privileges and live vault access were not verified.

Trust Boundaries and Controls

  • observed — Vault selectors are fixed rather than request-controlled. roleOrigin rejects invalid PostgreSQL URLs and usernames other than draft_collab. The existing preview handler rejects requests before dispatch unless Access-token signature, issuer, audience, time, and email checks succeed. Neither control is weakened by the PR.

Resilience and Maintainability Implications

  • observed — requiredSecret rejects missing or whitespace-only values but does not enforce answers-key format. Existing web and CLI consumers reject malformed keys, while sealing rejects invalid input and opening returns no plaintext on failure. A malformed vault value can therefore impair rounds without establishing a confidentiality bypass. These consumer controls and the presence-only stack helper predate the PR.

Hardening Proposals

  • proposed — Consider enforcing the existing answers-key format contract during preflight, and limiting collaboration-secret inheritance to commands that need those values. These are preventive improvements, not observed vulnerabilities.

Pre-merge checks | Passed 8
✅ Passed checks (8 passed)
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly describes the main change: reading all production secrets required by the move-day deployment and adding a test to verify them.
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files.
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.
Production Writes Take Their Planned Path Passed All four conditions hold. The changed deploy script still runs bun run plan --stage prod --profile allthings before bun run deploy --stage prod --profile allthings; the existing MediaUploadCheck…
Secrets Are Never Printed Passed The pull request changes only the move-day script and its test. The two added credentials use op read from the allthings vault, are captured in shell variables, checked for emptiness, and exported…
Database Roles Hold Only Listed Grants Passed All four conditions hold. The PR changes only infra/scripts/move-day-deploy.sh and adds infra/tests/move-day-deploy.test.ts; it changes no migration, role-grant, role-creation, or required privacy/acc…

✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR


✨ Simplify code
  • Commit to this branch
  • Create a new PR

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@esthor
esthor merged commit b954099 into main Oct 11, 2026
13 checks passed

This branch was successfully deployed

1 active deployment
preview — b1f02287 Deployed Oct 11, 2026 by esthor via 🚀 Deploy #786
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