diff --git a/src/codex/config-write-lock.ts b/src/codex/config-write-lock.ts index de12142fdb9..5ca68e04663 100644 --- a/src/codex/config-write-lock.ts +++ b/src/codex/config-write-lock.ts @@ -317,6 +317,38 @@ export function publishConfigWriteTarget( }); } +export function removeConfigWriteEntry(configPath: string, held: LockHandle, path: string): void { + assertConfigWriteDestination(configPath, held); + watchConfigWriteTargets(held, [path]); + const watched = targets.get(held)!; + const expected = watched.get(path)!; + const current = destination(path); + if (expected.canonical !== current.canonical || expected.inode !== current.inode + || current.canonical !== join(realpathSync.native(dirname(resolve(path))), basename(resolve(path)))) { + throw new Error("Codex artifact destination changed; files and recovery evidence were preserved."); + } + if (expected.inode === null) return; + + const entry = join(realpathSync.native(dirname(resolve(path))), basename(resolve(path))); + const identity = (file: { dev: bigint; ino: bigint }) => `${file.dev}:${file.ino}`; + const before = lstatSync(entry, { bigint: true }); + if (before.isSymbolicLink() || !before.isFile() || identity(before) !== expected.inode) { + throw new Error("Codex artifact destination changed; files and recovery evidence were preserved."); + } + assertConfigWriteDestination(configPath, held); + const now = lstatSync(entry, { bigint: true }); + if (now.isSymbolicLink() || !now.isFile() || identity(now) !== expected.inode) { + throw new Error("Codex artifact destination changed; files and recovery evidence were preserved."); + } + unlinkSync(entry); + assertConfigWriteDestination(configPath, held); + const after = destination(path); + if (after.inode !== null || after.canonical !== expected.canonical) { + throw new Error("Codex artifact destination changed; recovery evidence was preserved."); + } + watched.set(path, after); +} + export function configWriteLockFailureMessage(failure: Exclude): string { if (failure.error === "missing-parent") return `Codex configuration parent is missing or unresolved at ${failure.detail}; no files were changed.`; return failure.error === "unsafe" diff --git a/src/codex/inject/config-write-section.ts b/src/codex/inject/config-write-section.ts index 0b39f07c9e8..e343103effb 100644 --- a/src/codex/inject/config-write-section.ts +++ b/src/codex/inject/config-write-section.ts @@ -1,7 +1,7 @@ import type { AtomicWriteHooks } from "../../config/atomic-write"; import { existsSync, readFileSync } from "node:fs"; import { withConfigMutationLockSync } from "../../config"; -import { ConfigWriteLockRefusal, assertConfigWriteDestination, publishConfigWriteTarget, watchConfigWriteTargets, withConfigWriteLockHeld, type LockHandle } from "../config-write-lock"; +import { ConfigWriteLockRefusal, assertConfigWriteDestination, publishConfigWriteTarget, removeConfigWriteEntry, watchConfigWriteTargets, withConfigWriteLockHeld, type LockHandle } from "../config-write-lock"; import { CODEX_CONFIG_PATH, CODEX_PROFILE_PATH } from "../paths"; import { getCodexHome } from "../paths"; import { codexHomeIsAbsent } from "../codex-home-owner"; @@ -16,6 +16,9 @@ export function publishCodexArtifact(path: string, held: LockHandle | undefin if (!held) throw new Error("Codex artifact publication requires a held configuration lock"); return publishConfigWriteTarget(CODEX_CONFIG_PATH, held, path, run); } +export function removeCodexProfileEntry(held: LockHandle): void { + removeConfigWriteEntry(CODEX_CONFIG_PATH, held, CODEX_PROFILE_PATH); +} /** Courtesy cleanup re-reads provider ownership inside the same config section. */ export function cleanExternalProviderJournal(beforeClientWrite?: () => void): void { if (codexHomeIsAbsent(getCodexHome())) return; diff --git a/src/codex/inject/remove.ts b/src/codex/inject/remove.ts index cdba5430d3e..bed3de55882 100644 --- a/src/codex/inject/remove.ts +++ b/src/codex/inject/remove.ts @@ -1,8 +1,8 @@ -import { beginCodexWriteSection, publishCodexArtifact } from "./config-write-section"; -import { existsSync, readFileSync, unlinkSync } from "node:fs"; +import { beginCodexWriteSection, publishCodexArtifact, removeCodexProfileEntry } from "./config-write-section"; +import { existsSync, lstatSync, readFileSync } from "node:fs"; import { normalizeStructuralWhitespace, rootAssignmentKey, rootSourceLines, sourceAssignment, sourceText } from "../toml-source-lines"; import { atomicWriteFile } from "../../config"; -import { ConfigWriteLockRefusal, configWriteLockFailureMessage, withConfigWriteLockHeld } from "../config-write-lock"; +import { ConfigWriteLockRefusal, configWriteLockFailureMessage, withConfigWriteLockHeld, withConfigWriteRecovery } from "../config-write-lock"; import type { LockHandle } from "../config-write-lock"; import { REALTIME_WS_BASE_URL_KEY, @@ -204,11 +204,20 @@ export function removeCodexConfig( if (historyError && !(historyDisposition !== "refuse-on-any" && historyError === HISTORY_RELABEL_STANDS_DOWN)) { return { success: false, message: `Codex configuration preserved: ${historyError}. Native writer coordination is required.` }; } - const locked = withConfigWriteLockHeld(CODEX_CONFIG_PATH, options.heldConfigWriteLock, held => { + const locked = withConfigWriteLockHeld(CODEX_CONFIG_PATH, options.heldConfigWriteLock, held => withConfigWriteRecovery(CODEX_CONFIG_PATH, held, () => { beginCodexWriteSection(held); + if (!options.preserveProfile) { + try { + if (lstatSync(CODEX_PROFILE_PATH).isSymbolicLink()) { + return { success: false, message: "Codex profile is a symbolic link; configuration and recovery evidence were preserved." }; + } + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== "ENOENT") throw error; + } + } if (!existsSync(CODEX_CONFIG_PATH)) { if (!options.preserveProfile && existsSync(CODEX_PROFILE_PATH)) - publishCodexArtifact(CODEX_PROFILE_PATH, held, (path, hooks) => { hooks.validateBeforeRename?.(path); unlinkSync(path); hooks.afterRename?.(path); }); + removeCodexProfileEntry(held); return { success: true, message: `Codex config not found; no native restore was needed${options.preserveProfile ? "." : ", and the opencodex profile was removed if present."}`, @@ -248,7 +257,7 @@ export function removeCodexConfig( publishCodexArtifact(CODEX_CONFIG_PATH, held, (path, hooks) => atomicWriteFile(path, applyEol(finalContent, eol), undefined, hooks)); } if (!options.preserveProfile && existsSync(CODEX_PROFILE_PATH)) - publishCodexArtifact(CODEX_PROFILE_PATH, held, (path, hooks) => { hooks.validateBeforeRename?.(path); unlinkSync(path); hooks.afterRename?.(path); }); + removeCodexProfileEntry(held); const retainedNote = retainedBlock === null ? "" : " Kept [model_providers.opencodex] so conversations already tagged opencodex still open;" @@ -272,7 +281,7 @@ export function removeCodexConfig( message: removedMessage, ...(retainedBlock === null ? {} : { retainedProviderTable: retainedBlock.replace(/\n+$/, "").split("\n") }), }; - }); + })); if (!locked.ok) return { success: false, retryable: locked.error === "locked", message: configWriteLockFailureMessage(locked) }; return locked.value; } diff --git a/src/codex/journal.ts b/src/codex/journal.ts index daca6e9fd17..048b624345f 100644 --- a/src/codex/journal.ts +++ b/src/codex/journal.ts @@ -1,7 +1,7 @@ import { beginCodexWriteSection, publishCodexArtifact } from "./inject/config-write-section"; import { createHash } from "node:crypto"; import type { AtomicWriteHooks } from "../config/atomic-write"; -import { existsSync, readFileSync, unlinkSync } from "node:fs"; +import { existsSync, lstatSync, readFileSync, unlinkSync } from "node:fs"; import { join } from "node:path"; import { atomicWriteFile } from "../config"; import { assertCodexHomeOwner, codexHomeIsAbsent, CODEX_HOME_JOURNAL_FILE, CodexHomeOwnerRefusal, opencodexHomeForInjection, readCodexHomeJournal, type CodexHomeOwnerRefusalReason } from "./codex-home-owner"; @@ -347,6 +347,21 @@ export function restoreJournalState( }; } + // A matching byte hash does not authorize following a newly substituted profile link. + // Refuse the complete pair before restoring config or deleting its recovery journal. + let profileIsSymlink = false; + if (profileUnchanged && (!comparison.profileAlreadyOriginal || comparison.originalProfile === null)) { + try { profileIsSymlink = lstatSync(CODEX_PROFILE_PATH).isSymbolicLink(); } + catch (error) { if ((error as NodeJS.ErrnoException).code !== "ENOENT") throw error; } + } + if (profileIsSymlink) { + return { + configRestored: comparison.configAlreadyOriginal, profileRestored: false, + configRewritten: false, profileRewritten: false, + configChanged: !configUnchanged, profileChanged: false, + complete: false, unverified: false, profileRestoreFailed: true as const, + }; + } assertCodexHomeOwner(CODEX_HOME); let configRestored = comparison.configAlreadyOriginal; let profileRestored = comparison.profileAlreadyOriginal; diff --git a/structure/codex-home.md b/structure/codex-home.md index f3d3f85b81c..4624fbc3d4e 100644 --- a/structure/codex-home.md +++ b/structure/codex-home.md @@ -79,7 +79,7 @@ and absence is distinct from an empty file. The injector checks a retained hashl the same `baselineContent` it snapshots, plus the current profile, before writing or assigning a new injected hash. Native content can establish a fresh snapshot; routed content cannot promote an unverified older original. Existing hash-backed edit preservation and external-provider opt-out -remain separate paths. +remain separate paths. Removal with `preserveProfile: false` refuses a final-component profile symlink before changing config or journal state. Profile cleanup uses the held configuration lock, canonical parent and identity rechecks; its pathname unlink is not atomic against non-cooperating writers. A matching journal refuses profiles eligible for restoration or removal when they are symlinks, including dangling generated links, before changing either artifact or recovery evidence. Config publication is covered by `withConfigWriteRecovery` if later profile removal throws. `src/codex/journal.ts` distinguishes restored state from actual writes with `configRewritten` and `profileRewritten`; removing a generated profile counts as a write, while already-original diff --git a/tests/codex-integration/codex-journal-recovery.test.ts b/tests/codex-integration/codex-journal-recovery.test.ts index 2fe82661757..763cdc72cf0 100644 --- a/tests/codex-integration/codex-journal-recovery.test.ts +++ b/tests/codex-integration/codex-journal-recovery.test.ts @@ -260,4 +260,96 @@ describe("codex-journal recovery diagnostics", () => { else expect(existsSync(join(testDir, "opencodex.config.toml"))).toBe(mode === "unlink-EPERM"); }); } + + test("profile removal refusal rolls back published config", () => { + const journal = seed("process", injected, true); + const result = runScript(testDir, ` + const fs = require("node:fs"); + const path = require("node:path"); + const { spyOn } = require("bun:test"); + const section = require("./src/codex/inject/config-write-section"); + const { removeCodexConfig } = require("./src/codex/inject/remove"); + let reached = false, publishedFirst = false, refused = false; + const hook = spyOn(section, "removeCodexProfileEntry").mockImplementation(() => { + reached = true; + publishedFirst = !fs.readFileSync(path.join(process.env.CODEX_HOME, "config.toml"), "utf8").includes('model_provider = "opencodex"'); + throw new Error("fixture profile removal refused"); + }); + try { removeCodexConfig(); } catch { refused = true; } + finally { hook.mockRestore(); } + console.log(JSON.stringify({ reached, publishedFirst, refused })); + `); + expect(result.status, result.stderr).toBe(0); + expect(JSON.parse(result.stdout)).toEqual({ reached: true, publishedFirst: true, refused: true }); + expect(readFileSync(join(testDir, "config.toml"), "utf8")).toBe(injected); + expect(readFileSync(join(testDir, "opencodex.config.toml"), "utf8")).toBe(profile); + expect(readFileSync(join(testDir, "opencodex-journal.json"), "utf8")).toBe(journal); + }); + test("dangling generated-profile metadata refuses the complete pair", () => { + const journal = seed("process", injected, true); + const result = runScript(testDir, ` + const fs = require("node:fs"); + const path = require("node:path"); + const { spyOn } = require("bun:test"); + const profilePath = path.join(process.env.CODEX_HOME, "opencodex.config.toml"); + fs.unlinkSync(profilePath); + const lstat = fs.lstatSync; + const hook = spyOn(fs, "lstatSync").mockImplementation((target, ...args) => + path.basename(String(target)) === "opencodex.config.toml" + ? { isSymbolicLink: () => true } + : lstat(target, ...args)); + let out; + try { out = require("./src/codex/journal").restoreJournalState(); } + finally { hook.mockRestore(); } + console.log(JSON.stringify(out)); + `); + expect(result.status, result.stderr).toBe(0); + expect(JSON.parse(result.stdout)).toMatchObject({ complete: false, profileRestoreFailed: true, configRewritten: false, profileRewritten: false }); + expect(readFileSync(join(testDir, "config.toml"), "utf8")).toBe(injected); + expect(readFileSync(join(testDir, "opencodex-journal.json"), "utf8")).toBe(journal); + }); + test("generated profile disappearance at type observation is treated as absence", () => { + seed("process", injected, true); + const result = runScript(testDir, ` + const fs = require("node:fs"); + const path = require("node:path"); + const { spyOn } = require("bun:test"); + const profilePath = path.join(process.env.CODEX_HOME, "opencodex.config.toml"); + const lstat = fs.lstatSync; + let vanished = false; + const hook = spyOn(fs, "lstatSync").mockImplementation((target, ...args) => { + if (!vanished && path.basename(String(target)) === "opencodex.config.toml") { + vanished = true; + fs.unlinkSync(profilePath); + throw Object.assign(new Error("fixture missing profile"), { code: "ENOENT" }); + } + return lstat(target, ...args); + }); + let out; + try { out = require("./src/codex/journal").restoreJournalState(); } + finally { hook.mockRestore(); } + console.log(JSON.stringify({ vanished, out })); + `); + expect(result.status, result.stderr).toBe(0); + expect(JSON.parse(result.stdout)).toMatchObject({ vanished: true, out: { complete: true, profileRestored: true } }); + expect(readFileSync(join(testDir, "config.toml"), "utf8")).toBe(original); + expect(existsSync(join(testDir, "opencodex.config.toml"))).toBe(false); + expect(existsSync(join(testDir, "opencodex-journal.json"))).toBe(false); + }); + test.skipIf(process.platform === "win32")("a real dangling generated-profile symlink preserves recovery evidence", () => { + const journal = seed("process", injected, true); + const result = runScript(testDir, ` + const fs = require("node:fs"); + const path = require("node:path"); + const profilePath = path.join(process.env.CODEX_HOME, "opencodex.config.toml"); + fs.unlinkSync(profilePath); + fs.symlinkSync(path.join(process.env.CODEX_HOME, "missing-profile-target.toml"), profilePath); + const out = require("./src/codex/journal").restoreJournalState(); + console.log(JSON.stringify({ out, link: fs.lstatSync(profilePath).isSymbolicLink() })); + `); + expect(result.status, result.stderr).toBe(0); + expect(JSON.parse(result.stdout)).toMatchObject({ link: true, out: { complete: false, profileRestoreFailed: true } }); + expect(readFileSync(join(testDir, "config.toml"), "utf8")).toBe(injected); + expect(readFileSync(join(testDir, "opencodex-journal.json"), "utf8")).toBe(journal); + }); }); diff --git a/tests/codex-integration/codex-journal.test.ts b/tests/codex-integration/codex-journal.test.ts index 4af3e35ab70..5cd189eb3d5 100644 --- a/tests/codex-integration/codex-journal.test.ts +++ b/tests/codex-integration/codex-journal.test.ts @@ -1,6 +1,6 @@ import { describe, expect, test, beforeEach, afterEach, setDefaultTimeout } from "bun:test"; import { createHash } from "node:crypto"; -import { mkdtempSync, rmSync, writeFileSync, readFileSync, existsSync } from "node:fs"; +import { mkdtempSync, rmSync, writeFileSync, readFileSync, existsSync, lstatSync, symlinkSync } from "node:fs"; import { spawnSync } from "node:child_process"; import { tmpdir } from "node:os"; import { dirname, join } from "node:path"; @@ -420,6 +420,80 @@ describe("codex-journal", () => { expect(existsSync(join(testDir, "opencodex.config.toml"))).toBe(false); }); + for (const originalProfile of [null, 'model_provider = "openai"\n']) { + for (const symlinked of [true, false]) { + test.skipIf(process.platform === "win32")(`native journal restore ${symlinked ? "preserves a symlinked" : "restores a regular"} ${originalProfile === null ? "generated" : "existing"} profile before config writes`, () => { + const configPath = join(testDir, "config.toml"); + const profilePath = join(testDir, "opencodex.config.toml"); + const targetPath = join(testDir, "journal-profile-target.toml"); + const journalPath = join(testDir, "opencodex-journal.json"); + const injectedConfig = '# Auto-injected by opencodex\nopenai_base_url = "http://127.0.0.1:10100/v1"\n'; + const injectedProfile = 'model_provider = "opencodex"\n'; + const journal = JSON.stringify({ version: 1, originalConfig: Buffer.from('model_provider = "openai"\n').toString("base64"), + originalProfile: originalProfile === null ? null : Buffer.from(originalProfile).toString("base64"), + injectedConfigHash: createHash("sha256").update(injectedConfig).digest("hex"), + injectedProfileHash: createHash("sha256").update(injectedProfile).digest("hex"), + pid: 999999, timestamp: new Date().toISOString() }); + writeFileSync(configPath, injectedConfig); + writeFileSync(targetPath, injectedProfile); + writeFileSync(journalPath, journal); + if (symlinked) symlinkSync(targetPath, profilePath, "file"); + else writeFileSync(profilePath, injectedProfile); + const result = runScript(testDir, ` + const { restoreNativeCodex } = require("./src/codex/inject"); + const out = restoreNativeCodex({ skipHistory: true }); + console.log(JSON.stringify({success: out.success, config: out.artifacts.config})); + `); + expect(result.status).toBe(0); + const out = JSON.parse(result.stdout); + if (symlinked) { + expect(out.success).toBe(false); + expect(out.config.state).toBe("failed"); + expect(readFileSync(configPath, "utf8")).toBe(injectedConfig); + expect(readFileSync(journalPath, "utf8")).toBe(journal); + expect(existsSync(profilePath)).toBe(true); + expect(lstatSync(profilePath).isSymbolicLink()).toBe(true); + expect(out.config.changed).toBe(false); + } else { + expect(out.config.state).toBe("ok"); + expect(readFileSync(configPath, "utf8")).toBe('model_provider = "openai"\n'); + expect(existsSync(journalPath)).toBe(false); + if (originalProfile === null) expect(existsSync(profilePath)).toBe(false); + else expect(readFileSync(profilePath, "utf8")).toBe(originalProfile); + } + expect(existsSync(targetPath)).toBe(true); + expect(readFileSync(targetPath, "utf8")).toBe(injectedProfile); + }); + } + } + + test.skipIf(process.platform === "win32")("removeCodexConfig refuses a symlinked profile without changing its target or config", () => { + const configPath = join(testDir, "config.toml"); + const profilePath = join(testDir, "opencodex.config.toml"); + const targetPath = join(testDir, "external-profile.toml"); + const journalPath = join(testDir, "opencodex-journal.json"); + const config = '# Auto-injected by opencodex\nopenai_base_url = "http://127.0.0.1:10100/v1"\n'; + const target = 'model_provider = "openai"\nmodel = "external-profile"\n'; + const journal = '{"recovery":"sentinel"}\n'; + writeFileSync(configPath, config, "utf8"); + writeFileSync(targetPath, target, "utf8"); + writeFileSync(journalPath, journal, "utf8"); + symlinkSync(targetPath, profilePath, "file"); + + const r = runScript(testDir, ` + const { removeCodexConfig } = require("./src/codex/inject"); + console.log(JSON.stringify(removeCodexConfig())); + `); + + expect(r.status).toBe(0); + expect(JSON.parse(r.stdout).success).toBe(false); + expect(JSON.parse(r.stdout).message).toContain("symbolic link"); + expect(readFileSync(configPath, "utf8")).toBe(config); + expect(readFileSync(targetPath, "utf8")).toBe(target); + expect(readFileSync(journalPath, "utf8")).toBe(journal); + expect(existsSync(profilePath)).toBe(true); + }); + test("removeCodexConfig reports damaged managed-default cleanup and preserves the ambiguous value", () => { writeFileSync(join(testDir, "config.toml"), [ "# Auto-injected by opencodex",