fix(wasi): resolve path_open paths against preopen directory, not cwd#27724
fix(wasi): resolve path_open paths against preopen directory, not cwd#27724mkusaka wants to merge 3 commits intooven-sh:mainfrom
Conversation
path_open resolves file paths against cwd instead of the preopen directory's host path. This causes absolute paths to be doubled (e.g., /tmp/foo becomes <cwd>/tmp/foo) and fail with ENOENT. read-file.wasm is a minimal Rust WASI binary (wasm32-wasip1) that reads a file from args and prints its contents. Built with: cargo build --target wasm32-wasip1 --release
path_open was discarding the CHECK_FD result and resolving guest paths with path.resolve(p), which uses cwd as the base. This caused absolute paths like /tmp/foo to become <cwd>/tmp/foo, resulting in ENOENT. Capture the CHECK_FD result and use stats.path (the preopen host path) as the base directory, matching the pattern used by path_readlink, path_create_directory, path_unlink_file, and other path syscalls.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
WalkthroughPath resolution for WASI path_open now resolves relative to the opened directory (preopen) by using the directory's stored path when constructing the unresolved path. A test was added to verify path_open resolves against preopens rather than the current working directory. Changes
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/js/bun/wasm/wasi.test.js`:
- Around line 4-6: Replace direct use of fs.mkdtempSync with the test harness
helper: import and call tempDir (from harness) instead of mkdtempSync to create
the temporary directory used in this test (references: mkdtempSync -> tempDir).
Remove the explicit tmpdir/mkdtempSync import and any manual cleanup (rmSync)
for that directory and rely on harness-managed cleanup; keep using writeFileSync
and join to create files inside the tempDir path. Update any other occurrences
(lines ~29-31) that create or remove temp dirs to use tempDir consistently.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
test/js/bun/wasm/read-file.wasmis excluded by!**/*.wasm
📒 Files selected for processing (2)
src/js/node/wasi.tstest/js/bun/wasm/wasi.test.js
Replace manual mkdtempSync/rmSync with harness's tempDirWithFiles helper per Bun coding guidelines.
What does this PR do?
Fixes
path_openin the WASI implementation to resolve guest paths against the preopen directory's host path instead of the current working directory.Bug
When a WASI guest opens a file using an absolute path (e.g.,
/tmp/foo.txt) withpreopens: { "/": "/" }, Bun incorrectly resolves it as<cwd>/tmp/foo.txtinstead of/tmp/foo.txt, causing ENOENT.Root cause:
path_openinsrc/js/node/wasi.tsdiscards theCHECK_FD()result and callspath.resolve(p), which uses cwd as the base. All other path syscalls (path_readlink,path_create_directory,path_unlink_file, etc.) correctly usepath.resolve(stats.path, p).Fix
Two-line change:
Test
Added a test that reads a temp file via a WASI binary to verify
path_openresolves against preopens.read-file.wasmis a minimal Rust WASI binary (wasm32-wasip1, 91KB) that reads a file path from args and prints contents to stdout. Built with:How did you verify your code works?