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

What this does

Adds the encrypted credential unit to maven_remote_repositories: the ciphertext pair encrypted_username / encrypted_password, the wrapped_dek that opens them, and the ns_key_id / ns_key_version reference to the namespace key that wrapped the DEK. All five are nullable and all-or-none, held by check_maven_remote_repositories_enc_credentials_all_or_none.

Step 2 of the per-format credential columns plan. Schema only. Nothing writes these columns yet: the create statement and the credential update both still name the interim tmp_plaintext_* pair, which stays the live store until Step 4 moves reads and writes behind crypto.RowEncryptor. The two credential shapes coexist on the table on purpose, each with its own constraint over its own columns.

Two migrations rather than one. The add takes ACCESS EXCLUSIVE and declares the CHECK NOT VALID; the scan ships separately so it runs under SHARE UPDATE EXCLUSIVE instead of inheriting the heavier lock. The foreign key is added validating, because PostgreSQL 16 and 17 refuse ADD FOREIGN KEY ... NOT VALID on a partitioned table with SQLSTATE 42809.

The add migration's Down bounds its own lock waits with SET lock_timeout = '5s' / RESET lock_timeout, because its lock set is wider than the Up's: dropping ns_key_id drops the foreign key with it, and dropping a foreign key takes ACCESS EXCLUSIVE on the referenced side too, so namespace_encryption_keys and its 64 partitions where the Up took only SHARE ROW EXCLUSIVE. The npm sibling bounds the identical shape. the_add_down_bounds_its_lock_waits reuses npm's own assertDownBracketsItsDropsWithTheLockBound, parameterized on the drop count, so the pair cannot be removed with the suite green.

Migration safety, and the merge order this needs

The migration versions were renumbered, and that is the load-bearing part of this MR. They were originally 20260904131400 and 20260904131500, both below 20260904140100, which Step 6 landed on main while this branch sat on an older base. internal/datastore/migrations/migrations.go sets goose.WithAllowOutofOrder(false), so on any database that has already applied Step 6's pair, Up collects these two as missing out-of-order migrations and returns an error instead of applying anything. Runner.Start calls Up at pod startup and Runner.Check stays pending on failure, so that is a readiness failure on every new pod, not a slow deploy.

No test could have caught it. Every suite migrates from zero, where version order is self-consistent, and scripts/ci/check-migration-immutability.sh compares against the merge base with --diff-filter=MD, so it reads in-place edits and never version ordering. Its own header states the same limitation for the defect it does cover. What does catch it is the lint:migration-ordering CI job in .gitlab-ci.yml: it reads the target branch's latest version when the job runs, not when the pipeline was created, and fails a new version that does not sort strictly after it (an -le compare). That is where the failure shows up for the next step author, and it only shows on a pipeline created after the sibling merged.

They are now 20260904170000 and 20260904170100, renumbered a second time in 3ab22549d: !2397 (merged) (Step 9, npm) merged on 2026-09-08 at 09:41 UTC carrying 20260904160000 and 20260904160100, so this pair moved to sort strictly after main's head 20260904160100. knownHeadVersion in migrations_checksum_test.go follows to 20260904170100; it is the only hand-edited constant, because referenceChecksum recomputes from the embedded FS.

Merge order: discharged. There was no order-independent choice between this MR and !2397 (merged), because whichever of the two merges second must hold the higher versions. !2397 (merged) merged first, so this branch took the higher pair, and the constraint is settled rather than pending. The same renumber is owed a third time if another migration lands on main above 20260904160100 before this merges; lint:migration-ordering on the next pipeline is where that shows.

The plan's Step 2 Shared seams paragraph currently says "any order is fine", which is what this MR disproves. Correcting it needs its own docs(plans) MR, because a step MR does not edit the plan.

Both migrations rest on a dated row count: measured 2026-09-03, .com carries no traffic and staging carries test traffic only, so the foreign key's scan and the VALIDATE read no production row and the non-concurrent index build blocks nothing. The lock window is real and documented in the migration headers: ACCESS EXCLUSIVE on 130 relations, plus SHARE ROW EXCLUSIVE on namespace_encryption_keys and its 64 partitions, which blocks every format's first credential write, rotation and shred for the length of the scan.

