Merge pull request #17148 from alebcay/sandbox-strict-typing

sandbox: enable strict typing

Library/Homebrew/sandbox.rb

@@ -1,4 +1,4 @@
-# typed: true
+# typed: strict
 # frozen_string_literal: true
 
 require "erb"
@@ -20,51 +20,63 @@ class Sandbox
 
   sig { void }
   def initialize
-    @profile = SandboxProfile.new
+    @profile = T.let(SandboxProfile.new, SandboxProfile)
+    @failed = T.let(false, T::Boolean)
   end
 
+  sig { params(file: T.any(String, Pathname)).void }
   def record_log(file)
-    @logfile = file
+    @logfile = T.let(file, T.nilable(T.any(String, Pathname)))
   end
 
-  def add_rule(rule)
+  sig { params(allow: T::Boolean, operation: String, filter: T.nilable(String), modifier: T.nilable(String)).void }
+  def add_rule(allow:, operation:, filter: nil, modifier: nil)
+    rule = SandboxRule.new(allow:, operation:, filter:, modifier:)
     @profile.add_rule(rule)
   end
 
-  def allow_write(path, options = {})
+  sig { params(path: T.any(String, Pathname), type: Symbol).void }
-    add_rule allow: true, operation: "file-write*", filter: path_filter(path, options[:type])
+  def allow_write(path:, type: :literal)
-    add_rule allow: true, operation: "file-write-setugid", filter: path_filter(path, options[:type])
+    add_rule allow: true, operation: "file-write*", filter: path_filter(path, type)
+    add_rule allow: true, operation: "file-write-setugid", filter: path_filter(path, type)
   end
 
-  def deny_write(path, options = {})
+  sig { params(path: T.any(String, Pathname), type: Symbol).void }
-    add_rule allow: false, operation: "file-write*", filter: path_filter(path, options[:type])
+  def deny_write(path:, type: :literal)
+    add_rule allow: false, operation: "file-write*", filter: path_filter(path, type)
   end
 
+  sig { params(path: T.any(String, Pathname)).void }
   def allow_write_path(path)
-    allow_write path, type: :subpath
+    allow_write path:, type: :subpath
   end
 
+  sig { params(path: T.any(String, Pathname)).void }
   def deny_write_path(path)
-    deny_write path, type: :subpath
+    deny_write path:, type: :subpath
   end
 
+  sig { void }
   def allow_write_temp_and_cache
     allow_write_path "/private/tmp"
     allow_write_path "/private/var/tmp"
-    allow_write "^/private/var/folders/[^/]+/[^/]+/[C,T]/", type: :regex
+    allow_write path: "^/private/var/folders/[^/]+/[^/]+/[C,T]/", type: :regex
     allow_write_path HOMEBREW_TEMP
     allow_write_path HOMEBREW_CACHE
   end
 
+  sig { void }
   def allow_cvs
     allow_write_path "#{Dir.home(ENV.fetch("USER"))}/.cvspass"
   end
 
+  sig { void }
   def allow_fossil
     allow_write_path "#{Dir.home(ENV.fetch("USER"))}/.fossil"
     allow_write_path "#{Dir.home(ENV.fetch("USER"))}/.fossil-journal"
   end
 
+  sig { params(formula: Formula).void }
   def allow_write_cellar(formula)
     allow_write_path formula.rack
     allow_write_path formula.etc
@@ -72,17 +84,20 @@ class Sandbox
   end
 
   # Xcode projects expect access to certain cache/archive dirs.
+  sig { void }
   def allow_write_xcode
     allow_write_path "#{Dir.home(ENV.fetch("USER"))}/Library/Developer"
     allow_write_path "#{Dir.home(ENV.fetch("USER"))}/Library/Caches/org.swift.swiftpm"
   end
 
+  sig { params(formula: Formula).void }
   def allow_write_log(formula)
     allow_write_path formula.logs
   end
 
+  sig { void }
   def deny_write_homebrew_repository
