feat(insurance)!: make Config.APIKey an opaque Secret

[cristim]

Closes #330.

What and why

insurance.Config.APIKey was an exported string, so a caller that holds a Config in an unexported struct field printed the key raw: fmt does not call Format on unexported fields. APIKey is now insurance.Secret, which keeps the key in a closure so no reflection-based printer can reach it. Methods: NewSecret, Reveal (nil-guarded; used only at client.go header-set and the blank-key check), Format (all fmt verbs and slog text) and MarshalText (json, xml, yaml.v3, slog JSON). Config.Format and its %p/%T caveat are removed.

Fail-before control (scratch test on 16b5795, not shipped; one file cannot compile with both types)

Struct with an unexported cfg Config field:

%v  => {{SECRETKEY 11111111-1111-4111-8111-111111111111}}
%+v => {cfg:{APIKey:SECRETKEY OrgID:11111111-...}}

The gate reproduced the same with key fb-SECRET (APIKey:fb-SECRET). After this change no verb, json, xml, yaml, gob, slog or text/template output contains the key.

Tests

Extends TestConfigAndClient_NeverFormatTheKey and adds TestSecret_NeverRevealedByAnyPrinter (unexported Config/*Config/Secret holders, exact [redacted] on exported paths). Storing the key as a string field in Secret fails 44 assertions. go test ./insurance and golangci-lint on pkg pass.

Breaking changes (CHANGELOG)

Source break Config{APIKey: k} -> Config{APIKey: insurance.NewSecret(k)}; %v of a Config is {[redacted] <org>}; reflect.DeepEqual of two copies of one Config is false; Config is no longer comparable (== fails to compile, map key panics unhashable); json.Unmarshal into a Config with APIKey now errors (was ignored by json:"-").

Consumer migration (separate PRs after merge)

• platform internal/archera/provider.go:138: APIKey: insurance.NewSecret(key).
• cli #2166 pins 73d3366 (feat(insurance): Archera insured-commitment comparison contract #285, predates fix(insurance): keep the API key out of every field fmt can print #331), so its bump jumps from feat(insurance): Archera insured-commitment comparison contract #285 to this version and also takes fix(insurance): keep the API key out of every field fmt can print #331's behavior.
• mcp fix(config): pkg/config tests exercise a --dry-run flag the CLI does not register #55: keep key as the get() string for archeraError, wrap with NewSecret when building the Config, and edit archera_comparison_test.go call sites (:121, :137, :364, :366).

Labels mirrored from #330.

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{This review includes 5 billable files and costs up to $1.25.}

• Ask an admin to make reviews automatic

Open in CodeRabbit

Reviews can continue after your included limit without a manual trigger. An admin must approve usage-based billing.

Or wait 31 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available. Your 63 included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

• Configuration used: Repository: LeanerCloud/cloud-commitments-go/.coderabbit.yaml
• Review profile: CHILL
• Plan: Essentials
• Run ID: d4719569-b63b-4aa8-bca2-2ae16c70ce3a

---

📥 Commits

Reviewing files that changed from the base of the PR and between e88c535 and 11f0e03.

---

📒 Files selected for processing (5)

• CHANGELOG.md
• pkg/insurance/client.go
• pkg/insurance/client_test.go
• pkg/insurance/key_leak_test.go
• pkg/insurance/secret.go

---

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: LeanerCloud/cloud-commitments-go/.coderabbit.yaml
• Review profile: CHILL
• Plan: Essentials
• Run ID: c91a6162-a164-431c-a859-7a4041cde191

---

📥 Commits

Reviewing files that changed from the base of the PR and between 16b5795 and e88c535.

---

📒 Files selected for processing (5)

• CHANGELOG.md
• pkg/insurance/client.go
• pkg/insurance/client_test.go
• pkg/insurance/key_leak_test.go
• pkg/insurance/secret.go

---

Included review availability: This review used your included allowance. 0 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

---

---

📝 Walkthrough

📝 Walkthrough

📝 Walkthrough

📝 Walkthrough

Walkthrough

Config.APIKey now uses Secret instead of string. The new type reveals its value on request and emits [redacted] through formatting and text marshaling. Client construction uses Reveal() for key validation and client closures.

Changes

Secret API key handling

Layer / File(s)	Summary
Secret API and client integration pkg/insurance/secret.go, pkg/insurance/client.go, pkg/insurance/client_test.go, pkg/insurance/key_leak_test.go, CHANGELOG.md	Secret stores the key in a closure and redacts it in formatting and text marshaling. Config.APIKey uses Secret, and client construction calls Reveal(). Tests check redaction across formatting and serialization paths. The changelog records the API change and removal of Config.Format.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Feature

---

Merge Risk: ⚪ Minimal · up to e88c5

No actionable issue remains from these comments. The API-key type change is documented, and the redaction test still checks for credential disclosure.

Pre-merge checks | 4 | 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 41.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 4 files. (1 skipped: …	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Title check		The title clearly and concisely identifies the primary breaking change: Config.APIKey now uses an opaque Secret type.

---

Full details: Docstring Coverage

Explanation

Docstring coverage is 41.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 4 files. (1 skipped: 1 unsupported.)

---

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

---

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

---

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

_{Comment @coderabbitai help to get the list of available commands.}

[cristim]

Gate finding 1 (blocker) at e88c535: Lint Code (pkg) CI job fails, reproduced locally with GOTOOLCHAIN=go1.26.9 golangci-lint run ./... in pkg:

pkg/insurance/secret.go:27:48: Error return value of f.Write is not checked (errcheck)

errcheck (check-blank) flags _, _ = f.Write(...); the removed Config.Format used _, _ = fmt.Fprintf(...), which errcheck's default fmt exclusions allow. Verified locally: replacing the body with _, _ = fmt.Fprint(f, redactedText) gives golangci-lint: No issues found and pkg/insurance tests 229 passed. Please push that (or equivalent) so CI goes green.

[cristim]

Gate review (Opus 5.5, credential path) at e88c535: CHANGES REQUESTED, not merged.

Blockers:

1. CI Lint Code (pkg) fails: secret.go:27 errcheck on _, _ = f.Write(...) (see earlier comment; _, _ = fmt.Fprint(f, redactedText) verified lint-clean locally, 229 insurance tests pass).
2. PR body has no author text, only the CodeRabbit summary. The plan required the fail-before control quoted in the body, plus the consumer migration (platform internal/archera/provider.go:138 -> NewSecret(key); cli pins 73d3366, unaffected until bump; mcp keeps its key string). Fail-before I reproduced on 16b5795 with a scratch holder type fbHolder struct{ cfg Config }: fmt.Sprintf("%v %+v", ...) -> {{fb-SECRET 1111...}} {cfg:{APIKey:fb-SECRET ...}} (FAIL); feel free to quote it.

Non-blocking (please fold into the same push): CHANGELOG omits two further source/behaviour breaks caused by the func field: Config and Secret are no longer comparable (cfg == other stops compiling; as a map key or inside an any map key it panics "hash of unhashable type insurance.Secret"), and json.Unmarshal into a Config carrying an APIKey member now errors (the old json:"-" ignored it; Marshal->Unmarshal round trip fails). Also reflect.DeepEqual(cfg, cfg) is false even for the same value, not just copies.

Verified clean (local, GOTOOLCHAIN=go1.26.9, from git archive of the head):

• Own probe (external test package): holders {Config, *Config, unexported Config/*Config/Secret/*Secret/[]Secret/map, exported holder, []Secret, map[string]Secret, %w-wrapped error, struct embedding, nested unexported embed, []any} x 22 verbs (%v %+v %#v %s %q %x %X %d %T %p widths/flags %#x % x %U %c %b %o %e %t) x Sprint/Sprintln, spew, json, xml, yaml.v3, gob, slog text+JSON, text/template (printf/js/html/urlquery), reflect string walk: no raw/hex/HEX/base64/spaced-hex of the key anywhere. Probe is non-vacuous: against a string-field Secret mutant it reports 169 leaks.
• Zero Secret: Reveal()=="" no panic, prints [redacted]; NewClient rejects zero, NewSecret(""), NewSecret(" \t\n").
• Reveal() only at client.go:84 (TrimSpace check) and :95 (captured into setAuth/redact closures, as before fix(insurance): the API key is printed by fmt %s when a *insurance.Client is held in an unexported struct field #325); no other struct field holds it.
• pkg go test ./... rc 0, go vet 0; go build all 5 modules in workspace and GOWORK=off: ok.
• 17 mutants, all killed by a named test assertion (none by compile error): (a) string field -> TestSecret_NeverRevealedByAnyPrinter; (b) drop Format -> same; (c) drop MarshalText, (d) MarshalText returns key, (g) Format leaks on %x, (h) leaks on %#v, (i) redaction text changed, (n) MarshalText returns nil -> NeverFormatTheKey + NeverRevealedByAnyPrinter; (e) Reveal returns "" -> 20 tests; (f) no TrimSpace -> TestNewClient_RejectsBlankKey; (j) nil-guard removed -> TestNewClient_ValidatesConfig (panic); (k) header gets wrong value -> Comparison_RequestAndDecode; (l) redact disabled, (p) redact on altered key -> ErrorMessageIsSanitized; (m) key stored in a Client string field -> HoldsNoStringContainingKey + KeyNeverPrintedWherever; (o) pointer-receiver Format -> NeverRevealedByAnyPrinter.
• Labels mirror fix(insurance): Config.APIKey is still a printable string in caller-held structs #330. fix(insurance): cap and sanitize vendor values echoed in decode errors #337 touches quote.go/quote_test.go/CHANGELOG only: disjoint apart from a possible CHANGELOG hunk conflict.

Re-gate needed at the new head after the lint fix and body update.

[cristim]

Merge-order note from the #339 gate: #339 merges textually clean with this PR, but the combined tree does not compile. #339 adds if cfg.APIKey != strings.TrimSpace(cfg.APIKey) in newClient and TestNewClient_RejectsPaddedKey with Config{APIKey: <string>}. My recommendation is to merge #339 first (fix-only, non-breaking). This PR then rebases: compare key := cfg.APIKey.Reveal() against strings.TrimSpace(key) and wrap the test keys in the Secret constructor. Because GitHub reports no conflict, the re-gate here must rely on a fresh CI build after the update.

[cristim]

Gate re-review (Opus 5.5): APPROVED at 11f0e03, merging.

• 248c469 vs e88c535: only secret.go:27 (_, _ = fmt.Fprint(f, redactedText)) and the CHANGELOG breaking-change text. Secret behaviour is unchanged. I reran my leak probe (all holders x 22 verbs x json/xml/yaml/gob/slog/template/spew/reflect): PASS. The 17 mutants all fail a named test again. Spot checks of %v %x %#v %q %10s %d all give exactly [redacted]. pkg go test ./... rc 0, go vet 0, golangci-lint 0 issues, and all 5 modules build with GOWORK=off.
• PR body checked: fail-before from 16b5795 (reproduced independently), breaking changes, consumer migration (platform provider.go:138, cli #2166 pins 73d3366, mcp fix(config): pkg/config tests exercise a --dry-run flag the CLI does not register #55), Closes fix(insurance): Config.APIKey is still a printable string in caller-held structs #330. Labels match fix(insurance): Config.APIKey is still a printable string in caller-held structs #330.
• Head was BEHIND after fix(insurance): cap and sanitize vendor values echoed in decode errors #337 merged. Ran gh pr update-branch (merge commit 11f0e03). The PR's code files are unchanged vs 248c469 and the CHANGELOG merged cleanly (this PR's entry sits in Changed, fix(insurance): decode errors echo uncapped vendor values via %q (prompt-injection / log flooding) #336 in Fixed). pkg tests rc 0 at 11f0e03 (go1.26.9, -p 2).
• CI at 11f0e03: all 15 checks pass, including Lint Code (pkg). mergeStateStatus CLEAN.
• Wording nits, not blocking (for follow-up): map[Config]V fails to compile; the panic happens only when a Config is used as an interface map key. The older Unreleased "Added" bullet ("redacted ... except %p") is now stale.