Skip to content

Commit 2b8c82c

Browse files
committed
fix(security): retry required ACL harden after soft timeout
Drop OPENCODEX_ALLOW_UNVERIFIED_ADMIN_TOKEN_ACL. Namespace the timeout memo so loadConfig soft-fails no longer poison management required:true hardens, and surface OPENCODEX_ADMIN_AUTH_TOKEN as the fail-closed escape on 503.
1 parent f1d1893 commit 2b8c82c

10 files changed

Lines changed: 97 additions & 153 deletions

src/lib/allow-unverified-admin-token-acl.ts

Lines changed: 0 additions & 10 deletions
This file was deleted.

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/lib/winsw.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ import { join, resolve } from "node:path";
2121
import { expandUserPath, getConfigDir, loadConfig } from "../config";
2222
import { recordOwnedConfigPath } from "./config-ownership";
2323
import { durableBunPath } from "./bun-runtime";
24-
import { bakedAllowUnverifiedAdminTokenAcl } from "./allow-unverified-admin-token-acl";
2524
import { serviceApiTokenFilePath } from "./service-secrets";
2625

2726
export const WINSW_VERSION = "2.12.0";
@@ -101,7 +100,6 @@ export function buildWinswXml(entry: WinswEntry, env: NodeJS.ProcessEnv = proces
101100
env.CODEX_HOME?.trim() ? ` <env name="CODEX_HOME" value="${xmlEscape(currentCodexHomeAbsolute())}"/>` : null,
102101
` <env name="OPENCODEX_HOME" value="${xmlEscape(getConfigDir())}"/>`,
103102
aclTimeout ? ` <env name="OPENCODEX_ACL_TIMEOUT_MS" value="${xmlEscape(aclTimeout)}"/>` : null,
104-
bakedAllowUnverifiedAdminTokenAcl(env) ? ` <env name="OPENCODEX_ALLOW_UNVERIFIED_ADMIN_TOKEN_ACL" value="${xmlEscape(bakedAllowUnverifiedAdminTokenAcl(env)!)}"/>` : null,
105103
].filter((line): line is string => Boolean(line));
106104
return `<?xml version="1.0" encoding="UTF-8"?>
107105
<service>

src/server/management-auth.ts

Lines changed: 29 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import {
1313
} from "node:fs";
1414
import { dirname, join } from "node:path";
1515
import { adminApiTokenFilePath } from "../lib/admin-secrets";
16-
import { allowUnverifiedAdminTokenAcl } from "../lib/allow-unverified-admin-token-acl";
1716
import { hardenSecretDir, hardenSecretPath } from "../lib/windows-secret-acl";
1817
import type { OcxConfig } from "../types";
1918
import {
@@ -44,64 +43,53 @@ export type ManagementAuthState =
4443
token: string;
4544
source: "environment" | "file";
4645
sessions: Map<string, GuiSessionRecord>;
47-
/** Set when file-backed token was accepted despite unverified Windows ACL harden. */
48-
aclUnverified?: boolean;
4946
}
5047
| { available: false; reason: string };
5148

5249
function fail(reason: string): ManagementAuthState {
5350
return { available: false, reason };
5451
}
5552

56-
/** Returns true when the directory was accepted without verified NTFS ACL harden. */
57-
function assertSafeDirectory(path: string): boolean {
53+
function assertSafeDirectory(path: string): void {
5854
mkdirSync(path, { recursive: true, mode: 0o700 });
5955
const stat = lstatSync(path);
6056
if (!stat.isDirectory() || stat.isSymbolicLink()) throw new Error("management token directory is not a regular directory");
6157
chmodSync(path, 0o700);
6258
const hardened = hardenSecretDir(path, { required: true });
6359
if (!hardened.ok) {
64-
if (allowUnverifiedAdminTokenAcl() && /timed out|ETIMEDOUT|budget exhausted|previous attempt timed out/i.test(hardened.diagnostics ?? "")) {
65-
console.warn(`[opencodex] management token directory ACL unverified (${hardened.diagnostics}); continuing because OPENCODEX_ALLOW_UNVERIFIED_ADMIN_TOKEN_ACL is set`);
66-
return true;
67-
}
68-
throw new Error("management token directory ACL hardening did not complete");
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+
);
6963
}
70-
return false;
7164
}
7265

73-
function readExistingToken(path: string): { token: string; aclUnverified: boolean } {
66+
function readExistingToken(path: string): string {
7467
const stat = lstatSync(path);
7568
if (!stat.isFile() || stat.isSymbolicLink() || stat.size > 512) {
7669
throw new Error("management token path is not a regular secret file");
7770
}
7871
chmodSync(path, 0o600);
7972
const hardened = hardenSecretPath(path, { required: true });
80-
let aclUnverified = false;
8173
if (!hardened.ok) {
82-
if (allowUnverifiedAdminTokenAcl() && /timed out|ETIMEDOUT|budget exhausted|previous attempt timed out/i.test(hardened.diagnostics ?? "")) {
83-
console.warn(`[opencodex] management token file ACL unverified (${hardened.diagnostics}); continuing because OPENCODEX_ALLOW_UNVERIFIED_ADMIN_TOKEN_ACL is set`);
84-
aclUnverified = true;
85-
} else {
86-
throw new Error("management token file ACL hardening did not complete");
87-
}
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+
);
8877
}
8978
const token = readFileSync(path, "utf8").trim();
9079
if (!/^ocx_admin_[A-Za-z0-9_-]{43}$/.test(token)) throw new Error("management token file is invalid");
91-
return { token, aclUnverified };
80+
return token;
9281
}
9382

9483
function removeBestEffort(path: string): void {
9584
try { unlinkSync(path); } catch { /* fail-closed state is preserved by the caller */ }
9685
}
9786

98-
function createTokenFile(path: string): { token: string; aclUnverified: boolean } {
87+
function createTokenFile(path: string): string {
9988
const directory = dirname(path);
10089
const token = `ocx_admin_${randomBytes(32).toString("base64url")}`;
10190
const temporary = join(directory, `.${randomUUID()}.admin-token.tmp`);
10291
let linked = false;
10392
let fd: number | null = null;
104-
let aclUnverified = false;
10593
try {
10694
fd = openSync(temporary, "wx", 0o600);
10795
writeFileSync(fd, `${token}\n`, "utf8");
@@ -111,12 +99,9 @@ function createTokenFile(path: string): { token: string; aclUnverified: boolean
11199
chmodSync(temporary, 0o600);
112100
const temporaryHardened = hardenSecretPath(temporary, { required: true });
113101
if (!temporaryHardened.ok) {
114-
if (allowUnverifiedAdminTokenAcl() && /timed out|ETIMEDOUT|budget exhausted|previous attempt timed out/i.test(temporaryHardened.diagnostics ?? "")) {
115-
console.warn(`[opencodex] management token temporary ACL unverified (${temporaryHardened.diagnostics}); continuing because OPENCODEX_ALLOW_UNVERIFIED_ADMIN_TOKEN_ACL is set`);
116-
aclUnverified = true;
117-
} else {
118-
throw new Error("management token temporary ACL hardening did not complete");
119-
}
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+
);
120105
}
121106
try {
122107
linkSync(temporary, path);
@@ -127,14 +112,11 @@ function createTokenFile(path: string): { token: string; aclUnverified: boolean
127112
}
128113
const finalHardened = hardenSecretPath(path, { required: true });
129114
if (!finalHardened.ok) {
130-
if (allowUnverifiedAdminTokenAcl() && /timed out|ETIMEDOUT|budget exhausted|previous attempt timed out/i.test(finalHardened.diagnostics ?? "")) {
131-
console.warn(`[opencodex] management token file ACL unverified (${finalHardened.diagnostics}); continuing because OPENCODEX_ALLOW_UNVERIFIED_ADMIN_TOKEN_ACL is set`);
132-
aclUnverified = true;
133-
} else {
134-
throw new Error("management token file ACL hardening did not complete");
135-
}
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+
);
136118
}
137-
return { token, aclUnverified };
119+
return token;
138120
} catch (error) {
139121
if (linked) removeBestEffort(path);
140122
throw error;
@@ -146,52 +128,30 @@ function createTokenFile(path: string): { token: string; aclUnverified: boolean
146128
}
147129
}
148130

149-
function ready(
150-
token: string,
151-
source: "environment" | "file",
152-
config: OcxConfig,
153-
options: { aclUnverified?: boolean } = {},
154-
): ManagementAuthState {
131+
function ready(token: string, source: "environment" | "file", config: OcxConfig): ManagementAuthState {
155132
if (isDataPlaneAdmissionSecret(token, config)) {
156133
return fail("management credential conflicts with a data-plane credential");
157134
}
158-
return {
159-
available: true,
160-
token,
161-
source,
162-
sessions: new Map(),
163-
...(options.aclUnverified ? { aclUnverified: true } : {}),
164-
};
165-
}
166-
167-
let lastManagementAuthAclUnverified = false;
168-
169-
/** Whether the current process accepted a file-backed admin token without verified NTFS ACL harden. */
170-
export function managementAuthAclUnverified(): boolean {
171-
return lastManagementAuthAclUnverified;
135+
return { available: true, token, source, sessions: new Map() };
172136
}
173137

174138
export function initializeManagementAuthState(config: OcxConfig): ManagementAuthState {
175139
const environmentToken = process.env.OPENCODEX_ADMIN_AUTH_TOKEN?.trim();
176140
if (environmentToken) {
177-
lastManagementAuthAclUnverified = false;
178141
return ready(environmentToken, "environment", config);
179142
}
180143
try {
181144
const path = adminApiTokenFilePath();
182-
const directoryUnverified = assertSafeDirectory(dirname(path));
183-
let loaded: { token: string; aclUnverified: boolean };
145+
assertSafeDirectory(dirname(path));
146+
let token: string;
184147
try {
185-
loaded = readExistingToken(path);
148+
token = readExistingToken(path);
186149
} catch (error) {
187150
if ((error as NodeJS.ErrnoException).code !== "ENOENT") throw error;
188-
loaded = createTokenFile(path);
151+
token = createTokenFile(path);
189152
}
190-
const aclUnverified = directoryUnverified || loaded.aclUnverified === true;
191-
lastManagementAuthAclUnverified = aclUnverified;
192-
return ready(loaded.token, "file", config, { aclUnverified });
153+
return ready(token, "file", config);
193154
} catch (error) {
194-
lastManagementAuthAclUnverified = false;
195155
return fail(error instanceof Error ? error.message : "management token initialization failed");
196156
}
197157
}
@@ -246,7 +206,11 @@ export function requireManagementAuth(
246206
config?: OcxConfig,
247207
): Response | null {
248208
if (!state.available) {
249-
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 });
250214
}
251215
const actual = req.headers.get("x-opencodex-api-key")?.trim()
252216
|| req.headers.get("authorization")?.replace(/^Bearer\s+/i, "").trim();

src/server/management/config-routes.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,6 @@ import { filterRequestLogs, getRequestLogEntries, type RequestLogEntry } from ".
5555
import { estimateComboCost, estimateRequestCost, normalizeCostTokens, tokensPerSecond } from "../../usage/cost";
5656
import type { PersistedUsageAttempt } from "../../usage/log";
5757
import { isAllowedRequestOrigin, jsonResponse, providerManagementConfigError, publicProviderBaseUrl, safeConfigDTO } from "../auth-cors";
58-
import { managementAuthAclUnverified } from "../management-auth";
5958
import { applySystemEnvToggle } from "../system-env";
6059
import { getCachedStartupHealth, invalidateStartupHealthCache } from "../startup-health-cache";
6160
import { runWindowsTrayAction } from "../windows-tray-control";
@@ -122,7 +121,6 @@ export async function handleConfigRoutes(ctx: ManagementContext): Promise<Respon
122121
hostname: config.hostname ?? "127.0.0.1",
123122
streamMode: config.streamMode ?? "auto",
124123
startupHealth: await getCachedStartupHealth(config),
125-
managementAuthAclUnverified: managementAuthAclUnverified(),
126124
codexRuntime: {
127125
path: displayCodexRuntimePath(resolved.runtime.command),
128126
version: resolved.runtime.version,

src/service.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@ import {
3434
} from "./lib/windows-elevation";
3535
import { defaultWinswEntry, installWinswService, startWinswService, stopWinswService, statusWinswRaw, uninstallWinswService, winswStatusSummary, WINSW_SERVICE_ID, WINSW_SHA256, WINSW_VERSION } from "./lib/winsw";
3636
import { hardenSecretDir, hardenSecretPath } from "./lib/windows-secret-acl";
37-
import { bakedAllowUnverifiedAdminTokenAcl } from "./lib/allow-unverified-admin-token-acl";
3837
import { windowsEnvIndirectBatchPathList, windowsEnvIndirectBatchValue } from "./lib/win-paths";
3938
import { recordOwnedConfigPath } from "./lib/config-ownership";
4039
import { maybeShowStarPrompt } from "./cli/star-prompt";
@@ -1040,7 +1039,6 @@ export function buildWindowsServiceScript(entry = cliEntry(), port = resolveServ
10401039
windowsBatchSet("OCX_SERVICE_LOG", serviceLogPath(), "path"),
10411040
windowsBatchSet("OCX_BUN", bun, "path"),
10421041
windowsBatchSet("OCX_CLI", cli, "path"),
1043-
windowsBatchSet("OPENCODEX_ALLOW_UNVERIFIED_ADMIN_TOKEN_ACL", bakedAllowUnverifiedAdminTokenAcl()),
10441042
'if exist "%OCX_API_TOKEN_FILE%" (',
10451043
' set /p OPENCODEX_API_AUTH_TOKEN=<"%OCX_API_TOKEN_FILE%"',
10461044
")",

0 commit comments

Comments
 (0)