Skip to content

Commit 3b40989

Browse files
feat(ext/node): support path-scoped FFI for SQLite extension loading
Previously, using `allowExtension: true` or calling `loadExtension()` required unrestricted `--allow-ffi` permission. This made it impossible to sandbox code that needs to load only specific, pre-approved SQLite extensions. This change allows scoped FFI permissions: - `allowExtension: true` no longer runs an up-front / connection-time check (the `partial` check function did not return true as expected in practice when only scoped FFI permissions exist) - `loadExtension(path)` requires FFI permission covering that specific path Example: `--allow-ffi=/path/to/extension.so` now permits loading only that extension, rather than granting unrestricted FFI access. Note that this now universally disables the SQL `load_extension()` function, whether or not FFI is globally enabled. Fixes: #31426
1 parent dd80d75 commit 3b40989

File tree

3 files changed

+190
-36
lines changed

3 files changed

+190
-36
lines changed

ext/node/ops/sqlite/database.rs

Lines changed: 38 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -440,6 +440,16 @@ fn set_db_config(
440440
}
441441
}
442442

443+
/// Opens a SQLite database connection with appropriate permission checks.
444+
///
445+
/// Performs file-system permission checks via `state`, configures ATTACH
446+
/// restrictions when the caller lacks full permissions for the path, and
447+
/// enables or disables extension loading based on `allow_extension`.
448+
///
449+
/// When `allow_extension` is `true`, only the C API for extension loading is
450+
/// enabled (the SQL `load_extension()` function remains disabled). No FFI
451+
/// permission check is performed here; the check is deferred to `load_extension`
452+
/// where the specific extension path can be validated against scoped permissions.
443453
fn open_db(
444454
state: &mut OpState,
445455
readonly: bool,
@@ -462,15 +472,13 @@ fn open_db(
462472
conn.set_limit(Limit::SQLITE_LIMIT_ATTACHED, 0)?;
463473
}
464474

465-
if allow_extension {
466-
perms.check_ffi_all()?;
467-
} else {
468-
assert!(set_db_config(
469-
&conn,
470-
SQLITE_DBCONFIG_ENABLE_LOAD_EXTENSION,
471-
false
472-
));
473-
}
475+
// Enable or disable C API extension loading (SQL function always disabled)
476+
// Permission check deferred to loadExtension() where the specific path is validated
477+
assert!(set_db_config(
478+
&conn,
479+
SQLITE_DBCONFIG_ENABLE_LOAD_EXTENSION,
480+
allow_extension
481+
));
474482

475483
return Ok(conn);
476484
}
@@ -500,30 +508,26 @@ fn open_db(
500508
conn.set_limit(Limit::SQLITE_LIMIT_ATTACHED, 0)?;
501509
}
502510

503-
if allow_extension {
504-
perms.check_ffi_all()?;
505-
} else {
506-
assert!(set_db_config(
507-
&conn,
508-
SQLITE_DBCONFIG_ENABLE_LOAD_EXTENSION,
509-
false
510-
));
511-
}
511+
// Enable or disable C API extension loading (SQL function always disabled)
512+
// Permission check deferred to loadExtension() where the specific path is validated
513+
assert!(set_db_config(
514+
&conn,
515+
SQLITE_DBCONFIG_ENABLE_LOAD_EXTENSION,
516+
allow_extension
517+
));
512518

513519
return Ok(conn);
514520
}
515521

516522
let conn = rusqlite::Connection::open(location)?;
517523

518-
if allow_extension {
519-
perms.check_ffi_all()?;
520-
} else {
521-
assert!(set_db_config(
522-
&conn,
523-
SQLITE_DBCONFIG_ENABLE_LOAD_EXTENSION,
524-
false
525-
));
526-
}
524+
// Enable or disable C API extension loading (SQL function always disabled)
525+
// Permission check deferred to loadExtension() where the specific path is validated
526+
assert!(set_db_config(
527+
&conn,
528+
SQLITE_DBCONFIG_ENABLE_LOAD_EXTENSION,
529+
allow_extension
530+
));
527531

