Skip to content

feat(insurance): add default-off Archera settings, lazy client and status endpoint - #795

Merged
cristim merged 3 commits into
mainfrom
feat/785-archera-config-status
Oct 9, 2026
Merged

cristim merged 3 commits into
mainfrom
feat/785-archera-config-status

Conversation

@cristim

@cristim cristim commented Oct 9, 2026 •

Copy link
Copy Markdown
Member

PR2a of #785. Security-sensitive: handles an API key and gates on authz.

  • internal/archera: ARCHERA_ORG_ID, ARCHERA_PLAN_ID, ARCHERA_API_KEY_SECRET (secret reference). The key is resolved lazily on the first comparison call through the existing secret resolver; a mutex (not sync.Once) guards the build so a failed resolve is retried and never cached. Failures return a fixed string naming the setting only.
  • GET /api/insurance/status: presence of the three settings by name only, no values, no secret read, no outbound call. Gate: view:recommendations plus an unrestricted account scope; scoped sessions and scoped user-API-key principals get 404. Unwired or unconfigured means configured=false.
  • The comparison route, DTO and error mapping follow in PR2b.

Redaction hazard found here (relevant to every pkg/insurance consumer): a struct holding a plain *insurance.Client in an UNEXPORTED field, printed with fmt %s (also on any struct containing it), prints the client fields including the API key, because fmt does not call Client.Format on unexported fields and dereferences the pointer for that verb. Reproducer: type P struct{ client *insurance.Client }; fmt.Sprintf("%s", P{c}). %v/%+v/%#v did not leak in my run. Fix here: the client is held in an atomic.Pointer (fmt prints an address) and *Provider has a Format method. Tests print the Provider, a wrapping struct, Settings and the Handler with %v %+v %#v %s before and after the client is built and after a successful synthetic comparison, grepping for the synthetic key.

Verification (local macOS, synthetic key and IDs, no live api.archera.ai calls; RoundTripper asserts host api.archera.ai): go build, go vet, golangci-lint (0 issues), go test ./internal/... and go test -tags integration for internal/api, internal/server, internal/purchase all exit 0. Mock/httptest evidence only; no deploy.

Refs #785 #786

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
    • Added an authenticated insurance status endpoint that reports whether insurance settings are configured and identifies missing settings without exposing secret values.
    • Users need recommendation-viewing permission and an unrestricted account scope to access the status.
    • Insurance settings can now be supplied through the application environment; the API key is resolved only when needed.

…atus endpoint

Adds internal/archera (ARCHERA_ORG_ID, ARCHERA_PLAN_ID, ARCHERA_API_KEY_SECRET;
key resolved lazily through the secret resolver, mutex-guarded so a failed
resolve is retried) and GET /api/insurance/status, gated on view:recommendations
plus an unrestricted account scope (scoped sessions get 404). Status reports
setting presence by name only and makes no outbound call. The comparison
endpoint follows in a separate PR.

Refs #785 #786

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@cristim cristim added priority/p2 Backlog-worthy severity/medium Moderate harm urgency/this-sprint Within the current sprint effort/l Weeks triaged Item has been triaged impact/few Limited audience type/feat New capability labels Oct 9, 2026
@coderabbitai

coderabbitai Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: LeanerCloud/cloud-commitments-platform/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Essentials
  • Run ID: 294dd791-1ccf-4e96-85b0-ed011c49bd07

📥 Commits

Reviewing files that changed from the base of the PR and between 7d492ec and 0c2cb2e.


📒 Files selected for processing (8)
  • internal/api/handler.go
  • internal/api/handler_insurance.go
  • internal/api/handler_insurance_test.go
  • internal/api/router.go
  • internal/api/types.go
  • internal/archera/provider.go
  • internal/archera/provider_test.go
  • internal/server/app.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

Adds an Archera provider that reads settings from the environment, resolves the API key when building a client, and reports configuration status without exposing setting values. Adds an authenticated insurance-status endpoint and wires the provider into application API handlers.

Changes

Insurance status integration

Layer / File(s) Summary
Archera settings and client lifecycle
internal/archera/provider.go, internal/archera/provider_test.go
Adds environment-backed settings and presence-only status. The provider resolves the API key lazily, caches successfully built clients, and returns fixed errors for incomplete configuration or unavailable keys. Tests cover status, retries, client reuse, request composition, and secret redaction.
Insurance status endpoint and access checks
internal/api/handler.go, internal/api/handler_insurance.go, internal/api/router.go, internal/api/types.go, internal/api/handler_insurance_test.go
Adds the provider dependency and authenticated GET /api/insurance/status route. The handler requires view:recommendations permission and unrestricted account scope before returning status. Tests cover access outcomes, route authentication, status output, and secret redaction.
Application provider wiring
internal/server/app.go
Constructs and stores the Archera provider, then supplies it to the initial API handler and the handler rebuilt after database connection.

