Skip to content

feat(enrichment): secrets-in-logs & PII-egress scanner - #1614

Merged
JSONbored merged 1 commit into
JSONbored:mainfrom
oktofeesh1:feat/enrichment-secret-log-scanner
Jun 28, 2026
Merged

feat(enrichment): secrets-in-logs & PII-egress scanner#1614
JSONbored merged 1 commit into
JSONbored:mainfrom
oktofeesh1:feat/enrichment-secret-log-scanner

Conversation

@oktofeesh1

Copy link
Copy Markdown
Contributor

Closes #1507.

Adds a REES analyzer that flags added lines passing sensitive data into a logging or stdout sinkconsole.log(req.headers.authorization), logger.info(\token=${apiKey}`), console.log(req)`. This is distinct from the shipped hardcoded-secret scan: that inspects literal values; this inspects the data flow into a sink (a secret reaching a log is a leak even when the value isn't a literal). It's exactly the kind of pass the no-checkout in-prompt reviewer can't reliably do.

Approach

Pure compute, no network, no new dependency (a structural detector, not a bundled Semgrep). Precision-first so it doesn't generate noise:

  • A linear codeOnly() hand-scan blanks string-literal messages (keeping ${…} interpolation bodies, which are real code) before matching — so console.log("password reset") is not flagged.
  • A hit requires a sensitive name used as code: a property access (.authorization), a ${…} interpolation (${apiKey}), or a dumped request/session object (req, req.headers/body/cookies/session). Innocuous request scalars (req.method/url/path) are excluded.
  • All matchers are flat, linear-time regexes (no nested-quantifier shapes) — the analyzer can't be the DoS class it sits beside.
  • Reports location + sink + category only; never the logged value. Line-cited via hunk headers; findings bounded.

Files (all inside review-enrichment/, per the established analyzer pattern)

  • src/types.tsSecretLogFinding type + secretLog BriefFindings key
  • src/analyzers/secret-log.ts — the analyzer (codeOnly + detectSecretLog + scanSecretLog)
  • src/brief.ts — registered in ANALYZERS
  • src/render.ts — public-safe brief block
  • test/enrichment.test.tscodeOnly, classifier (incl. false-positive guards), line-cited scan, render, and a buildBrief integration test

Validation

  • npm test (build + node --test) inside review-enrichment/: 50/50 pass (6 new).
  • git diff --check clean; prettier-clean; engine src/** untouched (outside the engine tsc/vitest/codecov scope — zero conflict).

@oktofeesh1
oktofeesh1 requested a review from JSONbored as a code owner June 27, 2026 18:30
@dosubot dosubot Bot added the size:L label Jun 27, 2026
@superagent-security

Copy link
Copy Markdown
Contributor

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

@loopover-orb

loopover-orb Bot commented Jun 27, 2026

Copy link
Copy Markdown
Contributor

Tip

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

✅ Gittensory review — safe to merge

5 files · 1 AI reviewers · no blockers · readiness 55/100 · CI green · clean

✅ Approved — safe to merge

Review summary
Adds a secrets-in-logs / PII-egress analyzer that correctly distinguishes sensitive-value data flow into sinks from string literals that merely mention sensitive words. The `codeOnly` linear pass is well-implemented — the escape-aware single/double-quoted branch and the depth-tracking `${…}` interpolation keeper are both correct. Hunk-header line-citation matches the established pattern; the `ANALYZERS` registry wiring, `BriefFindings` key, and `renderBrief` block are all consistent with prior analyzers. No blocking defects found.

Signal Result Evidence
Code review ✅ No blockers 1 reviewers, synthesized
Linked issue ✅ Linked #1507
Related work ⚠️ 3 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:L.
Validation evidence ❌ 5/25 Cached preflight status is hold.
Open PR queue ❌ 3/10 33 open PR(s), 17 likely reviewable, 16 unlinked.
Contributor context ✅ Confirmed Gittensor contributor oktofeesh1; Gittensor profile; 668 PR(s), 7 issue(s).
Gate result ✅ Passing No configured blocker found.
Nits — 5 non-blocking
  • secret-log.ts: The second arm of `REQUEST_OBJECT_RE` — `\b(?:headers|session|cookies)\s*(?:\)|\.[\s\S]*?[\w$])` — flags ANY property access on a standalone `headers`/`session`/`cookies` variable (e.g., `logger.info(headers.contentType)` or `logger.info(cookies.preferredLanguage)`), not only dumps of the full object. The PR's benign-field exclusion logic covers only `req.method`/`req.url`/`req.path`, leaving the second arm notably broader than documented; add a comment acknowledging this false-positive surface or tighten the pattern to enumerate safe sub-properties the same way the `req.*` arm does.
  • enrichment.test.ts: The `process.stdout.write(session.cookie)` assertion only checks `.sink`, leaving the `.category` return path (which resolves to `request-object` via the second `REQUEST_OBJECT_RE` arm) entirely unverified — add `assert.equal(...?.category, 'request-object')` on the same line.
  • secret-log.ts (scanPatchForSecretLog): The `body.length > MAX_LINE_CHARS` skip branch has no test — add a test patch line of >2000 chars and assert it produces no finding.
  • enrichment.test.ts (scanSecretLog): The 25-finding cap (`MAX_FINDINGS`) in `scanSecretLog` is untested — the existing test sends exactly one finding. Add a test that drives 26+ findings across files and asserts exactly 25 are returned.
  • secret-log.ts: Added diff lines that are inline comments (`// console.log(req.headers.authorization)`) will be flagged since no comment-prefix filter is applied before `SINK_RE`; this is a known limitation of line-level structural scanning, but it is undocumented — add a comment or a TODO so future contributors don't mistake it for an oversight.
Review context
Contributor next steps
  • Review top overlaps.
  • Add scope summary.
  • Fix blocker.
  • Expect slower review.
  • Refresh registry data or choose a registered active repo.
  • 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.

Adds a secrets-in-logs / PII-egress analyzer that correctly distinguishes sensitive-value data flow into sinks from string literals that merely mention sensitive words. The `codeOnly` linear pass is well-implemented — the escape-aware single/double-quoted branch and the depth-tracking `${…}` interpolation keeper are both correct. Hunk-header line-citation matches the established pattern; the `ANALYZERS` registry wiring, `BriefFindings` key, and `renderBrief` block are all consistent with prior analyzers. No blocking defects found.

Nits (5)

  • secret-log.ts: The second arm of `REQUEST_OBJECT_RE` — `\b(?:headers|session|cookies)\s*(?:\)|\.[\s\S]*?[\w$])` — flags ANY property access on a standalone `headers`/`session`/`cookies` variable (e.g., `logger.info(headers.contentType)` or `logger.info(cookies.preferredLanguage)`), not only dumps of the full object. The PR's benign-field exclusion logic covers only `req.method`/`req.url`/`req.path`, leaving the second arm notably broader than documented; add a comment acknowledging this false-positive surface or tighten the pattern to enumerate safe sub-properties the same way the `req.*` arm does.
  • enrichment.test.ts: The `process.stdout.write(session.cookie)` assertion only checks `.sink`, leaving the `.category` return path (which resolves to `request-object` via the second `REQUEST_OBJECT_RE` arm) entirely unverified — add `assert.equal(...?.category, 'request-object')` on the same line.
  • secret-log.ts (scanPatchForSecretLog): The `body.length > MAX_LINE_CHARS` skip branch has no test — add a test patch line of >2000 chars and assert it produces no finding.
  • enrichment.test.ts (scanSecretLog): The 25-finding cap (`MAX_FINDINGS`) in `scanSecretLog` is untested — the existing test sends exactly one finding. Add a test that drives 26+ findings across files and asserts exactly 25 are returned.
  • secret-log.ts: Added diff lines that are inline comments (`// console.log(req.headers.authorization)`) will be flagged since no comment-prefix filter is applied before `SINK_RE`; this is a known limitation of line-level structural scanning, but it is undocumented — add a comment or a TODO so future contributors don't mistake it for an oversight.

🟩 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

@loopover-orb loopover-orb Bot added gittensor gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier. labels Jun 27, 2026
Adds a REES analyzer that flags added lines passing sensitive data into a
logging or stdout sink — console.log(req.headers.authorization),
logger.info(`token=${apiKey}`), console.log(req) — distinct from the
hardcoded-secret scan (which inspects literal values; this inspects the data
flow into a sink). Pure compute, no network.

Precision-first: string-literal messages are blanked by a linear hand-scan
before matching, so console.log("password reset") is not flagged; only a
sensitive name used as code (property access, a ${…} interpolation, or a dumped
request/session object) fires. Innocuous request scalars (req.method/url/path)
are excluded. All matchers are flat, linear-time regexes.

Closes JSONbored#1507.
@oktofeesh1
oktofeesh1 force-pushed the feat/enrichment-secret-log-scanner branch from 68cf7b4 to b73f860 Compare June 28, 2026 04:48
@dosubot dosubot Bot added the lgtm label Jun 28, 2026
@JSONbored
JSONbored merged commit b587948 into JSONbored:main Jun 28, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(enrichment): Secrets-in-logs & PII-egress scanner

2 participants