[WSLC] Alias cannonicalization follow-up in denied-path overlap validator

src/backends/wslc/common/src/policy_mapping.rs

@@ -185,10 +185,11 @@ impl NormalizedPath {
     /// Excluding exact match here is deliberate: an exact same-*string* deny==mount
     /// is already collapsed most-restrictive-wins at parse time
     /// (`normalize_filesystem_paths`), and an exact same-*object* alias (a deny
-    /// that canonicalizes onto the mount root) is collapsed by D6
-    /// (`normalize_object_conflicts`) before this validator runs. This check
-    /// therefore only owns the *strictly-nested* overlap that neither of those
-    /// layers can resolve (deny under a mount = distinct objects).
+    /// that canonicalizes onto the mount root) is normally collapsed by D6
+    /// (`normalize_object_conflicts`) before this validator runs. This method
+    /// therefore only owns the *strictly-nested* overlap; the Tier-2 caller
+    /// separately rejects alias-induced canonical equality (differing original
+    /// strings) that D6 missed as absent.
     fn contains_strictly(&self, child: &NormalizedPath) -> bool {
         if self.rooted && self.components.is_empty() {
             // Whole-drive mount: covers the entire drive, so any same-anchor path
@@ -252,13 +253,13 @@ impl NormalizedPath {
 /// `C:\secrets` through `C:\project\link` if the runtime follows the reparse
 /// point. Detecting this would require walking the mounted subtree for reparse
 /// points (expensive and still racy) or controlling traversal beneath the mount,
-/// which the WSLC SDK does not expose. Likewise, Tier 2 does not fold Unicode
-/// normalization forms and compares path endpoints, not object identity: a hard
-/// link inside a mounted tree pointing at a denied file resolves to its own
-/// in-tree name (not the denied one), so it is not caught here or by D6 (the two
-/// are distinct objects). Creating either alias requires write access to a
-/// location the guest does not have, so both are out of scope under the
-/// trusted-author threat model. A residual TOCTOU window also remains between
+/// which the WSLC SDK does not expose. The same gap applies to a hard link: since
+/// hard links are names for one object, hard-linked *policy entries* are caught by
+/// D6's file-ID grouping (`normalize_object_conflicts`); only an *unlisted* hard
+/// link inside a mounted subtree escapes both Tier 2 and D6. Creating either alias
+/// needs write access the guest lacks, so both are out of scope under the
+/// trusted-author threat model. Tier 2 also does not fold Unicode normalization
+/// forms. A residual TOCTOU window remains between
 /// canonicalization and the SDK mount (an alias could be swapped in between);
 /// fully closing it needs handle-based mounting the WSLC SDK does not expose, so
 /// it is likewise accepted.
@@ -354,11 +355,29 @@ fn validate_denied_path_overlap_with(
             continue; // Absent: no on-disk ancestor resolved, so nothing to alias.
         };
         let denied_norm = NormalizedPath::parse(denied_resolved);
+        let denied_lexical = NormalizedPath::parse(denied);
         for (mounted, list_name, mount_canon) in &mount_canon {
             let PathCanonical::Canonical(mount_resolved) = mount_canon else {
                 continue;
             };
-            if NormalizedPath::parse(mount_resolved).contains_strictly(&denied_norm) {
+            let mount_norm = NormalizedPath::parse(mount_resolved);
+            // Strict nesting is always an unenforceable overlap: the denied
+            // subtree lives inside a flat mount that WSLC cannot mask.
+            if mount_norm.contains_strictly(&denied_norm) {
+                return Err(overlap_error(denied, mounted, list_name, true));
+            }
+            // Exact canonical equality is an overlap only when it comes from an
+            // on-disk alias — the two spellings normalize to *different lexical*
+            // forms yet resolve to the same object. A case/separator/`..` variant
+            // of the same spelling folds to an equal `NormalizedPath` here (via
+            // `NormalizedPath::parse`, NOT `normalize_filesystem_paths`, which only
+            // dedupes byte-identical strings) and is the enforceable exact-match
+            // case (enforced by not mounting the path), so accept it. But a deny
+            // that aliases onto the mount root via a symlink, junction, or short
+            // name has no exclusion primitive — `contains_strictly` (strictly
+            // deeper only) misses that, so reject it here.
+            let mount_lexical = NormalizedPath::parse(mounted);
+            if mount_norm == denied_norm && mount_lexical != denied_lexical {
                 return Err(overlap_error(denied, mounted, list_name, true));
             }
         }
@@ -755,6 +774,93 @@ mod tests {
         assert!(err.contains("cannot be enforced"), "{err}");
     }
 
+    #[test]
+    fn canonical_rejects_denied_alias_equal_to_mount_root() {
+        // Absent-tail deny whose parent alias lands *exactly* on the mount root:
+        // D6 saw the deny as absent, but tail replay canonicalizes it onto the
+        // mount root. `contains_strictly` (strictly deeper only) misses this equal
+        // case, so Tier 2's explicit canonical-equality check must reject it.
+        let resolve = resolver(vec![
+            (r"C:\real", canonical(r"C:\real")),
+            (r"C:\link\missing\..", canonical(r"C:\real")),
+        ]);
+        let err = validate_denied_path_overlap_with(
+            &strings(&[r"C:\real"]),
+            &[],
+            &strings(&[r"C:\link\missing\.."]),
+            resolve,
+        )
+        .unwrap_err();
+
+        assert!(err.contains("cannot be enforced"), "{err}");
+    }
+
+    #[test]
+    fn canonical_allows_literal_exact_match_that_resolves() {
+        // A literal deny == mount (same original string) that both canonicalize to
+        // an existing object must still be accepted: parse-time normalization
+        // collapses it most-restrictive-wins, and it is enforceable by simply not
+        // mounting the path. Only alias-induced equality (differing originals) is
+        // rejected, so an identical string must survive Tier 2.
+        let resolve = resolver(vec![(r"C:\project", canonical(r"C:\project"))]);
+        validate_denied_path_overlap_with(
+            &strings(&[r"C:\project"]),
+            &[],
+            &strings(&[r"C:\project"]),
+            resolve,
+        )
+        .expect("literal exact-match deny is enforceable and must be accepted");
+    }
+
+    #[test]
+    fn canonical_allows_case_and_separator_variant_exact_match() {
+        // `NormalizedPath::parse` case-folds and folds `/` vs `\`, so these are the
+        // same spelling, not an on-disk alias: enforceable by not mounting → accept.
+        let resolve = resolver(vec![
+            (r"C:\Project", canonical(r"C:\project")),
+            ("c:/project", canonical(r"C:\project")),
+        ]);
+        validate_denied_path_overlap_with(
+            &strings(&[r"C:\Project"]),
+            &[],
+            &strings(&["c:/project"]),
+            resolve,
+        )
+        .expect("case/separator variant of the same path must be accepted");
+    }
+
+    #[test]
+    fn canonical_allows_dot_dot_variant_exact_match() {
+        // `C:\x\..\project` folds to `C:\project` lexically — same spelling, not an alias.
+        let resolve = resolver(vec![
+            (r"C:\project", canonical(r"C:\project")),
+            (r"C:\x\..\project", canonical(r"C:\project")),
+        ]);
+        validate_denied_path_overlap_with(
+            &strings(&[r"C:\project"]),
+            &[],
+            &strings(&[r"C:\x\..\project"]),
+            resolve,
+        )
+        .expect("`..`-variant of the same path must be accepted");
+    }
+
+    #[test]
+    fn canonical_allows_trailing_separator_variant_exact_match() {
+        // `parse` drops empty segments, so a trailing separator is the same spelling.
+        let resolve = resolver(vec![
+            (r"C:\project", canonical(r"C:\project")),
+            (r"C:\project\", canonical(r"C:\project")),
+        ]);
+        validate_denied_path_overlap_with(
+            &strings(&[r"C:\project"]),
+            &[],
+            &strings(&[r"C:\project\"]),
+            resolve,
+        )
+        .expect("trailing-separator variant of the same path must be accepted");
+    }
+
     #[test]
     fn empty_mounts_never_fail_closed() {
         // With no mounts nothing can overlap, so an unresolvable deny must NOT

src/core/wxc_common/src/filesystem_canonical.rs

@@ -42,9 +42,10 @@ pub fn canonicalize_path(path: &str) -> PathCanonical {
         CloseHandle, ERROR_FILE_NOT_FOUND, ERROR_PATH_NOT_FOUND, HANDLE,
     };
     use windows::Win32::Storage::FileSystem::{
-        CreateFileW, GetFinalPathNameByHandleW, FILE_FLAG_BACKUP_SEMANTICS, FILE_NAME_NORMALIZED,
-        FILE_READ_ATTRIBUTES, FILE_SHARE_DELETE, FILE_SHARE_READ, FILE_SHARE_WRITE,
-        GETFINALPATHNAMEBYHANDLE_FLAGS, OPEN_EXISTING, VOLUME_NAME_DOS,
+        CreateFileW, GetFinalPathNameByHandleW, FILE_FLAG_BACKUP_SEMANTICS,
+        FILE_FLAG_OPEN_REPARSE_POINT, FILE_NAME_NORMALIZED, FILE_READ_ATTRIBUTES,
+        FILE_SHARE_DELETE, FILE_SHARE_READ, FILE_SHARE_WRITE, GETFINALPATHNAMEBYHANDLE_FLAGS,
+        OPEN_EXISTING, VOLUME_NAME_DOS,
     };
 
     // RAII guard so the handle is closed on every exit path, including an
@@ -83,12 +84,53 @@ pub fn canonicalize_path(path: &str) -> PathCanonical {
             // the error captured by the failed call itself (no second, racy
             // GetLastError round-trip).
             let code = e.code();
-            return if code == HRESULT::from_win32(ERROR_FILE_NOT_FOUND.0)
-                || code == HRESULT::from_win32(ERROR_PATH_NOT_FOUND.0)
+            if code != HRESULT::from_win32(ERROR_FILE_NOT_FOUND.0)
+                && code != HRESULT::from_win32(ERROR_PATH_NOT_FOUND.0)
             {
-                PathCanonical::Absent
-            } else {
-                PathCanonical::Unknown
+                // Exists but unexaminable (access denied, I/O error, …).
+                return PathCanonical::Unknown;
+            }
+            // "Not found" from the open above is ambiguous: that open *follows*
+            // reparse points, so a dangling symlink/junction (the link object
+            // exists but its target is missing) also reports FILE/PATH_NOT_FOUND.
+            // Re-probe without following the reparse point; if the link itself
+            // opens, the path exists but cannot be resolved — classify it Unknown
+            // so denied-path callers fail closed rather than replay a stale alias.
+            // SAFETY: `wide` is a local NUL-terminated buffer; all other pointers
+            // are NULL.
+            let reparse = unsafe {
+                CreateFileW(
+                    PCWSTR(wide.as_ptr()),
+                    FILE_READ_ATTRIBUTES.0,
+                    share,
+                    None,
+                    OPEN_EXISTING,
+                    FILE_FLAG_BACKUP_SEMANTICS | FILE_FLAG_OPEN_REPARSE_POINT,
+                    None,
+                )
+            };
+            return match reparse {
+                // The link object itself exists: present but unresolvable →
+                // fail closed. Drop the probe handle immediately via the guard.
+                Ok(h) if !h.is_invalid() => {
+                    let _guard = OwnedHandle(h);
+                    PathCanonical::Unknown
+                }
+                // Success with an invalid handle: no error to read → unexaminable.
+                Ok(_) => PathCanonical::Unknown,
+                Err(e) => {
+                    let code = e.code();
+                    if code == HRESULT::from_win32(ERROR_FILE_NOT_FOUND.0)
+                        || code == HRESULT::from_win32(ERROR_PATH_NOT_FOUND.0)
+                    {
+                        // Genuinely missing (or a missing parent): absent.
+                        PathCanonical::Absent
+                    } else {
+                        // Present but unexaminable (access denied, sharing
+                        // violation, I/O) → fail closed.
+                        PathCanonical::Unknown
+                    }
+                }
             };
         }
         // CreateFileW reported success but returned an invalid handle: there is
@@ -354,6 +396,96 @@ mod tests {
         );
     }
 
+    #[cfg(windows)]
+    #[test]
+    fn dangling_reparse_point_is_unknown_not_absent() {
+        // A junction whose target is missing exists as an object but makes the
+        // *followed* open report FILE/PATH_NOT_FOUND. It must classify Unknown
+        // (present-but-unresolvable → fail closed), never Absent, so denied-path
+        // callers do not treat it as cleanly-missing and replay a stale alias.
+        use std::process::Command;
+
+        // Thread id keeps concurrent tests from colliding on the fixture dir.
+        let tmp = std::env::temp_dir();
+        let base = format!(
+            r"{}\mxc-dangling-{}-{:?}",
+            tmp.to_string_lossy().trim_end_matches('\\'),
+            std::process::id(),
+            std::thread::current().id()
+        );
+        let link = format!(r"{base}\link");
+        let missing_target = format!(r"{base}\no-such-target");
+        let _ = std::fs::remove_dir_all(&base); // clear any leftover from an aborted run
+        std::fs::create_dir_all(&base).unwrap();
+
+        // `mklink /J` creates a junction even when the target does not exist.
+        let status = Command::new("cmd")
+            .args(["/c", "mklink", "/J", &link, &missing_target])
+            .status()
+            .unwrap();
+        // mklink /J needs no elevation, so a failure is a broken environment,
+        // not an expected skip — surface it rather than silently no-op.
+        assert!(
+            status.success(),
+            "mklink /J failed ({status}); junction creation needs no elevation, so this \
+             indicates a broken test environment rather than an expected skip"
+        );
+
+        let result = canonicalize_path(&link);
+        // Remove the junction itself (do not follow it) before clearing base.
+        let _ = std::fs::remove_dir(&link);
+        let _ = std::fs::remove_dir_all(&base);
+
+        assert_eq!(
+            result,
+            PathCanonical::Unknown,
+            "a dangling junction must be Unknown (fail closed), not Absent"
+        );
+    }
+
+    #[cfg(windows)]
+    #[test]
+    fn dangling_junction_ancestor_is_unknown_not_canonical() {
+        // Chain-level guard where the security property lives: a deny under a
+        // dangling-junction *ancestor* must fail closed, never replay the absent
+        // tail onto a resolved grandparent and return Canonical(<path with the
+        // junction>) — which the overlap check would then miss.
+        use std::process::Command;
+
+        let tmp = std::env::temp_dir();
+        let base = format!(
+            r"{}\mxc-dangling-anc-{}-{:?}",
+            tmp.to_string_lossy().trim_end_matches('\\'),
+            std::process::id(),
+            std::thread::current().id()
+        );
+        let link = format!(r"{base}\link");
+        let missing_target = format!(r"{base}\no-such-target");
+        let leaf = format!(r"{link}\file");
+        let _ = std::fs::remove_dir_all(&base);
+        std::fs::create_dir_all(&base).unwrap();
+
+        let status = Command::new("cmd")
+            .args(["/c", "mklink", "/J", &link, &missing_target])
+            .status()
+            .unwrap();
+        assert!(
+            status.success(),
+            "mklink /J failed ({status}); junction creation needs no elevation"
+        );
+
+        let result = canonicalize_allowing_absent_tail(&leaf);
+        let _ = std::fs::remove_dir(&link);
+        let _ = std::fs::remove_dir_all(&base);
+
+        assert_eq!(
+            result,
+            PathCanonical::Unknown,
+            "a deny under a dangling-junction ancestor must fail closed, not \
+             resolve to a Canonical path still containing the junction"
+        );
+    }
+
     #[cfg(not(windows))]
     #[test]
     fn non_windows_is_unknown() {