refactor(cli): one credential reader for every client of the API

`readCodemanEnv()` in cli.ts and `parseEnvFile()`/`readCodemanCredentials()` in tui-client.ts were two copies of the same `.env` reader (the TUI's said so in a comment), and `codeman agent` was about to need a third. They move into `src/codeman-credentials.ts`; `codeman attach` and the TUI read from it, and the TUI keeps re-exporting its names so nothing that imports them changes. The lookup order is one pure function, `credentialsFrom(env, fileEnv)`: each field from the environment, then the file, username defaulting to `admin` — the order attach and the TUI already used. `readCodemanCredentials` takes the environment as a parameter so a caller with its own (the agent CLI's guard) gets the same answer. The parser now also tolerates an `export ` prefix, which the agent skill's preamble already accepted in the same file. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
2026-10-10 09:19:42 +02:00 · 2026-10-09 12:59:48 +02:00
parent 3a0cee6b90
commit 6d862d5323
4 changed files with 148 additions and 75 deletions
@@ -15,6 +15,7 @@ import { existsSync, readFileSync } from 'node:fs';
 import { isAbsolute, join } from 'node:path';
 import { homedir } from 'node:os';
 import { dataPath } from './config/instance.js';
+import { readCodemanCredentials } from './codeman-credentials.js';
 import { casePath } from './config/cases-dir.js';
 import { assertValidBasePath } from './config/base-path.js';
 import { installAgentSkillInto, removeAgentSkillFrom, type AgentSkillApplyResult } from './hooks-config.js';
@@ -42,32 +43,8 @@ function makeAttachmentMagicLink(filePath: string): string {
  return `codeman://attach?path=${encodeURIComponent(filePath)}`;
 }

