security(sqlsafety): deny side-effecting functions in read-only vxd db sql (P0-05)

CLAUDE.md

@@ -148,6 +148,7 @@ notify:
 devdb:  # planned — design spec complete, impl in SP1–SP6 PRs
   provider: null  # ghost | docker | null (default: null = disabled)
   template: ""    # source DB to fork from (required when provider != null)
+  function_denylist_extra: []  # extra side-effecting function names `vxd db sql` rejects without --write (adds to built-in pg_terminate_backend/lo_import/... list)
   on_failure:
     keep_db: false
     retain_hours: 24
@@ -219,7 +220,7 @@ dashboard:
 | `vxd db` | Manage ephemeral story databases (devdb). Provider set via `devdb.provider` in vxd.yaml. |
 | `vxd db list` | List all DBs known to the devdb provider |
 | `vxd db connect <db-name>` | Print psql connect command + DSN for a DB (alias: `psql`) |
-| `vxd db sql <db-name> "<query>"` | Run a one-shot SQL query against a DB (read-only by default; pass `--write` to allow INSERT/UPDATE/DELETE/DDL; multi-statement queries are always rejected) |
+| `vxd db sql <db-name> "<query>"` | Run a one-shot SQL query against a DB (read-only by default; pass `--write` to allow INSERT/UPDATE/DELETE/DDL; multi-statement queries are always rejected; known side-effecting functions — `pg_terminate_backend`, `pg_cancel_backend`, `lo_import`, `lo_export`, `pg_read_file`, `pg_ls_dir`, `pg_reload_conf`, `pg_stat_file` — are denied without `--write`, extendable via `devdb.function_denylist_extra`) |
 | `vxd db schema <db-name>` | Print agent-friendly schema dump for a DB |
 | `vxd db delete <db-name>` | Delete a DB permanently (requires `--confirm`) |
 | `vxd db gc` | Run orphan recovery — scan for stale DBs and release old ones |

README.md

@@ -432,9 +432,10 @@ devdb:
   provider: ghost           # or docker, or null
   template: my-prod-snapshot
   ghost: { api_key_env: GHOST_API_KEY }
