Skip to content

Commit 076069f

Browse files
committed
Fix symlink handling in FUSE and overlay filesystem
The FUSE handlers were using stat() which follows symlinks, causing symlinks to directories to be incorrectly reported as directories. This broke operations like `rm -rf` on pnpm's node_modules which uses symlinks extensively.
1 parent 6fcb78e commit 076069f

4 files changed

Lines changed: 99 additions & 21 deletions

File tree

cli/src/fuse.rs

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ impl Filesystem for AgentFSFuse {
8282
};
8383
let fs = self.fs.clone();
8484
let (result, path) = self.runtime.block_on(async move {
85-
let result = fs.stat(&path).await;
85+
let result = fs.lstat(&path).await;
8686
(result, path)
8787
});
8888
match result {
@@ -107,7 +107,7 @@ impl Filesystem for AgentFSFuse {
107107
};
108108

109109
let fs = self.fs.clone();
110-
let result = self.runtime.block_on(async move { fs.stat(&path).await });
110+
let result = self.runtime.block_on(async move { fs.lstat(&path).await });
111111

112112
match result {
113113
Ok(Some(stats)) => reply.attr(&TTL, &fillattr(&stats, self.uid, self.gid)),
@@ -489,7 +489,7 @@ impl Filesystem for AgentFSFuse {
489489
// Verify target is a directory
490490
let fs = self.fs.clone();
491491
let (stat_result, path) = self.runtime.block_on(async move {
492-
let result = fs.stat(&path).await;
492+
let result = fs.lstat(&path).await;
493493
(result, path)
494494
});
495495

@@ -620,20 +620,35 @@ impl Filesystem for AgentFSFuse {
620620
// Get inode before removing so we can uncache
621621
let fs = self.fs.clone();
622622
let (stat_result, path) = self.runtime.block_on(async move {
623-
let result = fs.stat(&path).await;
623+
let result = fs.lstat(&path).await;
624624
(result, path)
625625
});
626626

627-
let ino = stat_result.ok().flatten().map(|s| s.ino as u64);
627+
let stats = match &stat_result {
628+
Ok(Some(s)) => s,
629+
Ok(None) => {
630+
reply.error(libc::ENOENT);
631+
return;
632+
}
633+
Err(_) => {
634+
reply.error(libc::EIO);
635+
return;
636+
}
637+
};
638+
639+
if stats.is_directory() {
640+
reply.error(libc::EISDIR);
641+
return;
642+
}
643+
644+
let ino = stats.ino as u64;
628645

629646
let fs = self.fs.clone();
630647
let result = self.runtime.block_on(async move { fs.remove(&path).await });
631648

632649
match result {
633650
Ok(()) => {
634-
if let Some(ino) = ino {
635-
self.drop_path(ino);
636-
}
651+
self.drop_path(ino);
637652
reply.ok();
638653
}
639654
Err(_) => reply.error(libc::EIO),

cli/tests/all.sh

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,3 +6,4 @@ DIR="$(dirname "$0")"
66
"$DIR/test-syscalls.sh"
77
"$DIR/test-run-bash.sh"
88
"$DIR/test-mount.sh"
9+
"$DIR/test-symlinks.sh"

cli/tests/test-symlinks.sh

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
#!/bin/sh
2+
set -e
3+
4+
echo -n "TEST symlink handling... "
5+
6+
# Create test directory with symlinks on the host (these will be visible in the sandbox)
7+
TEST_DIR=".agentfs/symlink-test-$$"
8+
rm -rf "$TEST_DIR"
9+
mkdir -p "$TEST_DIR/target_dir"
10+
echo "test content" > "$TEST_DIR/target_dir/file.txt"
11+
ln -s target_dir "$TEST_DIR/link_to_dir"
12+
ln -s target_dir/file.txt "$TEST_DIR/link_to_file"
13+
14+
cleanup() {
15+
rm -rf "$TEST_DIR"
16+
}
17+
trap cleanup EXIT
18+
19+
# Test 1 & 2: Verify symlinks are reported correctly (not as directories)
20+
output=$(cargo run -- run /bin/bash -c "ls -la $TEST_DIR/" 2>&1 || true)
21+
22+
# The output should contain 'lrwxrwxrwx' for symlinks (not 'drwxr-xr-x' for directory)
23+
if ! echo "$output" | grep -E "^lrwx.* link_to_dir"; then
24+
echo "FAILED: symlink to directory not reported as symlink"
25+
echo "$output"
26+
exit 1
27+
fi
28+
29+
if ! echo "$output" | grep -E "^lrwx.* link_to_file"; then
30+
echo "FAILED: symlink to file not reported as symlink"
31+
echo "$output"
32+
exit 1
33+
fi
34+
35+
# Test 3: Verify rm can remove symlink to directory (this was the original bug)
36+
# Previously this would fail with "Is a directory" because symlinks were misidentified
37+
output=$(cargo run -- run /bin/bash -c "rm $TEST_DIR/link_to_dir && echo 'symlink removed successfully'" 2>&1 || true)
38+
39+
if ! echo "$output" | grep -q "symlink removed successfully"; then
40+
echo "FAILED: could not remove symlink to directory"
41+
echo "$output"
42+
exit 1
43+
fi
44+
45+
# Test 4: Verify the target directory still exists on host after removing symlink
46+
# (The removal was in the delta layer, host should still have it)
47+
if ! cat "$TEST_DIR/target_dir/file.txt" | grep -q "test content"; then
48+
echo "FAILED: target directory should still exist after removing symlink"
49+
exit 1
50+
fi
51+
52+
echo "OK"

sdk/rust/src/filesystem/overlayfs.rs

Lines changed: 23 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -543,31 +543,41 @@ impl FileSystem for OverlayFS {
543543
async fn remove(&self, path: &str) -> Result<()> {
544544
let normalized = self.normalize_path(path);
545545

546-
// Check if directory has children in delta - if so, can't remove
547-
if let Some(children) = self.delta.readdir(&normalized).await? {
548-
if !children.is_empty() {
549-
return Err(FsError::NotEmpty.into());
550-
}
551-
}
546+
// Check if path is a symlink - symlinks don't have children, so skip directory checks
547+
let is_symlink = if let Some(stats) = self.lstat(&normalized).await? {
548+
stats.is_symlink()
549+
} else {
550+
false
551+
};
552552

553-
// Check for visible children in base (not whiteout-ed)
554-
if let Some(base_children) = self.base.readdir(&normalized).await? {
555-
for child in base_children {
556-
let child_path = format!("{}/{}", normalized, child);
557-
if !self.is_whiteout(&child_path).await? {
553+
// Only check for children if not a symlink (directories need to be empty)
554+
if !is_symlink {
555+
// Check if directory has children in delta - if so, can't remove
556+
if let Some(children) = self.delta.readdir(&normalized).await? {
557+
if !children.is_empty() {
558558
return Err(FsError::NotEmpty.into());
559559
}
560560
}
561+
562+
// Check for visible children in base (not whiteout-ed)
563+
if let Some(base_children) = self.base.readdir(&normalized).await? {
564+
for child in base_children {
565+
let child_path = format!("{}/{}", normalized, child);
566+
if !self.is_whiteout(&child_path).await? {
567+
return Err(FsError::NotEmpty.into());
568+
}
569+
}
570+
}
561571
}
562572

563573
// Try to remove from delta
564574
let removed_from_delta = self.delta.remove(&normalized).await.is_ok();
565575

566-
// Check if it exists in base (and not already whiteout)
576+
// Check if it exists in base (and not already whiteout) - use lstat to not follow symlinks
567577
let exists_in_base = if self.is_whiteout(&normalized).await? {
568578
false
569579
} else {
570-
self.base.stat(&normalized).await?.is_some()
580+
self.base.lstat(&normalized).await?.is_some()
571581
};
572582

573583
// If exists in base, create whiteout

0 commit comments

Comments
 (0)