Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/js/node/wasi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1437,7 +1437,7 @@ var require_wasi = __commonJS({
path_open: wrap(
(dirfd, _dirflags, pathPtr, pathLen, oflags, fsRightsBase, fsRightsInheriting, fsFlags, fdPtr) => {
try {
CHECK_FD(dirfd, constants_1.WASI_RIGHT_PATH_OPEN);
const stats = CHECK_FD(dirfd, constants_1.WASI_RIGHT_PATH_OPEN);
fsRightsBase = BigInt(fsRightsBase);
fsRightsInheriting = BigInt(fsRightsInheriting);
const read =
Expand Down Expand Up @@ -1512,7 +1512,7 @@ var require_wasi = __commonJS({
if (p.startsWith("proc/")) {
throw new types_1.WASIError(constants_1.WASI_EBADF);
}
const fullUnresolved = path.resolve(p);
const fullUnresolved = stats.path ? path.resolve(stats.path, p) : path.resolve(p);
let full;
try {
full = fs.realpathSync(fullUnresolved);
Expand Down
89 changes: 89 additions & 0 deletions test/js/bun/wasm/preopen-wasi.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
// Minimal WASI program that opens a file from a preopen using path_open
// and reads it, then writes the contents to another file.
//
// We declare the WASI imports by hand so we don't depend on a WASI sysroot.
//
// Compile:
// clang --target=wasm32 -nostdlib -O2 -fno-builtin \
// -Wl,--no-entry -Wl,--export=_start -Wl,--export=memory \
// -Wl,--allow-undefined \
// -o preopen-wasi.wasm preopen-wasi.c
//
// Regression fixture for oven-sh/bun#30302.

typedef unsigned int u32;
typedef unsigned long long u64;

__attribute__((import_module("wasi_snapshot_preview1"), import_name("path_open")))
unsigned int wasi_path_open(u32 dirfd, u32 dirflags, const char *path, u32 path_len,
u32 oflags, u64 fs_rights_base, u64 fs_rights_inheriting,
u32 fdflags, u32 *opened_fd);

__attribute__((import_module("wasi_snapshot_preview1"), import_name("fd_read")))
unsigned int wasi_fd_read(u32 fd, const void *iovs, u32 iovs_len, u32 *nread);

__attribute__((import_module("wasi_snapshot_preview1"), import_name("fd_write")))
unsigned int wasi_fd_write(u32 fd, const void *iovs, u32 iovs_len, u32 *nwritten);

__attribute__((import_module("wasi_snapshot_preview1"), import_name("fd_close")))
unsigned int wasi_fd_close(u32 fd);

__attribute__((import_module("wasi_snapshot_preview1"), import_name("proc_exit")))
void wasi_proc_exit(u32 rval) __attribute__((noreturn));

struct ciovec {
const void *buf;
u32 buf_len;
};

// WASI rights for read/write files. Using the superset the Bun implementation
// grants to preopen directories keeps things simple.
#define RIGHTS_ALL ((u64)-1)

// O_CREAT | O_TRUNC in WASI oflags
#define WASI_O_CREAT (1 << 0)
#define WASI_O_TRUNC (1 << 3)

static char read_buf[256];
static char out_buf[256];

void _start() {
// Preopen dirfd is always 3 (first after stdin/stdout/stderr).
u32 dirfd = 3;

// Read "input.txt" from the preopen.
u32 in_fd = 0;
unsigned int err = wasi_path_open(dirfd, 0, "input.txt", 9, 0,
RIGHTS_ALL, RIGHTS_ALL, 0, &in_fd);
if (err != 0) wasi_proc_exit(10 + err);

struct ciovec read_iov = { read_buf, sizeof(read_buf) };
u32 nread = 0;
err = wasi_fd_read(in_fd, &read_iov, 1, &nread);
if (err != 0) wasi_proc_exit(30 + err);
wasi_fd_close(in_fd);

// Write "output.txt" in the preopen with "got: " prefix + contents.
u32 out_fd = 0;
err = wasi_path_open(dirfd, 0, "output.txt", 10,
WASI_O_CREAT | WASI_O_TRUNC,
RIGHTS_ALL, RIGHTS_ALL, 0, &out_fd);
if (err != 0) wasi_proc_exit(50 + err);

// Build "got: <contents>" in out_buf.
const char *prefix = "got: ";
u32 plen = 5;
for (u32 i = 0; i < plen; i++) out_buf[i] = prefix[i];
for (u32 i = 0; i < nread && plen + i < sizeof(out_buf); i++) {
out_buf[plen + i] = read_buf[i];
}
u32 total = plen + nread;

struct ciovec write_iov = { out_buf, total };
u32 nwritten = 0;
Comment on lines +77 to +83

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Clamp total to the number of bytes actually copied into out_buf.

nread is bounded during copy, but total = plen + nread is not. This can cause fd_write to read past valid output bytes.

Suggested fix
-    for (u32 i = 0; i < nread && plen + i < sizeof(out_buf); i++) {
+    u32 copied = 0;
+    for (u32 i = 0; i < nread && plen + i < sizeof(out_buf); i++) {
         out_buf[plen + i] = read_buf[i];
+        copied++;
     }
-    u32 total = plen + nread;
+    u32 total = plen + copied;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
for (u32 i = 0; i < nread && plen + i < sizeof(out_buf); i++) {
out_buf[plen + i] = read_buf[i];
}
u32 total = plen + nread;
struct ciovec write_iov = { out_buf, total };
u32 nwritten = 0;
u32 copied = 0;
for (u32 i = 0; i < nread && plen + i < sizeof(out_buf); i++) {
out_buf[plen + i] = read_buf[i];
copied++;
}
u32 total = plen + copied;
struct ciovec write_iov = { out_buf, total };
u32 nwritten = 0;
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/js/bun/wasm/preopen-wasi.c` around lines 77 - 83, The computed total
bytes variable can exceed the actual buffer capacity because total = plen +
nread doesn't account for the clamp in the copy loop; update the calculation of
total before creating write_iov so it is clamped to the number of bytes actually
copied (e.g., total = min(plen + nread, (u32)sizeof(out_buf)) or compute total =
plen + actual_copied) so fd_write / write_iov only sees the valid byte count;
adjust the total variable used by struct ciovec write_iov and any downstream
uses (variables: total, plen, nread, out_buf, write_iov, fd_write).

err = wasi_fd_write(out_fd, &write_iov, 1, &nwritten);
if (err != 0) wasi_proc_exit(70 + err);

wasi_fd_close(out_fd);
wasi_proc_exit(0);
}
Binary file added test/js/bun/wasm/preopen-wasi.wasm
Binary file not shown.
62 changes: 61 additions & 1 deletion test/js/bun/wasm/wasi.test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { spawnSync } from "bun";
import { expect, it } from "bun:test";
import { bunEnv, bunExe } from "harness";
import { bunEnv, bunExe, tempDir } from "harness";
import { join } from "node:path";

it("Should support printing 'hello world'", () => {
const { stdout, stderr, exitCode } = spawnSync({
Expand All @@ -20,3 +21,62 @@ it("Should support printing 'hello world'", () => {
exitCode: 0,
});
});

// node:wasi path_open must resolve the guest path against the preopen's
// mapped host directory, not against process.cwd(). Every other path_*
// handler in src/js/node/wasi.ts does this; path_open used to call
// path.resolve(p) (no base), making a WASM program that path_opens an
// entry under its preopen fail with ENOENT at cwd.
// Regression guard for oven-sh/bun#30302.
it("node:wasi path_open resolves against the preopen host dir, not cwd", async () => {
using dir = tempDir("wasi-preopen", {
"work/input.txt": "hello from host file",
// Deliberately place a wrong-looking file at `cwd/input.txt` so that
// the buggy cwd-relative lookup would pick this up instead of erroring
// — catches a regression that silently opens the wrong file.
"input.txt": "wrong file — should never be read",
"runner.mjs": `
import fs from "node:fs";
import { WASI } from "node:wasi";

const workDir = process.argv[2];
const wasmPath = process.argv[3];
const wasi = new WASI({
version: "preview1",
preopens: { "/work": workDir },
});
const wasmBytes = fs.readFileSync(wasmPath);
const module = await WebAssembly.compile(wasmBytes);
const instance = await WebAssembly.instantiate(module, wasi.getImports(module));
try {
wasi.start(instance);
} catch (err) {
process.stderr.write("wasi.start threw: " + (err?.message ?? err) + "\\n");
process.exit(2);
}
`,
});

const cwd = String(dir);
const workDir = join(cwd, "work");
const wasmPath = join(import.meta.dir, "preopen-wasi.wasm");

await using proc = Bun.spawn({
cmd: [bunExe(), "runner.mjs", workDir, wasmPath],
env: bunEnv,
cwd,
stdout: "pipe",
stderr: "pipe",
});

const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]);

// The WASM program proc_exits(0) on success; non-zero encodes which step
// failed. See test/js/bun/wasm/preopen-wasi.c.
expect(stderr).toBe("");
expect(exitCode).toBe(0);

// The preopen points at <cwd>/work, so the output file must land there,
// with "got: " prefixed to the host-dir input's contents.
expect(await Bun.file(join(workDir, "output.txt")).text()).toBe("got: hello from host file");
Comment on lines +72 to +81

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Normalize ASAN stderr noise and assert exitCode last.

This subprocess test uses bunEnv, so stderr should ignore the known ASAN startup warning, and exitCode should be asserted after content/filesystem checks.

Suggested fix
-  const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]);
+  const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]);
+  const filteredStderr = stderr
+    .split("\n")
+    .filter(line => !line.startsWith("WARNING: ASAN interferes"))
+    .filter(Boolean)
+    .join("\n");
@@
-  expect(stderr).toBe("");
-  expect(exitCode).toBe(0);
+  expect(filteredStderr).toBe("");
@@
   expect(await Bun.file(join(workDir, "output.txt")).text()).toBe("got: hello from host file");
+  expect(exitCode).toBe(0);

Based on learnings: assert command exit code last in Bun tests, and filter WARNING: ASAN interferes lines from subprocess stderr when using bunEnv.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]);
// The WASM program proc_exits(0) on success; non-zero encodes which step
// failed. See test/js/bun/wasm/preopen-wasi.c.
expect(stderr).toBe("");
expect(exitCode).toBe(0);
// The preopen points at <cwd>/work, so the output file must land there,
// with "got: " prefixed to the host-dir input's contents.
expect(await Bun.file(join(workDir, "output.txt")).text()).toBe("got: hello from host file");
const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]);
const filteredStderr = stderr
.split("\n")
.filter(line => !line.startsWith("WARNING: ASAN interferes"))
.filter(Boolean)
.join("\n");
// The WASM program proc_exits(0) on success; non-zero encodes which step
// failed. See test/js/bun/wasm/preopen-wasi.c.
expect(filteredStderr).toBe("");
// The preopen points at <cwd>/work, so the output file must land there,
// with "got: " prefixed to the host-dir input's contents.
expect(await Bun.file(join(workDir, "output.txt")).text()).toBe("got: hello from host file");
expect(exitCode).toBe(0);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/js/bun/wasm/wasi.test.js` around lines 72 - 81, The test should ignore
the known ASAN startup warning in the subprocess stderr and assert the process
exit code after validating filesystem/content; update the block that reads
proc.stderr.text() and proc.exited (variables stderr, exitCode, proc) to filter
out lines that start with or contain "WARNING: ASAN interferes" (or similar ASAN
startup text) from stderr before asserting it, perform the file content check
against Bun.file(join(workDir, "output.txt")).text() first, and only then assert
expect(exitCode).toBe(0); ensure the stderr assertion uses the filtered value
(expect(filteredStderr).toBe("")).

});
Loading