fix(claude): reject non-regular Desktop configuration reads by luvs01 · Pull Request #718 · luvs01/opencodex

luvs01 · 2026-10-12T04:32:36Z

Summary

Read Desktop metadata and profile contents through verified regular-file descriptors so special files or swapped path entries cannot introduce blocking FIFO reads.

Use nonblocking, no-follow opening on POSIX and validate descriptor type, bigint device/inode identity, and change time.
Apply the reader to metadata, profile inspection, picker profile objects, and library snapshots while preserving decoding and refusal behavior.
Cover directories, symlinks, FIFOs, and replacements during opening in the existing Desktop suites. The separate stacked PR [WRONG BRANCH] fix(desktop): bound library reads and preflight write sizes #730 adds byte bounds.
Preserve the OAuth multiprocess fixture's filtered failure metadata when a child exits unsuccessfully after registration, so CI reports the phase and bounded stack instead of only exit code 1.

Verification

Current head: a3137becda4102bfcc2c1cda6cdfe75aedb08a87.

Current-head hosted CI, attempt 2 passed. The AUTH-11 Windows shard passed on the first attempt; two other Windows jobs passed after one failed-job rerun on the same head.
The preceding 8aedd750b CI failed in AUTH-11: distinct cross-process credentials sharing a stored id preserve both namespaces, with a child exit code of 1. Its actual cause was not exposed by the parent assertion.
On Bun 1.4.3, bun test --isolate --timeout 60000 tests/oauth/oauth-anthropic-instance-multiprocess.test.ts passed 2 tests with the diagnostic patch. The earlier CI failure was not reproduced; this is not evidence that its underlying cause was fixed.
Negative control: forcing the child to throw during registration made the test fail with phase/name/stack metadata while the injected fake secret message was absent from the log. The temporary fixture change was restored.
bun run typecheck, bun run privacy:scan, and git diff --check passed on this patch.
The first current-head run hit a 20-second catalog-sync subprocess timeout and a 15-second CLI-resolve child timeout in unchanged tests. Both jobs had passed on the direct parent. Their timing causes were not established; no production behavior or timeout threshold was changed to address them.
A full local repository suite was not repeated after the earlier hour-long Windows shard attempt on this host. The completed hosted run above supplies the current-head broader evidence.
Relevant Desktop regressions are in claude-desktop-picker-profile.test.ts, desktop-3p-removal.test.ts, and desktop-remote-store.test.ts. Local profile-path attempts also encountered configuration-mutation-lock stalls on the unchanged baseline and are not treated as successful validation.

Checklist

Regular-file validation and descriptor cleanup remain enforced.
The diagnostic preserves exit-code and empty-output assertions.
Focused validation and its limits are recorded.
Current-head hosted CI passed.

Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

…riptors Preserve normal profile parsing and fingerprint bytes. Reject non-regular paths and descriptor replacements; use nonblocking no-follow opens on POSIX before checking the descriptor. Reuse the reader for metadata, observations, foreign-key preservation and refresh checks. Add six focused regression cases, including real FIFO inputs in bounded child processes. Keep the candidate in draft pending complete current-tree and native-platform validation.

coderabbitai · 2026-10-12T04:32:45Z

Important

🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Repository: luvs01/opencodex/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Advanced
Run ID: 0f257566-53c3-4e4a-aa6e-ffca6cf9aba5

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

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

luvs01 · 2026-10-12T04:50:31Z

No description provided.

…ile reads Co-Authored-By: Epinephrine <luvs01@hanmail.net>

devin-ai-integration · 2026-10-12T04:55:04Z

Review-findings sweep (2513ba05e)

Collected every unresolved thread on this PR: Devin Review flagged one item; CodeRabbit skipped review (draft); the Codex connector posted nothing.

Fixed

Devin Review — Update the Desktop ownership contract: structure/clients/claude-desktop.md now documents the shared verified regular-file reader covering all Desktop library config reads, not only the background-refresh refusal. Thread resolved.
While validating, the head commit's removal-suite edit had a stray ) that made tests/clients/desktop-3p-removal.test.ts entirely unparseable (Expected ";" but found ")"). Repaired — a real regression no reviewer had caught.

Checks (Windows / Bun 1.4.0 via OCX_TEST_RUNNER_BUN)

bun run typecheck, bun run privacy:scan, bun run structure:check — pass
bun test tests/clients/desktop-3p-removal.test.ts — 10 pass / 4 POSIX-only skips
bun run test:changed — could not finish inside the suite's 900s cap: this change's import graph pulls in 1665/2235 files and the run was terminated at the limit; pre-kill failures were Windows resource contention (EBUSY temp cleanup, port-record timeouts, EACLIDENTITY), and a previously-"failed" file (tests/codex-integration/active-registry-admission.test.ts) passes 11/11 in isolation. Not caused by this change.

Dismissed: none — no other unresolved findings exist on the PR.

devin-ai-integration · 2026-10-12T04:55:35Z

Done. Wrap-up on PR #718:

Findings swept (all streams, not just this review pass): Devin Review flagged 1 item; CodeRabbit skipped review entirely (draft PR); the Codex connector posted nothing.

Fixed — pushed 2513ba05e to codex/fix-desktop-library-regular-reads-20261012:

Devin Review's Update the Desktop ownership contract: structure/clients/claude-desktop.md now documents the shared verified regular-file reader for all Desktop library config reads, not only the background-refresh refusal. Thread replied + resolved.
A real bug no reviewer caught: the head commit left a stray ) making tests/clients/desktop-3p-removal.test.ts entirely unparseable — repaired.

Checks: typecheck, privacy:scan, structure:check all pass; focused test file green (10 pass, 4 POSIX-only skips on Windows). test:changed cannot finish inside its own 900s cap here (the import graph pulls 1665/2235 files); its mid-run failures were Windows resource flakes — spot-verified one "failing" file passes 11/11 standalone.

Dismissed: none — no other findings existed. Summary comment posted on the PR. Blueprint suggestion to pin Bun 1.4.0 for test:changed is pending your approval in the timeline.

…reader contract Co-Authored-By: Epinephrine <luvs01@hanmail.net>

devin-ai-integration · 2026-10-12T05:01:09Z

Fixed in 0aba4281f — the contract now scopes the shared reader to the local inspection and writer reads this change converted (selected-profile inspection, _meta.json parsing, foreign-key preservation, refresh mode re-resolution, refresh fingerprint hash), and explicitly names the connection-owned store as a separate reader: StoreIO.read in src/claude/desktop-remote-store-io.ts keeps its own lstat/size/post-read-identity contract and does not go through readDesktopLibraryBytes.

devin-ai-integration · 2026-10-12T05:01:12Z

Fixed in 0aba4281f — readDesktopLibraryBytes now catches ELOOP from the no-follow openSync and rethrows desktop_config_not_regular, so a regular→symlink swap between lstat and open surfaces the documented refusal name instead of the raw errno. Other open failures (e.g. ENOENT, EACCES) still propagate unchanged. Added a sibling regression case (a regular-to-symlink replacement between stat and open is refused, POSIX-only like the FIFO-swap case) and updated the contract sentence accordingly.

devin-ai-integration · 2026-10-12T05:01:17Z

Review-findings sweep (0aba4281f)

Collected every unresolved thread on this PR: Devin Review flagged 2 items; CodeRabbit skipped review (draft); the Codex connector posted nothing.

Fixed

Devin Review — The shared-reader contract omits connected-client reads: the contract now scopes the shared verified regular-file reader to the local inspection/writer reads this change converted and names StoreIO.read (src/claude/desktop-remote-store-io.ts) as the connection-owned store's separate path-based reader. Thread resolved.
Devin Review — The documented refusal error is not universal: readDesktopLibraryBytes now normalizes ELOOP from the no-follow openSync to desktop_config_not_regular, so a regular→symlink swap reports the documented refusal name. Added a sibling POSIX regression case for the swap. Thread resolved.

Checks (Windows / Bun 1.4.3)

bun run typecheck, bun run privacy:scan, bun run structure:check — pass
bun test tests/clients/desktop-3p-removal.test.ts — 10 pass / 5 POSIX-only skips / 0 fail
bun run test:changed — not rerun: this change's import graph is the one previously shown unable to finish inside the suite's 900s cap on Windows; the focused suite covers the touched code.

Dismissed: none — both findings were real and fixed.

devin-ai-integration · 2026-10-12T05:01:25Z

Done. Wrap-up on PR #718:

Findings swept (all streams): Devin Review flagged 2 new items on the contract doc; CodeRabbit skipped review (draft PR); the Codex connector posted nothing.

Fixed — pushed 0aba4281f to codex/fix-desktop-library-regular-reads-20261012:

Shared-reader contract omits connected-client reads: doc now scopes "every read" to the local inspection/writer reads this PR converted, and names StoreIO.read in desktop-remote-store-io.ts as the connection-owned store's separate path-based reader. Thread replied + resolved.
Refusal error not universal: readDesktopLibraryBytes now catches ELOOP from the no-follow openSync and rethrows desktop_config_not_regular, so a regular→symlink swap surfaces the documented name. Added a POSIX regression case mirroring the FIFO-swap test. Thread replied + resolved.

Checks: typecheck, privacy:scan, structure:check all pass; focused test file green (10 pass, 5 POSIX-only skips on Windows). test:changed not rerun — same import-graph/900s-cap limitation documented on the previous head.

Dismissed: none — both findings were real. Summary comment posted on the PR. Devin Review CI is running again on the new head; if it reports anything new, that will arrive as a separate review event.

Co-Authored-By: Epinephrine <luvs01@hanmail.net>

devin-ai-integration · 2026-10-12T05:11:07Z

Follow-up on the findings sweep (cfe764610, on top of 0aba4281f)

Two sessions of this automation ran this sweep in parallel; 0aba4281f landed first and resolved both threads. This commit strengthens that resolution rather than re-opening it — the parallel run is why the branch history shows two passes.

What changed beyond 0aba4281f

Connected-client reads: instead of scoping the contract around StoreIO.read, the store now acquires bytes through readDesktopLibraryBytes itself. Its readFileSync(path) had the same blocking-open hazard this PR removes elsewhere — a FIFO swapped in after lstat parks the open, and the post-read identity recheck can never run to flag it (the read happens inside withConfigMutationLockSync, so the wait would hold the mutation lock too). StoreIO's size cap, private-file mode, byte budget and post-read identity contract are unchanged layers on top; a refusal surfaces as unsafe as before.
Open-time refusal: generalized the ELOOP-only catch to a path re-observation — any swap that fails at open (ELOOP symlink, ENXIO socket, ENOENT deletion) now reports desktop_config_not_regular, while a same-path failure (EACCES/EMFILE) keeps its own errno. This makes the documented "non-regular or swapped path" guarantee hold for every swap class, not only symlinks.
structure/clients/claude-desktop.md updated to match: the store is in the shared-reader enumeration, and the open-time refusal rule is stated.

Checks (Windows / Bun)

bun run typecheck, bun run privacy:scan, bun run structure:check — pass
bun test tests/clients/desktop-3p-removal.test.ts tests/clients/desktop-remote-store.test.ts tests/clients/desktop-3p.test.ts — 57 pass / 5 POSIX-only skips / 0 fail; desktop-remote-store.test.ts exercises the store path end to end.

No dismissed findings — both Devin Review items remain resolved; this only closes the residual FIFO-block path the first one pointed at.

devin-ai-integration · 2026-10-12T05:12:05Z

Done — with one wrinkle worth knowing: a parallel session of this automation ran the same sweep and landed 0aba4281f minutes before my push, resolving both threads with a narrower fix. I kept its commit and pushed cfe764610 on top, closing what its version left open.

Findings on the PR (all streams): Devin Review flagged 2 items on the contract doc; CodeRabbit skipped (draft); Codex connector posted nothing. Both were real.

Now on codex/fix-desktop-library-regular-reads-20261012:

Connected-client reads (PRRT_...VTIa): the sibling narrowed the doc; I went further — StoreIO.read now acquires bytes through readDesktopLibraryBytes, since its blocking readFileSync had the same swap-to-FIFO hang this PR exists to kill (and it runs under the mutation lock). Its size/identity layers are unchanged.
Refusal not universal (PRRT_...VTJC): generalized the ELOOP-only catch to path re-observation — any open-time swap (ELOOP/ENXIO/ENOENT) reports desktop_config_not_regular; same-path failures keep their errno. Kept the sibling's symlink-swap regression test.
structure/clients/claude-desktop.md updated to match.

Checks: typecheck, privacy:scan, structure:check pass; focused tests 57 pass / 5 POSIX-only skips / 0 fail. Follow-up summary posted on the PR; threads stay resolved.

…FO refusal Co-Authored-By: Epinephrine <luvs01@hanmail.net>

devin-ai-integration · 2026-10-12T05:21:12Z

Review-findings sweep (60197f50d)

Collected every unresolved thread on this PR: Devin Review flagged 1 item on the latest pass; CodeRabbit skipped review (draft); the Codex connector posted nothing.

Fixed

Devin Review — Add a focused test for connection-owned store reads: tests/clients/desktop-remote-store.test.ts now covers StoreIO.read itself. A bounded child process swaps the observed regular profile for a FIFO at the shared reader's openSync, then runs the real store read: the refusal must classify as DesktopStoreError("unsafe") — distinct from conflict, which is what a surviving post-read identity recheck would produce. The 10s child bound makes a reintroduced blocking open a kill, not a hang. Thread resolved.

Checks (Windows / Bun 1.4.3)

bun run typecheck, bun run privacy:scan, bun run structure:check — pass
bun test tests/clients/desktop-remote-store.test.ts — 18 pass / 1 POSIX-only skip / 0 fail (the new case skips on Windows; its child-script mechanics were separately verified against a directory swap, which exercises the same refusal path)
bun test tests/clients/desktop-3p-removal.test.ts tests/clients/desktop-3p.test.ts — 39 pass / 5 POSIX-only skips / 0 fail
bun run test:changed — not rerun: this change's import graph is the one previously shown unable to finish inside the suite's 900s cap on Windows; the focused suite covers the touched file.

Dismissed: none — the finding was real and fixed.

devin-ai-integration · 2026-10-12T05:21:27Z

Done. Wrap-up on PR #718:

Findings swept (all streams): Devin Review's latest pass flagged 1 item; CodeRabbit skipped review (draft PR); the Codex connector posted nothing. Earlier threads from prior sweeps were already resolved.

Fixed — pushed 60197f50d to codex/fix-desktop-library-regular-reads-20261012:

Add a focused test for connection-owned store reads: tests/clients/desktop-remote-store.test.ts gained a bounded store-level swap test. A child process (10s kill bound) patches the shared reader's openSync to swap a FIFO over the observed profile, then runs a real StoreIO.read — the refusal must classify as DesktopStoreError("unsafe"), which the test distinguishes from the conflict a post-read identity recheck would produce. Thread replied + resolved.

Checks: typecheck, privacy:scan, structure:check pass; store suite 18 pass / 1 POSIX-only skip; removal + desktop-3p suites 39 pass / 5 POSIX-only skips / 0 fail. The new POSIX case's child mechanics were verified locally via an equivalent directory swap. test:changed not rerun — same import-graph/900s-cap limitation documented on prior heads.

Dismissed: none — the finding was real. Summary comment posted on the PR.

devin-ai-integration · 2026-10-12T05:24:30Z

Review-findings sweep (60197f50d) — clean pass.

The review that triggered this run (5486777068) was the container for the "Fixed in 60197f50d" resolution reply and carried no findings. Full sweep across all streams: 0 unresolved review threads on the PR (4/4 resolved), CodeRabbit skipped review (draft), the Codex connector posted nothing. All prior Devin Review findings were already fixed in 2513ba05e…60197f50d.

Fixed: nothing new — no open findings to act on. Dismissed: none.

devin-ai-integration · 2026-10-12T05:24:39Z

Done — clean sweep on #718, nothing left to fix.

What triggered this run: review 5486777068 carried no findings — it was just the review container for the "Fixed in 60197f50d" resolution reply.

Full sweep (all streams, not just this pass):

Devin Review: 4 review threads, 0 unresolved — all were already fixed and resolved by earlier runs of this automation (2513ba05e → 60197f50d, covering the ownership contract, StoreIO shared-reader routing, ELOOP→desktop_config_not_regular normalization, and the store-level swap regression test).
CodeRabbit: skipped review (draft PR), no findings.
Codex connector: posted nothing.

Fixed: nothing new needed. Dismissed: none. Posted a short sweep note on the PR; CI checks on head are green (CodeRabbit + Devin Review both pass).

luvs01 · 2026-10-12T05:51:34Z

No description provided.

luvs01 · 2026-10-12T11:58:15Z

No description provided.

luvs01 · 2026-10-12T19:23:20Z

No description provided.

github-actions · 2026-10-13T00:58:26Z

✅ Deterministic PR hygiene checks passed.

luvs01

Current-head CI found one reproducible documentation size-gate failure; author action is detailed inline.

chatgpt-codex-connector · 2026-10-13T03:55:06Z

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-13T03:58:43.793832Z	`fb55d32`	Draft marked ready

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fb55d32f3a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

readFileSnapshot and profileObject read _meta.json, the picker profile and its backup with unbounded readFileSync, so a swapped FIFO could block inspect/apply indefinitely despite the documented regular-file guarantee. Desktop library snapshots now go through readDesktopLibraryText; the opencodex-owned picker state file keeps the plain read. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

Retain the fixture's credential-free phase and stack metadata when a registration writer finishes with a nonzero exit, not only when it exits before the ready barrier. Keep the existing exit-code and empty-output assertions unchanged. Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

luvs01 · 2026-10-13T08:12:45Z

Closing duplicate tracking after preserving the complete change in independently verified integration commit e8afb8bb41786ddd595fd8ab9f36cff46d403dab based on development revision e6d7e561f566ab343fbf545a6fa3e1b63012f817. All 9 changed files match source HEAD a3137becda4102bfcc2c1cda6cdfe75aedb08a87 byte for byte. All nine changed files match the reviewed source revision; exact-source hosted CI completed successfully. Current integration-candidate type, structure, materialized-file privacy and diff checks passed. Source initial timeouts and rerun scope remain documented. Integration remains unmerged and requires its own CI and maintainer review. The source branch is retained.

This comment was marked as resolved.

Sign in to view

fix(claude): repair removal-suite paren and document shared regular-f…

2513ba0

…ile reads Co-Authored-By: Epinephrine <luvs01@hanmail.net>

This comment was marked as resolved.

Sign in to view

fix(claude): normalize swapped-symlink open refusal and scope shared-…

0aba428

…reader contract Co-Authored-By: Epinephrine <luvs01@hanmail.net>

fix(claude): route connection-owned store reads through shared reader

cfe7646

Co-Authored-By: Epinephrine <luvs01@hanmail.net>

This comment was marked as resolved.

Sign in to view

test(claude): cover connection-owned store reads against a swap-to-FI…

60197f5

…FO refusal Co-Authored-By: Epinephrine <luvs01@hanmail.net>

This was referenced Oct 12, 2026

[WRONG BRANCH] fix(desktop): bound library reads and preflight write sizes #730

Draft

security: harden filesystem, routing, and request resource boundaries #719

Closed

Merge fork dev dc45bb2 into PR #718

c0d2485

luvs01 added a commit that referenced this pull request Oct 12, 2026

Merge updated parent PR #718 into PR #730

7147629

Merge remote-tracking branch 'fork/dev' into merge718-dev

91df3f8

github-actions Bot added the bug Something isn't working label Oct 13, 2026

luvs01 commented Oct 13, 2026

View reviewed changes

Comment thread structure/clients/claude-desktop.md Outdated

docs(structure): keep claude-desktop contract within line budget

fb55d32

luvs01 added a commit that referenced this pull request Oct 13, 2026

Merge updated parent PR #718 into PR #730

69b4c5f

luvs01 marked this pull request as ready for review October 13, 2026 03:54

chatgpt-codex-connector Bot reviewed Oct 13, 2026

View reviewed changes

Comment thread structure/clients/claude-desktop.md

luvs01 and others added 2 commits October 13, 2026 13:32

luvs01 marked this pull request as draft October 13, 2026 05:40

luvs01 marked this pull request as ready for review October 13, 2026 06:21

luvs01 closed this Oct 13, 2026

Conversation

luvs01 commented Oct 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Verification

Checklist

Uh oh!

coderabbitai Bot commented Oct 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as resolved.

Uh oh!

luvs01 commented Oct 12, 2026

Uh oh!

devin-ai-integration Bot commented Oct 12, 2026

Uh oh!

devin-ai-integration Bot commented Oct 12, 2026

Uh oh!

This comment was marked as resolved.

Uh oh!

devin-ai-integration Bot commented Oct 12, 2026

Uh oh!

devin-ai-integration Bot commented Oct 12, 2026

Uh oh!

devin-ai-integration Bot commented Oct 12, 2026

Uh oh!

devin-ai-integration Bot commented Oct 12, 2026

Uh oh!

devin-ai-integration Bot commented Oct 12, 2026

Uh oh!

This comment was marked as resolved.

Uh oh!

devin-ai-integration Bot commented Oct 12, 2026

Uh oh!

devin-ai-integration Bot commented Oct 12, 2026

Uh oh!

devin-ai-integration Bot commented Oct 12, 2026

Uh oh!

devin-ai-integration Bot commented Oct 12, 2026

Uh oh!

devin-ai-integration Bot commented Oct 12, 2026

Uh oh!

luvs01 commented Oct 12, 2026

Uh oh!

luvs01 commented Oct 12, 2026

Uh oh!

luvs01 commented Oct 12, 2026

Uh oh!

github-actions Bot commented Oct 13, 2026

Uh oh!

luvs01 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

chatgpt-codex-connector Bot commented Oct 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codex Review Summary

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

luvs01 commented Oct 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

luvs01 commented Oct 12, 2026 •

edited

Loading

coderabbitai Bot commented Oct 12, 2026 •

edited

Loading

chatgpt-codex-connector Bot commented Oct 13, 2026 •

edited

Loading