Skip to content

fix(selfhost): stop the redaction scrubber nulling every AI token count - #10212

Merged
loopover-orb[bot] merged 1 commit into
mainfrom
feat/ai-content-and-trace-envelope
Jul 31, 2026
Merged

fix(selfhost): stop the redaction scrubber nulling every AI token count#10212
loopover-orb[bot] merged 1 commit into
mainfrom
feat/ai-content-and-trace-envelope

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

Not one AI call in the project has ever carried a token count. The privacy scrubber was eating them.

SECRET_KEY matches /token/i, and scrubRecord — wired as posthog-node's before_send — redacts on key match. So PostHog's own $ai_input_tokens and $ai_output_tokens were rewritten to the "[redacted]" string, which PostHog then coerced to null on its numerically-typed properties. Confirmed on a real live event:

{"$ai_input_tokens":null,"$ai_output_tokens":null,"$ai_latency":10.308,
 "$ai_model":"claude-sonnet-5","$ai_provider":"claude-code",
 "$ai_total_cost_usd":0.44771210000000006, ...}

$ai_total_cost_usd survives because it has no secret-shaped word in it. The token fields do not.

Impact

posthog.ai_events.input_tokens / output_tokens / total_tokens are NULL for every model over the retention window — including claude-sonnet-5 at 2,321 calls and $498.43 of real spend. PostHog's LLM cost views break spend down by token usage, so the ORB shows dollars but no tokens/call, no input-vs-output ratio, and no cost-per-token. $ai_input_cost_usd / $ai_output_cost_usd, which PostHog derives from tokens, could not be computed at all.

It would also have silently defeated the miner-side token split landed in #10199: those events pass through the same scrubber.

Fix

A secret-shaped key holding a number is a counter, not a credential. Every secret this module exists to catch — a token, an API key, a password, a cookie, a DSN, a bearer header — is a string; there is no numeric form of one to leak. Numbers are skipped; a string, object, array or boolean under the same key is still redacted exactly as before.

Deliberately general rather than an allowlist of the two $ai_* keys. An allowlist goes stale the moment PostHog adds $ai_cache_read_input_tokens or the code adds another counter — and it would fail the same silent way this bug did.

Closes #10211

