From 60b52ebc1ede680c00f3fcc7161b80afaad9f6a2 Mon Sep 17 00:00:00 2001 From: wan9chi Date: Thu, 5 Nov 2026 16:34:24 +0800 Subject: [PATCH 1/2] test(e2e): show that a task writing outside the workspace through `..` isn't cached A case for a task that writes outside the workspace through a relative path with `..`, like `cp -r dist ../site` publishing a build next to the workspace. In `write_outside_workspace_through_dotdot`, the task makes `../site` and writes `../site/index.html` through `vtt fs`; `..` is the folder holding the staged workspace, which the harness removes. Today fspy reports the write as `/../site/index.html`. That passes the check that a tracked path is in the workspace and becomes the output `../site/index.html`, so every run fails to update the cache with `paths in archives must not have ..`. The case runs on Unix only for now. On Windows, `..` is resolved before fspy reports the path, so the write is outside the workspace and the task is already cached there. Co-Authored-By: Claude Opus 5.5 --- .../package.json | 4 ++++ .../snapshots.toml | 9 ++++++++ .../write_outside_workspace_through_dotdot.md | 21 +++++++++++++++++++ .../vite-task.json | 8 +++++++ 4 files changed, 42 insertions(+) create mode 100644 crates/vt_bin/tests/e2e_snapshots/fixtures/write_outside_workspace_through_dotdot/package.json create mode 100644 crates/vt_bin/tests/e2e_snapshots/fixtures/write_outside_workspace_through_dotdot/snapshots.toml create mode 100644 crates/vt_bin/tests/e2e_snapshots/fixtures/write_outside_workspace_through_dotdot/snapshots/write_outside_workspace_through_dotdot.md create mode 100644 crates/vt_bin/tests/e2e_snapshots/fixtures/write_outside_workspace_through_dotdot/vite-task.json diff --git a/crates/vt_bin/tests/e2e_snapshots/fixtures/write_outside_workspace_through_dotdot/package.json b/crates/vt_bin/tests/e2e_snapshots/fixtures/write_outside_workspace_through_dotdot/package.json new file mode 100644 index 000000000..904a03ae9 --- /dev/null +++ b/crates/vt_bin/tests/e2e_snapshots/fixtures/write_outside_workspace_through_dotdot/package.json @@ -0,0 +1,4 @@ +{ + "name": "write-outside-workspace-through-dotdot", + "private": true +} diff --git a/crates/vt_bin/tests/e2e_snapshots/fixtures/write_outside_workspace_through_dotdot/snapshots.toml b/crates/vt_bin/tests/e2e_snapshots/fixtures/write_outside_workspace_through_dotdot/snapshots.toml new file mode 100644 index 000000000..08fe1ebb1 --- /dev/null +++ b/crates/vt_bin/tests/e2e_snapshots/fixtures/write_outside_workspace_through_dotdot/snapshots.toml @@ -0,0 +1,9 @@ +[[e2e]] +name = "write_outside_workspace_through_dotdot" +comment = """ +The task makes `../site` and writes `../site/index.html`, outside the workspace, like `cp -r dist ../site` publishing a build next to it. `..` is the folder holding the staged workspace. +""" +# On Windows, `..` is resolved before fspy reports the path, so the task is +# already cached there. +cfg = 'unix' +steps = [["vt", "run", "deploy"], ["vt", "run", "deploy"]] diff --git a/crates/vt_bin/tests/e2e_snapshots/fixtures/write_outside_workspace_through_dotdot/snapshots/write_outside_workspace_through_dotdot.md b/crates/vt_bin/tests/e2e_snapshots/fixtures/write_outside_workspace_through_dotdot/snapshots/write_outside_workspace_through_dotdot.md new file mode 100644 index 000000000..cfe33bf24 --- /dev/null +++ b/crates/vt_bin/tests/e2e_snapshots/fixtures/write_outside_workspace_through_dotdot/snapshots/write_outside_workspace_through_dotdot.md @@ -0,0 +1,21 @@ +# write_outside_workspace_through_dotdot + +The task makes `../site` and writes `../site/index.html`, outside the workspace, like `cp -r dist ../site` publishing a build next to it. `..` is the folder holding the staged workspace. + +## `vt run deploy` + +**Exit code:** 1 + +``` +$ vtt fs mkdir ../site write ../site/index.html hi +✗ Cache update failed: paths in archives must not have `..` when setting path for +``` + +## `vt run deploy` + +**Exit code:** 1 + +``` +$ vtt fs mkdir ../site write ../site/index.html hi +✗ Cache update failed: paths in archives must not have `..` when setting path for +``` diff --git a/crates/vt_bin/tests/e2e_snapshots/fixtures/write_outside_workspace_through_dotdot/vite-task.json b/crates/vt_bin/tests/e2e_snapshots/fixtures/write_outside_workspace_through_dotdot/vite-task.json new file mode 100644 index 000000000..ddfacb824 --- /dev/null +++ b/crates/vt_bin/tests/e2e_snapshots/fixtures/write_outside_workspace_through_dotdot/vite-task.json @@ -0,0 +1,8 @@ +{ + "tasks": { + "deploy": { + "command": "vtt fs mkdir ../site write ../site/index.html hi", + "cache": true + } + } +} From 827bef81ffd975dfaa48bf3f57f98377c6ab7ef4 Mon Sep 17 00:00:00 2001 From: wan9chi Date: Thu, 5 Nov 2026 16:38:11 +0800 Subject: [PATCH 2/2] fix(cache): resolve .. before checking that a tracked path is in the workspace On Unix, fspy reports a relative path as the working directory joined with it, `..` included, so a task that writes `../site/index.html` from the workspace root, like `cp -r dist ../site`, is tracked as writing `/../site/index.html`. vt stripped the workspace root from that path before resolving `..`, so it passed as `../site/index.html`, and with automatic output tracking, archiving it failed the run with `Cache update failed: paths in archives must not have ..`. A file read that way became an input outside the workspace. vt now resolves `.` and `..` lexically, as before, but before stripping the workspace root, so such a path is outside the workspace and skipped like any other. A path that stays inside, like `packages/sub-pkg/../shared/dist/output.js`, is still `packages/shared/dist/output.js`, and one that leaves the workspace and comes back, like `/..//a`, is now `a`. `vt_path` gets `clean_and_strip_path_prefix`, which fspy's `IpcPath` offers next to `strip_path_prefix`. `write_outside_workspace_through_dotdot` now runs on Windows too. There `..` is resolved before fspy sees the path, so the task was cached already, and the case now has the same result on every platform. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + crates/fspy_shared/src/ipc/ipc_path.rs | 26 +++++- .../src/session/execute/tracked_accesses.rs | 85 +++++++++++++------ .../snapshots.toml | 3 - .../write_outside_workspace_through_dotdot.md | 11 +-- crates/vt_path/src/lib.rs | 21 ++++- 6 files changed, 107 insertions(+), 40 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 037f94a9f..c6ae6d763 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ # Changelog +- **Fixed** On Linux and macOS, a task that writes outside the workspace through a relative path with `..`, like `cp -r dist ../site`, no longer fails with `Cache update failed: paths in archives must not have ..`, and is now cached. Like any other path outside the workspace, what the task writes there isn't among its outputs, and what it reads there isn't among its inputs ([#940](https://github.com/voidzero-dev/vite-task/pull/940)). - **Fixed** Tasks that read the task cache folder, `node_modules/.vite/task-cache`, now hit the cache. `vp run` changes that folder after every run, and tools such as Parcel and Gatsby read it once they rebuild with their own cache in place, so from then on those tasks missed every time with a reason like `'node_modules/.vite/task-cache/v20/last-summary.json' modified` ([#937](https://github.com/voidzero-dev/vite-task/pull/937)). - **Changed** Tasks that write a file they also read are now cached, instead of being reported as `not cached because it modified its inputs`. For example, `tsc --incremental` reads and rewrites `tsconfig.tsbuildinfo`: the next run is a cache hit if the file is as `tsc` left it, or if it is missing, as on a fresh clone, in which case the hit restores it ([#913](https://github.com/voidzero-dev/vite-task/pull/913)). - **Fixed** A cache hit restores the symbolic links a task's `output` globs match as links, instead of dropping them, so a replayed output that links a directory, such as a bundler's `node_modules/` link, is complete ([#885](https://github.com/voidzero-dev/vite-task/pull/885)). diff --git a/crates/fspy_shared/src/ipc/ipc_path.rs b/crates/fspy_shared/src/ipc/ipc_path.rs index 8906db243..255c09009 100644 --- a/crates/fspy_shared/src/ipc/ipc_path.rs +++ b/crates/fspy_shared/src/ipc/ipc_path.rs @@ -5,7 +5,7 @@ use std::os::unix::ffi::OsStrExt as _; use std::{ fmt::Debug, mem::MaybeUninit, - path::{Path, StripPrefixError}, + path::{Path, PathBuf, StripPrefixError}, }; use bumpalo::Bump; @@ -22,8 +22,9 @@ use wincode::{ /// /// On Windows, tracked paths are NT Object Manager paths (`\??` prefix), /// whose raw data is not meaningful for direct consumption. The only way -/// to use the path is through [`strip_path_prefix`](IpcPath::strip_path_prefix), -/// which normalizes platform differences and extracts a workspace-relative path. +/// to use the path is through [`strip_path_prefix`](IpcPath::strip_path_prefix) +/// or [`clean_and_strip_path_prefix`](IpcPath::clean_and_strip_path_prefix), +/// which normalize platform differences and extract a workspace-relative path. #[derive(TransparentWrapper, PartialEq, Eq)] #[repr(transparent)] pub struct IpcPath { @@ -74,6 +75,25 @@ impl IpcPath { let me = self.inner.to_cow_os_str(); f(vt_path::strip_path_prefix(&me, base.as_ref().as_os_str())) } + + /// Like [`strip_path_prefix`](IpcPath::strip_path_prefix), but resolves + /// `.` and `..` in the path lexically first. + /// + /// On Unix, a relative path is tracked as the working directory joined + /// with it, `..` included: opening `../site` in `/repo` is tracked as + /// `/repo/../site`, which isn't under `/repo`. On Windows, `..` is + /// resolved before fspy sees the path. + /// + /// # Errors + /// + /// Returns an error if `base` is not a path prefix of the cleaned path. + pub fn clean_and_strip_path_prefix>( + &self, + base: P, + ) -> Result { + let me = self.inner.to_cow_os_str(); + vt_path::clean_and_strip_path_prefix(&me, base.as_ref().as_os_str()) + } } impl Debug for IpcPath { diff --git a/crates/vt/src/session/execute/tracked_accesses.rs b/crates/vt/src/session/execute/tracked_accesses.rs index 77e896488..1596b38bd 100644 --- a/crates/vt/src/session/execute/tracked_accesses.rs +++ b/crates/vt/src/session/execute/tracked_accesses.rs @@ -23,9 +23,10 @@ pub struct TrackedPathAccesses { } impl TrackedPathAccesses { - /// Build from fspy's raw iterable by stripping the workspace prefix and - /// normalizing `..` components. `.git/*` paths, pnpm's workspace state - /// files and paths in the task cache at `cache_root` are skipped. + /// Build from fspy's raw iterable by resolving `..` components and then + /// stripping the workspace prefix, so paths outside the workspace are + /// skipped. `.git/*` paths, pnpm's workspace state files and paths in the + /// task cache at `cache_root` are skipped too. /// User-configured negatives are applied by the caller (see module docs). pub fn from_raw( raw: &PathAccessIterable, @@ -41,13 +42,8 @@ impl TrackedPathAccesses { .flatten() .filter(|cache_root| !cache_root.as_str().is_empty()); - // Strip workspace root and clean `..` components in one pass. - // fspy may report paths like `packages/sub-pkg/../shared/dist/output.js`. - let relative_path = |path: &IpcPath| { - path.strip_path_prefix(workspace_root, |strip_result| { - normalize_tracked_workspace_path(strip_result.ok()?, cache_root.as_deref()) - }) - }; + let relative_path = + |path: &IpcPath| tracked_workspace_path(path, workspace_root, cache_root.as_deref()); let mut accesses = Self::default(); for access in raw.iter() { @@ -83,24 +79,35 @@ impl TrackedPathAccesses { } } +/// The workspace-relative path of the tracked `path`, or `None` if it's +/// outside the workspace or skipped (see [`normalize_tracked_workspace_path`]). +fn tracked_workspace_path( + path: &IpcPath, + workspace_root: &AbsolutePath, + cache_root: Option<&RelativePath>, +) -> Option { + // Resolve `..` components before stripping the workspace root. fspy may + // report paths like `/packages/sub-pkg/../shared/dist/output.js`, + // which is `packages/shared/dist/output.js`, and on Unix reports a relative + // open of `../site/index.html` from the workspace root as + // `/../site/index.html`, which is outside the workspace. On + // Windows, cleaning also turns malformed paths like + // `c:\workspace\subdir\c:\workspace\subdir` into ones outside it. + let stripped_path = path.clean_and_strip_path_prefix(workspace_root).ok()?; + normalize_tracked_workspace_path(&stripped_path, cache_root) +} + #[expect( clippy::disallowed_types, - reason = "fspy strip_path_prefix exposes std::path::Path; convert to RelativePathBuf immediately" + reason = "fspy clean_and_strip_path_prefix returns std::path::PathBuf; convert to RelativePathBuf immediately" )] fn normalize_tracked_workspace_path( stripped_path: &std::path::Path, cache_root: Option<&RelativePath>, ) -> Option { - // On Windows, paths are possible to be still absolute after stripping the workspace root. - // For example: c:\workspace\subdir\c:\workspace\subdir - // Just ignore those accesses. + // Ignore paths that aren't portable, like ones with non-UTF-8 names. let relative = RelativePathBuf::new(stripped_path).ok()?; - // Clean `..` components — fspy may report paths like - // `packages/sub-pkg/../shared/dist/output.js`. Normalize them for - // consistent behavior across platforms and clean user-facing messages. - let relative = relative.clean().ok()?; - // Skip .git directory accesses (workaround for tools like oxlint) if relative.as_path().strip_prefix(".git").is_ok() { return None; @@ -137,6 +144,38 @@ fn is_pnpm_workspace_state(relative: &RelativePathBuf) -> bool { mod tests { use super::*; + /// The workspace-relative path of `path` under the workspace root `/repo` + /// (`C:\repo` on Windows), tracked as fspy reports it: joined to the root, + /// `..` included. + fn tracked(path: &str) -> Option { + let workspace_root = + AbsolutePath::new(if cfg!(windows) { r"C:\repo" } else { "/repo" }).unwrap(); + let path = workspace_root.join(path); + #[cfg(unix)] + let ipc_path = <&IpcPath>::from(path.as_path()); + #[cfg(windows)] + let wide: Vec = { + use std::os::windows::ffi::OsStrExt as _; + path.as_path().as_os_str().encode_wide().collect() + }; + #[cfg(windows)] + let ipc_path = IpcPath::from_wide(&wide); + tracked_workspace_path(ipc_path, workspace_root, None) + } + + #[test] + fn dot_dot_is_resolved_before_the_workspace_root_is_stripped() { + let rel = |path: &str| Some(RelativePathBuf::new(path).unwrap()); + assert_eq!( + tracked("packages/sub-pkg/../shared/dist/output.js"), + rel("packages/shared/dist/output.js") + ); + assert_eq!(tracked("../repo/dist/output.js"), rel("dist/output.js")); + // A relative open of `../site/index.html` from the workspace root. + assert_eq!(tracked("../site/index.html"), None); + assert_eq!(tracked(".."), None); + } + #[test] fn pnpm_workspace_state_is_ignored() { #[expect( @@ -173,12 +212,6 @@ mod tests { #[cfg(windows)] #[test] fn malformed_windows_drive_path_after_workspace_strip_is_ignored() { - #[expect( - clippy::disallowed_types, - reason = "normalize_tracked_workspace_path requires std::path::Path for fspy strip_path_prefix output" - )] - let relative_path = - normalize_tracked_workspace_path(std::path::Path::new(r"foo\C:\bar"), None); - assert!(relative_path.is_none()); + assert!(tracked(r"foo\C:\bar").is_none()); } } diff --git a/crates/vt_bin/tests/e2e_snapshots/fixtures/write_outside_workspace_through_dotdot/snapshots.toml b/crates/vt_bin/tests/e2e_snapshots/fixtures/write_outside_workspace_through_dotdot/snapshots.toml index 08fe1ebb1..437031b15 100644 --- a/crates/vt_bin/tests/e2e_snapshots/fixtures/write_outside_workspace_through_dotdot/snapshots.toml +++ b/crates/vt_bin/tests/e2e_snapshots/fixtures/write_outside_workspace_through_dotdot/snapshots.toml @@ -3,7 +3,4 @@ name = "write_outside_workspace_through_dotdot" comment = """ The task makes `../site` and writes `../site/index.html`, outside the workspace, like `cp -r dist ../site` publishing a build next to it. `..` is the folder holding the staged workspace. """ -# On Windows, `..` is resolved before fspy reports the path, so the task is -# already cached there. -cfg = 'unix' steps = [["vt", "run", "deploy"], ["vt", "run", "deploy"]] diff --git a/crates/vt_bin/tests/e2e_snapshots/fixtures/write_outside_workspace_through_dotdot/snapshots/write_outside_workspace_through_dotdot.md b/crates/vt_bin/tests/e2e_snapshots/fixtures/write_outside_workspace_through_dotdot/snapshots/write_outside_workspace_through_dotdot.md index cfe33bf24..f60bdaf19 100644 --- a/crates/vt_bin/tests/e2e_snapshots/fixtures/write_outside_workspace_through_dotdot/snapshots/write_outside_workspace_through_dotdot.md +++ b/crates/vt_bin/tests/e2e_snapshots/fixtures/write_outside_workspace_through_dotdot/snapshots/write_outside_workspace_through_dotdot.md @@ -4,18 +4,15 @@ The task makes `../site` and writes `../site/index.html`, outside the workspace, ## `vt run deploy` -**Exit code:** 1 - ``` $ vtt fs mkdir ../site write ../site/index.html hi -✗ Cache update failed: paths in archives must not have `..` when setting path for ``` ## `vt run deploy` -**Exit code:** 1 - ``` -$ vtt fs mkdir ../site write ../site/index.html hi -✗ Cache update failed: paths in archives must not have `..` when setting path for +$ vtt fs mkdir ../site write ../site/index.html hi ◉ cache hit, replaying + +--- +vt run: cache hit. ``` diff --git a/crates/vt_path/src/lib.rs b/crates/vt_path/src/lib.rs index 0cbd0e2d2..a42975185 100644 --- a/crates/vt_path/src/lib.rs +++ b/crates/vt_path/src/lib.rs @@ -6,7 +6,7 @@ pub mod relative; use std::{ ffi::OsStr, io, - path::{Path, StripPrefixError}, + path::{Path, PathBuf, StripPrefixError}, }; #[cfg(feature = "absolute-redaction")] @@ -53,6 +53,25 @@ pub fn strip_path_prefix<'a>(path: &'a OsStr, base: &OsStr) -> Result<&'a Path, Path::new(path).strip_prefix(base) } +/// Strips `base` from `path` after resolving `.` and `..` in `path`. +/// +/// Resolving is lexical, like [`AbsolutePath::clean`], and stripping is like +/// [`strip_path_prefix`]. So `/repo/../site` isn't under `/repo`, and +/// `/repo/../repo/a` is `a` under it. +/// +/// # Errors +/// +/// Returns an error if `base` is not a path prefix of the cleaned `path`. +pub fn clean_and_strip_path_prefix( + path: &OsStr, + base: &OsStr, +) -> Result { + use path_clean::PathClean as _; + + let path = Path::new(strip_windows_path_prefix(path)).clean(); + path.strip_prefix(strip_windows_path_prefix(base)).map(Path::to_path_buf) +} + /// Strip the `\\?\`, `\\.\`, `\??\` prefix from a Windows path, if present. /// Does nothing on non-Windows platforms. ///