-    deny_write HOMEBREW_BREW_FILE
+    deny_write path: HOMEBREW_BREW_FILE
     if HOMEBREW_PREFIX.to_s == HOMEBREW_REPOSITORY.to_s
       deny_write_path HOMEBREW_LIBRARY
       deny_write_path HOMEBREW_REPOSITORY/".git"
@@ -117,11 +132,12 @@ class Sandbox
     allow_network path:, type: :literal
   end
 
+  sig { params(args: T.any(String, Pathname)).void }
   def exec(*args)
     seatbelt = Tempfile.new(["homebrew", ".sb"], HOMEBREW_TEMP)
     seatbelt.write(@profile.dump)
     seatbelt.close
-    @start = Time.now
+    @start = T.let(Time.now, T.nilable(Time))
 
     begin
       command = [SANDBOX_EXEC, "-f", seatbelt.path, *args]
@@ -219,20 +235,46 @@ class Sandbox
 
   private
 
+  sig { params(path: Pathname).returns(Pathname) }
   def expand_realpath(path)
     raise unless path.absolute?
 
     path.exist? ? path.realpath : expand_realpath(path.parent)/path.basename
   end
 
+  sig { params(path: T.any(String, Pathname), type: Symbol).returns(String) }
   def path_filter(path, type)
     case type
-    when :regex        then "regex #\"#{path}\""
+    when :regex   then "regex #\"#{path}\""
-    when :subpath      then "subpath \"#{expand_realpath(Pathname.new(path))}\""
+    when :subpath then "subpath \"#{expand_realpath(Pathname.new(path))}\""
-    when :literal, nil then "literal \"#{expand_realpath(Pathname.new(path))}\""
+    when :literal then "literal \"#{expand_realpath(Pathname.new(path))}\""
+    else raise ArgumentError, "Invalid path filter type: #{type}"
     end
   end
 
+  class SandboxRule
+    sig { returns(T::Boolean) }
+    attr_reader :allow
+
+    sig { returns(String) }
+    attr_reader :operation
+
+    sig { returns(T.nilable(String)) }
+    attr_reader :filter
+
+    sig { returns(T.nilable(String)) }
+    attr_reader :modifier
+
+    sig { params(allow: T::Boolean, operation: String, filter: T.nilable(String), modifier: T.nilable(String)).void }
+    def initialize(allow:, operation:, filter:, modifier:)
+      @allow = allow
+      @operation = operation
+      @filter = filter
+      @modifier = modifier
+    end
+  end
+  private_constant :SandboxRule
+
   # Configuration profile for a sandbox.
   class SandboxProfile
     SEATBELT_ERB = <<~ERB
@@ -256,23 +298,26 @@ class Sandbox
       (allow default) ; allow everything else
     ERB
 
+    sig { returns(T::Array[String]) }
     attr_reader :rules
 
     sig { void }
     def initialize
-      @rules = []
+      @rules = T.let([], T::Array[String])
     end
 
+    sig { params(rule: SandboxRule).void }
     def add_rule(rule)
       s = +"("
-      s << (rule[:allow] ? "allow" : "deny")
+      s << (rule.allow ? "allow" : "deny")
-      s << " #{rule[:operation]}"
+      s << " #{rule.operation}"
-      s << " (#{rule[:filter]})" if rule[:filter]
+      s << " (#{rule.filter})" if rule.filter
-      s << " (with #{rule[:modifier]})" if rule[:modifier]
+      s << " (with #{rule.modifier})" if rule.modifier
       s << ")"
       @rules << s.freeze
     end
 
+    sig { returns(String) }
     def dump
       ERB.new(SEATBELT_ERB).result(binding)
     end

Library/Homebrew/test/sandbox_spec.rb

@@ -15,7 +15,7 @@ RSpec.describe Sandbox, :needs_macos do
   end
 
   specify "#allow_write" do
-    sandbox.allow_write file
+    sandbox.allow_write path: file
     sandbox.exec "touch", file
 
     expect(file).to exist