Compare commits

..
Author SHA1 Message Date
taodengandClaude Opus 4.8 69688a46c7 docs(readme): carry the ToS caveat into the MANUAL install path too (README:218)
Reviewer found a third instance, and it is the one that most directly reproduces #136.

README has two forms of the same LAN-mode install: the copy-paste AI prompt (line 132) and
the handbook form (line 218). Line 180 explicitly asserts they are 'the same steps in handbook
form'. They were not:

  line 132 (prompt)   'install OCP as a server so YOUR OWN DEVICES on the LAN can reach it
                       (Claude Pro/Max are per-user accounts — review Anthropic's Usage Policy
                       before extending access to other people)'   + example keys: laptop, tablet
  line 218 (handbook) 'share with other devices on your network:'  + 'create API keys for each
                       PERSON/device' + no ToS pointer at all

So a reader who takes the manual route instead of the copy-paste route is still walked into
per-person key creation with zero ToS mention — reproducing #136's trigger through the door
the first commit did not close. The caveat now matches its twin.

Deliberately NOT scrubbing the wife-laptop / son-ipad example key names: they recur in seven
further places, it is a bigger diff at a different severity, and it edges into scrubbing the
maintainer's household out of their own documentation. With the pointer restored at 218 those
examples inherit the caveat — which is exactly the posture § honest limits takes. It never
forbids family sharing; it says it is the account holder's call and their risk, and refuses to
hide that.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VqgWJcjxrjjL9L9SkpZyXR
2026-07-15 08:28:07 +10:00
taodengandClaude Opus 4.8 26b116deb8 docs(readme): fix the misdirected anchor + the same defect at line 52 (review fold-in)
Independent reviewer caught two things in the first cut:

1. The new link pointed at #auth-modes — which resolves to the '### Auth Modes' mode
   table (line 408), NOT to the 'Sharing with family / a team — honest limits'
   paragraph (line 422), which sits under '### Deployment model & security (read
   this)' (line 418). A bullet that says 'see the honest limits' and then sends you
   somewhere else is worse than no link. Correct anchor verified two ways (github-slugger
   + the live rendered page): #deployment-model--security-read-this (double hyphen — the
   '&' is stripped but both surrounding spaces survive).

2. Line 52 carried the SAME defect the PR was written to fix, a few lines below it:
   'share one Claude Pro/Max subscription across IDEs, devices, and people'. So the
   original claim — 'this only stops the document arguing with itself' — was not yet
   true: it stopped one instance and left an adjacent one standing, in the same
   top-of-README section an install-time reviewer reads first.

