fix(mosaic): harden claudex proxy preflight per review (P1 of #790)
All checks were successful
ci/woodpecker/pr/ci Pipeline was successful
All checks were successful
ci/woodpecker/pr/ci Pipeline was successful
Addresses the four findings from the independent review of #793 (exact head
0e41a5c2). TDD: each fix's failing test was added first (18 new tests), then
the implementation; full mosaic suite 1098 green, three gates green.
1. nohup fallback async spawn error (correctness): spawn() reports ENOENT/EACCES
asynchronously via the child 'error' event, which the old try/catch could not
see -> an unhandled 'error' would crash the launcher, and it returned status 0
before the child had started. Extract startNohupProxy(): attach the 'error'
listener BEFORE unref(), resolve non-zero on async error/sync throw, and
resolve success only after a confirmed 'spawn'. startNohup dep is now async.
2. systemd start not socket-bound (correctness): `systemctl --user start` exit 0
means the job was accepted, not bound within one probe. ensureProxyRunning now
polls liveness to a bounded startupDeadlineMs after a start before falling
back, so a slow systemd bind can't trigger a second, contending proxy.
3. liveness trust (security, CWE-345): probeLiveness hit the root and trusted ANY
HTTP response, so a local port-squatter could be taken for the proxy and MITM
Claude traffic. Now probe the proxy-specific GET /healthz and require a 2xx.
This also resolves gotcha #1 (root returns non-2xx): /healthz returns 2xx when
healthy, so a live proxy is never mistaken for dead. (Upstream offers no unix
socket or shared-secret handshake; /healthz is the in-scope identity ceiling.)
4. systemd ExecStart injection (security, CWE-74): buildSystemdUnitContent
interpolated binaryPath raw, so a CR/LF could inject unit directives. Validate
the path (absolute, reject control chars) and systemd-quote it when it carries
whitespace/quotes; installSystemdUnit now refuses to write a poisoned unit.
Coverage on claudex-proxy.ts: 96.3% stmts/lines, 87.1% branch.
Refs #790
This commit is contained in:
@@ -7,6 +7,8 @@ import {
|
|||||||
CLAUDEX_PROXY_PORT,
|
CLAUDEX_PROXY_PORT,
|
||||||
CLAUDEX_PROXY_URL,
|
CLAUDEX_PROXY_URL,
|
||||||
CLAUDEX_PROXY_BINARY,
|
CLAUDEX_PROXY_BINARY,
|
||||||
|
CLAUDEX_HEALTH_PATH,
|
||||||
|
CLAUDEX_HEALTH_URL,
|
||||||
buildAuthStatusArgs,
|
buildAuthStatusArgs,
|
||||||
buildDeviceAuthArgs,
|
buildDeviceAuthArgs,
|
||||||
buildServeArgs,
|
buildServeArgs,
|
||||||
@@ -18,21 +20,29 @@ import {
|
|||||||
buildSystemdUnitContent,
|
buildSystemdUnitContent,
|
||||||
systemdUnitPath,
|
systemdUnitPath,
|
||||||
installSystemdUnit,
|
installSystemdUnit,
|
||||||
|
startNohupProxy,
|
||||||
runProxyPreflight,
|
runProxyPreflight,
|
||||||
ensureProxyRunning,
|
ensureProxyRunning,
|
||||||
type AuthStatus,
|
type AuthStatus,
|
||||||
type ProxyRunResult,
|
type ProxyRunResult,
|
||||||
|
type SpawnedChild,
|
||||||
} from './claudex-proxy.js';
|
} from './claudex-proxy.js';
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* P1 — Proxy preflight + lifecycle helpers for `mosaic yolo claudex`.
|
* P1 — Proxy preflight + lifecycle helpers for `mosaic yolo claudex`.
|
||||||
*
|
*
|
||||||
* Security-relevant invariants exercised here:
|
* Security-relevant invariants exercised here:
|
||||||
* - Liveness probe treats ANY HTTP response (even non-2xx) as "alive" and only
|
* - Liveness probe hits the proxy's dedicated `GET /healthz` and treats only a
|
||||||
* a transport/connection failure as "dead" (spec gotcha #1: never `curl -f`;
|
* 2xx as "alive" — a *proxy-specific* health contract, not arbitrary HTTP on
|
||||||
* the original wrapper spawned duplicate proxies because of this).
|
* the port (CWE-345: a local port-squatter must not be trusted as the proxy).
|
||||||
|
* This also honors spec gotcha #1 (never `curl -f` the root, which returns
|
||||||
|
* non-2xx): `/healthz` returns 2xx when the proxy is up, so a healthy proxy is
|
||||||
|
* never mistaken for dead and no duplicate proxy is spawned.
|
||||||
* - Auth-status parsing NEVER surfaces OAuth token material — only a coarse
|
* - Auth-status parsing NEVER surfaces OAuth token material — only a coarse
|
||||||
* state + optional expiry — even if a token-shaped string appears in output.
|
* state + optional expiry — even if a token-shaped string appears in output.
|
||||||
|
* - The systemd unit's ExecStart never interpolates an unvalidated path
|
||||||
|
* (CWE-74: a CR/LF in the path could inject arbitrary systemd directives).
|
||||||
|
* - The nohup fallback captures spawn's *async* error event instead of crashing.
|
||||||
*/
|
*/
|
||||||
|
|
||||||
describe('claudex-proxy constants', () => {
|
describe('claudex-proxy constants', () => {
|
||||||
@@ -43,6 +53,11 @@ describe('claudex-proxy constants', () => {
|
|||||||
expect(CLAUDEX_PROXY_BINARY).toBe('claude-code-proxy');
|
expect(CLAUDEX_PROXY_BINARY).toBe('claude-code-proxy');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('exposes the dedicated /healthz liveness endpoint (not the root path)', () => {
|
||||||
|
expect(CLAUDEX_HEALTH_PATH).toBe('/healthz');
|
||||||
|
expect(CLAUDEX_HEALTH_URL).toBe('http://127.0.0.1:18765/healthz');
|
||||||
|
});
|
||||||
|
|
||||||
it('builds the documented codex subcommand argv', () => {
|
it('builds the documented codex subcommand argv', () => {
|
||||||
expect(buildAuthStatusArgs()).toEqual(['codex', 'auth', 'status']);
|
expect(buildAuthStatusArgs()).toEqual(['codex', 'auth', 'status']);
|
||||||
expect(buildDeviceAuthArgs()).toEqual(['codex', 'auth', 'device']);
|
expect(buildDeviceAuthArgs()).toEqual(['codex', 'auth', 'device']);
|
||||||
@@ -172,24 +187,46 @@ describe('checkProxyBinary', () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
describe('probeLiveness (gotcha #1 — any HTTP response is alive)', () => {
|
describe('probeLiveness (proxy-specific /healthz, not arbitrary HTTP)', () => {
|
||||||
it('treats 200 as alive', async () => {
|
it('defaults to probing the /healthz endpoint, never the root path', async () => {
|
||||||
const live = await probeLiveness(CLAUDEX_PROXY_URL, async () => ({ status: 200 }));
|
const seen: string[] = [];
|
||||||
|
await probeLiveness(undefined, async (u) => {
|
||||||
|
seen.push(u);
|
||||||
|
return { status: 200 };
|
||||||
|
});
|
||||||
|
expect(seen[0]).toBe(CLAUDEX_HEALTH_URL);
|
||||||
|
expect(seen[0]).toContain('/healthz');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('treats a 200 on /healthz as alive', async () => {
|
||||||
|
const live = await probeLiveness(CLAUDEX_HEALTH_URL, async () => ({ status: 200 }));
|
||||||
expect(live).toBe(true);
|
expect(live).toBe(true);
|
||||||
});
|
});
|
||||||
|
|
||||||
it('treats a non-2xx (404) response as alive — must NOT behave like curl -f', async () => {
|
it('treats a 204 on /healthz as alive', async () => {
|
||||||
const live = await probeLiveness(CLAUDEX_PROXY_URL, async () => ({ status: 404 }));
|
const live = await probeLiveness(CLAUDEX_HEALTH_URL, async () => ({ status: 204 }));
|
||||||
expect(live).toBe(true);
|
expect(live).toBe(true);
|
||||||
});
|
});
|
||||||
|
|
||||||
it('treats a 500 response as alive', async () => {
|
it('treats a 404 as DEAD — does not trust an arbitrary responder on the port (CWE-345)', async () => {
|
||||||
const live = await probeLiveness(CLAUDEX_PROXY_URL, async () => ({ status: 500 }));
|
// The whole point: a random local process squatting :18765 will not honor the
|
||||||
expect(live).toBe(true);
|
// proxy's /healthz contract, so a non-2xx there must not be mistaken for the proxy.
|
||||||
|
const live = await probeLiveness(CLAUDEX_HEALTH_URL, async () => ({ status: 404 }));
|
||||||
|
expect(live).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('treats a 500 as DEAD (unhealthy / not the proxy health contract)', async () => {
|
||||||
|
const live = await probeLiveness(CLAUDEX_HEALTH_URL, async () => ({ status: 500 }));
|
||||||
|
expect(live).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('treats a missing status as dead', async () => {
|
||||||
|
const live = await probeLiveness(CLAUDEX_HEALTH_URL, async () => ({}));
|
||||||
|
expect(live).toBe(false);
|
||||||
});
|
});
|
||||||
|
|
||||||
it('treats a connection failure (reject) as dead', async () => {
|
it('treats a connection failure (reject) as dead', async () => {
|
||||||
const live = await probeLiveness(CLAUDEX_PROXY_URL, async () => {
|
const live = await probeLiveness(CLAUDEX_HEALTH_URL, async () => {
|
||||||
throw new Error('ECONNREFUSED');
|
throw new Error('ECONNREFUSED');
|
||||||
});
|
});
|
||||||
expect(live).toBe(false);
|
expect(live).toBe(false);
|
||||||
@@ -197,7 +234,7 @@ describe('probeLiveness (gotcha #1 — any HTTP response is alive)', () => {
|
|||||||
|
|
||||||
it('treats a timeout as dead', async () => {
|
it('treats a timeout as dead', async () => {
|
||||||
const never = () => new Promise<{ status?: number }>(() => {});
|
const never = () => new Promise<{ status?: number }>(() => {});
|
||||||
const live = await probeLiveness(CLAUDEX_PROXY_URL, never, 20);
|
const live = await probeLiveness(CLAUDEX_HEALTH_URL, never, 20);
|
||||||
expect(live).toBe(false);
|
expect(live).toBe(false);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
@@ -217,6 +254,43 @@ describe('buildSystemdUnitContent', () => {
|
|||||||
expect(unit).not.toMatch(/token/i);
|
expect(unit).not.toMatch(/token/i);
|
||||||
expect(unit).not.toMatch(/auth\.json/i);
|
expect(unit).not.toMatch(/auth\.json/i);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('rejects a path containing a newline (CWE-74 systemd directive injection)', () => {
|
||||||
|
// A raw newline in ExecStart would let an attacker append arbitrary unit
|
||||||
|
// directives — e.g. `ExecStartPost=curl evil`. Must be rejected outright.
|
||||||
|
expect(() =>
|
||||||
|
buildSystemdUnitContent('/bin/claude-code-proxy\nExecStartPost=/bin/rm -rf /'),
|
||||||
|
).toThrow();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('rejects a path containing a carriage return', () => {
|
||||||
|
expect(() => buildSystemdUnitContent('/bin/claude-code-proxy\rmalicious')).toThrow();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('rejects a path with other control characters', () => {
|
||||||
|
expect(() => buildSystemdUnitContent('/bin/claude-code-proxy\x00nul')).toThrow();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('rejects a non-absolute path', () => {
|
||||||
|
expect(() => buildSystemdUnitContent('claude-code-proxy')).toThrow();
|
||||||
|
expect(() => buildSystemdUnitContent('')).toThrow();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('systemd-quotes a path that contains spaces', () => {
|
||||||
|
const unit = buildSystemdUnitContent('/home/u/my apps/claude-code-proxy');
|
||||||
|
expect(unit).toContain('ExecStart="/home/u/my apps/claude-code-proxy" serve --no-monitor');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('escapes embedded quotes and backslashes when quoting', () => {
|
||||||
|
const unit = buildSystemdUnitContent('/home/u/we"ird\\dir/claude-code-proxy');
|
||||||
|
// No unescaped closing quote can terminate the token early.
|
||||||
|
expect(unit).toContain('ExecStart="/home/u/we\\"ird\\\\dir/claude-code-proxy" serve');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('leaves a clean absolute path unquoted (no needless churn)', () => {
|
||||||
|
const unit = buildSystemdUnitContent('/home/u/.local/bin/claude-code-proxy');
|
||||||
|
expect(unit).toContain('ExecStart=/home/u/.local/bin/claude-code-proxy serve --no-monitor');
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
describe('systemdUnitPath', () => {
|
describe('systemdUnitPath', () => {
|
||||||
@@ -263,6 +337,18 @@ describe('installSystemdUnit', () => {
|
|||||||
expect(ok).toBe(false);
|
expect(ok).toBe(false);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('refuses to write a unit for an injection-bearing path (never writes a poisoned unit)', () => {
|
||||||
|
const writeUnit = vi.fn();
|
||||||
|
const ok = installSystemdUnit('/bin/claude-code-proxy\nExecStartPost=/bin/rm -rf /', {
|
||||||
|
home: '/home/u',
|
||||||
|
writeUnit,
|
||||||
|
run: () => ({ status: 0, stdout: '', stderr: '' }),
|
||||||
|
});
|
||||||
|
expect(ok).toBe(false);
|
||||||
|
// The poisoned unit content is never even produced, so nothing is written.
|
||||||
|
expect(writeUnit).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
it('writes to a real temp dir via the default writer', () => {
|
it('writes to a real temp dir via the default writer', () => {
|
||||||
const home = mkdtempSync(join(tmpdir(), 'claudex-unit-'));
|
const home = mkdtempSync(join(tmpdir(), 'claudex-unit-'));
|
||||||
try {
|
try {
|
||||||
@@ -353,12 +439,95 @@ describe('runProxyPreflight', () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
/**
|
||||||
|
* A minimal fake ChildProcess for the nohup-fallback tests: records once()
|
||||||
|
* handlers so a test can drive the async 'spawn'/'error' events, and tracks
|
||||||
|
* whether the 'error' listener was already attached at the moment unref() ran
|
||||||
|
* (the security-critical ordering from finding #1).
|
||||||
|
*/
|
||||||
|
function fakeChild() {
|
||||||
|
const handlers: Record<string, (arg?: unknown) => void> = {};
|
||||||
|
const state = { unreffed: false, errorHandlerAtUnref: false };
|
||||||
|
const child = {
|
||||||
|
once(event: string, listener: (arg?: unknown) => void) {
|
||||||
|
handlers[event] = listener;
|
||||||
|
return child;
|
||||||
|
},
|
||||||
|
unref() {
|
||||||
|
state.unreffed = true;
|
||||||
|
state.errorHandlerAtUnref = typeof handlers.error === 'function';
|
||||||
|
},
|
||||||
|
emit(event: string, arg?: unknown) {
|
||||||
|
handlers[event]?.(arg);
|
||||||
|
},
|
||||||
|
};
|
||||||
|
return {
|
||||||
|
child: child as unknown as SpawnedChild & { emit(e: string, a?: unknown): void },
|
||||||
|
state,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
describe('startNohupProxy (finding #1 — async spawn error must not crash)', () => {
|
||||||
|
it('resolves status 0 only after a confirmed spawn, and unrefs the child', async () => {
|
||||||
|
const { child, state } = fakeChild();
|
||||||
|
const spawnImpl = vi.fn((_cmd: string, _args: string[]) => {
|
||||||
|
queueMicrotask(() => child.emit('spawn'));
|
||||||
|
return child;
|
||||||
|
});
|
||||||
|
const r = await startNohupProxy({ resolveBin: () => '/bin/claude-code-proxy', spawnImpl });
|
||||||
|
expect(r.status).toBe(0);
|
||||||
|
expect(state.unreffed).toBe(true);
|
||||||
|
// The error listener MUST be registered before unref(), so an ENOENT that
|
||||||
|
// arrives asynchronously can never become an unhandled 'error' crash.
|
||||||
|
expect(state.errorHandlerAtUnref).toBe(true);
|
||||||
|
expect(spawnImpl).toHaveBeenCalledWith('/bin/claude-code-proxy', ['serve', '--no-monitor'], {
|
||||||
|
detached: true,
|
||||||
|
stdio: 'ignore',
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it('captures an async spawn error (ENOENT) as a failed start instead of crashing', async () => {
|
||||||
|
const { child, state } = fakeChild();
|
||||||
|
const spawnImpl = () => {
|
||||||
|
queueMicrotask(() => child.emit('error', new Error('spawn claude-code-proxy ENOENT')));
|
||||||
|
return child;
|
||||||
|
};
|
||||||
|
const r = await startNohupProxy({ resolveBin: () => '/bin/claude-code-proxy', spawnImpl });
|
||||||
|
expect(r.status).toBe(1);
|
||||||
|
expect(r.stderr).toContain('ENOENT');
|
||||||
|
expect(state.unreffed).toBe(false); // never unref a child that failed to start
|
||||||
|
});
|
||||||
|
|
||||||
|
it('captures a synchronous spawn throw as a failed start', async () => {
|
||||||
|
const spawnImpl = () => {
|
||||||
|
throw new Error('EACCES');
|
||||||
|
};
|
||||||
|
const r = await startNohupProxy({ resolveBin: () => '/bin/claude-code-proxy', spawnImpl });
|
||||||
|
expect(r.status).toBe(1);
|
||||||
|
expect(r.stderr).toContain('EACCES');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('ignores a late error after a successful spawn (settles once)', async () => {
|
||||||
|
const { child } = fakeChild();
|
||||||
|
const spawnImpl = () => {
|
||||||
|
queueMicrotask(() => {
|
||||||
|
child.emit('spawn');
|
||||||
|
child.emit('error', new Error('late boom'));
|
||||||
|
});
|
||||||
|
return child;
|
||||||
|
};
|
||||||
|
const r = await startNohupProxy({ resolveBin: () => '/bin/claude-code-proxy', spawnImpl });
|
||||||
|
expect(r.status).toBe(0); // first settle wins; the late error cannot flip it
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
describe('ensureProxyRunning', () => {
|
describe('ensureProxyRunning', () => {
|
||||||
const ok: ProxyRunResult = { status: 0, stdout: '', stderr: '' };
|
const ok: ProxyRunResult = { status: 0, stdout: '', stderr: '' };
|
||||||
|
const nohupOk = async (): Promise<ProxyRunResult> => ok;
|
||||||
|
|
||||||
it('is a no-op when the proxy is already live', async () => {
|
it('is a no-op when the proxy is already live', async () => {
|
||||||
const startSystemd = vi.fn(() => ok);
|
const startSystemd = vi.fn(() => ok);
|
||||||
const startNohup = vi.fn(() => ok);
|
const startNohup = vi.fn(nohupOk);
|
||||||
const r = await ensureProxyRunning({
|
const r = await ensureProxyRunning({
|
||||||
probe: async () => true,
|
probe: async () => true,
|
||||||
startSystemd,
|
startSystemd,
|
||||||
@@ -376,7 +545,7 @@ describe('ensureProxyRunning', () => {
|
|||||||
const r = await ensureProxyRunning({
|
const r = await ensureProxyRunning({
|
||||||
probe: async () => calls++ > 0, // dead first, live after start
|
probe: async () => calls++ > 0, // dead first, live after start
|
||||||
startSystemd: () => ok,
|
startSystemd: () => ok,
|
||||||
startNohup: () => {
|
startNohup: async () => {
|
||||||
throw new Error('should not fall back');
|
throw new Error('should not fall back');
|
||||||
},
|
},
|
||||||
waitMs: async () => {},
|
waitMs: async () => {},
|
||||||
@@ -385,15 +554,52 @@ describe('ensureProxyRunning', () => {
|
|||||||
expect(r.live).toBe(true);
|
expect(r.live).toBe(true);
|
||||||
});
|
});
|
||||||
|
|
||||||
it('falls back to nohup when systemd start fails', async () => {
|
it('waits past a slow systemd bind before falling back (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.
|
||||||
|
let probes = 0;
|
||||||
|
const startNohup = vi.fn(nohupOk);
|
||||||
|
const r = await ensureProxyRunning({
|
||||||
|
probe: async () => probes++ >= 3,
|
||||||
|
startSystemd: () => ok,
|
||||||
|
startNohup,
|
||||||
|
waitMs: async () => {},
|
||||||
|
settleMs: 10,
|
||||||
|
startupDeadlineMs: 200,
|
||||||
|
});
|
||||||
|
expect(r.method).toBe('systemd');
|
||||||
|
expect(r.live).toBe(true);
|
||||||
|
expect(startNohup).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('falls back to nohup when systemd is accepted but never binds in the deadline', async () => {
|
||||||
|
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,
|
||||||
|
startSystemd: () => ok,
|
||||||
|
startNohup,
|
||||||
|
waitMs: async () => {},
|
||||||
|
settleMs: 10,
|
||||||
|
startupDeadlineMs: 30,
|
||||||
|
});
|
||||||
|
expect(startNohup).toHaveBeenCalledTimes(1);
|
||||||
|
expect(r.method).toBe('nohup');
|
||||||
|
expect(r.live).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('falls back to nohup when systemd start fails outright', async () => {
|
||||||
let calls = 0;
|
let calls = 0;
|
||||||
const r = await ensureProxyRunning({
|
const r = await ensureProxyRunning({
|
||||||
// A failed systemd start skips its post-start probe, so probes are:
|
// 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).
|
||||||
probe: async () => calls++ > 0,
|
probe: async () => calls++ > 0,
|
||||||
startSystemd: () => ({ status: 1, stdout: '', stderr: 'no systemd' }),
|
startSystemd: () => ({ status: 1, stdout: '', stderr: 'no systemd' }),
|
||||||
startNohup: () => ok,
|
startNohup: nohupOk,
|
||||||
waitMs: async () => {},
|
waitMs: async () => {},
|
||||||
|
settleMs: 10,
|
||||||
|
startupDeadlineMs: 30,
|
||||||
});
|
});
|
||||||
expect(r.method).toBe('nohup');
|
expect(r.method).toBe('nohup');
|
||||||
expect(r.live).toBe(true);
|
expect(r.live).toBe(true);
|
||||||
@@ -403,8 +609,10 @@ describe('ensureProxyRunning', () => {
|
|||||||
const r = await ensureProxyRunning({
|
const r = await ensureProxyRunning({
|
||||||
probe: async () => false,
|
probe: async () => false,
|
||||||
startSystemd: () => ({ status: 1, stdout: '', stderr: '' }),
|
startSystemd: () => ({ status: 1, stdout: '', stderr: '' }),
|
||||||
startNohup: () => ({ status: 1, stdout: '', stderr: '' }),
|
startNohup: async () => ({ status: 1, stdout: '', stderr: '' }),
|
||||||
waitMs: async () => {},
|
waitMs: async () => {},
|
||||||
|
settleMs: 10,
|
||||||
|
startupDeadlineMs: 30,
|
||||||
});
|
});
|
||||||
expect(r.method).toBe('failed');
|
expect(r.method).toBe('failed');
|
||||||
expect(r.live).toBe(false);
|
expect(r.live).toBe(false);
|
||||||
|
|||||||
@@ -29,6 +29,17 @@ export const CLAUDEX_PROXY_URL = `http://${CLAUDEX_PROXY_HOST}:${CLAUDEX_PROXY_P
|
|||||||
export const CLAUDEX_PROXY_BINARY = 'claude-code-proxy';
|
export const CLAUDEX_PROXY_BINARY = 'claude-code-proxy';
|
||||||
export const CLAUDEX_SYSTEMD_UNIT = 'claude-code-proxy.service';
|
export const CLAUDEX_SYSTEMD_UNIT = 'claude-code-proxy.service';
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The proxy's dedicated liveness endpoint. We probe this — NOT the root path —
|
||||||
|
* for two reasons: (1) the root returns non-2xx (spec gotcha #1), which is why
|
||||||
|
* the original `curl -f` check spawned duplicate proxies; `/healthz` returns 2xx
|
||||||
|
* when the proxy is healthy. (2) It is a *proxy-specific* contract, so a 2xx here
|
||||||
|
* is a much stronger signal that the responder on :18765 is actually our proxy
|
||||||
|
* and not some other local process squatting the port (CWE-345).
|
||||||
|
*/
|
||||||
|
export const CLAUDEX_HEALTH_PATH = '/healthz';
|
||||||
|
export const CLAUDEX_HEALTH_URL = `${CLAUDEX_PROXY_URL}${CLAUDEX_HEALTH_PATH}`;
|
||||||
|
|
||||||
/** argv for `claude-code-proxy codex auth status`. */
|
/** argv for `claude-code-proxy codex auth status`. */
|
||||||
export function buildAuthStatusArgs(): string[] {
|
export function buildAuthStatusArgs(): string[] {
|
||||||
return ['codex', 'auth', 'status'];
|
return ['codex', 'auth', 'status'];
|
||||||
@@ -164,17 +175,25 @@ export function runDeviceReauth(spawnImpl: InheritSpawn = defaultInheritSpawn):
|
|||||||
return r.status ?? 1;
|
return r.status ?? 1;
|
||||||
}
|
}
|
||||||
|
|
||||||
// ─── Liveness (gotcha #1: any HTTP response is alive, NEVER `curl -f`) ────────
|
// ─── Liveness (probe the proxy-specific /healthz; require 2xx) ────────────────
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Probe the proxy for liveness. ANY resolved HTTP response — including non-2xx —
|
* Probe the proxy for liveness by hitting its dedicated `GET /healthz` endpoint
|
||||||
* means the proxy is up; only a transport/connection failure or a timeout means
|
* and requiring a 2xx response.
|
||||||
* it is down. This is the whole point of gotcha #1: the proxy's root path
|
*
|
||||||
* returns non-2xx, so a `curl -f`-style check reports it dead and spawns a
|
* This deliberately does NOT trust an arbitrary HTTP response on the port. The
|
||||||
* duplicate proxy.
|
* 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.
|
||||||
*/
|
*/
|
||||||
export async function probeLiveness(
|
export async function probeLiveness(
|
||||||
url: string = CLAUDEX_PROXY_URL,
|
url: string = CLAUDEX_HEALTH_URL,
|
||||||
fetchImpl: FetchLike = fetch as unknown as FetchLike,
|
fetchImpl: FetchLike = fetch as unknown as FetchLike,
|
||||||
timeoutMs = 1500,
|
timeoutMs = 1500,
|
||||||
): Promise<boolean> {
|
): Promise<boolean> {
|
||||||
@@ -193,7 +212,7 @@ export async function probeLiveness(
|
|||||||
});
|
});
|
||||||
|
|
||||||
const probe = fetchImpl(url, { signal: controller.signal })
|
const probe = fetchImpl(url, { signal: controller.signal })
|
||||||
.then(() => true) // any response at all → alive
|
.then((res) => typeof res.status === 'number' && res.status >= 200 && res.status < 300)
|
||||||
.catch(() => false); // connection refused / aborted → dead
|
.catch(() => false); // connection refused / aborted → dead
|
||||||
|
|
||||||
try {
|
try {
|
||||||
@@ -209,12 +228,50 @@ export function systemdUnitPath(home: string = homedir()): string {
|
|||||||
return join(home, '.config', 'systemd', 'user', CLAUDEX_SYSTEMD_UNIT);
|
return join(home, '.config', 'systemd', 'user', CLAUDEX_SYSTEMD_UNIT);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Validate a path destined for a systemd `ExecStart=` line. A raw newline (or
|
||||||
|
* other control character) in the path would let an attacker inject arbitrary
|
||||||
|
* unit directives (e.g. an extra `ExecStartPost=`), a CWE-74 command injection.
|
||||||
|
* We require a plain absolute path and reject any control character outright.
|
||||||
|
*/
|
||||||
|
function validateExecPath(binaryPath: string): string {
|
||||||
|
if (typeof binaryPath !== 'string' || binaryPath.length === 0) {
|
||||||
|
throw new Error('systemd ExecStart: binary path is empty');
|
||||||
|
}
|
||||||
|
if (!binaryPath.startsWith('/')) {
|
||||||
|
throw new Error(
|
||||||
|
`systemd ExecStart: binary path must be absolute: ${JSON.stringify(binaryPath)}`,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
if (/[\x00-\x1f\x7f]/.test(binaryPath)) {
|
||||||
|
throw new Error('systemd ExecStart: binary path contains control characters');
|
||||||
|
}
|
||||||
|
return binaryPath;
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Encode a validated path for a systemd `ExecStart=` token. systemd only needs
|
||||||
|
* quoting when the token carries whitespace or quote/backslash characters; a
|
||||||
|
* clean path is emitted verbatim. When quoting, we escape backslashes and double
|
||||||
|
* quotes per systemd's C-style rules so the token cannot be terminated early.
|
||||||
|
*/
|
||||||
|
function systemdQuoteExec(path: string): string {
|
||||||
|
if (!/[\s"'\\]/.test(path)) {
|
||||||
|
return path;
|
||||||
|
}
|
||||||
|
const escaped = path.replace(/\\/g, '\\\\').replace(/"/g, '\\"');
|
||||||
|
return `"${escaped}"`;
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Render the `claude-code-proxy.service` user unit. Contains no credential
|
* Render the `claude-code-proxy.service` user unit. Contains no credential
|
||||||
* material — the proxy reads its own OAuth token from its config dir at runtime.
|
* material — the proxy reads its own OAuth token from its config dir at runtime.
|
||||||
|
* The binary path is validated (absolute, no control characters) and systemd-
|
||||||
|
* quoted so it cannot inject unit directives.
|
||||||
*/
|
*/
|
||||||
export function buildSystemdUnitContent(binaryPath: string): string {
|
export function buildSystemdUnitContent(binaryPath: string): string {
|
||||||
const exec = `${binaryPath} ${buildServeArgs().join(' ')}`;
|
const exec = `${systemdQuoteExec(validateExecPath(binaryPath))} ${buildServeArgs().join(' ')}`;
|
||||||
return [
|
return [
|
||||||
'[Unit]',
|
'[Unit]',
|
||||||
'Description=claude-code-proxy (Anthropic->Codex translation proxy for mosaic claudex)',
|
'Description=claude-code-proxy (Anthropic->Codex translation proxy for mosaic claudex)',
|
||||||
@@ -333,12 +390,81 @@ export interface EnsureProxyResult {
|
|||||||
method: ProxyStartMethod;
|
method: ProxyStartMethod;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/** Minimal spawned-child shape used by the nohup fallback (testable seam). */
|
||||||
|
export interface SpawnedChild {
|
||||||
|
once(event: string, listener: (arg?: unknown) => void): unknown;
|
||||||
|
unref(): void;
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Spawn shape for the detached fallback process. */
|
||||||
|
export type SpawnLike = (
|
||||||
|
cmd: string,
|
||||||
|
args: string[],
|
||||||
|
opts: { detached: boolean; stdio: 'ignore' },
|
||||||
|
) => SpawnedChild;
|
||||||
|
|
||||||
|
export interface StartNohupDeps {
|
||||||
|
resolveBin?: () => string;
|
||||||
|
spawnImpl?: SpawnLike;
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Start the proxy as a detached background process (the fallback when no systemd
|
||||||
|
* user unit is available).
|
||||||
|
*
|
||||||
|
* `spawn()` reports launch failures (ENOENT/EACCES) ASYNCHRONOUSLY via the
|
||||||
|
* child's `error` event, which a `try/catch` cannot see. If left unhandled that
|
||||||
|
* event throws and crashes the launcher. So we: (1) attach the `error` listener
|
||||||
|
* BEFORE `unref()`, capturing a failed launch as a non-zero result instead of a
|
||||||
|
* crash; and (2) resolve success only after the child's `spawn` event fires —
|
||||||
|
* never optimistically before the process is known to have started.
|
||||||
|
*/
|
||||||
|
export function startNohupProxy(deps: StartNohupDeps = {}): Promise<ProxyRunResult> {
|
||||||
|
const resolveBin = deps.resolveBin ?? (() => checkProxyBinary().path ?? CLAUDEX_PROXY_BINARY);
|
||||||
|
const spawnImpl =
|
||||||
|
deps.spawnImpl ?? ((cmd, args, opts) => spawn(cmd, args, opts) as unknown as SpawnedChild);
|
||||||
|
|
||||||
|
return new Promise<ProxyRunResult>((resolve) => {
|
||||||
|
let settled = false;
|
||||||
|
const finish = (r: ProxyRunResult) => {
|
||||||
|
if (!settled) {
|
||||||
|
settled = true;
|
||||||
|
resolve(r);
|
||||||
|
}
|
||||||
|
};
|
||||||
|
|
||||||
|
let child: SpawnedChild;
|
||||||
|
try {
|
||||||
|
child = spawnImpl(resolveBin(), buildServeArgs(), { detached: true, stdio: 'ignore' });
|
||||||
|
} catch (err) {
|
||||||
|
finish({ status: 1, stdout: '', stderr: err instanceof Error ? err.message : String(err) });
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Register error handling BEFORE unref so an async spawn failure is caught.
|
||||||
|
child.once('error', (err) => {
|
||||||
|
finish({
|
||||||
|
status: 1,
|
||||||
|
stdout: '',
|
||||||
|
stderr: err instanceof Error ? err.message : String(err),
|
||||||
|
});
|
||||||
|
});
|
||||||
|
child.once('spawn', () => {
|
||||||
|
child.unref();
|
||||||
|
finish({ status: 0, stdout: '', stderr: '' });
|
||||||
|
});
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
export interface EnsureProxyDeps {
|
export interface EnsureProxyDeps {
|
||||||
probe?: () => Promise<boolean>;
|
probe?: () => Promise<boolean>;
|
||||||
startSystemd?: () => ProxyRunResult;
|
startSystemd?: () => ProxyRunResult;
|
||||||
startNohup?: () => ProxyRunResult;
|
startNohup?: () => Promise<ProxyRunResult>;
|
||||||
waitMs?: (ms: number) => Promise<void>;
|
waitMs?: (ms: number) => Promise<void>;
|
||||||
|
/** Interval between liveness polls while waiting for a start to bind. */
|
||||||
settleMs?: number;
|
settleMs?: number;
|
||||||
|
/** Total budget to wait for a started proxy to bind its socket. */
|
||||||
|
startupDeadlineMs?: number;
|
||||||
}
|
}
|
||||||
|
|
||||||
function defaultWait(ms: number): Promise<void> {
|
function defaultWait(ms: number): Promise<void> {
|
||||||
@@ -349,32 +475,47 @@ function defaultStartSystemd(): ProxyRunResult {
|
|||||||
return defaultRun('systemctl', ['--user', 'start', CLAUDEX_SYSTEMD_UNIT]);
|
return defaultRun('systemctl', ['--user', 'start', CLAUDEX_SYSTEMD_UNIT]);
|
||||||
}
|
}
|
||||||
|
|
||||||
function defaultStartNohup(): ProxyRunResult {
|
/**
|
||||||
try {
|
* Poll liveness up to a bounded startup deadline. A start command returning 0
|
||||||
const bin = checkProxyBinary().path ?? CLAUDEX_PROXY_BINARY;
|
* only means the job was ACCEPTED, not that the socket is bound — so we keep
|
||||||
const child = spawn(bin, buildServeArgs(), {
|
* probing at `intervalMs` until either a probe succeeds or the deadline elapses.
|
||||||
detached: true,
|
*/
|
||||||
stdio: 'ignore',
|
async function waitForLive(
|
||||||
});
|
probe: () => Promise<boolean>,
|
||||||
child.unref();
|
waitMs: (ms: number) => Promise<void>,
|
||||||
return { status: 0, stdout: '', stderr: '' };
|
intervalMs: number,
|
||||||
} catch (err) {
|
deadlineMs: number,
|
||||||
return { status: 1, stdout: '', stderr: err instanceof Error ? err.message : String(err) };
|
): Promise<boolean> {
|
||||||
|
let elapsed = 0;
|
||||||
|
while (elapsed < deadlineMs) {
|
||||||
|
await waitMs(intervalMs);
|
||||||
|
elapsed += intervalMs;
|
||||||
|
if (await probe()) {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Ensure a proxy is listening. No-op when already live. Otherwise prefer the
|
* Ensure a proxy is listening. No-op when already live. Otherwise prefer the
|
||||||
* systemd user unit, then fall back to a detached background process. Re-probes
|
* systemd user unit, then fall back to a detached background process.
|
||||||
* after each start attempt (respecting gotcha #1 via {@link probeLiveness}), so
|
*
|
||||||
* it never spawns a duplicate proxy.
|
* 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}).
|
||||||
*/
|
*/
|
||||||
export async function ensureProxyRunning(deps: EnsureProxyDeps = {}): Promise<EnsureProxyResult> {
|
export async function ensureProxyRunning(deps: EnsureProxyDeps = {}): Promise<EnsureProxyResult> {
|
||||||
const probe = deps.probe ?? (() => probeLiveness());
|
const probe = deps.probe ?? (() => probeLiveness());
|
||||||
const startSystemd = deps.startSystemd ?? defaultStartSystemd;
|
const startSystemd = deps.startSystemd ?? defaultStartSystemd;
|
||||||
const startNohup = deps.startNohup ?? defaultStartNohup;
|
const startNohup = deps.startNohup ?? (() => startNohupProxy());
|
||||||
const waitMs = deps.waitMs ?? defaultWait;
|
const waitMs = deps.waitMs ?? defaultWait;
|
||||||
const settleMs = deps.settleMs ?? 500;
|
const settleMs = deps.settleMs ?? 500;
|
||||||
|
const startupDeadlineMs = deps.startupDeadlineMs ?? 5000;
|
||||||
|
|
||||||
if (await probe()) {
|
if (await probe()) {
|
||||||
return { live: true, method: 'already' };
|
return { live: true, method: 'already' };
|
||||||
@@ -382,16 +523,14 @@ export async function ensureProxyRunning(deps: EnsureProxyDeps = {}): Promise<En
|
|||||||
|
|
||||||
const systemd = startSystemd();
|
const systemd = startSystemd();
|
||||||
if (systemd.status === 0) {
|
if (systemd.status === 0) {
|
||||||
await waitMs(settleMs);
|
if (await waitForLive(probe, waitMs, settleMs, startupDeadlineMs)) {
|
||||||
if (await probe()) {
|
|
||||||
return { live: true, method: 'systemd' };
|
return { live: true, method: 'systemd' };
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
const nohup = startNohup();
|
const nohup = await startNohup();
|
||||||
if (nohup.status === 0) {
|
if (nohup.status === 0) {
|
||||||
await waitMs(settleMs);
|
if (await waitForLive(probe, waitMs, settleMs, startupDeadlineMs)) {
|
||||||
if (await probe()) {
|
|
||||||
return { live: true, method: 'nohup' };
|
return { live: true, method: 'nohup' };
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user