Skip to content

Commit c6a9ab5

Browse files
penbergclaude
andcommitted
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. Changes: - Use lstat() instead of stat() in FUSE lookup(), getattr(), unlink(), and rmdir() to correctly identify symlinks - Add EISDIR check in unlink() to reject directory removal attempts - Fix overlay remove() to detect symlinks and skip directory-empty checks for them - Use lstat() in overlay when checking base layer existence - Add test for symlink handling Fixes the "Is a directory" error when removing symlinks to directories. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 6fcb78e commit c6a9ab5

4 files changed

Lines changed: 115 additions & 23 deletions

File tree

cli/src/fuse.rs

Lines changed: 27 additions & 10 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)),
@@ -486,10 +486,10 @@ impl Filesystem for AgentFSFuse {
486486
return;
487487
};
488488

489-
// Verify target is a directory
489+
// Verify target is a directory (use lstat to not follow symlinks)
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

@@ -505,6 +505,7 @@ impl Filesystem for AgentFSFuse {
505505
}
506506
};
507507

508+
// rmdir() only works on directories, not symlinks or files
508509
if !stats.is_directory() {
509510
reply.error(libc::ENOTDIR);
510511
return;
@@ -617,23 +618,39 @@ impl Filesystem for AgentFSFuse {
617618
return;
618619
};
619620

620-
// Get inode before removing so we can uncache
621+
// Get inode before removing so we can uncache (use lstat to not follow symlinks)
621622
let fs = self.fs.clone();
622623
let (stat_result, path) = self.runtime.block_on(async move {
623-
let result = fs.stat(&path).await;
624+
let result = fs.lstat(&path).await;
624625
(result, path)
625626
});
626627

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

629648
let fs = self.fs.clone();
630649
let result = self.runtime.block_on(async move { fs.remove(&path).await });
631650

632651
match result {
633652
Ok(()) => {
634-
if let Some(ino) = ino {
635-
self.drop_path(ino);
636-
}
653+
self.drop_path(ino);
637654
reply.ok();
638655
}
639656
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: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
#!/bin/sh
2+
set -e
3+
4+
echo -n "TEST symlink handling... "
5+
6+
# Create a test directory structure with symlinks
7+
TEST_DIR=$(mktemp -d)
8+
mkdir -p "$TEST_DIR/target_dir"
9+
echo "test content" > "$TEST_DIR/target_dir/file.txt"
10+
ln -s target_dir "$TEST_DIR/link_to_dir"
11+
ln -s target_dir/file.txt "$TEST_DIR/link_to_file"
12+
13+
cd "$TEST_DIR"
14+
15+
DIR="$(dirname "$0")"
16+
AGENTFS="cargo run --manifest-path $DIR/../Cargo.toml --"
17+
18+
# Test 1: Verify symlinks are reported as symlinks (not directories/files)
19+
# ls -la output starts with file type: 'l' for symlink, 'd' for directory
20+
output=$($AGENTFS run /bin/bash -c 'ls -la link_to_dir' 2>&1 || true)
21+
22+
# The output should contain a line starting with 'l' for the symlink
23+
if ! echo "$output" | grep -E "^l.* link_to_dir"; then
24+
echo "FAILED: symlink to directory not reported as symlink"
25+
echo "$output"
26+
rm -rf "$TEST_DIR"
27+
exit 1
28+
fi
29+
30+
# Test 2: Verify symlink to file is reported as symlink
31+
output=$($AGENTFS run /bin/bash -c 'ls -la link_to_file' 2>&1 || true)
32+
33+
if ! echo "$output" | grep -E "^l.* link_to_file"; then
34+
echo "FAILED: symlink to file not reported as symlink"
35+
echo "$output"
36+
rm -rf "$TEST_DIR"
37+
exit 1
38+
fi
39+
40+
# Test 3: Verify rm can remove symlink to directory (this was the original bug)
41+
# Previously this would fail with "Is a directory" because symlinks were misidentified
42+
output=$($AGENTFS run /bin/bash -c 'rm link_to_dir && echo "symlink removed successfully"' 2>&1 || true)
43+
44+
if ! echo "$output" | grep -q "symlink removed successfully"; then
45+
echo "FAILED: could not remove symlink to directory"
46+
echo "$output"
47+
rm -rf "$TEST_DIR"
48+
exit 1
49+
fi
50+
51+
# Test 4: Verify the target directory still exists (we removed the symlink, not the target)
52+
output=$($AGENTFS run /bin/bash -c 'cat target_dir/file.txt' 2>&1 || true)
53+
54+
if ! echo "$output" | grep -q "test content"; then
55+
echo "FAILED: target directory should still exist after removing symlink"
56+
echo "$output"
57+
rm -rf "$TEST_DIR"
58+
exit 1
59+
fi
60+
61+
# Cleanup
62+
rm -rf "$TEST_DIR"
63+
64+
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)