Skip to content

fix(sanitize): redact secrets inside escaped JSON - #134

Merged
M3gA-Mind merged 1 commit into
tinyhumansai:mainfrom
M3gA-Mind:fix/scrub-escaped-json
Oct 23, 2026
Merged

M3gA-Mind merged 1 commit into
tinyhumansai:mainfrom
M3gA-Mind:fix/scrub-escaped-json

Conversation

@M3gA-Mind

Copy link
Copy Markdown
Collaborator

Summary

What was wrong. scrub_credentials did not redact a labelled secret inside stringified JSON. In {"body":"{\"password\":\"hunter2secret\"}"} the key is followed by \", but the key/value regex only allowed an optional " or ' right after the key. A JSON body embedded as a string (common in tool results and provider error payloads) therefore came back with the password in clear.

Fix. One regex change, in the same key/value regex:

  • The key suffix is now (?:\\?["'])?, so an escaped closing quote after the key is accepted.
  • A new value alternative matches a value quoted with \"…\".
  • The capture groups are now named (op, dq, sq, edq, bare), and a value is "quoted" whenever it is not the bare alternative. The rest of the redaction logic is unchanged.

Known limit: a value that itself contains an escaped quote (\\\") inside stringified JSON is redacted only up to that quote.

Related issue

None.

API or behavior changes

No public API change. Labelled secrets inside escaped/stringified JSON are now redacted (first four characters kept, as for other quoted values).

Validation

  • N/A: cargo fmt --all -- --check. I ran rustfmt (edition 2024) on the touched files only; CI runs the full check.
  • N/A: cargo clippy --all-targets --all-features -- -D warnings. Not run locally; CI runs it.
  • N/A: cargo build --all-targets --all-features. Not run locally; CI runs it.
  • N/A: cargo test --all-features. I ran the sanitize module tests; CI runs the full suite.

Tests

Added secret_in_escaped_json_is_redacted to sanitize_tests.rs.

  • Before the fix: FAILED, leaked: {"body":"{\"password\":\"hunter2secret\"}"}
  • After the fix: PASSED.

cargo test -p tinyinference-core --lib sanitize: 35 passed, 0 failed.

Documentation

No docs change. The regex doc comment lists the new groups and the known limit.

Checklist

  • The change is focused on one logical change
  • No new #[allow(...)], #[ignore], or relaxed lints
  • No secrets, tokens, or .env contents in the diff or the description

@tinysweeper

tinysweeper Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

Tiny Sweeper review

Reviewed head: 73baa1911250e300ce84107e10cd813c0a73abe5

Verdict: Ready for maintainer review · Priority: none
Coverage: Reviewed 2 of 2 files.
Pull request purpose (title): fix(sanitize): redac…

Diagnostics and supporting details

Full summary

Pull request purpose (title): fix(sanitize): redact secrets inside escaped JSON. Tiny Sweeper completed its review; deterministic results follow.

Review snapshot

Change surface Files Review signal Count
Production 1 Active findings 0
Tests 1 Noted findings 0
Documentation 0 Resolved findings 0
Configuration 0 Pending checks/questions 0

Completeness: Complete
Test assessment: No supported feature-to-test mapping was available; this does not mean tests are absent or passed.

What changed

The credential scrubber now recognizes labelled secrets embedded in escaped/stringified JSON: the key suffix optionally accepts an escaped quote, a new value branch matches `\"…\"`-quoted values, and the regex captures are named. The callback reads `op`, `bare`, and the value through `dq`/`sq`/`edq`/`bare`, with `quoted` determined by whether the bare group matched; the rest of the redaction logic is described as unchanged. A test was added for the escaped-JSON case.

Features

  • Modified — Escaped-JSON value redaction in SENSITIVE_KV_REGEX: Labelled secrets whose value sits inside stringified JSON (escaped quotes) are now matched and redacted instead of leaking through; the key suffix also accepts an escaped closing quote. The PR description notes a known limit where a value containing its own escaped quote is redacted only up to that quote. (crates/tinyinference-core/src/sanitize.rs pub fn scrub_credentials(input: &str) -> String {)
  • Internal refactor — Positional captures replaced with named groups: The replacement closure now reads `op`, `bare`, `dq`, `sq`, and `edq` by name, and determines quoted-ness from whether `bare` matched. Security lane reports the rewrite is internally consistent and value span arithmetic still operates on the full match; this is an internal readability change with no stated public API change. (crates/tinyinference-core/src/sanitize.rs pub fn scrub_credentials(input: &str) -> String {, crates/tinyinference-core/src/sanitize.rs pub fn scrub_secret_patterns(input: &str) -> String {)

Tests

No supported feature-to-test mapping was produced. Test execution is not inferred.

Findings

No active actionable findings.

Before merge

None.

Agent review details

critique

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: Critique lane is Answered; it retained findings only at capped Likely confidence because retrieval was degraded, and separately noted truncation at an escaped backslash pair leaving the tail redacted and the idempotence guard being skipped for escaped-quote values.
  • Lane summary: Critique findings were capped at Likely confidence because retrieval was degraded: Truncate edq at escaped backslash pair, leaving the tail redacted; Skip the idempotence guard for escaped-quote values. (2 model findings retained at no more than Likely confidence because retrieval was degraded) _The code index is behind this pull request (indexed at `50e6d1ce19bd`), so retrieved context may be out of date._

security

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: Security lane is Answered and reports the regex rewrite is internally consistent (named captures consistently replace numbered ones, quoted-ness derived from `bare`, value span arithmetic unchanged), that the new `edq` alternative closes a real redaction gap for stringified JSON, and that no attacker-controlled path or auth change is visible.
  • Lane summary: The regex rewrite is internally consistent — the numbered captures are consistently replaced by named groups (`op`, `dq`, `sq`, `edq`, `bare`), quoted-ness is derived from `bare`, and the value span arithmetic still operates on the full match. The new `edq` alternative closes a real redaction gap for stringified JSON, and the added test asserts the secret is gone. No attacker-controlled path, auth change, or rule violation is visible in this diff. _The code index is behind this pull request (indexed at `50e6d1ce19bd`), so retrieved context may be out of date._

tests

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: Tests lane is Answered and states the change broadens redaction rather than narrowing it, and that the added test fails if the escaped-JSON value stops being redacted.
  • Lane summary: The change adds an `edq` alternative to the key/value credential regex so labelled secrets nested inside stringified JSON (escaped quotes) are redacted, switches the capture lookups to named groups, and adds a test that fails if `hunter2secret` leaks. The effect is broader redaction, not less, and the added test fails if the escaped-JSON value stops being redacted; I found no defect worth blocking on. _The code index is behind this pull request (indexed at `50e6d1ce19bd`), so retrieved context may be out of date._

commits

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: Commits lane is Answered and reports no sensitive content in committed files.
  • Lane summary: No sensitive content in committed files. _The code index is behind this pull request (indexed at `50e6d1ce19bd`), so retrieved context may be out of date._

description

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: Description lane is Answered and states the PR body accurately matches the diff: key suffix change, new escaped-quote alternative, named capture groups, quoted determination, and the stated known limitation.
  • Lane summary: The PR description accurately describes the regex change and the new test, and everything cited in the body matches the diff: the key suffix change, the new escaped-quote alternative, the named capture groups, the `quoted` determination, and the stated known limitation. I found no inaccurate or missing claims in the description, and I did not review the implementation itself for defects under this lane. _The code index is behind this pull request (indexed at `50e6d1ce19bd`), so retrieved context may be out of date._
Evidence and run details
  • Models: ladder/vectors-oai3, deepseek-v4.1-flash
  • Spend: $0.000564
  • Tokens: 200338 input · 4413 output · 33280 cached · 136 embedding
  • Updated: 2026-10-22T22:54:41Z
Head State Pass summary
73baa1911250 ready for maintainer review 0 active finding(s), 0 resolved finding(s) (at 2026-10-22T22:54:41Z)

tinysweeper 0.1.2

@coderabbitai

coderabbitai Bot commented Oct 22, 2026

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: a2a21e94-9d1f-4bb0-afde-fcab408dace4
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@tinysweeper tinysweeper Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

tinysweeper found nothing blocking. Approving.

Diagnostics

Total spend: $0.0006

@M3gA-Mind
M3gA-Mind merged commit eb346f1 into tinyhumansai:main Oct 23, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant