Skip to content

Commit b8d7571

Browse files
tianzhouclaude
andauthored
fix(postgres): deny set_config() in readonly mode (#448) (#464)
Readonly mode rejects `SET statement_timeout = 0`, but `SELECT set_config('statement_timeout', '0', false)` passed the access policy and made the same change. The setting is session-scoped, so it stuck on the pooled connection and later calls ran without the server-side deadline that query_timeout applies as statement_timeout. search_path could be rewritten the same way. Add set_config to the PostgreSQL escape-hatch list so it classifies as admin, like the other SELECT-invocable functions a read-only transaction does not contain. Matching is call-position only, so current_setting() and columns named set_config are unaffected. Co-authored-by: Claude Fable 5.1 <[email protected]>
1 parent b893b83 commit b8d7571

3 files changed

Lines changed: 24 additions & 2 deletions

File tree

‎src/utils/__tests__/allowed-keywords.test.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -311,6 +311,10 @@ describe("isReadOnlySQL", () => {
311311
["postgres", "SELECT pg_read_file('/etc/passwd')"],
312312
["postgres", "SELECT pg_read_binary_file('server.key')"],
313313
["postgres", "SELECT pg_ls_dir('/var/lib/postgresql')"],
314+
// set_config is SET in function form; the session-scoped variant undoes
315+
// the statement_timeout / search_path applied at connect (issue #448).
316+
["postgres", "SELECT set_config('statement_timeout', '0', false)"],
317+
["postgres", "SELECT pg_catalog.set_config('search_path', 'pg_catalog,public', false)"],
314318
// SQL Server pass-through sources share the same call-position guard:
315319
// OPENQUERY's payload runs on the remote server, OPENROWSET(BULK) reads
316320
// server-side files, OPENDATASOURCE opens ad-hoc connections.
@@ -344,6 +348,11 @@ describe("isReadOnlySQL", () => {
344348
expect(isReadOnlySQL("SELECT load_file FROM documents", "mysql")).toBe(true);
345349
expect(isReadOnlySQL("SELECT count(*) AS get_lock FROM t", "mysql")).toBe(true);
346350
expect(isReadOnlySQL("SELECT pg_read_file FROM audit", "postgres")).toBe(true);
351+
expect(isReadOnlySQL("SELECT set_config FROM audit", "postgres")).toBe(true);
352+
});
353+
354+
it("still allows reading settings through current_setting (issue #448)", () => {
355+
expect(isReadOnlySQL("SELECT current_setting('statement_timeout')", "postgres")).toBe(true);
347356
});
348357

349358
it("does not apply another dialect's escape-hatch list", () => {

‎src/utils/__tests__/sql-access-policy.test.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,15 @@ describe("classifyStatement", () => {
4848
expect(classifyStatement("SELECT get_lock('x', 10)", "mariadb")).toBe("admin");
4949
expect(classifyStatement("SELECT pg_read_file('/etc/passwd')", "postgres")).toBe("admin");
5050
});
51+
52+
it("classifies set_config as admin so readonly denies it like SET (issue #448)", () => {
53+
expect(classifyStatement("SELECT set_config('statement_timeout', '0', false)", "postgres")).toBe("admin");
54+
expect(classifyStatement("SELECT current_setting('statement_timeout')", "postgres")).toBe("read");
55+
const readonly = policyFromReadonly(true);
56+
expect(sqlVerdict(readonly, "SET statement_timeout = 0", "postgres")).toBe("deny");
57+
expect(sqlVerdict(readonly, "SELECT set_config('statement_timeout', '0', false)", "postgres")).toBe("deny");
58+
expect(sqlVerdict(readonly, "SELECT current_setting('statement_timeout')", "postgres")).toBe("allow");
59+
});
5160
});
5261

5362
describe("classifySQL (multi-statement)", () => {

‎src/utils/allowed-keywords.ts‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,11 @@ export const sqlServerPassThroughPattern = new RegExp(
125125
* advisory locks (a session side effect).
126126
* - PostgreSQL: pg_read_file / pg_read_binary_file / pg_ls_dir read the
127127
* server filesystem (pg_read_server_files role; default_transaction_read_only
128-
* does not apply).
128+
* does not apply). set_config is SET in function form: it rewrites session
129+
* state such as the statement_timeout DBHub applies from query_timeout or
130+
* the configured search_path, and the change outlives the statement on the
131+
* pooled connection (issue #448). current_setting is read-only and stays
132+
* allowed.
129133
* - SQL Server: OPENQUERY / OPENROWSET / OPENDATASOURCE run on a
130134
* remote/ad-hoc source outside the local read-only transaction
131135
* (sqlServerPassThroughKeywords, reused here as the single source of truth).
@@ -148,7 +152,7 @@ export const sqlServerPassThroughPattern = new RegExp(
148152
export const escapeHatchFunctionKeywords: Partial<Record<ConnectorType, readonly string[]>> = {
149153
mysql: ["load_file", "get_lock", "release_lock", "release_all_locks"],
150154
mariadb: ["load_file", "get_lock", "release_lock", "release_all_locks"],
151-
postgres: ["pg_read_file", "pg_read_binary_file", "pg_ls_dir"],
155+
postgres: ["pg_read_file", "pg_read_binary_file", "pg_ls_dir", "set_config"],
152156
sqlserver: sqlServerPassThroughKeywords,
153157
// Oracle packages a SELECT can call to reach outside the database, matched
154158
// as `package.member(` (see escapeHatchCallSuffix).

0 commit comments

Comments
 (0)