Merge pull request #4852 from amyspark/quarantine-per-install

Cask: fixes for quarantining

Library/Homebrew/cask/download.rb

@@ -7,7 +7,7 @@ module Cask
   class Download
     attr_reader :cask
 
-    def initialize(cask, force: false, quarantine: true)
+    def initialize(cask, force: false, quarantine: nil)
       @cask = cask
       @force = force
       @quarantine = quarantine
@@ -46,11 +46,14 @@ def fetch
     end
 
     def quarantine
-      return unless @quarantine
+      return if @quarantine.nil?
       return unless Quarantine.available?
-      return if Quarantine.detect(@downloaded_path)
 
-      Quarantine.cask(cask: @cask, download_path: @downloaded_path)
+      if @quarantine
+        Quarantine.cask!(cask: @cask, download_path: @downloaded_path)
+      else
+        Quarantine.release!(download_path: @downloaded_path)
+      end
     end
   end
 end

Library/Homebrew/cask/exceptions.rb

@@ -184,4 +184,18 @@ def to_s
       s
     end
   end
+
+  class CaskQuarantineReleaseError < CaskQuarantineError
+    def to_s
+      s = "Failed to release #{path} from quarantine."
+
+      unless reason.empty?
+        s << " Here's the reason:\n"
+        s << Formatter.error(reason)
+        s << "\n" unless reason.end_with?("\n")
+      end
+
+      s
+    end
+  end
 end

Library/Homebrew/cask/quarantine.rb

@@ -55,18 +55,50 @@ def status(file)
                      print_stderr: false).stdout.rstrip
     end
 
-    def cask(cask: nil, download_path: nil)
+    def toggle_no_translocation_bit(xattr)
+      fields = xattr.split(";")
+
+      # Fields: status, epoch, download agent, event ID
+      # Let's toggle the app translocation bit, bit 8
+      # http://openradar.me/radar?id=5022734169931776
+
+      fields[0] = (fields[0].to_i(16) | 0x0100).to_s(16).rjust(4, "0")
+
+      fields.join(";")
+    end
+
+    def release!(download_path: nil)
+      return unless detect(download_path)
+
+      odebug "Releasing #{download_path} from quarantine"
+
+      quarantiner = system_command("/usr/bin/xattr",
+                                  args: [
+                                    "-d",
+                                    QUARANTINE_ATTRIBUTE,
+                                    download_path,
+                                  ],
+                                  print_stderr: false)
+
+      return if quarantiner.success?
+
+      raise CaskQuarantineReleaseError.new(download_path, quarantiner.stderr)
+    end
+
+    def cask!(cask: nil, download_path: nil, action: true)
       return if cask.nil? || download_path.nil?
 
+      return if detect(download_path)
+
       odebug "Quarantining #{download_path}"
 
       quarantiner = system_command(swift,
-                                   args: [
-                                     QUARANTINE_SCRIPT,
-                                     download_path,
-                                     cask.url.to_s,
-                                     cask.homepage.to_s,
-                                   ])
+                                  args: [
+                                    QUARANTINE_SCRIPT,
+                                    download_path,
+                                    cask.url.to_s,
+                                    cask.homepage.to_s,
+                                  ])
 
       return if quarantiner.success?
 
@@ -85,10 +117,12 @@ def propagate(from: nil, to: nil)
 
       odebug "Propagating quarantine from #{from} to #{to}"
 
-      quarantine_status = status(from)
+      quarantine_status = toggle_no_translocation_bit(status(from))
 
       resolved_paths = Pathname.glob(to/"**/*", File::FNM_DOTMATCH)
 
+      system_command!("/bin/chmod", args: ["-R", "u+w", to])
+
       quarantiner = system_command("/usr/bin/xargs",
                                    args: [
                                      "-0",

Library/Homebrew/test/cask/cmd/fetch_spec.rb

@@ -37,7 +37,7 @@
 
     old_ctime = File.stat(cached_location).ctime
 
-    described_class.run("local-transmission")
+    described_class.run("local-transmission", "--no-quarantine")
     new_ctime = File.stat(cached_location).ctime
 
     expect(old_ctime.to_i).to eq(new_ctime.to_i)
@@ -49,7 +49,7 @@
     old_ctime = File.stat(cached_location).ctime
     sleep(1)
 
-    described_class.run("local-transmission", "--force")
+    described_class.run("local-transmission", "--force", "--no-quarantine")
     new_ctime = File.stat(cached_location).ctime
 
     expect(new_ctime.to_i).to be > old_ctime.to_i

Library/Homebrew/test/cask/cmd/quarantine_spec.rb → Library/Homebrew/test/cask/quarantine_spec.rb

@@ -23,7 +23,7 @@
     it "quarantines Cask fetches" do
       Cask::Cmd::Fetch.run("local-transmission")
       local_transmission = Cask::CaskLoader.load(cask_path("local-transmission"))
-      cached_location = Cask::Download.new(local_transmission, force: false, quarantine: false).perform
+      cached_location = Cask::Download.new(local_transmission).perform
 
       expect(cached_location).to be_quarantined
     end
@@ -32,11 +32,23 @@
       Cask::Cmd::Audit.run("local-transmission", "--download")
 
       local_transmission = Cask::CaskLoader.load(cask_path("local-transmission"))
-      cached_location = Cask::Download.new(local_transmission, force: false, quarantine: false).perform
+      cached_location = Cask::Download.new(local_transmission).perform
 
       expect(cached_location).to be_quarantined
     end
 
+    it "quarantines Cask installs even if the fetch was not" do
+      Cask::Cmd::Fetch.run("local-transmission", "--no-quarantine")
+
+      Cask::Cmd::Install.run("local-transmission")
+
+      expect(
+        Cask::CaskLoader.load(cask_path("local-transmission")),
+      ).to be_installed
+
+      expect(Cask::Config.global.appdir.join("Transmission.app")).to be_quarantined
+    end
+
     it "quarantines dmg-based Casks" do
       Cask::Cmd::Install.run("container-dmg")
 
@@ -124,11 +136,32 @@
     it "does not quarantine Cask fetches" do
       Cask::Cmd::Fetch.run("local-transmission", "--no-quarantine")
       local_transmission = Cask::CaskLoader.load(cask_path("local-transmission"))
-      cached_location = Cask::Download.new(local_transmission, force: false, quarantine: false).perform
+      cached_location = Cask::Download.new(local_transmission).perform
 
       expect(cached_location).to_not be_quarantined
     end
 
+    it "does not quarantine Cask audits" do
+      Cask::Cmd::Audit.run("local-transmission", "--download", "--no-quarantine")
+
+      local_transmission = Cask::CaskLoader.load(cask_path("local-transmission"))
+      cached_location = Cask::Download.new(local_transmission).perform
+
+      expect(cached_location).to_not be_quarantined
+    end
+
+    it "does not quarantine Cask installs even if the fetch was" do
+      Cask::Cmd::Fetch.run("local-transmission")
+
+      Cask::Cmd::Install.run("local-transmission", "--no-quarantine")
+
+      expect(
+        Cask::CaskLoader.load(cask_path("local-transmission")),
+      ).to be_installed
+
+      expect(Cask::Config.global.appdir.join("Transmission.app")).to_not be_quarantined
+    end
+
     it "does not quarantine dmg-based Casks" do
       Cask::Cmd::Install.run("container-dmg", "--no-quarantine")
 
@@ -200,14 +233,5 @@
 
       expect(Cask::Config.global.appdir.join("MyNestedApp.app")).to_not be_quarantined
     end
-
-    it "does not quarantine Cask audits" do
-      Cask::Cmd::Audit.run("local-transmission", "--download", "--no-quarantine")
-
-      local_transmission = Cask::CaskLoader.load(cask_path("local-transmission"))
-      cached_location = Cask::Download.new(local_transmission, force: false, quarantine: false).perform
-
-      expect(cached_location).to_not be_quarantined
-    end
   end
 end