Skip to content

Over-redaction splits a query's fingerprint, silently under-counting N+1 #64

Description

@KARTIKrocks

Problem

analyzer.Redact resolves the backslash-escape dialect ambiguity by scanning
under both readings and redacting the union (see PR #61). That is the right
fail-safe direction for a leak, but it has a consequence beyond readability:
an over-redacted query gets a different fingerprint.

Fingerprint(`SELECT * FROM t WHERE p = 'C:\' AND id = 5`)
// → "SELECT * FROM t WHERE p = ?"

Fingerprint(`SELECT * FROM t WHERE p = 'plain' AND id = 5`)
// → "SELECT * FROM t WHERE p = ? AND id = ?"

Fingerprint is the N+1 grouping key (middleware/n_plus_one.go) and the
dedup key (middleware/guard.go). So one logical query whose values only
sometimes contain a backslash — Windows paths, LIKE ... ESCAPE patterns,
regexes — groups under two fingerprints instead of one.

Impact

  • N+1 detection can silently miss. A 20-iteration loop can land as 12 + 8
    and fall under a threshold of 20 on both, reporting nothing.
  • De-duplication emits the same finding twice per window, once per group.
  • Metric-label cardinality grows with a value-dependent property, which is
    exactly what Fingerprint is documented to avoid.

This is a false negative in a detector, which is the failure mode the repo
treats as worse than a hard error elsewhere (see the MySQL EXPLAIN
fail-closed behaviour in 0.1.1).

Current state

Documented, not fixed — website/docs/redaction.md ("When the dialect is
ambiguous, it over-redacts") and the ## [Unreleased] CHANGELOG entry both
call it out, with bind parameters as the workaround. AGENTS.md records it
as the accepted cost of the union scan.

Possible directions

  • Narrow the union to the divergent tail, so the spans agree wherever the two
    readings agree and only the genuinely ambiguous region widens.
  • Derive the fingerprint from the narrow reading while Result.Query keeps
    the union. Needs care: the fingerprint must stay PII-free, so this only
    works if the narrow reading provably leaks nothing into the folded form.
  • Accept it and make it observable instead — e.g. mark a Result whose
    redaction was ambiguous so operators can spot skewed grouping.

Acceptance criteria

  • Two executions of the same query shape share a fingerprint whether or not a
    value contains a backslash, or the limitation is deliberately kept with
    a documented, tested rationale.
  • No regression in TestRedactNoLeakAcrossDialectAmbiguity — no literal byte
    may survive Redact or Fingerprint under either dialect reading.
  • N+1 grouping behaviour covered by a test at the middleware level.

References: #61

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions