mirror of
https://github.com/stablyai/orca.git
synced 2026-10-23 08:03:39 +00:00
test(codex): exercise live baseline read denial
This commit is contained in:
@@ -160,10 +160,10 @@ function syncSystemConfigIntoManagedCodexHomeUnsafe(
|
||||
): CodexConfigMirrorResult {
|
||||
const systemConfigPath = join(systemHomePath, 'config.toml')
|
||||
const runtimeConfigPath = join(runtimeHomePath, 'config.toml')
|
||||
// Why: `existsSync` answered `false` for a locked file exactly as for an absent
|
||||
// one, so a held handle on the RUNTIME config read as "no runtime config yet"
|
||||
// and the fresh-mirror branch below overwrote it wholesale. Neither side may
|
||||
// be acted on unless it was actually observed.
|
||||
// Why: `existsSync` collapses an indeterminate probe into the same `false` as
|
||||
// absence, so a transiently unavailable RUNTIME config could reach the fresh
|
||||
// mirror after the path recovered. Neither side may be acted on unless it was
|
||||
// actually observed.
|
||||
const systemConfigObservation = observeAgentStateFile(systemConfigPath)
|
||||
if (systemConfigObservation.kind === 'indeterminate') {
|
||||
return { status: 'refused-indeterminate', error: systemConfigObservation.error }
|
||||
|
||||
@@ -24,12 +24,10 @@ type StoredSettingsBaseline = {
|
||||
}
|
||||
|
||||
/**
|
||||
* Why callers need three answers, not two: the baseline records what Orca last
|
||||
* mirrored, so `null` drives "there is nothing to compare against, bootstrap
|
||||
* one". For a file that merely could not be READ, bootstrapping overwrites the
|
||||
* record of a change the user made inside Codex, and the next mirror pass then
|
||||
* writes the old value back over their edit. Absent and unparseable still map
|
||||
* to `absent` — rebuilding those is the intent.
|
||||
* Why callers need three answers, not two: without a readable baseline,
|
||||
* promotion cannot distinguish an in-Codex edit from Orca's last mirror. An
|
||||
* unreadable baseline must stall that mirror; absent and unparseable still map
|
||||
* to `absent` because rebuilding those is the intent.
|
||||
*/
|
||||
export type CodexSettingsBaselineObservation =
|
||||
| { kind: 'present'; baseline: CodexSettingsBaseline }
|
||||
|
||||
@@ -163,13 +163,6 @@ export function snapshotCodexRuntimeSettingsBaseline(
|
||||
runtimeHomePath = getOrcaManagedCodexHomePath(),
|
||||
conflicts: ReadonlyMap<string, CodexSettingsConflict> = new Map()
|
||||
): void {
|
||||
// Why: overwriting a baseline that could not be read records whatever the
|
||||
// runtime config says now as "Orca wrote this", so an edit the user made
|
||||
// inside Codex since the last pass is reclassified and never promoted again.
|
||||
// Absent and malformed are still rebuilt — that is what this function is for.
|
||||
if (observeCodexSettingsBaseline(runtimeHomePath).kind === 'indeterminate') {
|
||||
return
|
||||
}
|
||||
try {
|
||||
const runtimeTomlPath = join(runtimeHomePath, 'config.toml')
|
||||
// Why: record an empty baseline even for a missing runtime config, so Codex's first write still diffs and promotes.
|
||||
@@ -274,10 +267,10 @@ function promoteCodexRuntimeSettingsToSystemUnsafe(
|
||||
const writeTarget = resolvePromotionWriteTarget(systemTomlPath)
|
||||
// Why: a dangling symlink may target an unmade dir tree; create its real parent so the atomic temp write has a home.
|
||||
mkdirSync(dirname(writeTarget.path), { recursive: true, mode: 0o700 })
|
||||
// Why: this is the user's real ~/.codex/config.toml, and `existsSync` reading
|
||||
// `false` for a locked file sent it down the reconstruct branch below, which
|
||||
// replaces the canonical config with settings derived from Orca's runtime
|
||||
// copy. One read replaces the old existsSync + read pair and its TOCTOU gap.
|
||||
// Why: this is the user's real ~/.codex/config.toml, and an indeterminate
|
||||
// existence probe sent it down the reconstruct branch below, which replaces
|
||||
// the canonical config with settings derived from Orca's runtime copy. One
|
||||
// read replaces the old existsSync + read pair and its TOCTOU gap.
|
||||
// The indeterminate arm is a backstop rather than the live guard: an
|
||||
// unreadable system config already refused in readPromotedSettingValues,
|
||||
// because `writeTarget.path` always resolves to the same file as
|
||||
|
||||
@@ -27,9 +27,9 @@ export function getCodexConfigSyncStatus(
|
||||
const runtimeConfigPath = join(homes.runtimeHomePath, 'config.toml')
|
||||
// Why: a stall only withholds settings once a managed runtime config exists;
|
||||
// without one the mirror seeds it and there is nothing yet to fall behind.
|
||||
// But `existsSync` said "no runtime config" for one that was merely locked,
|
||||
// and this function then reported `synced` while the mirror was refusing —
|
||||
// telling the user their edits had been applied when nothing had run.
|
||||
// But `existsSync` never proved the config readable, so this function could
|
||||
// report `synced` while the mirror refused its content read — telling the
|
||||
// user their edits had been applied when nothing had run.
|
||||
const runtimeConfigObservation = observeAgentStateFile(runtimeConfigPath)
|
||||
if (runtimeConfigObservation.kind === 'absent') {
|
||||
return { state: 'synced', reason: null, systemConfigPath }
|
||||
|
||||
@@ -338,12 +338,10 @@ export function upsertHookTrustEntries(
|
||||
configPath: string,
|
||||
entries: readonly CodexTrustEntry[]
|
||||
): void {
|
||||
// Why: `existsSync` answered `false` for a locked config exactly as for an
|
||||
// absent one, so `existing` became '' and the upsert below rebuilt the file
|
||||
// from the trust entries alone — replacing the user's model, provider, MCP
|
||||
// servers, approvals and comments with a trust-only stub. Only a definitive
|
||||
// absence may seed from empty; every caller in hook-service already turns a
|
||||
// throw here into "trust entries could not be written. Run /hooks in Codex".
|
||||
// Why: `existsSync` collapses an indeterminate probe into the same `false` as
|
||||
// absence. If the path recovers before the write, rebuilding from '' replaces
|
||||
// the user's model, provider, MCP servers, approvals and comments with a
|
||||
// trust-only stub. Only a definitive absence may seed from empty.
|
||||
const observation = observe(() => readTomlFile(configPath))
|
||||
if (observation.kind === 'indeterminate') {
|
||||
throw observation.error
|
||||
|
||||
@@ -3,11 +3,12 @@ import type * as NodeFs from 'node:fs'
|
||||
import type * as NodeOs from 'node:os'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { join } from 'node:path'
|
||||
import { syncSystemConfigIntoManagedCodexHome } from './codex-config-mirror'
|
||||
import type { CodexTrustEntry } from './config-toml-trust'
|
||||
|
||||
// STA-4823: six shared Codex state files were rebuilt, erased or reported as
|
||||
// healthy after a read that had merely failed. Every one is reachable on the
|
||||
// host lane; the WSL-specific half of the catalogue stays in STA-4606.
|
||||
// healthy after a read that had merely failed. Host permission failures drive
|
||||
// four; the other guards cover transient transport and observation races.
|
||||
|
||||
const denials = vi.hoisted(() => {
|
||||
const state = {
|
||||
@@ -18,9 +19,8 @@ const denials = vi.hoisted(() => {
|
||||
state.paths.add(path)
|
||||
},
|
||||
/**
|
||||
* Content reads fail but `existsSync` still says the file is there — the
|
||||
* shape a Windows sharing violation takes, as distinct from `deny`, which
|
||||
* models an ACL denial where `existsSync` itself reports false.
|
||||
* Content reads fail but `existsSync` and metadata reads still succeed —
|
||||
* the measured host permission/sharing-denial shape.
|
||||
*/
|
||||
denyReads(path: string): void {
|
||||
state.readOnlyPaths.add(path)
|
||||
@@ -33,8 +33,9 @@ const denials = vi.hoisted(() => {
|
||||
* produce this. `chmod 000` on macOS and `icacls /deny (R)` on Windows both
|
||||
* leave `existsSync` TRUE, leave `stat`/`lstat` succeeding, and fail only
|
||||
* the content read (EACCES / EPERM errno -4048). The shape modelled here is
|
||||
* the UNC / `\\wsl$` transport, where an unreachable distro reports errno
|
||||
* `UNKNOWN` at every level and `existsSync` folds that to false.
|
||||
* a transient transport failure: a UNC / `\\wsl$` probe reports errno
|
||||
* `UNKNOWN`, `existsSync` folds it to false, and the transport recovers
|
||||
* before the following write.
|
||||
*
|
||||
* The distinction decides what the guard is worth: under a permission denial
|
||||
* the pre-fix code already failed safe, because `existsSync` was true so it
|
||||
@@ -46,6 +47,8 @@ const denials = vi.hoisted(() => {
|
||||
},
|
||||
release(path: string): void {
|
||||
state.paths.delete(path)
|
||||
state.readOnlyPaths.delete(path)
|
||||
state.existenceDeniedPaths.delete(path)
|
||||
},
|
||||
reset(): void {
|
||||
state.paths.clear()
|
||||
@@ -268,7 +271,7 @@ describe('STA-4823 D30 — an unreadable trust-grant ledger must not be rewritte
|
||||
it('skips the write instead of dropping every other home', () => {
|
||||
const ledgerPath = seedLedgerWithAnotherHome()
|
||||
const before = realFs.readFileSync(ledgerPath, 'utf-8')
|
||||
denials.deny(ledgerPath)
|
||||
denials.denyReads(ledgerPath)
|
||||
|
||||
writeCodexTrustGrantLedgerHome(
|
||||
runtimeHomePath,
|
||||
@@ -317,7 +320,7 @@ describe('STA-4823 D33 — an unreadable hooks.json is not an empty hooks.json',
|
||||
})
|
||||
})
|
||||
|
||||
describe('STA-4823 D26 — an unreadable settings baseline must not be overwritten', () => {
|
||||
describe('STA-4823 D26 — an unreadable settings baseline must stall the mirror', () => {
|
||||
function seedBaseline(): string {
|
||||
realFs.writeFileSync(
|
||||
baselinePath(),
|
||||
@@ -327,20 +330,29 @@ describe('STA-4823 D26 — an unreadable settings baseline must not be overwritt
|
||||
return realFs.readFileSync(baselinePath(), 'utf-8')
|
||||
}
|
||||
|
||||
it('leaves the record alone so the pending edit can still be promoted', () => {
|
||||
it('preserves the pending runtime edit until the baseline can be read', () => {
|
||||
const before = seedBaseline()
|
||||
realFs.writeFileSync(
|
||||
join(runtimeHomePath, 'config.toml'),
|
||||
'model = "edited-in-codex"\n',
|
||||
'utf-8'
|
||||
)
|
||||
denials.deny(baselinePath())
|
||||
const runtimeConfigPath = join(runtimeHomePath, 'config.toml')
|
||||
realFs.writeFileSync(runtimeConfigPath, 'model = "edited-in-codex"\n', 'utf-8')
|
||||
realFs.writeFileSync(join(systemHome(), 'config.toml'), 'model = "old-model"\n', 'utf-8')
|
||||
denials.denyReads(baselinePath())
|
||||
|
||||
syncSystemConfigIntoManagedCodexHome({ runtimeHomePath, systemHomePath: systemHome() })
|
||||
|
||||
// Before the fix the failed baseline read became "no baseline", promotion
|
||||
// returned an empty plan, and the mirror wrote the old system value over
|
||||
// the pending in-Codex edit.
|
||||
expect(realFs.readFileSync(runtimeConfigPath, 'utf-8')).toContain('edited-in-codex')
|
||||
expect(realFs.readFileSync(baselinePath(), 'utf-8')).toBe(before)
|
||||
})
|
||||
|
||||
it('does not replace a baseline after an indeterminate existence probe', () => {
|
||||
const before = seedBaseline()
|
||||
realFs.writeFileSync(join(runtimeHomePath, 'config.toml'), 'model = "edited"\n', 'utf-8')
|
||||
denials.denyExistence(baselinePath())
|
||||
|
||||
snapshotCodexRuntimeSettingsBaseline(runtimeHomePath)
|
||||
|
||||
// Before the fix this recorded "edited-in-codex" as Orca's own write, so
|
||||
// the user's in-Codex change was never promoted and the next mirror pass
|
||||
// wrote the old value back over it.
|
||||
expect(realFs.readFileSync(baselinePath(), 'utf-8')).toBe(before)
|
||||
})
|
||||
|
||||
@@ -396,9 +408,9 @@ describe('STA-4823 D15 — the sync status must not report synced while the mirr
|
||||
|
||||
const status = getCodexConfigSyncStatus({ runtimeHomePath, systemHomePath: systemHome() })
|
||||
|
||||
// Before the fix existsSync reported the locked runtime config as absent
|
||||
// and this returned `synced` — telling the user their edits had been
|
||||
// applied while nothing had run.
|
||||
// Before the fix existsSync collapsed this failed observation into absence
|
||||
// and returned `synced` — telling the user their edits had been applied
|
||||
// while nothing had run.
|
||||
expect(status).toMatchObject({ state: 'stalled', reason: 'managed-home-unavailable' })
|
||||
})
|
||||
|
||||
@@ -446,7 +458,7 @@ describe('STA-4823 D31 — an unreadable pane registry must not erase every attr
|
||||
it('does not persist an empty registry over the real one', () => {
|
||||
seedTwoAttributedPanes()
|
||||
const before = realFs.readFileSync(registryPath(), 'utf-8')
|
||||
denials.deny(registryPath())
|
||||
denials.denyReads(registryPath())
|
||||
|
||||
paneRegistry.recordCodexPaneAccount('pty-3', {
|
||||
selectionKey: 'host',
|
||||
@@ -461,7 +473,7 @@ describe('STA-4823 D31 — an unreadable pane registry must not erase every attr
|
||||
|
||||
it('does not cache the erasure, so attribution returns when the file does', () => {
|
||||
seedTwoAttributedPanes()
|
||||
denials.deny(registryPath())
|
||||
denials.denyReads(registryPath())
|
||||
|
||||
expect(paneRegistry.getCodexPaneAccount(PANE)).toBeNull()
|
||||
|
||||
|
||||
Reference in New Issue
Block a user