Skip to content

fix(conn): scrub SQL comment bodies in ScrubQueryText - #56

Merged
alexshapalov merged 1 commit into
pgrundev:mainfrom
lofoneh:fix/scrub-sql-comments
Sep 21, 2026
Merged

alexshapalov merged 1 commit into
pgrundev:mainfrom
lofoneh:fix/scrub-sql-comments

Conversation

@lofoneh

@lofoneh lofoneh commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

What and why

Closes #54. ScrubQueryText left comment bodies untouched, and pg_stat_statements
keeps comments verbatim even while normalizing literals:

before: SELECT pg_sleep($1) /* api_key=sk_live_PLANTED123 customer=…
after:  SELECT pg_sleep($1) /* ? */

A small lexer replaces each comment body with -- ? / /* ? */ before the existing
steps. A lexer, not a regex: -- inside a string isn't a comment, and ' inside a
comment isn't a string. Block comments nest; an unterminated one over-redacts.

Verification

  • Table tests for the issue cases plus the traps (-- in a literal or quoted
    identifier, don't in a comment, $1 placeholders, nesting).
  • Fuzzer invariant: whatever a comment holds, only the marker survives (300k+ execs).
  • PG18 end to end: planted secret appears twice in inspect --json on main, zero here.
  • scripts/gate.sh green.

Checklist

  • scripts/gate.sh passes
  • No PII enters a model.Context / --json / the store — this closes a hole
  • Remaining items n/a.

Comments are free text — an API key, a token, a customer name — and match none
of the literal-shape regexes, so they passed through untouched.
pg_stat_statements keeps them verbatim even while normalizing literals to $N,
so a tagged query reached --json, MCP, the HTML report and the AI prompt.

A small lexer now replaces each comment body with `-- ?` or `/* ? */` before
the existing steps. It is a lexer rather than a regex because `--` inside a
string or quoted identifier is not a comment, and an apostrophe inside a
comment is not a string. Block comments nest; an unterminated comment runs to
the end of the text, over-redacting.

The fuzzer gains the invariant that whatever a comment holds, only the marker
survives.

Closes pgrundev#54
@alexshapalov

Copy link
Copy Markdown
Contributor

@lofoneh great!!

@alexshapalov
alexshapalov merged commit a3479a2 into pgrundev:main Sep 21, 2026
19 checks passed
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.

ScrubQueryText leaves sql comments unchanged which may leak sensitive information

2 participants