Skip to content

audit: flag self-submissions by commit email - #24277

Merged
MikeMcQuaid merged 1 commit into
mainfrom
audit_cask_formula
Oct 17, 2026
Merged

MikeMcQuaid merged 1 commit into
mainfrom
audit_cask_formula

Conversation

@bevanjkay

@bevanjkay bevanjkay commented Oct 17, 2026 •

Copy link
Copy Markdown
Member
  • New cask and formula PRs for closed-source apps with little use are often self-submitted, but this is only caught by self-report or by a reviewer matching the commit author's email to the app's domain.
  • Treat a PR commit author whose email domain matches a non-forge homepage, url or head domain as a self-submission, so the existing self-submission notability thresholds apply.
  • Fail new packages self-submitted this way with no git forge repository, as their notability cannot be assessed automatically. The message makes clear this is not an automatic rejection and that the pull request author must provide evidence of notability.
  • Read commit emails from the GitHub API using the pull request's commits_url from GITHUB_EVENT_PATH, so checkout depth does not matter.

Open to discussion on the best way to handle the detected self-submissions. Currently failure with maintainer discretion to overrule is the method I chose.


  • 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 Opus 5.5 at Extra High effort, with local review and testing.


Summary by CodeRabbit

  • New Features
    • When online, audits check new formulae and casks for self-submission, including matches between package website or download domains and commit-author email domains.
    • Repository checks recognise projects hosted on GitHub, GitLab, Bitbucket and Forgejo.
  • Documentation
    • Clarified that repository owners and commit authors whose email domain matches a package URL are subject to the higher GitHub metrics threshold. Submissions without a public repository may still be considered if the pull request includes evidence of notability.

@coderabbitai

coderabbitai Bot commented Oct 17, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

📝 Walkthrough

Shared audits match package URL domains against pull-request commit author email domains. Online formula and cask audits check new submissions, and repository audits use shared notability patterns. The policy and CI matrix also account for self-submissions.

Changes: Self-submission detection

Layer / File(s) Summary
Shared URL and commit-email detection
Library/Homebrew/utils/shared_audits.rb, Library/Homebrew/test/utils/shared_audits_spec.rb
Shared helpers read commit author email domains from pull-request data, match those domains against package URL hosts, and report a self-submission when no recognised forge repository URL is present.
Formula and cask audit integration
Library/Homebrew/formula_auditor.rb, Library/Homebrew/cask/audit.rb, Library/Homebrew/test/formula_auditor_spec.rb, Library/Homebrew/test/cask/audit_spec.rb, Library/Homebrew/dev-cmd/generate-cask-ci-matrix.rb, docs/Package-Acceptance-Policy.md
Online audits check new formulas in homebrew/core and new casks in official taps. Repository-owner checks also consider URL domains matching commit email domains, and repository audits use shared notability patterns. The CI matrix applies the self_submission exception for ci-skip-self-submission. The policy describes the matching-domain threshold and evidence requirements.

Priority: ⬇️ Low

Merge Risk: 🟡 Moderate · up to 38538

Some qualifying self-submissions can pass without a request for notability evidence. Resolve the lookup-failure and nonexistent-repository gaps before merging.

Security architecture risk: 🔵 Low · up to 38538

The change adds a supplementary admission check without granting new privileges. Its main risk is that lookup failures silently disable email-based detection for the remainder of an audit process. Existing repository-owner checks remain in place, and notability decisions still allow maintainer discretion.

Retained concerns

  • Low · security · observed: The new email-based admission signal silently fails open after a terminal GitHub API error. Successfully fetched pages are discarded and an empty domain list is cached, so subsequent formula and cask checks in the same process cannot distinguish unavailable evidence from a genuine non-match. Existing owner-based checks remain active; this is a failure-containment gap in the added control, not a demonstrated regression in those checks.
Security review details

Security Blast Radius

  • observed — The new no-repository warning applies only to online audits of new core formulae and new official-tap casks. A shared lookup failure affects subsequent email-based decisions within the same audit process. The observed outcome is an omitted admission warning or higher notability threshold, not a grant of repository access or credentials.