Scope

  • The PR title follows type(scope): short summary Conventional Commit format, for example fix(api): restore profile access checks.
  • This PR is focused and does not mix unrelated backend, UI, MCP, docs, dependency, and deploy changes.
  • This follows CONTRIBUTING.md and does not reintroduce GitHub Pages, VitePress, site/, or CNAME.
  • I linked a currently open issue this PR resolves (e.g. Closes #123) — a linked open issue is required for every contributor PR.

Validation

  • git diff --check
  • npm run actionlint
  • npm run typecheck
  • npm run test:coverage locally; codecov/patch requires ≥99% coverage of the lines AND branches you changed (aim for 100% on your diff so CI variance does not fail near the threshold). Global coverage is a non-blocking trend with a loose 90% backstop, not the gate.
  • npm run test:workers
  • npm run build:mcp
  • npm run test:mcp-pack
  • npm run ui:openapi:check
  • npm run ui:lint
  • npm run ui:typecheck
  • npm run ui:build
  • npm audit --audit-level=moderate
  • New or changed behavior has unit/integration tests for new branches, fallback paths, and sanitizer boundaries

If any required check was skipped, explain why:

  • Every redaction-related suite was run and is green: selfhost-redaction-scrub, selfhost-posthog, redaction, prompt-packet-redaction (264 tests). Coverage measured scoped to the changed file: 100% of the changed lines and branches, verified line-by-line against lcov rather than read off a summary.
  • The regression tests were confirmed to fail with the fix reverted and pass with it.
  • The unchecked commands cover untouched surfaces (no workflow, binding, schema, MCP manifest or UI change here) and are left to CI.

Safety

  • No secrets, wallet details, hotkeys, coldkeys, user PATs, private keys, raw trust scores, private rankings, or private maintainer evidence are exposed.
  • Public GitHub text stays sanitized, low-noise, and does not imply compensation guarantees or optimization tactics.
  • Auth, cookie, CORS, GitHub App, Cloudflare, or session changes include negative-path tests.
  • API/OpenAPI/MCP behavior is updated and tested where needed.
  • UI changes use live API data or real empty/error/loading states, not production mock/demo fallbacks.
  • Visible UI changes include a UI Evidence section below with JPG/JPEG or PNG screenshots arranged as organized, captioned, clickable thumbnails. SVG screenshots are not used as review evidence. Review-only screenshots or recordings are not committed to the repository.
  • Public docs/changelogs are updated where needed; changelogs are only edited for release-prep PRs.

This narrows a redaction rule, so it deserves the scrutiny. The carve-out is numbers only, and three of the four new tests exist specifically to pin the boundary: a string under api_token / password / authorization / session_cookie is still redacted, and so is an object, an array, or a boolean under a secret-shaped key. The number case cannot carry a credential — a bearer token, PAT, JWT, DSN, cookie or password has no numeric representation. Value-based scrubbing (SECRET_VALUE, the JWT and query-string paths) is untouched and still applies to every string regardless of its key.

UI Evidence

Not applicable — no visible UI, frontend, docs, or extension change.

Notes

Found while investigating why PostHog's LLM Analytics Token usage column was empty for every trace. The same investigation confirmed the ORB is emitting the events correctly — cost, latency, model, provider, repo and trace id all arrive intact — so this was the only thing standing between the project and real token metrics.

SECRET_KEY matches /token/i, so scrubRecord -- wired as posthog-node's
before_send -- rewrote PostHog's own $ai_input_tokens and $ai_output_tokens to
the "[redacted]" STRING, which PostHog then coerced to null on its
numerically-typed properties.

The result: not one AI call in the project has ever carried a token count.
posthog.ai_events.input_tokens/output_tokens/total_tokens are NULL for every
model over the retention window, including claude-sonnet-5 at 2,321 calls and
$498.43 of real spend. $ai_total_cost_usd came through untouched because it has
no secret-shaped word in it. PostHog derives $ai_input_cost_usd/$ai_output_cost_usd
from tokens, so those could not be computed either -- and the miner-side split
landed in #10199 would have been scrubbed the same way.

A secret-shaped key holding a NUMBER is a counter, not a credential: every secret
this module exists to catch is a string, and there is no numeric form of one to
leak. Skip redaction for numbers only; a string, object, array or boolean under
the same key is still redacted exactly as before.

Deliberately general rather than an allowlist of the two $ai_* keys -- an
allowlist goes stale the moment PostHog adds $ai_cache_read_input_tokens, and it
would fail the same silent way.

Closes #10211
@loopover-orb

loopover-orb Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Tip

✅ LoopOver review result - approve/merge recommended

Review updated: 2026-07-31 13:02:38 UTC

2 files · 1 AI reviewer · no blockers · readiness 98/100 · CI green · clean

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
This fixes a real bug: SECRET_KEY's /token/i pattern matched PostHog's own numeric $ai_input_tokens/$ai_output_tokens keys, redacting them to the string "[redacted]" which PostHog then coerced to null. The fix adds a `typeof rec[key] !== "number"` guard so numeric values under secret-shaped keys survive, while strings/objects/arrays/booleans are still redacted as before. The change is narrow, well-targeted at the single call site in scrubRecord, and the added tests directly exercise the real regression (token counts surviving) plus confirm string/object/array/boolean secrets are still caught, so the carve-out doesn't overly widen.

Nits — 4 non-blocking
  • The number guard only applies at the flat `shouldRedactKey` branch in scrubRecord (src/selfhost/redaction-scrub.ts); scrubQueryString's `shouldRedactKey(key) ? REDACTED : scrubString(entry)` at line ~178 always redacts regardless of type since query values are always strings, so this is consistent, but worth a one-line confirmation comment since a future reader might wonder why the same guard isn't applied there.
  • The inline comment duplicating the PR description prose directly above the fixed line is quite long (7 lines) for a one-line conditional change; consider trimming to the essential 'numbers aren't secrets' rationale.
  • Consider whether NaN or non-finite numbers (e.g., a secret-shaped key holding `Infinity`) should still be treated as safe — currently `typeof rec[key] !== "number"` treats all numbers as safe regardless of finiteness, which seems fine for this use case but is worth a one-line note if it ever matters.
  • PR author also opened the linked issue — Link an issue that was opened by a different contributor, or provide a rationale for why this self-authored issue represents genuine discovery work.

Decision drivers

  • ✅ Code review — No blockers (1 reviewer)
  • ✅ Gate result — Passing (No configured blocker found.)
Context & advisory signals — never blocks the verdict
Signal Result Evidence
Linked issue ✅ Linked #10211
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 9 registered-repo PR(s), 8 merged, 310 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 9 PR(s), 310 issue(s).
Improvement ✅ Minor risk: clean · value: minor · LLM: moderate
Linked issue satisfaction

Addressed
The diff adds a typeof check skipping redaction for numeric values under secret-shaped keys while still redacting strings, objects, arrays, and booleans, matching the issue's exact fix proposal, and includes a regression test naming $ai_input_tokens/$ai_output_tokens plus a zero-value case.

Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: not available
  • Official Gittensor activity: 9 PR(s), 310 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Start here: Treat this as maintainer-lane context rather than normal contributor-lane activity.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.
🧪 Chat with LoopOver

Ask LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

  • @loopover ask <question> answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat <question> answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @loopover mention with a real question is routed to the closest matching read-only command automatically — no exact syntax required.

Full command reference: https://loopover.ai/docs/loopover-commands

🧪 Experimental — new and may change.

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.

  • Re-run LoopOver review

@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@JSONbored JSONbored self-assigned this Jul 31, 2026
@codecov

codecov Bot commented Jul 31, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.34%. Comparing base (c7645de) to head (fa83b6c).
⚠️ Report is 4 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #10212      +/-   ##
==========================================
- Coverage   92.21%   91.34%   -0.87%     
==========================================
  Files         935      935              
  Lines      114263   114263              
  Branches    27620    27620              
==========================================
- Hits       105371   104377     -994     
- Misses       7588     8777    +1189     
+ Partials     1304     1109     -195     
Flag Coverage Δ
backend 94.13% <100.00%> (-1.55%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/selfhost/redaction-scrub.ts 81.60% <100.00%> (ø)

... and 3 files with indirect coverage changes

@loopover-orb loopover-orb Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LoopOver approves — the gate is satisfied and CI is green.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ai(observability): the redaction scrubber nulls every AI token count — SECRET_KEY matches /token/i

1 participant