Skip to content

Stop fetching unsigned per-package API data - #23772

Open
MikeMcQuaid wants to merge 1 commit into
mainfrom
analytics-only-unsigned-api
Open

Stop fetching unsigned per-package API data#23772
MikeMcQuaid wants to merge 1 commit into
mainfrom
analytics-only-unsigned-api

Conversation

@MikeMcQuaid

Copy link
Copy Markdown
Member

Remove the fetching, caching and public accessors for the unsigned formula/<name>.json and cask/<token>.json API files. Everything they contain, other than analytics, is already available from the JWS-signed formula.jws.json, cask.jws.json and internal packages.*.jws.json files, so an endpoint that could be used insecurely was being kept around for data Homebrew did not need.

An audit found only two consumers. brew info read the analytics key, which the signed files do not carry. Cask uninstall recovery read the unsigned artifacts to decide which files to delete when installed metadata lacked them, and the public formula_json/cask_json accessors plus the on-disk cache under api/formula and api/cask made the whole unsigned response available to any other code path.

Uninstall recovery in cask/cask_loader.rb now loads the cask through the signed internal API loader or the installed tap and otherwise returns no artifacts, relying on the existing "files may remain" warning. brew info keeps its analytics summary and --analytics tables through new formula_analytics and cask_analytics methods in api/analytics.rb. These curl the endpoint straight into memory, keep the analytics hash only if it is one and cache just that slice; the HTTP helper and reducer are private so nothing else can observe the rest of the response. Homebrew::API.fetch had no other callers and becomes that private helper. cleanup.rb no longer prunes per-resource cache directories and cmd/update.sh deletes the leftover ones.

Specs stub the signed API loader instead of the removed accessors and a new api/analytics_spec checks that only analytics data survives.


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

Claude with Fable at High effort, with local review and testing.


Copilot AI balanced review requested due to automatic review settings September 4, 2026 13:16

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

馃煛 Changes recommended

Malformed analytics responses and missing signed payloads can escape recovery handling, while API-opt-out users retain legacy caches.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Removes general access to unsigned per-package API responses while retaining analytics and signed cask recovery paths.

Changes:

  • Adds analytics-only in-memory API accessors.
  • Uses signed API or tap metadata for cask recovery.
  • Removes obsolete per-resource caching and cleanup logic.
File summaries
File Description
Library/Homebrew/utils/analytics.rb Uses analytics-only responses.
Library/Homebrew/test/utils/analytics_spec.rb Updates analytics output tests.
Library/Homebrew/test/cmd/update-report_spec.rb Stubs signed cask loaders.
Library/Homebrew/test/cleanup_spec.rb Removes obsolete cache tests.
Library/Homebrew/test/cask/upgrade_spec.rb Updates recovery stubs.
Library/Homebrew/test/cask/uninstall_spec.rb Updates unavailable-cask setup.
Library/Homebrew/test/cask/caskroom_spec.rb Uses signed loader metadata.
Library/Homebrew/test/cask/cask_loader/from_api_loader_spec.rb Updates loader fallback tests.
Library/Homebrew/test/cask/cask_loader_spec.rb Revises artifact recovery coverage.
Library/Homebrew/test/api/formula_spec.rb Removes per-formula API tests.
Library/Homebrew/test/api/cask_spec.rb Removes per-cask API tests.
Library/Homebrew/test/api/analytics_spec.rb Tests analytics-only reduction.
Library/Homebrew/test/api_spec.rb Removes public fetch tests.
Library/Homebrew/cmd/update.sh Deletes legacy cache directories.
Library/Homebrew/cleanup.rb Stops enumerating obsolete caches.
Library/Homebrew/cask/cask_loader.rb Uses signed or tapped cask metadata.
Library/Homebrew/api/formula.rb Removes per-formula accessors.
Library/Homebrew/api/cask.rb Removes per-cask accessors.
Library/Homebrew/api/analytics.rb Adds private fetching and reduction.
Library/Homebrew/api.rb Removes the general unsigned fetch API.
Review details
  • Files reviewed: 20/20 changed files
  • Comments generated: 3
  • Review effort level: Balanced

馃挕 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

Comment thread Library/Homebrew/api/analytics.rb Outdated
Comment thread Library/Homebrew/cask/cask_loader.rb Outdated
Comment thread Library/Homebrew/cmd/update.sh Outdated
@MikeMcQuaid
MikeMcQuaid force-pushed the analytics-only-unsigned-api branch from 0be519e to cf8c049 Compare September 4, 2026 14:49
Remove the public accessors for the unsigned `formula/<name>.json` and
`cask/<token>.json` API files and use them for nothing but `brew info`
analytics. Everything else they contain is already available from the
JWS-signed `formula.jws.json`, `cask.jws.json` and internal
`packages.*.jws.json` files, so an endpoint that could be used
insecurely was being kept around for data Homebrew did not need.

An audit found only two consumers. `brew info` read the `analytics`
key, which the signed files do not carry. Cask uninstall recovery read
the unsigned `artifacts` to decide which files to delete when installed
metadata lacked them, and the public `formula_json`/`cask_json`
accessors made the whole unsigned response available to any other code
path.

Uninstall recovery in `cask/cask_loader.rb` now loads the cask through
the signed internal API loader or the installed tap and otherwise
returns no artifacts, relying on the existing "files may remain"
warning. `brew info` keeps its analytics summary and `--analytics`
tables through new `formula_analytics` and `cask_analytics` methods in
`api/analytics.rb`. These still download the file into the API cache,
revalidated hourly so repeated runs stay cheap and `brew cleanup` prunes
it as before, but return only the `analytics` hash, if it is one, and
keep only that slice in memory. The helper and reducer are private so
nothing else can observe the rest of the response. `Homebrew::API.fetch`
had no other callers and moves into the analytics module as a private
helper that also rejects non-object JSON.

Specs stub the signed API loader instead of the removed accessors and a
new `api/analytics_spec` checks that only analytics data survives and
that a fresh cache is reused.
@MikeMcQuaid
MikeMcQuaid force-pushed the analytics-only-unsigned-api branch from cf8c049 to fd2e45b Compare September 4, 2026 15:06
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.

2 participants