Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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/<package>` link, is complete ([#885](https://github.com/voidzero-dev/vite-task/pull/885)).
Expand Down
26 changes: 23 additions & 3 deletions crates/fspy_shared/src/ipc/ipc_path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 {
Expand Down Expand Up @@ -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<P: AsRef<Path>>(
&self,
base: P,
) -> Result<PathBuf, StripPrefixError> {
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 {
Expand Down
85 changes: 59 additions & 26 deletions crates/vt/src/session/execute/tracked_accesses.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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() {
Expand Down Expand Up @@ -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<RelativePathBuf> {
// Resolve `..` components before stripping the workspace root. fspy may
// report paths like `<workspace>/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
// `<workspace>/../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<RelativePathBuf> {
// 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;
Expand Down Expand Up @@ -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<RelativePathBuf> {
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<u16> = {
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(
Expand Down Expand Up @@ -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());
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"name": "write-outside-workspace-through-dotdot",
"private": true
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[[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.
"""
steps = [["vt", "run", "deploy"], ["vt", "run", "deploy"]]
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# 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`

```
$ vtt fs mkdir ../site write ../site/index.html hi
```

## `vt run deploy`

```
$ vtt fs mkdir ../site write ../site/index.html hi ◉ cache hit, replaying

---
vt run: cache hit.
```
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"tasks": {
"deploy": {
"command": "vtt fs mkdir ../site write ../site/index.html hi",
"cache": true
}
}
}
21 changes: 20 additions & 1 deletion crates/vt_path/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ pub mod relative;
use std::{
ffi::OsStr,
io,
path::{Path, StripPrefixError},
path::{Path, PathBuf, StripPrefixError},
};

#[cfg(feature = "absolute-redaction")]
Expand Down Expand Up @@ -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<PathBuf, StripPrefixError> {
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.
///
Expand Down
Loading