cask/quarantine: gate metadata operations behind a single doorway

[aholland]

@MikeMcQuaid, the current and ongoing requirement for every caller of any of the quarantine methods to be mindful of the seven that raise when eg not on macOS, and to remember to gate said calls with the likes of return unless Quarantine.available? was bothering me. The recent PRs #23608 and #23617 were the second and third bugs due to this and #23226 was the first. This struck me as too much contract, not enough DRY. Here is a solution. That it makes the call sites simpler is a good sign. However, I recognise that the changes to Cask::Quarantine are significant.

The operations that read or write quarantine metadata (detect, status, release!, propagate, inherit_user_approval!, cask!, copy_xattrs) move onto Quarantine::Metadata, a class whose constructor is private and whose only instance is handed out by a doorway that performs the availability check once:

Quarantine.when_available { it.propagate(from: container.path, to:) }

When quarantine is unavailable the block simply doesn't run (the two multi-statement call sites in upgrade.rb use the do |quarantine| form). A call site can no longer forget the check, because the check is no longer the call site's job, and since constructing Metadata anywhere else is a NoMethodError, holding the instance proves the check has happened. The methods that are already safe everywhere (available?, user_approved?, user_approved_paths, signing_identity, signing_identity_match, app_management_permissions_granted?) stay on the module, and their callers are unchanged.

All seven call sites of the moved operations are converted (two each in download.rb, audit.rb and upgrade.rb, one in moved.rb), and each conversion deletes the guard it previously had to carry. The macOS implementations of cask! and copy_xattrs become an ordinary instance-method module prepended to Metadata. Behaviour is unchanged on every platform; this is a restructuring with no new features.

Details

Why a class with a private constructor rather than something simpler? I tried the simpler shapes first. A plain nested module works but leaves the operations directly callable, so the contract this PR exists to enforce would survive only as documentation. Making that module a private_constant enforces it fully but cannot be expressed within the codebase's own rules — the macOS extension and the specs then need const_get or module reopening, which Sorbet/ConstantsFromStrings and Style/OneClassPerFile rightly reject. Gating construction instead keeps every name public for Sorbet, prepends and specs, while still making the bypass (send(:new)) something no one can type by accident.

What moved and what didn't. Metadata holds the seven operations above plus the xattr locator they share. The module keeps everything that already returns a safe value without quarantine support, so the only surface the doorway gates is the set of operations that previously raised (unexpected nil xattr or NotImplementedError) when reached without support.

The one send. The private factory constructs the instance with Metadata.send(:new), the sanctioned use of send for a deliberately private API (as in api_hashable.rb and pour_bottle_check.rb), with a comment explaining why new is private.

Testing. The existing specs for the moved operations obtain the instance the same way production code does — through the doorway — and a new .when_available spec asserts both halves: it yields the operations when quarantine is available and returns nil without yielding when it isn't. The regression tests from #23608 and #23617 still pass, now asserting against the doorway's instance. brew lgtm --online is green.

Why the raising contract stays inside the class. The operations still raise if reached with no xattr — that loud failure is correct, and this PR deliberately does not soften it into silent no-ops, which would let a broken quarantine setup on macOS pass unnoticed. The doorway only relocates who is responsible for the check.

• 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 Hyperfine benchmarks.
• 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.

Used Claude Code (Fable 5) to investigate and draft; I directed the investigation and design choices, and reviewed the diff and every line of this PR text.

---

[MikeMcQuaid]

This doesn't seem like a good API/improvement to me.

If anything I'd say just:

Suggested change

	Quarantine.when_available { it.copy_xattrs(source, target, command:) }
	Quarantine.copy_xattrs(source, target, command:)

and make it a no-op when quarantine isn't available.

[aholland]

Thanks, considering this. Will follow up but likely only over the w/e.