fix(project): guard --password-file reads on create/update (#79)

src/commands/project.test.ts

@@ -1044,6 +1044,122 @@ describe('runUpdate', () => {
   });
 });
 
+describe('#79 — an unreadable --password-file is a validation error, not a crash', () => {
+  const missing = join(tmpdir(), 'testsprite-issue-79-absent-password-file');
+
+  it('runCreate rejects a missing file with VALIDATION_ERROR (exit 5) before the network', async () => {
+    const { credentialsPath } = makeCreds();
+    const fetchImpl = vi.fn(async () => {
+      throw new Error('should not hit network');
+    });
+
+    await expect(
+      runCreate(
+        {
+          profile: 'default',
+          output: 'json',
+          debug: false,
+          type: 'backend',
+          name: 'Guarded',
+          passwordFile: missing,
+        },
+        {
+          credentialsPath,
+          fetchImpl: fetchImpl as unknown as typeof fetch,
+          stdout: () => {},
+          stderr: () => {},
+        },
+      ),
+    ).rejects.toMatchObject({ code: 'VALIDATION_ERROR', exitCode: 5 });
+
+    expect(fetchImpl).not.toHaveBeenCalled();
+  });
+
+  it('runUpdate rejects a missing file with VALIDATION_ERROR (exit 5) before the network', async () => {
+    const { credentialsPath } = makeCreds();
+    const fetchImpl = vi.fn(async () => {
+      throw new Error('should not hit network');
+    });
+
+    await expect(
+      runUpdate(
+        {
+          profile: 'default',
+          output: 'json',
+          debug: false,
+          projectId: 'proj_guarded',
+          passwordFile: missing,
+        },
+        {
+          credentialsPath,
+          fetchImpl: fetchImpl as unknown as typeof fetch,
+          stdout: () => {},
+          stderr: () => {},
+        },
+      ),
+    ).rejects.toMatchObject({ code: 'VALIDATION_ERROR', exitCode: 5 });
+
+    expect(fetchImpl).not.toHaveBeenCalled();
+  });
+
+  it('names the flag in nextAction instead of leaking a raw ENOENT', async () => {
+    const { credentialsPath } = makeCreds();
+
+    await expect(
+      runCreate(
+        {
+          profile: 'default',
+          output: 'json',
+          debug: false,
+          type: 'backend',
+          name: 'Guarded',
+          passwordFile: missing,
+        },
+        {
+          credentialsPath,
+          fetchImpl: (async () => {
+            throw new Error('should not hit network');
+          }) as unknown as typeof fetch,
+          stdout: () => {},
+          stderr: () => {},
+        },
+      ),
+    ).rejects.toMatchObject({
+      nextAction: expect.stringContaining('--password-file') as unknown as string,
+    });
+  });
+
+  it('still reads a password file that exists', async () => {
+    const { credentialsPath } = makeCreds();
+    const dir = mkdtempSync(join(tmpdir(), 'cli-p79-'));
+    const passwordFile = join(dir, 'pw.txt');
+    writeFileSync(passwordFile, 'from-file\n');
+
+    const sentBodies: unknown[] = [];
+    const fetchImpl = (async (_input: Parameters<typeof fetch>[0], init: RequestInit = {}) => {
+      if (init.body) sentBodies.push(JSON.parse(init.body as string) as unknown);
+      return new Response(JSON.stringify({ ...PROJECT_FIXTURE, id: 'proj_pw' }), {
+        status: 200,
+        headers: { 'content-type': 'application/json' },
+      });
+    }) as typeof fetch;
+
+    await runCreate(
+      {
+        profile: 'default',
+        output: 'json',
+        debug: false,
+        type: 'backend',
+        name: 'Guarded',
+        passwordFile,
+      },
+      { credentialsPath, fetchImpl, stdout: () => {}, stderr: () => {} },
+    );
+
+    expect((sentBodies[0] as Record<string, unknown>).password).toBe('from-file');
+  });
+});
+
 describe('runDelete', () => {
   it('refuses without --confirm and never hits the network (exit 5)', async () => {
     const { credentialsPath } = makeCreds();

src/commands/project.ts

@@ -11,6 +11,7 @@ import { ApiError } from '../lib/errors.js';
 import type { FetchImpl } from '../lib/http.js';
 import type { HttpClient } from '../lib/http.js';
 import { GLOBAL_OPTS_HINT, Output, resolveOutputMode, type OutputMode } from '../lib/output.js';
+import { readSecretFileGuarded } from '../lib/secret-file.js';
 import { assertNotLocal } from '../lib/target-url.js';
 import { renderTextTable, resolveTextColumns, type TextTableColumn } from '../lib/text-table.js';
 import { assertIdempotencyKey } from '../lib/validate.js';
@@ -210,7 +211,7 @@ export async function runCreate(
   // Resolve password: flag > file > none
   let password = opts.password;
   if (password === undefined && opts.passwordFile !== undefined) {
-    password = readFileSync(opts.passwordFile, 'utf8').trim();
+    password = readSecretFileGuarded('password-file', opts.passwordFile);
   }
 
   const idempotencyKey = opts.idempotencyKey ?? `cli-proj-create-${randomUUID()}`;
@@ -326,7 +327,7 @@ export async function runUpdate(
   // filesystem, even when --password-file is present.
   let password = opts.password;
   if (password === undefined && opts.passwordFile !== undefined) {
-    password = readFileSync(opts.passwordFile, 'utf8').trim();
+    password = readSecretFileGuarded('password-file', opts.passwordFile);
   }
 
   const idempotencyKey = opts.idempotencyKey ?? `cli-proj-update-${randomUUID()}`;

src/lib/secret-file.test.ts

@@ -0,0 +1,118 @@
+import { mkdirSync, mkdtempSync, writeFileSync } from 'node:fs';
+import { tmpdir } from 'node:os';
+import { join } from 'node:path';
+import { afterEach, beforeEach, describe, expect, it } from 'vitest';
+import { ApiError } from './errors.js';
+import { readSecretFileGuarded } from './secret-file.js';
+
+let tmpRoot: string;
+const originalCwd = process.cwd();
+
+beforeEach(() => {
+  tmpRoot = mkdtempSync(join(tmpdir(), 'testsprite-secret-file-'));
+});
+
+afterEach(() => {
+  // mkdtempSync directory is small and short-lived; OS cleans it up.
+  process.chdir(originalCwd);
+});
+
+describe('readSecretFileGuarded', () => {
+  it('returns the file contents', () => {
+    const path = join(tmpRoot, 'pw.txt');
+    writeFileSync(path, 'hunter2');
+    expect(readSecretFileGuarded('password-file', path)).toBe('hunter2');
+  });
+
+  it('trims surrounding whitespace and the trailing newline', () => {
+    const path = join(tmpRoot, 'pw-newline.txt');
+    writeFileSync(path, '  hunter2  \n');
+    expect(readSecretFileGuarded('password-file', path)).toBe('hunter2');
+  });
+
+  it('drops a leading UTF-8 BOM so PowerShell-written files still work', () => {
+    const path = join(tmpRoot, 'pw-bom.txt');
+    writeFileSync(path, 'hunter2\n');
+    expect(readSecretFileGuarded('password-file', path)).toBe('hunter2');
+  });
+
+  it('preserves interior whitespace', () => {
+    const path = join(tmpRoot, 'pw-spaces.txt');
+    writeFileSync(path, 'two words\n');
+    expect(readSecretFileGuarded('password-file', path)).toBe('two words');
+  });
+
+  it('resolves a relative path against the working directory', () => {
+    writeFileSync(join(tmpRoot, 'relative.txt'), 'from-cwd');
+    process.chdir(tmpRoot);
+    expect(readSecretFileGuarded('password-file', 'relative.txt')).toBe('from-cwd');
+  });
+
+  it('returns an empty string for an empty file rather than throwing', () => {
+    const path = join(tmpRoot, 'empty.txt');
+    writeFileSync(path, '');
+    expect(readSecretFileGuarded('password-file', path)).toBe('');
+  });
+
+  describe('missing file', () => {
+    it('throws VALIDATION_ERROR with exit code 5', () => {
+      const path = join(tmpRoot, 'nope.txt');
+      expect(() => readSecretFileGuarded('password-file', path)).toThrow(ApiError);
+      try {
+        readSecretFileGuarded('password-file', path);
+        expect.unreachable('should have thrown');
+      } catch (err) {
+        expect(err).toMatchObject({ code: 'VALIDATION_ERROR', exitCode: 5 });
+      }
+    });
+
+    it('names the offending flag and path in nextAction', () => {
+      const path = join(tmpRoot, 'nope.txt');
+      try {
+        readSecretFileGuarded('password-file', path);
+        expect.unreachable('should have thrown');
+      } catch (err) {
+        const { nextAction } = err as ApiError;
+        expect(nextAction).toContain('--password-file');
+        expect(nextAction).toContain('file does not exist');
+        expect(nextAction).toContain(path);
+      }
+    });
+
+    it('attributes the error to whichever flag the caller names', () => {
+      const path = join(tmpRoot, 'nope.txt');
+      try {
+        readSecretFileGuarded('client-secret-file', path);
+        expect.unreachable('should have thrown');
+      } catch (err) {
+        expect((err as ApiError).nextAction).toContain('--client-secret-file');
+      }
+    });
+
+    it('reports the path as typed, not the resolved absolute path', () => {
+      process.chdir(tmpRoot);
+      try {
+        readSecretFileGuarded('password-file', 'missing.txt');
+        expect.unreachable('should have thrown');
+      } catch (err) {
+        const { nextAction } = err as ApiError;
+        expect(nextAction).toContain('missing.txt');
+        expect(nextAction).not.toContain(tmpRoot);
+      }
+    });
+  });
+
+  describe('directory instead of a file', () => {
+    it('throws VALIDATION_ERROR instead of crashing with EISDIR', () => {
+      const path = join(tmpRoot, 'a-directory');
+      mkdirSync(path);
+      try {
+        readSecretFileGuarded('password-file', path);
+        expect.unreachable('should have thrown');
+      } catch (err) {
+        expect(err).toMatchObject({ code: 'VALIDATION_ERROR', exitCode: 5 });
+        expect((err as ApiError).nextAction).toContain('not a regular file');
+      }
+    });
+  });
+});

src/lib/secret-file.ts

@@ -0,0 +1,87 @@
+/**
+ * Guarded reader for the `--*-file` secret flags.
+ *
+ * Every one of these flags exists so a secret stays out of shell history, and
+ * every one of them is a path the user types by hand — so a typo is the
+ * expected failure, not an exceptional one. A bare
+ * `readFileSync(path, 'utf8').trim()` turns that typo into an unhandled Node
+ * exception: exit `1` instead of `5`, an `--output json` payload whose `error`
+ * is a bare string rather than the `{ code, message, nextAction }` envelope the
+ * rest of the CLI emits, and the absolute path plus errno leaked to stderr.
+ *
+ * This maps those failures onto the same typed `VALIDATION_ERROR` envelope the
+ * already-guarded file flags produce, mirroring `readCodeFileGuarded` in
+ * `src/commands/test.ts`. The payload cap is deliberately not carried over:
+ * secrets are small, and a size ceiling would be a behaviour change on a
+ * shipped flag rather than part of fixing the crash.
+ *
+ * Callers pass the flag name so the envelope names the flag the user actually
+ * typed — one helper serves `--password-file` today and the remaining
+ * credential/auto-auth file flags once they are migrated.
+ */
+import { readFileSync, statSync } from 'node:fs';
+import { isAbsolute, resolve } from 'node:path';
+import { localValidationError } from './errors.js';
+
+/**
+ * Read a secret from `path`, surfacing every filesystem failure as a typed
+ * `VALIDATION_ERROR` (exit 5) attributed to `flag`.
+ *
+ * The returned value is trimmed, matching what the unguarded call sites did.
+ * Trimming also drops a leading UTF-8 BOM: `U+FEFF` is ECMAScript whitespace,
+ * so a file written by PowerShell 5.1's default `Set-Content -Encoding utf8`
+ * no longer smuggles an invisible character into the secret.
+ *
+ * @param flag - Flag name without the leading dashes, e.g. `'password-file'`.
+ * @param path - Path as supplied by the user; may be relative.
+ * @throws {ApiError} `VALIDATION_ERROR` when the path is missing, unreadable,
+ *   or not a regular file.
+ */
+export function readSecretFileGuarded(flag: string, path: string): string {
+  const absolute = isAbsolute(path) ? path : resolve(process.cwd(), path);
+
+  let stat;
+  try {
+    stat = statSync(absolute);
+  } catch (err) {
+    throw secretFileError(flag, path, err, 'stat');
+  }
+
+  // A directory would otherwise reach readFileSync and throw EISDIR on Linux
+  // while resolving to an empty read on some platforms — reject it up front so
+  // the contract is the same everywhere.
+  if (!stat.isFile()) {
+    throw localValidationError(flag, `not a regular file: ${path}`);
+  }
+
+  try {
+    return readFileSync(absolute, 'utf8').trim();
+  } catch (err) {
+    throw secretFileError(flag, path, err, 'read');
+  }
+}
+
+/**
+ * Translate a Node filesystem error into the CLI's validation envelope,
+ * reporting the path the user typed rather than the resolved absolute path so
+ * no directory layout leaks into output.
+ */
+function secretFileError(
+  flag: string,
+  path: string,
+  err: unknown,
+  verb: 'stat' | 'read',
+): ReturnType<typeof localValidationError> {
+  const code = (err as NodeJS.ErrnoException).code;
+  if (code === 'ENOENT') {
+    return localValidationError(flag, `file does not exist: ${path}`);
+  }
+  if (code === 'EACCES' || code === 'EPERM') {
+    return localValidationError(flag, `permission denied reading ${path}`);
+  }
+  if (code === 'EISDIR') {
+    return localValidationError(flag, `not a regular file: ${path}`);
+  }
+  const reason = err instanceof Error ? err.message : 'unknown error';
+  return localValidationError(flag, `cannot ${verb} ${path}: ${reason}`);
+}