fix(cli): harden MCP transport, file writes, auth & input from review

README.md

@@ -111,15 +111,24 @@ server so AI clients (Claude Code, Cursor, Claude Desktop, …) can use Speechif
 directly. Tools:
 
 - **`search_docs`** — search the public Speechify docs. No auth required.
-- **`list_voices`** — list account voices. *(requires an API key)*
-- **`text_to_speech`** — synthesize audio, returned inline or written to a path.
-  *(requires an API key)*
+- **`list_voices`** / **`get_voice`** — list account voices, or fetch one by id. *(requires an API key)*
+- **`text_to_speech`** — synthesize audio, returned inline or written to a path. *(requires an API key)*
+- **`stream_text_to_speech`** — synthesize long-form audio straight to a file. *(requires an API key)*
+
+The TTS tools that write files confine `outputPath` to a relative path **inside
+the server's working directory** and never overwrite an existing file — a path
+that escapes the directory (absolute, `../…`) or collides with a file is refused.
 
 ```bash
 speechify mcp --accept-alpha                    # serve over stdio (the usual MCP transport)
 speechify mcp --accept-alpha --http --port 3000 # serve streamable HTTP at POST /mcp instead
 ```
 
+The HTTP transport binds **`127.0.0.1` only** by default: the endpoint is
+unauthenticated and uses your API key on every call, so it must not be reachable
+off-box. `--host <interface>` can bind a wider interface, but only put your own
+authentication (a reverse proxy, network policy) in front of it first.
+
 All tools are always registered, so they stay discoverable to agents regardless
 of auth state. Auth is resolved **per tool call**, so a server started before
 `speechify login` picks up the key the moment it's stored — no restart. Calling
@@ -139,9 +148,10 @@ speechify mcp install --accept-alpha --client vscode --embed-key # bake $SPEECHI
 
 Supported ids: `claude-code`, `cursor`, `claude-desktop`, `windsurf`, `vscode`.
 By default no credential is embedded — the spawned server reads your stored API
-key. Use `--embed-key` to bake `$SPEECHIFY_API_KEY` into the entry instead. An
-existing config that can't be parsed safely (e.g. JSONC with comments) is left
-untouched.
+key. `--embed-key` bakes `$SPEECHIFY_API_KEY` into the entry instead, writing the
+key **in plaintext** into the client's config (the file is set to `0600`); prefer
+the stored keychain credential unless a client can't reach it. An existing config
+that can't be parsed safely (e.g. JSONC with comments) is left untouched.
 
 To wire it up manually instead, the stdio entry looks like this (once the CLI is
 on your `PATH`):

src/audio/play.ts

