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_statusto a verdict taken under the wrong authentication posture. - Boot stays unchanged and the
encryptionblock 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)