feat(datastore): add npm's encrypted credential columns (S04-A per-format columns plan: 9/11)

What

Gives npm_remote_repositories its encrypted credential unit: four nullable columns (wrapped_dek, ns_key_id, ns_key_version, encrypted_auth_token), an all-or-none CHECK over exactly those four added NOT VALID and validated in a second migration, a (namespace_id, ns_key_id) index, and a composite foreign key to namespace_encryption_keys (id, namespace_id) with ON DELETE RESTRICT.

Schema only. Nothing reads or writes the new columns yet: no production write path sets them, and the only statements that write the four are the inline INSERT and UPDATE SQL in the migrations suite. The names do occur in non-generated Go outside the two regenerated jet files, but only inside comments, so grepping one returns prose rather than writers. npm's unit is four columns rather than Maven's and Container's five because npm stores a single bearer token; the shape for all three is fixed in S04-A's per-table instantiation, and every constraint, index and FK name here matches that section.

tmp_plaintext_auth_token stays, outside the unit, and stays the live credential store. Unlike the Maven and Container tables this one carries no CHECK over its plaintext column, so this migration adds a constraint rather than replacing a predecessor.

The reasoning behind each assertion lives in internal/datastore/migrations/npm_remote_encrypted_credentials.md, because check-comment-caps.sh caps a _test.go comment block at two lines.

Merge order

