Skip to content

Commit f8cdbc8

Browse files
committed
fix: avoid following cache cleanup symlinks
Backports critical upstream uv cache safety fixes: - bc1093a96 avoid following symlinks during cache prune - 436bf40fb avoid following external symlinks during cache clean
1 parent 4fbf06d commit f8cdbc8

2 files changed

Lines changed: 131 additions & 5 deletions

File tree

crates/fyn-cache/src/lib.rs

Lines changed: 92 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -567,9 +567,17 @@ impl Cache {
567567
summary += bucket.remove(self, name)?;
568568
}
569569

570+
if references.is_empty() {
571+
return Ok(summary);
572+
}
573+
574+
// Only remove targets in the archive bucket. Cache entries may contain unexpected links
575+
// to paths outside the cache.
576+
let archive_root = fs_err::canonicalize(&self.root)?.join(CacheBucket::Archive.to_str());
577+
570578
// Remove any archives that are no longer referenced.
571579
for (target, references) in references {
572-
if references.iter().all(|path| !path.exists()) {
580+
if target.starts_with(&archive_root) && references.iter().all(|path| !path.exists()) {
573581
debug!("Removing dangling cache entry: {}", target.display());
574582
summary += rm_rf(target)?;
575583
}
@@ -709,7 +717,7 @@ impl Cache {
709717
Ok(entries) => {
710718
for entry in entries {
711719
let entry = entry?;
712-
let path = fs_err::canonicalize(entry.path())?;
720+
let path = entry.path();
713721
debug!("Removing dangling cache environment: {}", path.display());
714722
summary += rm_rf(path)?;
715723
}
@@ -725,7 +733,7 @@ impl Cache {
725733
Ok(entries) => {
726734
for entry in entries {
727735
let entry = entry?;
728-
let path = fs_err::canonicalize(entry.path())?;
736+
let path = entry.path();
729737
if path.is_dir() {
730738
debug!("Removing unzipped wheel entry: {}", path.display());
731739
summary += rm_rf(path)?;
@@ -782,8 +790,9 @@ impl Cache {
782790
Ok(entries) => {
783791
for entry in entries {
784792
let entry = entry?;
785-
let path = fs_err::canonicalize(entry.path())?;
786-
if !references.contains_key(&path) {
793+
let path = entry.path();
794+
let target = fs_err::canonicalize(&path)?;
795+
if !references.contains_key(&target) {
787796
debug!("Removing dangling cache archive: {}", path.display());
788797
summary += rm_rf(path)?;
789798
}
@@ -1549,4 +1558,82 @@ mod tests {
15491558
assert!(Link::from_str("v1/foo").is_err());
15501559
assert!(Link::from_str("archive-v0/").is_err());
15511560
}
1561+
1562+
#[test]
1563+
#[cfg(unix)]
1564+
fn prune_does_not_follow_environment_symlinks() {
1565+
use super::{Cache, CacheBucket};
1566+
1567+
let cache_root = tempfile::tempdir().unwrap();
1568+
let victim_root = tempfile::tempdir().unwrap();
1569+
let environments = cache_root.path().join(CacheBucket::Environments.to_str());
1570+
let victim_dir = victim_root.path().join("victim-dir");
1571+
1572+
fs_err::create_dir_all(&environments).unwrap();
1573+
fs_err::create_dir_all(&victim_dir).unwrap();
1574+
fs_err::write(victim_dir.join("payload.txt"), "payload").unwrap();
1575+
fs_err::os::unix::fs::symlink(&victim_dir, environments.join("escape")).unwrap();
1576+
1577+
let summary = Cache::from_path(cache_root.path()).prune(false).unwrap();
1578+
1579+
assert_eq!(summary.num_files, 1);
1580+
assert_eq!(summary.num_dirs, 0);
1581+
assert!(victim_dir.is_dir());
1582+
assert!(victim_dir.join("payload.txt").is_file());
1583+
assert!(fs_err::symlink_metadata(environments.join("escape")).is_err());
1584+
}
1585+
1586+
#[test]
1587+
#[cfg(unix)]
1588+
fn prune_ci_does_not_follow_wheel_symlinks() {
1589+
use super::{Cache, CacheBucket};
1590+
1591+
let cache_root = tempfile::tempdir().unwrap();
1592+
let victim_root = tempfile::tempdir().unwrap();
1593+
let wheels = cache_root.path().join(CacheBucket::Wheels.to_str());
1594+
let source_distributions = cache_root
1595+
.path()
1596+
.join(CacheBucket::SourceDistributions.to_str());
1597+
let victim_dir = victim_root.path().join("victim-dir");
1598+
let symlink = wheels.join("escape");
1599+
1600+
fs_err::create_dir_all(&wheels).unwrap();
1601+
fs_err::create_dir_all(&source_distributions).unwrap();
1602+
fs_err::create_dir_all(&victim_dir).unwrap();
1603+
fs_err::write(victim_dir.join("payload.txt"), "payload").unwrap();
1604+
fs_err::os::unix::fs::symlink(&victim_dir, &symlink).unwrap();
1605+
1606+
let summary = Cache::from_path(cache_root.path()).prune(true).unwrap();
1607+
1608+
assert_eq!(summary.num_files, 1);
1609+
assert_eq!(summary.num_dirs, 0);
1610+
assert!(victim_dir.is_dir());
1611+
assert!(victim_dir.join("payload.txt").is_file());
1612+
assert!(fs_err::symlink_metadata(symlink).is_err());
1613+
}
1614+
1615+
#[test]
1616+
#[cfg(unix)]
1617+
fn prune_does_not_follow_archive_symlinks() {
1618+
use super::{Cache, CacheBucket};
1619+
1620+
let cache_root = tempfile::tempdir().unwrap();
1621+
let victim_root = tempfile::tempdir().unwrap();
1622+
let archives = cache_root.path().join(CacheBucket::Archive.to_str());
1623+
let victim_dir = victim_root.path().join("victim-dir");
1624+
let symlink = archives.join("escape");
1625+
1626+
fs_err::create_dir_all(&archives).unwrap();
1627+
fs_err::create_dir_all(&victim_dir).unwrap();
1628+
fs_err::write(victim_dir.join("payload.txt"), "payload").unwrap();
1629+
fs_err::os::unix::fs::symlink(&victim_dir, &symlink).unwrap();
1630+
1631+
let summary = Cache::from_path(cache_root.path()).prune(false).unwrap();
1632+
1633+
assert_eq!(summary.num_files, 1);
1634+
assert_eq!(summary.num_dirs, 0);
1635+
assert!(victim_dir.is_dir());
1636+
assert!(victim_dir.join("payload.txt").is_file());
1637+
assert!(fs_err::symlink_metadata(symlink).is_err());
1638+
}
15521639
}

crates/fyn/tests/it/cache_clean.rs

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -263,6 +263,45 @@ fn clean_package_index() -> Result<()> {
263263
Ok(())
264264
}
265265

266+
#[cfg(unix)]
267+
#[test]
268+
fn clean_package_does_not_follow_symlinks() -> Result<()> {
269+
let context = fyn_test::test_context!("3.12");
270+
let victim_dir = context.temp_dir.child("victim");
271+
let archive_entry = context.cache_dir.child("archive-v0").child("archive");
272+
let package_entry = context
273+
.cache_dir
274+
.child("wheels-v6")
275+
.child("pypi")
276+
.child("demo");
277+
278+
victim_dir.create_dir_all()?;
279+
victim_dir.child("payload.txt").write_str("payload")?;
280+
archive_entry.create_dir_all()?;
281+
archive_entry.child("payload.txt").write_str("payload")?;
282+
package_entry.create_dir_all()?;
283+
284+
// Preserve external targets while still removing unreferenced entries in the archive bucket.
285+
fs_err::os::unix::fs::symlink(&victim_dir, package_entry.join("escape"))?;
286+
fs_err::os::unix::fs::symlink(&archive_entry, package_entry.join("archive"))?;
287+
288+
fyn_snapshot!(context.filters(), context.clean().arg("demo"), @"
289+
success: true
290+
exit_code: 0
291+
----- stdout -----
292+
293+
----- stderr -----
294+
Removed 3 files ([SIZE])
295+
");
296+
297+
assert!(victim_dir.is_dir());
298+
assert!(victim_dir.child("payload.txt").is_file());
299+
assert!(fs_err::symlink_metadata(package_entry).is_err());
300+
assert!(fs_err::symlink_metadata(archive_entry).is_err());
301+
302+
Ok(())
303+
}
304+
266305
#[tokio::test]
267306
async fn cache_timeout() {
268307
let context = fyn_test::test_context!("3.12");

0 commit comments

Comments
 (0)