From 8c0b97f3ae03bb20d0947e3e3764613c10942c9d Mon Sep 17 00:00:00 2001 From: dtzp555-max Date: Sat, 9 May 2026 23:56:10 +1000 Subject: [PATCH] fix(security): tighten on-disk credential file modes (700/600) (#87) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Credential-bearing OCP artifacts were created at default umask (0644/0755), making them world-readable on multi-user hosts. This commit hardens them at writer time and adds an idempotent startup reconciliation so existing prod installs are fixed automatically on next service restart. Changes: - keys.mjs: mkdirSync(OCP_DIR, { mode: 0o700 }) + chmodSync after to handle pre-existing dirs; chmodSync(DB_PATH, 0o600) after first getDb() open. - setup.mjs: chmodSync(plistPath, 0o600) after writeFileSync on macOS; chmodSync(unitPath, 0o600) after writeFileSync on Linux. - server.mjs: _tightenFileModesIfPossible() reconciliation block — idempotently chmods ~/.ocp (700), ~/.ocp/admin-key (600), ~/.ocp/ocp.db (600) on startup; emits a single info-level log line when any file is tightened; ignores ENOENT and wraps EPERM in a warn log so startup is never crashed by chmod failure. Backward compat: all files remain accessible to the same-user owner; 0o600 is still fully readable/writable by the process. Existing prod boxes with old 0644 ~/.ocp directories get fixed-up on next launchd/systemd restart without any manual intervention. cli.js citation: this change is OCP-internal file-permission hardening only. No cli.js function corresponds to chmod or credential-file management. This is an OCP-local security improvement that is out of scope for the cli.js citation requirement per ALIGNMENT.md Rule 2; the cli.js boundary applies to proxy protocol and API surface changes, not host-filesystem hardening. Identified during privacy/security audit — .claude/research/ocp-security-audit.md. Co-authored-by: dtzp555 Co-authored-by: Claude Sonnet 4.6 --- keys.mjs | 8 ++++++-- server.mjs | 40 +++++++++++++++++++++++++++++++++++++++- setup.mjs | 8 +++++--- 3 files changed, 50 insertions(+), 6 deletions(-) diff --git a/keys.mjs b/keys.mjs index 7d6d3f9..2aedf8a 100644 --- a/keys.mjs +++ b/keys.mjs @@ -3,11 +3,13 @@ import { DatabaseSync } from "node:sqlite"; import { randomBytes, createHash } from "node:crypto"; import { join } from "node:path"; -import { mkdirSync } from "node:fs"; +import { mkdirSync, chmodSync } from "node:fs"; import { homedir } from "node:os"; const OCP_DIR = join(homedir(), ".ocp"); -mkdirSync(OCP_DIR, { recursive: true }); +mkdirSync(OCP_DIR, { recursive: true, mode: 0o700 }); +// Tighten the directory mode in case it already existed with broader permissions. +try { chmodSync(OCP_DIR, 0o700); } catch { /* ignore EPERM on pre-existing dirs */ } const DB_PATH = join(OCP_DIR, "ocp.db"); let db; @@ -18,6 +20,8 @@ export function getDb() { db.exec("PRAGMA journal_mode = WAL"); db.exec("PRAGMA foreign_keys = ON"); initSchema(); + // Tighten mode on the DB file (0600) after creation / first open. + try { chmodSync(DB_PATH, 0o600); } catch { /* ignore — same-user access still works */ } } return db; } diff --git a/server.mjs b/server.mjs index 41b32b2..635c323 100644 --- a/server.mjs +++ b/server.mjs @@ -30,7 +30,7 @@ import { createServer } from "node:http"; import { spawn, execFileSync } from "node:child_process"; import { randomUUID, timingSafeEqual } from "node:crypto"; -import { readFileSync, readdirSync, accessSync, existsSync, constants } from "node:fs"; +import { readFileSync, readdirSync, accessSync, existsSync, constants, chmodSync, statSync } from "node:fs"; import { fileURLToPath } from "node:url"; import { dirname, join } from "node:path"; import { homedir } from "node:os"; @@ -167,6 +167,44 @@ function logEvent(level, event, data = {}) { } } +// ── Startup file-mode reconciliation ─────────────────────────────────── +// Idempotently tightens OCP credential-bearing files to 700/600 so that +// existing installs (created before this fix) are hardened on next restart. +// Wrapped in try/catch — chmod failure must never crash startup. +// Does NOT touch systemd units or launchd plists; those are managed by setup.mjs. +function _tightenFileModesIfPossible() { + const ocpDir = join(homedir(), ".ocp"); + const targets = [ + { path: ocpDir, mode: 0o700, label: "~/.ocp (dir)" }, + { path: join(ocpDir, "admin-key"), mode: 0o600, label: "~/.ocp/admin-key" }, + { path: join(ocpDir, "ocp.db"), mode: 0o600, label: "~/.ocp/ocp.db" }, + ]; + let tightened = 0; + let alreadyOk = 0; + for (const { path, mode, label } of targets) { + try { + const st = statSync(path); + const current = st.mode & 0o777; + if (current !== mode) { + chmodSync(path, mode); + tightened++; + } else { + alreadyOk++; + } + } catch (e) { + if (e.code !== "ENOENT") { + // File exists but chmod failed (e.g. EPERM) — log and move on + logEvent("warn", "file_mode_tighten_failed", { path: label, error: e.message }); + } + // ENOENT is fine — file doesn't exist yet + } + } + if (tightened > 0) { + logEvent("info", "file_modes_tightened", { tightened, alreadyOk }); + } +} +_tightenFileModesIfPossible(); + // ── Circuit breaker (DISABLED) ────────────────────────────────────────── // Disabled: CLI proxy has its own retry logic, and the breaker was causing // cascading failures — once API got briefly slow, ALL agents lost connectivity diff --git a/setup.mjs b/setup.mjs index ff26c51..39b064e 100755 --- a/setup.mjs +++ b/setup.mjs @@ -12,7 +12,7 @@ * 4. Creates start.sh for easy launch * 5. Optionally starts the proxy */ -import { readFileSync, writeFileSync, existsSync, mkdirSync, unlinkSync, readdirSync } from "node:fs"; +import { readFileSync, writeFileSync, existsSync, mkdirSync, unlinkSync, readdirSync, chmodSync } from "node:fs"; import { execSync } from "node:child_process"; import { join, dirname } from "node:path"; import { homedir } from "node:os"; @@ -425,7 +425,8 @@ if (!DRY_RUN) { `; writeFileSync(plistPath, plistXml); - log(`Plist written: ${plistPath}`); + chmodSync(plistPath, 0o600); + 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,7 +460,8 @@ WantedBy=default.target `; writeFileSync(servicePath, serviceUnit); - log(`Service file written: ${servicePath}`); + chmodSync(servicePath, 0o600); + log(`Service file written: ${servicePath} (mode 600)`); execSync(`systemctl --user daemon-reload`); execSync(`systemctl --user enable ocp-proxy`);