Test coverage

Ten tests cover the unit. Three of them close gaps found by diffing this suite against the mirrored container and npm suites rather than reading it alone:

• TestMavenRemoteConstraints_CredentialUnitsAreIndependent. No test bound both credential units in one row, so nothing could see a constraint that had grown to span both. Adding AND tmp_plaintext_username IS NULL to the encrypted unit's all-bound arm keeps every other assertion in the file green and forbids exactly the row Steps 3 and 4 have to write, where ciphertext lands while the plaintext pair is still the live store.
• TestMavenRemoteConstraints_EncryptedUnitBindsUpdates. Every case was an INSERT, while namespace key rotation only ever UPDATEs the unit. Adds the partial-unit, unresolvable-key and clear-in-place arms. The third is the management API's credential removal, which the previous suite proved only by deleting the whole row.
• TestMavenRemoteRepositoriesSchema_EncCredentialsUnitReachesEveryPartition. The CHECK and the FK were read on the parent only, though the index already had a per-partition clone assertion, so a decomposition that missed one relation passed the whole suite. confupdtype was unasserted too, so an ON UPDATE CASCADE on ns_key_id would have shipped green.

The validate migration's Down is deliberately empty, and that diverges from the landed container sibling 20260904140100_validate_container_remote_enc_credentials_check.sql, which drops the constraint and re-adds it NOT VALID. Drop and re-add takes ACCESS EXCLUSIVE on 65 relations to reinstate a weaker marker over a scan already paid for, and rolling this migration back alone leaves the guard validated, which is stronger than the NOT VALID state container rolls back to rather than weaker. the_validate_down_is_empty asserts the section, so the divergence is pinned rather than only argued. The npm pair, landed on main as 20260904160100_validate_npm_remote_enc_credentials_check.sql, takes the same position for its table: its Down is empty on the same reasoning and its own comment says so.

Verification

Measured at 89e498185, the head after the second renumber. The database claims are read off the head pipeline #2829205555, the merged-result pipeline of that head onto main a990b3573; the rest were run locally on this worktree at the same commit.

Re-run locally at f3aa8ccab: TestMigrations_UpDownUp, the Maven and npm migration-shape suites, squawk 2.63.0 on both migrations (0 issues), and scripts/ci/check-comment-caps.sh --base origin/main (clean). The rest of the list below stays pinned to 89e498185.

The head is now 23d1e4760, after a second round of review fixes. The delta from 89e498185 is comments in the two migrations, the Down lock bound and its subtest, ten lines on the npm shape suite, three sidecar documents, and this round's four commits. Three of those are test-side: the plaintext CHECK's catalog assertion widened from one encrypted column to all five, a subtest pinning both enc migrations inside goose's transaction, and two seed comments corrected and compressed to the two-line test-file cap. The fourth changes the add migration's Up, and is described under Database Review Evidence.

Re-run at 23d1e4760: the Maven and npm migration-shape suites, the repo's pinned squawk and pgFormatter pre-commit hooks on the add migration (both pass), golangci-lint 2.13.2 with --build-tags=integration on internal/datastore/migrations with the caps off (no finding lands on a line this round changed), and scripts/ci/check-comment-caps.sh --base origin/main (clean). TestMigrations_UpDownUp was not re-run locally at this head: the local Docker VM ran out of disk part way through, after the Up chain had applied and during the rollback to zero. The head pipeline's test:integration:migrations legs are what cover it.

• Full chain applies in order on the db:migrate matrix, PostgreSQL 16, 17 and 18: Step 6's 140000 / 140100, Step 9's 160000 / 160100, then this MR's 170000 / 170100.
• db:structure-check green, so structure.sql regenerates byte-identical and the renumber changes no schema.
• lint:migration-ordering and lint:migration-immutability green on that pipeline; scripts/ci/check-migration-immutability.sh origin/main also clean locally.
• test:integration:migrations green on 16, 17 and 18; test:integration and the test:integration:datastore matrix green as well.
• TestHeadVersion green locally with knownHeadVersion at 20260904170100.
• squawk 2.63.0: 0 issues as shipped. All 9 squawk-ignore-file directives measured live by stripping them, which reports 16.
• golangci-lint 2.13.2 with --build-tags=integration, the caps off and --new-from-rev=origin/main: 0 issues. All 22 //nolint tokens on the diff's added lines measured firing: stripping every directive at once reports each token's linter on its own line, or inside its own function for the two function-level wrapcheck tokens.
• scripts/ci/check-comment-caps.sh --base origin/main: clean.

