diff --git a/src/cli/_utils.ts b/src/cli/_utils.ts index 5459e377..8db29a52 100644 --- a/src/cli/_utils.ts +++ b/src/cli/_utils.ts @@ -1,7 +1,16 @@ // Colors support for terminal output +// F58: honor the NO_COLOR convention (ANY non-empty value disables) and only +// emit escape sequences to an interactive TTY (unless FORCE_COLOR is set). const noColor = /* @__PURE__ */ (() => { - const env = globalThis.process?.env ?? {}; - return env.NO_COLOR === "1" || env.TERM === "dumb"; + const proc = globalThis.process; + const env = proc?.env ?? {}; + if (env.FORCE_COLOR) { + return false; + } + if (env.NO_COLOR || env.TERM === "dumb") { + return true; + } + return !proc?.stdout?.isTTY; })(); const _c = diff --git a/src/cli/fetch.ts b/src/cli/fetch.ts index b8555023..581a6d24 100644 --- a/src/cli/fetch.ts +++ b/src/cli/fetch.ts @@ -55,7 +55,10 @@ export async function cliFetch( fetchHandler = loaded.fetch; } else { stderr.write(`* Fetching remote URL: ${inputURL}\n`); - if (!URL?.canParse(inputURL)) { + // F44: `URL.canParse("localhost:3000/api")` is true (scheme "localhost:"), + // so only treat inputs with an explicit `scheme://` as absolute URLs; + // everything else (host, host:port, host/path) is a schemeless http host. + if (!/^[a-zA-Z][a-zA-Z\d+\-.]*:\/\//.test(inputURL)) { inputURL = `http${cliOpts.tls ? "s" : ""}://${inputURL}`; } fetchHandler = globalThis.fetch; @@ -85,9 +88,11 @@ export async function cliFetch( // Build body let body: BodyInit | undefined; + let isStream = false; if (cliOpts.data !== undefined) { if (cliOpts.data === "@-") { // Read from stdin + isStream = true; body = new ReadableStream({ async start(controller) { for await (const chunk of stdin) { @@ -98,6 +103,7 @@ export async function cliFetch( }); } else if (cliOpts.data.startsWith("@")) { // Read from file as stream + isStream = true; body = Readable.toWeb(createReadStream(cliOpts.data.slice(1))) as unknown as ReadableStream; } else { body = cliOpts.data; @@ -110,11 +116,12 @@ export async function cliFetch( inputURL, `http${cliOpts.tls ? "s" : ""}://${cliOpts.host || cliOpts.hostname || "localhost"}`, ); - const req = new Request(url, { - method, - headers, - body, - }); + // F39: a streamed body (`-d @-` / `-d @file`) requires `duplex: "half"` on Node. + const reqInit: RequestInit & { duplex?: "half" } = { method, headers, body }; + if (isStream) { + reqInit.duplex = "half"; + } + const req = new Request(url, reqInit); // Verbose: print request info if (cliOpts.verbose) { diff --git a/src/cli/main.ts b/src/cli/main.ts index d3f94dc4..62ad0440 100644 --- a/src/cli/main.ts +++ b/src/cli/main.ts @@ -10,8 +10,19 @@ import { usage } from "./usage.ts"; import { srvxMeta } from "./_meta.ts"; export async function main(mainOpts: MainOptions): Promise { - const args = process.argv.slice(2); - const cliOpts = parseArgs(args); + // F60: honor an explicit `args` array (used by tests); fall back to process.argv + const args = mainOpts.args ?? process.argv.slice(2); + + let cliOpts: CLIOptions; + try { + cliOpts = parseArgs(args); + } catch (error) { + // F44: surface parse/entry-resolution problems as a one-line message + hint + const command = mainOpts.usage?.command || "srvx"; + console.error(c.red((error as Error).message || String(error))); + console.error(c.gray(`Run \`${command} --help\` for usage.`)); + process.exit(1); + } // Handle version flag if (cliOpts.version) { @@ -22,11 +33,20 @@ export async function main(mainOpts: MainOptions): Promise { // Handle help flag if (cliOpts.help) { console.log(usage(mainOpts)); - process.exit(cliOpts.help ? 0 : 1); + process.exit(0); } + // Resolve .env files (used by both serve and fetch modes) + const envFiles = [".env", cliOpts.prod ? ".env.production" : ".env.local"].filter((f) => + existsSync(f), + ); + // Fetch mode if (cliOpts.mode === "fetch") { + // F44: load env before fetching (the entry/handler may rely on env vars) + for (const envFile of [...envFiles].reverse() /* overrides first */) { + process.loadEnvFile?.(envFile); + } try { const res = await cliFetch(cliOpts); process.exit(res.ok ? 0 : 22); @@ -44,10 +64,6 @@ export async function main(mainOpts: MainOptions): Promise { // Log versions console.log(c.gray([...versions(mainOpts), cliOpts.prod ? "prod" : "dev"].join(" ยท "))); - // Resolve .env files - const envFiles = [".env", cliOpts.prod ? ".env.production" : ".env.local"].filter((f) => - existsSync(f), - ); if (envFiles.length > 0) { console.log( `${c.gray(`Loading environment variables from ${c.magenta(envFiles.join(", "))}`)}`, @@ -81,78 +97,69 @@ export async function main(mainOpts: MainOptions): Promise { } function parseArgs(args: string[]): CLIOptions { - const pArg0 = args.find((a) => !a.startsWith("-")); - const mode = pArg0 === "fetch" || pArg0 === "curl" ? "fetch" : "serve"; - - const commonArgs = { - help: { type: "boolean" }, + // Parse with a combined schema so option VALUES are never mistaken for the + // subcommand (F40). serve/fetch short flags don't collide, so one pass is safe. + const options = { + // --- Common flags --- + help: { type: "boolean", short: "h" }, // F41: -h is documented in usage version: { type: "boolean" }, dir: { type: "string" }, entry: { type: "string" }, host: { type: "string" }, hostname: { type: "string" }, tls: { type: "boolean" }, + url: { type: "string" }, + // --- Serve mode --- + prod: { type: "boolean" }, + port: { type: "string", short: "p" }, + static: { type: "string", short: "s" }, + import: { type: "string" }, + cert: { type: "string" }, + key: { type: "string" }, + // --- Fetch mode --- + method: { type: "string", short: "X" }, + request: { type: "string" }, // curl compatibility + header: { type: "string", multiple: true, short: "H" }, + verbose: { type: "boolean", short: "v" }, + data: { type: "string", short: "d" }, } as const; - if (mode === "serve") { - // Serve mode - const { values, positionals } = parseNodeArgs({ - args, - allowPositionals: true, - options: { - ...commonArgs, - url: { type: "string" }, - prod: { type: "boolean" }, - port: { type: "string", short: "p" }, - static: { type: "string", short: "s" }, - import: { type: "string" }, - cert: { type: "string" }, - key: { type: "string" }, - }, - }); - if (positionals[0] === "serve") { - positionals.shift(); - } + const { values, positionals } = parseNodeArgs({ args, allowPositionals: true, options }); - // Backward compatibility: allow entry or dir as positional argument - const maybeEntryOrDir = positionals[0]; - if (maybeEntryOrDir) { - if (values.entry || values.dir) { - throw new Error( - "Cannot specify entry or dir as positional argument when --entry or --dir is used!", - ); - } - const stat = statSync(maybeEntryOrDir); - if (stat.isDirectory()) { - values.dir = maybeEntryOrDir; - } else { - values.entry = maybeEntryOrDir; - } - } + // Detect mode from the first real positional (the subcommand), then drop it. + let mode: "serve" | "fetch" = "serve"; + const sub = positionals[0]; + if (sub === "fetch" || sub === "curl") { + mode = "fetch"; + positionals.shift(); + } else if (sub === "serve") { + positionals.shift(); + } - return { mode, ...values }; + if (mode === "fetch") { + const method = values.method || values.request; + const url = values.url || positionals[0] || "/"; + return { mode, ...values, url, method }; } - // Fetch mode - const { values, positionals } = parseNodeArgs({ - args, - allowPositionals: true, - options: { - ...commonArgs, - url: { type: "string" }, - method: { type: "string", short: "X" }, - request: { type: "string" }, // curl compatibility - header: { type: "string", multiple: true, short: "H" }, - verbose: { type: "boolean", short: "v" }, - data: { type: "string", short: "d" }, - }, - }); - if (positionals[0] === "fetch" || positionals[0] === "curl") { - positionals.shift(); + // Serve mode: allow entry or dir as a positional argument + const maybeEntryOrDir = positionals[0]; + if (maybeEntryOrDir) { + if (values.entry || values.dir) { + throw new Error("Cannot use a positional path together with --entry or --dir."); + } + // F44: turn a raw statSync ENOENT into a friendly message + if (!existsSync(maybeEntryOrDir)) { + throw new Error(`No such file or directory: ${maybeEntryOrDir}`); + } + if (statSync(maybeEntryOrDir).isDirectory()) { + values.dir = maybeEntryOrDir; + } else { + values.entry = maybeEntryOrDir; + } } - const method = values.method || values.request; - const url = values.url || positionals[0] || "/"; - return { mode, ...values, url, method }; + + return { mode, ...values }; } async function startServer(cliOpts: CLIOptions) { @@ -199,7 +206,9 @@ async function forkCLI(args: string[], runtimeArgs: string[]) { }; process.on("exit", () => cleanup("SIGTERM")); process.on("SIGTERM", () => cleanup("SIGTERM", 143)); - if (args.includes("--watch")) { + // F38: watch mode pushes `--watch` into runtimeArgs (not args), so check there; + // otherwise the SIGINT handler never installs and the child is orphaned. + if (runtimeArgs.includes("--watch")) { process.on("SIGINT" /* ctrl+c */, () => cleanup("SIGINT", 130)); } } diff --git a/src/cli/serve.ts b/src/cli/serve.ts index 190b6497..d28fb8e8 100644 --- a/src/cli/serve.ts +++ b/src/cli/serve.ts @@ -32,11 +32,20 @@ export async function cliServe(cliOpts: CLIOptions): Promise { const { serveStatic } = await import("srvx/static"); const { log } = await import("srvx/log"); + // F43: an explicit `--static` pointing at a missing dir must error; the + // implicit `public` default may stay silent. + const explicitStatic = !!cliOpts.static; const staticDir = resolve( cliOpts.dir || (loaded.url ? dirname(fileURLToPath(loaded.url)) : "."), cliOpts.static || "public", ); - cliOpts.static = existsSync(staticDir) ? staticDir : ""; + if (existsSync(staticDir)) { + cliOpts.static = staticDir; + } else if (explicitStatic) { + throw new Error(`--static directory not found: ${staticDir}`); + } else { + cliOpts.static = ""; + } if (loaded.notFound && !cliOpts.static) { process.send?.({ error: "no-entry" }); @@ -49,13 +58,23 @@ export async function cliServe(cliOpts: CLIOptions): Promise { ...loaded.module, } as Partial; + // F42: only override the entry module's `tls` when CLI flags actually supply + // TLS. A bare `--tls` (no cert/key) must error rather than silently downgrade. + let tls = serverOptions.tls; + if (cliOpts.tls) { + if (!cliOpts.cert || !cliOpts.key) { + throw new Error("--tls requires both --cert and --key."); + } + tls = { cert: cliOpts.cert, key: cliOpts.key }; + } + printInfo(cliOpts, loaded); server = srvxServe({ ...serverOptions, gracefulShutdown: !!cliOpts.prod, port: cliOpts.port ?? serverOptions.port, hostname: cliOpts.hostname ?? cliOpts.host ?? serverOptions.hostname, - tls: cliOpts.tls ? { cert: cliOpts.cert, key: cliOpts.key } : undefined, + tls, error: (error) => { console.error(error); return renderError(cliOpts, error); @@ -91,9 +110,10 @@ function renderError( status = 500, title = "Server Error", ): Response { - let html = `${title}`; + const safeTitle = escapeHtml(title); + let html = `${safeTitle}`; if (cliOpts.prod) { - html += `

