<!-- task=m-principal-provider-credential-slot-routing/07+01,02,05_secret_material plan=1 tag=API milestone-task=secret-at-rest -->

# Code Review Reference - API

> **[IMPLEMENTING AGENT — READ FIRST] Filling in this file is the mandatory final step of implementation.**
> The task is NOT complete until every implementation-owned section below is filled in.
> Complete the `Implementation Checklist`; the final checklist item is mandatory before saving.
> Fill implementation-owned sections, then stop with active files in place and report ready for review.
> If implementation is blocked, record the exact blocker, attempted commands/output, and resume condition only in implementation-owned evidence fields.
> Do not ask the user directly, present choices, call user-input tools, create control-plane stop files, or classify the next state.
> Finalization (`Code Review Result`, log rename, `complete.log`, archive moves, `Review-Only Checklist`) is review-agent-only, even after compaction/resume.
> Follow the ownership table at the bottom of this file for which sections you own.

## Overview

date=2026-08-01
task=m-principal-provider-credential-slot-routing/07+01,02,05_secret_material, plan=1, tag=API

## Roadmap Targets

- Milestone: `agent-roadmap/phase/operational-observability-provider-management/milestones/principal-provider-credential-slot-routing.md`
- Milestone link: [Milestone 문서](agent-roadmap/phase/operational-observability-provider-management/milestones/principal-provider-credential-slot-routing.md)
- Task ids:
  - `secret-at-rest`: provider raw token envelope encryption과 외부 key source
- Completion mode: check-on-pass

## Archive Evidence Snapshot

- `agent-task/archive/2026/08/m-principal-provider-credential-slot-routing/01_principal_store/complete.log`: principal/token durable ledger와 restart persistence PASS.
- `agent-task/archive/2026/08/m-principal-provider-credential-slot-routing/02+01_credential_catalog/complete.log`: slot/revision/envelope metadata와 lifecycle PASS.
- `agent-task/archive/2026/08/m-principal-provider-credential-slot-routing/05+01,02_management_core/complete.log`: principal-scoped management core, zeroization과 secret-blind failure surface PASS.
- prior unimplemented pair: `plan_cloud_G08_0.log`, `code_review_cloud_G09_0.log`.

## For the Review Agent

> **[REVIEW AGENT ONLY]** The finalization steps below are review-agent only. Implementing agents must not execute this section.

Compare implementation of each item against source files and verify that output in `Verification Results` matches code.
Review completion means the following steps are finished:

1. Append verdict and `review_rework_count` / `evidence_integrity_failure` routing signals.
2. Archive `CODE_REVIEW-cloud-G09.md` → `code_review_cloud_G09_1.log` and `PLAN-cloud-G08.md` → `plan_cloud_G08_1.log`.
3. If PASS, write `complete.log` and move active task directory to `agent-task/archive/YYYY/MM/m-principal-provider-credential-slot-routing/07+01,02,05_secret_material/`. If WARN/FAIL, fully write the next filesystem state required by the code-review skill.
4. If PASS, report milestone completion event metadata; roadmap mutation is runtime-owned.
5. Check applicable `Review-Only Checklist` items at the final `.log` location.

---

## Implementation Item Completion

| Item | Status |
|------|---------|
| API-1 Versioned authenticated keyring without layer inversion | [x] |
| API-2 Slot-bound seal and durable revision inspection | [x] |
| API-3 Production server/store/service composition | [x] |

## Implementation Checklist

- [x] Implement a versioned AES-256-GCM keyring with a lower-layer context type, canonical AAD, external mount loading, decrypt support, and cleanup.
- [x] Allocate a stable slot id before sealing and preserve transactional create/rotate CAS and failure-no-mutation behavior.
- [x] Wire one keyring instance into Control Plane server/store/service composition with fail-closed startup while keeping principal bootstrap store-only.
- [x] Add fresh crypto, key-version, restart, DB inspection, permissions, and secret-leak tests.
- [x] Fill implementation-owned sections in CODE_REVIEW-*-G??.md with actual implementation notes and verification output.

## Review-Only Checklist

> **[REVIEW AGENT ONLY]** This checklist is used only by the review agent.
> Implementing agents must not modify or check this section.

