Skip to content

cask/audit: skip resource-only bundles - #24247

Merged
MikeMcQuaid merged 1 commit into
mainfrom
cask-audit-resource-bundles
Oct 13, 2026
Merged

MikeMcQuaid merged 1 commit into
mainfrom
cask-audit-resource-bundles

Conversation

@MikeMcQuaid

Copy link
Copy Markdown
Member
  • Avoid false Gatekeeper failures for localisation resources.
  • Preserve checks for bundles with metadata or native code.

Fixes #24246


  • Have you followed our Contributing guidelines?
  • Have you checked for other open Pull Requests for the same change?
  • Have you explained what your changes do? Performance claims (e.g. "this is faster") must include brew benchmark results.
  • Have you explained why you'd like these changes included, not just what they do?
  • For bug fixes, have you given step-by-step brew commands to reproduce the bug?
  • Have you written new tests (excluding integration tests)? Here's an example.
  • Have you successfully run brew lgtm (style, typechecking and tests) locally?

  • I did not use AI/LLM to create this PR, or I disclosed the tool/model below and reviewed its output; I did not attribute commits to AI and will answer maintainer questions and review comments myself without AI/LLM.

Codex with GPT 6 Astra at Extra High effort, with local review and testing.


@coderabbitai

coderabbitai Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 15a77896-b271-485f-a652-6a9f944df6e2
📥 Commits

Reviewing files that changed from the base of the PR and between f9c62f6 and 574f86e.

📒 Files selected for processing: 2
  • Library/Homebrew/cask/audit.rb
  • Library/Homebrew/test/cask/audit_spec.rb

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
  • GitHub Check: docker (x86_64 Ubuntu 24.04)
  • GitHub Check: docker (x86_64 Ubuntu 26.04)
  • GitHub Check: docker (arm64 Ubuntu 24.04)
  • GitHub Check: docker (arm64 Ubuntu 26.04)
  • GitHub Check: tests (no Sorbet)
  • GitHub Check: workflow_syntax
  • GitHub Check: tests (generic OS, 2/2)
  • GitHub Check: tests (Linux, 2/2)
  • GitHub Check: tests (load-only macOS)
  • GitHub Check: tests (online, 1/2)
  • GitHub Check: tests (macOS, 1/2)
  • GitHub Check: tests (Linux, 1/2)
  • GitHub Check: tests (load-only Linux)
  • GitHub Check: tests (online, 2/2)
  • GitHub Check: tests (macOS, 2/2)
  • GitHub Check: Analyze
  • GitHub Check: tests (generic OS, 1/2)
  • GitHub Check: syntax
  • GitHub Check: docs
🔇 Additional comments: 2
Library/Homebrew/cask/audit.rb: 1

677-679: LGTM!

Also applies to: 684-696

Library/Homebrew/test/cask/audit_spec.rb: 1

1017-1060: LGTM!

📝 Summary

Summary by CodeRabbit

  • Bug Fixes
    • Signing audits now distinguish code-bearing .bundle folders from resource-only folders, including when bundle metadata is missing.
    • Code-bearing bundles are checked for signature issues, while resource-only bundles are skipped. Nested apps and bundles with supported metadata continue to be checked.

The signing audit now checks .bundle directories without supported Info.plist files for executable, dylib, or Mach-O bundle contents before Gatekeeper scanning. Tests cover resource-only bundles and bundles that remain subject to scanning.

Changes: Bundle signing audit

Layer / File(s) Summary
Filter and test suite bundles
Library/Homebrew/cask/audit.rb, Library/Homebrew/test/cask/audit_spec.rb
When a .bundle lacks both supported Info.plist paths, the audit searches its descendants for executable, dylib, or Mach-O bundle files before scanning. Tests cover resource-only bundles, bundles with supported plist files or code, and nested bundles.

Priority: ➖ Normal

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

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: bevanjkay

Merge Risk: ⚪ Minimal · up to 574f8

Resource-only bundles remain traversable, and nested apps remain eligible for signing checks. No actionable merge-blocking issue remains after normal checks.

Security architecture risk: 🔵 Low · up to 574f8

The change is narrowly scoped and preserves checks for bundles with metadata, recognized native code, and nested bundles. An uncertainty remains for code that cannot be classified, but a practical signing-check bypass has not been demonstrated.

Retained concerns

  • Low · security · inferred: The resource-only exemption does not distinguish absence of native code from unsuccessful native-code inspection. Outside developer mode, parsing errors yield false predicates, so a plist-free bundle with no other qualifying descendant can lose its directory-level Gatekeeper scan. A bypass involving code accepted by macOS is possible but unverified.
Security review details

Security Blast Radius

  • inferred — A publisher controlling downloaded Suite/Artifact contents can influence bundle extensions, metadata presence, and inspected files. The changed boundary affects confidence in signing-audit outcomes; the modified branch does not itself execute those payloads or introduce additional privileges.

Security Findings and Attack Paths

  • inferred — The unresolved attack path requires a metadata-free bundle containing native code that macOS accepts but binary inspection cannot classify, with no other qualifying descendant. In non-developer mode this could omit the bundle scan; the loose-file fallback also requires a positive executable predicate. Neither a loadable example nor effective workflow reachability was demonstrated, so this is an inferred control gap rather than a verified vulnerability.

Trust Boundaries and Controls

  • observed — Either recognized Info.plist location preserves direct scanning. Any successfully recognized executable, dylib, or Mach-O bundle descendant also preserves it, without requiring an executable permission bit during eligibility classification. Nested resource-parent traversal preserves independently assessed bundles.
  • observed — If gktool is unavailable, the existing adapter warns and returns no result, which the audit does not treat as a signing failure. This behavior predates the PR and is not an introduced concern.

Resilience and Maintainability Implications

  • observed — The new tests establish routing decisions using stubbed binary predicates and scanner results. They do not establish compatibility between parser rejection and macOS code acceptance, or live Gatekeeper behavior for those inputs.

Hardening Proposals

  • proposed — Distinguish known resource files from indeterminate native-code inspection. An indeterminate result could retain scanning or explicitly fail eligibility assessment rather than grant the resource-only exemption.
Pre-merge checks: Passed 4
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main change: skipping resource-only bundles during cask auditing.
Description check Passed The description directly explains the false Gatekeeper failure fix, preserves checks for relevant bundles, and identifies the related issue and tests.
Linked Issues check Passed The PR meets issue #24246. audit_signing skips .bundle directories that have no Contents/Info.plist or Info.plist and no assessable native code. It still searches for executable, dylib, and Ma…
Out of Scope Changes check Passed The changes stay within issue #24246. Production changes are limited to cask signing-audit bundle detection. The added tests verify the required skip and preserve behavior. No unrelated production beh…
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 10268e64-c4ef-4271-a7ed-1a7367a8135f
📥 Commits

Reviewing files that changed from the base of the PR and between eb56e2b and f9c62f6.

📒 Files selected for processing: 2
  • Library/Homebrew/cask/audit.rb
  • Library/Homebrew/test/cask/audit_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 (x86_64 Ubuntu 26.04)
  • GitHub Check: docker (arm64 Ubuntu 24.04)
  • GitHub Check: docker (x86_64 Ubuntu 24.04)
  • GitHub Check: docker (arm64 Ubuntu 26.04)
  • GitHub Check: docs
  • GitHub Check: tests (generic OS, 2/2)
  • GitHub Check: tests (online, 1/2)
  • GitHub Check: tests (macOS, 1/2)
  • GitHub Check: tests (macOS, 2/2)
  • GitHub Check: tests (load-only macOS)
  • GitHub Check: tests (online, 2/2)
  • GitHub Check: tests (load-only Linux)
  • GitHub Check: tests (generic OS, 1/2)
  • GitHub Check: tests (no Sorbet)
  • GitHub Check: tests (Linux, 1/2)
  • GitHub Check: tests (Linux, 2/2)
  • GitHub Check: syntax
  • GitHub Check: workflow_syntax
  • GitHub Check: Analyze

Comment thread Library/Homebrew/cask/audit.rb
- Avoid false Gatekeeper failures for localisation resources.
- Preserve checks for bundles with metadata or native code.
- Assess nested bundles separately from their resource-only parents.

Fixes #24246
@MikeMcQuaid
MikeMcQuaid force-pushed the cask-audit-resource-bundles branch from f9c62f6 to 574f86e Compare October 13, 2026 19:51
@MikeMcQuaid
MikeMcQuaid enabled auto-merge October 13, 2026 19:56
@MikeMcQuaid
MikeMcQuaid added this pull request to the merge queue Oct 13, 2026
Merged via the queue into main with commit 642c2c4 Oct 13, 2026
52 checks passed
@MikeMcQuaid
MikeMcQuaid deleted the cask-audit-resource-bundles branch October 13, 2026 20:24
@bevanjkay

Copy link
Copy Markdown
Member

Thanks for fixing this one @MikeMcQuaid

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

cask/audit: gktool scan rejects resource-only .bundle directories inside suites, causing false fails_gatekeeper_check failures

3 participants