528532
if disable_attach {
529533
conn.set_limit(Limit::SQLITE_LIMIT_ATTACHED, 0)?;
@@ -1084,10 +1088,11 @@ impl DatabaseSync {
10841088
}
10851089
}
10861090

1087-
// Loads a SQLite extension.
1091+
// Loads a SQLite extension from the specified path.
10881092
//
1089-
// This is a wrapper around `sqlite3_load_extension`. It requires FFI permission
1090-
// to be granted and allowExtension must be set to true when opening the database.
1093+
// This is a wrapper around `sqlite3_load_extension`. It requires:
1094+
// - `allowExtension: true` when opening the database (which requires partial FFI permission)
1095+
// - FFI permission covering the extension path (e.g., `--allow-ffi=/path/to/extension.so`)
10911096
fn load_extension(
10921097
&self,
10931098
state: &mut OpState,
@@ -1106,7 +1111,9 @@ impl DatabaseSync {
11061111
));
11071112
}
11081113

1109-
state.borrow::<PermissionsContainer>().check_ffi_all()?;
1114+
state
1115+
.borrow_mut::<PermissionsContainer>()
1116+
.check_ffi_partial_with_path(Cow::Borrowed(Path::new(path)))?;
11101117

11111118
// SAFETY: lifetime of the connection is guaranteed by reference counting.
11121119
let raw_handle = unsafe { db.handle() };

tests/sqlite_extension_test/sqlite_extension_test.ts