- [x] Append one verdict of `PASS`, `WARN`, or `FAIL` and verified `review_rework_count`, `evidence_integrity_failure` to `Code Review Result`.
- [x] Verify that verdict, `Dimension Assessment`, and Required/Suggested/Nit classifications match.
- [x] Archive active `CODE_REVIEW-*-G??.md` to `code_review_cloud_G09_1.log`.
- [x] Archive active `PLAN-*-G??.md` to `plan_cloud_G08_1.log`.
- [x] Verify that the Agent-Ops managed block in `.gitignore` unignores `agent-task/**/*.md` and `agent-task/**/*.log` and ignores `agent-roadmap/current.md`.
- [ ] If PASS, write `complete.log` and leave no active `.md` files.
- [ ] If PASS, move the active task directory to the dated archive and update this checklist at the final archive path.
- [ ] If PASS, report milestone completion event metadata without modifying roadmap.
- [ ] If PASS for split work, preserve the active parent because later siblings remain.
- [x] If WARN/FAIL, write the next filesystem state matching the verdict and do not write `complete.log`.

## Deviations from Plan

- **Envelope nonce validation is a minimum, not an exact length.** `credentialstore.SecretEnvelope.Validate` now requires the nonce to be at least the 96-bit GCM standard (`>= 12` bytes) rather than exactly 12. The production `credentialseal` keyring always emits 12-byte nonces, but pre-existing `credentialops`/`credentialstore` fake sealers use 16-byte nonces; a hard `== 12` rule would have broken unrelated existing tests without any security benefit, so short/weak nonces are rejected while longer opaque nonces stay valid. AAD stays optional (nullable) at the store boundary for the same compatibility reason (legacy/migration rows persist `aad NULL`).
- **`Open` double-checks the AAD instead of trusting the stored field.** Decryption recomputes the canonical AAD from the caller-supplied `Context` and rejects any mismatch with the stored `env.AAD` before calling GCM. This makes both a swapped context and a tampered stored-AAD field fail with the constant `ErrOpenFailed`, which the plan's "wrong AAD/ciphertext/nonce tamper" strategy expects; the stored AAD is never the source of authentication truth.
- **`credentialseal.FileConfig` carries the YAML tags.** Rather than a separate config struct in `main` plus a conversion shim, the crypto package owns `FileConfig` (plain struct tags, no yaml import) so `controlPlaneConfig.CredentialEncryption` embeds it and `composeCredentialRuntime` calls `credentialseal.LoadFile(cfg.CredentialEncryption)` exactly as the plan wrote it.
- **The credential service is composed but not yet attached to a listener.** Per the plan ("expose the service only to later secure handlers"), `composeCredentialRuntime` constructs `credentialops.Service` into a `credentialRuntime` struct and surfaces its presence through a `secret_encryption` boolean log field; no wire handler consumes it in this packet (08+ owns activation).
- **Environment note (not a code deviation).** The sandbox's default `GOCACHE` is root-owned and `/tmp` is `noexec`, so every `go` command was run uncached with `GOCACHE=/tmp/gocache-claude GOTMPDIR=/config/tmp` and `-count=1`. Results below are from that fresh, uncached run.

## Key Design Decisions

- **No layer inversion.** `credentialseal` depends only on the data-only `credentialstore.SecretEnvelope`; `credentialstore` declares its own local `algorithmAES256GCM` constant so it never imports `credentialseal`. Verified graph: `credentialops -> credentialseal -> credentialstore`, and `credentialseal` does not import `credentialops`.
- **Canonical, domain-separated AAD.** `canonicalAAD` writes a fixed `iop.credentialseal.v1` domain prefix plus length-delimited principal/slot/kind fields (8-byte big-endian length + bytes each), so no two distinct contexts can collide (`"ab"+"c"` ≠ `"a"+"bc"`).
- **Secret-manager-safe file load.** `readSecureManifestFile` resolves atomic symlinks once via `EvalSymlinks`, opens the resolved path, and validates through the returned file descriptor's `Stat` (regular file, owner is current uid or root, no group/world permission bits) before reading. Kubernetes-style `..data` symlink rotation is supported by reloading; dangling/non-regular/insecure/missing targets are rejected with typed errors.
- **Keyring is both `Sealer` and `EnvelopeKeyRegistry`.** One instance seals with the configured active key, opens any held revision, and answers `HasEnvelopeKey` for the store. Decoded base64 key material is zeroed immediately after the AES key schedule is built, decoded manifest bytes are zeroed after parse, and `Seal`/`Open` return only the constant `ErrSealFailed`/`ErrOpenFailed`.
- **Create-time AAD binds to a stable slot id.** `credentialstore.CreateSlotInput.SlotID` is an additive optional validated-UUID field (legacy zero value keeps store generation); `credentialops.Service.CreateSlot` generates the UUID before sealing so the create envelope's AAD binds to the exact row that persists it. Rotation continues to read principal/slot/kind from the current row. Both paths validate/seal before the store transaction, so a failed crypto/validation step advances no rows.
- **Fail-closed composition.** When the encryption triple is fully omitted, no keyring is built, no sealer is injected, and provider-secret mutations still return `ErrSealerUnavailable`; any partial triple or invalid manifest fails inside `composeCredentialRuntime` before a listener starts. First-principal bootstrap is unchanged and remains store-only.

