Surfaced by the #328 blocking reviewer (findings 3 and 5).
What is covered today
nan-credential-scrub-stopped is
scalar(sum(rate(nan_credential_header_redacted_total[15m])) or vector(0)) == bool 0
(MONITORING.md:800) — a sum with no by.
#328 made the counter increment only AFTER del succeeds. So a scrub that
fails COMPLETELY drives the series to zero and the alert fires. That much is
closed.
What is not
A scrub that fails for SOME sinks or SOME header names keeps the series alive
while a real credential sits in LiteLLM_SpendLogs. The aggregate cannot
distinguish "8 sinks scrubbed" from "7 scrubbed, 1 leaking".
Concrete shapes that produce it:
- one of the eight header dicts raising inside its per-dict
try (the loop
continues, by design, so the request never breaks)
del failing for one header name while others succeed- a sink disappearing from the request shape after a LiteLLM upgrade while the
other seven keep counting
Options
- A per-label arm: alert on a
header label whose rate drops to zero while
the others keep moving. Cheap, but header is BUCKETED, so it only catches
a whole bucket going quiet.
- A dedicated
nan_credential_scrub_failed_total{header,cause} counter,
incremented in the except of each per-dict/per-element block, alerted on
> 0. This is the honest signal and was deliberately NOT done in #328:
it changes the counter census (community 9 -> 10, enterprise 7 -> 8) and
would have meant rewriting the MONITORING.md denominators (a number that
has already been wrong three times) inside a PR that was ready to merge.
- Compare the scrub counter against request volume rather than against zero.
Option 2 is the recommendation.
Why this is not urgent
The failure modes above are not attacker-reachable today: secret_fields and
the header dicts are built by LiteLLM from a JSON body, which yields plain
dicts and lists whose del cannot fail. #328 added guards for the
pathological shapes anyway. This issue is about the SIGNAL, not a live leak.
Related: #46, #43.
Surfaced by the #328 blocking reviewer (findings 3 and 5).
What is covered today
nan-credential-scrub-stoppedisscalar(sum(rate(nan_credential_header_redacted_total[15m])) or vector(0)) == bool 0(MONITORING.md:800) — a
sumwith noby.#328 made the counter increment only AFTER
delsucceeds. So a scrub thatfails COMPLETELY drives the series to zero and the alert fires. That much is
closed.
What is not
A scrub that fails for SOME sinks or SOME header names keeps the series alive
while a real credential sits in
LiteLLM_SpendLogs. The aggregate cannotdistinguish "8 sinks scrubbed" from "7 scrubbed, 1 leaking".
Concrete shapes that produce it:
try(the loopcontinues, by design, so the request never breaks)
delfailing for one header name while others succeedother seven keep counting
Options
headerlabel whose rate drops to zero whilethe others keep moving. Cheap, but
headeris BUCKETED, so it only catchesa whole bucket going quiet.
nan_credential_scrub_failed_total{header,cause}counter,incremented in the
exceptof each per-dict/per-element block, alerted on> 0. This is the honest signal and was deliberately NOT done in #328:it changes the counter census (community 9 -> 10, enterprise 7 -> 8) and
would have meant rewriting the MONITORING.md denominators (a number that
has already been wrong three times) inside a PR that was ready to merge.
Option 2 is the recommendation.
Why this is not urgent
The failure modes above are not attacker-reachable today:
secret_fieldsandthe header dicts are built by LiteLLM from a JSON body, which yields plain
dicts and lists whose
delcannot fail. #328 added guards for thepathological shapes anyway. This issue is about the SIGNAL, not a live leak.
Related: #46, #43.