Left alone deliberately: the maintainer's own account of their household's use (lines 7
and 1178). That is theirs to make, and a docs PR should not quietly rewrite it.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VqgWJcjxrjjL9L9SkpZyXR
2026-07-15 08:03:10 +10:00
taodengandClaude Opus 4.8 b7f5f4d144 docs(readme): stop the feature bullet promising what § honest limits forbids (#136)
Issue #136: an external user reported that Claude Code REFUSES to run OCP's own
copy-paste install prompt, on the grounds that the premise — pooling one Pro/Max
subscription across a family — violates Anthropic's Usage Policy.

The install prompts themselves were already fixed since that report ('my own devices
on the network', plus a ToS warning on the LAN section). What remained was a
self-contradiction in the README:

  line 27  (feature bullet):  'share one Claude Pro/Max subscription with family,
                               friends, or your own devices'
  line 427 (§ honest limits): 'The defensible framing is "one person, your own
                               devices" — sharing with friends or a team is not.'

The top-of-funnel bullet was promoting exactly what the project's own ToS section
calls indefensible. That is a defect on its own terms, independent of anyone's view
on the underlying policy: a reader who trusts the bullet is walked straight into the
thing the same document later tells them not to do.

Aligned the bullet to the position the project ALREADY took, and linked the honest-limits
section from it. No change to the auth modes, the LAN feature, or the maintainer's own
account of how they use it — this only stops the doc arguing with itself.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VqgWJcjxrjjL9L9SkpZyXR
2026-07-15 07:56:16 +10:00
13 changed files with 60 additions and 468 deletions
-27
View File
@@ -1,32 +1,5 @@
# Changelog # Changelog
## v3.22.1 — 2026-07-17
Minor release: TUI-mode latency and streaming features — **all opt-in and off by default**, so the default request path (`-p` / `--output-format stream-json`) is byte-for-byte unchanged — plus hardening from an independent (Codex) re-review of the streaming work, Windows `claude.exe` startup resolution, and the Claude Sonnet 5 model entry. No new `cli.js` wire behavior and no new endpoint; the new surface is entirely OCP-owned TUI-mode configuration (env vars), startup binary discovery, model metadata, and `/health` observation. Every code PR carried a fresh-context reviewer (Iron Rule 10). (Version note: v3.22.0 was prepared but never tagged; its contents ship here as v3.22.1 together with the additions below.)
### Added
- **Claude Sonnet 5 in the model SPOT (#152, contributed by @vvlasy-openclaw)** — `claude-sonnet-5` added to `models.json` (`contextWindow` 200000 / `maxTokens` 16384 / `reasoning` true, consistent with existing entries), exposed via `/v1/models` and the OpenClaw sync. Purely additive: the `sonnet` alias still resolves to `claude-sonnet-4-6` (the repoint is tracked separately in #168). `ocp-connect`'s model classifier now matches on the model *family* prefix (`claude-sonnet`/`claude-opus`/`claude-haiku`) instead of version-pinned prefixes, so current and future versioned IDs register with correct `reasoning`/`maxTokens` metadata. New referential-integrity tests guard that every alias target exists in `models[]`.
- **Windows `claude.exe` startup resolution (#161, contributed by @nyxst4ck, diagnosis credit #147 @Justinsato)** — on Windows, `resolveClaude()` now discovers a native `claude.exe` (`%USERPROFILE%\.local\bin`, WinGet Links, WindowsApps, then `where.exe`) and rejects npm `.cmd`/`.bat`/`.ps1` shims, which cannot be spawned without a shell — previously startup resolved a shim and failed. A non-`.exe` `CLAUDE_BIN` on Windows is a fatal error with an actionable hint. The macOS/Linux path is byte-for-byte unchanged. Note: this is startup binary resolution only — full Windows support is not yet claimed (snapshot-path portability is tracked in #167).
### Added — TUI mode (all opt-in, default off)
- **Spawn effort control — `OCP_TUI_EFFORT` (default `low`) (#156)** — the interactive `claude` is now spawned with an explicit `--effort` flag. `low` cuts measured TTFT p50 by ~40% and collapses run-to-run variance ~15× versus an inherited `xhigh`; proxied requests rarely benefit from extended thinking. Set `inherit` to omit the flag and restore the pre-flag HOME-dependent behaviour. Banner-verified to stay on the subscription pool (`· Claude Max`); an invalid value warns and falls back to `low`. README § "Environment Variables".
- **Warm pane pool — `OCP_TUI_POOL_SIZE` (default `0` / off) (#158)** — pre-boots up to 4 single-use `claude` panes so a request skips the cold boot: measured end-to-end p50 `10.17s``6.00s` (41%) on a Mac mini (Sonnet 4.6, `--effort low`). Opt-in because each warm pane is a live idle process held whether or not a request ever arrives. Panes are single-use (one turn, then killed and replaced in the background), port-scoped (`ocp-tui-<port>-p<hex>`), and coexist with the zombie reaper by a synchronous drain→reap→resume sweep. README §§ "Environment Variables" + "How It Works".
- **Real SSE streaming — `OCP_TUI_STREAM` (default `0` / off) (#159, #160)** — `stream:true` turns emit real `delta.content` chunks as `claude` generates them, sourced from `claude`'s own `MessageDisplay` hook (registered via `--settings` on the ordinary interactive spawn — banner-verified on the subscription pool). Granularity is block-level, and it moves the *first* byte, not the last. The transcript stays authoritative: streamed text is asserted equal to it at end-of-turn, the auth-banner and truncation gates still run before anything is committed, and a turn whose stream cannot be reconciled is **refused** (SSE error frame, not cached) and counted on `/health` (`tui.streamDivergences`; a silent total-hook-failure is counted separately as `tui.streamZeroDeltaTurns`). Tunables: `OCP_TUI_STREAM_HOLDBACK` (default `100`), `OCP_TUI_STREAM_DIR`, `OCP_TUI_STREAM_POLL_MS`. See ADR 0007 (2026-07-13 amendment). README §§ "Environment Variables" + "How It Works".
### Fixed
- **Streaming auth-banner guard: a null `message_id` on the first hook fire (#160)** — a first `MessageDisplay` fire with a null `message_id` could disarm the auth-banner guard; re-landed after a #159 squash dropped it (`lib/tui/stream.mjs`).
- **Test suite wrote live, unrevoked API keys into the operator's real key store (#163)** — `npm test` had been opening `~/.ocp/ocp.db` (the running server's DB) and writing two junk `api_keys` rows per run (737 accumulated on the maintainer's host), because the isolation the comments claimed was never wired (ESM import hoisting). `keys.mjs` now honors `OCP_DIR_OVERRIDE` under `NODE_ENV=test` and the suite points at a scratch dir; a child-process probe verifies a production process (no `NODE_ENV`) cannot be redirected.
- **Streaming holdback floor + billing-pool observation on failed turns (#164)** — (A1) `OCP_TUI_STREAM_HOLDBACK` now clamps up to the safe floor (`100`) with a boot warning, closing a latent auth-banner leak when an operator set a sub-floor value. (A3) the `cc_entrypoint` (billing-pool) observation is now recorded before the honesty gates that throw, so `/health` no longer goes blind to exactly the failed turns most likely to signal a silent degrade to the metered Agent SDK pool.
- **Test-only key-store redirection vars can no longer reach a server OCP launches (#165)** — (A4) `NODE_ENV`/`OCP_DIR_OVERRIDE` are stripped from every service unit `setup.mjs` writes (`plist-merge`'s `NEVER_PRESERVE`) and from the `ocp restart` manual nohup fallback (`env -u`); #163's overstated "a prod server can NEVER be redirected" comments were softened to name the one residual hand-launch path and the loud `getDb()` "NOT the default" backstop.
### Docs
- **README billing honesty (#162, closes #136)** — removed a feature bullet that promised what the § "honest limits" section forbids.
- **TUI latency plans + streaming-achievability spike (#155, #157)** — measured latency decomposition, backlog, and the `MessageDisplay`-hook streaming prereq spike under `docs/plans/2026-07-13-tui-latency/`.
## v3.21.1 — 2026-07-07 ## v3.21.1 — 2026-07-07
Patch release: three bug fixes from an independent concurrency/session-lifecycle audit, each its own PR with a fresh-context reviewer (Iron Rule 10). No new `cli.js` wire behavior, no new endpoint, header, or env var; the `/health` field set is unchanged (only value truthfulness improved). Patch release: three bug fixes from an independent concurrency/session-lifecycle audit, each its own PR with a fresh-context reviewer (Iron Rule 10). No new `cli.js` wire behavior, no new endpoint, header, or env var; the `/health` field set is unchanged (only value truthfulness improved).
-1
View File
@@ -716,7 +716,6 @@ Any tool use happens server-side, under the `--allowedTools` set configured on t
| `claude-opus-4-8` | Most capable (default for `opus` alias) | | `claude-opus-4-8` | Most capable (default for `opus` alias) |
| `claude-opus-4-7` | Previous Opus, retained for pinning | | `claude-opus-4-7` | Previous Opus, retained for pinning |
| `claude-opus-4-6` | Older Opus, retained for pinning | | `claude-opus-4-6` | Older Opus, retained for pinning |
| `claude-sonnet-5` | Latest Sonnet (available by full ID; `sonnet` alias repoint tracked separately) |
| `claude-sonnet-4-6` | Good balance of speed/quality (default for `sonnet` alias) | | `claude-sonnet-4-6` | Good balance of speed/quality (default for `sonnet` alias) |
| `claude-haiku-4-5-20251001` | Fastest, lightweight (default for `haiku` alias) | | `claude-haiku-4-5-20251001` | Fastest, lightweight (default for `haiku` alias) |
+8 -56
View File
@@ -6,74 +6,26 @@ import { join } from "node:path";
import { mkdirSync, chmodSync } from "node:fs"; import { mkdirSync, chmodSync } from "node:fs";
import { homedir } from "node:os"; import { homedir } from "node:os";
// Resolved LAZILY, on first getDb() — not at module top-level. Two reasons, and the second is const OCP_DIR = join(homedir(), ".ocp");
// the bug this fixes: mkdirSync(OCP_DIR, { recursive: true, mode: 0o700 });
// // Tighten the directory mode in case it already existed with broader permissions.
// 1. Merely IMPORTING keys.mjs should not, as a side effect, create directories in the try { chmodSync(OCP_DIR, 0o700); } catch { /* ignore EPERM on pre-existing dirs */ }
// operator's home. const DB_PATH = join(OCP_DIR, "ocp.db");
// 2. OCP_DIR_OVERRIDE exists so the test suite can point the key store at a scratch dir — and
// because ESM hoists imports, a top-level `const OCP_DIR = ...` here would be evaluated
// BEFORE an importing module's body could set the env var. Eager resolution made the
// override unsettable in the one place that needs it. (test-features.mjs carried a comment
// claiming it could "set env before the first getDb() call" — it could not, because nothing
// here ever read an env var. So `npm test` wrote real, UNREVOKED api_keys rows into the
// operator's live ~/.ocp/ocp.db: two per run, unbounded — 737 junk keys against 12 real ones
// on the maintainer's host — and two concurrent runs raced one file, which is the ~1-in-6
// flake in `listKeys includes quota fields`.)
//
// The override is gated on NODE_ENV === "test", and that gate is the ACTUAL guard. An earlier
// cut of this fix relied on the variable merely having an awkward name — i.e. a naming convention
// plus a comment — which is precisely the failure mode this whole change exists to indict (a
// comment describing an intention that nothing enforces). The two-key gate means NEITHER var
// alone does anything: a stray OCP_DIR_OVERRIDE with no NODE_ENV is inert, and NODE_ENV=test with
// no override just resolves the default dir.
//
// This gate does NOT, by itself, prove a production daemon can't be redirected — an earlier
// version of this comment overclaimed that ("a production server runs without NODE_ENV, so it
// CANNOT honor the override no matter how the variable got in"). That is only true while the
// daemon's env actually lacks NODE_ENV=test, which is an assumption, not something this file can
// enforce. What makes it hold in the shipped configuration is defense-in-depth in OCP's launchers:
// the plist/systemd units strip both vars on every (re)install (scripts/lib/plist-merge.mjs
// NEVER_PRESERVE), and `ocp` restart's manual nohup fallback strips them (`env -u`). So a server
// OCP itself started cannot carry the test-only redirection. The one residual path is an operator
// who hand-launches `node server.mjs` with BOTH vars explicitly exported, bypassing every
// launcher — a case no library-level gate can catch. The loud getDb() log below ("NOT the default
// ~/.ocp/ocp.db") is the backstop there: a wrong key store is at least never silent (in
// AUTH_MODE=multi that would otherwise be a total auth outage with nothing on /health to show it).
function resolveOcpDir() {
const override = process.env.NODE_ENV === "test" ? process.env.OCP_DIR_OVERRIDE : null;
const dir = override || join(homedir(), ".ocp");
mkdirSync(dir, { recursive: true, mode: 0o700 });
// Tighten the directory mode in case it already existed with broader permissions.
try { chmodSync(dir, 0o700); } catch { /* ignore EPERM on pre-existing dirs */ }
return dir;
}
let db; let db;
let dbPath; // resolved on first open, alongside the db handle
export function getDb() { export function getDb() {
if (!db) { if (!db) {
dbPath = join(resolveOcpDir(), "ocp.db"); db = new DatabaseSync(DB_PATH);
// Say which store we opened. Silence was the other half of the bug: a server on the wrong
// key store looks exactly like a server on the right one until every request 401s.
if (dbPath !== join(homedir(), ".ocp", "ocp.db")) {
console.error(`[keys] key store: ${dbPath} (NOT the default ~/.ocp/ocp.db)`);
}
db = new DatabaseSync(dbPath);
db.exec("PRAGMA journal_mode = WAL"); db.exec("PRAGMA journal_mode = WAL");
db.exec("PRAGMA foreign_keys = ON"); db.exec("PRAGMA foreign_keys = ON");
initSchema(); initSchema();
// Tighten mode on the DB file (0600) after creation / first open. // Tighten mode on the DB file (0600) after creation / first open.
try { chmodSync(dbPath, 0o600); } catch { /* ignore — same-user access still works */ } try { chmodSync(DB_PATH, 0o600); } catch { /* ignore — same-user access still works */ }
} }
return db; return db;
} }
// Which file the key store actually opened. Exported so a test can ASSERT it is not the
// operator's real db — the bug this replaced was invisible precisely because nothing checked.
export function getDbPath() { return dbPath; }
function initSchema() { function initSchema() {
db.exec(` db.exec(`
CREATE TABLE IF NOT EXISTS api_keys ( CREATE TABLE IF NOT EXISTS api_keys (
@@ -474,5 +426,5 @@ export function findKey(idOrName) {
} }
export function closeDb() { export function closeDb() {
if (db) { db.close(); db = null; dbPath = undefined; } // clear both — a path to a closed db is a footgun if (db) { db.close(); db = null; }
} }
-15
View File
@@ -45,21 +45,6 @@ import { detectTuiUpstreamError } from "./transcript.mjs";
// Default holdback before the first byte is released to the client. See TuiDeltaAssembler. // Default holdback before the first byte is released to the client. See TuiDeltaAssembler.
export const DEFAULT_HOLDBACK_CHARS = 100; export const DEFAULT_HOLDBACK_CHARS = 100;
// Resolve OCP_TUI_STREAM_HOLDBACK to a SAFE value. The whole C-1 auth-banner guarantee rests
// on the holdback being at least the default banner detector's max message length — which is
// exactly DEFAULT_HOLDBACK_CHARS. So this is a FLOOR, not a hint: a smaller value (or a NaN
// typo like "unlimited"/"5MB") would let a real banner fragment release before the terminal
// detector could classify the whole message, silently reopening the leak the assembler exists
// to prevent. The env var's own doc says "Only raise it"; this enforces that instead of trusting
// it. Returns { value, clamped } so the caller can warn when it had to clamp — a silent floor is
// less honest than a noticed one.
export function resolveStreamHoldback(raw, floor = DEFAULT_HOLDBACK_CHARS) {
const parsed = parseInt(raw ?? "", 10);
if (!Number.isFinite(parsed)) return { value: floor, clamped: raw != null && String(raw).trim() !== "" };
if (parsed < floor) return { value: floor, clamped: true };
return { value: parsed, clamped: false };
}
// The hook script. POSIX sh, no interpreter startup beyond /bin/sh, one fork (`cat`). // The hook script. POSIX sh, no interpreter startup beyond /bin/sh, one fork (`cat`).
// //
// - `printf` is a shell BUILTIN in sh/dash/bash, so the newline costs no fork. // - `printf` is a shell BUILTIN in sh/dash/bash, so the newline costs no fork.
-8
View File
@@ -26,14 +26,6 @@
"contextWindow": 200000, "contextWindow": 200000,
"maxTokens": 16384 "maxTokens": 16384
}, },
{
"id": "claude-sonnet-5",
"displayName": "Claude Sonnet 5",
"openclawName": "Claude Sonnet 5 (via CLI)",
"reasoning": true,
"contextWindow": 200000,
"maxTokens": 16384
},
{ {
"id": "claude-sonnet-4-6", "id": "claude-sonnet-4-6",
"displayName": "Claude Sonnet 4.6", "displayName": "Claude Sonnet 4.6",
+1 -6
View File
@@ -622,12 +622,7 @@ cmd_restart() {
self_r="${BASH_SOURCE[0]}" self_r="${BASH_SOURCE[0]}"
while [[ -L "$self_r" ]]; do self_r="$(readlink "$self_r")"; done while [[ -L "$self_r" ]]; do self_r="$(readlink "$self_r")"; done
script_dir="$(cd "$(dirname "$self_r")" && pwd)" script_dir="$(cd "$(dirname "$self_r")" && pwd)"
# env -u strips test-only key-store redirection vars (A4): if the invoking shell had DISABLE_AUTOUPDATER=1 nohup node "$script_dir/server.mjs" >> "$HOME/.ocp/logs/proxy.log" 2>&1 &
# NODE_ENV=test + OCP_DIR_OVERRIDE exported (e.g. from a debugging session), this manual
# fallback would otherwise inherit them and start the daemon against a scratch/empty key
# store — a silent auth outage in AUTH_MODE=multi. The plist/systemd paths strip these via
# plist-merge's NEVER_PRESERVE; this covers the one direct-launch path OCP controls.
DISABLE_AUTOUPDATER=1 env -u NODE_ENV -u OCP_DIR_OVERRIDE nohup node "$script_dir/server.mjs" >> "$HOME/.ocp/logs/proxy.log" 2>&1 &
fi fi
sleep 3 sleep 3
if curl -sf --max-time 5 "$PROXY/health" > /dev/null 2>&1; then if curl -sf --max-time 5 "$PROXY/health" > /dev/null 2>&1; then
+8 -14
View File
@@ -122,17 +122,11 @@ provider = {
"models": [] "models": []
} }
# Model metadata mapping. Prefix match on the model FAMILY (claude-opus / -sonnet / # Model metadata mapping (prefix match for versioned IDs like claude-haiku-4-5-20251001)
# -haiku), not a pinned version. A version-pinned prefix like "claude-sonnet-4"
# silently misses "claude-sonnet-5" and falls through to the non-reasoning /
# 8k-output default (PR #152 review) — every future Sonnet/Opus/Haiku bump would
# re-trip it. Family prefixes classify any versioned ID correctly with no per-model
# edit. (ADR 0003: models.json is the SPOT for model existence; /v1/models does not
# expose reasoning/maxTokens, so family classification stays here.)
model_meta = { model_meta = {
"claude-opus": {"name": "Claude Opus (OCP)", "reasoning": True, "maxTokens": 16384}, "claude-opus-4": {"name": "Claude Opus (OCP)", "reasoning": True, "maxTokens": 16384},
"claude-sonnet": {"name": "Claude Sonnet (OCP)", "reasoning": True, "maxTokens": 16384}, "claude-sonnet-4": {"name": "Claude Sonnet (OCP)", "reasoning": True, "maxTokens": 16384},
"claude-haiku": {"name": "Claude Haiku (OCP)", "reasoning": False, "maxTokens": 8192}, "claude-haiku-4": {"name": "Claude Haiku (OCP)", "reasoning": False, "maxTokens": 8192},
} }
def get_model_meta(mid): def get_model_meta(mid):
@@ -184,11 +178,11 @@ config.setdefault("agents", {})
config["agents"].setdefault("defaults", {}) config["agents"].setdefault("defaults", {})
config["agents"]["defaults"].setdefault("models", {}) config["agents"]["defaults"].setdefault("models", {})
# Build alias map (family prefix match — version-agnostic, see model_meta note) # Build alias map (prefix match)
alias_prefixes = { alias_prefixes = {
"claude-opus": "Claude Opus", "claude-opus-4": "Claude Opus",
"claude-sonnet": "Claude Sonnet", "claude-sonnet-4": "Claude Sonnet",
"claude-haiku": "Claude Haiku", "claude-haiku-4": "Claude Haiku",
} }
for mid in model_ids: for mid in model_ids:
+1 -1
View File
@@ -1,6 +1,6 @@
{ {
"name": "open-claude-proxy", "name": "open-claude-proxy",
"version": "3.22.1", "version": "3.21.1",
"description": "OCP (Open Claude Proxy) — use your Claude Pro/Max subscription as an OpenAI-compatible API for any IDE. Works with Cline, OpenCode, Aider, Continue.dev, OpenClaw, and more.", "description": "OCP (Open Claude Proxy) — use your Claude Pro/Max subscription as an OpenAI-compatible API for any IDE. Works with Cline, OpenCode, Aider, Continue.dev, OpenClaw, and more.",
"type": "module", "type": "module",
"bin": { "bin": {
+2 -15
View File
@@ -8,19 +8,6 @@
// //
// No new dependencies — regex-based, plist <key>X</key><string>Y</string> shape // No new dependencies — regex-based, plist <key>X</key><string>Y</string> shape
// is stable enough for our hand-written templates in setup.mjs. // is stable enough for our hand-written templates in setup.mjs.
//
// SECURITY DENYLIST (A4): keys that must NEVER be carried into a service unit, even when a
// prior unit already contained them. OCP's key store honors OCP_DIR_OVERRIDE only when
// NODE_ENV === "test" (keys.mjs). If BOTH somehow reached a daemon's environment, the server
// would open a scratch/empty key store instead of ~/.ocp/ocp.db — in AUTH_MODE=multi a silent
// total auth outage. The preservation rule below ("keys only in EXISTING are kept verbatim")
// is exactly a vector for that: a unit that once carried these test-only vars would otherwise
// survive every setup re-run. So we strip them from the preserved set unconditionally. This is
// defense-in-depth: setup.mjs's own template never injects them, so the only way they enter is
// preservation, and this closes it. (The residual path — a hand-rolled `node server.mjs` with
// both vars exported — is out of any launcher's reach; keys.mjs's loud "NOT the default" log is
// the backstop there.)
export const NEVER_PRESERVE = new Set(["NODE_ENV", "OCP_DIR_OVERRIDE"]);
// Note: setup.mjs XML-escapes all injected values before writing (via xmlEscape()), // Note: setup.mjs XML-escapes all injected values before writing (via xmlEscape()),
// so raw `<` / `>` / `&` never appear in plist <string> bodies — the [^<]* regex below is safe. // so raw `<` / `>` / `&` never appear in plist <string> bodies — the [^<]* regex below is safe.
@@ -49,7 +36,7 @@ export function mergePlistEnv(existing, template) {
const preserved = {}; const preserved = {};
for (const [k, v] of Object.entries(existingEnv)) { for (const [k, v] of Object.entries(existingEnv)) {
if (!KNOWN.has(k) && !NEVER_PRESERVE.has(k)) preserved[k] = v; if (!KNOWN.has(k)) preserved[k] = v;
} }
if (Object.keys(preserved).length === 0) return template; if (Object.keys(preserved).length === 0) return template;
@@ -85,7 +72,7 @@ export function mergeSystemdEnv(existing, template) {
const KNOWN = new Set(Object.keys(templateEnv)); const KNOWN = new Set(Object.keys(templateEnv));
const preservedLines = Object.entries(existingEnv) const preservedLines = Object.entries(existingEnv)
.filter(([k]) => !KNOWN.has(k) && !NEVER_PRESERVE.has(k)) .filter(([k]) => !KNOWN.has(k))
.map(([k, v]) => `Environment=${k}=${v}`); .map(([k, v]) => `Environment=${k}=${v}`);
if (preservedLines.length === 0) return template; if (preservedLines.length === 0) return template;
+29 -97
View File
@@ -47,7 +47,7 @@ import { runTuiTurn, reapStaleTuiSessions, resolveTuiHome, bootTuiPane, tuiPaneH
import { detectTuiUpstreamError } from "./lib/tui/transcript.mjs"; import { detectTuiUpstreamError } from "./lib/tui/transcript.mjs";
import { TuiSemaphore, SemaphoreAbortError, recordTuiEntrypoint, buildTuiHealthBlock } from "./lib/tui/semaphore.mjs"; import { TuiSemaphore, SemaphoreAbortError, recordTuiEntrypoint, buildTuiHealthBlock } from "./lib/tui/semaphore.mjs";
import { TuiPanePool, resolvePoolSize, POOL_MAX_SIZE } from "./lib/tui/pool.mjs"; import { TuiPanePool, resolvePoolSize, POOL_MAX_SIZE } from "./lib/tui/pool.mjs";
import { TuiDeltaAssembler, DEFAULT_HOLDBACK_CHARS, resolveStreamHoldback } from "./lib/tui/stream.mjs"; import { TuiDeltaAssembler, DEFAULT_HOLDBACK_CHARS } from "./lib/tui/stream.mjs";
import { createSerialMutex, createTtlCache, isTokenExpiring, orderLabelsLastGoodFirst } from "./lib/spawn-auth.mjs"; import { createSerialMutex, createTtlCache, isTokenExpiring, orderLabelsLastGoodFirst } from "./lib/spawn-auth.mjs";
const __dirname = dirname(fileURLToPath(import.meta.url)); const __dirname = dirname(fileURLToPath(import.meta.url));
@@ -97,40 +97,8 @@ function _collectNodeManagerCandidates(home) {
return out; return out;
} }
function _joinIfBase(base, ...parts) {
return base ? join(base, ...parts) : null;
}
function _collectWindowsClaudeCandidates() {
const userProfile = process.env.USERPROFILE || process.env.HOME || "";
const localAppData = process.env.LOCALAPPDATA || "";
return [
_joinIfBase(userProfile, ".local", "bin", "claude.exe"),
_joinIfBase(localAppData, "Microsoft", "WinGet", "Links", "claude.exe"),
_joinIfBase(localAppData, "Microsoft", "WindowsApps", "claude.exe"),
].filter(Boolean);
}
function _isWindowsSpawnableBinary(path) {
return /\.exe$/i.test(path);
}
function _lookupLines(out) {
return out.split(/\r?\n/).map(line => line.trim()).filter(Boolean);
}
function _warnUnspawnableWindowsMatches(lines) {
const unspawnable = lines.filter(p => !/\.exe$/i.test(p));
if (unspawnable.length > 0) {
console.warn(`[init] Ignoring non-exe Windows claude command(s): ${unspawnable.join(", ")}`);
}
}
function resolveClaude() { function resolveClaude() {
const isWin = process.platform === "win32";
if (process.env.CLAUDE_BIN) { if (process.env.CLAUDE_BIN) {
if (isWin && !_isWindowsSpawnableBinary(process.env.CLAUDE_BIN)) {
console.error(
`FATAL: CLAUDE_BIN="${process.env.CLAUDE_BIN}" is not a native Windows executable.\n` +
" Set CLAUDE_BIN to claude.exe; shell shims cannot be spawned without a shell."
);
process.exit(1);
}
try { try {
accessSync(process.env.CLAUDE_BIN, constants.X_OK); accessSync(process.env.CLAUDE_BIN, constants.X_OK);
return process.env.CLAUDE_BIN; return process.env.CLAUDE_BIN;
@@ -140,43 +108,28 @@ function resolveClaude() {
} }
} }
const home = process.env.HOME || process.env.USERPROFILE || ""; const home = process.env.HOME || "";
const candidates = isWin const candidates = [
? _collectWindowsClaudeCandidates() "/opt/homebrew/bin/claude",
: [ "/usr/local/bin/claude",
"/opt/homebrew/bin/claude", "/usr/bin/claude",
"/usr/local/bin/claude", join(home, ".local/bin/claude"),
"/usr/bin/claude", ..._collectNodeManagerCandidates(home),
join(home, ".local/bin/claude"), ];
..._collectNodeManagerCandidates(home),
];
for (const p of candidates) { for (const p of candidates) {
try { accessSync(p, constants.X_OK); console.warn(`[init] CLAUDE_BIN not set, resolved to ${p}`); return p; } catch {} try { accessSync(p, constants.X_OK); console.warn(`[init] CLAUDE_BIN not set, resolved to ${p}`); return p; } catch {}
} }
if (isWin) { try {
try { const resolved = execFileSync("which", ["claude"], { encoding: "utf8", timeout: 5000 }).trim();
const lines = _lookupLines(execFileSync("where.exe", ["claude"], { encoding: "utf8", timeout: 5000 })); if (resolved) { console.warn(`[init] CLAUDE_BIN not set, resolved via which: ${resolved}`); return resolved; }
const resolved = lines.find(_isWindowsSpawnableBinary); } catch {}
if (resolved) { console.warn(`[init] CLAUDE_BIN not set, resolved via where.exe: ${resolved}`); return resolved; }
_warnUnspawnableWindowsMatches(lines);
} catch {}
} else {
try {
const resolved = execFileSync("which", ["claude"], { encoding: "utf8", timeout: 5000 }).trim();
if (resolved) { console.warn(`[init] CLAUDE_BIN not set, resolved via which: ${resolved}`); return resolved; }
} catch {}
}
console.error( console.error(
"FATAL: claude binary not found.\n" + "FATAL: claude binary not found.\n" +
(isWin " Set CLAUDE_BIN=/path/to/claude or ensure claude is in PATH.\n" +
? " Set CLAUDE_BIN to the absolute path of claude.exe or ensure claude.exe is in PATH.\n" + " Hint: if you use nvm/fnm/asdf, set CLAUDE_BIN to the absolute path\n" +
" Hint: npm .cmd/.bat/.ps1 shims cannot be spawned without a shell.\n" + " shown by `which claude` in your interactive shell.\n" +
" The .exe requirement is an intentional allow-list for shell-less spawning.\n"
: " Set CLAUDE_BIN=/path/to/claude or ensure claude is in PATH.\n" +
" Hint: if you use nvm/fnm/asdf, set CLAUDE_BIN to the absolute path\n" +
" shown by `which claude` in your interactive shell.\n") +
" Checked: " + candidates.join(", ") " Checked: " + candidates.join(", ")
); );
process.exit(1); process.exit(1);
@@ -426,20 +379,7 @@ const TUI_STREAM_DIR = process.env.OCP_TUI_STREAM_DIR || `${process.env.HOME}/.o
// exceeds this, which puts it out of the default banner detector's <=100-char reach — the // exceeds this, which puts it out of the default banner detector's <=100-char reach — the
// FIRST of the two halves of the guarantee (see the assembler's class comment for the second: // FIRST of the two halves of the guarantee (see the assembler's class comment for the second:
// no further emission at all once a message boundary follows an emit). Only raise it. // no further emission at all once a message boundary follows an emit). Only raise it.
// resolveStreamHoldback enforces the DEFAULT_HOLDBACK_CHARS floor: the "Only raise it" comment const TUI_STREAM_HOLDBACK = parseInt(process.env.OCP_TUI_STREAM_HOLDBACK || String(DEFAULT_HOLDBACK_CHARS), 10);
// above is now load-bearing, not advisory. A sub-floor value (or garbage) is clamped UP to the
// floor and reported via `_holdback.clamped`, because a holdback below the default banner
// detector's 100-char reach would let the first chars of a real auth banner stream before the
// end-of-turn gate rejects the turn (the A1 leak). We can only ever raise the guarantee, never
// weaken it below the detector's bound.
const _holdback = resolveStreamHoldback(process.env.OCP_TUI_STREAM_HOLDBACK);
const TUI_STREAM_HOLDBACK = _holdback.value;
if (TUI_MODE && TUI_STREAM && _holdback.clamped) {
console.error(
`[tui] WARNING: OCP_TUI_STREAM_HOLDBACK=${JSON.stringify(process.env.OCP_TUI_STREAM_HOLDBACK)} is below the\n` +
` safe floor (${DEFAULT_HOLDBACK_CHARS}) or not a number; clamped up to ${DEFAULT_HOLDBACK_CHARS}. The holdback can only be raised.`
);
}
if (TUI_MODE && TUI_STREAM && process.env.CLAUDE_TUI_ERROR_PATTERNS != null && TUI_STREAM_HOLDBACK <= DEFAULT_HOLDBACK_CHARS) { if (TUI_MODE && TUI_STREAM && process.env.CLAUDE_TUI_ERROR_PATTERNS != null && TUI_STREAM_HOLDBACK <= DEFAULT_HOLDBACK_CHARS) {
// The holdback's FIRST-MESSAGE half (see TuiDeltaAssembler) is sound for the DEFAULT // The holdback's FIRST-MESSAGE half (see TuiDeltaAssembler) is sound for the DEFAULT
// auth-banner detector (which cannot match a message longer than 100 chars). An // auth-banner detector (which cannot match a message longer than 100 chars). An
@@ -534,12 +474,7 @@ const SPAWN_HOME_DIR = `${process.env.HOME}/.ocp/spawn-home`;
// erroring loudly — never a silent auth/credential corruption (there are no credentials here). // erroring loudly — never a silent auth/credential corruption (there are no credentials here).
function prepareSpawnHome(dir = SPAWN_HOME_DIR) { function prepareSpawnHome(dir = SPAWN_HOME_DIR) {
try { try {
// mode 0700, and it matters for the PARENT: with `recursive`, this call can create ~/.ocp mkdirSync(`${dir}/.claude`, { recursive: true });
// itself on a fresh install (spawn homes live under it), and without an explicit mode that
// parent lands at the umask default — world-listable 0755. keys.mjs used to pre-create it
// 0700 as an import side effect; it no longer does (it resolves its dir lazily), so the
// 0700 guarantee has to be stated here rather than inherited by luck.
mkdirSync(`${dir}/.claude`, { recursive: true, mode: 0o700 });
// Belt-and-braces: ensure no settings.json/plugins leak in (this home is fully ours). // Belt-and-braces: ensure no settings.json/plugins leak in (this home is fully ours).
for (const f of [`${dir}/.claude/settings.json`, `${dir}/.claude/settings.local.json`]) { for (const f of [`${dir}/.claude/settings.json`, `${dir}/.claude/settings.local.json`]) {
try { if (existsSync(f)) rmSync(f, { force: true }); } catch { /* best effort */ } try { if (existsSync(f)) rmSync(f, { force: true }); } catch { /* best effort */ }
@@ -1554,17 +1489,6 @@ async function callClaudeTui(model, messages, _conversationId, _keyName, res, st
streamDir: TUI_STREAM ? TUI_STREAM_DIR : null, streamDir: TUI_STREAM ? TUI_STREAM_DIR : null,
abortSignal: streamCtx ? streamCtx.signal : null, abortSignal: streamCtx ? streamCtx.signal : null,
}); });
// ── Billing-pool observation (issue #115, #133) — A3 fix: record the entrypoint the moment
// runTuiTurn returns, BEFORE the honesty gates below that can throw. The entrypoint (cli vs
// sdk-cli) is which BILLING POOL the turn consumed; a turn that then fails a gate (wall-clock
// truncation, auth banner, stream divergence) STILL spent that pool — and those failed turns
// are exactly the ones most likely to signal a silent degrade to the metered Agent SDK pool.
// Recording only on the success path (the old placement) blinded /health's entrypointMismatches
// and lastEntrypoint to every failed turn. recordModelSuccess still runs later, only on success.
if (recordTuiEntrypoint(tuiStats, entrypoint, TUI_ENTRYPOINT)) {
logEvent("warn", "tui_entrypoint_mismatch", { expected: "cli", got: entrypoint, model: cliModel });
}
// ── Honesty gates (issue #133) ─ run BEFORE recordModelSuccess / cache write-back. // ── Honesty gates (issue #133) ─ run BEFORE recordModelSuccess / cache write-back.
// A throw here propagates to the catch below (recordModelError + reject), so the // A throw here propagates to the catch below (recordModelError + reject), so the
// result never reaches the downstream setCachedResponse / singleflight / SUCCESS path. // result never reaches the downstream setCachedResponse / singleflight / SUCCESS path.
@@ -1642,9 +1566,17 @@ async function callClaudeTui(model, messages, _conversationId, _keyName, res, st
} }
recordModelSuccess(cliModel, 0); // elapsed not measurable here; wallclock at reader level recordModelSuccess(cliModel, 0); // elapsed not measurable here; wallclock at reader level
// Entrypoint/billing-pool observation was already recorded above, right after runTuiTurn // Assert the subscription-pool classification. TUI exists to keep cc_entrypoint=cli
// returned — see the A3-fix comment there (it must cover failed turns too, so it cannot live // (subscription pool); a silent degrade to sdk-cli (metered Agent SDK pool) would still
// on this success-only path). // return text but cost money — warn loudly so it's visible. (issue #115)
// C-5: also surface the observation on /health. recordTuiEntrypoint sets lastEntrypoint
// unconditionally (operators can poll it to confirm cli) and increments
// entrypointMismatches when expected=cli but observed≠cli — the same condition the
// journald warning already covers — so a silent metered-pool drift is visible on /health
// without tailing logs.
if (recordTuiEntrypoint(tuiStats, entrypoint, TUI_ENTRYPOINT)) {
logEvent("warn", "tui_entrypoint_mismatch", { expected: "cli", got: entrypoint, model: cliModel });
}
return text; return text;
} catch (err) { } catch (err) {
// A mid-turn client disconnect (streaming path only — abortSignal) is NOT an upstream // A mid-turn client disconnect (streaming path only — abortSignal) is NOT an upstream
+1 -3
View File
@@ -390,9 +390,7 @@ if (!DRY_RUN) {
// and "ocp-proxy" keeps the proxy invisible to that heuristic. // and "ocp-proxy" keeps the proxy invisible to that heuristic.
const OCP_HOME = join(HOME, ".ocp"); const OCP_HOME = join(HOME, ".ocp");
const ocpLogsDir = join(OCP_HOME, "logs"); const ocpLogsDir = join(OCP_HOME, "logs");
// mode 0700: with `recursive`, this call can create ~/.ocp ITSELF on a fresh install, and if (!existsSync(ocpLogsDir)) mkdirSync(ocpLogsDir, { recursive: true });
// without an explicit mode that parent lands at the umask default (world-listable 0755).
if (!existsSync(ocpLogsDir)) mkdirSync(ocpLogsDir, { recursive: true, mode: 0o700 });
// Uninstall legacy service names if present (upgrade path) // Uninstall legacy service names if present (upgrade path)
if (platform === "darwin") { if (platform === "darwin") {
-26
View File
@@ -1,26 +0,0 @@
// Imported FIRST by test-features.mjs, before keys.mjs, so this runs before anything can open
// the key store. ESM hoists imports and evaluates them in order, so a `process.env.X = ...`
// statement in the test's own body would run too late — hence a separate module.
//
// Why this exists: `npm test` used to write real, UNREVOKED api_keys rows into the operator's
// live ~/.ocp/ocp.db (the same database the running server reads) — two per run, unbounded.
// It also made the suite racy: two concurrent runs (e.g. review worktrees) shared one file, so
// `listKeys()` could miss "test-user-1" and the `in` check would throw on undefined.
import { mkdtempSync, rmSync } from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";
export const TEST_OCP_DIR = mkdtempSync(join(tmpdir(), "ocp-test-"));
// BOTH are required. keys.mjs honors OCP_DIR_OVERRIDE only when NODE_ENV === "test", so neither
// var alone redirects anything — a stray OCP_DIR_OVERRIDE in a production env is inert without
// NODE_ENV=test alongside it. (A daemon OCP launches never carries either: the service units and
// the `ocp` restart fallback strip both — see plist-merge NEVER_PRESERVE / keys.mjs's comment.)
process.env.NODE_ENV = "test";
process.env.OCP_DIR_OVERRIDE = TEST_OCP_DIR;
// Remove the scratch store on exit. Without this the fix would trade unbounded growth in
// ~/.ocp/ocp.db for unbounded growth in $TMPDIR — better, but still litter.
process.on("exit", () => {
try { rmSync(TEST_OCP_DIR, { recursive: true, force: true }); } catch { /* best effort */ }
});
+10 -199
View File
@@ -3,24 +3,22 @@
* Integration test for Quota + Cache features. * Integration test for Quota + Cache features.
* Tests database layer functions directly — no server needed. * Tests database layer functions directly — no server needed.
*/ */
// MUST come before keys.mjs: redirects the key store to a scratch dir (see test-env.mjs). import { getDb, createKey, listKeys, validateKey, recordUsage, checkQuota, updateKeyQuota, getKeyQuota, findKey, cacheHash, getCachedResponse, setCachedResponse, clearCache, getCacheStats, closeDb, hasCacheControl, singleflight, getInflightStats } from "./keys.mjs";
import { TEST_OCP_DIR } from "./test-env.mjs";
import { getDb, getDbPath, createKey, listKeys, validateKey, recordUsage, checkQuota, updateKeyQuota, getKeyQuota, findKey, cacheHash, getCachedResponse, setCachedResponse, clearCache, getCacheStats, closeDb, hasCacheControl, singleflight, getInflightStats } from "./keys.mjs";
import { isLoopbackBind } from "./lib/net.mjs"; import { isLoopbackBind } from "./lib/net.mjs";
import { createSerialMutex, createTtlCache, isTokenExpiring, orderLabelsLastGoodFirst } from "./lib/spawn-auth.mjs"; import { createSerialMutex, createTtlCache, isTokenExpiring, orderLabelsLastGoodFirst } from "./lib/spawn-auth.mjs";
import { createHash } from "node:crypto"; import { createHash } from "node:crypto";
import { strict as assert } from "node:assert"; import { strict as assert } from "node:assert";
import { unlinkSync } from "node:fs";
import { join } from "node:path"; import { join } from "node:path";
import { pathToFileURL } from "node:url";
import { execFileSync } from "node:child_process";
import { homedir } from "node:os"; import { homedir } from "node:os";
process.env.HOME = homedir(); // normalize HOME so homedir()-derived paths are stable across shells // Use a test database to avoid corrupting real data
const TEST_DB = join(homedir(), ".ocp", "ocp-test.db");
try { unlinkSync(TEST_DB); } catch {}
// The scaffolding that used to live here CLAIMED to use "a test database to avoid corrupting // Monkey-patch DB_PATH for testing (override the module-level variable)
// real data" by setting an env var before the first getDb(). It never worked: keys.mjs read no // Since keys.mjs uses lazy init, we can set env before first getDb() call
// env var, and ESM hoisting meant the assignment ran after the import anyway. The redirect is process.env.HOME = homedir(); // ensure consistent
// now real, and lives in test-env.mjs (imported above, before keys.mjs). This test proves it.
let passed = 0; let passed = 0;
let failed = 0; let failed = 0;
@@ -537,7 +535,7 @@ async function runSingleflightTests() {
await runSingleflightTests(); await runSingleflightTests();
// ── Plist Env Merge Tests ── // ── Plist Env Merge Tests ──
import { mergePlistEnv, mergeSystemdEnv, NEVER_PRESERVE } from "./scripts/lib/plist-merge.mjs"; import { mergePlistEnv, mergeSystemdEnv } from "./scripts/lib/plist-merge.mjs";
console.log("\nPlist env merge:"); console.log("\nPlist env merge:");
@@ -651,78 +649,6 @@ test("mergePlistEnv is idempotent", () => {
assert.equal(mergePlistEnv(r1, SAMPLE_TEMPLATE_PLIST), r1); assert.equal(mergePlistEnv(r1, SAMPLE_TEMPLATE_PLIST), r1);
}); });
// ── A4: security denylist — test-only key-store redirection vars must NEVER survive a setup
// re-run, even when a prior unit already carried them. Mutation-proof: drop the
// `!NEVER_PRESERVE.has(k)` guard in either merge fn and these fail (the vars get preserved).
test("NEVER_PRESERVE denylists exactly the two key-store redirection vars", () => {
assert.ok(NEVER_PRESERVE.has("NODE_ENV") && NEVER_PRESERVE.has("OCP_DIR_OVERRIDE"));
assert.equal(NEVER_PRESERVE.size, 2, "exactly two — a new entry needs its own rationale + test");
});
const PLIST_EXISTING_WITH_TEST_VARS = `<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
<dict>
<key>Label</key>
<string>dev.ocp.proxy</string>
<key>EnvironmentVariables</key>
<dict>
<key>CLAUDE_PROXY_PORT</key>
<string>3456</string>
<key>CLAUDE_CACHE_TTL</key>
<string>600</string>
<key>NODE_ENV</key>
<string>test</string>
<key>OCP_DIR_OVERRIDE</key>
<string>/tmp/scratch-store</string>
</dict>
</dict>
</plist>`;
test("mergePlistEnv strips test-only redirection vars (A4) but keeps legit user keys", () => {
const merged = mergePlistEnv(PLIST_EXISTING_WITH_TEST_VARS, SAMPLE_TEMPLATE_PLIST);
assert.match(merged, /<key>CLAUDE_CACHE_TTL<\/key>\s*<string>600<\/string>/, "a legit user key is still preserved");
assert.doesNotMatch(merged, /<key>NODE_ENV<\/key>/, "NODE_ENV must never reach a service unit");
assert.doesNotMatch(merged, /OCP_DIR_OVERRIDE/, "OCP_DIR_OVERRIDE must never reach a service unit (key or value)");
});
test("mergePlistEnv: an existing unit whose ONLY extras are denylisted → template unchanged", () => {
const existing = `<?xml version="1.0" encoding="UTF-8"?>
<plist version="1.0">
<dict>
<key>EnvironmentVariables</key>
<dict>
<key>CLAUDE_PROXY_PORT</key>
<string>3456</string>
<key>NODE_ENV</key>
<string>test</string>
<key>OCP_DIR_OVERRIDE</key>
<string>/tmp/scratch-store</string>
</dict>
</dict>
</plist>`;
assert.equal(mergePlistEnv(existing, SAMPLE_TEMPLATE_PLIST), SAMPLE_TEMPLATE_PLIST, "nothing left to preserve → clean template");
});
const SYSTEMD_EXISTING_WITH_TEST_VARS = `[Unit]
Description=OCP — Open Claude Proxy
[Service]
ExecStart=/usr/bin/node /home/u/ocp/server.mjs
Environment=CLAUDE_PROXY_PORT=3456
Environment=CLAUDE_CACHE_TTL=600
Environment=NODE_ENV=test
Environment=OCP_DIR_OVERRIDE=/tmp/scratch-store
Restart=always
`;
test("mergeSystemdEnv strips test-only redirection vars (A4) but keeps legit user keys", () => {
const merged = mergeSystemdEnv(SYSTEMD_EXISTING_WITH_TEST_VARS, SAMPLE_TEMPLATE_SYSTEMD);
assert.match(merged, /Environment=CLAUDE_CACHE_TTL=600/, "a legit user key is still preserved");
assert.doesNotMatch(merged, /Environment=NODE_ENV=/, "NODE_ENV must never reach a service unit");
assert.doesNotMatch(merged, /OCP_DIR_OVERRIDE/, "OCP_DIR_OVERRIDE must never reach a service unit");
});
test("mergeSystemdEnv is idempotent", () => { test("mergeSystemdEnv is idempotent", () => {
const r1 = mergeSystemdEnv(SAMPLE_EXISTING_SYSTEMD, SAMPLE_TEMPLATE_SYSTEMD); const r1 = mergeSystemdEnv(SAMPLE_EXISTING_SYSTEMD, SAMPLE_TEMPLATE_SYSTEMD);
assert.equal(mergeSystemdEnv(r1, SAMPLE_TEMPLATE_SYSTEMD), r1); assert.equal(mergeSystemdEnv(r1, SAMPLE_TEMPLATE_SYSTEMD), r1);
@@ -3326,31 +3252,6 @@ test("models.json aliases.sonnet === 'claude-sonnet-4-6' (default-request-model
assert.equal(_spotModels.aliases.sonnet, "claude-sonnet-4-6"); assert.equal(_spotModels.aliases.sonnet, "claude-sonnet-4-6");
}); });
// ── Referential integrity (PR #152 review) ──────────────────────────────────
// The value-mirror assertions above only prove the alias equals a string literal —
// they pass even if that literal points at a model that does not exist in
// models[]. A one-line slip (edit an alias, forget the models[] entry) would leave
// /v1/models missing the model while every `model: "<alias>"` request passes
// validation and then fails at CLI spawn. VALID_MODELS keys on alias *names*, so
// nothing else checks alias *targets*. This is the guard with teeth.
const _spotModelIds = new Set(_spotModels.models.map(m => m.id));
test("models.json: claude-sonnet-5 is present in models[] (the entry this PR adds)", () => {
assert.ok(_spotModelIds.has("claude-sonnet-5"), "claude-sonnet-5 must exist as a models[].id");
});
test("models.json: every aliases value resolves to a real models[].id (referential integrity)", () => {
for (const [name, target] of Object.entries(_spotModels.aliases)) {
assert.ok(_spotModelIds.has(target), `aliases.${name} -> '${target}' is a dangling alias (no matching models[].id)`);
}
});
test("models.json: every legacyAliases value resolves to a real models[].id (referential integrity)", () => {
for (const [name, target] of Object.entries(_spotModels.legacyAliases || {})) {
assert.ok(_spotModelIds.has(target), `legacyAliases.${name} -> '${target}' is a dangling alias (no matching models[].id)`);
}
});
// ── escapeHtml + key-name validator (issue #114) ──────────────────────────── // ── escapeHtml + key-name validator (issue #114) ────────────────────────────
// Replicated verbatim from dashboard.html so tests run without a browser. // Replicated verbatim from dashboard.html so tests run without a browser.
function escapeHtml(s) { function escapeHtml(s) {
@@ -3550,7 +3451,7 @@ async function runAsyncTests() {
// ── TUI real streaming: MessageDisplay hook sink (backlog #2) ─────────────── // ── TUI real streaming: MessageDisplay hook sink (backlog #2) ───────────────
// Pure-logic coverage for lib/tui/stream.mjs: sink parsing, the concat===T assertion, // Pure-logic coverage for lib/tui/stream.mjs: sink parsing, the concat===T assertion,
// prefix-stability, the auth-banner holdback, message scoping, and the error paths. // prefix-stability, the auth-banner holdback, message scoping, and the error paths.
import { TuiDeltaAssembler, parseDeltaChunk, buildStreamSettings, streamFilePath, HOOK_SCRIPT, prepareStreamHook, resolveStreamHoldback, DEFAULT_HOLDBACK_CHARS } from "./lib/tui/stream.mjs"; import { TuiDeltaAssembler, parseDeltaChunk, buildStreamSettings, streamFilePath, HOOK_SCRIPT, prepareStreamHook } from "./lib/tui/stream.mjs";
test("stream: parseDeltaChunk consumes only COMPLETE lines (a torn write stays unread)", () => { test("stream: parseDeltaChunk consumes only COMPLETE lines (a torn write stays unread)", () => {
const p = (i, d, final = false) => JSON.stringify({ hook_event_name: "MessageDisplay", session_id: "s", message_id: "m", index: i, final, delta: d }); const p = (i, d, final = false) => JSON.stringify({ hook_event_name: "MessageDisplay", session_id: "s", message_id: "m", index: i, final, delta: d });
@@ -3618,42 +3519,6 @@ test("stream: holdback releases once past the detector's reach, and only then",
assert.equal(a.push(mdFire(2, "tail")), "tail", "subsequent deltas stream straight through"); assert.equal(a.push(mdFire(2, "tail")), "tail", "subsequent deltas stream straight through");
}); });
// ── resolveStreamHoldback: the FLOOR under OCP_TUI_STREAM_HOLDBACK (A1 fix) ────────────
// The C-1 auth-banner guarantee holds only while the holdback >= the default detector's
// 100-char reach. These tests pin that the resolver CLAMPS UP to the floor. They are
// mutation-proof: delete the `parsed < floor` branch and the sub-floor cases below fail
// (a 50 would pass straight through, reopening the leak). The clamped flag drives the boot
// warning in server.mjs, so its truthiness is asserted alongside every value.
test("holdback: a sub-floor value is clamped UP to the floor and flagged", () => {
assert.deepEqual(resolveStreamHoldback("50"), { value: DEFAULT_HOLDBACK_CHARS, clamped: true });
assert.deepEqual(resolveStreamHoldback("0"), { value: DEFAULT_HOLDBACK_CHARS, clamped: true });
assert.deepEqual(resolveStreamHoldback("-5"), { value: DEFAULT_HOLDBACK_CHARS, clamped: true });
assert.deepEqual(resolveStreamHoldback("99"), { value: DEFAULT_HOLDBACK_CHARS, clamped: true });
});
test("holdback: garbage / NaN falls back to the floor and is flagged (not silently 0)", () => {
assert.deepEqual(resolveStreamHoldback("unlimited"), { value: DEFAULT_HOLDBACK_CHARS, clamped: true });
assert.deepEqual(resolveStreamHoldback("5MB"), { value: DEFAULT_HOLDBACK_CHARS, clamped: true });
});
test("holdback: an above-floor value passes through unchanged and is NOT flagged", () => {
assert.deepEqual(resolveStreamHoldback("200"), { value: 200, clamped: false });
assert.deepEqual(resolveStreamHoldback("101"), { value: 101, clamped: false });
assert.deepEqual(resolveStreamHoldback(String(DEFAULT_HOLDBACK_CHARS)), { value: DEFAULT_HOLDBACK_CHARS, clamped: false });
});
test("holdback: an unset env var takes the floor WITHOUT flagging (no spurious boot warning)", () => {
assert.deepEqual(resolveStreamHoldback(undefined), { value: DEFAULT_HOLDBACK_CHARS, clamped: false });
assert.deepEqual(resolveStreamHoldback(null), { value: DEFAULT_HOLDBACK_CHARS, clamped: false });
assert.deepEqual(resolveStreamHoldback(""), { value: DEFAULT_HOLDBACK_CHARS, clamped: false });
assert.deepEqual(resolveStreamHoldback(" "), { value: DEFAULT_HOLDBACK_CHARS, clamped: false });
});
test("holdback: the floor is a parameter, so a deployment can raise (never lower) it", () => {
assert.deepEqual(resolveStreamHoldback("150", 200), { value: 200, clamped: true }, "custom floor still clamps up");
assert.deepEqual(resolveStreamHoldback("300", 200), { value: 300, clamped: false });
});
test("stream: a short answer never passes the holdback and is delivered whole at terminal", () => { test("stream: a short answer never passes the holdback and is delivered whole at terminal", () => {
const T = "The capital of France is Paris."; const T = "The capital of France is Paris.";
const a = new TuiDeltaAssembler(); const a = new TuiDeltaAssembler();
@@ -3946,60 +3811,6 @@ test("REGRESSION: a WARM (pooled) pane streams — the sink comes off the pane,
assert.equal(out.text, "Hello world", "and the transcript stays authoritative for the final text"); assert.equal(out.text, "Hello world", "and the transcript stays authoritative for the final text");
}); });
console.log("\nTest isolation (the suite must never touch the operator's live key store):");
test("the key store under test is a scratch db, NOT the operator's real ~/.ocp/ocp.db", () => {
// The guard that was missing. `npm test` wrote live, UNREVOKED api_keys rows straight into the
// operator's real ~/.ocp/ocp.db — the same database the running server reads — two per run,
// unbounded (737 junk keys vs 12 real ones on the maintainer's host before this landed). It
// went unnoticed for so long precisely because NOTHING asserted where the store actually was.
const real = join(homedir(), ".ocp", "ocp.db");
const used = getDbPath();
assert.ok(used, "getDb() must have opened something by now");
assert.notEqual(used, real, "the suite must NOT open the operator's live key database");
assert.ok(used.startsWith(TEST_OCP_DIR), `expected a scratch db under ${TEST_OCP_DIR}, got ${used}`);
});
test("a PRODUCTION process (no NODE_ENV) must IGNORE OCP_DIR_OVERRIDE", () => {
// Must run OUT OF PROCESS. The parent is irreversibly NODE_ENV=test by the time any test runs
// (test-env.mjs set it before keys.mjs was imported), so the production path is unreachable
// from in here — and an in-process test can only ever RE-IMPLEMENT the predicate, which is
// worthless: the first cut of this test did exactly that, and deleting the whole NODE_ENV gate
// from keys.mjs still left the suite at 320 passed / 0 failed. A copy of the predicate is not
// the predicate. So: spawn a child with no NODE_ENV, the override set, and HOME redirected to
// a temp dir (so the real key store is never opened), and assert what the REAL keys.mjs did.
const home = mkdtempSync(join(tmpdir(), "ocp-prodsim-"));
const evil = mkdtempSync(join(tmpdir(), "ocp-evil-"));
try {
const keysUrl = pathToFileURL(join(import.meta.dirname, "keys.mjs")).href;
// The child prints the override it SAW, then the store it actually opened. Printing both is
// the negative control: without it, a future refactor that renamed the env var and missed
// this test's `env` object would leave the child with no override at all — and "prod opened
// the right store" would pass for the wrong reason. Asserting the child saw it and ignored
// it anyway is the claim we actually want to make.
const probe = `import { getDb, getDbPath, closeDb } from ${JSON.stringify(keysUrl)};
getDb(); process.stdout.write(process.env.OCP_DIR_OVERRIDE + "\\n" + getDbPath()); closeDb();`;
const env = { ...process.env, HOME: home, OCP_DIR_OVERRIDE: evil };
delete env.NODE_ENV; // a production server has no NODE_ENV
const [seen, opened] = execFileSync(process.execPath, ["--input-type=module", "-e", probe],
{ env, encoding: "utf8" }).trim().split("\n");
assert.equal(seen, evil, "precondition: the child must actually SEE the override");
assert.equal(opened, join(home, ".ocp", "ocp.db"), "a prod process must open HOME/.ocp/ocp.db");
assert.ok(!opened.startsWith(evil), "…having seen the override, a prod process must IGNORE it");
} finally {
rmSync(home, { recursive: true, force: true });
rmSync(evil, { recursive: true, force: true });
}
});
test("listKeys does not depend on rows left behind by an earlier or concurrent run", () => {
// The ~1-in-6 flake: two runs sharing one db file. keys.find() returned undefined and the
// caller's `in` check threw a TypeError instead of failing cleanly. With a per-run scratch db
// the store starts empty, so the count is exactly what THIS run created.
const mine = listKeys().filter((k) => k.name === "test-user-1");
assert.equal(mine.length, 1, "exactly one test-user-1 — a shared store would accumulate duplicates");
});
runAsyncTests().then(() => Promise.all(pendingAsync)).then(() => { runAsyncTests().then(() => Promise.all(pendingAsync)).then(() => {
closeDb(); closeDb();
console.log(`\n=== Results: ${passed} passed, ${failed} failed ===\n`); console.log(`\n=== Results: ${passed} passed, ${failed} failed ===\n`);