## Reviewer Checkpoints

- Confirm `credentialseal` does not import `credentialops` and no import cycle/layer inversion was introduced.
- Confirm the external key file safely supports secret-manager atomic symlinks, validates the resolved regular target/permissions, is never tracked, and loads before listeners.
- Confirm the server injects one keyring into store and `credentialops.Service`, while first-principal bootstrap remains store-only.
- Confirm database-only/all-fields-omitted startup stays compatible and secret mutations fail closed; any partial encryption triple fails before listeners.
- Confirm create-time AAD uses the persisted slot ID and rotation cannot mutate rows on seal/open failure.
- Confirm DB, logs, errors, review evidence and config contain no raw key or provider secret.

## Verification Results

### Environment and worktree

Command:

```bash
command -v go && go version && go env GOROOT
git status --short --branch
```

Output and exit status:

```
$ command -v go && go version && go env GOROOT
/config/.local/bin/go
go version go1.26.2 linux/arm64
/config/opt/go

$ git status --short --branch
## agent/dispatcher-work-log-artifact-loop...origin/agent/dispatcher-work-log-artifact-loop
 M agent-contract/index.md
 M agent-contract/inner/control-plane-edge-wire.md
 ... (pre-existing unrelated working-tree changes from prior packets, not modified by this task)
```

exit status: 0. Note: the `credentialstore`, `credentialops`, `credentialseal` packages and `credential_store_test.go` are untracked working-tree state carried from predecessor packets (01/02/05), so this task's edits to them are part of that untracked tree; `main.go`, `server.go`, `config_test.go`, and `configs/control-plane.yaml` are tracked modifications. Go commands were run with `GOCACHE=/tmp/gocache-claude GOTMPDIR=/config/tmp` because the default cache is root-owned and `/tmp` is `noexec`.

### Targeted and full Control Plane tests

Command:

```bash
go test -count=1 ./apps/control-plane/internal/credentialseal ./apps/control-plane/internal/credentialstore ./apps/control-plane/internal/credentialops
go test -count=1 ./apps/control-plane/cmd/control-plane
go test -count=1 ./apps/control-plane/...
```

Output and exit status:

```
$ go test -count=1 ./apps/control-plane/internal/credentialseal ./apps/control-plane/internal/credentialstore ./apps/control-plane/internal/credentialops
ok  	iop/apps/control-plane/internal/credentialseal	0.188s
ok  	iop/apps/control-plane/internal/credentialstore	2.798s
ok  	iop/apps/control-plane/internal/credentialops	0.265s
(exit 0)

$ go test -count=1 ./apps/control-plane/cmd/control-plane
ok  	iop/apps/control-plane/cmd/control-plane	3.820s
(exit 0)

$ go test -count=1 ./apps/control-plane/...
ok  	iop/apps/control-plane/cmd/control-plane	4.703s
ok  	iop/apps/control-plane/internal/credentialops	0.415s
ok  	iop/apps/control-plane/internal/credentialseal	0.246s
ok  	iop/apps/control-plane/internal/credentialstore	3.574s
ok  	iop/apps/control-plane/internal/wire	1.522s
(exit 0)
```

exit status: 0 for all three commands. Plan verification blocks for API-1 (`credentialseal`+`credentialstore`+`credentialops`), API-2 (`credentialstore`+`credentialops`), and API-3 (`cmd/control-plane`) are all covered by the above.

### Vet, diff and leak guard

Command:

```bash
go vet ./apps/control-plane/...
git diff --check
credential_leak_pattern='BEGIN'' AGE PRIVATE KEY|active_key_value[[:space:]]*:[[:space:]]*[^<$]'
if rg --sort path -n --hidden --glob '!agent-task/archive/**' --glob '!*.sops.yaml' --glob '!PLAN-*.md' --glob '!*.log' "$credential_leak_pattern" apps/control-plane configs agent-task/m-principal-provider-credential-slot-routing/07+01,02,05_secret_material; then exit 1; fi
```

Output and exit status:

```
$ go vet ./apps/control-plane/...
(no output; exit 0)

$ git diff --check
(no output; exit 0)

$ credential_leak_pattern='BEGIN'' AGE PRIVATE KEY|active_key_value[[:space:]]*:[[:space:]]*[^<$]'
$ if rg --sort path -n --hidden --glob '!agent-task/archive/**' --glob '!*.sops.yaml' --glob '!PLAN-*.md' --glob '!*.log' "$credential_leak_pattern" apps/control-plane configs agent-task/m-principal-provider-credential-slot-routing/07+01,02,05_secret_material; then exit 1; fi
(rg emitted no match, exit 1 -> guard did not trip; overall guarded block exit 0)
```

Guard result: no match. Supplementary layering evidence (`go list`): `credentialseal` imports `credentialstore` but not `credentialops`; `credentialops` imports both — no import cycle or layer inversion. The tracked `configs/control-plane.yaml` ships only empty `credential_encryption` placeholders (`key_file: ""`, `active_key_id: ""`, `active_key_version: 0`); no key bytes, plaintext, or ciphertext appear anywhere in the scanned trees.

---

> **[IMPLEMENTING AGENT — BEFORE SAVING] Have you filled in every implementation-owned section?**
> If anything is blank, go back and fill it in before saving this file.
> Leave review-agent-only sections unchanged.

## Section Ownership

| Section | Owner | Note |
|---------|-------|------|
| Header comment, Overview, Review Agent Instructions | Fixed at stub creation | Implementing agent must not modify or execute these |
| Roadmap Targets | Fixed at stub creation from plan | Implementing agent must not modify |
| Archive Evidence Snapshot | Fixed at stub creation from plan | Read only cited files when more detail is needed |
| Implementation Item Completion (item names) | Fixed at stub creation | Implementing agent checks `[ ]` → `[x]` only |
| Implementation Checklist (item text/order) | Fixed at stub creation from plan | Implementing agent checks `[ ]` → `[x]` only |
| Review-Only Checklist | Review agent only | Implementing agent must not modify or check |
| Deviations from Plan, Key Design Decisions | Implementing agent | Replace placeholder text with actual content |
| Reviewer Checkpoints | Fixed at stub creation | Pre-filled from plan |
| Verification Results | Fixed headings/commands | Implementing agent fills actual output only |
| Code Review Result | Review agent appends | Not included in stub |

## Code Review Result

- **Overall Verdict:** FAIL
- **Dimension Assessment:**
  - Correctness: Fail — create-time AAD can use a credential kind different from the normalized kind persisted in the slot row, making the initial envelope undecryptable from its durable context.
  - Completeness: Fail — a complete encryption triple with no database silently starts without the required store/service composition.
  - Test coverage: Fail — the accepted normalized-kind variants and encryption-without-database boundary are not covered.
  - API contract: Fail — the persisted principal/slot/kind context does not always authenticate the envelope, and enabled encryption does not always compose one keyring into both consumers.
  - Code quality: Pass — layering, redaction, and cleanup are otherwise coherent; stale sealer comments and an unused test sentinel were repaired during review without changing behavior.
  - Implementation deviation: Fail — both defects contradict the plan's exact durable-context and fail-closed composition requirements.
  - Verification trust: Pass — fresh targeted/full Control Plane tests, vet, diff check, leak guard, import graph, and compiled startup matched the submitted passing evidence; the missing input boundary was independently reproduced.
  - Spec conformance: Fail — SDD scenario S09 requires ciphertext to open under the durable slot context and the configured credential runtime to fail closed.
- **Findings:**
  - **Required — `apps/control-plane/internal/credentialops/service.go:242`:** `CreateSlot` passes the caller's raw `CredentialKind` to `credentialseal.Context`, while `credentialstore.CreateSlot` normalizes that value before persistence. A reviewer probe created a slot with `" BEARER "`, observed persisted kind `"bearer"`, and then failed to open the initial envelope with the exact persisted principal/slot/kind context (`credentialseal: open failed`). Normalize and validate the credential kind before sealing, use that same canonical value for both AAD and store input, and add a real-keyring regression covering accepted case/whitespace variants plus rotation/restart opening.
  - **Required — `apps/control-plane/cmd/control-plane/server.go:105`:** with a complete `credential_encryption` triple and an empty `database.url`, `credentialstore.Open` returns `(nil, nil)`, `composeCredentialRuntime` returns success with no store/service, and `run` starts listeners. Reject this unusable enabled configuration before listeners (or otherwise guarantee a non-nil store and service whenever encryption is configured) and add composition/run tests for the full-encryption-without-database boundary.
- **Routing Signals:** `review_rework_count=1`, `evidence_integrity_failure=false`
- **Next Step:** Prepare and route a focused `REVIEW_API` follow-up plan for both Required fixes, then archive this reviewed pair and materialize the validated next pair.
