Compare commits

..
Author SHA1 Message Date
taodengandClaude Sonnet 4.6 c06b1c7bc7 fix(security): tighten on-disk credential file modes (700/600)
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: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-09 23:47:57 +10:00
4 changed files with 56 additions and 62 deletions
+2 -21
View File
@@ -55,13 +55,7 @@
</div> </div>
<div class="section"> <div class="section">
<div class="flex" style="justify-content: space-between; align-items: center;"> <h2>Usage by Key</h2>
<h2 style="margin: 0;">Usage by Key</h2>
<label id="usage-scope-toggle" class="flex" style="display:none; gap: 0.4rem; font-size: 0.8rem; color: #94a3b8; cursor: pointer;">
<input type="checkbox" id="usage-show-all" style="cursor: pointer;">
<span>Show all keys</span>
</label>
</div>
<table id="key-usage-table"> <table id="key-usage-table">
<thead><tr><th>Key</th><th>Requests</th><th>OK</th><th>Err</th><th>Avg Time</th><th>Last Request</th></tr></thead> <thead><tr><th>Key</th><th>Requests</th><th>OK</th><th>Err</th><th>Avg Time</th><th>Last Request</th></tr></thead>
<tbody></tbody> <tbody></tbody>
@@ -176,8 +170,7 @@ async function refreshStatus() {
async function refreshUsage() { async function refreshUsage() {
try { try {
const showAll = localStorage.getItem("ocp_usage_show_all") === "1"; const data = await api("/api/usage");
const data = await api(showAll ? "/api/usage?all=true" : "/api/usage");
const tbody = document.querySelector("#key-usage-table tbody"); const tbody = document.querySelector("#key-usage-table tbody");
tbody.innerHTML = (data.byKey || []).map(k => ` tbody.innerHTML = (data.byKey || []).map(k => `
<tr> <tr>
@@ -211,8 +204,6 @@ async function refreshKeys() {
try { try {
const data = await api("/api/keys"); const data = await api("/api/keys");
document.getElementById("key-mgmt-section").style.display = ""; document.getElementById("key-mgmt-section").style.display = "";
// Admin-only "Show all keys" toggle for /api/usage scope.
document.getElementById("usage-scope-toggle").style.display = "flex";
const tbody = document.querySelector("#keys-table tbody"); const tbody = document.querySelector("#keys-table tbody");
tbody.innerHTML = (data.keys || []).map(k => ` tbody.innerHTML = (data.keys || []).map(k => `
<tr> <tr>
@@ -249,16 +240,6 @@ async function refreshAll() {
document.getElementById("refresh-indicator").textContent = `Updated ${new Date().toLocaleTimeString()}`; document.getElementById("refresh-indicator").textContent = `Updated ${new Date().toLocaleTimeString()}`;
} }
// Wire "Show all keys" toggle (visibility gated to admin via refreshKeys()).
(function setupUsageScopeToggle() {
const cb = document.getElementById("usage-show-all");
cb.checked = localStorage.getItem("ocp_usage_show_all") === "1";
cb.addEventListener("change", () => {
localStorage.setItem("ocp_usage_show_all", cb.checked ? "1" : "0");
refreshUsage();
});
})();
refreshAll(); refreshAll();
setInterval(refreshAll, 30000); setInterval(refreshAll, 30000);
</script> </script>
+6 -2
View File
@@ -3,11 +3,13 @@
import { DatabaseSync } from "node:sqlite"; import { DatabaseSync } from "node:sqlite";
import { randomBytes, createHash } from "node:crypto"; import { randomBytes, createHash } from "node:crypto";
import { join } from "node:path"; import { join } from "node:path";
import { mkdirSync } from "node:fs"; import { mkdirSync, chmodSync } from "node:fs";
import { homedir } from "node:os"; import { homedir } from "node:os";
const OCP_DIR = join(homedir(), ".ocp"); 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"); const DB_PATH = join(OCP_DIR, "ocp.db");
let db; let db;
@@ -18,6 +20,8 @@ export function getDb() {
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.
try { chmodSync(DB_PATH, 0o600); } catch { /* ignore — same-user access still works */ }
} }
return db; return db;
} }
+43 -36
View File
@@ -30,7 +30,7 @@
import { createServer } from "node:http"; import { createServer } from "node:http";
import { spawn, execFileSync } from "node:child_process"; import { spawn, execFileSync } from "node:child_process";
import { randomUUID, timingSafeEqual } from "node:crypto"; 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 { fileURLToPath } from "node:url";
import { dirname, join } from "node:path"; import { dirname, join } from "node:path";
import { homedir } from "node:os"; 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) ────────────────────────────────────────── // ── Circuit breaker (DISABLED) ──────────────────────────────────────────
// Disabled: CLI proxy has its own retry logic, and the breaker was causing // Disabled: CLI proxy has its own retry logic, and the breaker was causing
// cascading failures — once API got briefly slow, ALL agents lost connectivity // cascading failures — once API got briefly slow, ALL agents lost connectivity
@@ -1616,45 +1654,14 @@ const server = createServer(async (req, res) => {
} }
if (req.url?.startsWith("/api/usage") && req.method === "GET") { if (req.url?.startsWith("/api/usage") && req.method === "GET") {
// Least-privilege scope rules (security audit follow-up): if (!isAdmin) return jsonResponse(res, 403, { error: "Admin access required" });
// - non-admin authenticated key → only own rows
// - anonymous (PROXY_ANONYMOUS_KEY) → only "anonymous" rows; ?all=true ignored
// - admin without ?all=true → only own ("admin") rows
// - admin with ?all=true → full byKey/recent (legacy behavior); audited
// Authenticated callers are required (anyone reaching here passed the auth gate above);
// remote+no-auth requests would have been rejected before this point.
const url = new URL(req.url, `http://${BIND_ADDRESS}:${PORT}`); const url = new URL(req.url, `http://${BIND_ADDRESS}:${PORT}`);
const since = url.searchParams.get("since"); const since = url.searchParams.get("since");
const until = url.searchParams.get("until"); const until = url.searchParams.get("until");
const wantAll = url.searchParams.get("all") === "true";
const callerName = req._authKeyName;
// Anonymous callers may never opt into all-keys view, even if they pass ?all=true.
const isAnonCaller = callerName === "anonymous";
const fullScope = isAdmin && wantAll && !isAnonCaller;
// scopeName === null when fullScope is true (no filter); otherwise the key_name to filter by.
const scopeName = fullScope ? null : callerName;
if (fullScope) {
logEvent("info", "admin_usage_full_scope", { caller: callerName, ip: req.socket.remoteAddress || null });
}
const byKeyAll = getUsageByKey({ since, until });
const recentAll = getRecentUsage(Math.min(parseInt(url.searchParams.get("limit") || "50", 10), 500));
const timeline = getUsageTimeline({
keyName: scopeName || undefined,
hours: Math.min(parseInt(url.searchParams.get("hours") || "24", 10), 720),
});
const byKey = scopeName ? byKeyAll.filter((row) => row.key_name === scopeName) : byKeyAll;
const recent = scopeName ? recentAll.filter((row) => row.key_name === scopeName) : recentAll;
return jsonResponse(res, 200, { return jsonResponse(res, 200, {
byKey, byKey: getUsageByKey({ since, until }),
timeline, timeline: getUsageTimeline({ hours: Math.min(parseInt(url.searchParams.get("hours") || "24", 10), 720) }),
recent, recent: getRecentUsage(Math.min(parseInt(url.searchParams.get("limit") || "50", 10), 500)),
scope: { self: scopeName, all: fullScope },
}); });
} }
+5 -3
View File
@@ -12,7 +12,7 @@
* 4. Creates start.sh for easy launch * 4. Creates start.sh for easy launch
* 5. Optionally starts the proxy * 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 { execSync } from "node:child_process";
import { join, dirname } from "node:path"; import { join, dirname } from "node:path";
import { homedir } from "node:os"; import { homedir } from "node:os";
@@ -425,7 +425,8 @@ if (!DRY_RUN) {
`; `;
writeFileSync(plistPath, plistXml); 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 // Bootout first (in case it was already loaded) then bootstrap
try { execSync(`launchctl bootout gui/$(id -u) "${plistPath}" 2>/dev/null`); } catch { /* ignore */ } try { execSync(`launchctl bootout gui/$(id -u) "${plistPath}" 2>/dev/null`); } catch { /* ignore */ }
@@ -459,7 +460,8 @@ WantedBy=default.target
`; `;
writeFileSync(servicePath, serviceUnit); 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 daemon-reload`);
execSync(`systemctl --user enable ocp-proxy`); execSync(`systemctl --user enable ocp-proxy`);