Version-specific claims in the migration comments were checked against live PostgreSQL 16.15 and 18.6 rather than from memory: FK NOT VALID on a partitioned table fails with 42809 on 16.15 and is accepted on 18.6; a CHECK NOT VALID lands one copy per partition and one parent VALIDATE clears all 65; the reversed FK column pairing is silently accepted; and the delete arm reports 23503 on 16 against 23001 on 18, with an unstable clone name, which is why that assertion uses TableName.

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. The re-apply phase those jobs also run is idempotency verification only, so it is left out of the table.

Migration	PG 16	PG 17	PG 18
20260904170000_add_maven_remote_encrypted_credentials.sql	OK (567ms / 184.41ms)	OK (545.23ms / 210.96ms)	OK (374.38ms / 188.96ms)
20260904170100_validate_maven_remote_enc_credentials_check.sql	OK (10.57ms / 2.71ms, empty Down)	OK (13.24ms / 2.68ms, empty Down)	OK (17.76ms / 2.16ms, empty Down)

Collected from pipeline #2829107935, a merged-results pipeline on d34db65db, which merges this branch's head 3ab22549d into main a990b3573. All three matrix legs passed. It is the first db:migrate matrix to run on the renumbered 170000 / 170100 versions, which is what the earlier local numbers could not cover. The head has since moved to 23d1e4760, and several of the commits since do touch internal/datastore/migrations/sql/. The validating migration's statements are unchanged in both directions.

The add migration's Up is no longer statement-identical to the one timed. 23d1e4760 drops IF NOT EXISTS from its five ADD COLUMN clauses and from its CREATE INDEX, so those six statements now fail on a pre-existing object instead of skipping it, matching both landed siblings (20260904140000_add_container_remote_encrypted_credentials.sql and 20260904160000_add_npm_remote_encrypted_credentials.sql) and the reason container's own prefer-robust-stmts directive gives for the bare form: inside goose's transaction the guard recovers nothing, and it would turn a pre-existing object of the wrong shape into a silent no-op. Against the empty database these figures were taken on, no object pre-exists and the guard was never taken, so the apply column stands on the same work; the numbers are still from the guarded spelling. The add migration's Down also gained SET lock_timeout = '5s' and RESET lock_timeout, so its rollback figure predates that pair.

Migration notes:

