docs(specs): specify the per-format credential columns and bind AAD to the row

What

S04-A owns the DDL template and the AAD rules, S04-C owns the wiring, and both disown the per-format columns. Nothing specified what the three remote-repository tables actually get, so #417 (closed) had no design to implement. This settles the parts that are schema or AAD decisions, because each becomes a migration over live encrypted rows once the first credential row is written.

The nullability problem

S04-A marks wrapped_dek, ns_key_id and ns_key_version NOT NULL. That holds for a table whose every row carries credentials — and none of these three is. An anonymous upstream is legal and is the common case. NOT NULL would oblige every remote repository to carry a DEK, which mints a namespace key for every namespace that creates one, including namespaces that never store a credential.

So the trio is nullable here, which moves it inside the all-or-none unit. The composite FK tolerates that: it is MATCH SIMPLE, so a row with a NULL ns_key_id is not checked against the key table at all.

The unit is the whole encrypted record per table — the trio plus that table's credential attributes — because the partial states it exists to reject are exactly ciphertext with no DEK (undecryptable) and a DEK with no ciphertext (a dangling key reference that blocks retirement for nothing).

npm's constraint, which S13 was asked for and never gave

S04-a left encrypted_auth_token unconstrained and delegated it to S13's remote-repository auth model. S13 never defined it, and npm's interim table carries no constraint on tmp_plaintext_auth_token at all.

npm's unit is the trio plus encrypted_auth_token. The delegated question — whether token auth is independent of, mutually exclusive with, or required alongside basic auth — does not need answering, because no credential table declares both shapes: Maven and Container declare the pair, npm declares the token. The spec keeps the question rather than deleting it, and says a table declaring both would need its own rule and would be the point at which the auth-model question has to be settled.

The CHECK is added NOT VALID, then validated in a separate migration

Two migrations, matching the two-step pattern database-migrations.md prescribes for foreign keys. NOT VALID makes the add instant and still enforces the constraint on every subsequent write; only the validation scans.

A second statement in the same migration would not buy the shorter window. goose runs Up in one transaction with no NO TRANSACTION directive, and PostgreSQL never downgrades a lock, so the add's ACCESS EXCLUSIVE on the parent and all 64 partitions is held to COMMIT and a VALIDATE beside it runs under that lock rather than the lighter SHARE UPDATE EXCLUSIVE a standalone VALIDATE takes. 20260831103000_add_id_uuid_version_checks.sql reaches the same conclusion and carries the 65-locks-per-partitioned-table arithmetic these tables inherit.

The index build sits in that same budget. A non-concurrent CREATE INDEX takes SHARE on its own, but CONCURRENTLY is unsupported on a partitioned parent and inside these migrations the build runs under the ACCESS EXCLUSIVE already held, so it extends the exclusive window rather than adding a lock of its own. 20260814150911_add_repositories_remotes_index.sql is named as the per-partition decomposition precedent.

The measured-row-count standard the FK note states governs that scan too. The validating migration states the count it relies on, with the constraint already guarding writes from the moment the NOT VALID add lands — the one property the FK cannot have on PostgreSQL 16 and 17, which refuse NOT VALID on a partitioned table outright (SQLSTATE 42809).

Validation cannot fail on the rows present when the columns are added, since all of them are NULL and the all-NULL arm holds. That is stated as the weak claim it is: a constraint that cannot fail still reads every row, so it does not remove the need for the count.

