Skip to content

Commit 78416d9

Browse files
authored
Merge pull request #782 from Wibias/fix/766-admin-acl-opt-in
fix(security): retry required ACL harden after soft timeout
2 parents aa924f3 + 0538cc1 commit 78416d9

5 files changed

Lines changed: 150 additions & 11 deletions

File tree

src/lib/windows-secret-acl.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -363,7 +363,11 @@ async function describeAclStateAfterTimeoutAsync(targetPath: string, deadline: n
363363
function timeoutMemoKey(targetPath: string, opts: HardenOptions): string {
364364
// Destination-path memo only (issue #612). Never a parent directory — directory ACLs
365365
// are not authoritative for newly created temps.
366-
return opts.timeoutMemoKey ?? targetPath;
366+
//
367+
// Namespace by required-ness (#766): a soft `required:false` timeout during loadConfig
368+
// must not poison a later `required:true` management-token harden of the same path.
369+
const base = opts.timeoutMemoKey ?? targetPath;
370+
return `${opts.required ? "required" : "optional"}:${base}`;
367371
}
368372

369373
/**

src/server/management-auth.ts

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,11 @@ function assertSafeDirectory(path: string): void {
5656
if (!stat.isDirectory() || stat.isSymbolicLink()) throw new Error("management token directory is not a regular directory");
5757
chmodSync(path, 0o700);
5858
const hardened = hardenSecretDir(path, { required: true });
59-
if (!hardened.ok) throw new Error("management token directory ACL hardening did not complete");
59+
if (!hardened.ok) {
60+
throw new Error(
61+
"management token directory ACL hardening did not complete; set OPENCODEX_ADMIN_AUTH_TOKEN to use an environment token instead of a file-backed token",
62+
);
63+
}
6064
}
6165

6266
function readExistingToken(path: string): string {
@@ -66,7 +70,11 @@ function readExistingToken(path: string): string {
6670
}
6771
chmodSync(path, 0o600);
6872
const hardened = hardenSecretPath(path, { required: true });
69-
if (!hardened.ok) throw new Error("management token file ACL hardening did not complete");
73+
if (!hardened.ok) {
74+
throw new Error(
75+
"management token file ACL hardening did not complete; set OPENCODEX_ADMIN_AUTH_TOKEN to use an environment token instead of a file-backed token",
76+
);
77+
}
7078
const token = readFileSync(path, "utf8").trim();
7179
if (!/^ocx_admin_[A-Za-z0-9_-]{43}$/.test(token)) throw new Error("management token file is invalid");
7280
return token;
@@ -90,7 +98,11 @@ function createTokenFile(path: string): string {
9098
fd = null;
9199
chmodSync(temporary, 0o600);
92100
const temporaryHardened = hardenSecretPath(temporary, { required: true });
93-
if (!temporaryHardened.ok) throw new Error("management token temporary ACL hardening did not complete");
101+
if (!temporaryHardened.ok) {
102+
throw new Error(
103+
"management token temporary ACL hardening did not complete; set OPENCODEX_ADMIN_AUTH_TOKEN to use an environment token instead of a file-backed token",
104+
);
105+
}
94106
try {
95107
linkSync(temporary, path);
96108
linked = true;
@@ -99,7 +111,11 @@ function createTokenFile(path: string): string {
99111
throw error;
100112
}
101113
const finalHardened = hardenSecretPath(path, { required: true });
102-
if (!finalHardened.ok) throw new Error("management token file ACL hardening did not complete");
114+
if (!finalHardened.ok) {
115+
throw new Error(
116+
"management token file ACL hardening did not complete; set OPENCODEX_ADMIN_AUTH_TOKEN to use an environment token instead of a file-backed token",
117+
);
118+
}
103119
return token;
104120
} catch (error) {
105121
if (linked) removeBestEffort(path);
@@ -190,7 +206,11 @@ export function requireManagementAuth(
190206
config?: OcxConfig,
191207
): Response | null {
192208
if (!state.available) {
193-
return Response.json({ error: "management API unavailable" }, { status: 503 });
209+
return Response.json({
210+
error: "management API unavailable",
211+
reason: state.reason,
212+
hint: "Set OPENCODEX_ADMIN_AUTH_TOKEN to bypass file-backed admin token ACL hardening",
213+
}, { status: 503 });
194214
}
195215
const actual = req.headers.get("x-opencodex-api-key")?.trim()
196216
|| req.headers.get("authorization")?.replace(/^Bearer\s+/i, "").trim();

tests/server-management-auth.test.ts

Lines changed: 101 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import {
1616
resetHardenedStateForTests,
1717
setIcaclsRunnerForTests,
1818
setPlatformForTests,
19+
hardenSecretDir,
1920
} from "../src/lib/windows-secret-acl";
2021

2122
const previousHome = process.env.OPENCODEX_HOME;
@@ -204,7 +205,11 @@ describe("management and data-plane credential separation", () => {
204205
headers: { "x-opencodex-api-key": "ocx_admin_unhardened" },
205206
});
206207
expect(management.status).toBe(503);
207-
expect(await management.json()).toEqual({ error: "management API unavailable" });
208+
const body = await management.json() as { error?: string; hint?: string; reason?: string };
209+
expect(body.error).toBe("management API unavailable");
210+
expect(body.hint).toContain("OPENCODEX_ADMIN_AUTH_TOKEN");
211+
expect(typeof body.reason).toBe("string");
212+
expect(body.reason!.length).toBeGreaterThan(0);
208213
} finally {
209214
await server.stop(true);
210215
}
@@ -436,4 +441,99 @@ describe("management and data-plane credential separation", () => {
436441
await server.stop(true);
437442
}
438443
});
444+
445+
test("directory ACL timeout keeps management unavailable and names OPENCODEX_ADMIN_AUTH_TOKEN", async () => {
446+
delete process.env.OPENCODEX_ADMIN_AUTH_TOKEN;
447+
saveConfig(remoteConfig());
448+
const adminToken = `ocx_admin_${"d".repeat(43)}`;
449+
writeFileSync(join(testHome, "admin-api-token"), `${adminToken}\n`, { mode: 0o600 });
450+
process.env.USERNAME ??= "tester";
451+
setPlatformForTests("win32");
452+
setIcaclsRunnerForTests(args => {
453+
const target = args[0] ?? "";
454+
if (target.endsWith("admin-api-token")) {
455+
return { success: true, exitCode: 0, timedOut: false, stdout: "" };
456+
}
457+
return { success: false, exitCode: null, timedOut: true, stdout: "" };
458+
});
459+
resetHardenedStateForTests();
460+
const state = initializeManagementAuthState(remoteConfig());
461+
expect(state.available).toBe(false);
462+
if (state.available) return;
463+
expect(state.reason).toContain("OPENCODEX_ADMIN_AUTH_TOKEN");
464+
465+
const server = startServer(0);
466+
try {
467+
const settings = await fetch(new URL("/api/settings", server.url), {
468+
headers: { "x-opencodex-api-key": adminToken },
469+
});
470+
expect(settings.status).toBe(503);
471+
const body = await settings.json() as { error?: string; hint?: string; reason?: string };
472+
expect(body.error).toBe("management API unavailable");
473+
expect(body.hint).toContain("OPENCODEX_ADMIN_AUTH_TOKEN");
474+
expect(body.reason).toContain("OPENCODEX_ADMIN_AUTH_TOKEN");
475+
expect((await fetch(new URL("/healthz", server.url))).status).toBe(200);
476+
} finally {
477+
await server.stop(true);
478+
}
479+
});
480+
481+
test("required management harden retries after a soft loadConfig directory timeout", async () => {
482+
delete process.env.OPENCODEX_ADMIN_AUTH_TOKEN;
483+
saveConfig(remoteConfig());
484+
const adminToken = `ocx_admin_${"f".repeat(43)}`;
485+
writeFileSync(join(testHome, "admin-api-token"), `${adminToken}\n`, { mode: 0o600 });
486+
process.env.USERNAME ??= "tester";
487+
setPlatformForTests("win32");
488+
489+
let softPhase = true;
490+
let requiredPhaseCalls = 0;
491+
setIcaclsRunnerForTests(args => {
492+
const target = args[0] ?? "";
493+
if (target.endsWith("admin-api-token")) {
494+
return { success: true, exitCode: 0, timedOut: false, stdout: "" };
495+
}
496+
if (softPhase) {
497+
return { success: false, exitCode: null, timedOut: true, stdout: "" };
498+
}
499+
requiredPhaseCalls += 1;
500+
return { success: true, exitCode: 0, timedOut: false, stdout: "" };
501+
});
502+
resetHardenedStateForTests();
503+
504+
const soft = hardenSecretDir(testHome, { required: false });
505+
expect(soft.ok).toBe(false);
506+
expect(soft.diagnostics).toMatch(/timed out|budget exhausted|previous attempt/i);
507+
508+
softPhase = false;
509+
const state = initializeManagementAuthState(remoteConfig());
510+
expect(state.available).toBe(true);
511+
if (!state.available) return;
512+
expect(state.source).toBe("file");
513+
expect(requiredPhaseCalls).toBeGreaterThan(0);
514+
});
515+
516+
test("OPENCODEX_ADMIN_AUTH_TOKEN bypasses file-backed ACL hardening", async () => {
517+
process.env.OPENCODEX_ADMIN_AUTH_TOKEN = "env-admin-secret";
518+
saveConfig(remoteConfig());
519+
process.env.USERNAME ??= "tester";
520+
setPlatformForTests("win32");
521+
setIcaclsRunnerForTests(() => ({ success: false, exitCode: null, timedOut: true, stdout: "" }));
522+
resetHardenedStateForTests();
523+
524+
const state = initializeManagementAuthState(remoteConfig());
525+
expect(state.available).toBe(true);
526+
if (!state.available) return;
527+
expect(state.source).toBe("environment");
528+
529+
const server = startServer(0);
530+
try {
531+
const management = await fetch(new URL("/api/config", server.url), {
532+
headers: { "x-opencodex-api-key": "env-admin-secret" },
533+
});
534+
expect(management.status).toBe(200);
535+
} finally {
536+
await server.stop(true);
537+
}
538+
});
439539
});

tests/storage-cleanup.test.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,7 @@ describe("previewArchivedCleanup", () => {
220220
"archived_sessions/rollout-new.jsonl",
221221
]);
222222
expect(listed.some(c => c.relPath.includes("sessions/2026"))).toBe(false);
223-
});
223+
}, { timeout: STORE_BUDGET_MS });
224224

225225
test("percent selects oldest subset and includes digest", () => {
226226
home = buildHome();
@@ -236,7 +236,7 @@ describe("previewArchivedCleanup", () => {
236236
expect(preview.bytes).toBe(preview.candidates[0]!.bytes);
237237
expect(preview.digest).toBe(computePreviewDigest(preview.candidates, 50));
238238
expect(preview.digest).toMatch(/^[a-f0-9]{64}$/);
239-
});
239+
}, { timeout: STORE_BUDGET_MS });
240240

241241
test("treats .jsonl and .jsonl.zst as one logical rollout", () => {
242242
home = buildHome();
@@ -250,7 +250,7 @@ describe("previewArchivedCleanup", () => {
250250
"archived_sessions/rollout-old.jsonl.zst",
251251
]);
252252
expect(listed.filter(c => c.relPath.includes("rollout-old"))).toHaveLength(1);
253-
});
253+
}, { timeout: STORE_BUDGET_MS });
254254
});
255255

256256
describe("normalizeArchivedRolloutPath", () => {
@@ -267,7 +267,7 @@ describe("normalizeArchivedRolloutPath", () => {
267267
// ISO timestamps in filenames must not be treated as Windows drive letters.
268268
expect(normalizeArchivedRolloutPath("archived_sessions/rollout-2026-01-01T10:00:00.jsonl", home))
269269
.toBe("archived_sessions/rollout-2026-01-01T10:00:00.jsonl");
270-
});
270+
}, { timeout: STORE_BUDGET_MS });
271271
});
272272

273273
describe("executeArchivedCleanup", () => {

tests/windows-secret-acl.test.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -548,6 +548,21 @@ describe("async hardenSecretPath (issue #612)", () => {
548548
expect(calls).toBe(0); // destination-keyed memo; not a parent-directory shortcut
549549
});
550550

551+
test("optional timeout memo does not poison a later required harden of the same path", () => {
552+
setIcaclsRunnerForTests(() => timeout);
553+
const first = hardenSecretPath(secretFile(), { required: false });
554+
expect(first.ok).toBe(false);
555+
556+
let calls = 0;
557+
setIcaclsRunnerForTests(() => {
558+
calls += 1;
559+
return ok;
560+
});
561+
const second = hardenSecretPath(secretFile(), { required: true });
562+
expect(second.ok).toBe(true);
563+
expect(calls).toBeGreaterThan(0);
564+
});
565+
551566
test("async harden still grants owner before inheritance removal", async () => {
552567
const steps: string[] = [];
553568
setAsyncIcaclsRunnerForTests(async args => {

0 commit comments

Comments
 (0)