mirror of
https://github.com/dtzp555-max/ocp.git
synced 2026-07-19 09:44:07 +00:00
fix(setup): rescue the semicolon from the comment; assert the child SAW the override
Two review nits on the way in. setup.mjs:393 — the statement's semicolon had been swallowed INTO the trailing comment, so the line parsed only because ASI rescued it (the next token is `if` on a new line). The repo has no linter, so nothing would have caught it. Comment moved above the statement. test-features.mjs — negative control on the prod-gate probe. The reviewer noticed the test's robustness was INCIDENTAL: because the child env is spread from process.env, it inherits the parent's own OCP_DIR_OVERRIDE, so a future refactor that renamed the var and missed this test's explicit `env` object would still have gone red — but by luck, not by assertion. The child now prints the override it SAW as well as the store it opened, and the test asserts both. The claim is now 'a prod process saw the override and ignored it', not merely 'a prod process opened the right store' (which could pass for the wrong reason). Mutation re-proven after both edits: delete the NODE_ENV gate -> 319 passed, 1 failed; restore -> 320 passed, 0 failed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VqgWJcjxrjjL9L9SkpZyXR
This commit is contained in:
@@ -390,7 +390,9 @@ 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");
|
||||||
if (!existsSync(ocpLogsDir)) mkdirSync(ocpLogsDir, { recursive: true, mode: 0o700 }) // 0700: this call can create ~/.ocp itself on a fresh install;
|
// mode 0700: with `recursive`, this call can create ~/.ocp ITSELF on a fresh install, and
|
||||||
|
// 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") {
|
||||||
|
|||||||
+11
-5
@@ -3839,14 +3839,20 @@ test("a PRODUCTION process (no NODE_ENV) must IGNORE OCP_DIR_OVERRIDE", () => {
|
|||||||
const evil = mkdtempSync(join(tmpdir(), "ocp-evil-"));
|
const evil = mkdtempSync(join(tmpdir(), "ocp-evil-"));
|
||||||
try {
|
try {
|
||||||
const keysUrl = pathToFileURL(join(import.meta.dirname, "keys.mjs")).href;
|
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)};
|
const probe = `import { getDb, getDbPath, closeDb } from ${JSON.stringify(keysUrl)};
|
||||||
getDb(); process.stdout.write(getDbPath()); closeDb();`;
|
getDb(); process.stdout.write(process.env.OCP_DIR_OVERRIDE + "\\n" + getDbPath()); closeDb();`;
|
||||||
const env = { ...process.env, HOME: home, OCP_DIR_OVERRIDE: evil };
|
const env = { ...process.env, HOME: home, OCP_DIR_OVERRIDE: evil };
|
||||||
delete env.NODE_ENV; // a production server has no NODE_ENV
|
delete env.NODE_ENV; // a production server has no NODE_ENV
|
||||||
const out = execFileSync(process.execPath, ["--input-type=module", "-e", probe],
|
const [seen, opened] = execFileSync(process.execPath, ["--input-type=module", "-e", probe],
|
||||||
{ env, encoding: "utf8" }).trim();
|
{ env, encoding: "utf8" }).trim().split("\n");
|
||||||
assert.equal(out, join(home, ".ocp", "ocp.db"), "a prod process must open HOME/.ocp/ocp.db");
|
assert.equal(seen, evil, "precondition: the child must actually SEE the override");
|
||||||
assert.ok(!out.startsWith(evil), "a prod process must NEVER honor OCP_DIR_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 {
|
} finally {
|
||||||
rmSync(home, { recursive: true, force: true });
|
rmSync(home, { recursive: true, force: true });
|
||||||
rmSync(evil, { recursive: true, force: true });
|
rmSync(evil, { recursive: true, force: true });
|
||||||
|
|||||||
Reference in New Issue
Block a user