fix(cli): don't let an unreadable credentials file crash the telemetry hook

src/index.ts

@@ -50,7 +50,7 @@ import {
   isTelemetryOptedOut,
   recordOutcome,
   recordTelemetryExtras,
-  resolveTelemetryAuth,
+  tryResolveTelemetryAuth,
   type ResolvedTelemetryAuth,
   type WaitTimeoutTelemetry,
 } from './lib/telemetry.js';
@@ -304,13 +304,25 @@ program.hook('preAction', (_thisCommand, actionCommand) => {
   // Telemetry gate, same reasons as the update notice below: skip `completion`
   // (its stdout is eval'd by shells) and `--plan-template` (pure-local). Resolve
   // auth now, before the command's action runs, so `auth remove` still reports
-  // on the key it used; skip the read for opt-out / dry-run.
+  // on the key it used; skip the read for opt-out / dry-run. The read is
+  // best-effort: an unreadable credentials file must not end the process here,
+  // before the command could run — `doctor` exists to diagnose exactly that, and
+  // offline commands never need the key. On failure the command runs as if
+  // telemetry were off (no event, and no fallback read at exit either).
   telemetryEmit = commandPath !== 'completion' && !isPlanTemplate;
   if (telemetryEmit && !isTelemetryOptedOut(process.env) && globals.dryRun !== true) {
-    telemetryAuth = resolveTelemetryAuth({
-      profile: globals.profile,
-      endpointUrl: globals.endpointUrl,
-    });
+    telemetryAuth = tryResolveTelemetryAuth(
+      { profile: globals.profile, endpointUrl: globals.endpointUrl },
+      {
+        onError: globals.debug
+          ? error =>
+              process.stderr.write(
+                `[debug] telemetry skipped: ${error instanceof Error ? error.message : String(error)}\n`,
+              )
+          : undefined,
+      },
+    );
+    if (telemetryAuth === undefined) telemetryEmit = false;
   }
   if (!isPlanTemplate) {
     maybeEmitSkillNudge({

src/lib/telemetry.spec.ts

@@ -20,6 +20,7 @@ import {
   recordOutcome,
   recordTelemetryExtras,
   resolveTelemetryAuth,
+  tryResolveTelemetryAuth,
   sanitizeTelemetryExtras,
   takeTelemetryExtras,
 } from './telemetry.js';
@@ -539,6 +540,47 @@ describe('resolveTelemetryAuth', () => {
   });
 });
 
+// tryResolveTelemetryAuth — the pre-action hook's variant; must never throw
+describe('tryResolveTelemetryAuth', () => {
+  /** A credentials path that exists but cannot be read as a file, on every OS. */
+  function unreadableCredsPath(): string {
+    const dir = mkdtempSync(join(tmpdir(), 'ts-telemetry-unreadable-'));
+    const path = join(dir, 'credentials');
+    // eslint-disable-next-line security/detect-non-literal-fs-filename -- test fixture path built from this test's own mkdtempSync() dir, never user input
+    mkdirSync(path);
+    return path;
+  }
+
+  it('returns the same auth as resolveTelemetryAuth when the file reads', () => {
+    const credentialsPath = writeCreds(true);
+    expect(tryResolveTelemetryAuth({}, { env: {}, credentialsPath })).toEqual(
+      resolveTelemetryAuth({}, { env: {}, credentialsPath }),
+    );
+  });
+
+  it('returns undefined instead of throwing when the credentials read fails', () => {
+    const credentialsPath = unreadableCredsPath();
+    expect(() => resolveTelemetryAuth({}, { env: {}, credentialsPath })).toThrow();
+    expect(tryResolveTelemetryAuth({}, { env: {}, credentialsPath })).toBeUndefined();
+  });
+
+  it('hands the swallowed error to onError (the --debug line)', () => {
+    const onError = vi.fn();
+    tryResolveTelemetryAuth({}, { env: {}, credentialsPath: unreadableCredsPath(), onError });
+    expect(onError).toHaveBeenCalledOnce();
+    expect(onError.mock.calls[0]?.[0]).toBeInstanceOf(Error);
+  });
+
+  it('stays silent when onError itself throws', () => {
+    const onError = vi.fn(() => {
+      throw new Error('stderr closed');
+    });
+    expect(
+      tryResolveTelemetryAuth({}, { env: {}, credentialsPath: unreadableCredsPath(), onError }),
+    ).toBeUndefined();
+  });
+});
+
 describe('wait timeout event fields', () => {
   const base = {
     command: 'test run',

src/lib/telemetry.ts

@@ -645,6 +645,35 @@ export function resolveTelemetryAuth(
   return { apiKey: config.apiKey, apiUrl: config.apiUrl };
 }
 
+/**
+ * {@link resolveTelemetryAuth} for the pre-action hook, which runs before every
+ * command: never throws. An unreadable credentials file (EPERM/EACCES — the
+ * Windows ACL lock-out `doctor` diagnoses — or any other read/parse failure)
+ * returns `undefined` so the caller can skip the beacon and let the command run
+ * and report the problem its own way, as the module contract requires
+ * (telemetry never changes a command's behavior, output, or exit code).
+ * `onError` receives the swallowed error, for `--debug`.
+ */
+export function tryResolveTelemetryAuth(
+  opts: { profile?: string; endpointUrl?: string },
+  deps: {
+    env?: NodeJS.ProcessEnv;
+    credentialsPath?: string;
+    onError?: (error: unknown) => void;
+  } = {},
+): ResolvedTelemetryAuth | undefined {
+  try {
+    return resolveTelemetryAuth(opts, deps);
+  } catch (error) {
+    try {
+      deps.onError?.(error);
+    } catch {
+      // A broken diagnostic sink must not turn a skipped beacon into a failure.
+    }
+    return undefined;
+  }
+}
+
 /**
  * Fire one command-outcome event to the beacon. Awaited by `index.ts` before
  * `process.exit` (the flush) — bounded and fully best-effort, so it can neither

test/e2e/telemetry-unreadable-credentials.e2e.test.ts

@@ -0,0 +1,173 @@
+/**
+ * An unreadable credentials file must not crash commands in the telemetry
+ * pre-action hook (#465). The hook resolves telemetry auth before every
+ * command's action; a raw EACCES/EPERM from that read used to end the process
+ * before `doctor` could diagnose the file or an offline command could run.
+ * Now the command runs as if telemetry were off: no beacon, and `doctor`
+ * reports the problem itself.
+ *
+ * Exercises the built entrypoint against a local server that records every
+ * request. `chmod 000` is how the read is made to fail, so the suite needs a
+ * POSIX host and a non-root user (root reads through mode bits); the hook's
+ * error handling itself is unit-tested cross-platform in `telemetry.spec.ts`.
+ */
+
+import { chmodSync, existsSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs';
+import { createServer, type Server } from 'node:http';
+import { tmpdir } from 'node:os';
+import { dirname, join, resolve } from 'node:path';
+import { spawn } from 'node:child_process';
+import { fileURLToPath } from 'node:url';
+import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it } from 'vitest';
+
+const __dirname = dirname(fileURLToPath(import.meta.url));
+const REPO_ROOT = resolve(__dirname, '../..');
+const BIN_PATH = join(REPO_ROOT, 'dist', 'index.js');
+
+const canDenyRead = process.platform !== 'win32' && process.getuid?.() !== 0;
+
+let server: Server;
+let baseUrl = '';
+const requests: string[] = [];
+let home = '';
+let credentialsPath = '';
+
+beforeAll(async () => {
+  if (!existsSync(BIN_PATH)) {
+    throw new Error('dist/index.js not found — run `npm run test:e2e` which builds first.');
+  }
+  server = createServer((req, res) => {
+    const path = (req.url ?? '').split('?')[0];
+    requests.push(`${req.method} ${path}`);
+    req.resume();
+    if (req.method === 'POST' && path === '/api/cli/v1/telemetry') {
+      res.writeHead(204, { connection: 'close' });
+      res.end();
+      return;
+    }
+    if (req.method === 'GET' && path === '/api/cli/v1/me') {
+      res.writeHead(200, { 'content-type': 'application/json', connection: 'close' });
+      res.end(
+        JSON.stringify({
+          userId: 'user_e2e',
+          keyId: 'key_e2e',
+          scopes: ['tests:read'],
+          env: 'e2e',
+        }),
+      );
+      return;
+    }
+    res.writeHead(404, { connection: 'close' });
+    res.end();
+  });
+  await new Promise<void>(resolveListen => server.listen(0, '127.0.0.1', () => resolveListen()));
+  const address = server.address();
+  if (address === null || typeof address === 'string') throw new Error('no server address');
+  baseUrl = `http://127.0.0.1:${address.port}`;
+});
+
+beforeEach(() => {
+  home = mkdtempSync(join(tmpdir(), 'ts-e2e-unreadable-creds-'));
+  mkdirSync(join(home, '.testsprite'));
+  credentialsPath = join(home, '.testsprite', 'credentials');
+  writeFileSync(credentialsPath, '[default]\napi_key = sk-user-from-file\n', { mode: 0o600 });
+});
+
+afterEach(() => {
+  requests.length = 0;
+  // Restore access first so the temp tree can be removed.
+  if (existsSync(credentialsPath)) chmodSync(credentialsPath, 0o600);
+  rmSync(home, { recursive: true, force: true });
+});
+
+afterAll(async () => {
+  await new Promise<void>(resolveClose => {
+    server.close(() => resolveClose());
+    server.closeAllConnections();
+  });
+});
+
+async function runCli(args: string[]): Promise<{ status: number; stdout: string; stderr: string }> {
+  const child = spawn(process.execPath, [BIN_PATH, ...args], {
+    cwd: home,
+    stdio: ['ignore', 'pipe', 'pipe'],
+    timeout: 20_000,
+    env: {
+      ...process.env,
+      HOME: home,
+      USERPROFILE: home,
+      // A key in the environment: the file is still read (loadConfig always
+      // reads the profile), and without the fix that read is what crashes.
+      TESTSPRITE_API_KEY: 'sk-user-e2e-env-key',
+      TESTSPRITE_API_URL: baseUrl,
+      TESTSPRITE_PROFILE: '',
+      TESTSPRITE_NO_SKILL_WARNING: '1',
+      TESTSPRITE_NO_UPDATE_NOTIFIER: '1',
+      // Telemetry must be ON — that is the path under test.
+      TESTSPRITE_NO_TELEMETRY: '',
+      DO_NOT_TRACK: '',
+      CI: '',
+    },
+  });
+  let stdout = '';
+  let stderr = '';
+  child.stdout.setEncoding('utf8').on('data', (chunk: string) => (stdout += chunk));
+  child.stderr.setEncoding('utf8').on('data', (chunk: string) => (stderr += chunk));
+  const status = await new Promise<number>((resolveExit, reject) => {
+    child.once('error', reject);
+    child.once('close', code => resolveExit(code ?? -1));
+  });
+  return { status, stdout, stderr };
+}
+
+const telemetryPosts = (): string[] => requests.filter(r => r === 'POST /api/cli/v1/telemetry');
+
+describe('telemetry pre-action hook — readable credentials (control)', () => {
+  it('sends the beacon, so its absence below is meaningful', async () => {
+    const result = await runCli(['agent', 'list']);
+    expect(result.status).toBe(0);
+    expect(telemetryPosts()).toHaveLength(1);
+  });
+});
+
+describe.skipIf(!canDenyRead)('telemetry pre-action hook — unreadable credentials file', () => {
+  beforeEach(() => {
+    chmodSync(credentialsPath, 0o000);
+  });
+
+  it('doctor runs and reports the Credentials problem itself; no beacon', async () => {
+    const result = await runCli(['doctor']);
+    const output = result.stdout + result.stderr;
+    // TESTSPRITE_API_KEY is set and /me answers, so the unreadable file is a
+    // WARN (commands still work) and doctor completes with exit 0.
+    expect(result.status).toBe(0);
+    expect(output).not.toMatch(/Error: (EACCES|EPERM)/);
+    expect(output).toContain('TestSprite doctor');
+    expect(output).toMatch(
+      /\[WARN\]\s+Credentials\s+credentials file exists but cannot be read \((EACCES|EPERM)\)/,
+    );
+    expect(telemetryPosts()).toEqual([]);
+  });
+
+  it('an offline command (agent list) runs and exits 0; no beacon', async () => {
+    const result = await runCli(['agent', 'list']);
+    expect(result.stderr).not.toMatch(/EACCES|EPERM/);
+    expect(result.status).toBe(0);
+    expect(result.stdout).toContain('AGENT');
+    expect(telemetryPosts()).toEqual([]);
+  });
+
+  it('completion still prints its script and exits 0; no beacon', async () => {
+    const result = await runCli(['completion', 'bash']);
+    expect(result.status).toBe(0);
+    expect(result.stdout).toContain('_testsprite_completion');
+    expect(telemetryPosts()).toEqual([]);
+  });
+
+  it('--debug names why the beacon was skipped', async () => {
+    const result = await runCli(['--debug', 'agent', 'list']);
+    expect(result.status).toBe(0);
+    expect(result.stderr).toMatch(/\[debug\] telemetry skipped: (EACCES|EPERM)/);
+    expect(telemetryPosts()).toEqual([]);
+  });
+});