Follow-ups from nan-devops#290 (credential scrub): unhooked routes, previous_models, enterprise pod-age, and test/CI pins

[sre-helmcode]

nan-devops#290 merged as a548384 and is deployed. Its three blocking reviewers each answered MERGE-BLOCKING: none on the deployed artifact and filed these as follow-ups. Grouped by whether they can leak.

Can leak, latent today

1. Unhooked routes get no scrub, and the counter cannot see it. Every sink is reachable only because the scrub runs in async_pre_call_hook. google_endpoints, batches, openai_files_endpoints, rag_endpoints and the assistants/threads handlers call add_litellm_data_to_request — which is what creates the leak — but never pre_call_hook. There it is sink 1 that leaks, the one that was in ~100% of rows. Measured: zero requests to any of them in 7 days, so nothing flows today. But nan-credential-scrub-stopped keeps moving on the hooked traffic, so if one of those routes is ever enabled the alert stays green through a leak.

2. metadata.previous_models — LiteLLM: metadata.previous_models leaks one member's credentials into another member's SpendLogs row #45, still open, and not mentioned in MONITORING.md. Router.previous_models is an INSTANCE attribute, so a row can carry another member's headers. Needs the post-deploy probe with a forced fallback that the hook docstring calls for, and a line in MONITORING.md so a responder reading "clean" from step 3 knows a second column can still carry a token.

3. _redact_credential_url does not visit metadata.requester_metadata.endpoint. The header scrub covers requester_metadata.headers; the URL scrub covers proxy_server_request.url, metadata.endpoint and litellm_metadata.endpoint only. Inert in practice (only the Google generateContent routes accept key= in the query, and those are the unhooked routes from item 1 with zero traffic), but it is an asymmetry against the header scrub.

4. raw_headers as a list of tuples is not scrubbed (isinstance(raw, dict)). Not silent: the counter would fall to zero and nan-credential-scrub-stopped would fire. Worth a Mapping/sequence branch if upstream ever changes the shape.

5. data["api_key"] is a top-level STRING, frozen by the shallow copy, so no in-place scrub reaches it. It only appears if forward_llm_provider_auth_headers is set, which is unset in both tenants. If anyone turns that on, this scrub does not cover that sink.

Operational

6. Enterprise still has the pod-age bug. litellm-enterprise/values.yaml keeps maximum_spend_logs_retention_interval: "6h" with no cron. Same latent failure the community comment diagnoses in litellm-community: the SpendLogs pruner is not running — retention window grows monotonically #44 — it looks healthy only because its pods happen to outlive the interval. Give it maximum_spend_logs_cleanup_cron too.

7. The traffic gate is the only consumer of litellm_proxy_total_requests_metric_total in the whole alerting file. If a LiteLLM upgrade renames it, nan-credential-scrub-stopped goes silent and no other rule notices. Either an absent() canary on that metric or reuse a metric more rules depend on.

8. Stale operator-facing figures. The runbook and the values.yaml comment say "~136 GB"; the table measured 154 GB with ~970k rows during the review (the width is TOAST, not the row count). Also worth recording: the interval job DID fire once, ~24h after the 2026-09-08 22:56 restart, so the backlog beyond the 2d window is only ~30k rows — which confirms the pod-age diagnosis rather than contradicting it.

9. MONITORING.md places the counter bullet in the ENTERPRISE counters section with a bold "THIS IS ALERTED", while the rule is community-only (job="litellm-community"). The section preamble already says enterprise is not paged, so no wrong action follows, but "community only" would close the ambiguity. Related: the enterprise reading of the runbook table gives rows>0, path_present=0, which the table calls "THE PATH MOVED"; on enterprise it just means store_prompts_in_spend_logs is unset.

10. Step 3 of the runbook has no SET statement_timeout. The startTime predicate is indexed and returned well under a second on the primary, so it is not a danger today, but prefixing it costs nothing — and a reviewer's exploratory query took that primary down during this review.

Test and CI pins

11. Assorted surviving mutants in test files and workflow wiring, none of which reach production: if:/continue-on-error at other levels, unpinned fields, and the general problem that each pin is itself new unpinned surface. Seventeen rounds of review closed ~125 of these; the remainder are bounded by the structural checks now in place (label parity by corpus, sink parity by result, and the CI meta-gate that requires every file a suite reads to trigger a workflow that runs it).

Not doing

Key rotation. The owner decided against it on #43, recorded there with the reasoning and the compensating controls. That decision stands; do not reopen it by default.