Repository navigation
feat(deployment): move the hidden variables members sealed for teams onto team keys - #4477
Conversation
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.
Tip: disable this comment in your organization's Code Review settings.
|
Warning Review limit reached
This review includes 9 billable files and costs up to $2.25.
Reviews can continue after your included limit without a manual trigger. An admin must approve usage-based billing. Or wait 25 minutes for your next included review. View limit details
Comment |
There was a problem hiding this comment.
ℹ️ No critical issues — one minor test-coverage suggestion inline. Everything else checks out: I verified the repository iterator is unscoped in the CLI context, the
#sealrefactor is behavior-preserving, the departed-member classification matches whatopenStoredthrows, and the full unit suite, the integration spec (real Postgres + KMS stand-in),tsc --noEmitand eslint all pass.
Reviewed changes
- Team secrets reseal command —
reseal-team-deployment-secrets [--dry-run] [--batch-size N]wired inconsole.tsthroughTeamSecretsResealController, keyset-paging every live-team deployment that holds a secret via the newfindStoredSecretsOfTeamsIteratively(inner join onOrganizations,deleted_at IS NULL,type = 'team', cursor onid). - Reseal pass — already-team-sealed values are counted and skipped (idempotent, resumable); member-sealed values are opened through the deployment read path (
openStored, so a value bound to another member is refused rather than moved), re-sealed under the team's key, and written throughresealIfUnchangedso a concurrent writer's token is never overwritten. SdlSecretsService.sealForTeam— seals undersealerOfOrganizationregardless ofteam_data_keys, withsealForStoragerefactored onto a shared#seal(behavior-preserving; covered by the flag-off spec).- Error handling — 409
stored_secrets_unavailable(departed member) is reported asdeploymentsWhoseMemberKeyIsGonewithout failing the run; other failures are logged per deployment and fail the run non-zero; each page runs in its own execution context so unwrapped keys are held per page, not per fleet.
GLM-5.3 | 𝕏
d01e92e to
3ee3bb8
Compare
63066e0 to
d884935
Compare
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
This run covers the delta since the prior pullfrog review — a single test-only commit landing the repository-level coverage that review asked for.
- Pinned the team iterator's multi-page behavior against a real database —
3a0f226adds a "re-sealing the stored secrets filed in teams" describe todeployment-setting.repository.integration.ts: three live-team rows holding secrets are seeded alongside a no-secret team row, a personal row and a deleted-team row, thenfindStoredSecretsOfTeamsIterativelyis iterated withbatchSize: 1, asserting every batch is single-row, no id repeats across pages, all three team rows are paged, and the control rows are left out.
Verified on this run: the new test passes against real Postgres, tsc --noEmit is clean, and two mutations confirm the test's teeth — dropping the isNotNull(sealedSecrets) filter and breaking the single-page stop condition each fail their respective assertions.
GLM-5.3 | 𝕏
3ee3bb8 to
ec03cac
Compare
…onto team keys The reseal-team-deployment-secrets command reads every deployment filed in a live team that holds a secret, opens a value its member sealed through the path a deployment reads it by, seals it again under the team's key and writes it back only if the row still holds what was read. A value already sealed for the team is counted and left alone, so a re-run picks up where an earlier one stopped. A value whose member key went with the member's account cannot be moved and is reported. A dry run opens and counts without writing anything. Part of CON-1213
… a real database Part of CON-1213
3a0f226 to
f72cedd
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4477 +/- ##
==========================================
+ Coverage 93.89% 94.07% +0.18%
==========================================
Files 1726 1619 -107
Lines 49658 46908 -2750
Branches 12165 11617 -548
==========================================
- Hits 46624 44131 -2493
+ Misses 2726 2485 -241
+ Partials 308 292 -16
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|

Why
Part of CON-1213. Builds on #4473, #4474 and #4476 (all merged).
With
team_data_keyson, new and updated team deployments are sealed under the team's key. Values sealed before that still sit under the key of the member who filed the deployment, and they still go if that member deletes their account. This moves them onto the team's key.It's a console command rather than a local script. Opening a member's value unwraps that member's data key through the prod KMS key, so it should run in the cluster with the API's own KMS binding, like
rekey-user-data-keyandrewrap-data-keys. A local script would put prod KMS access and plaintext hidden variables on a laptop.What
reseal-team-deployment-secrets [--dry-run] [--batch-size N]reads every deployment filed in a live (not deleted) team that holds a secret, keyset-paged by id.openStored, sealed again with the newsealForTeam(which ignores the flag, since this is an explicit operator action) and written withresealIfUnchanged. A row another writer changed in between is counted, not overwritten.deploymentsWhoseMemberKeyIsGone(organizationId/dseq) and doesn't fail the run. Any other failure is logged, the rest still move, and the run exits non-zero.--dry-runopens and counts without sealing or writing.Rollout
team_data_keyson for everyone. If it's off for some members, their later writes move values back to their own keys.reseal-team-deployment-secrets --dry-run, compare with the SQL.reseal-team-deployment-secrets. Re-run untildeploymentsResealedis 0. The SQL should then show onlyteam keyandmember key gone.Read-only sizing SQL: team deployments with hidden variables, grouped by the key their token names.
filer's keyis what the command moves.member key gonecan't be recovered.Testing
sealForTeam.findStoredSecretsOfTeamsIterativelypages live teams' tokens one row at a time and leaves out rows with no token, personal deployments and deleted teams.