+  function_denylist_extra: []  # extra function names `vxd db sql` rejects without --write
 ```
 
-Agents read `DATABASE_URL` from `.vxd-db/connect.env` (auto-injected into the worktree). Humans use `vxd db list/connect/logs/delete` plus the dashboard's per-story DB column.
+Agents read `DATABASE_URL` from `.vxd-db/connect.env` (auto-injected into the worktree). Humans use `vxd db list/connect/logs/delete` plus the dashboard's per-story DB column. `vxd db sql` without `--write` statically rejects known side-effecting functions (`pg_terminate_backend`, `pg_cancel_backend`, `lo_import`, `lo_export`, `pg_read_file`, `pg_ls_dir`, `pg_reload_conf`, `pg_stat_file`) that a `READ ONLY` transaction cannot block; extend the list per project with `devdb.function_denylist_extra`.
 
 For runtime usage run `vxd db --help`.
 

internal/cli/db.go

@@ -53,6 +53,19 @@ func dbProviderFor(cmd *cobra.Command) (devdb.Provider, error) {
 	return newDevDBProvider(cfg)
 }
 
+// devdbFunctionDenylistExtra returns the operator-supplied additions to the
+// SQL side-effecting-function denylist. Best-effort: a config-load failure
+// returns nil — the built-in denylist still applies, and the provider load
+// that follows will surface the real error.
+func devdbFunctionDenylistExtra(cmd *cobra.Command) []string {
+	s, err := loadStores(cmd)
+	if err != nil {
+		return nil
+	}
+	defer s.Close()
+	return projectRuntimeConfig(s).DevDB.FunctionDenylistExtra
+}
+
 // findDBByNameOrID looks up a DB from the provider list by name or ID.
 func findDBByNameOrID(dbs []devdb.DB, nameOrID string) (*devdb.DB, error) {
 	for i := range dbs {
@@ -208,15 +221,21 @@ Defense layers (in order):
      transaction and ROLLBACKs at the end, so Postgres itself rejects
      any data-modifying statement that slipped past the static check.
 
-Caveat: a READ ONLY transaction does NOT block side-effecting function
-calls (e.g. pg_terminate_backend, lo_unlink, user functions that write).
-Audit the connection's privileges accordingly.`,
+  4. Known side-effecting functions that READ ONLY does not block
+     (pg_terminate_backend, pg_cancel_backend, lo_import, lo_export,
+     pg_read_file, pg_ls_dir, pg_reload_conf, pg_stat_file) are rejected
+     without --write. Extend the list per project via
+     devdb.function_denylist_extra in vxd.yaml.
+
+Caveat: user-defined functions that write are NOT auto-detected — add
+them to devdb.function_denylist_extra and audit the connection's
+privileges accordingly.`,
 		Args:         cobra.ExactArgs(2),
 		SilenceUsage: true,
 		RunE: func(cmd *cobra.Command, args []string) error {
 			writeFlag, _ := cmd.Flags().GetBool("write")
 			query := args[1]
-			if err := sqlsafety.ValidateSQLForReadOnly(query, writeFlag); err != nil {
+			if err := sqlsafety.ValidateSQL(query, writeFlag, devdbFunctionDenylistExtra(cmd)); err != nil {
 				return err
 			}
 

internal/config/config.go

@@ -419,6 +419,11 @@ type DevDBConfig struct {
 	OnFailure DevDBFailurePolicy `yaml:"on_failure"`
 	Ghost     DevDBGhostConfig   `yaml:"ghost"`
 	Docker    DevDBDockerConfig  `yaml:"docker"`
+	// FunctionDenylistExtra appends operator-supplied function names to the
+	// built-in side-effecting-function denylist enforced by `vxd db sql`
+	// without --write (pg_terminate_backend, lo_import, pg_read_file, …).
+	// Use it to block site-specific functions that write despite READ ONLY.
+	FunctionDenylistExtra []string `yaml:"function_denylist_extra,omitempty"`
 }
 
 // DevDBFailurePolicy controls behaviour when a story finishes with an error.

internal/sqlsafety/sql_safety.go

@@ -2,7 +2,9 @@ package sqlsafety
 
 import (
 	"fmt"
+	"regexp"
 	"strings"
+	"sync"
 )
 
 // QueryKind classifies the leading statement of a SQL query for safety
@@ -29,8 +31,10 @@ const (
 // Even for the keywords below, the read-only path executes inside a
 // `BEGIN READ ONLY ... ROLLBACK` transaction so Postgres enforces the
 // promise at the protocol level. Side-effecting *function calls* (e.g.
-// `SELECT pg_terminate_backend(...)`) are NOT blocked by READ ONLY;
-// the --write flag and SQL parser don't catch those either, and the
+// `SELECT pg_terminate_backend(...)`) are NOT blocked by READ ONLY —
+// the known dangerous ones are rejected statically instead (see
+// defaultFunctionDenylist); anything not on the denylist (user-defined
+// functions that write) remains the operator's responsibility, and the
 // command's Long help calls this out explicitly.
 var readOnlyLeadingKeywords = map[string]struct{}{
 	"SELECT": {},
@@ -39,6 +43,70 @@ var readOnlyLeadingKeywords = map[string]struct{}{
 	"TABLE":  {}, // TABLE foo; is shorthand for SELECT * FROM foo
 }
 
+// defaultFunctionDenylist names Postgres functions with side effects that a
+// READ ONLY transaction does NOT block: session termination, large-object
+// and server-filesystem access, and config reload. Calling any of these from
+// `vxd db sql` without --write is rejected by the static classifier — on a
+// shared devdb host they let one local user disrupt or read another user's
+// session even though the transaction never "writes".
+var defaultFunctionDenylist = []string{
+	"pg_terminate_backend",
+	"pg_cancel_backend",
+	"lo_import",
+	"lo_export",
+	"pg_read_file",
+	"pg_ls_dir",
+	"pg_reload_conf",
+	"pg_stat_file",
+}
+
+// denyPatternCache holds compiled per-function regexes. Patterns are tiny and
+// the set is small; the cache just avoids recompiling on every CLI query.
+var (
+	denyPatternMu    sync.Mutex
+	denyPatternCache = map[string]*regexp.Regexp{}
+)
+
+// denyPattern returns a case-insensitive pattern matching a *call* of fn:
+// the function name as a whole identifier (so `my_pg_terminate_backend(` does
+// not match, while `pg_catalog.pg_terminate_backend (` does) followed by an
+// opening parenthesis with optional whitespace.
+func denyPattern(fn string) *regexp.Regexp {
+	denyPatternMu.Lock()
+	defer denyPatternMu.Unlock()
+	if re, ok := denyPatternCache[fn]; ok {
+		return re
+	}
+	re := regexp.MustCompile(`(?i)(^|[^a-zA-Z0-9_])` + regexp.QuoteMeta(strings.ToLower(fn)) + `[\s]*\(`)
+	denyPatternCache[fn] = re
+	return re
+}
+
+// ContainsDeniedFunction reports the first denied side-effecting function
+// called in the query, checking the built-in denylist plus any extra names
+// supplied by the operator (devdb.function_denylist_extra). Comments and
+// string literals are stripped first, so a `pg_terminate/**/_backend(...)`
+// comment ambush reassembles into the plain call and is caught, while a
+// quoted mention ('pg_terminate_backend') does not false-positive.
+func ContainsDeniedFunction(query string, extra []string) (string, bool) {
+	stripped := stripSQLCommentsAndStrings(query)
+	for _, fn := range defaultFunctionDenylist {
+		if denyPattern(fn).MatchString(stripped) {
+			return fn, true
+		}
+	}
+	for _, fn := range extra {
+		fn = strings.TrimSpace(fn)
+		if fn == "" {
+			continue
+		}
+		if denyPattern(fn).MatchString(stripped) {
+			return fn, true
+		}
+	}
+	return "", false
+}
+
 // IsMultiStatement reports whether the query contains more than one
 // top-level statement separated by `;`. Trailing semicolons are tolerated.
 // Detection is intentionally simple: any `;` followed by non-whitespace,
@@ -77,6 +145,16 @@ func ClassifyQuery(query string) QueryKind {
 // in that case multi-statement is still rejected so a typo can't silently
 // run extra statements, but the leading-keyword check is skipped.
 func ValidateSQLForReadOnly(query string, writeFlag bool) error {
+	return ValidateSQL(query, writeFlag, nil)
+}
+
+// ValidateSQL is ValidateSQLForReadOnly plus the side-effecting-function
+// denylist: in read-only mode (writeFlag=false) a call to any function in
+// defaultFunctionDenylist or extraDeny is rejected, because BEGIN READ ONLY
+// does not stop pg_terminate_backend / lo_import / pg_read_file-class calls.
+// Operators explicitly opting into mutations with --write skip the denylist,
+// same as they skip the leading-keyword check.
+func ValidateSQL(query string, writeFlag bool, extraDeny []string) error {
 	if strings.TrimSpace(query) == "" {
 		return fmt.Errorf("query is empty")
 	}
@@ -86,6 +164,9 @@ func ValidateSQLForReadOnly(query string, writeFlag bool) error {
 	if writeFlag {
 		return nil
 	}
+	if fn, hit := ContainsDeniedFunction(query, extraDeny); hit {
+		return fmt.Errorf("query calls side-effecting function %s(), which READ ONLY transactions do not block; re-run with --write if this is intentional", fn)
+	}
 	switch ClassifyQuery(query) {
 	case QueryReadOnly:
 		return nil

internal/sqlsafety/sql_safety_test.go

@@ -143,3 +143,98 @@ func TestStripSQLCommentsAndStrings(t *testing.T) {
 		}
 	}
 }
+
+// TestSQLSafety_FunctionDenylist pins the side-effecting-function denylist:
+// every built-in denied function is rejected in read-only mode, matching is
+// case-insensitive and whitespace-invariant, comment ambushes reassemble and
+// are caught, and near-miss identifiers do NOT false-positive.
+func TestSQLSafety_FunctionDenylist(t *testing.T) {
+	denied := []struct {
+		name  string
+		query string
+	}{
+		{"pg_terminate_backend", "SELECT pg_terminate_backend(1234)"},
+		{"pg_cancel_backend", "SELECT pg_cancel_backend(1234)"},
+		{"lo_import", "SELECT lo_import('/etc/passwd')"},
+		{"lo_export", "SELECT lo_export(12345, '/tmp/out')"},
+		{"pg_read_file", "SELECT pg_read_file('postgresql.conf')"},
+		{"pg_ls_dir", "SELECT pg_ls_dir('.')"},
+		{"pg_reload_conf", "SELECT pg_reload_conf()"},
+		{"pg_stat_file", "SELECT pg_stat_file('postgresql.conf')"},
+		{"case-insensitive", "SELECT PG_TERMINATE_BACKEND(1)"},
+		{"mixed-case", "SELECT Pg_Terminate_Backend(1)"},
+		{"whitespace before paren", "SELECT pg_terminate_backend   (1)"},
+		{"newline before paren", "SELECT pg_terminate_backend\n(1)"},
+		{"schema-qualified", "SELECT pg_catalog.pg_terminate_backend(1)"},
+		{"comment ambush reassembles", "SELECT pg_terminate/**/_backend(1)"},
+		{"nested in expression", "SELECT 1 WHERE pg_terminate_backend(pid) IS NOT NULL"},
+	}
+	for _, tt := range denied {
+		t.Run("denied/"+tt.name, func(t *testing.T) {
+			err := ValidateSQL(tt.query, false, nil)
+			if err == nil {
+				t.Fatalf("ValidateSQL(%q, false) = nil, want denylist rejection", tt.query)
+			}
+			if !strings.Contains(err.Error(), "side-effecting function") {
+				t.Errorf("error %v does not mention side-effecting function", err)
+			}
+		})
+	}
+
+	allowed := []struct {
+		name  string
+		query string
+	}{
+		{"prefix identifier is a different function", "SELECT my_pg_terminate_backend(1)"},
+		{"column named like function, no call", "SELECT pg_terminate_backend FROM audit_log"},
+		{"quoted string mention", "SELECT * FROM logs WHERE msg = 'pg_terminate_backend(1)'"},
+		{"comment mention", "SELECT 1 -- pg_terminate_backend(1)"},
+		{"plain select", "SELECT id, name FROM users"},
+	}
+	for _, tt := range allowed {
+		t.Run("allowed/"+tt.name, func(t *testing.T) {
+			if err := ValidateSQL(tt.query, false, nil); err != nil {
+				t.Errorf("ValidateSQL(%q, false) = %v, want nil", tt.query, err)
+			}
+		})
+	}
+
+	// --write skips the denylist — the operator explicitly opted in.
+	if err := ValidateSQL("SELECT pg_terminate_backend(1)", true, nil); err != nil {
+		t.Errorf("ValidateSQL with --write = %v, want nil (denylist skipped)", err)
+	}
+}
+
+// TestSQLSafety_DenylistExtraConfig pins the operator-extended denylist
+// (devdb.function_denylist_extra): extra names are denied with the same
+// call-shape matching, and blank entries are ignored.
+func TestSQLSafety_DenylistExtraConfig(t *testing.T) {
+	extra := []string{"dangerous_write_fn", "  audit_purge  ", ""}
+
+	for _, q := range []string{
+		"SELECT dangerous_write_fn(1)",
+		"SELECT DANGEROUS_WRITE_FN (1)",
+		"SELECT audit_purge()",
+	} {
+		if err := ValidateSQL(q, false, extra); err == nil {
+			t.Errorf("ValidateSQL(%q) = nil, want extra-denylist rejection", q)
+		}
+	}
+
+	// Non-listed functions still pass; extra list doesn't over-match.
+	for _, q := range []string{
+		"SELECT count(*) FROM t",
+		"SELECT not_dangerous_write_fn2(1)",
+		"SELECT dangerous_write_fn_v2(1)",
+	} {
+		if err := ValidateSQL(q, false, extra); err != nil {
+			t.Errorf("ValidateSQL(%q) = %v, want nil", q, err)
+		}
+	}
+
+	// ContainsDeniedFunction reports which function fired.
+	fn, hit := ContainsDeniedFunction("SELECT audit_purge()", extra)
+	if !hit || fn != "audit_purge" {
+		t.Errorf("ContainsDeniedFunction = (%q, %v), want (audit_purge, true)", fn, hit)
+	}
+}