!2359 (merged) (step 6, Container's columns) merged on 2026-09-07. The plan's step 9 records Depends on: Step 6, for merge order only, and the two migrations here are stamped 20260904160000 / 20260904160100, above container's ...140000 / ...140100, so lint:migration-ordering was satisfied in that order and there is now nothing left to order against.

That dependency is now discharged. This branch is rebased on main at dbf8b383f: the one conflict was migrations_checksum_test.go, where knownHeadVersion resolves to 20260904160100, since npm's stamps sort above container's ...140100. structure.sql did not conflict, which is the case worth checking rather than trusting, because this branch's copy carried no container credential constraints while main's carries 65 and the two sets land in disjoint regions. Post-rebase it holds main's content plus this step's 586 lines with zero removals, and both credential constraint sets are present at 65 each. db:structure-check is the byte-exact gate and runs on this head for the first time.

Step Change MR
1 Bind the table and row identity into the column AAD !2349 (merged) (merged)
6 Add Container's encrypted columns !2359 (merged) (merged)
9 Add npm's encrypted columns this MR

Three open MRs also touch migrations_checksum_test.go and structure.sql: !2411 (merged), !2277 (merged) and !1011 (closed). With this branch rebased, the hazard is between the four of them rather than with main: knownHeadVersion is a single line and structure.sql a generated dump, so whichever lands second must rebase and regenerate, and no pipeline reports the overlap in advance. Do not settle it from GitLab's conflict flag — !2277 (merged)'s flipped from conflicting to clean over a few hours on 2026-09-08 with its own head unchanged, so the flag answers for the moment it was read and not for merge time. Derived on 2026-09-08 by reading the changed paths of the open MRs whose titles cover datastore, migrations or schema work, so the list is complete to that bound rather than to a scan of every open MR.

What this does not reach

Named here because each is silent, and each is checkable:

  • CountRowsForVersion still counts no table. Its body is a constant 0, and retire-key-version's pre-flight logs that count, so once a write path populates these columns the pre-flight can log 0 for a version the new FK then refuses to release. The refusal is the fail-closed direction; it is the diagnostic that under-reports. #417 (closed) owns extending it per table.
  • deleteNamespaceKeysTx has no sentinel for the new refusal. The retirement path maps 23001/23503 through isKeyStillReferencedErr; the post-shred hard delete wraps generically, so a refusal there cannot be told from a transient failure. Unreachable while no write path sets ns_key_id.
  • The credential FK has no classifier arm. S04-A has the caller key its retry on the constraint name, and the fallback renders with %v, which drops the PgError from the chain, so no caller can errors.As its way to ConstraintName. The name itself still reaches a log, inside the violation message. The write path owes an arm.

Reviewable LOC

2157 reviewable LOC (2177 total, less 20 in the two // Code generated by go-jet DO NOT EDIT. files), measured at 5a5689ec7 against merge base dbf8b383f, past the 500 the development model asks about. Splitting would not help: the migration pair, the schema dump it produces, the suite that pins it and the prose explaining that suite are one reviewable unit, and separating them would put a schema change in one MR and its only evidence in another.

LOC Group
745 tests (.go)
586 structure.sql — generated dump, 0 removals, 65 identical copies per object
506 sidecar docs (.md)
308 migrations (.sql), rationale comments included
12 production Go — one doc comment on an existing sentinel
20 jet output, generated, excluded from the count

The two largest groups are the cheapest to read. structure.sql reduces to 65 copies each of the four columns and the CHECK (parent plus 64 partitions), 65 CREATE INDEX plus 65 ATTACH PARTITION, one ON ONLY index and one ADD CONSTRAINT; judge it against the migrations rather than line by line.

Testing

test:integration runs PostgreSQL 16, 17 and 18; a local run covers 16.

Assertion Test
Exact column set, type, nullability TestNPMRemoteSchema_Columns
All 2^4 subsets: 2 whole-record arms admitted, 14 partial refused TestNPMRemoteConstraints_EncCredentialsAllOrNone
CHECK covers npm's four, names no plaintext or basic-auth column, convalidated TestNPMRemoteSchema_EncCredentialsCheckIsValidated
FK columns, referent, confdeltype, confupdtype, confmatchtype from the catalog TestNPMRemoteSchema_EncCredentialsFKShape
INSERT and UPDATE refused for an unresolvable key row, including cross-namespace TestNPMRemoteConstraints_EncCredentialsFKRejectsUnknownKeyRow
Key-row delete refused, then permitted once the unit is cleared TestNPMRemoteConstraints_EncCredentialsFKBlocksKeyRowDelete
Index clones on all 64 partitions, exact key columns TestNPMRemoteSchema_EncCredentialsIndexOnEveryPartition
CHECK and FK clones on all 64 partitions TestNPMRemoteSchema_EncCredentialsUnitReachesEveryPartition
Two-file split, Down symmetry, no NO TRANSACTION, measured row count, and the Down's one SET/RESET lock_timeout pair bracketing every drop TestNPMRemoteEncCredentials_MigrationTextSplitsTheValidate
Each pinned secondary index is present, has its key columns, is partial only where intended, and is not UNIQUE TestNPMRemoteSchema_SecondaryIndexesExist
Columns stay absent from the virtual tables TestNPMVirtualRepositoriesSchema_Columns, ..._CarriesOnlyTheIDVersionCheck

No docs/testing/ e2e scenario is added or affected: nothing reads or writes the new columns, so no request-path behaviour changes and no scenario can observe the unit. The e2e catalogs become relevant at step 10, which moves npm's reads and writes behind RowEncryptor.

Migration safety

Applied against PostgreSQL 16.15. ADD COLUMN with no default is metadata-only; the CHECK is added NOT VALID, so it binds every new write immediately and defers the scan; the FK must be added validating because PostgreSQL 16 and 17 refuse NOT VALID on a partitioned referencing table (SQLSTATE 42809).

The split into two migrations buys the transaction boundary, not operator scheduling: migrations.Up applies every pending migration in one pass and exposes no UpTo or UpByOne, and mise run db:migrate is goose up, so nothing here can run the two apart. Separate migrations are separate transactions, which is what releases the ADD's ACCESS EXCLUSIVE before the scan takes the lighter SHARE UPDATE EXCLUSIVE, and what lets a failed scan retry without redoing the ADD.

Rolling deploys are safe in both directions: all four columns are nullable, the old binary's INSERT lists its columns explicitly and leaves the unit all-NULL, which satisfies the CHECK from the moment it lands. The Down names the one condition under which reversal stops being safe — a binary that has stored credentials in these columns — and bounds each of its lock acquisitions with SET lock_timeout = '5s', because a rollback runs against a serving database and this table is read on live paths. That bound is per acquisition, not per section: three of the four dropping statements acquire across 65 relations (the parent and its 64 partitions) and the FK DROP CONSTRAINT reaches namespace_encryption_keys and its 64 partitions too, for 130, so a rollback window is sized as a multiple of 5s rather than as 5s. No statement_timeout is added, which keeps the failure mode a stall-then-rollback and keeps the section with its sibling's precedent.

Database Review Evidence

Migrations

Note

Timings are from CI (db:migrate matrix, goose verbose) against an empty database, in apply / rollback order per PG version. Production-scale validation via Database Lab is not yet available. See Database review evidence for the matrix rationale and how to read the numbers.

Migration PG 16 PG 17 PG 18
20260904160000_add_npm_remote_encrypted_credentials.sql OK (391.21ms / 173.44ms) OK (277.23ms / 168.85ms) OK (422.63ms / 172.14ms)
20260904160100_validate_npm_remote_enc_credentials_check.sql OK (14.74ms / EMPTY, 2.52ms) OK (15.24ms / EMPTY, 3.46ms) OK (17.79ms / EMPTY, 2.11ms)

Collected from pipeline #2825464906, a merged-results pipeline on 3f2d105a5, which merges this branch's head 50e22818b into main 2b6f70fce. All three matrix legs passed.

Migration notes:

  • The validating migration's rollback is reported as EMPTY on all three legs rather than as a duration, because its -- +goose Down carries comments and no statement, so the times in those cells are goose's own overhead and not work. That emptiness is deliberate, and the Down says why: PostgreSQL has no statement that returns a validated constraint to NOT VALID, and a rollback therefore leaves the constraint validated over the same predicate, which enforces the unit on new writes identically. It is a deliberate divergence from step 6's equivalent, 20260904140100_validate_container_remote_enc_credentials_check.sql in !2359 (merged), whose Down does drop the constraint and re-add it NOT VALID; npm's Down comment names that route and declines it as buying nothing back for ACCESS EXCLUSIVE on 65 relations.
  • No version-specific regression. For both migrations the slowest leg is within 1.17x of the second-slowest (adding: 422.63ms on PG 18 against 391.21ms on PG 16, 1.08x; validating: 17.79ms on PG 18 against 15.24ms on PG 17, 1.17x), against the 2x that would flag one.
  • Neither migration reaches 1s on any leg. The relevant ceiling is the 5-minute upTimeout in internal/datastore/migrations/runner.go, which caps the whole migrations.Up pass (advisory-lock wait plus every pending migration, not one migration), and these two together add at most 440.42ms to that pass, on PG 18.

What this evidence does not cover, stated because the table reads as complete and is not:

  • No query plans, because query mode does not apply. No changed hand-written, non-test Go file adds or modifies a query-producing method. The one file that reaches that filter, internal/datastore/npm_remote_repositories_errors.go, is a doc-comment edit that dispatches no statement, and the two files under internal/datastore/jet/ are regenerated DO NOT EDIT output.
  • No lock measurement. These are apply and rollback durations against an empty database. They do not measure the lock hold, nor the wait to be granted it: neither Up sets a lock_timeout, both carrying the require-timeout-settings squawk exemption under #548, so that wait is bounded in scope but not in time. The adding migration's Down does set lock_timeout = '5s', and no number here exercises it.
  • No at-scale validation. Lock-acquisition timing and row counts at production scale need Database Lab, which this project does not have; see the note above the table. The row-count claims in ## Migration safety and in the migrations' own comments rest on the 2026-09-03 observation that npm_remote_repositories held no production rows, not on anything measured here.

Related to #417 (closed)

Edited by Dzmitry (Dima) Meshcharakou

Merge request reports

Loading
Loading