• Nothing here is near the Up budget. The slowest single apply is the adding migration's 567ms on PG 16, and upTimeout (internal/datastore/migrations/runner.go:28) caps the whole pending set at five minutes.
• No version-specific regression. For each migration and each direction the slowest leg is within 1.35x of the second-slowest (the widest is the validating migration's apply, 17.76ms on 18 against 13.24ms on 17), against the 2x that would flag one.
• The rollback column is where this MR parts company with its siblings, and it is the number the empty Down argument turns on. Goose reports that Down EMPTY, at 2.16ms to 2.71ms, which is the no-op itself rather than any work. The container sibling's validating migration, whose Down drops the constraint and re-adds it NOT VALID, rolled back at 48.11ms / 51.61ms / 48.07ms on these same three legs. That is the work this file's Down declines to repeat.

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

• No lock measurement, and the rollback comparison above is not one. These are durations against an empty database, so those milliseconds are statement overhead rather than the cost the Down argument is about. That cost is the ACCESS EXCLUSIVE the container shape's two ALTER TABLEs take on the parent and all 64 partitions, 65 relations, which is a count off the DDL rather than a measurement. Nothing in this table says what holding that lock costs under load, nor how long the wait to be granted it would run. Neither Up sets a lock_timeout, per #548. The add migration's Down does set one, and no number here measures what that bound costs or saves.
• No production row counts. The scan the validating migration runs reads zero rows here, and per the row count both migrations date to 2026-09-03, zero rows on .com as well. A populated table would move the apply column and nothing here predicts by how much.
• 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 two internal/datastore/jet/ files in the diff are regenerated go-jet output carrying a Code generated header, and neither contains a .QueryContext(, .QueryRowContext( or .ExecContext( call, so there is no new or changed statement to EXPLAIN.

Diff size

2417 reviewable LOC at f3aa8ccab (git diff --numstat origin/main...HEAD, added plus deleted lines), past the 500 the development model asks about, so here is the split by file group:

Group	LOC
maven_remote_repositories_schema_integration_test.go, plus a one-line dupl token on the container sibling and ten lines on the npm one	991
structure.sql, regenerated dump	651
maven_remote_encrypted_credentials.md, sidecar reasoning	487
The two migrations	262
namespace_encryption_keys_rotation.md and namespace_encryption_keys_schema_notes.md	24
migrations_checksum_test.go, one constant	2
go-jet regenerated output, excluded from the count above	24

Splitting would not help. The dump is machine-generated from the migrations in the same change and cannot land separately without leaving db:structure-check red. The test file and the sidecar are the same unit of review as the migrations, and the two-line comment cap on _test.go files is why the reasoning sits in the sidecar rather than beside the assertions. The two bulk items, the dump and the test, are 1642 of the 2417 between them. The ten lines on the npm shape suite are the drop-count parameter on the helper this suite now shares with it, rather than a twin of it.

Scenario catalogs

No e2e scenario is added or affected. This change adds columns and constraints that no route reaches: no production query names them, MavenRemoteRepositories.AllColumns and MutableColumns have no non-test consumer, and the write paths still name only the interim plaintext pair. The first user-visible behaviour arrives with Step 4, which is where the scenario work belongs. Step 6 took the same position for its table.

ADR conformance

Conforms to ADR-007 as amended upstream by handbook commit 417f7b765, "Artifact Registry ADR 007: publish the credential key-wrap columns" (2026-09-04), which publishes the key-wrap trio, the (namespace_id, ns_key_id) index and the composite FK with ON DELETE RESTRICT. The amendment is the gate the plan sets on this step, and it has cleared.

The mirror has since caught up, so docs/adr/ is fine to read for this MR. 8fa841cf1 synced the amendment down and is an ancestor of this branch, and docs/adr/007_database_schema.md in this tree lists wrapped_dek, ns_key_id and ns_key_version in the maven_remote_repositories block beside encrypted_username and encrypted_password, with the (namespace_id, ns_key_id) index described under Indexes. ./scripts/adr-freshness.sh, run on this branch at 89e498185 on 2026-09-08, reports the mirror current: no upstream decision commits since the last local change.

Residuals this change does not reach

• Two failure arms the new FK opens have no mapper yet. On the insert side classifyMavenRemoteRepositoryWriteError matches only the repository FK, so a 23503 naming the key reference falls through to a generic write error. On the delete side deleteNamespaceKeysTx wraps the refusal without a sentinel. Both are unreachable today because nothing writes ns_key_id, and both belong to #417 (closed)'s wiring steps. Named in the sidecar.
• docs/specs/S14-maven-remote.md calls itself the authoritative source for these tables and its column table lists none of the five new columns. The plan's Step 4 amendment list names S16, S04-b, S04-c and docs/dev/column-encryption.md, not S14, so nobody owns it. Flagged rather than fixed here, since adding it to that list is a plan edit.
• One inert //nolint:gosec token stays on mavenRemoteCoreMigrationTokens. Measured dead two ways: stripping it changes nothing, and G101 provably cannot read a string out of a slice literal. It is left because removing the line puts its nine-line doc comment, which is about the Down-test token slice and not about credentials, under the two-line cap and forces a rewrite this change has no reason to make. Nothing tracks it: #682 is scoped to thelper, so it does not cover this one. Happy to drop it here instead if a reviewer would rather take the comment rewrite.

Related to #417 (closed)