fix(fs): exists rethrows Deno.errors.PermissionDenied when existence is indeterminate

[LeSingh1]

Fixes #6528.

exists and existsSync returned true for any path that produced a PermissionDenied at stat() time (with --allow-read granted), on the assumption that the OS error meant the item is there, just not readable. The far more common cause is that the parent directory isn't traversable, in which case we genuinely cannot determine existence and the true answer silently misleads the caller.

Repro from the issue:

$ ls /root
ls: cannot open directory '/root': Permission denied
$ cat test.ts
import { exists } from "jsr:@std/fs/exists";
console.log(await exists("/root/foo"));
$ deno run --allow-read test.ts
true

After the patch, exists("/root/foo") rethrows Deno.errors.PermissionDenied and the caller can pick the right policy for their context.

exists(path, { isReadable: true }) is unchanged. That question has a defensible answer (the OS just told us "can't read" → false), and the existing exists() returns true for an existing dir symlink test in fs/exists_test.ts relies on it (it chmod-0o000s the parent and expects false).

Two new Unix-gated regression tests cover both exists() and existsSync(), asserting:

• Deno.errors.PermissionDenied is rethrown when the parent is unreadable
• { isReadable: true } still returns false in the same scenario

This is a small but real behavior change for callers that today swallow a misleading true. Net diff is small and is documented inline at both call sites.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.57%. Comparing base (cdf74a8) to head (7d1e8db).
⚠️ Report is 18 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7170      +/-   ##
==========================================
- Coverage   94.57%   94.57%   -0.01%     
==========================================
  Files         636      636              
  Lines       52142    52146       +4     
  Branches     9401     9403       +2     
==========================================
+ Hits        49315    49318       +3     
  Misses       2249     2249              
- Partials      578      579       +1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[bartlomieju]

Overview

Fixes #6528. When Deno.stat throws PermissionDenied and --allow-read is granted, exists()/existsSync() previously returned true (assuming "item is there, just not readable"). This PR instead rethrows Deno.errors.PermissionDenied for the default case, since the more common cause is a non-traversable parent directory where existence is genuinely indeterminate. The { isReadable: true } branch keeps returning false. Two Unix-gated regression tests are added.

The reasoning is sound and the diagnosis of the original bug is correct — a stat() PermissionDenied almost always means a parent directory lacks the execute/search bit, in which case true was an actively misleading answer.

Main concern: this is an undocumented contract change on a stable API

The JSDoc for both functions still says @returns ... true if the path exists, false otherwise with no mention that the function can throw PermissionDenied. With this change, a call that previously always returned a boolean (under --allow-read) can now throw. That needs to be reflected in the public docs:

• Add a @throws {Deno.errors.PermissionDenied} tag to both exists and existsSync describing the indeterminate-existence case.
• Consider a short note in the description, since callers relying on the old true will now see an exception.

Separately — this is a stable module, so the maintainers should consciously sign off on the behavior change (vs. the old buggy-but-non-throwing behavior). The fix is correct in principle; just flagging that some existing callers swallow the old true and will now get an exception instead.

Minor

• The { isReadable: true } branch returning false in the same indeterminate scenario is a slight philosophical inconsistency (if we can't stat, we can't actually know it's unreadable either). Keeping it for backward-compat with the existing symlink test is a defensible call — worth a one-line code comment noting this is intentional.
• Tests could be tightened with assertRejects/assertThrows (inline).

Overall the change is well-reasoned and well-tested; the documentation update is the one thing I'd consider blocking.

[bartlomieju]

The { isReadable: true } path confidently returns false for the exact scenario the default path now treats as indeterminate (parent not traversable → can't even stat, so we can't truly know it's unreadable). Keeping it for backward-compat with the existing exists() returns true for an existing dir symlink test is reasonable, but a one-liner noting this asymmetry is intentional would help future readers.

[bartlomieju]

Consider assertRejects here (and assertThrows in the sync test) instead of the manual try/catch + instanceof assert — it's the std convention and reads cleaner:

await assertRejects(
  () => exists(innerPath),
  Deno.errors.PermissionDenied,
);

They'd need adding to the @std/assert import at the top of the file.