Skip to content
Draft
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
32 changes: 21 additions & 11 deletions src/presets/cloudflare/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -228,17 +228,27 @@ export async function writeWranglerConfig(nitro: Nitro, cfTarget: "pages" | "mod
overrides.pages_build_output_dir = relative(wranglerConfigDir, nitro.options.output.dir);
} else {
// Modules
overrides.main = relative(wranglerConfigDir, join(nitro.options.output.serverDir, "index.mjs"));
overrides.assets = {
binding: "ASSETS",
directory: relative(
const assetsDirectory = relative(
wranglerConfigDir,
resolve(
nitro.options.output.publicDir,
"..".repeat(nitro.options.baseURL.split("/").filter(Boolean).length)
)
);
Comment on lines +231 to +237
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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Confirm the `"..".repeat` pattern's behavior and check if it appears elsewhere in the repo.
rg -nP '\.\.".repeat\(' --type=ts -C2

# Sanity check pathe resolve semantics
cat <<'EOF' > /tmp/check.mjs
import { resolve } from "pathe";
console.log("repeat(0):", JSON.stringify("..".repeat(0)));
console.log("repeat(2):", JSON.stringify("..".repeat(2)));
console.log("resolve depth 2 (broken):", resolve("/tmp/out/public", "..".repeat(2)));
console.log("resolve depth 2 (spread):", resolve("/tmp/out/public", "..", ".."));
EOF
# (Cannot execute in sandbox; included for reference.)
cat /tmp/check.mjs

Repository: nitrojs/nitro

Length of output: 703


🏁 Script executed:

# First, check if pathe is available in the repo
cd /tmp && node --input-type=module <<'EOF'
import { resolve } from "pathe";

console.log("=== String repetition results ===");
console.log('"..".repeat(0):', JSON.stringify("..".repeat(0)));
console.log('"..".repeat(1):', JSON.stringify("..".repeat(1)));
console.log('"..".repeat(2):', JSON.stringify("..".repeat(2)));
console.log('"..".repeat(3):', JSON.stringify("..".repeat(3)));

console.log("\n=== pathe.resolve() behavior ===");
const base = "/app/dist/public";

// Test with broken input (4 consecutive dots)
const brokenPath = resolve(base, "..".repeat(2));
console.log("resolve(base, '....'):", brokenPath);

// Test with correct input (spread operator)
const correctPath1 = resolve(base, "..", "..");
console.log("resolve(base, '..', '..'):", correctPath1);

// Test with correct input (alternative: "../" repeated)
const correctPath2 = resolve(base, "../".repeat(2).slice(0, -1));
console.log("resolve(base, '../'.repeat(2).slice(0,-1)):", correctPath2);

console.log("\n=== Comparison ===");
console.log("brokenPath === correctPath1:", brokenPath === correctPath1);
console.log("correctPath1 === correctPath2:", correctPath1 === correctPath2);
EOF

Repository: nitrojs/nitro

Length of output: 1045


🏁 Script executed:

# Search for tests related to cloudflare preset and assetsDirectory
fd -e test.ts -e spec.ts -e test.js | xargs grep -l "cloudflare\|assetsDirectory" 2>/dev/null || echo "No direct test files found"

# Search in test directory
fd . test --type f 2>/dev/null | head -20

Repository: nitrojs/nitro

Length of output: 855


🏁 Script executed:

# Check the actual code context around the fix to understand the impact
sed -n '220,250p' src/presets/cloudflare/utils.ts

Repository: nitrojs/nitro

Length of output: 1109


🏁 Script executed:

# Check the cloudflare utils test file for baseURL and assetsDirectory tests
cat -n test/unit/cloudflare.utils.test.ts

Repository: nitrojs/nitro

Length of output: 3232


🏁 Script executed:

# Also check the presets test files for baseURL coverage
rg -n "baseURL" test/presets/cloudflare-module.test.ts test/presets/cloudflare-pages.test.ts --context=3

Repository: nitrojs/nitro

Length of output: 39


🏁 Script executed:

# Check pathe package source or docs to understand resolve behavior
fd package.json -x grep -l pathe {} \; | head -5

Repository: nitrojs/nitro

Length of output: 72


🏁 Script executed:

# Let's verify the JavaScript string behavior and reason about resolve semantics
node --eval='
console.log("JavaScript string repeat:");
console.log("\"..\" + \"..\" =", "\"..\" + \"..\"" + " -> " + (".." + ".."));
console.log(".repeat(2) =", ".repeat(2)" + " -> " + ("..".repeat(2)));
console.log("\nNode.js path.resolve semantics:");
console.log("path.resolve treats \"..\" as parent dir reference");
console.log("path.resolve treats \".....\" (or any non-.. string) as a directory name");
console.log("\nTherefore:");
console.log("resolve(base, \"..\", \"..\") -> climbs 2 levels");
console.log("resolve(base, \"....\") -> resolves to base/.... (literal directory)");
'

Repository: nitrojs/nitro

Length of output: 426


🏁 Script executed:

# Check if there are any tests with baseURL other than "/"
rg "baseURL" test/ -t ts --context=2 | head -30

Repository: nitrojs/nitro

Length of output: 2024


🏁 Script executed:

# Verify the proposed fix syntax is valid TypeScript
cat > /tmp/test_fix.ts << 'EOF'
import { resolve } from "pathe";

// Original broken approach
const broken = resolve("/tmp", "..".repeat(2));

// Proposed fix: spread operator
const fixed1 = resolve("/tmp", ...Array.from(
  { length: 2 },
  () => ".."
));

// Alternative mentioned in the comment
const fixed2 = resolve("/tmp", "../".repeat(2).slice(0, -1));

console.log("Broken:", broken);
console.log("Fixed1:", fixed1);
console.log("Fixed2:", fixed2);
EOF
cat /tmp/test_fix.ts

Repository: nitrojs/nitro

Length of output: 481


"..".repeat(N) produces broken paths for baseURL depth > 1.

"..".repeat(2) returns the string "...." (a single 4-dot segment), not "../..". pathe.resolve(publicDir, "....") then treats "...." as a literal directory name and resolves to publicDir + "/...." instead of climbing two levels up. So assetsDirectory is incorrect for any baseURL with two or more non-empty segments (e.g. "/foo/bar/"). The default baseURL = "/" (length 0 β†’ "") and single-segment baseURL = "/foo/" (length 1 β†’ "..") happen to work, which is why the new tests don't catch this.

This may be pre-existing (just extracted into a variable here), but the diff surface is a good place to fix it.

πŸ› Proposed fix
     const assetsDirectory = relative(
       wranglerConfigDir,
       resolve(
         nitro.options.output.publicDir,
-        "..".repeat(nitro.options.baseURL.split("/").filter(Boolean).length)
+        ...Array.from(
+          { length: nitro.options.baseURL.split("/").filter(Boolean).length },
+          () => ".."
+        )
       )
     );

Note: pathe.resolve is variadic, so spreading multiple ".." segments climbs the directory tree correctly for any depth. Alternatively, "../".repeat(N).slice(0, -1) as a single argument also resolves correctly.

πŸ€– 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 `@src/presets/cloudflare/utils.ts` around lines 231 - 237, assetsDirectory
calculation uses "..".repeat(N) which concatenates dots (e.g. "....") instead of
producing repeated "../" segments, breaking path traversal for baseURL depth >1;
update the resolve call that computes assetsDirectory (the call using
wranglerConfigDir, resolve(nitro.options.output.publicDir, "..".repeat(...))) to
supply N separate ".." path segments rather than a single repeated-dot
stringβ€”e.g., compute count =
nitro.options.baseURL.split("/").filter(Boolean).length and pass either an array
of ".." repeated count spread into resolve or a single
"../".repeat(count).slice(0,-1) string so pathe.resolve correctly climbs N
directories from nitro.options.output.publicDir when resolving against
wranglerConfigDir.

if (nitro.options.static) {
// No worker script is emitted; `main` and `assets.binding` only apply
// when there is one. https://developers.cloudflare.com/workers/static-assets/
overrides.assets = { directory: assetsDirectory };
} else {
overrides.main = relative(
wranglerConfigDir,
resolve(
nitro.options.output.publicDir,
"..".repeat(nitro.options.baseURL.split("/").filter(Boolean).length)
)
),
};
join(nitro.options.output.serverDir, "index.mjs")
);
overrides.assets = {
binding: "ASSETS",
directory: assetsDirectory,
};
}
}