@@ -25,7 +25,13 @@ function candidates(): Player[] {
     case "win32":
       return [
         { cmd: "ffplay", args: (f) => ["-nodisp", "-autoexit", "-loglevel", "quiet", f] },
-        { cmd: "powershell", args: (f) => ["-c", `(New-Object Media.SoundPlayer '${f}').PlaySync();`] },
+        {
+          cmd: "powershell",
+          // Double any single quote so a path can't break out of the literal string
+          // and inject PowerShell (the '' escape is how single-quoted strings quote
+          // a quote). The whole path stays a single-quoted literal, never code.
+          args: (f) => ["-c", `(New-Object Media.SoundPlayer '${f.replace(/'/g, "''")}').PlaySync();`],
+        },
       ];
     default:
       return [

src/auth/session.test.ts

@@ -77,6 +77,27 @@ describe("resolveAuth", () => {
     expect(auth).toMatchObject({ baseUrl: "https://example.test", apiVersion: "2026-01-01" });
   });
 
+  it("does NOT let an ad-hoc flag key inherit the stored base_url/api_version", async () => {
+    // A staging host was configured for the stored key. Passing a different key on
+    // the fly must not silently ship its Bearer to that host.
+    await writeConfigFile({ api_key: "sk_stored", base_url: "https://staging.test", api_version: "2026-01-01" });
+    const auth = await resolveAuth({ apiKey: "sk_flag" });
+    expect(auth).toMatchObject({ bearer: "sk_flag", baseUrl: DEFAULT_BASE_URL, keySource: "flag" });
+    expect(auth.apiVersion).toBeUndefined();
+  });
+
+  it("does NOT let an env key inherit the stored base_url either", async () => {
+    await writeConfigFile({ api_key: "sk_stored", base_url: "https://staging.test" });
+    vi.stubEnv("SPEECHIFY_API_KEY", "sk_env");
+    const auth = await resolveAuth();
+    expect(auth).toMatchObject({ bearer: "sk_env", baseUrl: DEFAULT_BASE_URL, keySource: "env" });
+  });
+
+  it("still honors an explicit --base-url alongside a flag key", async () => {
+    const auth = await resolveAuth({ apiKey: "sk_flag", baseUrl: "https://my.proxy" });
+    expect(auth).toMatchObject({ bearer: "sk_flag", baseUrl: "https://my.proxy" });
+  });
+
   it("throws a CliError when nothing is configured", async () => {
     await expect(resolveAuth()).rejects.toBeInstanceOf(CliError);
   });

src/auth/session.ts

@@ -35,20 +35,26 @@ function clean(value: string | undefined): string | undefined {
 
 export async function resolveAuth(input: AuthInput = {}): Promise<AuthContext> {
   const stored = (await readConfigFile()) ?? {};
-  const baseUrl =
-    clean(input.baseUrl) ?? clean(process.env[BASE_URL_ENV]) ?? clean(stored.base_url) ?? DEFAULT_BASE_URL;
-  const apiVersion = clean(input.apiVersion) ?? clean(process.env[API_VERSION_ENV]) ?? clean(stored.api_version);
 
-  // Precedence: explicit --api-key flag → $SPEECHIFY_API_KEY → stored key.
+  // The stored base_url / api_version belong to the STORED key's configuration —
+  // they must not silently apply to an ad-hoc flag/env key, or a prod key passed on
+  // the fly would inherit a previously-configured staging/proxy host and leak the
+  // Bearer there. Explicit flag/env inputs always win; stored values fill in only
+  // when the key itself is the stored one.
   const flagKey = clean(input.apiKey);
   const envKey = clean(process.env[API_KEY_ENV]);
   const explicitKey = flagKey ?? envKey;
   if (explicitKey) {
+    const baseUrl = clean(input.baseUrl) ?? clean(process.env[BASE_URL_ENV]) ?? DEFAULT_BASE_URL;
+    const apiVersion = clean(input.apiVersion) ?? clean(process.env[API_VERSION_ENV]);
     return { bearer: explicitKey, baseUrl, apiVersion, keySource: flagKey ? "flag" : "env" };
   }
 
   const storedKey = clean(stored.api_key);
   if (storedKey) {
+    const baseUrl =
+      clean(input.baseUrl) ?? clean(process.env[BASE_URL_ENV]) ?? clean(stored.base_url) ?? DEFAULT_BASE_URL;
+    const apiVersion = clean(input.apiVersion) ?? clean(process.env[API_VERSION_ENV]) ?? clean(stored.api_version);
     return { bearer: storedKey, baseUrl, apiVersion, keySource: "stored" };
   }
 

src/bin.ts

@@ -6,7 +6,7 @@ import { registerAuthCommands } from "./commands/auth.js";
 import { registerMcpCommand } from "./commands/mcp.js";
 import { registerSayCommand } from "./commands/say.js";
 import { registerVoicesCommand } from "./commands/voices.js";
-import { NeedsInputError, normalizeError } from "./core/errors.js";
+import { CliError, ExitCode, NeedsInputError, normalizeError } from "./core/errors.js";
 import { emitNeedsInput } from "./output.js";
 import { type OutputMode, outputMode } from "./runtime.js";
 
@@ -48,6 +48,19 @@ function buildProgram(): Command {
   // After all commands exist, hang the globals off the whole tree.
   applyGlobalOptions(program);
 
+  // --json and --agent-friendly are contradictory output contracts (bare payload
+  // vs. wrapped envelope). Passing both is a mistake, not a silent precedence
+  // decision — reject it before any command runs.
+  program.hook("preAction", (_thisCommand, actionCommand) => {
+    const opts = actionCommand.optsWithGlobals() as { json?: boolean; agentFriendly?: boolean };
+    if (opts.json && opts.agentFriendly) {
+      throw new CliError("Use either --json or --agent-friendly, not both.", {
+        exitCode: ExitCode.DATA_ERR,
+        code: "conflicting_output",
+      });
+    }
+  });
+
   return program;
 }
 

src/commands/api.test.ts

@@ -35,7 +35,22 @@ describe("buildApiRequest", () => {
     const req = await buildApiRequest(auth, "/v1/audio/speech", { field: ["input=hello", "voice_id=george"] });
     expect(req.method).toBe("POST");
     expect(req.headers["content-type"]).toBe("application/json");
-    expect(JSON.parse(req.body ?? "")).toEqual({ input: "hello", voice_id: "george" });
+    expect(JSON.parse(String(req.body ?? ""))).toEqual({ input: "hello", voice_id: "george" });
+  });
+
+  it("coerces true/false/null and numbers in --field to typed JSON, leaving other values strings", async () => {
+    const req = await buildApiRequest(auth, "/v1/x", {
+      field: ["speed=1.5", "count=3", "loud=true", "quiet=false", "voice=null", "id=007", "name=george"],
+    });
+    expect(JSON.parse(String(req.body ?? ""))).toEqual({
+      speed: 1.5,
+      count: 3,
+      loud: true,
+      quiet: false,
+      voice: null,
+      id: "007", // leading zero → stays a string, not 7
+      name: "george",
+    });
   });
 
   it("treats raw non-JSON --data as a body without forcing a content-type", async () => {
@@ -45,6 +60,30 @@ describe("buildApiRequest", () => {
     expect(req.headers["content-type"]).toBeUndefined();
   });
 
+  it("preserves any path in the base URL when resolving a relative endpoint", async () => {
+    const based: AuthContext = { ...auth, baseUrl: "https://api.example/api/v2" };
+    const req = await buildApiRequest(based, "voices", {});
+    expect(req.url).toBe("https://api.example/api/v2/voices");
+  });
+
+  it("neutralizes a protocol-relative endpoint so the Bearer never leaves the API host", async () => {
+    // `//evil.example/x` must not become https://evil.example/x — leading slashes
+    // are stripped, so it stays a path on the configured origin.
+    const req = await buildApiRequest(auth, "//evil.example/steal", {});
+    expect(new URL(req.url).origin).toBe("https://api.example");
+    // Same for a backslash-based protocol-relative form (URL treats `\` as `/`).
+    const req2 = await buildApiRequest(auth, "\\\\evil.example/steal", {});
+    expect(new URL(req2.url).origin).toBe("https://api.example");
+  });
+
+  it("rejects an endpoint that resolves off the API host via a control-char prefix", async () => {
+    // A leading tab is stripped by the URL parser, re-enabling `//host` — the origin
+    // backstop catches it instead of letting the Bearer go to evil.example.
+    await expect(buildApiRequest(auth, "\t//evil.example/steal", {})).rejects.toMatchObject({
+      code: "endpoint_off_origin",
+    });
+  });
+
   it("honors an explicit --method and parses --header", async () => {
     const req = await buildApiRequest(auth, "/v1/x", { method: "delete", header: ["X-Foo: bar"] });
     expect(req.method).toBe("DELETE");

src/commands/api.ts

@@ -8,7 +8,7 @@ import type { AuthContext } from "../auth/session.js";
 import { resolveAuth } from "../auth/session.js";
 import { CliError, ExitCode, exitCodeForStatus } from "../core/errors.js";
 import { fetchWithTimeout } from "../core/fetchWithTimeout.js";
-import { readStdin } from "../io.js";
+import { readStdinBytes } from "../io.js";
 import type { GlobalOptions } from "../options.js";
 import { logWarning } from "../output.js";
 
@@ -25,14 +25,34 @@ export interface ApiRequest {
   url: string;
   method: string;
   headers: Record<string, string>;
-  body?: string;
+  /** A string body for text/JSON, or raw bytes for a binary body (`-d -`, `@file`). */
+  body?: string | Buffer;
 }
 
 function buildUrl(base: string, endpoint: string, query: string[] = []): string {
-  // A full URL is used as-is; otherwise the path is resolved against the API base.
-  const url = /^https?:\/\//i.test(endpoint)
-    ? new URL(endpoint)
-    : new URL((endpoint.startsWith("/") ? "" : "/") + endpoint, `${base.replace(/\/+$/, "")}/`);
+  let url: URL;
+  if (/^https?:\/\//i.test(endpoint)) {
+    // A full http(s) URL is an explicit, deliberate target — used as-is.
+    url = new URL(endpoint);
+  } else {
+    // Resolve as a path relative to the base, preserving ANY path the base carries
+    // (e.g. `--base-url https://host/api` keeps `/api`). Strip leading slashes so
+    // the endpoint appends to the base path instead of resetting to the origin —
+    // this also neutralizes a protocol-relative `//evil.com/x`, which would
+    // otherwise re-target the host and leak the Bearer off-origin.
+    const baseUrl = new URL(base.endsWith("/") ? base : `${base}/`);
+    // Strip leading slashes AND backslashes (the URL parser treats `\` as `/` for
+    // http(s)) so the endpoint appends to the base path instead of resetting to the
+    // origin or going protocol-relative. The origin check below is the backstop for
+    // anything exotic (e.g. a control-char prefix that re-enables `//host`).
+    url = new URL(endpoint.replace(/^[/\\]+/, ""), baseUrl);
+    if (url.origin !== baseUrl.origin) {
+      throw new CliError(
+        `Endpoint "${endpoint}" resolves off the API host (${baseUrl.origin}). Pass a path, or a full https:// URL to target another host deliberately.`,
+        { exitCode: ExitCode.DATA_ERR, code: "endpoint_off_origin" },
+      );
+    }
+  }
   for (const q of query) {
     const i = q.indexOf("=");
     if (i === -1) throw new CliError(`Invalid --query "${q}" (expected key=value).`, { exitCode: ExitCode.DATA_ERR });
@@ -41,20 +61,54 @@ function buildUrl(base: string, endpoint: string, query: string[] = []): string
   return url.toString();
 }
 
-async function resolveBody(opts: ApiOptions): Promise<{ body?: string; contentType?: string }> {
+/**
+ * Coerce a --field value to a typed JSON scalar so numeric/boolean/null API
+ * parameters aren't sent as strings (which some endpoints reject with 422). Only
+ * the unambiguous literals `true`/`false`/`null` and plain decimal numbers are
+ * converted; everything else stays a string. Use --data for a body that needs the
+ * literal string "true"/"123".
+ */
+function coerceFieldValue(value: string): unknown {
+  if (value === "true") return true;
+  if (value === "false") return false;
+  if (value === "null") return null;
+  // Strict decimal number: no hex, no leading +, no surrounding space — so an id
+  // like "007" or "1e" stays a string rather than silently becoming a number.
+  if (/^-?(?:0|[1-9]\d*)(?:\.\d+)?(?:[eE][+-]?\d+)?$/.test(value)) return Number(value);
+  return value;
+}
+
+/** JSON content-type sniff on raw bytes: first non-whitespace byte is `{` or `[`. */
+function sniffJsonContentType(body: Buffer): string | undefined {
+  for (const byte of body) {
+    if (byte === 0x20 || byte === 0x09 || byte === 0x0a || byte === 0x0d) continue;
+    return byte === 0x7b || byte === 0x5b ? "application/json" : undefined;
+  }
+  return undefined;
+}
+
+async function resolveBody(opts: ApiOptions): Promise<{ body?: string | Buffer; contentType?: string }> {
   if (opts.field?.length) {
-    const obj: Record<string, string> = {};
+    const obj: Record<string, unknown> = {};
     for (const f of opts.field) {
       const i = f.indexOf("=");
       if (i === -1) throw new CliError(`Invalid --field "${f}" (expected key=value).`, { exitCode: ExitCode.DATA_ERR });
-      obj[f.slice(0, i)] = f.slice(i + 1);
+      obj[f.slice(0, i)] = coerceFieldValue(f.slice(i + 1));
     }
     return { body: JSON.stringify(obj), contentType: "application/json" };
   }
   if (opts.data != null) {
-    let raw = opts.data;
-    if (raw === "-") raw = await readStdin();
-    else if (raw.startsWith("@")) raw = await readFile(raw.slice(1), "utf8");
+    // `-` (stdin) and `@file` are read as raw bytes so a binary body (audio, etc.)
+    // is sent verbatim rather than mangled through a UTF-8 round-trip.
+    if (opts.data === "-") {
+      const bytes = await readStdinBytes();
+      return { body: bytes, contentType: sniffJsonContentType(bytes) };
+    }
+    if (opts.data.startsWith("@")) {
+      const bytes = await readFile(opts.data.slice(1));
+      return { body: bytes, contentType: sniffJsonContentType(bytes) };
+    }
+    const raw = opts.data;
     const trimmed = raw.trimStart();
     const isJson = trimmed.startsWith("{") || trimmed.startsWith("[");
     return { body: raw, contentType: isJson ? "application/json" : undefined };
@@ -88,7 +142,10 @@ export function registerApiCommand(program: Command): void {
     .command("api <endpoint>")
     .description("Authenticated raw request to any API endpoint (gh-api style).")
     .option("-X, --method <method>", "HTTP method (default GET, or POST when a body is present)")
-    .option("-f, --field <key=value...>", "body field key=value; repeatable, builds a JSON body")
+    .option(
+      "-f, --field <key=value...>",
+      "body field key=value; repeatable, builds a JSON body (true/false/null and numbers become typed; use --data for literal strings)",
+    )
     .option("-d, --data <data>", "raw request body; @file reads a file, - reads stdin")
     .option("-q, --query <key=value...>", "query parameter key=value; repeatable")
     .option("-H, --header <header...>", "extra header 'Key: Value'; repeatable")

src/commands/mcp-install.ts

@@ -5,8 +5,9 @@
 // the encrypted-file fallback) on its own, so we DON'T embed a credential by
 // default. `--embed-key` opts into baking $SPEECHIFY_API_KEY into the client env
 // instead.
+import { randomBytes } from "node:crypto";
 import { existsSync } from "node:fs";
-import { mkdir, readFile, writeFile } from "node:fs/promises";
+import { mkdir, readFile, rename, rm, writeFile } from "node:fs/promises";
 import os from "node:os";
 import path from "node:path";
 import { CliError, ExitCode } from "../core/errors.js";
@@ -132,6 +133,33 @@ export function mergeConfig(
 
 export type WriteStatus = "installed" | "skipped-unparsable";
 
+/** True when the entry carries an embedded credential (so the file must be private). */
+function entryHasSecret(entry: Record<string, unknown>): boolean {
+  const env = entry.env as Record<string, unknown> | undefined;
+  return Boolean(env && typeof env.SPEECHIFY_API_KEY === "string" && env.SPEECHIFY_API_KEY.length > 0);
+}
+
+/**
+ * Write `content` to `destination` atomically: a sibling temp file, fsync-free
+ * rename into place. A crash mid-write can never leave a half-written (corrupt)
+ * config — the old file stays until the rename swaps it. When `mode` is given the
+ * temp file is created with it, so a config carrying a plaintext key is never
+ * momentarily world-readable.
+ */
+async function writeFileAtomic(destination: string, content: string, mode?: number): Promise<void> {
+  const temporary = path.join(
+    path.dirname(destination),
+    `.${path.basename(destination)}.${randomBytes(6).toString("hex")}.tmp`,
+  );
+  try {
+    await writeFile(temporary, content, mode !== undefined ? { mode } : undefined);
+    await rename(temporary, destination);
+  } catch (err) {
+    await rm(temporary, { force: true });
+    throw err;
+  }
+}
+
 export async function writeClientConfig(client: McpClient, entry: Record<string, unknown>): Promise<WriteStatus> {
   let config: Record<string, unknown> = {};
   if (existsSync(client.configPath)) {
@@ -144,7 +172,11 @@ export async function writeClientConfig(client: McpClient, entry: Record<string,
   }
   const merged = mergeConfig(config, client.serversKey, entry);
   await mkdir(path.dirname(client.configPath), { recursive: true });
-  await writeFile(client.configPath, `${JSON.stringify(merged, null, 2)}\n`);
+  // A config with an embedded key is written 0600 so the plaintext secret isn't
+  // left readable by other users. Without a key we don't tighten an existing
+  // shared config's permissions.
+  const mode = entryHasSecret(entry) ? 0o600 : undefined;
+  await writeFileAtomic(client.configPath, `${JSON.stringify(merged, null, 2)}\n`, mode);
   return "installed";
 }
 
@@ -171,6 +203,11 @@ export async function runMcpInstall(opts: McpInstallOptions): Promise<void> {
   if (opts.embedKey && !apiKey) {
     logWarning("--embed-key set but no API key found (--api-key / $SPEECHIFY_API_KEY); writing config without one.");
   }
+  if (apiKey && !opts.print) {
+    logWarning(
+      "--embed-key writes your API key in PLAINTEXT into each client config (files set to 0600). Prefer omitting it and relying on the OS keychain (`speechify login`).",
+    );
+  }
 
   // --print: show the canonical config block, write nothing.
   if (opts.print) {