Skip to content

workload-replay: harden workload anonymization#36745

Closed
jasonhernandez wants to merge 1 commit into
mainfrom
workload-anonymize-hardening
Closed

workload-replay: harden workload anonymization#36745
jasonhernandez wants to merge 1 commit into
mainfrom
workload-anonymize-hardening

Conversation

@jasonhernandez
Copy link
Copy Markdown
Contributor

Motivation

bin/mz-workload-anonymize scrubs identifiers and string literals from workload captures so they can be shared. While looking at it I found several gaps where sensitive data leaks through or the output is silently corrupted. This PR fixes the cheap, high-value ones and adds a fail-loud verification backstop. It deliberately does not attempt the parser-level rewrite (see "Out of scope").

What changed

Privacy leaks

  • Connections and sinks now have their literals anonymized. Previously their create_sql only had identifiers replaced, never literals — so hostnames, usernames, regions, broker lists, bucket/path URLs, and topic names were emitted verbatim. Types and indexes also get literal anonymization now, for consistency.

Correctness / robustness

  • ReDoS fix: the string-literal regex '(?:[^']*(?:'')?)*' had a nested quantifier (catastrophic-backtracking shape). Rewrote as the linear '(?:[^']|'')*'.
  • KeyError fix: child sources looked up child["schema"]/["database"] directly in the mapping, crashing on any builtin/uncaptured name. Now uses .get, matching the query path.

Footgun

  • No more silent overwrite: output defaulted to overwriting the input capture. Now requires an explicit -o (- for stdout) or the new --in-place flag.

Fail-loud verification (--verify, on by default)

  • After anonymizing, re-scans the output for whole-word survivals of original identifiers and for string literals not reduced to a literal_N placeholder, and refuses to write if any are found. --no-verify overrides.
  • Cluster create_sql is exempt from the literal check: its literals (SIZE, replication factor, AZs) are non-sensitive config that replay must preserve.

Out of scope (parser-level, tracked by existing TODOs)

These require resolving SQL with the Mz parser instead of text substitution, and are left for follow-up:

  • Case folding and word-boundary matching in the identifier regex.
  • The global mapping collapsing duplicate column/table names across schemas (distinct objects can map to one name).
  • Dollar-quoted strings, SQL comments, and numeric literals are still not anonymized.

The --verify pass is meant as the safety net for this residual risk: anything it can detect becomes a hard error rather than a silent leak.

Testing

  • bin/fmt clean; ruff check clean; py_compile clean.
  • Manual runs on a synthetic capture: confirmed connection broker/username, sink topic, table defaults, and query predicate literals are scrubbed; confirmed the no-output-target error, --in-place, --no-verify bypass, and that --verify catches an intentionally-leaked literal.

🤖 Generated with Claude Code

The anonymizer scrubs identifiers and string literals from workload
captures so they can be shared. Several gaps let sensitive data leak
through or silently corrupted the output:

- Connection and sink create_sql only had identifiers replaced, never
  literals. Those statements carry hostnames, usernames, regions, broker
  lists, bucket/path URLs, and topic names as string literals, all of
  which were emitted verbatim. Also anonymize literals for types and
  indexes for consistency.
- The string-literal regex used a nested quantifier
  (`'(?:[^']*(?:'')?)*'`), a catastrophic-backtracking shape. Rewrote it
  as the linear `'(?:[^']|'')*'`.
- Child sources looked up `child["schema"]`/`["database"]` directly in
  the mapping, crashing with KeyError on any builtin/uncaptured name.
  Use `.get` so unmapped names pass through, matching the query path.
- Output defaulted to overwriting the input capture, destroying the
  original. Now require an explicit `-o` (use `-` for stdout) or the new
  `--in-place` flag.

Add a `--verify` pass (on by default) that re-scans the anonymized
output for whole-word survivals of original identifiers and for string
literals that were not reduced to a `literal_N` placeholder, and refuses
to write if any are found. Cluster create_sql is exempt from the literal
check because its literals (SIZE, replication factor, AZs) are
non-sensitive config that replay must preserve. This converts silent
leaks into a hard failure, which is the property that matters for a
privacy tool.

This does not address the deeper, parser-level issues (case folding,
word boundaries in the identifier regex, the global mapping collapsing
duplicate column/table names, dollar-quoted strings, comments, numeric
literals); those require resolving SQL with the Mz parser rather than
text substitution, per the existing TODOs.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@jasonhernandez
Copy link
Copy Markdown
Contributor Author

Superseded by #36803, which squashes this stack into a single PR against main.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant