Audit scrubber over-redacts any 32+ character token-like string: UUIDs and long identifiers are mangled in audit details

[usmanimamu17-create]

Problem

scrub_details (app/services/scrubber.py) redacts any string of 32+ characters composed of word-ish characters:

LONG_KEY_RE = re.compile(r"^[A-Za-z0-9+/=_\-]{32,}$")
...
elif len(value) >= 32 and LONG_KEY_RE.match(value):
    safe[key] = "[REDACTED_KEY_MATERIAL]"

The pattern matches a 32-character hex string, a 36-character UUID, a long single-word identifier, a base64 blob, or a hyphenated memo — none of which are necessarily secrets.

Consequences:

• Legitimate diagnostic data is destroyed: a details value like {"token_family": "fam_3f2a...64-chars"} or {"job_id": "<36-char-uuid>"} is replaced with [REDACTED_KEY_MATERIAL], so auditors lose exactly the identifiers they need to correlate events.
• The redaction is permanent in the chain: once scrubbed, the original value cannot be recovered from the audit log, and downstream consumers (GDPR export, dispute review) see the placeholder.
• The heuristic is both too aggressive and too weak: it mangles UUIDs/long ids (false positives) while still missing real secrets in nested structures (tracked separately).

Root cause

A length+character-class heuristic was chosen as a catch-all for "key-like" strings without considering the codebase's own long identifiers (UUIDs, token family ids, job ids).

Why this is architecturally hard

1. Distinguishing secrets from long identifiers requires more than length: the checksummed formats (Stellar secret S... 56 chars, ed25519 88 chars) are precise, but the 32+ catch-all exists to cover unknown formats — narrowing it risks leaking genuinely new secret formats.
2. The redaction policy is a security-vs-observability tradeoff that should be configurable per event type; today it is global, so a fix that removes the catch-all affects every audit consumer.
3. Tests must assert both directions: known secret formats are redacted and legitimate long identifiers (UUIDs, fam_/job_/user_ ids) survive.

Proposed design

Tighten LONG_KEY_RE to the formats the codebase actually treats as secret material (or gate it behind an AUDIT_SCRUB_LONG_STRINGS flag defaulting to the precise regexes only), and add tests covering the codebase's own id formats (UUID, fam_*, job_*, atk_*/rtk_* token prefixes are not stored raw).

Acceptance criteria

Service

• The codebase's own long identifiers (UUIDs, prefixed ids) survive scrubbing.
• Known secret formats (Stellar secret, ed25519) remain redacted.

Tests

• Tests cover UUIDs and prefixed ids passing through and real secrets being redacted.
• Existing audit/GDPR tests pass.

Out of scope

Nested-secret recursion (tracked separately) and retention.

Getting started

pytest tests/test_gdpr.py -q
make typecheck

Good first files to read: app/services/scrubber.py, app/models/orm/audit_log.py.

[Paranoa-dev]

I’d like to take this on. I have experience with Python backend services, security-sensitive data handling, and test-driven validation of edge cases where overly broad redaction can create as much operational risk as insufficient redaction.

My approach:

• First inspect app/services/scrubber.py, the audit-log model, and the existing GDPR/audit tests to understand every value currently passed through scrub_details and identify the long identifier formats the application legitimately relies on. I’ll also verify how Stellar secret material and other sensitive credentials are represented before narrowing the heuristic.
• Replace the broad 32+ character catch-all with a more precise secret-material detection strategy based on formats the application actually treats as credentials/keys. UUIDs and known application identifiers such as fam_* and job_* will no longer be classified as key material simply because they are long.
• Preserve redaction for known secret formats, including Stellar secret keys and ed25519/key-like material supported by the application. I’ll avoid weakening the security boundary merely to improve observability, and I’ll document the security/observability tradeoff in the implementation if the policy changes materially.
• Add focused regression tests in both directions: legitimate long identifiers must survive unchanged, while known secret formats must continue to become [REDACTED_KEY_MATERIAL]. I’ll also cover boundary lengths and representative values that previously triggered the overly broad regex to ensure the old false-positive behavior cannot return unnoticed.

I’ll keep nested-secret recursion and retention completely out of scope as requested. The change will remain limited to the scrubber policy and its regression coverage, with no unrelated audit-model or logging refactors.

I’ll validate with pytest tests/test_gdpr.py -q, the relevant broader audit/scrubber tests, and make typecheck. I’ll also run the existing test suite applicable to the touched package to ensure the tighter policy does not break GDPR exports, audit correlation, or other consumers.

The key acceptance checks will be explicit:

• UUIDs remain intact.
• fam_* / job_* identifiers remain intact.
• Other legitimate long identifiers are not automatically destroyed.
• Stellar secret material remains redacted.
• ed25519/key-format secret material remains redacted.
• Existing audit/GDPR behavior remains compatible.

Estimate: I can open the first draft PR within 1–2 days of maintainer assignment.