Priority: ⬇️ Low

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

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant Router
  participant AuthUser
  participant getInsuranceStatusHandler
  participant Handler
  participant InsuranceProvider
  Client->>Router: GET /api/insurance/status
  Router->>AuthUser: Authenticate request
  AuthUser->>getInsuranceStatusHandler: Dispatch authenticated request
  getInsuranceStatusHandler->>Handler: Delegate to getInsuranceStatus
  Handler->>Handler: Check permission and account scope
  Handler->>InsuranceProvider: Read Status
  InsuranceProvider-->>Handler: Return status
  Handler-->>Client: Return status or error
Loading

Merge Risk: ⚪ Minimal · up to 0c2cb

The insurance status integration has no established merge-blocking issue after normal checks.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 27.27% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 22 functions across 8 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely summarizes the main changes: default-off Archera settings, lazy client construction, and the insurance status endpoint.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.


  • Fix all pre-merge checks with AI
✨ 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.

Comment thread internal/archera/provider.go Fixed
@cristim

cristim commented Oct 9, 2026

Copy link
Copy Markdown
Member Author

Gate review at cd16c3d: BLOCKED by CI (gosec), and the branch is behind main by 7d492ec. The code review itself is clean.

Blocker:

  1. gosec G101 "Potential hardcoded credentials" at internal/archera/provider.go:25 (EnvKeySecret = "ARCHERA_API_KEY_SECRET"). It is a false positive (setting name, not a credential) but it turns gosec and Security Scanning red. Fix: // #nosec G101 -- setting name, not a credential value, the same pattern as internal/api/middleware.go:63. When you push the fix, merge origin/main in the same push (normal merge).

Verified independently (git archive, go1.26.9):

  • Redaction: I printed %v %+v %#v %s %q %x %X %d %T on *Provider, Provider, Settings, and structs holding the provider in exported, unexported and interface fields, before and after a built client. The full key, fragments of it and its hex form never appeared. Fails-before: replacing atomic.Pointer with a plain *insurance.Client AND removing Format makes TestComposition_EnvToDecodedComparison_AndRedaction fail with the key printed. Either fix alone is enough for the shapes tested.
  • Residual hazard (go#325, open): any struct holding the raw *insurance.Client in an unexported field leaks the key under %s and %q (reproduced). PR2b must keep the client returned by Client() in locals only, until go#325 adds Format to *insurance.Client.
  • Authz mutations: removing the gate fails DeniedUser403, ScopedSession404 and ScopedUserAPIKey404. Removing AllowsAll fails both scoped tests. A sync.Once-style cached failure fails ResolveFailureIsNotCached. Adding OrgID to Status fails TestStatus_NamesOnly. /api/insurance/status returns only configured plus setting names and makes no outbound call or secret read. Production wiring passes hc=nil. The transport test asserts https://api.archera.ai.
  • Wiring is present at both NewHandler sites in internal/server/app.go. Gap (non-blocking): dropping Insurance: app.archera from reinitializeAfterConnect fails no test. PR2b should add a test that the handler rebuilt after DB connect still carries the provider.
  • Nit (non-blocking): provider.go logs the resolver error with %v. The real resolvers only include the secret reference and the SDK error, never the value, so this is fine today.
  • Local results: go build 0, go vet 0, go test ./internal/... 0, golangci-lint (archera/api/server) 0 issues, go test -tags integration ./internal/api/ ./internal/server/ 0 (under the test-suite lock).
  • Size: about 207 non-test lines (provider.go is mostly doc comments) plus about 310 test lines. Under the 400-line code target; the tests are not a problem.

@cristim

cristim commented Oct 9, 2026

Copy link
Copy Markdown
Member Author

Gate re-run at 0c2cb2e: CLEAN, merging.

  • Since cd16c3d: 8a278c4 is a normal merge of origin/main (7d492ec, fix(purchase): scope plan fan-out recommendations to each account #793). Its tree equals git merge-tree --write-tree cd16c3db 7d492ecd (e9a1b5be), so no hand edits. 0c2cb2e changes one line: // #nosec G101 -- setting name, not a credential value on internal/archera/provider.go:25. The PR's own patch is otherwise unchanged, so the earlier review (redaction across all fmt verbs, authz/once/status mutations, fails-before) carries over.
  • Local from git archive (go1.26.9, GOWORK=off, -mod=readonly): go build ./... 0, go vet ./... 0, go test ./internal/... 0, golangci-lint (archera/api/server) 0 issues, gosec ./internal/archera/ 0, go test -count=1 -tags integration ./internal/api/ ./internal/server/ 0 under the test-suite lock (api 158.2s, server 43.6s).
  • CI 26/26 pass, mergeStateStatus CLEAN at this SHA.
  • Follow-ups for PR2b (non-blocking): add a test for the provider wiring in reinitializeAfterConnect, and keep the *insurance.Client in local variables only until go#325 lands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/l Weeks impact/few Limited audience priority/p2 Backlog-worthy severity/medium Moderate harm triaged Item has been triaged type/feat New capability urgency/this-sprint Within the current sprint

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants