fix(codex): reject profile FIFOs without blocking

[luvs01]

Summary

• open bounded native-profile files with O_NONBLOCK on POSIX while retaining O_NOFOLLOW;
• reject FIFOs and other non-regular entries after the descriptor is opened, without waiting for a writer;
• run the FIFO regression in a separate Bun process with a two-second OS-level timeout so a future regression cannot hang the test runner.

Why

The bounded reader validates the opened descriptor with fstat, but a POSIX openSync(..., O_RDONLY) can block before that validation when an untrusted profile path is a FIFO with no writer. The size cap and regular-file check therefore never get a chance to reject it.

Adding O_NONBLOCK makes the open return promptly. The existing descriptor check then rejects the FIFO as a non-regular file, while regular file, replacement-race, and bounded-read semantics remain unchanged.

The regression is intentionally isolated in a child process. If the flag is removed later, the OS timeout terminates only the child and reports a deterministic test failure instead of freezing the entire suite.

Verification

• Bun 1.3.14 on Windows: tests/native-profile-store.test.ts 15 passed, 1 POSIX-only test skipped.
• Bun 1.4.0-canary.1 (b22e0e6d0) on Windows: the same result.
• bun x tsc --noEmit: passed.
• bun scripts/privacy-scan.ts: passed.
• git diff --check: passed.

The FIFO assertion is POSIX-only and could not run locally because this host has no WSL distribution; the draft intentionally leaves that OS-specific execution to Linux CI.

Security boundary

This narrows a local availability edge in the native-profile metadata reader. It does not broaden accepted file types, paths, permissions, or credential-store behavior. Symlinks remain rejected by O_NOFOLLOW, and the descriptor must still be a regular file within the existing byte cap.

Checklist

• Product change is limited to the POSIX open flags.
• A hanging regression cannot block the parent test runner.
• Existing bounded-read and keyring tests remain green.

Ready for review; Linux CI and maintainer review remain pending.

Summary by CodeRabbit

• Bug Fixes

  • Prevented vault reads from hanging when encountering special files without an available writer.
  • Invalid vault entries now fail promptly with a clear unsupported authentication-store error.

• Tests

  • Added Unix-specific coverage to verify that special-file reads complete promptly rather than blocking.

Review readiness checklist

This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:

• All CI tests are green on my local testing.

• I pushed my PR to the latest dev commit.

• I resolved all correct Codex and CodeRabbit findings.

• My PR is ready for review.

[coderabbitai]

📝 Walkthrough

Walkthrough

On non-Windows platforms, bounded vault reads now open files with O_NONBLOCK. A Unix-only regression test confirms that FIFO reads terminate within two seconds with UNSUPPORTED_AUTH_STORE.

Changes

FIFO read protection

Layer / File(s)	Summary
Non-blocking vault read and regression test src/codex/native-profile-store.ts, tests/native-profile-store.test.ts	readBounded adds O_NONBLOCK on non-Windows platforms. The Unix-only test runs credential-store resolution in a child process and verifies prompt UNSUPPORTED_AUTH_STORE termination for a FIFO.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

• lidge-jun/opencodex#863: Both changes modify credential-envelope file reading in src/codex/native-profile-store.ts; this PR hardens that implementation against FIFO blocking.

Suggested reviewers: lidge-jun, wibias, ingwannu

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: preventing blocking when profile paths are FIFOs.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

✅ Deterministic PR hygiene checks passed.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

[Ingwannu]

The O_NONBLOCK change is a sensible defense for direct readBounded callers, but the new regression test does not reach that code path and currently fails on the PR head. readNativeProfileVault() first calls assertNativeProfileMetadataLayout(), which rejects the FIFO as PROFILE_STORAGE_UNSAFE; the child only accepts VAULT_INVALID, so it exits 92 (locally: 15 pass, 1 fail; the macOS CI job also failed). Please make the test exercise a direct bounded-read surface such as the config/auth path, or explicitly test the existing metadata-layout rejection separately, and include child.stderr in the assertion message so future failures are diagnosable. I will re-review once the test proves the intended nonblocking boundary and CI is green.

[Ingwannu]

Re-checked rebased exact head 7c3fa541. The blocker is unchanged: the new test still calls readNativeProfileVault(), so assertNativeProfileMetadataLayout() rejects the FIFO as PROFILE_STORAGE_UNSAFE before the O_NONBLOCK open path is exercised. The child accepts only VAULT_INVALID and exits 92; exact-head result remains 15 pass / 1 fail. Please apply the requested direct bounded-read regression (or separate layout-contract assertion) rather than only rebasing the same patch.

[github-actions]

⏳ DRAFT

• review readiness checklist open (0/4 boxes ticked).

What to do

• Tick all four boxes in the PR description once you're done (currently 0/4).

Review readiness checklist

• ⬜ All CI tests are green on my local testing.
• ⬜ I pushed my PR to the latest dev commit.
• ⬜ I resolved all correct Codex and CodeRabbit findings.
• ⬜ My PR is ready for review.

0/4 boxes ticked.

This PR stays in draft until every box above is ticked.

[luvs01]

Updated exact head 49537aea on the latest dev base.

The POSIX regression now creates a FIFO at config.toml and calls resolveNativeCredentialStoreMode(), so it reaches the direct bounded config read rather than stopping at the vault metadata-layout preflight. It expects UNSUPPORTED_AUTH_STORE, retains the two-second subprocess timeout, and reports status, signal, process error, and stderr on failure.

Local Windows verification remains 15 passed / 1 POSIX-only skipped on Bun 1.3.14 and Bun 1.4.0-canary.1; the POSIX assertion now needs exact-head Linux/macOS CI confirmation before the readiness checklist is advanced.

[lidge-jun]

The fix is right and the test is wrong — CI is failing for that reason, not because of the code.

I first concluded the defect did not exist: with a FIFO placed at the vault path before the call, assertCanonicalFile rejects it in ~0ms with PROFILE_STORAGE_UNSAFE, well before readBounded reaches openSync. That observation was correct and my conclusion from it was not.

The real defect is a TOCTOU race. A regular file that passes assertNativeProfileMetadataLayout can be replaced with a FIFO before openSync, and by then the guard is behind us. Using the beforeOpen seam the module already ships for exactly this window:

clean origin/dev (no patch):  hangs until the harness kills it
this branch:                  throws VAULT_INVALID in ~3ms

So O_NONBLOCK fixes something real.

The test creates the FIFO before the call, so it trips the upstream guard and never reaches the code you changed — it gets PROFILE_STORAGE_UNSAFE while asserting VAULT_INVALID. Correcting the assertion to the pre-guard code would be worse: that test would then pass without the patch too.

Suggested shape:

• child process (mandatory — unpatched code hangs and would take the runner with it)
• parent passes isolated codexHome/configDir via env; child calls resolveNativeProfileContext with them
• child writes a regular vault file so the layout check passes
• child installs the seam under Symbol.for("opencodex.native-profile-store.bounded-read-test-seam") and, in beforeOpen, unlinks and mkfifos the vault path
• parent uses spawnSync with a bounded timeout and asserts no error, null signal, status 0
• test.skipIf(process.platform === "win32") since mkfifo is POSIX-only

Asserting signal/timeout is what separates "hung" from "rejected quickly". Happy to take it once that lands.