Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cask/audit: add audit_min_os #16013

Merged
merged 3 commits into from
Nov 6, 2023
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
117 changes: 91 additions & 26 deletions Library/Homebrew/cask/audit.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
require "digest"
require "livecheck/livecheck"
require "source_location"
require "system_command"
require "utils/curl"
require "utils/git"
require "utils/shared_audits"
Expand Down Expand Up @@ -474,21 +475,8 @@
return if !signing? || download.blank? || cask.url.blank?

odebug "Auditing signing"
artifacts = cask.artifacts.select do |k|
k.is_a?(Artifact::Pkg) || k.is_a?(Artifact::App) || k.is_a?(Artifact::Binary)
end

return if artifacts.empty?

downloaded_path = download.fetch
primary_container = UnpackStrategy.detect(downloaded_path, type: @cask.container&.type, merge_xattrs: true)

return if primary_container.nil?

Dir.mktmpdir do |tmpdir|
tmpdir = Pathname(tmpdir)
primary_container.extract_nestedly(to: tmpdir, basename: downloaded_path.basename, verbose: false)

extract_artifacts do |artifacts, tmpdir|
artifacts.each do |artifact|
artifact_path = artifact.is_a?(Artifact::Pkg) ? artifact.path : artifact.source
path = tmpdir/artifact_path.relative_path_from(cask.staged_path)
Expand All @@ -509,6 +497,38 @@
end
end

sig { void }
def extract_artifacts
return unless online?

artifacts = cask.artifacts.select do |artifact|
artifact.is_a?(Artifact::Pkg) || artifact.is_a?(Artifact::App) || artifact.is_a?(Artifact::Binary)

Check warning on line 505 in Library/Homebrew/cask/audit.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/cask/audit.rb#L504-L505

Added lines #L504 - L505 were not covered by tests
end

if @artifacts_extracted && @tmpdir
yield artifacts, @tmpdir if block_given?
return

Check warning on line 510 in Library/Homebrew/cask/audit.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/cask/audit.rb#L510

Added line #L510 was not covered by tests
end

return if artifacts.empty?

@tmpdir ||= Pathname(Dir.mktmpdir)

Check warning on line 515 in Library/Homebrew/cask/audit.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/cask/audit.rb#L515

Added line #L515 was not covered by tests

ohai "Downloading and extracting artifacts"

Check warning on line 517 in Library/Homebrew/cask/audit.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/cask/audit.rb#L517

Added line #L517 was not covered by tests

downloaded_path = download.fetch

Check warning on line 519 in Library/Homebrew/cask/audit.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/cask/audit.rb#L519

Added line #L519 was not covered by tests

primary_container = UnpackStrategy.detect(downloaded_path, type: @cask.container&.type, merge_xattrs: true)
return if primary_container.nil?

# Extract the container to the temporary directory.
primary_container.extract_nestedly(to: @tmpdir, basename: downloaded_path.basename, verbose: false)
@artifacts_extracted = true # Set the flag to indicate that extraction has occurred.

Check warning on line 526 in Library/Homebrew/cask/audit.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/cask/audit.rb#L525-L526

Added lines #L525 - L526 were not covered by tests

# Yield the artifacts and temp directory to the block if provided.
yield artifacts, @tmpdir if block_given?
end

sig { returns(T.any(NilClass, T::Boolean, Symbol)) }
def audit_livecheck_version
return unless online?
Expand Down Expand Up @@ -540,7 +560,39 @@
false
end

def audit_livecheck_min_os
sig { void }
def audit_min_os
return unless online?
return unless strict?

odebug "Auditing minimum OS version"

Check warning on line 568 in Library/Homebrew/cask/audit.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/cask/audit.rb#L568

Added line #L568 was not covered by tests

plist_min_os = cask_plist_min_os
sparkle_min_os = livecheck_min_os

Check warning on line 571 in Library/Homebrew/cask/audit.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/cask/audit.rb#L570-L571

Added lines #L570 - L571 were not covered by tests

debug_messages = []

Check warning on line 573 in Library/Homebrew/cask/audit.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/cask/audit.rb#L573

Added line #L573 was not covered by tests
debug_messages << "Plist #{plist_min_os}" if plist_min_os
debug_messages << "Sparkle #{sparkle_min_os}" if sparkle_min_os
odebug "Minimum OS version: #{debug_messages.join(" | ")}" unless debug_messages.empty?
min_os = [sparkle_min_os, plist_min_os].compact.max

Check warning on line 577 in Library/Homebrew/cask/audit.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/cask/audit.rb#L577

Added line #L577 was not covered by tests

return if min_os.nil? || min_os <= HOMEBREW_MACOS_OLDEST_ALLOWED

cask_min_os = cask.depends_on.macos&.version
return if cask_min_os == min_os

min_os_symbol = if cask_min_os.present?

Check warning on line 584 in Library/Homebrew/cask/audit.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/cask/audit.rb#L584

Added line #L584 was not covered by tests
cask_min_os.to_sym.inspect
else
"no minimum OS version"
end
add_error "Upstream defined #{min_os.to_sym.inspect} as the minimum OS version " \

Check warning on line 589 in Library/Homebrew/cask/audit.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/cask/audit.rb#L589

Added line #L589 was not covered by tests
"and the cask defined #{min_os_symbol}",
strict_only: true
end

sig { returns(T.nilable(MacOSVersion)) }
def livecheck_min_os
return unless online?
return unless cask.livecheckable?
return if cask.livecheck.strategy != :sparkle
Expand All @@ -566,24 +618,37 @@
return if min_os.blank?

begin
min_os_string = MacOSVersion.new(min_os).strip_patch
MacOSVersion.new(min_os).strip_patch

Check warning on line 621 in Library/Homebrew/cask/audit.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/cask/audit.rb#L621

Added line #L621 was not covered by tests
rescue MacOSVersion::Error
return
nil

Check warning on line 623 in Library/Homebrew/cask/audit.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/cask/audit.rb#L623

Added line #L623 was not covered by tests
end
end

return if min_os_string <= HOMEBREW_MACOS_OLDEST_ALLOWED
sig { returns(T.nilable(MacOSVersion)) }
def cask_plist_min_os
return unless online?

cask_min_os = cask.depends_on.macos&.version
plist_min_os = T.let(nil, T.untyped)
@staged_path ||= cask.staged_path

Check warning on line 632 in Library/Homebrew/cask/audit.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/cask/audit.rb#L631-L632

Added lines #L631 - L632 were not covered by tests

return if cask_min_os == min_os_string
extract_artifacts do |artifacts, tmpdir|
artifacts.each do |artifact|

Check warning on line 635 in Library/Homebrew/cask/audit.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/cask/audit.rb#L634-L635

Added lines #L634 - L635 were not covered by tests
artifact_path = artifact.is_a?(Artifact::Pkg) ? artifact.path : artifact.source
path = tmpdir/artifact_path.relative_path_from(cask.staged_path)
plist_path = "#{path}/Contents/Info.plist"

Check warning on line 638 in Library/Homebrew/cask/audit.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/cask/audit.rb#L637-L638

Added lines #L637 - L638 were not covered by tests
Copy link
Member

Choose a reason for hiding this comment

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

Can this somehow be the only file that's extracted?

Copy link
Member Author

Choose a reason for hiding this comment

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

There's some overlap between this method and audit_signing and perhaps it could make sense to have a common method for both? Otherwise, I'm happy to take a look at fixing this later on.

Copy link
Member

Choose a reason for hiding this comment

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

@razvanazamfirei any more thoughts on this? I don't know if there's decent enough support in these classes for partial extraction, ok if not, but nice to avoid extracting the entire archive if we can avoid it.

next unless File.exist?(plist_path)

min_os_symbol = if cask_min_os.present?
cask_min_os.to_sym.inspect
else
"no minimum OS version"
plist = system_command!("plutil", args: ["-convert", "xml1", "-o", "-", plist_path]).plist
plist_min_os = plist["LSMinimumSystemVersion"].presence

Check warning on line 642 in Library/Homebrew/cask/audit.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/cask/audit.rb#L641-L642

Added lines #L641 - L642 were not covered by tests
break if plist_min_os
end
end

begin
MacOSVersion.new(plist_min_os).strip_patch
rescue MacOSVersion::Error
nil

Check warning on line 650 in Library/Homebrew/cask/audit.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/cask/audit.rb#L648-L650

Added lines #L648 - L650 were not covered by tests
end
add_error "Upstream defined #{min_os_string.to_sym.inspect} as the minimum OS version " \
"and the cask defined #{min_os_symbol}"
end

sig { void }
Expand Down