fix(fs): exists rethrows Deno.errors.PermissionDenied when existence is indeterminate#7170
Open
LeSingh1 wants to merge 1 commit into
Open
fix(fs): exists rethrows Deno.errors.PermissionDenied when existence is indeterminate#7170LeSingh1 wants to merge 1 commit into
LeSingh1 wants to merge 1 commit into
Conversation
exists/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 can't determine existence and the "true" answer silently misleads callers (denoland#6528). Rethrow Deno.errors.PermissionDenied for the no-isReadable path so callers can decide how to handle the indeterminate case. Keep the old behavior for `isReadable: true`: that question always has a defensible answer (the OS just told us "can't read" → false). This preserves the existing `exists() returns true for an existing dir symlink` test that exercises the isReadable branch with chmod 000 on the parent. Adds Unix-gated regression tests for both exists() and existsSync() covering the chmod-000-parent case and confirming `isReadable: true` still returns false. Fixes denoland#6528
Codecov Report✅ All modified and coverable lines are covered by tests. 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 Sentry. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #6528.
existsandexistsSyncreturnedtruefor any path that produced aPermissionDeniedatstat()time (with--allow-readgranted), 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 thetrueanswer silently misleads the caller.Repro from the issue:
After the patch,
exists("/root/foo")rethrowsDeno.errors.PermissionDeniedand 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 existingexists() returns true for an existing dir symlinktest infs/exists_test.tsrelies on it (it chmod-0o000s the parent and expectsfalse).Two new Unix-gated regression tests cover both
exists()andexistsSync(), asserting:Deno.errors.PermissionDeniedis rethrown when the parent is unreadable{ isReadable: true }still returnsfalsein the same scenarioThis 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.