feat(datastore): move the Maven credential store behind the row encryptor (S04-A per-format columns plan: 4/11)
Step 4 of the S04-A per-format credential-column plan, split in two so neither half is reviewed at the 5192 LOC the two halves carry together at their current heads (2fc08349 and 54ffc3e1, against merge base 5e8a6449). This is part 1.
The stack, in merge order
| MR | What it lands | |
|---|---|---|
| 1 | !2554 (merged) | The create, update and details paths, the leftover-plaintext boot report, and the removal of part 1's interim read fallback |
| 2 | this MR | The RemoteCredentialStore read and write behind RowEncryptor, the retired-key rejection, and the health sweep's refusal verdict |
Part 2 targets this branch, so !2554 (merged) merges into this branch first and this
MR then carries both halves into main. A merge request dependency on !2554 (merged)
enforces that order: a stack can merge out of order, and nothing else would
stop part 1 reaching main without part 2.
What this does
RemoteCredentialStore reads and writes the encrypted unit (wrapped_dek,
ns_key_id, ns_key_version, encrypted_username, encrypted_password)
instead of the interim plaintext pair, and its constructor takes the encryption
stack. A nil client still panics as a wiring bug; a nil encryptor does not,
because that is a deployment with no encryption block, and the credential
arms refuse rather than boot failing.
Every write arm on RemoteCredentialStore nulls the interim pair alongside
the unit, so a clear or a URL change through the store leaves no secret on
the row. That covers the store's own write arms, not the live create and
update path, which part 2 moves.
While !2554 (merged) is open, no production path may call the store's write arms, so
mavenRemoteHasCredentialsProjection
(internal/datastore/maven_remote_repositories_details.go:267-271) keeps
deriving has_credentials from tmp_plaintext_*, and the credential
assignments in mavenRemoteUpdateAssignments
(internal/datastore/maven_remote_repositories_update.go:388-400) keep
setting and clearing only that pair. Once !2554 (merged) merges, both move to the
encrypted unit. Line numbers are at a001e596.
The two halves ship in one release because part 2 deletes the interim read
fallback. Every credential written while part 1 runs without part 2 lands in
tmp_plaintext_* with no unit beside it, and part 2's reader then answers
anonymous for those rows. One release bounds that discard to rows predating
part 1; two releases widen it to every credential write in between. Part 1
stays correct on its own while it runs, which is what the fallback buys, so
the requirement is about the size of the discard rather than about part 1
breaking. See "The stack, in merge order" for the order that requires.
A write stamped from a namespace key retired in between is refused with
ErrMavenRemoteStaleNamespaceKey and a warn line carrying the constraint, the
namespace and the stale key id. It is not retried in place: the rejection has
already aborted the caller's transaction, and the key manager would serve the
same cached key. maven_remote_repositories_errors.md records that, and what
the health sweep does and does not book.
The health sweep books a policy-refused resolution as a failing probe instead of skipping the remote, so a repository that cannot serve stops reading healthy. It reaches an absent encryption stack and the permanent crypto verdicts; the retryable and transient ones stay skips, so a blip cannot flip a healthy row.
The interim read fallback
The read falls back to tmp_plaintext_* for a row carrying no unit. This is
what makes part 1 safe on its own: CreateMavenRemoteRepository and
UpdateMavenRemoteRepository still write the interim pair until part 2, so
without the fallback every credentialed Maven remote would pull anonymously
the moment this merges.
Part 2 deletes it. A row still holding only the interim pair then reads anonymous, which is the discard !2542 (merged) records.
Size
2831 reviewable LOC across 29 files, measured at 2fc083494 against merge
base 5e8a6449a: 1921 test Go (17 files), 711 production Go (10 files), 199
Markdown (2 files). Added plus removed, per
.claude/skills/review-branch/SKILL.md; nothing in the diff matches its
exclusion patterns, so the filtered and raw counts agree.
Past the 500-LOC guideline, and this is the larger of the two halves by LOC (2831 against part 2's 2653 at 54ffc3e1), the smaller by file count (29 against 58). The credential read and write cannot be separated further: any boundary between them leaves a window where a credentialed Maven remote silently loses its credentials. Two thirds of the diff is tests.
Before merge
- !2542 (merged) merges first. Five merged spec lines still prescribe a backfill this plan discards.
- #513 clears. Merging this step deploys it, and no environment carries
the
encryptionblock yet.
Testing
go test ./... green; integration suites compile and run under
-tags=integration; golangci-lint clean for the changed files under both tag
sets; comment-caps gate green.
No e2e scenario is added or affected: docs/testing/ carries no
upstream-credential scenario, and this change alters no client-facing route.
Related to #417 (closed)