Fix descriptor-backed artifact thumbnail decoding

[austinywang]

Summary

• Keep ArtifactByteReader’s verified regular-file descriptor open through thumbnail classification and ImageIO decoding.
• Decode through a CGDataProvider backed by a duplicated descriptor and positional reads, so pathname replacement cannot redirect the decode or block on a FIFO.
• Add a regression test that replaces the validated PNG pathname with a FIFO and still decodes the original descriptor.
• PR Fix iOS artifact failure messaging #9961 already covers the Iroh transfer FIFO validation on current main; this PR addresses the remaining thumbnail close-and-reopen TOCTOU tracked by Artifact transfer paths can block on special files (FIFO) — extend descriptor-verified opens to Iroh transfers and thumbnail decode #8581.

Fixes #8581

Testing

• swift test --package-path Packages/Shared/CmuxAgentChat --filter ArtifactByteReaderTests — 18 passed.
• swift test --package-path Packages/Shared/CmuxAgentChat — 273 passed in 32 suites.
• The first commit is test-only (de4823744b); the second contains the fix (ee04ffec6f).
• No local Xcode app build or XCUITest was run, per workspace instructions.

---

^{Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.}

---

Summary by cubic

Fixes thumbnail decoding to read directly from the verified file descriptor instead of reopening by path. This removes the TOCTOU window that allowed pathname swaps to FIFOs and could block or redirect decoding (fixes #8581).

Bug Fixes

• Decode via CGDataProvider backed by a duplicated descriptor with positional reads, using ArtifactImageDataProvider.
• Keep the verified descriptor open for the entire operation and never consult the pathname after verification.
• For thumbnails, disable unknown-regular-file sniffing; eligibility is extension-based only (extensionless images won’t be thumbnailed).
• Add a regression test that replaces the validated PNG path with a FIFO and still decodes from the original descriptor.

^{Written for commit cb12a14. Summary will update on new commits.}

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 4 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository: manaflow-ai/cmux/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: e5a6abcc-396b-4fd2-99e7-9c5841eb6c4d

📥 Commits

Reviewing files that changed from the base of the PR and between 086c8cb and cb12a14.

📒 Files selected for processing (4)

• Packages/Shared/CmuxAgentChat/Sources/CmuxAgentChat/Artifacts/ArtifactByteReader+Thumbnail.swift
• Packages/Shared/CmuxAgentChat/Sources/CmuxAgentChat/Artifacts/ArtifactByteReader.swift
• Packages/Shared/CmuxAgentChat/Sources/CmuxAgentChat/Artifacts/ArtifactImageDataProvider.swift
• Packages/Shared/CmuxAgentChat/Tests/CmuxAgentChatTests/ArtifactByteReaderTests.swift

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

Autopilot is currently an internal CodeRabbit preview.

---

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.}

[greptile-apps]

Greptile Summary

This PR removes a thumbnail-decoding pathname race by retaining the verified regular-file descriptor and giving ImageIO a duplicated, positional-read-backed data provider.

• Moves thumbnail encoding and decoding into a descriptor-oriented helper.
• Adds explicit ownership of the duplicated descriptor for the ImageIO provider lifetime.
• Avoids UTF-8 sniffing and pathname reopening during thumbnail classification and decoding.
• Adds regression coverage for replacing the validated pathname with a FIFO before decode.

Confidence Score: 5/5

The PR appears safe to merge, with descriptor ownership and decode routing consistently preventing pathname replacement from redirecting thumbnail reads.

The provider retains a duplicated verified descriptor for ImageIO’s lifetime, uses positional reads instead of reopening the path, and current production callers keep synchronous decoding off the main actor.

Important Files Changed

Filename	Overview
Packages/Shared/CmuxAgentChat/Sources/CmuxAgentChat/Artifacts/ArtifactByteReader.swift	Keeps the verified descriptor open through classification and delegates decoding without reopening the pathname.
Packages/Shared/CmuxAgentChat/Sources/CmuxAgentChat/Artifacts/ArtifactByteReader+Thumbnail.swift	Encapsulates ImageIO thumbnail decoding and JPEG encoding using the verified descriptor-backed provider.
Packages/Shared/CmuxAgentChat/Sources/CmuxAgentChat/Artifacts/ArtifactImageDataProvider.swift	Duplicates the verified descriptor, serves positional reads to CoreGraphics, and releases the descriptor with provider context lifetime.
Packages/Shared/CmuxAgentChat/Tests/CmuxAgentChatTests/ArtifactByteReaderTests.swift	Adds regression coverage proving pathname replacement with a FIFO cannot redirect or block decoding.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
    A[Authorized artifact path] --> B[Open nonblocking descriptor]
    B --> C[fstat verifies regular file]
    C --> D[Retain verified FileHandle]
    D --> E[Duplicate descriptor with close-on-exec]
    E --> F[CGDataProvider positional pread callbacks]
    F --> G[ImageIO thumbnail decode]
    G --> H[JPEG thumbnail]
    A -. pathname replacement does not redirect decode .-> D

Loading

_{Reviews (1): Last reviewed commit: "fix: decode artifact thumbnails from ver..." | Re-trigger Greptile}

[teamleaderleo]

Review: no correctness findings in the descriptor-backed thumbnail path or its tests. Fixed: merged current main into the branch. Left: CI validation.

[github-actions]

All contributors have signed the CLA ✍️ ✅
_{Posted by the CLA Assistant Lite bot.}

[github-actions]

Merge receipt for cb12a144e6, merged 2026-09-30 18:59:18 UTC

• Not verified at merge: macOS status (in progress)
• Verified: app-host unit tests (2), ci-status, CI fast guards, CI timing, detect-ios-changes, Fast static checks, GhosttyKit release check, guards (18), ios-simulator (ipad), ios-simulator (iphone), ios-simulator-build, ios-tests, and 10 more
• Skipped by policy: macOS compile admission, admission-placement, browser, Claude wrapper regressions, CLI product tests, Dogfood build #​${{ github.event.pull_request.number }}, late-placement, release-admission, release-build, remote-daemon, suite-coverage, tests-build-and-lag, and 5 more
• Full suite: runs on main after merge.