Lines changed: 151 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ Deno.test({
4949
db.loadExtension(extensionPath);
5050

5151
const stmt = db.prepare("SELECT test_func('Hello, World!') AS result");
52-
const { result } = stmt.get();
52+
const { result } = stmt.get()!;
5353
assertEquals(result, "Hello, World!");
5454

5555
db.close();
@@ -60,12 +60,19 @@ Deno.test({
6060
name: "[node/sqlite] DatabaseSync loadExtension with FFI permission denied",
6161
permissions: { read: true, write: true, ffi: false },
6262
fn() {
63+
// Creating a database with allowExtension: true should succeed
64+
// (permission check deferred to loadExtension)
65+
const db = new DatabaseSync(":memory:", {
66+
allowExtension: true,
67+
readOnly: false,
68+
});
69+
70+
// The error should occur when actually trying to load an extension
6371
assertThrows(() => {
64-
new DatabaseSync(":memory:", {
65-
allowExtension: true,
66-
readOnly: false,
67-
});
72+
db.loadExtension("/some/extension/path");
6873
}, Deno.errors.NotCapable);
74+
75+
db.close();
6976
},
7077
});
7178

@@ -86,3 +93,142 @@ Deno.test({
8693
db.close();
8794
},
8895
});
96+
97+
// Tests for scoped FFI permissions (--allow-ffi=/path/to/extension)
98+
// These require subprocess spawning since Deno.test permissions don't support scoped FFI
99+
100+
Deno.test({
101+
name: "[node/sqlite] DatabaseSync with scoped FFI permission succeeds",
102+
ignore: !extensionExists,
103+
permissions: { read: true, run: true },
104+
async fn() {
105+
const code = `
106+
import { DatabaseSync } from "node:sqlite";
107+
const extensionPath = Deno.args[0];
108+
const db = new DatabaseSync(":memory:", { allowExtension: true });
109+
db.loadExtension(extensionPath);
110+
const stmt = db.prepare("SELECT test_func('test') AS result");
111+
const { result } = stmt.get()!;
112+
if (result !== "test") throw new Error("Unexpected result: " + result);
113+
db.close();
114+
console.log("OK");
115+
`;
116+
117+
const command = new Deno.Command(Deno.execPath(), {
118+
args: [
119+
"run",
120+
`--allow-read=${extensionPath}`,
121+
`--allow-ffi=${extensionPath}`,
122+
"--no-lock",
123+
"-",
124+
extensionPath,
125+
],
126+
stdin: "piped",
127+
stdout: "piped",
128+
stderr: "piped",
129+
});
130+
131+
const child = command.spawn();
132+
const writer = child.stdin.getWriter();
133+
await writer.write(new TextEncoder().encode(code));
134+
await writer.close();
135+
136+
const { code: exitCode, stdout, stderr } = await child.output();
137+
const stdoutText = new TextDecoder().decode(stdout);
138+
const stderrText = new TextDecoder().decode(stderr);
139+
140+
assertEquals(exitCode, 0, `Expected success but got: ${stderrText}`);
141+
assertEquals(stdoutText.trim(), "OK");
142+
},
143+
});
144+
145+
Deno.test({
146+
name:
147+
"[node/sqlite] DatabaseSync loadExtension fails for path outside scoped FFI",
148+
ignore: !extensionExists,
149+
permissions: { read: true, run: true },
150+
async fn() {
151+
// Grant FFI only for a different path, not the actual extension
152+
const wrongPath = "/some/other/path";
153+
154+
const code = `
155+
import { DatabaseSync } from "node:sqlite";
156+
const extensionPath = Deno.args[0];
157+
const db = new DatabaseSync(":memory:", { allowExtension: true });
158+
try {
159+
db.loadExtension(extensionPath);
160+
console.log("UNEXPECTED_SUCCESS");
161+
} catch (e) {
162+
if (e instanceof Deno.errors.NotCapable) {
163+
console.log("EXPECTED_PERMISSION_ERROR");
164+
} else {
165+
console.log("UNEXPECTED_ERROR: " + e.constructor.name + ": " + e.message);
166+
}
167+
}
168+
db.close();
169+
`;
170+
171+
const command = new Deno.Command(Deno.execPath(), {
172+
args: [
173+
"run",
174+
`--allow-read=${extensionPath}`,
175+
`--allow-ffi=${wrongPath}`,
176+
"--no-lock",
177+
"-",
178+
extensionPath,
179+
],
180+
stdin: "piped",
181+
stdout: "piped",
182+
stderr: "piped",
183+
});
184+
185+
const child = command.spawn();
186+
const writer = child.stdin.getWriter();
187+
await writer.write(new TextEncoder().encode(code));
188+
await writer.close();
189+
190+
const { stdout } = await child.output();
191+
const stdoutText = new TextDecoder().decode(stdout);
192+
193+
assertEquals(
194+
stdoutText.trim(),
195+
"EXPECTED_PERMISSION_ERROR",
196+
`Expected NotCapable error but got: ${stdoutText}`,
197+
);
198+
},
199+
});
200+
201+
Deno.test({
202+
name:
203+
"[node/sqlite] SQL load_extension() is disabled even with allowExtension: true",
204+
ignore: !extensionExists,
205+
permissions: { read: true, write: true, ffi: true },
206+
fn() {
207+
// Even with allowExtension: true and full FFI permissions,
208+
// the SQL function load_extension() should be disabled.
209+
// Only the C API loadExtension() method should work.
210+
const db = new DatabaseSync(":memory:", {
211+
allowExtension: true,
212+
readOnly: false,
213+
});
214+
215+
// Attempting to load extension via SQL should fail with "not authorized",
216+
// even though the same extension loads successfully via C API
217+
const loadExtStmt = db.prepare("SELECT load_extension($path)");
218+
assertThrows(
219+
() => {
220+
loadExtStmt.get({ $path: extensionPath });
221+
},
222+
Error,
223+
"not authorized",
224+
);
225+
226+
// Verify the C API still works with the same extension
227+
db.loadExtension(extensionPath);
228+
const stmt = db.prepare("SELECT test_func('works') AS result");
229+
const { result } = stmt.get()!;
230+
assertEquals(result, "works");
231+
232+
db.close();
233+
},
234+
});

tests/sqlite_extension_test/tests/integration_tests.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ fn sqlite_extension_test() {
5050
.arg("--allow-read")
5151
.arg("--allow-write")
5252
.arg("--allow-ffi")
53+
.arg("--allow-run")
5354
.arg("--config")
5455
.arg(deno_config_path())
5556
.arg("--no-check")

0 commit comments

Comments
 (0)