Add open-time authorization hook#1
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6cf7487751
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if let Err(e) = open_support.open(request_ctx, &handle, request).await { | ||
| return NfsResop4::Open(e.to_nfsstat4(), None); |
There was a problem hiding this comment.
When a client sends OPEN4_CREATE for a missing regular file and OpenSupport::open denies the write, this return happens after the earlier create_file(...) path has already created and registered the backend object. The client sees NFS4ERR_ACCESS with no stateid, but the unauthorized file is left behind, so approval-gated write opens can still mutate the exported filesystem via failed creates; authorize the create/open before committing the backend creation or roll it back on hook failure.
Useful? React with 👍 / 👎.
| closer | ||
| .close(request_ctx, &handle) | ||
| .await |
There was a problem hiding this comment.
This invokes CloseSupport::close before state.close_state, whose implementation can still reject the CLOSE with NFS4ERR_LOCKS_HELD when byte-range locks are active. In that locked-open scenario, the backend close/flush hook runs even though the NFS open state remains active and the client is expected to unlock and retry CLOSE, which can prematurely flush or duplicate close-side effects.
Useful? React with 👍 / 👎.
| /// OPEN with write share access runs optional OpenSupport before a stateid is | ||
| /// granted and returns the hook's NFS error. |
There was a problem hiding this comment.
The repository instructions for integration tests require the doc comment to be exactly one summary line followed by one Origin: line and one RFC: line. This new test splits the summary over two /// lines, making it the only four-line test doc block under crates/embednfs/tests and violating the documented test format.
Useful? React with 👍 / 👎.
1 participant
Summary
OpenSupporthook to the filesystem trait.OPENbefore issuing a stateid when the client requests write share access.OPENstatus so clients see failures fromopen(2)instead of delayed write/commit/close paths.NFS4ERR_ACCESSbefore a stateid is granted.Why
Bloom needs approval-gated write sinks to fail synchronously for shell redirects. NFS clients can cache writes and shells often ignore close-time errors, so value-moving or approval-gated writes need an open-time authorization point.
This supports Bloom PR bloom-directory/bloom#92 and issue bloom-directory/bloom#77.
Validation
cargo test -p embednfsbloom-mountto6cf74877518ea0b5e7f9d3c1fa5ee0b461cd3eb3:BLOOM_MOUNT_TEST_REQUIRE_REAL=1 cargo test -p bloom-mount --features mount --test write_permission_denied -- --ignored --nocapture