Repository navigation
Preserve cask approval for copied helpers - #24191
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (4)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review. 📜 Recent review details⏰ Context from checks skipped due to timeout. (19)
🔇 Additional comments (5)
📝 SummarySummary by CodeRabbit
WalkthroughQuarantine approval inheritance now includes the download path and unique, non-symlink paths found beneath it. Cask upgrades no longer collect or pass approved paths. Tests cover nested helper-app paths and exclude symlink targets. ChangesQuarantine Approval Inheritance
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~12 minutes Change: Bug fix Suggested reviewers: Merge Risk: ⚪ Minimal · up to Approval inheritance remains within the intended app bundle, and no actionable merge-blocking risk remains. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to Approval now extends to more application contents, but upgrades still require prior app approval or absence of quarantine and successful signing-identity verification. No introduced boundary bypass was established. Concurrent filesystem changes and partial approval writes remain insufficiently characterized. Retained concerns Security review detailsSecurity Blast Radius
Security Findings and Attack Paths
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Organization UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
97c8c07a-7e7c-447f-8234-97f244256bfd
📒 Files selected for processing (4)
Library/Homebrew/cask/quarantine.rbLibrary/Homebrew/cask/upgrade.rbLibrary/Homebrew/test/cask/quarantine_spec.rbLibrary/Homebrew/test/cask/upgrade_spec.rb
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.
📜 Review details
⏰ Context from checks skipped due to timeout. (19)
- GitHub Check: docker (arm64 Ubuntu 24.04)
- GitHub Check: docker (x86_64 Ubuntu 26.04)
- GitHub Check: docker (arm64 Ubuntu 26.04)
- GitHub Check: docker (x86_64 Ubuntu 24.04)
- GitHub Check: tests (macOS, 2/2)
- GitHub Check: tests (load-only Linux)
- GitHub Check: tests (Linux, 2/2)
- GitHub Check: tests (Linux, 1/2)
- GitHub Check: tests (generic OS, 1/2)
- GitHub Check: tests (macOS, 1/2)
- GitHub Check: tests (no Sorbet)
- GitHub Check: tests (load-only macOS)
- GitHub Check: tests (generic OS, 2/2)
- GitHub Check: tests (online, 2/2)
- GitHub Check: tests (online, 1/2)
- GitHub Check: syntax
- GitHub Check: docs
- GitHub Check: workflow_syntax
- GitHub Check: Analyze
🔇 Additional comments (2)
Library/Homebrew/cask/upgrade.rb (1)
438-438: LGTM!Also applies to: 471-471
Library/Homebrew/test/cask/upgrade_spec.rb (1)
701-701: LGTM!
- ChatGPT launches its computer-use helper from an external copy. Approval recorded there is absent from the app bundle, so scanning only the bundle causes a fresh prompt after every upgrade. - Replace per-file approval tracking with approval throughout the verified app bundle, including new or previously unapproved helpers. The app becomes the approval boundary. - Keep the requirement that the old app was approved or unquarantined and the upgraded app passes signing-identity verification. Retain quarantine metadata and skip symlinks. - Inline `user_approved_status?` into `user_approved?` without changing its flag check, now that the per-file scanner no longer needs it. - Use the literal app directory as the glob base so wildcard characters in its name cannot grant approval to sibling apps.
b0dd035 to
0631239
Compare
user_approved_status?intouser_approved?without changing its flag check, now that the per-file scanner no longer needs it.brew benchmarkresults.brewcommands to reproduce the bug?brew lgtm(style, typechecking and tests) locally?Codex with GPT 6 Astra at Extra High effort, with local review and testing.