// Read user config
Expand Down Expand Up @@ -274,7 +284,7 @@ export async function writeWranglerConfig(nitro: Nitro, cfTarget: "pages" | "mod
wranglerConfig.compatibility_flags.push("nodejs_compat");
}

if (cfTarget === "module") {
if (cfTarget === "module" && !nitro.options.static) {
// Avoid double bundling
if (wranglerConfig.no_bundle === undefined) {
wranglerConfig.no_bundle = true;
Expand Down
72 changes: 72 additions & 0 deletions test/unit/cloudflare.utils.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
import { mkdtempSync, readFileSync, rmSync } from "node:fs";
import { tmpdir } from "node:os";
import { join } from "pathe";
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { writeWranglerConfig } from "../../src/presets/cloudflare/utils.ts";

function createNitroStub(overrides: { static?: boolean } = {}) {
const root = mkdtempSync(join(tmpdir(), "nitro-cf-wrangler-"));
return {
root,
nitro: {
options: {
static: overrides.static ?? false,
baseURL: "/",
rootDir: root,
workspaceDir: root,
output: {
dir: join(root, ".output"),
serverDir: join(root, ".output/server"),
publicDir: join(root, ".output/public"),
},
compatibilityDate: { cloudflare: "2025-10-24", default: "2025-10-24" },
cloudflare: { deployConfig: true, nodeCompat: true },
experimental: {},
scheduledTasks: {},
},
logger: {
info: vi.fn(),
warn: vi.fn(),
error: vi.fn(),
success: vi.fn(),
},
} as any,
};
}

describe("writeWranglerConfig (cloudflare-module)", () => {
let cleanup: string[] = [];
beforeEach(() => {
cleanup = [];
});
afterEach(() => {
for (const dir of cleanup) {
rmSync(dir, { recursive: true, force: true });
}
});

it("emits main and ASSETS binding for a normal (non-static) build", async () => {
const { root, nitro } = createNitroStub({ static: false });
cleanup.push(root);
await writeWranglerConfig(nitro, "module");
const config = JSON.parse(readFileSync(join(root, ".output/server/wrangler.json"), "utf8"));
expect(config.main).toBe("index.mjs");
expect(config.assets).toEqual({ binding: "ASSETS", directory: "../public" });
expect(config.no_bundle).toBe(true);
expect(config.rules).toEqual([{ type: "ESModule", globs: ["**/*.mjs", "**/*.js"] }]);
});

it("omits main, ASSETS binding and module bundling rules for a static build", async () => {
// Regression test for nuxt/nuxt#34186 β€” `nuxt generate` (which sets
// `nitro.options.static = true`) targeting cloudflare_module previously
// wrote a wrangler.json pointing at a non-existent index.mjs.
const { root, nitro } = createNitroStub({ static: true });
cleanup.push(root);
await writeWranglerConfig(nitro, "module");
const config = JSON.parse(readFileSync(join(root, ".output/server/wrangler.json"), "utf8"));
expect(config.main).toBeUndefined();
expect(config.assets).toEqual({ directory: "../public" });
expect(config.no_bundle).toBeUndefined();
expect(config.rules).toBeUndefined();
});
});
Loading