[Merlin0220]

Fixes #3552.

Problem

The NFS proxy creates a helpers.CachingHandler to cache directory listings by their NFS cookie verifier. However, the caching handler is subsequently wrapped by tracing, metrics, logging, and recovery middleware.

go-nfs detects verifier caching support by asserting the outermost handler as nfs.CachingHandler:

handler, ok := userHandle.(nfs.CachingHandler)

The middleware handlers only implemented nfs.Handler, so this assertion failed before reaching the underlying caching handler. Because the recovery middleware is always installed, verifier caching was bypassed regardless of whether the optional tracing, metrics, or logging middleware was enabled.

As a result, paginated READDIR and READDIRPLUS requests could not reuse the directory listing associated with their verifier and had to read the directory again.

Fix

Preserve nfs.CachingHandler through the complete middleware chain by forwarding VerifierFor and DataForVerifier to the wrapped handler.

Behavior	Before	After
Outermost handler implements nfs.CachingHandler	No	Yes
Verifier cache reaches helpers.CachingHandler	No	Yes
Directory listings can be reused across paginated requests	No	Yes

Changes

• Add nfs.CachingHandler compile-time interface assertions to the tracing, metrics, logging, and recovery handlers.
• Forward VerifierFor and DataForVerifier through each middleware layer.
• Preserve panic recovery for verifier cache operations in the recovery handler.
• Add a middleware-chain test that verifies the outer handler satisfies nfs.CachingHandler and forwards arguments and return values to the underlying handler.

Testing

go test ./pkg/nfsproxy/... -count=1
go test -race ./pkg/nfsproxy/... -count=1

Risk: activeVerifiers Cache Key Collision Across Sandboxes

Enabling verifier caching introduces a potential cache-key collision in
activeVerifiers. If the mount path and an enumerated file name are identical,
they may produce the same ID. This could cause cache entries to be reused across
different sandboxes, violating cache isolation and potentially leading to
incorrect verifier or file-handle resolution.

If this scenario is possible, the current cache should not be used as-is.
The cache should either be disabled or redesigned with mount-point-scoped, collision-resistant keys.

This risk is related to #3555.
The proposed solution is to allocate an independent file-handle cache for each
mount point, using the mount-point identity as the cache
shard key. This prevents cache eviction, lookup, and synchronization in one
sandbox from affecting another.

[cla-bot]

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: shangyan02.
This is most likely caused by a git client misconfiguration; please make sure to:

1. check if your git client is configured with an email to sign commits git config --list | grep email
2. If not, set it up using git config --global user.email email@example.com
3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

[cla-bot]

We require contributors to sign our Contributor License Agreement, and we don't have @Merlin0220 on file. You can sign our CLA at https://e2b.dev/docs/cla . Once you've signed, post a comment here that says '@cla-bot check'

[cla-bot]

The cla-bot has been summoned, and re-checked this pull request!

[cla-bot]

The cla-bot has been summoned, and re-checked this pull request!

[Merlin0220]

We require contributors to sign our Contributor License Agreement, and we don't have @Merlin0220 on file. You can sign our CLA at https://e2b.dev/docs/cla . Once you've signed, post a comment here that says '@cla-bot check'

@cla-bot check

[cla-bot]

The cla-bot has been summoned, and re-checked this pull request!

[leonmeijer]

This exposes one process-wide verifier cache across every mounted sandbox, but the pinned go-nfs caching handler derives a verifier only from the namespace-relative path and entry names and DataForVerifier does not validate the path or filesystem. Two sandboxes with the same directory path and names can therefore collide and receive each others cached FileInfo listings on subsequent READDIR pages. Isolate verifier state by mounted filesystem or include and validate mount identity before enabling this passthrough.

[ryan-sana]

Thank you, @Merlin0220. This was a sharp catch: go-nfs only checks the outermost handler for nfs.CachingHandler, so wrapping the caching handler in middleware quietly switched verifier caching off.

Main fixes it from the other side in 5457911: a new directory-verifier cache is applied last, after recovery, so go-nfs finds it on the outermost handler and paginated READDIR/READDIRPLUS reuse the cached listing again.

Since the problem is solved on main, I'm closing this PR. Your test that asserts the cache passes through the middleware chain was a nice touch. Thanks for taking the time to work this out.

[Merlin0220]

This exposes one process-wide verifier cache across every mounted sandbox, but the pinned go-nfs caching handler derives a verifier only from the namespace-relative path and entry names and DataForVerifier does not validate the path or filesystem. Two sandboxes with the same directory path and names can therefore collide and receive each others cached FileInfo listings on subsequent READDIR pages. Isolate verifier state by mounted filesystem or include and validate mount identity before enabling this passthrough.

Yes. See #3592. My current implementation maintains a separate file handle cache for each mount point. I also modified go-nfs to support this behavior by including a mountID when resolving the file handle cache, which is used to map the handle to its corresponding mount point. This prevents the issue from occurring.

[Merlin0220]

#3592

Thank you, @Merlin0220. This was a sharp catch: go-nfs only checks the outermost handler for nfs.CachingHandler, so wrapping the caching handler in middleware quietly switched verifier caching off.

Main fixes it from the other side in 5457911: a new directory-verifier cache is applied last, after recovery, so go-nfs finds it on the outermost handler and paginated READDIR/READDIRPLUS reuse the cached listing again.

Since the problem is solved on main, I'm closing this PR. Your test that asserts the cache passes through the middleware chain was a nice touch. Thanks for taking the time to work this out.

Thank you for accepting the PR. However, there is still room for improvement: ideally, CachingHandler should be used together with the changes in #3592. Otherwise, identical paths in different sandboxes may end up sharing the same cache entry.
In my current implementation, I added a mountID field to the file handle to identify the specific mount point, and each verifier cache is also maintained separately per mount point.

[Merlin0220]

Thank you, @Merlin0220. This was a sharp catch: go-nfs only checks the outermost handler for nfs.CachingHandler, so wrapping the caching handler in middleware quietly switched verifier caching off.

Main fixes it from the other side in 5457911: a new directory-verifier cache is applied last, after recovery, so go-nfs finds it on the outermost handler and paginated READDIR/READDIRPLUS reuse the cached listing again.

Since the problem is solved on main, I'm closing this PR. Your test that asserts the cache passes through the middleware chain was a nice touch. Thanks for taking the time to work this out.

Additionally, I noticed that the current main branch has already implemented similar per-mount-point cache allocation, organized by volume + lifecycleID. If this provides functionality similar to #3592, please leave a comment and close the PR.