<!-- task=m-principal-provider-credential-slot-routing/05+01,02_management_core plan=3 tag=REVIEW_API -->

# Credential Management Core Correctness Repair

## For the Implementing Agent

Filling implementation-owned sections of `CODE_REVIEW-cloud-G07.md` is mandatory. Run every verification command, paste actual output, keep active files in place, and report ready for review; only code-review may finalize, rename logs, write `complete.log`, or archive. If blocked, record exact blocker evidence, attempted commands/output, and resume conditions only in implementation-owned fields; do not ask the user, call user-input tools, create stop files, classify the next state, archive logs, or write `complete.log`.

## Background

The first management-core implementation authenticates principals and wraps several lifecycle calls, but secret mutation is non-functional because it always seals an empty value. It also returns the encrypted envelope from secret-blind DTOs and constructs managed resources directly through the store in tests, leaving the planned create and token/route management boundary incomplete. This follow-up repairs those defects without activating a network operation or claiming S08 completion.

## Archive Evidence Snapshot

- Parent pair after archive: `agent-task/m-principal-provider-credential-slot-routing/05+01,02_management_core/plan_local_G06_2.log`, `agent-task/m-principal-provider-credential-slot-routing/05+01,02_management_core/code_review_cloud_G06_2.log`.
- Verdict: FAIL; Required 3, Suggested 0, Nit 0; `review_rework_count=1`, `evidence_integrity_failure=false`.
- Required repairs: seal the supplied provider secret for create/rotate, remove encrypted envelopes from public DTOs, and implement/test authenticated token/slot/route management without direct-store resource creation.
- Fresh reviewer verification: credentialops race tests, Control Plane regression tests, structural absence, predecessor checks, and `git diff --check` passed; correctness and coverage failed by source inspection.
- Roadmap carryover: this remains a preparatory S08 foundation. PASS must not check `credential-management`; a production sealer and confidential Client transport remain later S09/S10 work.

## Analysis

### Files Read

- `apps/control-plane/internal/credentialops/service.go`
- `apps/control-plane/internal/credentialops/service_test.go`
- `apps/control-plane/internal/credentialstore/principal.go`
- `apps/control-plane/internal/credentialstore/slot.go`
- `apps/control-plane/internal/credentialstore/route.go`
- `apps/control-plane/internal/credentialstore/secret_envelope.go`
- `apps/control-plane/internal/wire/client.go`
- `proto/iop/control.proto`
- `agent-contract/inner/client-control-plane-wire.md`
- `agent-spec/control/control-plane-operations.md`
- `agent-roadmap/phase/operational-observability-provider-management/PHASE.md`
- `agent-roadmap/phase/operational-observability-provider-management/milestones/principal-provider-credential-slot-routing.md`
- `agent-roadmap/sdd/operational-observability-provider-management/principal-provider-credential-slot-routing/SDD.md`
- `agent-task/archive/2026/08/m-principal-provider-credential-slot-routing/01_principal_store/complete.log`
- `agent-task/archive/2026/08/m-principal-provider-credential-slot-routing/02+01_credential_catalog/complete.log`

### SDD Criteria

- SDD: `agent-roadmap/sdd/operational-observability-provider-management/principal-provider-credential-slot-routing/SDD.md`; status `[승인됨]`, lock released, no user review.
- Targeted foundation criterion: S08 / `credential-management`; Evidence Map row S04-S08 requires principal isolation, management authorization, lifecycle revision behavior, and secret-blind results.
- The checklist derives create/rotate sealing, principal ownership, CAS, token/slot/route lifecycle, and response/log redaction from that row. It intentionally omits `Roadmap Targets` because a concrete at-rest sealer and confidential remote operation required by S09/S10 are outside this packet.

### Verification Context

- No external verification handoff was supplied. Repository-native sources were the local test rules, Control Plane smoke profile, Go module, existing credentialops/store tests, approved SDD, and Client wire contract.
- Environment preflight: `/config/.local/bin/go`, Go 1.26.2 linux/arm64, `GOROOT=/config/opt/go`, repository root `/config/workspace/iop-s0`, current dirty worktree preserved.
- Preconditions: archived `01_principal_store` and `02+01_credential_catalog` each have exactly one `complete.log`; both were read and satisfy indices 01 and 02.
- Required criteria: fresh race-enabled credentialops tests, Control Plane regression tests, deterministic plaintext-wire absence search, formatting, vet, and diff checks.
- No external runner, provider, credential, TLS endpoint, Flutter change, or full-cycle user flow is required because the service remains dormant and transport-neutral. Confidence: high.

### Test Coverage Gaps

- `RotateSlot` has no provider-secret input and existing tests accept a fake envelope generated from an empty slice.
- `TestServiceResponsesAreSecretBlind` serializes and accepts ciphertext, nonce, key metadata, and AAD from `SlotRecord`; it never supplies a raw provider secret.
- Lifecycle tests create slots/routes directly through `credentialstore`, so they do not cover authenticated service create/update authorization.
- No service-level token mutation test exists, and the claimed captured-log redaction test neither captures logs nor asserts the sealer plaintext.

### Symbol References

- `RotateSlotInput`, `SlotRecord`, `NewService`, and `RotateSlot` are referenced only by `apps/control-plane/internal/credentialops/service.go` and `service_test.go`; no production caller exists.
- New create/update/token methods have no existing call sites. `wire.ClientServer`, protobuf, Dart, CLI, and Control Plane server wiring remain unchanged.

### Split Judgment

- Keep one packet: secret input ownership, sealer handoff, secret-blind DTOs, and service-level lifecycle tests form one security invariant and cannot independently PASS if split.
- Runtime predecessors 01 and 02 are satisfied by `agent-task/archive/2026/08/m-principal-provider-credential-slot-routing/01_principal_store/complete.log` and `agent-task/archive/2026/08/m-principal-provider-credential-slot-routing/02+01_credential_catalog/complete.log`.

### Scope Rationale

- Modify only `credentialops` service and tests. Reuse the completed store APIs; do not change store schema, protobuf, ClientServer, Dart, CLI, config, server wiring, or contracts.
- Add no concrete encryption/key implementation, remote management adapter, credential reveal operation, or plaintext network payload.
- Initial principal bootstrap remains sibling `06+01,02_host_local_bootstrap`; direct-store setup is allowed only for that prerequisite fixture and post-operation persistence inspection.

### Final Routing

- `status=routed`, `evaluation_mode=isolated-reassessment`, finalizer=`finalize-task-policy.sh`, pair mode.
- Build closures all true; scores `2+2+1+1+1=G07`; base/route=`local-fit`, lane=`local`, canonical=`PLAN-local-G07.md`.
- Review closures all true; scores `2+2+1+1+1=G07`; route=`official-review`, lane=`cloud`, model=`gpt-5.6-sol` xhigh, canonical=`CODE_REVIEW-cloud-G07.md`.
- `large_indivisible_context=false`; risks=`temporal_state`, `concurrent_consistency`, `boundary_contract` (3); `review_rework_count=1`; `evidence_integrity_failure=false`; capability gap none.

## Dependencies and Execution Order

1. Archived predecessors `01_principal_store` and `02+01_credential_catalog` are complete.
2. Define secret-blind DTOs and functional authenticated operations, then rewrite tests around that public service boundary.
3. Keep the plaintext Client wire unchanged.

## Implementation Checklist

- [ ] Make slot create/rotate seal the caller-supplied provider secret through an owned, zeroized buffer and fail without mutation when the sealer is unavailable.
- [ ] Expose secret-blind authenticated token/slot/route create, update, list, and lifecycle operations using existing store CAS/ownership rules, with no envelope or token digest in returned DTOs.
- [ ] Replace direct-store managed-resource setup with deterministic service-level lifecycle, cross-principal, stale-CAS, sealer-handoff, zeroization, and response/error/log redaction tests.
- [ ] Run fresh formatting, vet, race, Control Plane regression, predecessor, structural-absence, and diff verification.
- [ ] Fill implementation-owned sections in CODE_REVIEW-*-G??.md with actual implementation notes and verification output.

