Repository navigation
fix(tinyredact): redact cards next to other digits and quoted or prefixed credentials - #308
Conversation
…ixed credentials A separated card run that failed Luhn as a whole (a card followed by a CVV or preceded by a quantity) was dropped, because the greedy card regex cannot give digits back. Such a run is now retried on windows of its digit groups; a window must pass Luhn and be corroborated. The credential patterns allowed no quote between the key and its separator, so JSON-quoted credentials in prose survived, and their leading word boundary never fired after an underscore, so DB_PASSWORD=, *_SECRET= and *_TOKEN= survived. Both now match; the trailing word boundary still keeps token_count, max_tokens and tokenizer clean.
|
Warning Review limit reached
This review includes 5 billable files and costs up to $1.25. Or wait 26 minutes for your next included review. View limit details
Comment |
Tiny Sweeper reviewReviewed head: Verdict: Ready for maintainer review · Priority: none Diagnostics and supporting detailsFull summaryPull request purpose (title): fix(tinyredact): redact cards next to other digits and quoted or prefixed credentials. Tiny Sweeper completed its review; deterministic results follow. Review snapshot
Completeness: Complete What changedCard detection now routes every `CC_RE` match through a new `card_span` helper. A run that passes Luhn is still taken as the card (under LuhnOnly, when separated, or when a bare run is corroborated). A separated run that fails Luhn as a whole is retried on windows of its contiguous digit groups, 13-19 digits, longest first then leftmost, and a window must pass Luhn and be corroborated by `plausible_card_number` or `cc_keyword_near` under either gate. Bare runs are a single group and are never split, so the epoch-millisecond protection is unchanged. Credential patterns in `patterns.rs` now accept an optional quote between the key and its separator for `api_key`, the `password`/`secret`/`token` group, and the OAuth-names group, and the broad credential pattern starts with `(?:\b|_)` while keeping the trailing `\b`. The description notes two accepted side effects: `has_likely_secret` will also refuse lookalikes such as `is_secret: false`, and a prefix match starts at the `_`, so `DB_PASSWORD=x` renders as `DB[REDACTED]`. Not included: a `secret_access_key` arm and the fail-closed `dedupe_overlaps` merge. Features
Tests
FindingsNo active actionable findings. Before mergeNone. Agent review detailscritique
security
tests
commits
description
Evidence and run details
|
Summary
Three content-redaction gaps let real credentials through
redact_pii*/sanitize_text*, under both card policies.Card next to other digits.
CC_REis greedy, socard 4111 1111 1111 1111 123matches all 19 digits. That run fails Luhn, andfind_iternever retries a shorter one, so the card survived. A leading number (qty 2 4111 1111 1111 1111) fails the same way. The regex crate has no lookaround to fix this in the pattern.plausible_card_number) or a card keyword nearby (cc_keyword_near). This applies under either gate, because splitting a run multiplies the chance of a Luhn pass by luck.card_spanhelper. TheBareCardGatedoc now describes the retry.JSON-quoted credentials in prose. The
api_keypattern and thepassword/secret/tokenpattern allowed no quote between the key and its separator. As a result,{"password": "hunter2", "api_key":"abcd1234"}inside text was left as is. The OAuth-names pattern had the same gap. All three now accept an optional quote there.DB_PASSWORD=/*_SECRET=/*_TOKEN=. The leading\bnever fires after_, so prefixed environment-style assignments survived. The pattern now starts with(?:\b|_). The trailing\bis kept on purpose: it is what leavestoken_count=5,max_tokens: 10,tokenizer=x,password_reset_sent: trueandsecret_santa=…alone.Related issue
None. Found in an audit for tinyhumansai/openhuman.
API or behavior changes
No API change. Behaviour only ever adds redactions:
_-prefixed credential assignments are redacted. The prefix match starts at the_, soDB_PASSWORD=xbecomesDB[REDACTED]. That is cosmetic; the value is gone.has_likely_secret, which openhuman uses to refuse flow-memory writes. The intended new refusal is{"token":"abc"}. Reading the regex, it will also refuse lookalikes whose value follows a credential word directly, e.g.is_secret: false,has_password: true,max_token=5. Those false positives are accepted rather than weakening the pattern. Making the match depend on the value is the follow-up if they show up in practice.secret_access_key(AWS_SECRET_ACCESS_KEY=,STRIPE_SECRET_KEY=), and the fail-closed merge of partially overlapping hits indedupe_overlaps. Both were left out to keep this change to the three reported gaps.Validation
cargo fmt --all -- --checkwas not run;rustfmt --edition 2024was run on each touched file. CI runs the full check.cargo clippy --all-targets --all-features -- -D warningswas not run locally; CI runs it.cargo build --all-targets --all-featureswas not run locally; CI runs it.cargo test --all-featureswas not run locally. Only the six tests below were run, in one targeted invocation; CI runs the full suite.cargo test -p tinyredact --lib -- <the six tests below>:pii/mod.rsandsanitize/patterns.rsreset tomain(tests kept): FAILED, 2 passed (the two guards), 4 failed:credit_card_followed_by_space_and_digits_redacted:card survivedunder both policies;credit_card_after_a_leading_number_is_redacted_and_the_number_kept:card survived: "qty 2 4111 1111 1111 1111";sanitize_text_redacts_json_quoted_credentials_in_prose:hunter2andabcd1234survived under both policies;sanitize_text_redacts_prefixed_env_style_credentials: all three values survived under both policies.Tests
a_separated_digit_list_with_no_card_is_left_alone:ids 123 1000 2000 1000 0002. The whole run fails Luhn, and its only Luhn-valid window has no network IIN and no keyword, so it stays unredacted under both gates.sanitize_text_leaves_credential_lookalike_names_alone:token_count,max_tokens,tokenizer,password_reset_sentandsecret_santaare unchanged.Documentation
The
BareCardGatedoc and thecard_spandoc describe the window retry. No external docs cover these patterns.Checklist
#[allow(...)],#[ignore], or relaxed lints.envcontents in the diff or the description