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 encryption block 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)

Edited by Dzmitry (Dima) Meshcharakou

Merge request reports

Loading
Loading