fix(chatgpt): validate bundle trust before restore

[luvs01]

Summary

• Apply the shared ChatGPT bundle trust policy before either relaunch path can quit the app or pass its discovered path to LaunchServices.
• Restore validates the bundle and main app executable without requiring the experimental opt-in flag or a bundled app-server binary. Preserve no-install launcher cleanup, read-only status behavior, and launcher removal only after a successful open.
• Check POSIX directory-entry replacement permissions through the filesystem root. Refuse foreign-owned or ordinarily writable ancestors while preserving trusted sticky parents and root-owned admin-group install folders using a bounded lookup of the local admin GID.
• Add isolated command-level regressions using synthetic ownership, mode, signature, symlink and process evidence. Update the current architecture contract and user guide. Related: feat(chatgpt): experimental macOS app-server quota-gate shim (split from #5947) #6361 and fix(chatgpt): close the #6361 review follow-ups (over-cap line, docs, structure map) #6412; this independently addresses restore admission.

Verification

• Red before source fix: bun test tests/clients/desktop-app-server-shim-launcher.test.ts — 18 pass, 6 fail. Five unsafe-bundle restore cases reached success; the legitimate control exposed missing signature checks.
• Green before ancestry extension: the launcher file — 24 pass, 0 fail. Final combined run: bun test tests/clients/desktop-app-server-shim-launcher.test.ts tests/clients/desktop-app-server-shim.test.ts — 51 pass, 0 fail. Covers unsafe restore rejection before quit/open, valid restore with the flag disabled and no app-server, failed-open preservation, no-install cleanup, status, and existing launch requirements.
• The confirmed foreign-owned-parent case was red with a signed/owned bundle under that parent; it now rejects before quit/open. Added controls cover traversal through root, root/admin group-write with the actual group ID, failed admin lookup, trusted sticky parents, foreign sticky owners, and symlink/writable ancestors. The admin and sticky semantics follow Apple's filesystem documentation.
• bun run typecheck, bun run structure:check, bun run privacy:scan, bun scripts/file-size-ratchet.ts, and git diff HEAD --check — passed.
• bun test tests/test-layout.test.ts tests/test-layout-tooling.test.ts — 18 pass, 0 fail.
• ASTRO_TELEMETRY_DISABLED=1 XDG_CONFIG_HOME=../.tmp/security-macrestore/docs-config bun run build from docs-site — passed: 561 pages and 77,818 internal links. The initial ordinary build failed because its telemetry config path was outside the writable workspace; the successful retry used an isolated local config directory and existing shared dependencies.
• Full-suite/import-derived changed-test execution was not run: concurrent security worktrees share limited resources, and authorized verification excludes live provider tests. Explicit offline regressions cover the changed boundaries; exact-head CI has passed under the repository's scope exception.
• Tests use synthetic metadata, signatures and app processes. Actual macOS code-signing, LaunchServices, ACL grants and volume ownership-policy behavior are not verified in this Linux environment. POSIX ancestry checks are not a complete attestation of native execution trust. Exact-head CI has passed. Native verification remains incomplete, so the fix outcome is still blocked for full native execution-trust assurance; that limitation and the maintainer security-review hold remain in force while this PR is open for review. One fresh read-only review identified the ancestor replacement case; it was reproduced and corrected within this scope.
• Remaining native plan: run these synthetic regressions on macOS; read-only check the local admin lookup, standard install-directory permissions and strict signing on a known app; validate ACL and ownership-disabled-volume cases in disposable fixtures. A real app quit/open check requires a separately authorized lifecycle run. Do not mutate an installed app or system permissions for testing.

Checklist

• Scope stays focused and avoids unrelated cleanup.
• Docs or release notes were updated when needed.
• Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

Exact-head review evidence

Cross-platform CI completed successfully for 50d9acd38133f8612a468397607c2469f81a940d. The documented focused local validation satisfies the repository's scoped-validation exception, and the branch is one commit behind current dev, within its readiness tolerance. No open Codex/CodeRabbit review threads were present at attestation. The previous CodeRabbit Draft status was a review skip; substantive review is now requested. Review readiness does not establish actual signing, LaunchServices, ACL or volume-policy verification, and does not grant merge or security-closure approval.

Summary by CodeRabbit

• Security
  • ChatGPT launch and restore now verify bundle ownership, permissions, and OpenAI signatures before proceeding. Checks also cover parent folders up to the filesystem root, with exceptions for trusted sticky folders and qualifying administrator-managed folders.
  • Restore checks the main app executable and can run without the experimental flag or a bundled app-server binary. If verification or relaunch fails, the existing launcher is retained.
• Documentation
  • Updated the ChatGPT desktop security guidance to describe verification requirements and note that native ACL and volume ownership-policy behavior is unverified.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

🧰 Additional context used

📚 Code guidelines (2)

src/AGENTS.md — auto-discovered
structure/AGENTS.md — auto-discovered

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: lidge-jun/opencodex/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: fca97804-eacc-4b49-890f-b74703076ddb

📥 Commits

Reviewing files that changed from the base of the PR and between 10428d0 and 50d9acd.

📒 Files selected for processing (6)

• docs-site/src/content/docs/guides/chatgpt-desktop.md
• src/chatgpt/app-server-shim/bundle-trust.ts
• src/cli/chatgpt-command.ts
• structure/clients/chatgpt-desktop.md
• tests/clients/desktop-app-server-shim-launcher.test.ts
• tests/helpers/desktop-app-server-shim-command-child.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review.

---

📝 Walkthrough

Walkthrough

Launch and restore now use bundle trust validation. The checks cover executable signatures, ownership, permissions, and ancestor directories. Restore can run without the experimental flag or a bundled app-server binary.

Changes

ChatGPT bundle trust

Layer / File(s)	Summary
Bundle targets and ancestor trust src/chatgpt/app-server-shim/bundle-trust.ts, tests/clients/desktop-app-server-shim-launcher.test.ts	The trust function checks either the app-server binary or, by default, the ChatGPT app executable. It also checks ancestor directories through the filesystem root, with exceptions for trusted sticky directories and qualifying root-owned admin-group directories. Tests cover ancestor ownership, permissions, and symlinks.
Launch and restore validation src/cli/chatgpt-command.ts, tests/helpers/desktop-app-server-shim-command-child.ts, tests/clients/desktop-app-server-shim-launcher.test.ts, docs-site/src/content/docs/guides/chatgpt-desktop.md, structure/clients/chatgpt-desktop.md	Launch retains its opt-in and bundled-binary requirements. Restore validates the bundle without either requirement. Tests cover validation failures, relaunch ordering, and launcher handling. Documentation describes the checks, exceptions, and limits.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant handleChatgptCommand
  participant untrustedChatgptBundleReason
  participant ChatGPTProcess
  participant ShimLauncher
  handleChatgptCommand->>untrustedChatgptBundleReason: validate bundle and app executable
  untrustedChatgptBundleReason-->>handleChatgptCommand: return trust result
  handleChatgptCommand->>ChatGPTProcess: quit and open when trusted
  ChatGPTProcess-->>handleChatgptCommand: return open result
  handleChatgptCommand->>ShimLauncher: retain on failure or remove after successful open

Loading

Merge Risk: ⚪ Minimal · up to 50d9a

No actionable defect is established for this change. Native security validation remains incomplete, and the stated maintainer security-review hold still applies.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 50d9a

The change strengthens validation before quitting or relaunching ChatGPT while preserving recovery behavior. No introduced security regression was established, but native filesystem permissions and replacement behavior remain incompletely verified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The inspected execution boundary concerns the invoking user's local ChatGPT relaunch and shim executable. Admission failures stop before process inspection or quit/open; successful admission leads to local application execution. The changed path does not request additional operating-system privileges.

Security Findings and Attack Paths

• observed — Validation and subsequent path-based opening are separate operations without identity pinning. That execution structure predates this PR, and base omitted ancestor admission entirely. The new exceptions therefore do not establish increased attacker reachability; native replacement-race exploitability remains unverified rather than a confirmed introduced finding.

Trust Boundaries and Controls

• observed — Ancestor admission rejects foreign ownership, symlinks, non-directories, and ordinary group/other write through the root. Write exceptions require either a trusted-owner sticky directory or a root-owned, non-world-writable directory whose group matches the locally resolved administrator group. These exceptions do not bypass bundle ownership or signature checks.

Resilience and Maintainability Implications

• observed — Command regressions exercise the real trust policy with synthetic filesystem, signature, discovery, and process responses. They support admission ordering and recovery-state assertions, but do not demonstrate native ACL, volume-ownership, or LaunchServices guarantees. The guide and architecture contract explicitly acknowledge the native permission limitations.

Hardening Proposals

• proposed — Validate the documented permission exceptions on native macOS, including ACLs, volume ownership settings, and replacement between verification and open. Use those results to define whether administrator-group replacement authority is trusted and whether stronger identity binding is needed.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 4 files. (2 skipped: 2…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the primary change: validating ChatGPT bundle trust before the restore flow.
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.

Full details: Docstring Coverage

Explanation

Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 4 files. (2 skipped: 2 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

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

[github-actions]

✅ Deterministic PR hygiene checks passed.

[lidge-jun]

Superseded by the integration in #6487, with reviewed follow-up fixes in #6490 and Windows validation repairs in #6494/#6495, all merged into dev.

Bundle trust validation before restore was carried. The recovery also makes the ancestor-boundary test fixture portable; it does not claim real installed-app trust/relaunch acceptance.

Original carry commit: 0358e72c8c8ec9a4708aff7401632454ff178a3e. Attribution to @luvs01 is preserved in the integration history and merge trailers. The final integrated candidate passed the complete cross-platform CI run.

Closing this PR as superseded, not claiming that its original head was merged. Thank you for the contribution.