Skip to content

Commit f1d1893

Browse files
committed
fix(security): propagate directory ACL unverified into management auth
assertSafeDirectory soft-continues were discarded, so /api/settings could under-report. OR directory status into aclUnverified and cover dir-timeout vs file-ok with and without the opt-in.
1 parent 20d7201 commit f1d1893

2 files changed

Lines changed: 77 additions & 5 deletions

File tree

src/server/management-auth.ts

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,8 @@ function fail(reason: string): ManagementAuthState {
5353
return { available: false, reason };
5454
}
5555

56-
function assertSafeDirectory(path: string): void {
56+
/** Returns true when the directory was accepted without verified NTFS ACL harden. */
57+
function assertSafeDirectory(path: string): boolean {
5758
mkdirSync(path, { recursive: true, mode: 0o700 });
5859
const stat = lstatSync(path);
5960
if (!stat.isDirectory() || stat.isSymbolicLink()) throw new Error("management token directory is not a regular directory");
@@ -62,10 +63,11 @@ function assertSafeDirectory(path: string): void {
6263
if (!hardened.ok) {
6364
if (allowUnverifiedAdminTokenAcl() && /timed out|ETIMEDOUT|budget exhausted|previous attempt timed out/i.test(hardened.diagnostics ?? "")) {
6465
console.warn(`[opencodex] management token directory ACL unverified (${hardened.diagnostics}); continuing because OPENCODEX_ALLOW_UNVERIFIED_ADMIN_TOKEN_ACL is set`);
65-
return;
66+
return true;
6667
}
6768
throw new Error("management token directory ACL hardening did not complete");
6869
}
70+
return false;
6971
}
7072

7173
function readExistingToken(path: string): { token: string; aclUnverified: boolean } {
@@ -177,16 +179,17 @@ export function initializeManagementAuthState(config: OcxConfig): ManagementAuth
177179
}
178180
try {
179181
const path = adminApiTokenFilePath();
180-
assertSafeDirectory(dirname(path));
182+
const directoryUnverified = assertSafeDirectory(dirname(path));
181183
let loaded: { token: string; aclUnverified: boolean };
182184
try {
183185
loaded = readExistingToken(path);
184186
} catch (error) {
185187
if ((error as NodeJS.ErrnoException).code !== "ENOENT") throw error;
186188
loaded = createTokenFile(path);
187189
}
188-
lastManagementAuthAclUnverified = loaded.aclUnverified === true;
189-
return ready(loaded.token, "file", config, { aclUnverified: loaded.aclUnverified });
190+
const aclUnverified = directoryUnverified || loaded.aclUnverified === true;
191+
lastManagementAuthAclUnverified = aclUnverified;
192+
return ready(loaded.token, "file", config, { aclUnverified });
190193
} catch (error) {
191194
lastManagementAuthAclUnverified = false;
192195
return fail(error instanceof Error ? error.message : "management token initialization failed");

tests/server-management-auth.test.ts

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -475,4 +475,73 @@ describe("management and data-plane credential separation", () => {
475475
await server.stop(true);
476476
}
477477
});
478+
479+
test("directory ACL timeout without opt-in keeps management unavailable even when the token file hardens", async () => {
480+
delete process.env.OPENCODEX_ADMIN_AUTH_TOKEN;
481+
delete process.env.OPENCODEX_ALLOW_UNVERIFIED_ADMIN_TOKEN_ACL;
482+
saveConfig(remoteConfig());
483+
const adminToken = `ocx_admin_${"d".repeat(43)}`;
484+
writeFileSync(join(testHome, "admin-api-token"), `${adminToken}\n`, { mode: 0o600 });
485+
process.env.USERNAME ??= "tester";
486+
setPlatformForTests("win32");
487+
setIcaclsRunnerForTests(args => {
488+
const target = args[0] ?? "";
489+
// Directory harden times out; token-file harden succeeds.
490+
if (target.endsWith("admin-api-token")) {
491+
return { success: true, exitCode: 0, timedOut: false, stdout: "" };
492+
}
493+
return { success: false, exitCode: null, timedOut: true, stdout: "" };
494+
});
495+
// Drop any directory cache left by saveConfig so this case actually re-hardens.
496+
resetHardenedStateForTests();
497+
const state = initializeManagementAuthState(remoteConfig());
498+
expect(state.available).toBe(false);
499+
expect(managementAuthAclUnverified()).toBe(false);
500+
501+
const server = startServer(0);
502+
try {
503+
const settings = await fetch(new URL("/api/settings", server.url), {
504+
headers: { "x-opencodex-api-key": adminToken },
505+
});
506+
expect(settings.status).toBe(503);
507+
expect((await fetch(new URL("/healthz", server.url))).status).toBe(200);
508+
} finally {
509+
await server.stop(true);
510+
}
511+
});
512+
513+
test("directory ACL timeout with opt-in surfaces aclUnverified when the token file hardens", async () => {
514+
delete process.env.OPENCODEX_ADMIN_AUTH_TOKEN;
515+
process.env.OPENCODEX_ALLOW_UNVERIFIED_ADMIN_TOKEN_ACL = "1";
516+
saveConfig(remoteConfig());
517+
const adminToken = `ocx_admin_${"e".repeat(43)}`;
518+
writeFileSync(join(testHome, "admin-api-token"), `${adminToken}\n`, { mode: 0o600 });
519+
process.env.USERNAME ??= "tester";
520+
setPlatformForTests("win32");
521+
setIcaclsRunnerForTests(args => {
522+
const target = args[0] ?? "";
523+
if (target.endsWith("admin-api-token")) {
524+
return { success: true, exitCode: 0, timedOut: false, stdout: "" };
525+
}
526+
return { success: false, exitCode: null, timedOut: true, stdout: "" };
527+
});
528+
resetHardenedStateForTests();
529+
const state = initializeManagementAuthState(remoteConfig());
530+
expect(state.available).toBe(true);
531+
if (!state.available) return;
532+
expect(state.aclUnverified).toBe(true);
533+
expect(managementAuthAclUnverified()).toBe(true);
534+
535+
const server = startServer(0);
536+
try {
537+
const settings = await fetch(new URL("/api/settings", server.url), {
538+
headers: { "x-opencodex-api-key": adminToken },
539+
});
540+
expect(settings.status).toBe(200);
541+
const body = await settings.json() as { managementAuthAclUnverified?: boolean };
542+
expect(body.managementAuthAclUnverified).toBe(true);
543+
} finally {
544+
await server.stop(true);
545+
}
546+
});
478547
});

0 commit comments

Comments
 (0)