-function readCodemanEnv(): Record<string, string> {
-  const envPath = dataPath('.env');
-  try {
-    const text = readFileSync(envPath, 'utf-8');
-    const result: Record<string, string> = {};
-    for (const rawLine of text.split(/\r?\n/)) {
-      const line = rawLine.trim();
-      if (!line || line.startsWith('#')) continue;
-      const match = line.match(/^([A-Za-z_][A-Za-z0-9_]*)=(.*)$/);
-      if (!match) continue;
-      let value = match[2].trim();
-      if ((value.startsWith('"') && value.endsWith('"')) || (value.startsWith("'") && value.endsWith("'"))) {
-        value = value.slice(1, -1);
-      }
-      result[match[1]] = value;
-    }
-    return result;
-  } catch {
-    return {};
-  }
-}
-
 async function postAttachment(apiUrl: string, sessionId: string, filePath: string): Promise<boolean> {
-  const envFile = readCodemanEnv();
-  const username = process.env.CODEMAN_USERNAME || envFile.CODEMAN_USERNAME || 'admin';
-  const password = process.env.CODEMAN_PASSWORD || envFile.CODEMAN_PASSWORD;
+  const { username, password } = readCodemanCredentials();
  const url = new URL(`/api/sessions/${encodeURIComponent(sessionId)}/attachments`, apiUrl);
  const body = JSON.stringify({ path: filePath });
  const transport = url.protocol === 'https:' ? https : http;
@@ -641,9 +618,7 @@ function probeWebServerAt(base: string): Promise<WebServerProbe | null> {
  } catch {
    return Promise.resolve(null);
  }
-  const envFile = readCodemanEnv();
-  const username = process.env.CODEMAN_USERNAME || envFile.CODEMAN_USERNAME || 'admin';
-  const password = process.env.CODEMAN_PASSWORD || envFile.CODEMAN_PASSWORD;
+  const { username, password } = readCodemanCredentials();
  const transport = url.protocol === 'https:' ? https : http;
  const headers: Record<string, string> = { Accept: 'application/json' };
  if (password) {
@@ -0,0 +1,75 @@
+/**
+ * @fileoverview Credentials for a client of this Codeman instance's own API.
+ *
+ * Env first, the data dir's `.env` as the fallback — the hand-authored file
+ * `codeman attach`, `codeman tui` and `codeman agent` all read. One reader, so the
+ * three clients cannot drift on quoting, comments or the default username.
+ *
+ * @module codeman-credentials
+ */
+import { readFileSync } from 'node:fs';
+import { dataPath } from './config/instance.js';
+
+export interface CodemanCredentials {
+  username: string;
+  /** Absent when no password is configured (or only the server's environment has it). */
+  password?: string;
+}
+
+/**
+ * Parse a `KEY=value` env file: blank lines and `#` comments skipped, an `export `
+ * prefix tolerated (the file is hand-authored, often sourced by a shell too), one
+ * layer of matching quotes stripped, anything that is not an assignment ignored.
+ */
+export function parseEnvFile(text: string): Record<string, string> {
+  const result: Record<string, string> = {};
+  for (const rawLine of text.split(/\r?\n/)) {
+    const line = rawLine.trim();
+    if (!line || line.startsWith('#')) continue;
+    const match = line.match(/^(?:export\s+)?([A-Za-z_][A-Za-z0-9_]*)=(.*)$/);
+    if (!match) continue;
+    let value = match[2].trim();
+    if ((value.startsWith('"') && value.endsWith('"')) || (value.startsWith("'") && value.endsWith("'"))) {
+      value = value.slice(1, -1);
+    }
+    result[match[1]] = value;
+  }
+  return result;
+}
+
+/** The data dir's `.env`, parsed. Absent or unreadable means `{}`. */
+export function readCodemanEnvFile(envFilePath: string = dataPath('.env')): Record<string, string> {
+  try {
+    return parseEnvFile(readFileSync(envFilePath, 'utf-8'));
+  } catch {
+    return {};
+  }
+}
+
+/**
+ * The lookup order every client uses, per field: the environment, then the `.env`
+ * file, then (username only) `admin`. Pure, so a caller with its own environment
+ * object (`codeman agent`'s guard takes one for testability) gets the same answer.
+ */
+export function credentialsFrom(env: NodeJS.ProcessEnv, fileEnv: Record<string, string>): CodemanCredentials {
+  const username = env.CODEMAN_USERNAME || fileEnv.CODEMAN_USERNAME || 'admin';
+  const password = env.CODEMAN_PASSWORD || fileEnv.CODEMAN_PASSWORD;
+  return password ? { username, password } : { username };
+}
+
+/**
+ * Credentials for the API. No password means no auth is configured, or the user has
+ * it only in the server's environment, in which case the API answers 401.
+ */
+export function readCodemanCredentials(
+  envFilePath: string = dataPath('.env'),
+  env: NodeJS.ProcessEnv = process.env
+): CodemanCredentials {
+  return credentialsFrom(env, readCodemanEnvFile(envFilePath));
+}
+
+/** `Authorization` header value, or undefined when there is no password to send. */
+export function basicAuthHeader(credentials: CodemanCredentials): string | undefined {
+  if (!credentials.password) return undefined;
+  return `Basic ${Buffer.from(`${credentials.username}:${credentials.password}`).toString('base64')}`;
+}
@@ -48,6 +48,12 @@ import https from 'node:https';
 import { hostname as osHostname } from 'node:os';
 import { promisify } from 'node:util';
 import { CODEMAN_INSTANCE, dataPath, resolveTmuxSocketName } from '../config/instance.js';
+import {
+  basicAuthHeader,
+  parseEnvFile,
+  readCodemanCredentials,
+  type CodemanCredentials,
+} from '../codeman-credentials.js';
 import { EXEC_TIMEOUT_MS } from '../config/exec-timeout.js';
 import { probeServer } from '../daemon-control.js';
 import { getErrorMessage } from '../types/api.js';
@@ -281,53 +287,9 @@ export function tuiServerCandidates(env: { apiUrl?: string; port?: string | numb
  return [`https://127.0.0.1:${port}`, `http://127.0.0.1:${port}`];
 }

-/**
- * Parse a `KEY=value` env file. Mirrors `readCodemanEnv()` in `cli.ts`: blank
- * lines and `#` comments skipped, one layer of matching quotes stripped.
- */
-export function parseEnvFile(text: string): Record<string, string> {
-  const result: Record<string, string> = {};
-  for (const rawLine of text.split(/\r?\n/)) {
-    const line = rawLine.trim();
-    if (!line || line.startsWith('#')) continue;
-    const match = line.match(/^([A-Za-z_][A-Za-z0-9_]*)=(.*)$/);
-    if (!match) continue;
-    let value = match[2].trim();
-    if ((value.startsWith('"') && value.endsWith('"')) || (value.startsWith("'") && value.endsWith("'"))) {
-      value = value.slice(1, -1);
-    }
-    result[match[1]] = value;
-  }
-  return result;
-}
-
-export interface TuiCredentials {
-  username: string;
-  password?: string;
-}
-
-/**
- * Credentials for the API, env first and the data dir's `.env` as the fallback,
- * exactly like the `codeman attach` path. No password means no auth is
- * configured (or the user has it only in the server's environment, in which
- * case the API answers 401 and `connect()` reports `authRequired`).
- */
-export function readCodemanCredentials(envFilePath = dataPath('.env')): TuiCredentials {
-  let fileEnv: Record<string, string> = {};
-  try {
-    fileEnv = parseEnvFile(readFileSync(envFilePath, 'utf-8'));
-  } catch {
-    /* absent or unreadable: env-only */
-  }
-  const username = process.env.CODEMAN_USERNAME || fileEnv.CODEMAN_USERNAME || 'admin';
-  const password = process.env.CODEMAN_PASSWORD || fileEnv.CODEMAN_PASSWORD;
-  return password ? { username, password } : { username };
-}
-
-export function basicAuthHeader(credentials: TuiCredentials): string | undefined {
-  if (!credentials.password) return undefined;
-  return `Basic ${Buffer.from(`${credentials.username}:${credentials.password}`).toString('base64')}`;
-}
+// One credential reader for every client of the API (attach, tui, agent).
+export { parseEnvFile, readCodemanCredentials, basicAuthHeader };
+export type TuiCredentials = CodemanCredentials;

 // ─────────────────────────────────────────────────────────────────────────────
 // Degraded mode
@@ -0,0 +1,61 @@
+/**
+ * @fileoverview The one credential reader `codeman attach`, `codeman tui` and
+ * `codeman agent` share: env first, the data dir's `.env` as the fallback.
+ */
+import { mkdtempSync, rmSync, writeFileSync } from 'node:fs';
+import { tmpdir } from 'node:os';
+import { join } from 'node:path';
+import { afterEach, beforeEach, describe, expect, it } from 'vitest';
+import { basicAuthHeader, readCodemanCredentials, readCodemanEnvFile } from '../src/codeman-credentials.js';
+
+describe('readCodemanCredentials', () => {
+  let dir: string;
+  const saved = { user: process.env.CODEMAN_USERNAME, pass: process.env.CODEMAN_PASSWORD };
+
+  beforeEach(() => {
+    dir = mkdtempSync(join(tmpdir(), 'codeman-cred-'));
+    delete process.env.CODEMAN_USERNAME;
+    delete process.env.CODEMAN_PASSWORD;
+  });
+
+  afterEach(() => {
+    rmSync(dir, { recursive: true, force: true });
+    if (saved.user === undefined) delete process.env.CODEMAN_USERNAME;
+    else process.env.CODEMAN_USERNAME = saved.user;
+    if (saved.pass === undefined) delete process.env.CODEMAN_PASSWORD;
+    else process.env.CODEMAN_PASSWORD = saved.pass;
+  });
+
+  it('falls back to the .env file, quotes and an export prefix stripped, default user admin', () => {
+    const env = join(dir, '.env');
+    writeFileSync(env, '# a comment\nnot an assignment\nexport CODEMAN_PASSWORD="hunter2"\n');
+    expect(readCodemanCredentials(env)).toEqual({ username: 'admin', password: 'hunter2' });
+  });
+
+  it('prefers the environment over the file', () => {
+    const env = join(dir, '.env');
+    writeFileSync(env, 'CODEMAN_USERNAME=file\nCODEMAN_PASSWORD=file-pass\n');
+    process.env.CODEMAN_USERNAME = 'envuser';
+    process.env.CODEMAN_PASSWORD = 'env-pass';
+    expect(readCodemanCredentials(env)).toEqual({ username: 'envuser', password: 'env-pass' });
+  });
+
+  it('an absent file means no password, and no header to send', () => {
+    const creds = readCodemanCredentials(join(dir, 'missing'));
+    expect(creds).toEqual({ username: 'admin' });
+    expect(basicAuthHeader(creds)).toBeUndefined();
+    expect(readCodemanEnvFile(join(dir, 'missing'))).toEqual({});
+  });
+
+  it('takes an explicit environment, field by field', () => {
+    const env = join(dir, '.env');
+    writeFileSync(env, 'CODEMAN_USERNAME=joe\n');
+    expect(readCodemanCredentials(env, { CODEMAN_PASSWORD: 'pw' })).toEqual({ username: 'joe', password: 'pw' });
+  });
+
+  it('builds a Basic header from a password', () => {
+    expect(basicAuthHeader({ username: 'joe', password: 'pw' })).toBe(
+      `Basic ${Buffer.from('joe:pw').toString('base64')}`
+    );
+  });
+});