### [REVIEW_API-1] Repair the authenticated management boundary

**Problem:** `apps/control-plane/internal/credentialops/service.go:103-107` carries no provider secret in `RotateSlotInput`, line 210 seals `[]byte{}`, line 81 exposes `SecretEnvelope`, and the service has no create-slot, create-route/update-route, or token lifecycle method.

**Solution:** Add service-owned input/DTO types. `CreateSlot` and `RotateSlot` copy the supplied provider secret into an owned byte slice, defer zeroization, pass it to `SecretSealer`, and send only the resulting envelope to `credentialstore`. Remove `Envelope` from `SlotRecord`; add a digest-free token DTO plus current-token disable/revoke wrappers, and add authenticated create/update route wrappers that authorize both route and target-slot ownership before store calls. Map store CAS/not-found/lifecycle errors consistently and remove unused constructor dependencies/import sentinels.

Before (`apps/control-plane/internal/credentialops/service.go:103-107,207-210`):

```go
type RotateSlotInput struct {
    SlotID   string
    Revision int64
}

sealed, err := s.sealer.Seal(ctx, []byte{}, sealCtx)
```

After:

```go
type CreateSlotInput struct {
    Vendor         string
    CredentialKind string
    Alias          string
    ProviderSecret []byte
}

type RotateSlotInput struct {
    SlotID         string
    Revision       int64
    ProviderSecret []byte
}

owned := append([]byte(nil), input.ProviderSecret...)
defer clear(owned)
sealed, err := s.sealer.Seal(ctx, owned, sealCtx)
```

**Modified Files and Checklist:**

- [ ] `apps/control-plane/internal/credentialops/service.go`: add functional slot create/rotate, authenticated route create/update, safe current-token lifecycle, secret-blind DTOs, consistent error mapping, and owned secret zeroization.
- [ ] `apps/control-plane/internal/credentialops/service_test.go`: update constructor and operation call sites and add focused boundary assertions.

**Test Strategy:** Update `TestServiceScopesOperationsToAuthenticatedPrincipal` so all slot/route actions use `Service`; add `TestServiceCreateAndRotatePassSuppliedSecretOnlyToSealer`, `TestServiceTokenLifecycle`, and `TestServiceRouteCreateUpdateLifecycle`. The fake sealer copies observed plaintext before return, store inspection confirms only its envelope is persisted, and missing-sealer cases assert no row/revision change.

**Verification:** `GOTMPDIR=/config/workspace/iop-s0/.cache/go-build GOCACHE=/config/workspace/iop-s0/.cache/go-cache go test -count=1 -race ./apps/control-plane/internal/credentialops -run 'CreateAndRotate|TokenLifecycle|RouteCreateUpdate|ScopesOperations|RequiresSealer'` exits 0.

### [REVIEW_API-2] Make secret-blind evidence meaningful

**Problem:** `apps/control-plane/internal/credentialops/service_test.go:443-522` calls a response secret-blind while deliberately serializing its envelope, and lines 771-825 never supply or assert provider plaintext and do not capture logs.

**Solution:** Rewrite redaction fixtures with unique raw IOP token, provider secret, ciphertext, nonce, AAD, and digest sentinels. Assert every returned slot/token/route DTO, error, and captured log field excludes all sensitive sentinels; reflect on `SlotRecord` to prevent envelope reintroduction. Test cross-principal create/update/rotate and stale-CAS variants through `Service`, with direct store use limited to prerequisite principal setup and persistence inspection. Preserve a source-level and runtime guard that construction does not register a wire listener.

Before (`apps/control-plane/internal/credentialops/service_test.go:475-483`):

