Skip to content

Commit 69cfa6b

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` now requires partial FFI permission (any scope) - `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. Fixes: #31426
1 parent faf9505 commit 69cfa6b

File tree

3 files changed

+137
-8
lines changed

3 files changed

+137
-8
lines changed

ext/node/ops/sqlite/database.rs

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -273,13 +273,22 @@ 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`, partial FFI permission is required
283+
/// (any `--allow-ffi` scope suffices). The actual extension path is checked
284+
/// later when `load_extension` is called.
276285
fn open_db(
277286
state: &mut OpState,
278287
readonly: bool,
279288
location: &str,
280289
allow_extension: bool,
281290
) -> Result<rusqlite::Connection, SqliteError> {
282-
let perms = state.borrow::<PermissionsContainer>();
291+
let perms = state.borrow_mut::<PermissionsContainer>();
283292
let disable_attach = perms
284293
.check_has_all_permissions(Path::new(location))
285294
.is_err();
@@ -296,7 +305,7 @@ fn open_db(
296305
}
297306

298307
if allow_extension {
299-
perms.check_ffi_all()?;
308+
perms.check_ffi_partial_no_path()?;
300309
} else {
301310
assert!(set_db_config(
302311
&conn,
@@ -334,7 +343,7 @@ fn open_db(
334343
}
335344

336345
if allow_extension {
337-
perms.check_ffi_all()?;
346+
perms.check_ffi_partial_no_path()?;
338347
} else {
339348
assert!(set_db_config(
340349
&conn,
@@ -349,7 +358,7 @@ fn open_db(
349358
let conn = rusqlite::Connection::open(location)?;
350359

351360
if allow_extension {
352-
perms.check_ffi_all()?;
361+
perms.check_ffi_partial_no_path()?;
353362
} else {
354363
assert!(set_db_config(
355364
&conn,
@@ -917,10 +926,11 @@ impl DatabaseSync {
917926
}
918927
}
919928

920-
// Loads a SQLite extension.
929+
// Loads a SQLite extension from the specified path.
921930
//
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.
931+
// This is a wrapper around `sqlite3_load_extension`. It requires:
932+
// - `allowExtension: true` when opening the database (which requires partial FFI permission)
933+
// - FFI permission covering the extension path (e.g., `--allow-ffi=/path/to/extension.so`)
924934
fn load_extension(
925935
&self,
926936
state: &mut OpState,
@@ -939,7 +949,9 @@ impl DatabaseSync {
939949
));
940950
}
941951

942-
state.borrow::<PermissionsContainer>().check_ffi_all()?;
952+
state
953+
.borrow_mut::<PermissionsContainer>()
954+
.check_ffi_partial_with_path(Cow::Borrowed(Path::new(path)))?;
943955

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

tests/sqlite_extension_test/sqlite_extension_test.ts

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,3 +86,119 @@ Deno.test({
8686
db.close();
8787
},
8888
});
89+
90+
// Tests for scoped FFI permissions (--allow-ffi=/path/to/extension)
91+
// These require subprocess spawning since Deno.test permissions don't support scoped FFI
92+
93+
Deno.test({
94+
name: "[node/sqlite] DatabaseSync with scoped FFI permission succeeds",
95+
permissions: { read: true, run: true },
96+
async fn() {
97+
// Skip if extension not found
98+
try {
99+
Deno.statSync(extensionPath);
100+
} catch {
101+
return;
102+
}
103+
104+
const code = `
105+
import { DatabaseSync } from "node:sqlite";
106+
const extensionPath = Deno.args[0];
107+
const db = new DatabaseSync(":memory:", { allowExtension: true });
108+
db.loadExtension(extensionPath);
109+
const stmt = db.prepare("SELECT test_func('test') AS result");
110+
const { result } = stmt.get();
111+
if (result !== "test") throw new Error("Unexpected result: " + result);
112+
db.close();
113+
console.log("OK");
114+
`;
115+
116+
const command = new Deno.Command(Deno.execPath(), {
117+
args: [
118+
"run",
119+
`--allow-read=${extensionPath}`,
120+
`--allow-ffi=${extensionPath}`,
121+
"--no-lock",
122+
"-",
123+
extensionPath,
124+
],
125+
stdin: "piped",
126+
stdout: "piped",
127+
stderr: "piped",
128+
});
129+
130+
const child = command.spawn();
131+
const writer = child.stdin.getWriter();
132+
await writer.write(new TextEncoder().encode(code));
133+
await writer.close();
134+
135+
const { code: exitCode, stdout, stderr } = await child.output();
136+
const stdoutText = new TextDecoder().decode(stdout);
137+
const stderrText = new TextDecoder().decode(stderr);
138+
139+
assertEquals(exitCode, 0, `Expected success but got: ${stderrText}`);
140+
assertEquals(stdoutText.trim(), "OK");
141+
},
142+
});
143+
144+
Deno.test({
145+
name:
146+
"[node/sqlite] DatabaseSync loadExtension fails for path outside scoped FFI",
147+
permissions: { read: true, run: true },
148+
async fn() {
149+
// Skip if extension not found
150+
try {
151+
Deno.statSync(extensionPath);
152+
} catch {
153+
return;
154+
}
155+
156+
// Grant FFI only for a different path, not the actual extension
157+
const wrongPath = "/some/other/path";
158+
159+
const code = `
160+
import { DatabaseSync } from "node:sqlite";
161+
const extensionPath = Deno.args[0];
162+
const db = new DatabaseSync(":memory:", { allowExtension: true });
163+
try {
164+
db.loadExtension(extensionPath);
165+
console.log("UNEXPECTED_SUCCESS");
166+
} catch (e) {
167+
if (e instanceof Deno.errors.NotCapable) {
168+
console.log("EXPECTED_PERMISSION_ERROR");
169+
} else {
170+
console.log("UNEXPECTED_ERROR: " + e.constructor.name + ": " + e.message);
171+
}
172+
}
173+
db.close();
174+
`;
175+
176+
const command = new Deno.Command(Deno.execPath(), {
177+
args: [
178+
"run",
179+
`--allow-read=${extensionPath}`,
180+
`--allow-ffi=${wrongPath}`,
181+
"--no-lock",
182+
"-",
183+
extensionPath,
184+
],
185+
stdin: "piped",
186+
stdout: "piped",
187+
stderr: "piped",
188+
});
189+
190+
const child = command.spawn();
191+
const writer = child.stdin.getWriter();
192+
await writer.write(new TextEncoder().encode(code));
193+
await writer.close();
194+
195+
const { stdout } = await child.output();
196+
const stdoutText = new TextDecoder().decode(stdout);
197+
198+
assertEquals(
199+
stdoutText.trim(),
200+
"EXPECTED_PERMISSION_ERROR",
201+
`Expected NotCapable error but got: ${stdoutText}`,
202+
);
203+
},
204+
});

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)