fix(permissions): treat Windows \\?\ verbatim paths as equivalent#35096
Merged
Conversation
On Windows, requesting a path via its `\\?\` verbatim (extended-length) form, e.g. `\\?\C:\foo`, was permission-checked as distinct from the regular `C:\foo`, so a path already granted access would unexpectedly trigger a new permission prompt (#18597). The path descriptor normalization in `deno_permissions` builds its comparison key with `deno_path_util::normalize_path`, which preserves the `\\?\` verbatim prefix that Rust parses as a `Prefix::VerbatimDisk` component. The two forms therefore never compared equal. Strip the verbatim prefix via `dunce::simplified` before normalization at the three path-descriptor construction sites. `dunce::simplified` only strips the prefix when the regular form is genuinely equivalent (it refuses paths with `.`/`..` components, reserved DOS device names, or that exceed the legacy length limit), so this does not over-grant. Co-Authored-By: Divy Srivastava <[email protected]>
The TestPermissionDescriptorParser stub builds path descriptors directly without normalization or verbatim-prefix stripping, so testing through `check_open` bypassed the fix entirely. Test the descriptor construction functions (`PathDescriptor`/`PathQueryDescriptor::new_known_absolute`) directly instead, which is what the real runtime parser uses. Co-Authored-By: Divy Srivastava <[email protected]>
Co-Authored-By: Divy Srivastava <[email protected]>
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.
On Windows, requesting a path through its
\\?\verbatim (extended-length) form — e.g.\\?\C:\foo— was permission-checked as a distinct path from the regularC:\foo. As a result, a directory already granted read/write access would unexpectedly trigger a new permission prompt:This also breaks node polyfills that hand
\\?\-prefixed paths to fs ops (e.g._fs_chmod.ts, see wojpawlik/deno2node#39).Cause
The path descriptors in
deno_permissionsbuild their comparison key withdeno_path_util::normalize_path, which walkspath.components(). Rust parses\\?\C:\fooas aComponent::Prefix(VerbatimDisk), andnormalize_pathpreserves that verbatim prefix, so\\?\C:\fooandC:\foonever compare equal. The existingis_windows_device_pathbranch only handled\\.\device-namespace paths.Fix
Strip the verbatim prefix via
dunce::simplifiedbefore normalization, at the three path-descriptor construction sites (PathQueryDescriptor::new,PathQueryDescriptor::new_known_absolute,PathDescriptor::new_known_cwd).dunce::simplifiedonly strips the prefix when the regular form is genuinely equivalent — it refuses paths containing./..components (taken literally in verbatim mode), reserved DOS device names (CON,NUL,COM1, …), or paths that exceed the legacy length limit. Those forms are not the same file as their stripped counterpart, so this conservative behavior avoids over-granting. On non-Windows platforms the helper is a no-op, so behavior there is unchanged.A
#[cfg(windows)]unit test (check_path_verbatim_prefix) covers both directions (grant regular → request verbatim, and grant verbatim → request regular) plus a negative case.Closes #18597
Closes denoland/divybot#553