[fas89]

Closes the roadmap board's "[P0][Security][forge-cli] A declared remote prefix resolves as a local path, granting the host to contract SQL" (Trello 1209). Found by the security review of #743 and left open there deliberately, because the root cause is code #743 does not touch.

The defect

DuckDB resolves every allowed_directories entry against the file system. A contract declares the remote prefix, and its own repository supplies the working directory — so a link named for the scheme is enough. With s3: -> / present:

allow = DuckDBAllowlist.none().with_remote("s3://etc/")   # allow.dirs is EMPTY
con = secure_duckdb_connect(":memory:", allow=allow)
# allowed_directories == ['/private/etc/', <spill>]
con.execute("SELECT count(*) FROM read_text('/etc/hosts')")   # returns a row

Reproduced on duckdb 1.5.1, 1.5.6 and 2.0.0. Reachable from real call sites: providers/local/local.py hands a raw s3:// URI to with_declared, and cli/discover/filesystem.py hands one to with_locations.

#743's read-back could not catch it. Its accommodation for DuckDB 1.5 — which reports s3://bucket/landing/ as <cwd>/s3:/bucket/landing/ — was written as realpath(prefix), and that follows symlinks inside the prefix, so the escape's own resolution entered the granted set and excused the grant it had just produced.

Two measurements that killed two earlier plans

Both are worth recording, because each looked right on paper.

1. "Remote prefixes are inert under enable_external_access = false, so stop putting them in allowed_directories." False. With httpfs loaded before the lock, remote reads work after it, and the prefix is what bounds them. Against the public s3://noaa-ghcn-pds/ bucket on 1.5.1 and 1.5.6: no prefix granted → refused; a different bucket granted → refused; the target's own prefix → reads. Dropping the entry would have broken legitimate remote reads.
2. "The honest form is a path that does not exist, so test existence." Wrong test. DuckDB grants a resolved path that is not there yet — s3://staging-area/ with s3: -> / grants /staging-area — and contract SQL can write into it.

What actually separates the honest case from the escape is symlink resolution: DuckDB 1.5's reported form is the lexical join of cwd and the prefix, and an escaping prefix differs from it only because its components follow a link.

The fix

• _escaping_remote_prefix(prefix, cwd) compares the lexical join against realpath, and returns the directory DuckDB would grant.
• _refuse_escaping_remote_prefixes(allow) runs before SET allowed_directories, so the connection never holds the grant.
• _refuse_ungranted_dirs keeps catching it as a backstop, now that its accommodation is the lexical form, and defers to the escape message — the generic one tells the reader to report a sandbox bug, which is wrong here, since a contract plus its repository is all it takes.

Two barriers, each tested with the other bypassed, plus a test that witnesses the grant with both bypassed, so the file proves what it guards instead of only asserting the guard runs.

A second defect the live run surfaced

Driving the real CLI (not the library) exposed that a sandbox refusal reached the discover handler's except Exception and came out framed as ConnectivityProbeError, whose fix reads "Check VPN / network policy / credentials" — sending the reader to their network for a link in their working directory. A typed error now keeps its own what/why/fix, still masked by the existing machinery, because the refusal names the location it refused and a URL can carry a password in its userinfo. The prefixes moved from what into why: the renderer uses what as the panel title and truncates it to one line, which was cutting off the detail the message exists to give.

What the adversarial review found

Four lenses (bypass, other-security, correctness, test-quality), each finding then handed to a skeptic told to refute it: 17 raised, 12 refuted, 5 confirmed, all LOW. All five are fixed in the second commit. The one that mattered was mine and newly introduced here:

_kept_or_reframed masked four of the six fields as_json emits and copied extras and doc through verbatim. On main the function always built a fresh ConnectivityProbeError, whose extras is empty, so no field of the raised error could survive; this branch created the first path propagating a discoverer-supplied extras dict and then left the one dict-valued field out of the masking applied in the same expression. Latent — no in-tree error sets extras on this path, and the uri key is overwritten — but a discoverer registered out of tree carrying the location under a key of its own would have had the credential serialised in full, and as_json is exactly what the sibling tests assert a credential never reaches. _masked_extras now walks nested dicts and lists; doc is masked; the rebuild's except is broad because this runs while an error is live, where raising a second one would attach the driver's error, URI and password as context.

The other three were all fair criticisms of my tests: the read-back's deferral was never actually exercised (the patched no-op is the function it defers to, so that test witnessed the generic path); the masking test put the credential only in why, making its other assertions vacuous; and the file's docstring claimed "6 of 8 fail on revert" without saying which revert was measured. All three corrected, and each new test mutation-checked — dropping the extras/doc masking, the try/except, or the deferral call fails its own test.

Refuted, for the record: .. as the URL host, a scheme-named real directory rather than a symlink, and a scheme-stripped copy appearing in the message. A plain directory named s3: does yield a grant inside the repository the contract author already controls, which is why the symlink case is the one that escapes.

Validation

• 9 tests in the new file pass on duckdb 1.5.1, 1.5.6 and 2.0.0.
• Full suite, CI-shaped, both floors, run sequentially so nothing contends: 27934 passed / 4 failed (2.0.0) and 27932 passed / 4 failed (1.5.6) — the same four that fail on clean base, verified in a detached worktree at 7cf5471a. Perf suite alone: 38 passed / 7 xfailed on both. Startup budget 6/6.
• Two extra failures appeared only in a contended run and are wall-clock, with the reasons printed by the runner: hypothesis's Input generation is slow health check, and subprocess.TimeoutExpired ... after 120 seconds on a fluid validate child, with 32 workers across two venvs. With a single venv this branch shows exactly base's four.
• Live, through the CLI: fluid init --discover s3://noaa-ghcn-pds/csv/by_year/2024.csv emits a contract from a clean directory (8 columns, exit 0) and is refused with exit 1 and no contract written from one holding s3: -> /, naming the prefix and where it resolved. With https://alice:<secret>@example.com/data.csv the output shows ***REDACTED*** and no copy of the secret.
• ruff, black==24.10.0, license headers, import-linter (6 kept / 0 broken), duckdb-floor gate, and detect-secrets (CI-pinned 1.5.0, baseline untouched) all clean.
• ripwire --quality-delta still exits 2 on 29 pre-existing rows from elsewhere in the tree. Of the rows on these files, the clone and verbosity ones are fixed (the test probes are now imported from the sibling suite rather than copied). What remains is three duplication rows pairing a three-line mkdir/symlink/return helper with unrelated tmp-dir fixtures at 25–30 tokens, and three short-horizon-churn rows caused by iterating on these very functions. Merging that helper with dotenv-project fixtures would be a wrong abstraction, so they are left and named here rather than gamed.

Found in passing — not fixed here

Remote discovery is broken on duckdb 2.0.0. fluid init --discover s3://… fails with Cannot read a CA certificate bundle: file system access is restricted by configuration, because the sandbox forbids reading /etc/ssl/.... Works on 1.5.1 and 1.5.6. Reproduced on clean 7cf5471a before any edit here, so it is independent of this change and wants its own card.

Documentation

• No docs needed — this change does not affect documented behaviour. It refuses a configuration that was never intended to work; the refusal explains itself and names what to change.