Skip to content

Commit b2eabbf

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 91b0cf3 commit b2eabbf

File tree

3 files changed

+190
-37
lines changed

3 files changed

+190
-37
lines changed

ext/node/ops/sqlite/database.rs

Lines changed: 39 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -273,13 +273,23 @@ fn set_db_config(
273273
}
274274
}
275275

276+
/// Opens a SQLite database connection with appropriate permission checks.
277+
///
278+
/// Performs file-system permission checks via `state`, configures ATTACH
279+
/// restrictions when the caller lacks full permissions for the path, and
280+
/// enables or disables extension loading based on `allow_extension`.
281+
///
282+
/// When `allow_extension` is `true`, only the C API for extension loading is
283+
/// enabled (the SQL `load_extension()` function remains disabled). No FFI
284+
/// permission check is performed here; the check is deferred to `load_extension`
285+
/// where the specific extension path can be validated against scoped permissions.
276286
fn open_db(
277287
state: &mut OpState,
278288
readonly: bool,
279289
location: &str,
280290
allow_extension: bool,
281291
) -> Result<rusqlite::Connection, SqliteError> {
282-
let perms = state.borrow::<PermissionsContainer>();
292+
let perms = state.borrow_mut::<PermissionsContainer>();
283293
let disable_attach = perms
284294
.check_has_all_permissions(Path::new(location))
285295
.is_err();
@@ -295,15 +305,13 @@ fn open_db(
295305
conn.set_limit(Limit::SQLITE_LIMIT_ATTACHED, 0)?;
296306
}
297307

298-
if allow_extension {
299-
perms.check_ffi_all()?;
300-
} else {
301-
assert!(set_db_config(
302-
&conn,
303-
SQLITE_DBCONFIG_ENABLE_LOAD_EXTENSION,
304-
false
305-
));
306-
}
308+
// Enable or disable C API extension loading (SQL function always disabled)
309+
// Permission check deferred to loadExtension() where the specific path is validated
310+
assert!(set_db_config(
311+
&conn,
312+
SQLITE_DBCONFIG_ENABLE_LOAD_EXTENSION,
313+
allow_extension
314+
));
307315

308316
return Ok(conn);
309317
}
@@ -333,30 +341,26 @@ fn open_db(
333341
conn.set_limit(Limit::SQLITE_LIMIT_ATTACHED, 0)?;
334342
}
335343

336-
if allow_extension {
337-
perms.check_ffi_all()?;
338-
} else {
339-
assert!(set_db_config(
340-
&conn,
341-
SQLITE_DBCONFIG_ENABLE_LOAD_EXTENSION,
342-
false
343-
));
344-
}
344+
// Enable or disable C API extension loading (SQL function always disabled)
345+
// Permission check deferred to loadExtension() where the specific path is validated
346+
assert!(set_db_config(
347+
&conn,
348+
SQLITE_DBCONFIG_ENABLE_LOAD_EXTENSION,
349+
allow_extension
350+
));
345351

346352
return Ok(conn);
347353
}
348354

349355
let conn = rusqlite::Connection::open(location)?;
350356

351-
if allow_extension {
352-
perms.check_ffi_all()?;
353-
} else {
354-
assert!(set_db_config(
355-
&conn,
356-
SQLITE_DBCONFIG_ENABLE_LOAD_EXTENSION,
357-
false
358-
));
359-
}
357+
// Enable or disable C API extension loading (SQL function always disabled)
358+
// Permission check deferred to loadExtension() where the specific path is validated
359+
assert!(set_db_config(
360+
&conn,
361+
SQLITE_DBCONFIG_ENABLE_LOAD_EXTENSION,
362+
allow_extension
363+
));
360364

361365
if disable_attach {
362366
conn.set_limit(Limit::SQLITE_LIMIT_ATTACHED, 0)?;
@@ -917,10 +921,11 @@ impl DatabaseSync {
917921
}
918922
}
919923

920-
// Loads a SQLite extension.
924+
// Loads a SQLite extension from the specified path.
921925
//
922-
// This is a wrapper around `sqlite3_load_extension`. It requires FFI permission
923-
// to be granted and allowExtension must be set to true when opening the database.
926+
// This is a wrapper around `sqlite3_load_extension`. It requires:
927+
// - `allowExtension: true` when opening the database (which requires partial FFI permission)
928+
// - FFI permission covering the extension path (e.g., `--allow-ffi=/path/to/extension.so`)
924929
fn load_extension(
925930
&self,
926931
state: &mut OpState,
@@ -939,7 +944,9 @@ impl DatabaseSync {
939944
));
940945
}
941946

942-
state.borrow::<PermissionsContainer>().check_ffi_all()?;
947+
state
948+
.borrow_mut::<PermissionsContainer>()
949+
.check_ffi_partial_with_path(Cow::Borrowed(Path::new(path)))?;
943950

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

tests/sqlite_extension_test/sqlite_extension_test.ts

Lines changed: 150 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ Deno.test({
5757
db.loadExtension(extensionPath);
5858

5959
const stmt = db.prepare("SELECT test_func('Hello, World!') AS result");
60-
const { result } = stmt.get();
60+
const { result } = stmt.get()!;
6161
assertEquals(result, "Hello, World!");
6262

6363
db.close();
@@ -68,12 +68,19 @@ Deno.test({
6868
name: "[node/sqlite] DatabaseSync loadExtension with FFI permission denied",
6969
permissions: { read: true, write: true, ffi: false },
7070
fn() {
71+
// Creating a database with allowExtension: true should succeed
72+
// (permission check deferred to loadExtension)
73+
const db = new DatabaseSync(":memory:", {
74+
allowExtension: true,
75+
readOnly: false,
76+
});
77+
78+
// The error should occur when actually trying to load an extension
7179
assertThrows(() => {
72-
new DatabaseSync(":memory:", {
73-
allowExtension: true,
74-
readOnly: false,
75-
});
80+
db.loadExtension("/some/extension/path");
7681
}, Deno.errors.NotCapable);
82+
83+
db.close();
7784
},
7885
});
7986

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

tests/sqlite_extension_test/tests/integration_tests.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ fn sqlite_extension_test() {
6868
.arg("--allow-read")
6969
.arg("--allow-write")
7070
.arg("--allow-ffi")
71+
.arg("--allow-run")
7172
.arg("--config")
7273
.arg(deno_config_path())
7374
.arg("--no-check")

0 commit comments

Comments
 (0)