Skip to content

[Bug]: Windows ACL hardening can strand zero-ACE atomic temp files #596

Description

@luvs01

Client or integration

Codex App

Area

Service lifecycle

Summary

On Windows, OpenCodex 2.7.42 can leave protected atomic-write temp files with no usable access-control entry (ACE) for the current user. The affected files can contain authentication, configuration, or response-state data and then cannot be reliably read, truncated, renamed, or removed by the process that created them.

The expected behavior is that the current user retains explicit Full Control throughout ACL hardening, including when a later icacls step times out or fails.

Reproduction

The production occurrence is timing-dependent, but the failure is deterministic with an injected command runner:

  1. Begin with a Windows temp file whose current-user access is inherited.
  2. Run the current runIcacls() sequence: /inheritance:r, broad-group removal, then /grant:r CURRENTUSER:(F).
  3. Inject a timeout or failure in the broad-group removal step after inheritance has already been removed.
  4. Inspect the temp file ACL and attempt to read, rename, or unlink it.
  5. Observe that the file is owned by the current user but has a protected DACL with zero ACEs, so ordinary access and cleanup fail until a current-user Full Control ACE is restored.

This corresponds to the stale files observed in production. A regression test can model the same sequence without depending on timing.

Version

2.7.42

Operating system

Microsoft Windows 11 Pro 25H2, x64 (build 26200.8894)

Provider and model

Not provider-specific.

Logs or error output

No application stack trace is required to identify the failure. The decisive evidence is the ACL state: the stale temp files are owned by the current user, inheritance is protected, and the DACL contains zero ACEs. Ordinary read, copy, rename, and removal operations fail until an explicit current-user Full Control ACE is restored.

Screenshots and supporting files

No secret-bearing files are attached. The deterministic regression and aggregate production evidence are described below.

Redacted configuration

Not configuration-dependent.

Checks

Production evidence

On Windows 11 with OpenCodex 2.7.42:

  • 65 stale atomic temp files were present under the OpenCodex home.
  • Affected basenames included auth.json, config.json, and responses-state.json.
  • Total size was about 1.09 GB.
  • Every PID encoded in those filenames was no longer running.
  • The files were owned by the current user but had a protected ACL with zero ACE entries.
  • Ordinary read, copy, rename, and removal attempts failed until an explicit current-user Full Control ACE was restored.
  • The final destination files had the expected single current-user Full Control ACE.

No usernames, home paths, tokens, credentials, or response contents are included here.

Root cause

/inheritance:r is a destructive transition: it removes inherited ACEs. The current implementation performs that transition before making owner access explicit.

The timeout/retry work tracked in #160 correctly prevents partial hardening from being reported as success, but it does not preserve cleanup access while the sequence is incomplete. The stale-temp recovery added for #499 is complementary; it cannot reliably clean a temp whose DACL no longer permits the owner to open or unlink it.

Proposed invariant and fix

Install the explicit owner ACE before any destructive ACL change:

  1. /grant:r CURRENTUSER:(F) (or (OI)(CI)(F) for directories)
  2. /inheritance:r
  3. Remove broad group SIDs, retaining the existing /findsid verification

This preserves the following invariant:

  • If the grant fails, inheritance is untouched.
  • If any later step times out or fails, the writer still has explicit Full Control and can scrub or unlink the temp.
  • Removing broad group SIDs cannot remove the explicit current-user ACE.
  • No duplicate final grant is required.

Deterministic regression

An injected-runner test models inherited owner access, removes inheritance, then forces /remove:g to time out. The expected operation order is:

grant-owner
remove-inheritance
remove-broad

Effective owner access must remain true after the timeout.

A second regression asserts the exact successful order. The existing EPERM case is also strengthened so the sole attempted operation is /grant:r, proving a grant failure cannot modify inheritance first.

Validation

The focused patch was validated with:

  • 28 Windows ACL tests, including the new failure-order and success-order regressions.
  • 113 related tests across ACL, atomic-write startup, service, scheduler, and lifecycle suites.
  • TypeScript type-check.
  • Privacy scan.
  • A real Windows probe confirming the hardened file remained readable and showed only the current-user Full Control ACE.

The old implementation fails the new ordering regression; the owner-first implementation passes it.

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions