amadan
carlos db query: one statement, no pragma changes, temp storage in memory
'PRAGMA query_only=0; CREATE TEMP TABLE stolen AS SELECT * FROM main.users; UPDATE stolen ...' succeeded: mode=ro covers main only, the attach limit does not cover the built-in temp schema, and the driver runs every statement in a list. modernc.org/sqlite v1.55.0 exposes no authorizer and no sqlite3_stmt_readonly, so the guard is ours: - one statement per query, found by a splitter that ignores semicolons inside strings, quoted identifiers and comments; - no pragma given a value, except the read-only schema and check pragmas (table_info, index_list, integrity_check and the like), so query_only, temp_store, temp_store_directory, writable_schema and the rest cannot be changed; - temp_store=MEMORY on open, so a large sort cannot spill plaintext to a file outside the private directory. mode=ro, query_only and the attach limit stay. A single statement cannot loosen them for a second one, because there is no second one.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2 files changed,
+266
−4
db.go+152 −1db_test.go+114 −3
diff --git a/db.go b/db.go| index 48cb325ba..796578588 100644 |
| --- a/db.go |
| +++ b/db.go |
| @@ -300,7 +300,13 @@ func runQuery(w io.Writer, path, sqlText string) error { |
| // tab-separated line per row. A write fails with SQLite's own "attempt to |
| // write a readonly database", returned as it is: the error is the truth. |
| func runQueryContext(ctx context.Context, w io.Writer, path, sqlText string) error { |
| - db, err := sql.Open("sqlite", "file:"+path+"?mode=ro&_pragma=query_only(1)&_pragma=busy_timeout(5000)") |
| + if err := checkQuerySQL(sqlText); err != nil { |
| + return err |
| + } |
| + // temp_store(2): sorts and temp b-trees stay in memory, so a large |
| + // query cannot spill plaintext pages to a file outside the private |
| + // directory. |
| + db, err := sql.Open("sqlite", "file:"+path+"?mode=ro&_pragma=query_only(1)&_pragma=busy_timeout(5000)&_pragma=temp_store(2)") |
| if err != nil { |
| return err |
| } |
| @@ -386,3 +392,148 @@ var dbStopSignals = []os.Signal{os.Interrupt, syscall.SIGTERM, syscall.SIGHUP, s |
| func dbSignalContext() (context.Context, context.CancelFunc) { |
| return signal.NotifyContext(context.Background(), dbStopSignals...) |
| } |
| + |
| +// errOneStatement and errPragmaChange are query's two refusals before |
| +// SQLite sees anything. |
| +var ( |
| + errOneStatement = errors.New("carlos db query runs one statement at a time. Run each statement on its own") |
| + errPragmaChange = errors.New("this pragma changes how the copy is read, so carlos db query does not run it. Reading a pragma's value is fine") |
| +) |
| + |
| +// dbReadPragmas are the pragmas that may take an argument here: each reads |
| +// the schema or checks the file, and none changes the connection. Any |
| +// other pragma may be read bare but not given a value. |
| +var dbReadPragmas = map[string]bool{ |
| + "table_info": true, "table_xinfo": true, "table_list": true, |
| + "index_info": true, "index_xinfo": true, "index_list": true, |
| + "foreign_key_list": true, "foreign_key_check": true, |
| + "integrity_check": true, "quick_check": true, |
| +} |
| + |
| +// checkQuerySQL is the guard modernc.org/sqlite v1.55.0 leaves to its |
| +// caller: it exposes no authorizer and no sqlite3_stmt_readonly, and it runs |
| +// every statement in a list. mode=ro, query_only and the attach limit hold |
| +// for any ONE statement, so the rule is one statement, and no pragma that |
| +// changes the connection (query_only, temp_store, temp_store_directory, |
| +// writable_schema and the rest), since a pragma is the one statement that |
| +// can loosen what follows it. |
| +func checkQuerySQL(q string) error { |
| + first, rest := splitFirstStatement(q) |
| + if strings.TrimSpace(stripSQLComments(rest)) != "" { |
| + return errOneStatement |
| + } |
| + stmt := strings.ToLower(strings.TrimSpace(stripSQLComments(first))) |
| + stmt = strings.TrimSpace(strings.TrimSuffix(stmt, ";")) |
| + if !strings.HasPrefix(stmt, "pragma") || (len(stmt) > 6 && isSQLIdentByte(stmt[6])) { |
| + return nil |
| + } |
| + // PRAGMA [schema.]name [= value | (value)] |
| + body := strings.TrimSpace(stmt[6:]) |
| + n := 0 |
| + for n < len(body) && (isSQLIdentByte(body[n]) || body[n] == '.') { |
| + n++ |
| + } |
| + name := body[:n] |
| + if i := strings.LastIndexByte(name, '.'); i >= 0 { |
| + name = name[i+1:] |
| + } |
| + after := strings.TrimSpace(body[n:]) |
| + if after == "" { |
| + return nil // a bare read of the pragma's value |
| + } |
| + if dbReadPragmas[name] && strings.HasPrefix(after, "(") { |
| + return nil |
| + } |
| + return errPragmaChange |
| +} |
| + |
| +func isSQLIdentByte(c byte) bool { |
| + return c == '_' || c >= 'a' && c <= 'z' || c >= 'A' && c <= 'Z' || c >= '0' && c <= '9' |
| +} |
| + |
| +// splitFirstStatement returns the text up to and including the first |
| +// top-level semicolon, and everything after it. Semicolons inside string |
| +// literals, quoted identifiers and comments do not count. |
| +func splitFirstStatement(q string) (first, rest string) { |
| + for i := 0; i < len(q); i++ { |
| + switch c := q[i]; c { |
| + case '\'', '"', '`': |
| + i = skipQuoted(q, i, c) |
| + case '[': |
| + if j := strings.IndexByte(q[i:], ']'); j >= 0 { |
| + i += j |
| + } else { |
| + i = len(q) |
| + } |
| + case '-': |
| + if i+1 < len(q) && q[i+1] == '-' { |
| + if j := strings.IndexByte(q[i:], '\n'); j >= 0 { |
| + i += j |
| + } else { |
| + i = len(q) |
| + } |
| + } |
| + case '/': |
| + if i+1 < len(q) && q[i+1] == '*' { |
| + if j := strings.Index(q[i+2:], "*/"); j >= 0 { |
| + i += j + 3 |
| + } else { |
| + i = len(q) |
| + } |
| + } |
| + case ';': |
| + return q[:i+1], q[i+1:] |
| + } |
| + } |
| + return q, "" |
| +} |
| + |
| +// skipQuoted returns the index of the quote closing the one at i, where a |
| +// doubled quote is an escaped one, or len(q) when it never closes. |
| +func skipQuoted(q string, i int, quote byte) int { |
| + for j := i + 1; j < len(q); j++ { |
| + if q[j] == quote { |
| + if j+1 < len(q) && q[j+1] == quote { |
| + j++ |
| + continue |
| + } |
| + return j |
| + } |
| + } |
| + return len(q) |
| +} |
| + |
| +// stripSQLComments removes -- and /* */ comments outside quotes, leaving a |
| +// space in their place. |
| +func stripSQLComments(q string) string { |
| + var b strings.Builder |
| + for i := 0; i < len(q); i++ { |
| + c := q[i] |
| + switch { |
| + case c == '\'' || c == '"' || c == '`': |
| + j := skipQuoted(q, i, c) |
| + if j >= len(q) { |
| + j = len(q) - 1 |
| + } |
| + b.WriteString(q[i : j+1]) |
| + i = j |
| + case c == '-' && i+1 < len(q) && q[i+1] == '-': |
| + j := strings.IndexByte(q[i:], '\n') |
| + if j < 0 { |
| + return b.String() |
| + } |
| + b.WriteByte(' ') |
| + i += j |
| + case c == '/' && i+1 < len(q) && q[i+1] == '*': |
| + j := strings.Index(q[i+2:], "*/") |
| + if j < 0 { |
| + return b.String() |
| + } |
| + b.WriteByte(' ') |
| + i += j + 3 |
| + default: |
| + b.WriteByte(c) |
| + } |
| + } |
| + return b.String() |
| +} |
diff --git a/db_test.go b/db_test.go| index 7dbf11c13..f904e1ef3 100644 |
| --- a/db_test.go |
| +++ b/db_test.go |
| @@ -16,6 +16,7 @@ import ( |
| "os/exec" |
| "path/filepath" |
| "runtime" |
| + "slices" |
| "strings" |
| "testing" |
| @@ -75,9 +76,10 @@ func TestQueryRefusesAWriteWithSQLitesOwnError(t *testing.T) { |
| // write refused, and the data unchanged. |
| func TestQueryCannotTurnOffItsOwnGuard(t *testing.T) { |
| path := fixtureDB(t) |
| - err := runQuery(&bytes.Buffer{}, path, "PRAGMA query_only=0; UPDATE users SET name='x'") |
| - if err == nil || !strings.Contains(err.Error(), "attempt to write a readonly database") { |
| - t.Fatalf("error = %v, want SQLite's readonly refusal", err) |
| + for _, stmt := range []string{"PRAGMA query_only=0; UPDATE users SET name='x'", "PRAGMA query_only=0"} { |
| + if err := runQuery(&bytes.Buffer{}, path, stmt); err == nil { |
| + t.Fatalf("%q: succeeded, want a refusal", stmt) |
| + } |
| } |
| var out bytes.Buffer |
| if err := runQuery(&out, path, "SELECT name FROM users ORDER BY id"); err != nil || out.String() != "name\nada\ngrace\n" { |
| @@ -358,3 +360,112 @@ func TestQueryCleansUpWhenStdoutIsClosed(t *testing.T) { |
| t.Fatalf("private directory survived a closed stdout (%v)", err) |
| } |
| } |
| + |
| +// dirEntries lists a directory's names, sorted. |
| +func dirEntries(t *testing.T, dir string) []string { |
| + t.Helper() |
| + ents, err := os.ReadDir(dir) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + var out []string |
| + for _, e := range ents { |
| + out = append(out, e.Name()) |
| + } |
| + return out |
| +} |
| + |
| +// The review's reproduction, verbatim. mode=ro covers main only, the |
| +// attach limit does not cover the built-in temp schema, and the driver runs |
| +// every statement in a list, so switching query_only off first let a temp |
| +// copy of the data be written and changed with no error. It must fail, and |
| +// leave no file anywhere: not beside the copy, not in the temp directory. |
| +func TestQueryRefusesTheTempTableBypass(t *testing.T) { |
| + tmp := t.TempDir() |
| + t.Setenv("TMPDIR", tmp) |
| + path := fixtureDB(t) |
| + before := dirEntries(t, filepath.Dir(path)) |
| + err := runQuery(&bytes.Buffer{}, path, |
| + "PRAGMA query_only=0; CREATE TEMP TABLE stolen AS SELECT * FROM main.users; UPDATE stolen SET name='x'") |
| + if err == nil { |
| + t.Fatal("the temp-table bypass succeeded") |
| + } |
| + if got := dirEntries(t, tmp); len(got) != 0 { |
| + t.Fatalf("temp directory holds %v after the refusal", got) |
| + } |
| + after := dirEntries(t, filepath.Dir(path)) |
| + for _, name := range after { |
| + if !slices.Contains(before, name) && name != "copy.db-wal" && name != "copy.db-shm" { |
| + t.Fatalf("a new file %s appeared beside the copy", name) |
| + } |
| + } |
| +} |
| + |
| +// One statement per query. The guards that matter (mode=ro, query_only) |
| +// hold for a single statement; a list is how one statement turns them off |
| +// for the next. |
| +func TestQueryRunsOneStatementOnly(t *testing.T) { |
| + path := fixtureDB(t) |
| + for _, stmt := range []string{ |
| + "SELECT 1; SELECT 2", |
| + "PRAGMA query_only=0; UPDATE users SET name='x'", |
| + "SELECT 1;\n-- trailing comment\nDELETE FROM users", |
| + "SELECT 1; /* c */ SELECT 2", |
| + } { |
| + if err := runQuery(&bytes.Buffer{}, path, stmt); err == nil || !strings.Contains(err.Error(), "one statement") { |
| + t.Errorf("%q: error = %v, want the one-statement refusal", stmt, err) |
| + } |
| + } |
| + for _, stmt := range []string{ |
| + "SELECT name FROM users;", |
| + "SELECT name FROM users; -- trailing comment", |
| + "SELECT ';' AS semi, \"a;b\" FROM (SELECT 1 AS \"a;b\")", |
| + "SELECT name FROM users /* ; */", |
| + "WITH x AS (SELECT 1) SELECT * FROM x", |
| + } { |
| + if err := runQuery(&bytes.Buffer{}, path, stmt); err != nil { |
| + t.Errorf("%q: error = %v, want it to run", stmt, err) |
| + } |
| + } |
| +} |
| + |
| +// A pragma that takes an argument can change how this connection reads or |
| +// where it spills (query_only, temp_store, temp_store_directory, |
| +// writable_schema...). Only the read-only introspection pragmas may take |
| +// one; bare reads of any pragma are fine. |
| +func TestQueryRefusesPragmaChanges(t *testing.T) { |
| + path := fixtureDB(t) |
| + for _, stmt := range []string{ |
| + "PRAGMA query_only=0", |
| + "PRAGMA query_only = OFF", |
| + "pragma temp_store=FILE", |
| + "PRAGMA temp_store_directory='/tmp/x'", |
| + "PRAGMA main.writable_schema=1", |
| + "PRAGMA journal_mode(DELETE)", |
| + " /* lead */ PRAGMA user_version=9", |
| + } { |
| + if err := runQuery(&bytes.Buffer{}, path, stmt); err == nil || !strings.Contains(err.Error(), "pragma") { |
| + t.Errorf("%q: error = %v, want the pragma refusal", stmt, err) |
| + } |
| + } |
| + for _, stmt := range []string{ |
| + "PRAGMA table_info(users)", |
| + "PRAGMA main.table_info('users')", |
| + "PRAGMA query_only", |
| + "PRAGMA integrity_check", |
| + "SELECT * FROM pragma_table_info('users')", |
| + } { |
| + if err := runQuery(&bytes.Buffer{}, path, stmt); err != nil { |
| + t.Errorf("%q: error = %v, want it to run", stmt, err) |
| + } |
| + } |
| +} |
| + |
| +// Sorts and temp b-trees stay in memory, so a big query cannot spill |
| +// plaintext pages to a file outside the private directory. |
| +func TestQueryKeepsTempStorageInMemory(t *testing.T) { |
| + var out bytes.Buffer |
| + if err := runQuery(&out, fixtureDB(t), "PRAGMA temp_store"); err != nil || out.String() != "temp_store\n2\n" { |
| + t.Fatalf("PRAGMA temp_store = %v %q, want 2 (memory)", err, out.String()) |
| + } |
| +} |