${title}

Something went wrong while processing your request.

`; + html += `

${safeTitle}

Something went wrong while processing your request.

`; } else { html += /* html */ ` -

${title}

${error instanceof Error ? error.stack || error.message : String(error)}
+

${safeTitle}

${escapeHtml(
+      error instanceof Error ? error.stack || error.message : String(error),
+    )}
`; } @@ -113,6 +135,19 @@ function renderError( }); } +const HTML_ESCAPES: Record = { + "&": "&", + "<": "<", + ">": ">", + '"': """, + "'": "'", +}; + +// F59: escape untrusted text before interpolating into the dev error page HTML. +function escapeHtml(str: string): string { + return str.replace(/[&<>"']/g, (ch) => HTML_ESCAPES[ch]); +} + function printInfo(cliOpts: CLIOptions, loaded: Awaited>) { let entryInfo: string; if (loaded.notFound) { diff --git a/src/loader.ts b/src/loader.ts index 716830a6..b14eabfa 100644 --- a/src/loader.ts +++ b/src/loader.ts @@ -158,7 +158,7 @@ export async function loadServerEntry(opts: LoadOptions): Promise= 22.18 / 24). +import { main } from "../src/cli.ts"; + +// Point any forked child at this same source runner instead of ./bin/srvx.mjs. +(globalThis as any).__SRVX_BIN__ = new URL("./_cli-run.ts", import.meta.url).href; + +await main({ + usage: { + command: "srvx", + docs: "https://srvx.h3.dev", + issues: "https://github.com/h3js/srvx/issues", + }, +}); diff --git a/test/cli.test.ts b/test/cli.test.ts new file mode 100644 index 00000000..6c06f1c1 --- /dev/null +++ b/test/cli.test.ts @@ -0,0 +1,187 @@ +import { describe, it, expect, vi } from "vitest"; +import { fileURLToPath } from "node:url"; +import { resolve } from "node:path"; +import { createServer } from "node:http"; +import { mkdtemp, writeFile, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { execa, type Options as ExecaOptions } from "execa"; +import { getRandomPort, waitForPort } from "get-port-please"; + +const runnerPath = fileURLToPath(new URL("./_cli-run.ts", import.meta.url)); +const fixtureDir = fileURLToPath(new URL("./fixtures/cli", import.meta.url)); +const entryFile = resolve(fixtureDir, "server.ts"); + +function runCli( + args: string[], + opts: { cwd?: string; input?: string; env?: Record } = {}, +) { + return execa(process.execPath, [runnerPath, ...args], { + cwd: opts.cwd, + input: opts.input, + reject: false, // don't throw on non-zero exit; assert on exitCode instead + env: { NO_COLOR: "1", ...opts.env }, + } as ExecaOptions); +} + +describe("cli", () => { + describe("info flags", () => { + it("--version prints srvx and runtime versions", async () => { + const { stdout, exitCode } = await runCli(["--version"]); + expect(exitCode).toBe(0); + expect(stdout).toContain("srvx"); + expect(stdout).toMatch(/node|bun|deno/); + }); + + it("--help and -h (F41) print usage", async () => { + for (const flag of ["--help", "-h"]) { + const { stdout, exitCode } = await runCli([flag]); + expect(exitCode, `${flag} should exit 0`).toBe(0); + expect(stdout).toContain("SERVE MODE"); + expect(stdout).toContain("FETCH MODE"); + } + }); + + it("F60: main({ args }) honors an explicit args array (in-process)", async () => { + const { main } = await import("../src/cli.ts"); + let out = ""; + const writeSpy = vi.spyOn(process.stdout, "write").mockImplementation((chunk: any) => { + out += chunk; + return true; + }); + const exitSpy = vi.spyOn(process, "exit").mockImplementation(((code?: number) => { + throw new Error(`__exit__:${code}`); + }) as never); + try { + await expect(main({ args: ["--version"] })).rejects.toThrow("__exit__:0"); + expect(out).toContain("srvx"); + } finally { + writeSpy.mockRestore(); + exitSpy.mockRestore(); + } + }); + }); + + describe("fetch mode", () => { + it("exits 0 for a 2xx response", async () => { + const { stdout, exitCode } = await runCli(["fetch", "/", "--dir", fixtureDir]); + expect(exitCode).toBe(0); + expect(stdout).toContain("ok"); + }); + + it("exits 22 for a non-2xx response", async () => { + const { exitCode } = await runCli(["fetch", "/bad", "--dir", fixtureDir]); + expect(exitCode).toBe(22); + }); + + it("F40: `-p fetch ...` parses as fetch mode (value not treated as subcommand)", async () => { + const { stdout, exitCode } = await runCli(["-p", "8080", "fetch", "/", "--dir", fixtureDir]); + expect(exitCode).toBe(0); + expect(stdout).toContain("ok"); + }); + + it("F39: `-d @-` reads the request body from stdin", async () => { + const { stdout, exitCode } = await runCli( + ["fetch", "/echo", "-d", "@-", "--dir", fixtureDir], + { input: "hello-from-stdin" }, + ); + expect(exitCode).toBe(0); + expect(stdout).toContain("hello-from-stdin"); + }); + + it("F39: `-d @file` reads the request body from a file", async () => { + const { stdout, exitCode } = await runCli([ + "fetch", + "/echo", + "-d", + `@${resolve(fixtureDir, "data.txt")}`, + "--dir", + fixtureDir, + ]); + expect(exitCode).toBe(0); + expect(stdout).toContain("file-body-content"); + }); + + it("F44: loads .env before fetching", async () => { + // `.env` is written into a temp cwd at runtime (a committed `.env` fixture + // would be swallowed by .gitignore). The entry is loaded via --entry. + const dir = await mkdtemp(resolve(tmpdir(), "srvx-cli-env-")); + try { + await writeFile(resolve(dir, ".env"), "CLI_TEST_VAR=from-env\n"); + const { stdout, exitCode } = await runCli(["fetch", "/env", "--entry", entryFile], { + cwd: dir, + }); + expect(exitCode).toBe(0); + expect(stdout).toContain("from-env"); + } finally { + await rm(dir, { recursive: true, force: true }); + } + }); + + it("F44: schemeless host:port is treated as an http URL", async () => { + const port = await getRandomPort("localhost"); + const server = createServer((_req, res) => res.end("remote-ok")); + await new Promise((r) => server.listen(port, "localhost", () => r())); + try { + const { stdout, exitCode } = await runCli(["fetch", `localhost:${port}/`]); + expect(exitCode).toBe(0); + expect(stdout).toContain("remote-ok"); + } finally { + server.close(); + } + }); + }); + + describe("serve mode", () => { + it("serves a fixture entry and responds to requests", async () => { + const port = await getRandomPort("localhost"); + const child = runCli(["--prod", "--entry", entryFile, "--port", String(port)]); + try { + await waitForPort(port, { host: "localhost", delay: 50, retries: 100 }); + const res = await fetch(`http://localhost:${port}/`); + expect(res.status).toBe(200); + expect(await res.text()).toBe("ok"); + } finally { + child.kill("SIGTERM"); + await child.catch(() => {}); + } + }); + + it("F42: `--tls` without cert/key errors instead of downgrading to http", async () => { + const port = await getRandomPort("localhost"); + const { stderr, exitCode } = await runCli([ + "--prod", + "--tls", + "--entry", + entryFile, + "--port", + String(port), + ]); + expect(exitCode).not.toBe(0); + expect(stderr).toMatch(/--cert|--key|tls/i); + }); + + it("F43: an explicit missing `--static` directory errors", async () => { + const port = await getRandomPort("localhost"); + const { stderr, exitCode } = await runCli([ + "--prod", + "--entry", + entryFile, + "--static", + "./definitely-missing-dir", + "--port", + String(port), + ]); + expect(exitCode).not.toBe(0); + expect(stderr).toMatch(/static/i); + }); + }); + + describe("errors", () => { + it("F44: an unknown flag prints a one-line message + usage hint (no stack trace)", async () => { + const { stderr, exitCode } = await runCli(["--nope"]); + expect(exitCode).toBe(1); + expect(stderr).toContain("--help"); + expect(stderr).not.toMatch(/\n\s+at\s/); // no stack-trace frames + }); + }); +}); diff --git a/test/fixtures/cli/data.txt b/test/fixtures/cli/data.txt new file mode 100644 index 00000000..b7c14dd7 --- /dev/null +++ b/test/fixtures/cli/data.txt @@ -0,0 +1 @@ +file-body-content \ No newline at end of file diff --git a/test/fixtures/cli/server.ts b/test/fixtures/cli/server.ts new file mode 100644 index 00000000..dc9ec448 --- /dev/null +++ b/test/fixtures/cli/server.ts @@ -0,0 +1,14 @@ +// Fixture server for CLI tests. +export const fetch = async (req: Request): Promise => { + const url = new URL(req.url); + if (url.pathname === "/bad") { + return new Response("bad", { status: 404 }); + } + if (url.pathname === "/echo") { + return new Response(await req.text()); + } + if (url.pathname === "/env") { + return new Response(process.env.CLI_TEST_VAR || ""); + } + return new Response("ok"); +};