feat: enforce a read-only policy per database role - #3
Merged
Conversation
…nd EXPLAIN bypasses
…ed escape hatches
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Turns the always-allow handler into a real policy. A YAML file maps each database role to an agent and can mark it read-only; the proxy resolves the policy to a per-session handler after the handshake and refuses disallowed statements before they reach the server. Without
-policy, every statement is still allowed.What is in here
internal/sqlscan: a lexical statement classifier. It tokenizes far enough to skip strings, line and block comments, dollar-quoted bodies, and quoted identifiers, then classifies each top-level statement by its leading keyword. It is deliberately conservative for read-only: writes hidden inEXPLAIN ANALYZE, data-modifying CTEs (WITH d AS (DELETE ...)), andCOPY ... FROMare treated as mutating, and an unrecognized statement fails closed.internal/policy: loads the YAML file (fail: open|closed,roleskeyed by database user withagent,purpose,read_only), and implementswire.Guard. A read-only role denies any mutating statement with anErrorResponsewritten for an LLM to act on. Unlisted roles are allowed under fail-open and denied under fail-closed. The zero-value policy allows everything.internal/wire: adds theGuardinterface (Resolve(Startup) Handler) andAllowAll.internal/proxy: resolves the guard to a per-session handler once the handshake yields the client's identity.internal/cli: a-policy PATHflag.Dependency
Adds
gopkg.in/yaml.v3— the first dependency, for the policy file. Chosen over JSON so the file can carry comments and read naturally.Testing
make testandmake lintpass.internal/sqlscan: classification of each statement kind, keywords hidden in literals/identifiers/comments/dollar-quotes, data-modifying CTEs,EXPLAINwith and withoutANALYZE, andCOPYdirection.internal/policy: parsing (including rejection of unknown fields and badfailvalues), read-only denial and its message, writable roles, and fail-open vs fail-closed for unlisted roles.postgres:16: a read-only role'sSELECTreaches the database whileUPDATE,EXPLAIN ANALYZE DELETE, andWITH ... DELETEare refused before it, leaving the data unchanged; a writable role'sUPDATEsucceeds.Not in this PR
Table/column allowlists and row limits (they need column-level parsing), purpose and session from SQL-comment tags,
writes: approval, and the access ledger.Review round: hardened read-only enforcement
A review found that lexical read-only classification alone is bypassable. Addressed:
internal/sqlscan): line comments now end at\ras well as\n;$inside identifiers no longer starts a false dollar-quote;COPY ... FROM ... WITH (...)andCOPY ... TOa file or program are classified as writes (onlyCOPY ... TO STDOUTreads);EXPLAIN ANALYZE CREATE TABLE ASandSELECT ... INTOare classified as writes. The tokenizer now keeps quoted identifiers distinct so a quoted GUC name is comparable.SET default_transaction_read_only = on, so the server refuses writes made through functions (nextval,setval, volatile user functions) that no lexical layer can see. Verified againstpostgres:16:SELECT setval(...)on a read-only role is refused by the server.SET/RESETof the read-only GUCs (quoted or not),SET TRANSACTION READ WRITE,BEGIN/START ... READ WRITE,RESET ALL, andset_config(...).wire.Guardnow returns anEnforcement{Prime, Handler};wire.SessiongainsPrime, run after the handshake and before the relay.Documented residual: a role that already holds write privileges is not fully sandboxed by a lexical proxy; the strongest read-only control is a database role granted only
SELECT. rollcall prevents accidental and obvious writes, hardens against function writes via the server, and blocks the common escape hatches.