Skip to content

nfs: don't reject WRITE on read-only-mode files (fixes git fsync EPERM on macOS)#339

Open
connorblack wants to merge 2 commits into
tursodatabase:mainfrom
connorblack:fix/nfs-fsync-readonly-mode-eperm
Open

nfs: don't reject WRITE on read-only-mode files (fixes git fsync EPERM on macOS)#339
connorblack wants to merge 2 commits into
tursodatabase:mainfrom
connorblack:fix/nfs-fsync-readonly-mode-eperm

Conversation

@connorblack

Copy link
Copy Markdown

Summary

On macOS, agentfs run exposes the copy-on-write overlay via a localhost NFSv3 server. nfsproc3_write gated every WRITE on the file's current mode bits (permissions::can_write), returning NFS3ERR_ACCES when the owner-write bit was clear.

That's incorrect for NFSv3: the protocol is stateless and has no OPEN procedure, so the server never sees the open(2) that authorized the descriptor. POSIX checks write permission once, at open time — later write(2)/fsync(2) on an already-open writable fd must succeed regardless of the file's mode bits.

Impact: git is unusable inside agentfs run on macOS

git creates loose objects with git_mkstemp_mode(tmp, 0444) (read-only mode, writable fd), writes, then fsyncs in close_loose_object(). On macOS the NFS client buffers the write and flushes it as a synchronous WRITE RPC at fsync/close — which hit the can_write check:

$ agentfs run -- bash -lc 'git add -A'
fatal: error when closing loose object file: Permission denied

So git add / git commit fail in the sandbox, which breaks the documented agentic-coding "commit + push" workflow on macOS.

Minimal reproduction

import os
fd = os.open("probe", os.O_WRONLY | os.O_CREAT | os.O_EXCL, 0o444)  # read-only mode, writable fd
os.write(fd, b"x" * 64)   # ok
os.fsync(fd)              # before this change: OSError errno=13 (Permission denied)

Run it from inside the overlaid cwd — on macOS /tmp is a host passthrough, so a path there does not exercise the NFS server.

Fix

Remove the mode-bit gate from the WRITE handler. Write authorization is enforced where NFSv3 expects it: the client kernel checks permission at open time via the ACCESS procedure (nfsproc3_access -> permissions::compute_access), which is unchanged — unauthorized opens are still rejected. This also matches upstream nfsserve, whose WRITE handler performs no mode-bit check.

Tests

  • New cli/src/nfsserve/nfs_handlers_write_perm_test.rs drives nfsproc3_write end-to-end over a real AgentNFS VFS: asserts WRITE on a 0444-mode file owned by the caller returns NFS3_OK (regression: was NFS3ERR_ACCES), and that a 0644 file still writes; reads the bytes back to avoid a vacuous pass.
  • cargo test passes (cli 32, sdk/rust 101); cargo clippy clean; git add/git commit verified working inside agentfs run with a freshly built binary.

Note

nfsproc3_setattr carries the same-class gate on truncate (a can_write check before ftruncate). It is not exercised by this bug (git never truncates a read-only loose object), so it is left as a follow-up to keep this change focused.

…M on macOS)

The macOS `agentfs run` overlay is exposed over a localhost NFSv3 server.
`nfsproc3_write` gated every WRITE on the file's current mode bits via
`permissions::can_write`, returning NFS3ERR_ACCES when the owner-write bit was
clear. That is wrong for NFSv3: the protocol is stateless and has no OPEN
procedure, so the server never sees the open(2) that authorized the descriptor.
POSIX checks write permission once, at open time; later write(2)/fsync(2) on an
already-open writable fd must succeed regardless of the file's mode bits.

git relies on exactly this pattern: it creates loose objects with
`git_mkstemp_mode(tmp, 0444)` (read-only mode, writable fd), writes, then fsyncs
in `close_loose_object()`. On macOS the NFS client buffers the write and only
flushes it as a synchronous WRITE RPC at fsync/close time, which hit the
can_write check and failed with "fatal: error when closing loose object file:
Permission denied" — making `git add`/`git commit` unusable inside `agentfs run`.

Remove the mode-bit gate from the WRITE handler. Write authorization is enforced
where NFSv3 expects it: the client kernel checks permission at open time via the
ACCESS procedure (`nfsproc3_access` -> `permissions::compute_access`), which is
unchanged. This matches upstream nfsserve, whose WRITE handler performs no
mode-bit check.

Adds nfs_handlers_write_perm_test.rs driving nfsproc3_write end-to-end over a
real AgentNFS VFS: asserts WRITE on a 0444-mode file owned by the caller returns
NFS3_OK (regression: was NFS3ERR_ACCES) and that an ordinary 0644 file still
writes.

Verified with a freshly built binary: the documented fsync repro and a real
`git add`/`git commit` now succeed inside `agentfs run`; cli (32) and sdk (101)
test suites pass; clippy clean.
Copilot AI review requested due to automatic review settings June 15, 2026 22:01

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds regression coverage and adjusts NFSv3 WRITE handling to avoid rejecting writes based on file mode bits, fixing a macOS agentfs run failure when git writes to loose objects with mode 0444.

Changes:

  • Remove the mode-bit write permission gate in nfsproc3_write.
  • Add regression tests to ensure WRITE succeeds on 0444 and 0644 files.
  • Wire the new test module into nfs_handlers.rs under cfg(test).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
cli/src/nfsserve/nfs_handlers.rs Removes server-side mode-bit gating for WRITE and registers new regression tests.
cli/src/nfsserve/nfs_handlers_write_perm_test.rs Adds tokio-based regression tests validating WRITE behavior for readonly/writable modes.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread cli/src/nfsserve/nfs_handlers.rs Outdated
Comment thread cli/src/nfsserve/nfs_handlers_write_perm_test.rs Outdated
Comment thread cli/src/nfsserve/nfs_handlers_write_perm_test.rs
@connorblack

Copy link
Copy Markdown
Author

Follow-up on this branch: while addressing the review nits I found a second real bug on the same NFSv3 permission path and folded it into head 8323418.

In nfsproc3_setattr(), a stale guarded SETATTR was serializing NFS3ERR_NOT_SYNC but still falling through into context.vfs.setattr(...), which meant the stale request could still mutate the file and append a second RPC reply into the same response buffer. Head now returns immediately after the NOT_SYNC reply and adds cli/src/nfsserve/nfs_handlers_setattr_guard_test.rs covering three invariants: stale guarded SETATTR returns NFS3ERR_NOT_SYNC, the requested mutation is not applied, and the serialized response contains exactly one reply.

Local verification on the pushed head:

  • cargo test --manifest-path cli/Cargo.toml -- --nocapture -> 33 passed, 0 failed
  • upstream CI on 8323418 is green.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants