Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 15 additions & 3 deletions src/codex/inject.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import {
} from "./user-identity";
import {
markJournalInjectedState,
journaledInjectedOpenaiBaseUrl,
removeJournal,
restoreJournalState,
writeJournal,
Expand All @@ -52,6 +53,7 @@ import {
providerTableStart,
providerTableString,
rootTomlString,
stripJournaledOpenaiBaseUrl,
tomlStringPattern,
} from "./injected-marker";
import {
Expand Down Expand Up @@ -1138,12 +1140,18 @@ interface StripOpencodexConfigResult {
*/
function stripOpencodexConfigResult(
content: string,
journaledBaseUrl: string | null = null,
): StripOpencodexConfigResult {
let out = content;
const hadRootOcxProvider =
readRootTomlString(out, "model_provider") === "opencodex";
const hadInjectedBaseUrl = hasInjectedOpenaiBaseUrl(out);
// #1798: marker adjacency is FORMATTING evidence, and a Codex app rewrite keeps values
// while dropping comments. Fall back to VALUE evidence -- the exact URL we recorded
// writing -- so an app-rewritten config is still recognized as ours.
const hadInjectedBaseUrl = hasInjectedOpenaiBaseUrl(out)
|| (journaledBaseUrl !== null && rootTomlString(out, "openai_base_url") === journaledBaseUrl);
out = stripInjectedOpenaiBaseUrl(out); // before removeOcxSection — it keys on the marker line too
out = stripJournaledOpenaiBaseUrl(out, journaledBaseUrl);
if (out.includes("[model_providers.opencodex]")) {
out = removeOcxSection(out);
}
Expand Down Expand Up @@ -1195,8 +1203,12 @@ export function removeCodexConfig(
// The unchanged fast path compares in LF space so an untouched file is never rewritten.
const eol = dominantEol(rawContent);
const content = applyEol(rawContent, "\n");
const had = hasOpencodexRouting(content);
const stripped = stripOpencodexConfigResult(content);
// Read the recorded injection once: the strip below consumes it, and so does the
// ownership verdict, which must agree with what was actually removed.
const journaledBaseUrl = journaledInjectedOpenaiBaseUrl();
const had = hasOpencodexRouting(content)
|| (journaledBaseUrl !== null && rootTomlString(content, "openai_base_url") === journaledBaseUrl);
const stripped = stripOpencodexConfigResult(content, journaledBaseUrl);
if (had || stripped.content !== content) {
atomicWriteFile(CODEX_CONFIG_PATH, applyEol(stripped.content, eol));
}
Expand Down
28 changes: 28 additions & 0 deletions src/codex/injected-marker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,34 @@ export function providerTableString(content: string, provider: string, key: stri
return null;
}

/**
* Drop a root `openai_base_url` whose VALUE is the one a recorded injection wrote.
*
* #1798: the marker-adjacency rule below is formatting evidence, and the Codex app
* reserializes the file -- values kept, comments dropped. This rule is value evidence
* instead, so it still recognizes our URL after that rewrite. It is deliberately an
* EXACT value match against what we recorded writing: a user gateway we never wrote
* cannot match, so restore can never delete a URL that was not ours.
*/
export function stripJournaledOpenaiBaseUrl(content: string, injectedUrl: string | null): string {
if (!injectedUrl) return content;
const lines = content.split(String.fromCharCode(10));
const firstTable = lines.findIndex(l => /^\s*\[/.test(l));
const rootEnd = firstTable === -1 ? lines.length : firstTable;
const drop = new Set<number>();
for (let i = 0; i < rootEnd; i++) {
const line = lines[i]!;
if (!isRootOpenaiBaseUrlLine(line)) continue;
if (rootTomlString(line, "openai_base_url") !== injectedUrl) continue;
drop.add(i);
// Take an ownership marker directly above it too, so repeated cycles cannot
// accumulate orphaned comments.
if (i > 0 && lines[i - 1]!.includes(OCX_SECTION_MARKER)) drop.add(i - 1);
}
if (drop.size === 0) return content;
return lines.filter((_, i) => !drop.has(i)).join(String.fromCharCode(10));
}

export function hasInjectedOpenaiBaseUrl(content: string): boolean {
const lines = content.split("\n");
const firstTable = lines.findIndex(l => /^\s*\[/.test(l));
Expand Down
26 changes: 25 additions & 1 deletion src/codex/journal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { createHash } from "node:crypto";
import { existsSync, readFileSync, unlinkSync } from "node:fs";
import { join } from "node:path";
import { atomicWriteFile } from "../config";
import { hasInjectedCodexRouting } from "./injected-marker";
import { hasInjectedCodexRouting, rootTomlString } from "./injected-marker";
import { CODEX_HOME, CODEX_CONFIG_PATH, CODEX_PROFILE_PATH } from "./paths";

/**
Expand All @@ -22,6 +22,15 @@ interface Journal {
originalProfile: string | null;
injectedConfigHash?: string;
injectedProfileHash?: string | null;
/**
* The exact root `openai_base_url` this injection wrote, when it wrote one.
*
* #1798: ownership used to be inferred from a marker COMMENT on the preceding line,
* which a reserializing Codex app deletes while keeping the value. Recording the value
* we actually wrote makes ownership provable from evidence rather than from formatting,
* and it is what lets restore tell OUR loopback URL apart from a gateway the user set.
*/
injectedOpenaiBaseUrl?: string | null;
pid: number;
timestamp: string;
}
Expand Down Expand Up @@ -96,9 +105,24 @@ export function markJournalInjectedState(config: string, profile: string | null)
if (journal.injectedConfigHash) return;
journal.injectedConfigHash = sha256(config) ?? undefined;
journal.injectedProfileHash = sha256(profile);
// Read from the bytes we are about to install, not from the file: another writer may
// already have rewritten it, and then the recorded value would describe their config.
journal.injectedOpenaiBaseUrl = rootTomlString(config, "openai_base_url");

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Record only a URL that the injection owns

When config.toml already contains an unmarked root openai_base_url, setRootOpenaiBaseUrl deliberately preserves it, but this assignment nevertheless records that user-owned value as injected. If Codex later reserializes the file differently or the user edits any other setting, the journal hash no longer matches and stripJournaledOpenaiBaseUrl deletes the user's gateway because its value matches this record. Record null unless the transform actually inserted or owned the URL, and cover the fallback path with a pre-existing URL plus an unrelated edit.

AGENTS.md reference: src/AGENTS.md:L7-L10

Useful? React with 👍 / 👎.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Refresh the recorded URL after reinjection

When routing is reinjected while the existing journal remains—for example, after changing the proxy port or hostname—the config receives the new URL, but the injectedConfigHash guard above causes this new field to retain the first injection's URL. If the Codex app then drops the marker comment, fallback restore compares against the stale value and leaves the latest proxy URL in place, so plain Codex still targets a stopped endpoint. Refresh the injected-URL evidence on every successful reinjection without replacing the original snapshot.

AGENTS.md reference: src/AGENTS.md:L7-L10

Useful? React with 👍 / 👎.

atomicWriteFile(JOURNAL_PATH, JSON.stringify(journal));
}

/**
* The root `openai_base_url` the last injection wrote, or null when it wrote none.
*
* #1798: the fallback strip recognizes an injected URL by the marker COMMENT above it,
* and a Codex app rewrite keeps values while dropping comments. This is the evidence that
* survives such a rewrite, so restore can still prove the URL is ours -- and, just as
* importantly, prove that a DIFFERENT URL is not.
*/
export function journaledInjectedOpenaiBaseUrl(): string | null {
return readJournal()?.injectedOpenaiBaseUrl ?? null;
}

export function removeJournal(): void {
try { unlinkSync(JOURNAL_PATH); } catch { /* ignore */ }
}
Expand Down
100 changes: 100 additions & 0 deletions tests/codex-restore-app-rewrite.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
import { describe, expect, test, beforeEach, afterEach } from "bun:test";
import { mkdtempSync, rmSync, writeFileSync, readFileSync } from "node:fs";
import { spawnSync } from "node:child_process";
import { tmpdir } from "node:os";
import { dirname, join } from "node:path";
import { fileURLToPath } from "node:url";

/**
* #1798: the Codex app rewrites config.toml AFTER injection, so the journal's
* exact-bytes restore no longer fires and the fallback strip is the only thing left.
* That fallback recognizes an injected `openai_base_url` ONLY by the marker comment
* on the line above it. An app rewrite reserializes the file and drops the comment, so
* the proxy URL stops being recognized as ours and survives `ocx stop` / `ocx restore`
* while the command reports success -- leaving plain Codex pointed at a dead port.
*
* These tests reproduce that state literally: inject, drop every comment the way a TOML
* reserializer would (values kept, comments gone), then restore.
*/

const repoRoot = dirname(fileURLToPath(new URL("../package.json", import.meta.url)));

/** Inject, simulate the app's comment-dropping rewrite, then restore. */
const INJECT_REWRITE_RESTORE = [
'const fs = require("fs");',
'const path = require("path");',
'const { injectCodexConfig, restoreNativeCodex } = require("./src/codex/inject");',
"(async () => {",
" await injectCodexConfig(10100, {",
" port: 10100,",
" providers: {},",
' defaultProvider: "openai",',
' injectionModel: "gpt-5.6-sol",',
' injectionEffort: "high",',
" }, { catalogPath: null });",
' const configPath = path.join(process.env.CODEX_HOME, "config.toml");',
' const injected = fs.readFileSync(configPath, "utf8");',
" // Exactly what a reserializing app writer produces: every VALUE survives,",
" // every COMMENT -- including our ownership marker -- is gone.",
" const rewritten = injected",
" .split(String.fromCharCode(10))",
' .filter(line => !line.trim().startsWith("#"))',
" .join(String.fromCharCode(10));",
' fs.writeFileSync(configPath, rewritten, "utf8");',
" const result = restoreNativeCodex();",
" console.log(JSON.stringify({ success: result.success, message: result.message }));",
"})();",
].join("\n");

function runScript(codexHome: string, script: string): { stdout: string; stderr: string; status: number } {
const result = spawnSync(process.execPath, ["--eval", script], {
cwd: repoRoot,
env: { ...process.env, CODEX_HOME: codexHome },
encoding: "utf8",
});
return { stdout: result.stdout?.trim() ?? "", stderr: result.stderr?.trim() ?? "", status: result.status ?? 1 };
}

describe("#1798 restore after the Codex app rewrites the config", () => {
let testDir: string;

beforeEach(() => {
testDir = mkdtempSync(join(tmpdir(), "ocx-1798-"));
});

afterEach(() => {
rmSync(testDir, { recursive: true, force: true });
});

test("an unmarked injected openai_base_url is still removed", () => {
writeFileSync(join(testDir, "config.toml"), '# original config\nmodel = "gpt-5.5"\n', "utf8");

const r = runScript(testDir, INJECT_REWRITE_RESTORE);
if (r.status !== 0) throw new Error(r.stderr || r.stdout);

const restored = readFileSync(join(testDir, "config.toml"), "utf8");
// The reported defect: the proxy URL survives, so plain Codex talks to a dead port.
expect(restored).not.toContain("openai_base_url");
expect(restored).not.toContain("127.0.0.1:10100");
// The user's own pre-injection content is still theirs.
expect(restored).toContain("gpt-5.5");
});

test("a user's own openai_base_url written before injection is preserved", () => {
// The mirror-image risk of the fix: stripping ANY unmarked openai_base_url would
// delete a URL we never wrote. The journaled baseline is the arbiter, not the key name.
writeFileSync(
join(testDir, "config.toml"),
'openai_base_url = "https://my-own-gateway.example/v1"\nmodel = "gpt-5.5"\n',
"utf8",
);

const r = runScript(testDir, INJECT_REWRITE_RESTORE);
if (r.status !== 0) throw new Error(r.stderr || r.stdout);

const restored = readFileSync(join(testDir, "config.toml"), "utf8");
expect(restored).toContain("https://my-own-gateway.example/v1");
expect(restored).not.toContain("127.0.0.1:10100");
});
});

Loading