```go
recordStr := slotRecordString(rotated)
if strings.Contains(recordStr, issued.RawToken) {
    t.Fatal("SlotRecord must not contain raw IOP token")
}
if strings.Contains(recordStr, "sealed-plaintext") {
    t.Fatal("SlotRecord must not contain sealer plaintext")
}
```

After:

```go
for _, forbidden := range []string{rawIOPToken, providerSecret, ciphertext, nonce, aad, tokenDigest} {
    require.NotContains(t, renderedResponsesErrorsAndLogs, forbidden)
}
if _, ok := reflect.TypeOf(SlotRecord{}).FieldByName("Envelope"); ok {
    t.Fatal("secret-blind SlotRecord must not expose an envelope")
}
```

**Modified Files and Checklist:**

- [ ] `apps/control-plane/internal/credentialops/service_test.go`: replace vacuous redaction checks and direct-store lifecycle setup with public-boundary coverage for the complete known defect set.

**Test Strategy:** Keep the existing plan-named tests, strengthen their assertions, and add focused table-driven variants for create/rotate secret handoff, cross-principal slot/route targets, token/slot/route stale revisions, missing sealer with no mutation, and sensitive sentinel absence from responses/errors/logs. Do not create a speculative wire integration matrix because no wire adapter is added.

**Verification:** `GOTMPDIR=/config/workspace/iop-s0/.cache/go-build GOCACHE=/config/workspace/iop-s0/.cache/go-cache go test -count=1 -race ./apps/control-plane/internal/credentialops -run 'SecretBlind|Redact|CrossPrincipal|StaleRevision|NoNetwork'` exits 0.

## Modified Files Summary

| File | Items |
|------|-------|
| `apps/control-plane/internal/credentialops/service.go` | REVIEW_API-1 |
| `apps/control-plane/internal/credentialops/service_test.go` | REVIEW_API-1, REVIEW_API-2 |
| `agent-task/m-principal-provider-credential-slot-routing/05+01,02_management_core/CODE_REVIEW-cloud-G07.md` | REVIEW_API-1, REVIEW_API-2 |

## Final Verification

Run from `/config/workspace/iop-s0`; fresh Go results are required and cached output is not acceptable.

1. `test "$(find agent-task/archive -type f -path '*/m-principal-provider-credential-slot-routing/01_principal_store/complete.log' -print | wc -l)" -eq 1 && test "$(find agent-task/archive -type f -path '*/m-principal-provider-credential-slot-routing/02+01_credential_catalog/complete.log' -print | wc -l)" -eq 1` — both predecessors remain uniquely complete.
2. `mkdir -p .cache/go-build .cache/go-cache` — workspace-local Go temp/cache paths exist.
3. `test -z "$(gofmt -d apps/control-plane/internal/credentialops/service.go apps/control-plane/internal/credentialops/service_test.go)"` — no Go formatting drift.
4. `GOTMPDIR=/config/workspace/iop-s0/.cache/go-build GOCACHE=/config/workspace/iop-s0/.cache/go-cache go vet ./apps/control-plane/internal/credentialops` — no vet findings.
5. `GOTMPDIR=/config/workspace/iop-s0/.cache/go-build GOCACHE=/config/workspace/iop-s0/.cache/go-cache go test -count=1 -race ./apps/control-plane/internal/credentialops` — all management-boundary and race tests pass.
6. `GOTMPDIR=/config/workspace/iop-s0/.cache/go-build GOCACHE=/config/workspace/iop-s0/.cache/go-cache go test -count=1 ./apps/control-plane/...` — the Control Plane regression suite passes.
7. `if rg --sort path -n 'message[[:space:]]+.*(Bootstrap|CredentialOperation)|AddRequestListenerTyped.*Credential' proto/iop/control.proto apps/control-plane/internal/wire/client.go; then exit 1; else exit 0; fi` — no plaintext Client management operation exists.
8. `git diff --check` — no whitespace errors.

After completing all code changes, fill implementation-owned sections in `CODE_REVIEW-*-G??.md`.
