Skip to content

fix(observability): redact Sentry log summaries - #1662

Merged
JSONbored merged 2 commits into
mainfrom
codex/propose-fix-for-sentry-log-vulnerability
Jun 29, 2026
Merged

fix(observability): redact Sentry log summaries#1662
JSONbored merged 2 commits into
mainfrom
codex/propose-fix-for-sentry-log-vulnerability

Conversation

@JSONbored

@JSONbored JSONbored commented Jun 28, 2026

Copy link
Copy Markdown
Owner

Summary

No issue because this is maintainer-side confidentiality hardening for structured Sentry log forwarding. Field-only error logs should keep useful context in the issue title without promoting token/password/key-like fields into Sentry titles or notifications.

What changed

  • Skips top-level secret-keyed summary fields before building field-only Sentry titles.
  • Recursively redacts secret-keyed nested object and array values before serializing object fields into summaries.
  • Redacts overly deep summary subtrees instead of serializing beyond the recursion cap.
  • Adds regression coverage for top-level scalar secrets, nested object/array secrets, and deeply nested secret-bearing objects.

Validation

  • npx vitest run test/unit/selfhost-sentry.test.ts
  • npm run typecheck
  • git diff --check
  • npm audit --audit-level=moderate
  • npm run test:ci

@codecov

codecov Bot commented Jun 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.58%. Comparing base (a39a340) to head (6323086).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1662   +/-   ##
=======================================
  Coverage   95.58%   95.58%           
=======================================
  Files         204      204           
  Lines       22295    22303    +8     
  Branches     8053     8059    +6     
=======================================
+ Hits        21310    21318    +8     
  Misses        408      408           
  Partials      577      577           
Files with missing lines Coverage Δ
src/selfhost/sentry.ts 100.00% <100.00%> (ø)
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@loopover-orb

loopover-orb Bot commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

Tip

🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩

✅ Gittensory review — safe to merge

2 files · 1 AI reviewers · no blockers · readiness 75/100 · CI green · clean

✅ Approved — safe to merge

Review summary
The change narrows field-only Sentry title summaries by skipping secret-keyed top-level fields and redacting secret-keyed nested values before JSON serialization. The implementation is correct for the described title-summary path, and the added tests cover scalar, nested array/object, and depth-cap behavior through the production forwarding function. The most notable remaining risk is that explicit `message`/`error` strings are still promoted unchanged, but that is outside the field-summary path this PR changes.

Signal Result Evidence
Code review ✅ No blockers 1 reviewers, synthesized
Linked issue ✅ No-issue rationale PR body explains why no issue is linked.
Related work ⚠️ 2 scoped overlaps Top overlaps are listed below; lower-confidence bulk is hidden.
Review load ❌ 8/20 Readiness component derived from cached public PR metadata and labels; size label size:S.
Validation evidence ✅ 25/25 PR body includes validation/test evidence.
Open PR queue ❌ 3/10 20 open PR(s), 13 likely reviewable, 7 unlinked.
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 80 PR(s), 298 issue(s).
Gate result ✅ Passing No configured blocker found.
Nits — 4 non-blocking
  • nit: src/selfhost/sentry.ts:146 uses a separate depth rule from scrubEvent's `depth > 6`, so keep an eye on the off-by-one semantic difference if these redaction paths are expected to match exactly.
  • nit: test/unit/selfhost-sentry.test.ts:327 validates nested redaction, but it does not include a safe nested field beyond the depth cap, so the intentional loss of deep non-secret context is only indirectly documented.
  • src/selfhost/sentry.ts:146: Consider extracting the shared depth limit into a named constant so `scrubEvent` and `redactSummaryValue` do not drift independently.
  • test/unit/selfhost-sentry.test.ts:327: Add one assertion or fixture comment making it explicit that the depth cap redacts the whole subtree, including non-secret fields, because that is the privacy tradeoff this helper is making.
Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository registration is not available in the local Gittensory cache.
  • Public profile languages: not available
  • Official Gittensor activity: 80 PR(s), 298 issue(s).
  • Related work: Titles/paths share 6 meaningful terms. (PR #1679)
  • Related work: Titles/paths share 8 meaningful terms. (PR #1679, PR #1690)
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • Review top overlaps.
  • Add scope summary.
  • Expect slower review.
  • No action.
  • Link the issue being solved, or explicitly explain why this is a no-issue PR.
  • Check active issues and PRs before submitting.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Review load = cached public PR metadata such as size labels, changed paths, and preflight status.
  • Open PR queue = repo-wide review pressure; it is not a PR quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.
Review details

Generated from public PR metadata and the diff. Advisory only; deterministic signals remain authoritative.

The change narrows field-only Sentry title summaries by skipping secret-keyed top-level fields and redacting secret-keyed nested values before JSON serialization. The implementation is correct for the described title-summary path, and the added tests cover scalar, nested array/object, and depth-cap behavior through the production forwarding function. The most notable remaining risk is that explicit `message`/`error` strings are still promoted unchanged, but that is outside the field-summary path this PR changes.

Nits (4)

  • nit: src/selfhost/sentry.ts:146 uses a separate depth rule from scrubEvent's `depth > 6`, so keep an eye on the off-by-one semantic difference if these redaction paths are expected to match exactly.
  • nit: test/unit/selfhost-sentry.test.ts:327 validates nested redaction, but it does not include a safe nested field beyond the depth cap, so the intentional loss of deep non-secret context is only indirectly documented.
  • src/selfhost/sentry.ts:146: Consider extracting the shared depth limit into a named constant so `scrubEvent` and `redactSummaryValue` do not drift independently.
  • test/unit/selfhost-sentry.test.ts:327: Add one assertion or fixture comment making it explicit that the depth cap redacts the whole subtree, including non-secret fields, because that is the privacy tradeoff this helper is making.

🟩 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 Gittensory, a quiet PR intelligence layer for OSS maintainers.

  • Re-run Gittensory review

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jun 28, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
gittensory-ui 6323086 Commit Preview URL

Branch Preview URL
Jun 28 2026, 09:50 PM

@JSONbored
JSONbored merged commit 6fd79eb into main Jun 29, 2026
21 checks passed
@JSONbored
JSONbored deleted the codex/propose-fix-for-sentry-log-vulnerability branch June 29, 2026 01:01
@github-project-automation github-project-automation Bot moved this from Todo to Done in gittensory - v1 roadmap Jun 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier.

Projects

No open projects
Status: Done

1 participant