Security Findings and Attack Paths

  • observed — A terminal commit lookup failure silently removes the added email signal for the remainder of the process, including any evidence already fetched successfully. Repository-owner classification remains an independent fallback. The source establishes this degraded outcome, but not an attacker-controlled way to induce it.
  • observed — The new forge exemption checks URL syntax, not repository existence. GitHub repository-notability checks return no warning for missing metadata in both base and head. That missing-repository behavior predates this PR and is not treated as a newly established admission bypass: separate URL checks provide counterevidence, and formula HEAD validation was not fully traced.

Trust Boundaries and Controls

  • inferred — The commit-domain signal is an admission heuristic, not authenticated proof of package ownership: it consumes raw commit author email fields without checking email verification or signature identity. Its use increases scrutiny rather than granting authority. Authenticated request destinations come from event metadata rather than package URLs; no PR-author control over that event field was established.

Resilience and Maintainability Implications

  • inferred — Cache reuse is coherent for one unchanged pull-request event. Reusing a process across different events could retain stale domains because the cache is not identity-keyed, but production reuse across pull requests was not established; this is a lifecycle uncertainty, not a verified cross-PR leak.

Hardening Proposals

  • proposed — Represent successful non-matches separately from unavailable commit evidence, expose the unavailable state to maintainers, and avoid permanently memoizing terminal lookup failures as successful empty results.
Pre-merge checks: Passed 4
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 main change: detecting self-submissions through commit email domains.
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.
  • 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: 3


ℹ️ Review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: eb7b0cf2-8a57-4a15-920b-087ed5ac70fa
📥 Commits

Reviewing files that changed from the base of the PR and between 19fe946 and f090944.

📒 Files selected for processing: 8
  • Library/Homebrew/cask/audit.rb
  • Library/Homebrew/dev-cmd/generate-cask-ci-matrix.rb
  • Library/Homebrew/formula_auditor.rb
  • Library/Homebrew/test/cask/audit_spec.rb
  • Library/Homebrew/test/formula_auditor_spec.rb
  • Library/Homebrew/test/utils/shared_audits_spec.rb
  • Library/Homebrew/utils/shared_audits.rb
  • docs/Package-Acceptance-Policy.md

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

Comment thread Library/Homebrew/utils/shared_audits.rb Outdated
Comment thread Library/Homebrew/utils/shared_audits.rb Outdated
Comment thread Library/Homebrew/utils/shared_audits.rb
- New cask and formula PRs for closed-source apps with little use are
  often self-submitted, but this is only caught by self-report or by a
  reviewer matching the commit author's email to the app's domain.
- Treat a PR commit author whose email domain matches a non-forge
  `homepage`, `url` or `head` domain as a self-submission, so the
  existing self-submission notability thresholds apply.
- Fail new packages self-submitted this way that have no repository the
  notability audits can assess. The message makes clear this is not an
  automatic rejection and that the pull request author must provide
  evidence of notability.
- Share the notability audits' repository regexes, anchored to the
  start of the URL, so "no repository" means exactly what those audits
  cannot measure.
- Add a `ci-skip-self-submission` label for casks so maintainers can
  skip the audit once they have judged notability themselves.
- Read every page of commit emails from the GitHub API using the pull
  request's `commits_url` from `GITHUB_EVENT_PATH`, so checkout depth
  does not matter.

@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: 78e93701-c05f-4960-9e00-f0e35b879ad9
📥 Commits

Reviewing files that changed from the base of the PR and between f090944 and 3853882.

📒 Files selected for processing: 2
  • Library/Homebrew/test/utils/shared_audits_spec.rb
  • Library/Homebrew/utils/shared_audits.rb

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

Comment thread Library/Homebrew/utils/shared_audits.rb

@MikeMcQuaid MikeMcQuaid left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Great, thanks!

@MikeMcQuaid

Copy link
Copy Markdown
Member

Currently failure with maintainer discretion to overrule is the method I chose.

Seems good.

@MikeMcQuaid
MikeMcQuaid added this pull request to the merge queue Oct 17, 2026
Merged via the queue into main with commit 433c0ad Oct 17, 2026
54 checks passed
@MikeMcQuaid
MikeMcQuaid deleted the audit_cask_formula branch October 17, 2026 08:43
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