mirror of
https://github.com/dtzp555-max/ocp.git
synced 2026-07-21 21:15:09 +00:00
eeec2bf83d
* fix(setup): never carry test-only key-store redirection vars into a server OCP launches (A4) Defense-in-depth for the key-store isolation shipped in #163, plus a correction to the overstated claim that fix's comments made. Surfaced by an independent (Codex) re-review. Background: keys.mjs honors OCP_DIR_OVERRIDE only when NODE_ENV === "test", so the key store can be pointed at a scratch dir for the test suite. If BOTH vars reached a production daemon's environment, it would open a scratch/empty key store instead of ~/.ocp/ocp.db — in AUTH_MODE=multi a silent total auth outage. #163's comments claimed a production server "runs without NODE_ENV, so it CANNOT honor the override no matter how the variable got in." That is not something keys.mjs can enforce — it is only true while the daemon's env happens to lack NODE_ENV=test. This PR makes it true for every server OCP itself launches, and softens the docs to stop overclaiming. Three parts (all in OCP's own launch/installer paths — no server.mjs change, no cli.js analogue): 1. scripts/lib/plist-merge.mjs — new exported NEVER_PRESERVE = {NODE_ENV, OCP_DIR_OVERRIDE}, stripped from the preserved set in BOTH mergePlistEnv and mergeSystemdEnv. The preservation rule ("keys only in the EXISTING unit are kept verbatim") was the vector: a unit that once carried these test-only vars would otherwise survive every setup re-run. setup.mjs's template never injects them, so preservation was the only entry path, and this closes it. 2. ocp (cmd_restart manual fallback) — the one direct `node server.mjs` launch OCP controls now runs under `env -u NODE_ENV -u OCP_DIR_OVERRIDE`, so a maintainer who exported both while debugging and then restarted can't silently boot the daemon onto a scratch store. 3. keys.mjs + test-env.mjs — softened the overstated comments to state what is actually enforced (the two-key gate makes neither var alone do anything; OCP's launchers strip both) and to name the one residual path honestly: a hand-rolled `node server.mjs` with both vars explicitly exported, bypassing every launcher — for which the loud getDb() "NOT the default" log is the backstop. No library-level gate can catch an operator who both sets a test flag and bypasses the launchers; the honest fix is a non-silent wrong-store, which #163 already provides. Severity: LOW (defense-in-depth; the default/shipped path was already safe). No behavior change on any correctly-configured install. ALIGNMENT.md: this PR does not touch server.mjs, so the cli.js-citation hard requirement does not apply; and no cli.js operation is involved — key-store isolation and installer env hygiene are entirely OCP-owned (no Class A / cli.js-mirror surface). Tests: +4 mutation-proven (3 behavioral: drop the `!NEVER_PRESERVE.has(k)` guard in either merge fn and they fail — verified 326 passed / 3 failed under mutation; restored). The `ocp` bash `env -u` line is verified by `bash -n` + inspection (the suite does not exec the installer/daemon). Full suite: 329 passed / 0 failed (was 325). Version bump + CHANGELOG deferred to the later chore(release) PR, per the repo's #148/#149/#150 -> #151 convention (matching PR #164). Co-Authored-By: Claude <claude-opus-4-8> <noreply@anthropic.com> * test(setup): assert NEVER_PRESERVE.size === 2 so the "exactly two" test matches its name Reviewer nit (LOW): the membership assertion let a future spurious third entry slip past a test whose name promises "exactly the two". Behavior stays guarded by the 3 mutation-proof tests; this just makes the contract test honest. Co-Authored-By: Claude <claude-opus-4-8> <noreply@anthropic.com> --------- Co-authored-by: dtzp555 <dtzp555@gmail.com> Co-authored-by: Claude <claude-opus-4-8> <noreply@anthropic.com>
102 lines
4.2 KiB
JavaScript
102 lines
4.2 KiB
JavaScript
// 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.
|
|
//
|
|
// 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()),
|
|
// so raw `<` / `>` / `&` never appear in plist <string> bodies — the [^<]* regex below is safe.
|
|
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) && !NEVER_PRESERVE.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) && !NEVER_PRESERVE.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`
|
|
);
|
|
}
|