AAD binds the row (resolves #250 / F16)

Without a row binding, database write access buys copying every encrypted_* column and wrapped_dek from one row onto another in the same namespace, and the application serves the first row's credentials from the second. That write satisfies the composite FK, trips no constraint, and fails no tag check.

Caller AAD becomes tableName || 0x00 || logicalName || 0x00 || namespaceID[16] || rowID[16] — UUIDs as raw bytes to match NamespaceKeyBinding, with 0x00 after each variable-length name. Stored sizes do not move, because AAD is authenticated and never stored.

The table is bound because row identity alone does not separate the three credential tables (review round 3). id is unique per table, not across them — pk_maven_remote_repositories and pk_container_remote_repositories are independent — and Maven and Container share the logical names username and password. Global UUIDv7 uniqueness is a property of how the application generates ids, and this threat model's attacker is defined by database write access, which is the power to ignore it. Without a table in the AAD they can give a Container row a Maven row's (id, namespace_id), copy the wrapped_dek and both ciphertexts across, point its url at a host they control, and the tag verifies. The gain is bounded and the spec says so: the same attacker can already rewrite the Maven row's own url, so this closes ciphertext movement, not write access.

The bound identity is the row's primary key, not the (namespace_id, repository_id) pair #250 proposes. Both are UUIDv7 and both are cells-safe, but repository_id is reusable in a way id is not: deleting a remote repository and creating another for the same repository yields a fresh id and the same repository_id, so a repository_id-keyed AAD would still authenticate ciphertext captured before the delete onto the new row.

Resolved now because the cost is asymmetric, in F16's own words: retrofitting is not a key re-wrap, it decrypts and re-encrypts every credential column. Before the first encrypted row exists it costs nothing.

Scope consequence a reviewer should weigh

This obliges a change to crypto.RowEncryptor, which derives AAD from the field map's key alone and receives no row identity in EncryptRow or DecryptRow. Both now take a RowRef carrying the storage table and the row's id, declared beside them in ## Interfaces. That is S04-A framework code, not per-format work, so it is not in #417 (closed)'s scope as the work item writes it. The resolution records the ordering: the interface change lands before the first encrypted write, since the identity must be bound by that write rather than added to later ones. It is cheap to do first — the interface has zero non-test consumers today.

The FK note's premise expired

S04-a said to add the FK validating "since S13 creates these tables fresh and empty". They are deployed now, so the row count is an environment fact. A migration adding the FK must state its measured count and justify its squawk suppressions on that, and must not reuse the greenfield rationale — the nearest precedent (20260618150000_add_maven_files_version_package_composite_fk.sql:1-6) justifies five suppressions on "the table is empty in dev with no production deployment", which is false now.

S13

  • Gains the fail-closed policy S04-C delegates to the format slices. The read arm is stated explicitly because falling back to an anonymous fetch is the plausible-looking failure: an upstream configured with credentials serves different content anonymously, so the fallback changes what the registry serves without changing anything a caller can see, and it would drive auth_status to a verdict taken under the wrong authentication posture.
  • Boot stays unchanged and the encryption block stays optional, because a credential-free remote repository is legal — refusing at boot would stop artifact serving for a configuration only the credential arms need.
  • No longer asserts the migration "backfills them from these plaintext columns". Whether existing plaintext values are carried across or discarded is not settled — see !2275 (merged), where it is a merge gate.

Review rounds

Rounds 1 to 3 found the same class of defect: places elsewhere in S04-A still describing the scheme this MR replaces, so a reader met two contradictory definitions of one thing. Everything was reconciled to the new scheme rather than annotated as a pre-change baseline, because there is no version of this spec where the old encoding or the old unit is the right answer.

Round Site Was
1 (4b2f66fca) per-table CHECK omitted NOT VALID and took a version-support argument where the FK above it takes a measured-row-count one
1 wire-format table listed [0x01 || "username"] as the credential AAD
1 AAD code example passed the bare column name
2 (3fa18a014) generic template CHECK covered only the basic-auth pair, four lines under the paragraph defining the unit
2 delegation paragraph still deferred encrypted_auth_token to S13 beside the section resolving it
2 write path / read path passed the bare column name
3 (46592e26e) ## Interfaces still declared the pre-binding signatures — the section an implementer copies
3 acceptance criteria no criterion for the encoding or the replay it closes; two entries named the column name alone
3 the AAD itself design fix: bound the row but not the table, so a cross-table same-namespace replay survived
3 FK note named the lock on the altered table only; it is taken on namespace_encryption_keys too
3 "validation cannot fail" unconditional, but holds only where nothing populates the columns first
3 per-table CHECK reused the plaintext constraint's name, which would strand the plaintext pair unguarded
3 S13 fail-closed table omitted the health-probe and bearer-discovery arms its own prose relies on
3 S13 discard branch left the state the fail-closed section forbids, with nothing owed to the operator
4 (976468ca) VALIDATE CONSTRAINT lock claim correction: said the validation holds SHARE UPDATE EXCLUSIVE, so splitting shortened the window; goose's single transaction means it runs under the add's ACCESS EXCLUSIVE
4 index build absent from the lock budget; it extends the exclusive window and cannot be CONCURRENTLY on a partitioned parent
4 partitioned-parent NOT VALID written as an expectation the suite would confirm; 20260831103000 already does it on 37 partitioned tables
4 (8f209eb6) ## Resolutions still answered the row-swap question as accepted, pointing at an Open Questions section this MR empties and contradicting the first resolution
4 (b6d7a6da) S13 fail-closed placement landed between the Credential management table and the paragraphs explaining it, falsifying "this table" and the #credential-management anchor
5 (6d6e5fc0) ## Security Considerations still named the column-name AAD this MR replaces, and carried no bullet for the row and table binding at all
5 (ea8c59db) Key hierarchy diagram DEK tier still read "column name as AAD", so the first picture in ## Design contradicted the encoding defined below it
5 (6de17cfe) zero RowRef round-tripped cleanly, so a caller that forgot to thread it stored permanently unbound ciphertext with nothing failing
5 (e8e14815) RowRef.Table design call: a bare string left the bound identity uncheckable until the rows are read; now a named crypto.TableName

Round 3 changed the design rather than only the prose, and two of its fixes went further than the suggestion: the per-table CHECK keeps the plaintext guard alive under a new name instead of documenting the window, and the conditional-validation fix carries the ordering requirement a carry-across migration owes.

Round 1 also corrected a claim of mine that only the validation takes a lock.

Round 2 also fixed the lint failure this MR introduced: the AAD encoding block used a text fence, which gitlab_base.CodeblockFences rejects. It is plaintext, the only form any spec under docs/specs uses.

Round 4 corrected the mirror of that round 1 claim: splitting the add from the validation does not shorten the window either, because goose runs both in one transaction and PostgreSQL never downgrades a lock. Only a separate migration does.

Round 5 closed the last two members of the set rounds 1 to 3 worked through — ## Security Considerations and the key hierarchy diagram were the sites that were never opened — and then went past prose twice. The zero RowRef and the untyped RowRef.Table were both silent failures already baked into stored ciphertext by the time anyone could notice, so both are now contracts in ## Interfaces rather than notes. RowRef.Table is a named crypto.TableName whose values are declared beside the datastore's table bindings, which keeps internal/crypto free of datastore table names; the spec states that RowRef does not validate against an enumeration, so the constant's single declaration site is the guard.

Size

Twelve commits, all prose, DDL and one table, on two files. Not split further because the nullability rule, the unit, the per-table DDL and the AAD binding are one decision each way: the unit only makes sense once the trio is nullable, and the AAD's bound identity is what the per-table primary keys settle.

Related to #417 (closed)

Edited by David Fernandez

Merge request reports

Loading
Loading