mirror of
https://github.com/dtzp555-max/ocp.git
synced 2026-07-19 09:44:07 +00:00
Two TUI-mode findings from the 2026-05-31 audit (ADR 0007):
1. The fail-loud LAN gate refused TUI boot only when CLAUDE_BIND === "0.0.0.0",
but binding to a concrete LAN IP, a Tailscale 100.x address, or IPv6 :: is
equally network-exposed and slipped through — letting any reachable peer drive
the operator's full-filesystem claude session. New isLoopbackBind() treats only
127.0.0.0/8 / ::1 / localhost / ::ffff:127.0.0.1 as safe; the gate now trips on
any non-loopback bind (OCP_TUI_ALLOW_LAN=1 escape hatch retained).
2. verifyEntrypoint() was implemented and unit-tested but never wired: readTuiTranscript
discarded the events and returned only text, so a silent degrade to the metered
sdk-cli (Agent SDK) billing pool — which still returns text but costs money — went
undetected. readTuiTranscript now returns { text, entrypoint }; runTuiTurn passes it
through; callClaudeTui logs a "tui_entrypoint_mismatch" warning when configured cli
mode yields a non-cli entrypoint (including null/unverified). callClaudeTui still
returns Promise<string>, so singleflight/cache/completion downstream is unchanged.
ALIGNMENT.md: TUI is a proxy-side execution-mode bridge (ADR 0007); this adds a local
bind classifier (startup gate) + a non-fatal log assertion of an existing classification
— no Anthropic operation forwarded, so a cli.js citation is N/A under Rule 2. No
blacklisted tokens or port literals introduced; alignment.yml passes.
Independent fresh-context reviewer (opus): APPROVE WITH MINOR (Iron Rule 10) — verified
isLoopbackBind via truth table (no exposed address misclassified as loopback — the
dangerous direction), the full string→{text,entrypoint} contract ripple across all
callers (Promise<string> preserved), the mismatch logic (warns on sdk-cli AND null in
cli mode, silent for auto/off, non-fatal), and ALIGNMENT N/A via a fetch/header grep.
Minors are process-only (this commit body; a future lib/ extraction of the test-mirrored
helper). npm test → 181 passed, 0 failed.
Closes #115.
Co-authored-by: dtzp555 <dtzp555@gmail.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -168,6 +168,8 @@ function buildTuiCmd(claudeBin, model, sessionId) {
|
|||||||
// 5. Block on the native JSONL transcript (located by session-id) until terminal
|
// 5. Block on the native JSONL transcript (located by session-id) until terminal
|
||||||
// marker or wall-clock cap.
|
// marker or wall-clock cap.
|
||||||
// 6. Always teardown: kill session + rm temp dir (even on throw).
|
// 6. Always teardown: kill session + rm temp dir (even on throw).
|
||||||
|
// Returns { text, entrypoint } from readTuiTranscript (entrypoint is the billing-pool
|
||||||
|
// classifier, e.g. "cli", or null if the transcript did not include a turn_duration).
|
||||||
export async function runTuiTurn({
|
export async function runTuiTurn({
|
||||||
prompt,
|
prompt,
|
||||||
model,
|
model,
|
||||||
@@ -238,6 +240,7 @@ export async function runTuiTurn({
|
|||||||
tmux(["send-keys", "-t", tmuxName, "Enter"]);
|
tmux(["send-keys", "-t", tmuxName, "Enter"]);
|
||||||
|
|
||||||
// 3. Block on the native transcript (resolved by session-id) until terminal.
|
// 3. Block on the native transcript (resolved by session-id) until terminal.
|
||||||
|
// Returns { text, entrypoint } from readTuiTranscript.
|
||||||
return await readTuiTranscript({ home: ehome, sessionId, wallclockMs });
|
return await readTuiTranscript({ home: ehome, sessionId, wallclockMs });
|
||||||
} finally {
|
} finally {
|
||||||
// 4. Teardown — always, even on throw.
|
// 4. Teardown — always, even on throw.
|
||||||
|
|||||||
@@ -104,8 +104,10 @@ export function verifyEntrypoint(events) {
|
|||||||
|
|
||||||
// Block until the session transcript is terminal (turn_duration) or
|
// Block until the session transcript is terminal (turn_duration) or
|
||||||
// the wall-clock cap elapses, polling the file (no fs.watch — robust over NFS /
|
// the wall-clock cap elapses, polling the file (no fs.watch — robust over NFS /
|
||||||
// editors). Returns the latest assistant text. On cap with text, returns the
|
// editors). Returns { text, entrypoint } where text is the latest assistant text
|
||||||
// partial text; on cap with no text at all, throws.
|
// and entrypoint is the billing-pool classifier from the turn_duration line (e.g.
|
||||||
|
// "cli"), or null if not yet present. On cap with text, returns the partial result;
|
||||||
|
// on cap with no text at all, throws.
|
||||||
//
|
//
|
||||||
// No quiescence heuristic by design: a long Opus thinking turn stalls transcript
|
// No quiescence heuristic by design: a long Opus thinking turn stalls transcript
|
||||||
// growth and a "file stable for N s" rule would false-abort it (spec §4.3).
|
// growth and a "file stable for N s" rule would false-abort it (spec §4.3).
|
||||||
@@ -115,15 +117,18 @@ export function verifyEntrypoint(events) {
|
|||||||
export async function readTuiTranscript({ transcriptPath: p, home, sessionId, wallclockMs = 120000, pollMs = 250 }) {
|
export async function readTuiTranscript({ transcriptPath: p, home, sessionId, wallclockMs = 120000, pollMs = 250 }) {
|
||||||
const deadline = Date.now() + wallclockMs;
|
const deadline = Date.now() + wallclockMs;
|
||||||
let lastText = "";
|
let lastText = "";
|
||||||
|
let lastEntrypoint = null;
|
||||||
while (Date.now() < deadline) {
|
while (Date.now() < deadline) {
|
||||||
const resolved = p || findTranscriptPath(home, sessionId);
|
const resolved = p || findTranscriptPath(home, sessionId);
|
||||||
if (resolved && existsSync(resolved)) {
|
if (resolved && existsSync(resolved)) {
|
||||||
const events = parseTranscriptLines(readFileSync(resolved, "utf8"));
|
const events = parseTranscriptLines(readFileSync(resolved, "utf8"));
|
||||||
lastText = extractLatestAssistantText(events) || lastText;
|
lastText = extractLatestAssistantText(events) || lastText;
|
||||||
if (events.some(isTerminalLine)) return lastText;
|
const ep = verifyEntrypoint(events);
|
||||||
|
if (ep != null) lastEntrypoint = ep;
|
||||||
|
if (events.some(isTerminalLine)) return { text: lastText, entrypoint: lastEntrypoint };
|
||||||
}
|
}
|
||||||
await sleep(pollMs);
|
await sleep(pollMs);
|
||||||
}
|
}
|
||||||
if (lastText) return lastText;
|
if (lastText) return { text: lastText, entrypoint: lastEntrypoint };
|
||||||
throw new Error("tui_transcript_timeout: no assistant text within wallclock cap");
|
throw new Error("tui_transcript_timeout: no assistant text within wallclock cap");
|
||||||
}
|
}
|
||||||
|
|||||||
+22
-8
@@ -299,12 +299,20 @@ const TUI_CWD = process.env.OCP_TUI_CWD || `${process.env.HOME}/.ocp-tui/work`
|
|||||||
const TUI_HOME = process.env.OCP_TUI_HOME || process.env.HOME;
|
const TUI_HOME = process.env.OCP_TUI_HOME || process.env.HOME;
|
||||||
const TUI_ENTRYPOINT = process.env.OCP_TUI_ENTRYPOINT || "cli"; // cli|auto|off — see ADR 0007
|
const TUI_ENTRYPOINT = process.env.OCP_TUI_ENTRYPOINT || "cli"; // cli|auto|off — see ADR 0007
|
||||||
|
|
||||||
|
// A bind address is "loopback" only if it cannot be reached from another host.
|
||||||
|
// Any other address (0.0.0.0, ::, a concrete LAN/Tailscale IP, etc.) is
|
||||||
|
// network-exposed and must trigger the TUI LAN gate. (issue #115)
|
||||||
|
function isLoopbackBind(addr) {
|
||||||
|
return addr === "127.0.0.1" || addr === "::1" || addr === "localhost" ||
|
||||||
|
addr === "::ffff:127.0.0.1" || /^127\./.test(addr);
|
||||||
|
}
|
||||||
|
|
||||||
// SECURITY fail-loud: TUI-mode is incompatible with any configuration that allows
|
// SECURITY fail-loud: TUI-mode is incompatible with any configuration that allows
|
||||||
// non-operator prompts to reach the interactive claude session. Three cases:
|
// non-operator prompts to reach the interactive claude session. Three cases:
|
||||||
// 1. AUTH_MODE=multi — guest/anonymous keys can submit prompts.
|
// 1. AUTH_MODE=multi — guest/anonymous keys can submit prompts.
|
||||||
// 2. BIND_ADDRESS=0.0.0.0 — server is LAN-exposed; any LAN peer can send prompts
|
// 2. a non-loopback BIND_ADDRESS — server is network-exposed; any reachable peer
|
||||||
// unless per-request trust is in place. Override with OCP_TUI_ALLOW_LAN=1
|
// can send prompts unless per-request trust is in place. Override with
|
||||||
// ONLY if you have a separate network-layer trust (firewall, VPN).
|
// OCP_TUI_ALLOW_LAN=1 ONLY if you have a separate network-layer trust (firewall, VPN).
|
||||||
// 3. PROXY_ANONYMOUS_KEY set — anonymous callers can submit prompts without a key.
|
// 3. PROXY_ANONYMOUS_KEY set — anonymous callers can submit prompts without a key.
|
||||||
// In all three cases TUI runs interactive claude with the OPERATOR's full filesystem
|
// In all three cases TUI runs interactive claude with the OPERATOR's full filesystem
|
||||||
// access — home is NOT isolation. Refuse to boot. See ADR 0007.
|
// access — home is NOT isolation. Refuse to boot. See ADR 0007.
|
||||||
@@ -317,11 +325,11 @@ if (TUI_MODE && AUTH_MODE === "multi") {
|
|||||||
);
|
);
|
||||||
process.exit(1);
|
process.exit(1);
|
||||||
}
|
}
|
||||||
if (TUI_MODE && BIND_ADDRESS === "0.0.0.0" && process.env.OCP_TUI_ALLOW_LAN !== "1") {
|
if (TUI_MODE && !isLoopbackBind(BIND_ADDRESS) && process.env.OCP_TUI_ALLOW_LAN !== "1") {
|
||||||
console.error(
|
console.error(
|
||||||
"FATAL: CLAUDE_TUI_MODE=true with CLAUDE_BIND=0.0.0.0 is unsafe.\n" +
|
`FATAL: CLAUDE_TUI_MODE=true with a non-loopback CLAUDE_BIND (${BIND_ADDRESS}) is unsafe.\n` +
|
||||||
" TUI runs interactive claude with operator filesystem access; LAN-exposed without\n" +
|
" TUI runs interactive claude with operator filesystem access; network-exposed without\n" +
|
||||||
" per-request isolation means any LAN peer could drive the operator's claude session.\n" +
|
" per-request isolation means any reachable peer could drive the operator's claude session.\n" +
|
||||||
" Either bind to 127.0.0.1 (default) or set OCP_TUI_ALLOW_LAN=1 if you have a\n" +
|
" Either bind to 127.0.0.1 (default) or set OCP_TUI_ALLOW_LAN=1 if you have a\n" +
|
||||||
" separate network-layer trust (firewall/VPN). See docs/adr/0007-tui-interactive-mode.md."
|
" separate network-layer trust (firewall/VPN). See docs/adr/0007-tui-interactive-mode.md."
|
||||||
);
|
);
|
||||||
@@ -925,8 +933,14 @@ function callClaudeTui(model, messages, _conversationId, _keyName) {
|
|||||||
cwd: TUI_CWD,
|
cwd: TUI_CWD,
|
||||||
wallclockMs: TUI_WALLCLOCK_MS,
|
wallclockMs: TUI_WALLCLOCK_MS,
|
||||||
entrypointMode: TUI_ENTRYPOINT,
|
entrypointMode: TUI_ENTRYPOINT,
|
||||||
}).then((text) => {
|
}).then(({ text, entrypoint }) => {
|
||||||
recordModelSuccess(cliModel, 0); // elapsed not measurable here; wallclock at reader level
|
recordModelSuccess(cliModel, 0); // elapsed not measurable here; wallclock at reader level
|
||||||
|
// Assert the subscription-pool classification. TUI exists to keep cc_entrypoint=cli
|
||||||
|
// (subscription pool); a silent degrade to sdk-cli (metered Agent SDK pool) would still
|
||||||
|
// return text but cost money — warn loudly so it's visible. (issue #115)
|
||||||
|
if (TUI_ENTRYPOINT === "cli" && entrypoint !== "cli") {
|
||||||
|
logEvent("warn", "tui_entrypoint_mismatch", { expected: "cli", got: entrypoint, model: cliModel });
|
||||||
|
}
|
||||||
return text;
|
return text;
|
||||||
}).catch((err) => {
|
}).catch((err) => {
|
||||||
recordModelError(cliModel, false);
|
recordModelError(cliModel, false);
|
||||||
|
|||||||
+46
-4
@@ -1438,10 +1438,11 @@ await asyncTest("readTuiTranscript returns assistant text when terminal marker p
|
|||||||
const p = `${dir}/s.jsonl`;
|
const p = `${dir}/s.jsonl`;
|
||||||
tuiWriteFile(p, [
|
tuiWriteFile(p, [
|
||||||
JSON.stringify({ type: "assistant", message: { content: [{ type: "text", text: "hello world" }] } }),
|
JSON.stringify({ type: "assistant", message: { content: [{ type: "text", text: "hello world" }] } }),
|
||||||
JSON.stringify({ type: "system", subtype: "turn_duration", durationMs: 1200 }),
|
JSON.stringify({ type: "system", subtype: "turn_duration", durationMs: 1200, entrypoint: "cli" }),
|
||||||
].join("\n") + "\n");
|
].join("\n") + "\n");
|
||||||
const out = await readTuiTranscript({ transcriptPath: p, wallclockMs: 2000, pollMs: 50 });
|
const out = await readTuiTranscript({ transcriptPath: p, wallclockMs: 2000, pollMs: 50 });
|
||||||
assert.equal(out, "hello world");
|
assert.equal(out.text, "hello world");
|
||||||
|
assert.equal(out.entrypoint, "cli");
|
||||||
});
|
});
|
||||||
|
|
||||||
await asyncTest("readTuiTranscript honours wall-clock cap and returns partial text", async () => {
|
await asyncTest("readTuiTranscript honours wall-clock cap and returns partial text", async () => {
|
||||||
@@ -1449,7 +1450,12 @@ await asyncTest("readTuiTranscript honours wall-clock cap and returns partial te
|
|||||||
const p = `${dir}/s.jsonl`;
|
const p = `${dir}/s.jsonl`;
|
||||||
tuiWriteFile(p, JSON.stringify({ type: "assistant", message: { content: [{ type: "text", text: "partial" }] } }) + "\n");
|
tuiWriteFile(p, JSON.stringify({ type: "assistant", message: { content: [{ type: "text", text: "partial" }] } }) + "\n");
|
||||||
const out = await readTuiTranscript({ transcriptPath: p, wallclockMs: 300, pollMs: 50 });
|
const out = await readTuiTranscript({ transcriptPath: p, wallclockMs: 300, pollMs: 50 });
|
||||||
assert.equal(out, "partial");
|
assert.equal(out.text, "partial");
|
||||||
|
});
|
||||||
|
|
||||||
|
await asyncTest("readTuiTranscript against real fixture: entrypoint is 'cli'", async () => {
|
||||||
|
const out = await readTuiTranscript({ transcriptPath: "./lib/tui/fixtures/complete-haiku.jsonl", wallclockMs: 2000, pollMs: 50 });
|
||||||
|
assert.equal(out.entrypoint, "cli");
|
||||||
});
|
});
|
||||||
|
|
||||||
await asyncTest("readTuiTranscript throws when no text and cap elapses", async () => {
|
await asyncTest("readTuiTranscript throws when no text and cap elapses", async () => {
|
||||||
@@ -1600,7 +1606,7 @@ if (process.env.OCP_TUI_LIVE === "1") {
|
|||||||
cwd: `${process.env.HOME}/.ocp-tui/work`,
|
cwd: `${process.env.HOME}/.ocp-tui/work`,
|
||||||
wallclockMs: 120000,
|
wallclockMs: 120000,
|
||||||
});
|
});
|
||||||
assert.ok(/PONG/i.test(out), `expected PONG, got: ${out.slice(0, 200)}`);
|
assert.ok(/PONG/i.test(out.text), `expected PONG, got: ${out.text.slice(0, 200)}`);
|
||||||
});
|
});
|
||||||
} else {
|
} else {
|
||||||
test("runTuiTurn (live) — SKIPPED (set OCP_TUI_LIVE=1 on PI231 to run)", () => {
|
test("runTuiTurn (live) — SKIPPED (set OCP_TUI_LIVE=1 on PI231 to run)", () => {
|
||||||
@@ -1792,6 +1798,42 @@ test("KEY_NAME_RE: 65-char string → invalid", () => {
|
|||||||
assert.ok(!KEY_NAME_RE.test("x".repeat(65)));
|
assert.ok(!KEY_NAME_RE.test("x".repeat(65)));
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// ── isLoopbackBind helper (issue #115) ──────────────────────────────────────
|
||||||
|
// MIRRORS server.mjs isLoopbackBind — copied verbatim to avoid importing server.mjs
|
||||||
|
// (top-level server.listen() would start a live HTTP server).
|
||||||
|
// Keep in sync with the definition in server.mjs above the TUI gate block.
|
||||||
|
console.log("\nisLoopbackBind helper (issue #115):");
|
||||||
|
|
||||||
|
function isLoopbackBind(addr) {
|
||||||
|
return addr === "127.0.0.1" || addr === "::1" || addr === "localhost" ||
|
||||||
|
addr === "::ffff:127.0.0.1" || /^127\./.test(addr);
|
||||||
|
}
|
||||||
|
|
||||||
|
test("isLoopbackBind: '127.0.0.1' → true", () => {
|
||||||
|
assert.equal(isLoopbackBind("127.0.0.1"), true);
|
||||||
|
});
|
||||||
|
test("isLoopbackBind: '::1' → true", () => {
|
||||||
|
assert.equal(isLoopbackBind("::1"), true);
|
||||||
|
});
|
||||||
|
test("isLoopbackBind: 'localhost' → true", () => {
|
||||||
|
assert.equal(isLoopbackBind("localhost"), true);
|
||||||
|
});
|
||||||
|
test("isLoopbackBind: '127.0.0.5' → true (127.x.x.x range)", () => {
|
||||||
|
assert.equal(isLoopbackBind("127.0.0.5"), true);
|
||||||
|
});
|
||||||
|
test("isLoopbackBind: '0.0.0.0' → false (any-interface)", () => {
|
||||||
|
assert.equal(isLoopbackBind("0.0.0.0"), false);
|
||||||
|
});
|
||||||
|
test("isLoopbackBind: '192.168.1.5' → false (LAN IP)", () => {
|
||||||
|
assert.equal(isLoopbackBind("192.168.1.5"), false);
|
||||||
|
});
|
||||||
|
test("isLoopbackBind: '::' → false (IPv6 any-interface)", () => {
|
||||||
|
assert.equal(isLoopbackBind("::"), false);
|
||||||
|
});
|
||||||
|
test("isLoopbackBind: '100.64.0.1' → false (Tailscale IP)", () => {
|
||||||
|
assert.equal(isLoopbackBind("100.64.0.1"), false);
|
||||||
|
});
|
||||||
|
|
||||||
// ── Cleanup ──
|
// ── Cleanup ──
|
||||||
closeDb();
|
closeDb();
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user