fix(mosaic): close dup-proxy race + verify listener identity (re-review #2, #790)
All checks were successful
ci/woodpecker/pr/ci Pipeline was successful

Addresses the three items from PR #793 re-review #2, all TDD red-first on this
branch:

F1 (BLOCKER, dup-proxy race) — ensureProxyRunning no longer falls back to nohup
after `systemctl start` is ACCEPTED. An accepted-but-not-yet-healthy systemd
unit may bind late or be restarted; spawning nohup then would create a second
proxy contending for :18765. Once systemd accepts the job we poll to the startup
deadline and, on a miss, report a managed-service startup failure. nohup is now
reachable only when systemd never accepted the job. New test: "does NOT fall back
to nohup after systemd accepts but never binds".

F2 (BLOCKER, CWE-345) — a 2xx on /healthz is liveness, not identity. Added
verifyListenerIdentity(): an OS-level check (via `ss` + /proc) that the process
owning :18765 is the current uid running the expected proxy executable, failing
CLOSED (`unknown`) when identity can't be established — needs no upstream
shared-secret/unix-socket support. ensureProxyRunning now gates EVERY trust point
on it: an already-present responder that fails identity returns `untrusted`
without starting anything; a started proxy is trusted only once it is both live
AND identity-verified. probeLiveness doc-comment updated to record the residual
CWE-345 risk and point at the identity check (multi-user warning deferred).

F3 (SHOULD-FIX) — parseAuthStatus no longer treats a signal-terminated check
(status null) with an auth-looking line as valid; a clean exit 0 is required.
Regression test: { status: null, stdout: 'Authenticated' } → unknown.

Gates green: typecheck, lint, format:check; full mosaic suite 1110 passing;
claudex-proxy.ts coverage 92.3% stmts/lines, 88.13% branch (>= 85%).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Hermes Agent
2026-07-16 15:43:16 -05:00
parent 7f987f52fc
commit 6bef94ea0d
2 changed files with 309 additions and 40 deletions

View File

@@ -21,11 +21,13 @@ import {
systemdUnitPath,
installSystemdUnit,
startNohupProxy,
verifyListenerIdentity,
runProxyPreflight,
ensureProxyRunning,
type AuthStatus,
type ProxyRunResult,
type SpawnedChild,
type ListenerIdentity,
} from './claudex-proxy.js';
/**
@@ -94,6 +96,14 @@ describe('parseAuthStatus', () => {
expect(s.state).toBe('unknown');
});
it('does NOT trust a signal-terminated check (status null) even with an auth-looking line', () => {
// status: null means the process was killed by a signal — an INCOMPLETE
// check. An auth-looking line that happened to be flushed must not be read
// as valid, or preflight passes on a check that never finished.
const s = parseAuthStatus({ status: null, stdout: 'Authenticated', stderr: '' });
expect(s.state).toBe('unknown');
});
it('extracts a best-effort expiry in days when present', () => {
const s = parseAuthStatus({
status: 0,
@@ -521,15 +531,93 @@ describe('startNohupProxy (finding #1 — async spawn error must not crash)', ()
});
});
describe('verifyListenerIdentity (finding #2 — OS-level listener identity, CWE-345)', () => {
const me: ListenerIdentity = {
pid: 4242,
uid: 1000,
exePath: '/home/me/.local/bin/claude-code-proxy',
};
it('accepts a listener owned by the current uid whose exe is the expected proxy path', () => {
const verdict = verifyListenerIdentity({
identify: () => me,
currentUid: () => 1000,
expectedExe: () => '/home/me/.local/bin/claude-code-proxy',
});
expect(verdict).toBe('ok');
});
it('accepts by basename when the expected path is unknown but the exe is claude-code-proxy', () => {
const verdict = verifyListenerIdentity({
identify: () => me,
currentUid: () => 1000,
expectedExe: () => null,
});
expect(verdict).toBe('ok');
});
it('rejects a listener owned by a DIFFERENT uid (foreign-user) — fail closed', () => {
const verdict = verifyListenerIdentity({
identify: () => ({ ...me, uid: 0 }),
currentUid: () => 1000,
expectedExe: () => '/home/me/.local/bin/claude-code-proxy',
});
expect(verdict).toBe('foreign-user');
});
it('rejects a same-user listener whose exe is NOT the proxy (wrong-exe)', () => {
const verdict = verifyListenerIdentity({
identify: () => ({ ...me, exePath: '/usr/bin/nc' }),
currentUid: () => 1000,
expectedExe: () => '/home/me/.local/bin/claude-code-proxy',
});
expect(verdict).toBe('wrong-exe');
});
it('returns unknown (fail closed) when the listener cannot be identified', () => {
const verdict = verifyListenerIdentity({
identify: () => null,
currentUid: () => 1000,
expectedExe: () => '/home/me/.local/bin/claude-code-proxy',
});
expect(verdict).toBe('unknown');
});
it('returns unknown when the current uid is unavailable (non-posix)', () => {
const verdict = verifyListenerIdentity({
identify: () => me,
currentUid: () => -1,
expectedExe: () => '/home/me/.local/bin/claude-code-proxy',
});
expect(verdict).toBe('unknown');
});
it('returns unknown when the listener exe path cannot be read', () => {
const verdict = verifyListenerIdentity({
identify: () => ({ ...me, exePath: null }),
currentUid: () => 1000,
expectedExe: () => '/home/me/.local/bin/claude-code-proxy',
});
expect(verdict).toBe('unknown');
});
it('runs with real defaults without throwing (identity may be unresolved → verdict)', () => {
const verdict = verifyListenerIdentity();
expect(['ok', 'foreign-user', 'wrong-exe', 'unknown']).toContain(verdict);
});
});
describe('ensureProxyRunning', () => {
const ok: ProxyRunResult = { status: 0, stdout: '', stderr: '' };
const nohupOk = async (): Promise<ProxyRunResult> => ok;
const trusted = () => 'ok' as const;
it('is a no-op when the proxy is already live', async () => {
it('is a no-op when the proxy is already live AND identity-verified', async () => {
const startSystemd = vi.fn(() => ok);
const startNohup = vi.fn(nohupOk);
const r = await ensureProxyRunning({
probe: async () => true,
verifyListener: trusted,
startSystemd,
startNohup,
waitMs: async () => {},
@@ -540,10 +628,30 @@ describe('ensureProxyRunning', () => {
expect(startNohup).not.toHaveBeenCalled();
});
it('starts via systemd when available and then becomes live', async () => {
it('fails closed as untrusted when a responder holds :18765 but identity is NOT ours', async () => {
// A foreign process answers /healthz but the listener is not our proxy
// (foreign uid / wrong exe / unidentifiable). We must NOT trust it and must
// NOT start a second proxy (the port is already taken) — fail closed.
const startSystemd = vi.fn(() => ok);
const startNohup = vi.fn(nohupOk);
const r = await ensureProxyRunning({
probe: async () => true,
verifyListener: () => 'foreign-user',
startSystemd,
startNohup,
waitMs: async () => {},
});
expect(r.method).toBe('untrusted');
expect(r.live).toBe(false);
expect(startSystemd).not.toHaveBeenCalled();
expect(startNohup).not.toHaveBeenCalled();
});
it('starts via systemd when available and then becomes trusted-live', async () => {
let calls = 0;
const r = await ensureProxyRunning({
probe: async () => calls++ > 0, // dead first, live after start
verifyListener: trusted,
startSystemd: () => ok,
startNohup: async () => {
throw new Error('should not fall back');
@@ -554,7 +662,7 @@ describe('ensureProxyRunning', () => {
expect(r.live).toBe(true);
});
it('waits past a slow systemd bind before falling back (finding #2 — no duplicate proxy)', async () => {
it('waits past a slow systemd bind before giving up (finding #2 — no duplicate proxy)', async () => {
// systemd `start` returns 0 (job accepted) but the socket only binds on the
// 4th probe — still well within the startup deadline. nohup must NOT run,
// or two proxies would contend for :18765.
@@ -562,6 +670,7 @@ describe('ensureProxyRunning', () => {
const startNohup = vi.fn(nohupOk);
const r = await ensureProxyRunning({
probe: async () => probes++ >= 3,
verifyListener: trusted,
startSystemd: () => ok,
startNohup,
waitMs: async () => {},
@@ -573,28 +682,54 @@ describe('ensureProxyRunning', () => {
expect(startNohup).not.toHaveBeenCalled();
});
it('falls back to nohup when systemd is accepted but never binds in the deadline', async () => {
it('does NOT fall back to nohup after systemd accepts but never binds (finding #1 — dup-proxy race)', async () => {
// systemctl start exit 0 means the job was ACCEPTED, not bound. If it binds
// just after our deadline (or systemd restarts it), a nohup fallback would
// create a SECOND proxy contending for :18765. Once systemd has accepted the
// job we never spawn nohup — we report a managed-service startup failure.
const startNohup = vi.fn(nohupOk);
const r = await ensureProxyRunning({
// Dead until nohup has actually run; systemd's whole poll window stays dead.
probe: async () => startNohup.mock.calls.length > 0,
probe: async () => false, // never becomes live within the deadline
verifyListener: trusted,
startSystemd: () => ok,
startNohup,
waitMs: async () => {},
settleMs: 10,
startupDeadlineMs: 30,
});
expect(startNohup).toHaveBeenCalledTimes(1);
expect(r.method).toBe('nohup');
expect(r.live).toBe(true);
expect(startNohup).not.toHaveBeenCalled();
expect(r.method).toBe('failed');
expect(r.live).toBe(false);
});
it('falls back to nohup when systemd start fails outright', async () => {
it('does NOT trust a systemd-started responder whose identity cannot be verified', async () => {
// Dead at first (so we reach the systemd start), then the socket binds — but
// identity never verifies (e.g. a squatter beat systemd to the port). A live
// responder that fails identity must never be reported as a successful start.
let calls = 0;
const startNohup = vi.fn(nohupOk);
const r = await ensureProxyRunning({
probe: async () => calls++ > 0,
verifyListener: () => 'wrong-exe',
startSystemd: () => ok,
startNohup,
waitMs: async () => {},
settleMs: 10,
startupDeadlineMs: 30,
});
expect(startNohup).not.toHaveBeenCalled();
expect(r.method).toBe('failed');
expect(r.live).toBe(false);
});
it('falls back to nohup only when systemd start FAILS outright (not accepted)', async () => {
let calls = 0;
const r = await ensureProxyRunning({
// A failed systemd start skips its post-start poll, so probes are:
// #0 initial (dead), #1 after nohup (live).
// #0 initial (dead), #1 after nohup (live). nohup fallback is reachable
// ONLY because systemd never accepted the job (status 1).
probe: async () => calls++ > 0,
verifyListener: trusted,
startSystemd: () => ({ status: 1, stdout: '', stderr: 'no systemd' }),
startNohup: nohupOk,
waitMs: async () => {},
@@ -605,9 +740,25 @@ describe('ensureProxyRunning', () => {
expect(r.live).toBe(true);
});
it('does NOT trust a nohup-started responder whose identity cannot be verified', async () => {
let calls = 0;
const r = await ensureProxyRunning({
probe: async () => calls++ > 0,
verifyListener: () => 'unknown',
startSystemd: () => ({ status: 1, stdout: '', stderr: 'no systemd' }),
startNohup: nohupOk,
waitMs: async () => {},
settleMs: 10,
startupDeadlineMs: 30,
});
expect(r.method).toBe('failed');
expect(r.live).toBe(false);
});
it('reports failed when nothing brings the proxy up', async () => {
const r = await ensureProxyRunning({
probe: async () => false,
verifyListener: trusted,
startSystemd: () => ({ status: 1, stdout: '', stderr: '' }),
startNohup: async () => ({ status: 1, stdout: '', stderr: '' }),
waitMs: async () => {},

View File

@@ -17,9 +17,9 @@
*/
import { execFileSync, spawn, spawnSync } from 'node:child_process';
import { mkdirSync, writeFileSync } from 'node:fs';
import { mkdirSync, readFileSync, readlinkSync, writeFileSync } from 'node:fs';
import { homedir } from 'node:os';
import { dirname, join } from 'node:path';
import { basename, dirname, join } from 'node:path';
// ─── Endpoint / command constants (spec table) ──────────────────────────────
@@ -126,7 +126,10 @@ export function parseAuthStatus(result: ProxyRunResult): AuthStatus {
state = 'expired';
} else if (unauth) {
state = 'unauthenticated';
} else if (authed && (result.status === 0 || result.status === null)) {
} else if (authed && result.status === 0) {
// A `null` status means the check was killed by a signal — an INCOMPLETE
// run. We require a clean exit 0 for `valid`; a partially-flushed auth line
// from a signal-terminated check must never be trusted (finding #3).
state = 'valid';
} else if (result.status === 0) {
state = 'valid';
@@ -181,16 +184,21 @@ export function runDeviceReauth(spawnImpl: InheritSpawn = defaultInheritSpawn):
* Probe the proxy for liveness by hitting its dedicated `GET /healthz` endpoint
* and requiring a 2xx response.
*
* This deliberately does NOT trust an arbitrary HTTP response on the port. The
* proxy binds loopback with no client authentication, so on a shared host any
* local process could occupy :18765; trusting "any response = alive" (as an
* earlier revision did) would let such a squatter be mistaken for the proxy and
* intercept Claude traffic (CWE-345). Requiring a 2xx on the proxy's own
* `/healthz` contract is the strongest identity signal available without an
* upstream shared-secret/unix-socket handshake (which `claude-code-proxy` does
* not provide). It also resolves spec gotcha #1: the root path returns non-2xx,
* but `/healthz` returns 2xx when healthy, so a live proxy is never mistaken for
* dead and no duplicate is spawned.
* This is a LIVENESS check only — it answers "is a healthy proxy responding?",
* not "is that responder actually ours?". Requiring a 2xx on the proxy's own
* `/healthz` contract (rather than "any HTTP response = alive") resolves spec
* gotcha #1: the root path returns non-2xx, but `/healthz` returns 2xx when
* healthy, so a live proxy is never mistaken for dead and no duplicate proxy is
* spawned.
*
* Residual risk (CWE-345): the proxy binds loopback with NO client
* authentication, so on a shared host a local process could occupy :18765 and
* serve a 2xx here. A 2xx therefore does NOT by itself establish that the
* listener is our proxy. Identity is verified SEPARATELY and at every trust
* point by {@link verifyListenerIdentity} (OS-level uid + executable check),
* which fails closed when identity can't be established. See
* {@link ensureProxyRunning}. (Broader multi-user hardening — a persistent
* warning when a foreign listener is seen — is tracked for a later phase.)
*/
export async function probeLiveness(
url: string = CLAUDEX_HEALTH_URL,
@@ -222,6 +230,92 @@ export async function probeLiveness(
}
}
// ─── Listener identity (OS-level, CWE-345 mitigation) ─────────────────────────
/**
* The result of verifying who actually owns the :18765 listener.
* - `ok` — same-user process running the expected proxy binary.
* - `foreign-user` — a process owned by a DIFFERENT uid holds the port.
* - `wrong-exe` — same-user, but the executable is not the proxy.
* - `unknown` — identity could not be established (fail closed).
*/
export type ListenerVerdict = 'ok' | 'foreign-user' | 'wrong-exe' | 'unknown';
/** OS-level identity of the process bound to the proxy port. */
export interface ListenerIdentity {
pid: number;
uid: number;
/** Absolute path of the process executable, or null if unreadable. */
exePath: string | null;
}
export interface VerifyListenerDeps {
/** Resolve the process bound to the proxy port (null → unidentifiable). */
identify?: () => ListenerIdentity | null;
/** The current process uid (-1 when unavailable, e.g. non-posix). */
currentUid?: () => number;
/** The expected proxy executable path (null when it can't be resolved). */
expectedExe?: () => string | null;
}
/**
* Identify the process listening on the proxy port via `ss` + `/proc`. Every
* failure path returns null so the caller fails closed. Reads no credential
* material — only pid/uid/exe path of the listener.
*/
function defaultIdentifyListener(port: number = CLAUDEX_PROXY_PORT): ListenerIdentity | null {
try {
const out = execFileSync('ss', ['-H', '-ltnp', `sport = :${port}`], { encoding: 'utf8' });
const pidMatch = /pid=(\d+)/.exec(out);
if (!pidMatch) return null;
const pid = Number(pidMatch[1]);
if (!Number.isInteger(pid) || pid <= 0) return null;
const status = readFileSync(`/proc/${pid}/status`, 'utf8');
const uidLine = /^Uid:\s*(\d+)/m.exec(status);
if (!uidLine) return null;
const uid = Number(uidLine[1]);
let exePath: string | null = null;
try {
exePath = readlinkSync(`/proc/${pid}/exe`);
} catch {
exePath = null;
}
return { pid, uid, exePath };
} catch {
return null;
}
}
/**
* Verify that the process owning :18765 is genuinely OUR proxy before trusting
* it. The proxy binds loopback with NO client authentication, so on a shared
* host any local process could squat the port and a liveness 2xx alone does not
* prove identity (CWE-345). We check the OS-level identity of the listener — it
* must be owned by the current uid and be the expected proxy executable — and
* FAIL CLOSED (`unknown`) whenever identity cannot be established. This needs no
* upstream shared-secret/unix-socket support from `claude-code-proxy`.
*/
export function verifyListenerIdentity(deps: VerifyListenerDeps = {}): ListenerVerdict {
const identify = deps.identify ?? (() => defaultIdentifyListener());
const currentUid =
deps.currentUid ?? (() => (typeof process.getuid === 'function' ? process.getuid() : -1));
const expectedExe = deps.expectedExe ?? (() => checkProxyBinary().path);
const id = identify();
if (!id) return 'unknown'; // can't see the listener → don't trust it
const uid = currentUid();
if (uid < 0) return 'unknown'; // can't establish our own identity → fail closed
if (id.uid !== uid) return 'foreign-user'; // someone else's process holds the port
if (!id.exePath) return 'unknown'; // can't confirm the executable → fail closed
const expected = expectedExe();
if (expected && id.exePath === expected) return 'ok';
if (basename(id.exePath) === CLAUDEX_PROXY_BINARY) return 'ok';
return 'wrong-exe';
}
// ─── systemd user unit ───────────────────────────────────────────────────────
export function systemdUnitPath(home: string = homedir()): string {
@@ -383,7 +477,7 @@ export async function runProxyPreflight(deps: PreflightDeps = {}): Promise<Prefl
// ─── Lifecycle: ensure the proxy is running ──────────────────────────────────
export type ProxyStartMethod = 'already' | 'systemd' | 'nohup' | 'failed';
export type ProxyStartMethod = 'already' | 'systemd' | 'nohup' | 'untrusted' | 'failed';
export interface EnsureProxyResult {
live: boolean;
@@ -458,6 +552,8 @@ export function startNohupProxy(deps: StartNohupDeps = {}): Promise<ProxyRunResu
export interface EnsureProxyDeps {
probe?: () => Promise<boolean>;
/** OS-level identity check for the process holding the proxy port. */
verifyListener?: () => ListenerVerdict;
startSystemd?: () => ProxyRunResult;
startNohup?: () => Promise<ProxyRunResult>;
waitMs?: (ms: number) => Promise<void>;
@@ -476,12 +572,15 @@ function defaultStartSystemd(): ProxyRunResult {
}
/**
* Poll liveness up to a bounded startup deadline. A start command returning 0
* only means the job was ACCEPTED, not that the socket is bound — so we keep
* probing at `intervalMs` until either a probe succeeds or the deadline elapses.
* Poll for a TRUSTED-live proxy up to a bounded startup deadline. A start command
* returning 0 only means the job was ACCEPTED, not that the socket is bound — so
* we keep probing at `intervalMs` until either the deadline elapses or the port
* both responds AND passes the OS-level identity check. Liveness alone is not
* enough: a responder that fails identity (a squatter) must never be trusted.
*/
async function waitForLive(
async function waitForTrusted(
probe: () => Promise<boolean>,
verifyListener: () => ListenerVerdict,
waitMs: (ms: number) => Promise<void>,
intervalMs: number,
deadlineMs: number,
@@ -490,7 +589,7 @@ async function waitForLive(
while (elapsed < deadlineMs) {
await waitMs(intervalMs);
elapsed += intervalMs;
if (await probe()) {
if ((await probe()) && verifyListener() === 'ok') {
return true;
}
}
@@ -501,16 +600,29 @@ async function waitForLive(
* Ensure a proxy is listening. No-op when already live. Otherwise prefer the
* systemd user unit, then fall back to a detached background process.
*
* After a start command is accepted we poll liveness to a bounded startup
* deadline BEFORE considering the next method: `systemctl start` exit 0 means
* the job was accepted, not that the socket bound within one probe interval. If
* we fell back on the first miss we could launch a second proxy that then
* contends with the (slower) systemd one for :18765 — the very duplicate-proxy
* outcome this function exists to prevent. Liveness itself is the proxy-specific
* `/healthz` check (see {@link probeLiveness}).
* Every trust point is gated on OS-level listener identity, not just liveness:
* the proxy has no client authentication, so on a shared host a local process
* could squat :18765 and a 2xx `/healthz` alone would not prove it is our proxy
* (CWE-345, finding #2). We only trust a responder whose owning process is the
* current uid running the expected proxy binary; otherwise we fail closed.
*
* If a responder is already present but its identity does NOT verify, we return
* `untrusted` WITHOUT starting anything — the port is taken, so spawning would
* only create contention, and we must never route Claude traffic through an
* unverified listener.
*
* After a start command is accepted we poll to a bounded startup deadline before
* giving up: `systemctl start` exit 0 means the job was accepted, not that the
* socket bound within one probe interval. Critically, once systemd ACCEPTS the
* job we do NOT fall back to nohup even if it never becomes trusted-live in the
* deadline (finding #1): the accepted unit may bind late or be restarted by
* systemd, and a second proxy would then contend for :18765 — the very
* duplicate-proxy outcome this function exists to prevent. nohup is reachable
* only when systemd never accepted the job at all.
*/
export async function ensureProxyRunning(deps: EnsureProxyDeps = {}): Promise<EnsureProxyResult> {
const probe = deps.probe ?? (() => probeLiveness());
const verifyListener = deps.verifyListener ?? (() => verifyListenerIdentity());
const startSystemd = deps.startSystemd ?? defaultStartSystemd;
const startNohup = deps.startNohup ?? (() => startNohupProxy());
const waitMs = deps.waitMs ?? defaultWait;
@@ -518,19 +630,25 @@ export async function ensureProxyRunning(deps: EnsureProxyDeps = {}): Promise<En
const startupDeadlineMs = deps.startupDeadlineMs ?? 5000;
if (await probe()) {
return { live: true, method: 'already' };
// Something answers on :18765 — trust it ONLY if it is provably our proxy.
return verifyListener() === 'ok'
? { live: true, method: 'already' }
: { live: false, method: 'untrusted' };
}
const systemd = startSystemd();
if (systemd.status === 0) {
if (await waitForLive(probe, waitMs, settleMs, startupDeadlineMs)) {
// systemd accepted the job. Wait for a trusted-live bind, but never fall
// back to nohup afterward — that would risk a duplicate proxy (finding #1).
if (await waitForTrusted(probe, verifyListener, waitMs, settleMs, startupDeadlineMs)) {
return { live: true, method: 'systemd' };
}
return { live: false, method: 'failed' };
}
const nohup = await startNohup();
if (nohup.status === 0) {
if (await waitForLive(probe, waitMs, settleMs, startupDeadlineMs)) {
if (await waitForTrusted(probe, verifyListener, waitMs, settleMs, startupDeadlineMs)) {
return { live: true, method: 'nohup' };
}
}