fix(setup): plist-merge code-quality follow-up

Three issues raised by the code-quality reviewer on 0fd1838:

1. mergeSystemdEnv now early-returns when the template has no
   Environment= anchor, instead of silently dropping preserved lines
   (defensive guard for a future template change).
2. Both parsers (parsePlistEnv, parseSystemdEnv) now Buffer-normalise
   their input, removing the TypeError-vs-coerce asymmetry.
3. Two idempotency tests added (calling merge twice on the same
   input/output pair must be stable).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

CHANGELOG.md

@@ -1,5 +1,31 @@
 # Changelog
 
+## v3.14.0 — 2026-05-10
+
+### Features (security hardening)
+
+- **Per-key session isolation** (PR #86, S1) — the `sessions` Map in `server.mjs` is now keyed by `${keyName}|${conversationId}` instead of bare `conversationId`. Before this fix, two clients using distinct API keys but the same `session_id` value (e.g. both defaulting to `"default"`) would share the same `cli.js` subprocess and conversation history, creating a cross-tenant leak path. Post-fix each (key, session) pair is isolated end-to-end, extending the per-key cache isolation shipped in v3.13.0 D1 to the session layer.
+- **On-disk credential file modes 0700/0600** (PR #87, S2) — `setup.mjs` now creates `~/.ocp` at mode 0700 and both `admin-key` and `ocp.db` at mode 0600. An idempotent `reconcileFileModes()` call in `server.mjs` startup tightens any existing installation to these modes automatically on every launch, so existing prod boxes fix themselves without manual `chmod`. Before this fix, all three files were created at the process's default umask (typically world-readable 0644 / 0755), leaving plaintext credentials readable by other local users.
+- **`/api/usage` default scope = self; admin all-keys requires `?all=true`** (PR #88, S3) — the usage endpoint now applies a least-privilege default: anonymous callers receive only their own rows, non-admin authenticated callers receive only their own rows, and admin callers receive only their own rows unless they explicitly pass `?all=true`. When `?all=true` is used, an audit log line is emitted. Before this fix, any admin-token holder could silently enumerate usage data for every key on the server.
+
+### Behavior changes
+
+- **Breaking change for admin tooling**: `/api/usage` no longer returns all-keys data by default. Existing cron jobs, dashboards, or scripts that rely on the admin token seeing all-keys output must add `?all=true` to their request URL after upgrading to v3.14.0.
+- **File mode reconcile at server startup** logs a one-line notice per path when mode is tightened (e.g. `[security] tightened ~/.ocp/ocp.db → 0600`). No action is required from the operator; the reconcile is idempotent and silent when modes are already correct.
+- **`sessions` Map key is now `${keyName}|${conversationId}` internally.** No client-visible wire change — the `session_id` field in request/response is unchanged.
+
+### Verification
+
+- Stress-test pass: 11/11 phases including S1/S2/S3 security regression checks (Phase E, I, J). 35-minute sustained run, 60 calls, 0 errors, 0 timeouts. RSS dropped 51→47 MB across the window. Per-key cache isolation, singleflight, cache_control bypass, quota enforcement, file-mode reconcile, and scope guard against escalation all verified against running code.
+
+### Governance
+
+- All three PRs (#86, #87, #88) include the explicit `cli.js`-citation-not-applicable disclaimer (per PR #75 pattern) since they are OCP-internal access-control, session-state, and file-permission changes with no corresponding `cli.js` operation to cite.
+
+### No new env vars / no public API surface change beyond the documented breaking change
+
+This release adds no new env vars or endpoints. The only externally visible change is the `/api/usage` scope guard (breaking for admin all-keys consumers; see Behavior changes above).
+
 ## v3.13.0 — 2026-05-07
 
 ### Features (cache layer hardening)

README.md

@@ -392,6 +392,8 @@ ocp keys revoke son-ipad   # Revoke a key
 | `shared` | `CLAUDE_AUTH_MODE=shared` + `PROXY_API_KEY=xxx` | Everyone shares one key |
 | `multi` | `CLAUDE_AUTH_MODE=multi` + `OCP_ADMIN_KEY=xxx` | Per-person keys with usage tracking (recommended) |
 
+> **Usage scope (v3.14.0+):** `/api/usage` returns the caller's own rows by default. Admin callers must pass `?all=true` to retrieve data for all keys; doing so emits an audit log line.
+
 ### Anonymous Access (optional)
 
 In `multi` mode, the admin can designate a single well-known "anonymous" key that bypasses `validateKey()` and grants public read/write access. This is useful for letting LAN users (or clients like OpenClaw multi-agent setups) connect without individual per-user keys.
@@ -460,6 +462,7 @@ When a key exceeds its quota, OCP returns HTTP 429 with a structured error:
 - Keys are stored in `~/.ocp/ocp.db` (SQLite, zero external dependencies)
 - Admin key is required for key management API endpoints
 - The dashboard (`/dashboard`) and health check (`/health`) are always public
+- File modes for `~/.ocp` (0700), `admin-key` + `ocp.db` (0600) are auto-tightened at server startup as of v3.14.0
 
 ## Built-in Usage Monitoring
 
@@ -657,7 +660,7 @@ The canonical list lives in [`models.json`](./models.json) — the single source
 | `/api/keys` | GET/POST | List or create API keys (admin only) |
 | `/api/keys/:id` | DELETE | Revoke an API key (admin only) |
 | `/api/keys/:id/quota` | GET/PATCH | View or set per-key quota (admin only) |
-| `/api/usage` | GET | Per-key usage stats (`?since=&until=&hours=&limit=`) |
+| `/api/usage` | GET | Per-key usage stats (`?since=&until=&hours=&limit=`); returns self only by default — pass `?all=true` (admin only) for all-keys data |
 | `/cache/stats` | GET | Cache statistics (admin only) |
 | `/cache` | DELETE | Clear response cache (admin only) |
 

package.json

@@ -1,6 +1,6 @@
 {
   "name": "open-claude-proxy",
-  "version": "3.13.0",
+  "version": "3.14.0",
   "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",
   "bin": {

scripts/lib/plist-merge.mjs

@@ -0,0 +1,86 @@
+// scripts/lib/plist-merge.mjs
+//
+// Preserves user-customised env vars when setup.mjs rewrites the unit file.
+//
+// Rule:
+//   - keys present in NEW template → template value wins (template is source of truth)
+//   - keys ONLY in EXISTING (not in template) → preserved verbatim
+//
+// No new dependencies — regex-based, plist <key>X</key><string>Y</string> shape
+// is stable enough for our hand-written templates in setup.mjs.
+
+const PLIST_KV_RE = /<key>([^<]+)<\/key>\s*<string>([^<]*)<\/string>/g;
+
+export function parsePlistEnv(plistContent) {
+  if (!plistContent) return {};
+  if (Buffer.isBuffer(plistContent)) plistContent = plistContent.toString("utf8");
+  // Restrict to the EnvironmentVariables dict to avoid catching Label, etc.
+  const envBlock = plistContent.match(/<key>EnvironmentVariables<\/key>\s*<dict>([\s\S]*?)<\/dict>/);
+  if (!envBlock) return {};
+  const out = {};
+  let m;
+  PLIST_KV_RE.lastIndex = 0;
+  while ((m = PLIST_KV_RE.exec(envBlock[1])) !== null) {
+    out[m[1]] = m[2];
+  }
+  return out;
+}
+
+export function mergePlistEnv(existing, template) {
+  if (!existing) return template;
+  const existingEnv = parsePlistEnv(existing);
+  const templateEnv = parsePlistEnv(template);
+  const KNOWN = new Set(Object.keys(templateEnv));
+
+  const preserved = {};
+  for (const [k, v] of Object.entries(existingEnv)) {
+    if (!KNOWN.has(k)) preserved[k] = v;
+  }
+  if (Object.keys(preserved).length === 0) return template;
+
+  const lines = Object.entries(preserved)
+    .map(([k, v]) => `    <key>${k}</key>\n    <string>${v}</string>`)
+    .join("\n");
+
+  // Inject before the closing </dict> of EnvironmentVariables
+  return template.replace(
+    /(<key>EnvironmentVariables<\/key>\s*<dict>[\s\S]*?)(\n\s*<\/dict>)/,
+    `$1\n${lines}$2`
+  );
+}
+
+const SYSTEMD_KV_RE = /^Environment=([^=]+)=(.*)$/gm;
+
+export function parseSystemdEnv(serviceContent) {
+  if (!serviceContent) return {};
+  if (Buffer.isBuffer(serviceContent)) serviceContent = serviceContent.toString("utf8");
+  const out = {};
+  let m;
+  SYSTEMD_KV_RE.lastIndex = 0;
+  while ((m = SYSTEMD_KV_RE.exec(serviceContent)) !== null) {
+    out[m[1]] = m[2];
+  }
+  return out;
+}
+
+export function mergeSystemdEnv(existing, template) {
+  if (!existing) return template;
+  const existingEnv = parseSystemdEnv(existing);
+  const templateEnv = parseSystemdEnv(template);
+  const KNOWN = new Set(Object.keys(templateEnv));
+
+  const preservedLines = Object.entries(existingEnv)
+    .filter(([k]) => !KNOWN.has(k))
+    .map(([k, v]) => `Environment=${k}=${v}`);
+  if (preservedLines.length === 0) return template;
+
+  // Guard: if template has no Environment= anchor, cannot inject — return template as-is.
+  // (In practice the OCP systemd template always has Environment= lines.)
+  if (!/^Environment=/m.test(template)) return template;
+
+  // Inject after the last existing Environment= line in the template
+  return template.replace(
+    /(^Environment=[^\n]+\n)((?!Environment=).*$)/ms,
+    `$1${preservedLines.join("\n")}\n$2`
+  );
+}

setup.mjs

@@ -13,6 +13,7 @@
  *   5. Optionally starts the proxy
  */
 import { readFileSync, writeFileSync, existsSync, mkdirSync, unlinkSync, readdirSync, chmodSync } from "node:fs";
+import { mergePlistEnv, mergeSystemdEnv } from "./scripts/lib/plist-merge.mjs";
 import { execSync } from "node:child_process";
 import { join, dirname } from "node:path";
 import { homedir } from "node:os";
@@ -424,9 +425,15 @@ if (!DRY_RUN) {
 </plist>
 `;
 
-    writeFileSync(plistPath, plistXml);
+    const existingPlist = existsSync(plistPath) ? readFileSync(plistPath, "utf8") : null;
+    const finalPlistXml = mergePlistEnv(existingPlist, plistXml);
+    writeFileSync(plistPath, finalPlistXml);
     chmodSync(plistPath, 0o600);
-    log(`Plist written: ${plistPath} (mode 600)`);
+    if (existingPlist && finalPlistXml !== plistXml) {
+      log(`Plist written: ${plistPath} (mode 600, preserved user env vars)`);
+    } else {
+      log(`Plist written: ${plistPath} (mode 600)`);
+    }
 
     // Bootout first (in case it was already loaded) then bootstrap
     try { execSync(`launchctl bootout gui/$(id -u) "${plistPath}" 2>/dev/null`); } catch { /* ignore */ }
@@ -459,9 +466,15 @@ StandardError=append:${logPath}
 WantedBy=default.target
 `;
 
-    writeFileSync(servicePath, serviceUnit);
+    const existingService = existsSync(servicePath) ? readFileSync(servicePath, "utf8") : null;
+    const finalServiceUnit = mergeSystemdEnv(existingService, serviceUnit);
+    writeFileSync(servicePath, finalServiceUnit);
     chmodSync(servicePath, 0o600);
-    log(`Service file written: ${servicePath} (mode 600)`);
+    if (existingService && finalServiceUnit !== serviceUnit) {
+      log(`Service file written: ${servicePath} (mode 600, preserved user env vars)`);
+    } else {
+      log(`Service file written: ${servicePath} (mode 600)`);
+    }
 
     execSync(`systemctl --user daemon-reload`);
     execSync(`systemctl --user enable ocp-proxy`);

test-features.mjs

@@ -454,6 +454,126 @@ async function runSingleflightTests() {
 
 await runSingleflightTests();
 
+// ── Plist Env Merge Tests ──
+import { mergePlistEnv, mergeSystemdEnv } from "./scripts/lib/plist-merge.mjs";
+
+console.log("\nPlist env merge:");
+
+const SAMPLE_TEMPLATE_PLIST = `<?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>3478</string>
+    <key>CLAUDE_BIND</key>
+    <string>127.0.0.1</string>
+    <key>CLAUDE_AUTH_MODE</key>
+    <string>multi</string>
+  </dict>
+</dict>
+</plist>`;
+
+const SAMPLE_EXISTING_PLIST = `<?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_BIND</key>
+    <string>127.0.0.1</string>
+    <key>CLAUDE_AUTH_MODE</key>
+    <string>none</string>
+    <key>CLAUDE_HEARTBEAT_INTERVAL</key>
+    <string>2000</string>
+    <key>CLAUDE_CACHE_TTL</key>
+    <string>600</string>
+  </dict>
+</dict>
+</plist>`;
+
+test("mergePlistEnv preserves unknown user keys", () => {
+  const merged = mergePlistEnv(SAMPLE_EXISTING_PLIST, SAMPLE_TEMPLATE_PLIST);
+  assert.match(merged, /<key>CLAUDE_HEARTBEAT_INTERVAL<\/key>\s*<string>2000<\/string>/);
+  assert.match(merged, /<key>CLAUDE_CACHE_TTL<\/key>\s*<string>600<\/string>/);
+});
+
+test("mergePlistEnv overrides known template keys", () => {
+  const merged = mergePlistEnv(SAMPLE_EXISTING_PLIST, SAMPLE_TEMPLATE_PLIST);
+  assert.match(merged, /<key>CLAUDE_PROXY_PORT<\/key>\s*<string>3478<\/string>/);
+  assert.match(merged, /<key>CLAUDE_AUTH_MODE<\/key>\s*<string>multi<\/string>/);
+});
+
+test("mergePlistEnv first-install returns template unchanged when existing is null", () => {
+  const merged = mergePlistEnv(null, SAMPLE_TEMPLATE_PLIST);
+  assert.equal(merged, SAMPLE_TEMPLATE_PLIST);
+});
+
+test("mergePlistEnv first-install returns template unchanged when existing is empty", () => {
+  const merged = mergePlistEnv("", SAMPLE_TEMPLATE_PLIST);
+  assert.equal(merged, SAMPLE_TEMPLATE_PLIST);
+});
+
+const SAMPLE_TEMPLATE_SYSTEMD = `[Unit]
+Description=OCP — Open Claude Proxy
+After=network.target
+
+[Service]
+ExecStart=/usr/bin/node /home/u/ocp/server.mjs
+Environment=CLAUDE_PROXY_PORT=3478
+Environment=CLAUDE_BIND=127.0.0.1
+Environment=CLAUDE_AUTH_MODE=multi
+Restart=always
+`;
+
+const SAMPLE_EXISTING_SYSTEMD = `[Unit]
+Description=OCP — Open Claude Proxy
+After=network.target
+
+[Service]
+ExecStart=/usr/bin/node /home/u/ocp/server.mjs
+Environment=CLAUDE_PROXY_PORT=3456
+Environment=CLAUDE_BIND=127.0.0.1
+Environment=CLAUDE_AUTH_MODE=none
+Environment=CLAUDE_HEARTBEAT_INTERVAL=2000
+Environment=CLAUDE_CACHE_TTL=600
+Restart=always
+`;
+
+test("mergeSystemdEnv preserves unknown user Environment lines", () => {
+  const merged = mergeSystemdEnv(SAMPLE_EXISTING_SYSTEMD, SAMPLE_TEMPLATE_SYSTEMD);
+  assert.match(merged, /Environment=CLAUDE_HEARTBEAT_INTERVAL=2000/);
+  assert.match(merged, /Environment=CLAUDE_CACHE_TTL=600/);
+});
+
+test("mergeSystemdEnv overrides known template keys", () => {
+  const merged = mergeSystemdEnv(SAMPLE_EXISTING_SYSTEMD, SAMPLE_TEMPLATE_SYSTEMD);
+  assert.match(merged, /Environment=CLAUDE_PROXY_PORT=3478/);
+  assert.match(merged, /Environment=CLAUDE_AUTH_MODE=multi/);
+});
+
+test("mergeSystemdEnv first-install returns template unchanged", () => {
+  assert.equal(mergeSystemdEnv(null, SAMPLE_TEMPLATE_SYSTEMD), SAMPLE_TEMPLATE_SYSTEMD);
+  assert.equal(mergeSystemdEnv("", SAMPLE_TEMPLATE_SYSTEMD), SAMPLE_TEMPLATE_SYSTEMD);
+});
+
+test("mergePlistEnv is idempotent", () => {
+  const r1 = mergePlistEnv(SAMPLE_EXISTING_PLIST, SAMPLE_TEMPLATE_PLIST);
+  assert.equal(mergePlistEnv(r1, SAMPLE_TEMPLATE_PLIST), r1);
+});
+
+test("mergeSystemdEnv is idempotent", () => {
+  const r1 = mergeSystemdEnv(SAMPLE_EXISTING_SYSTEMD, SAMPLE_TEMPLATE_SYSTEMD);
+  assert.equal(mergeSystemdEnv(r1, SAMPLE_TEMPLATE_SYSTEMD), r1);
+});
+
 // ── Cleanup ──
 closeDb();