Skip to content

Commit bc6d9cb

Browse files
committed
Sandbox cask completions
- Avoid unsandboxed upstream code when generating cask completions. - Keep installer scripts on `SystemCommand` because most run installers. - Document why cask installers and packages are not sandboxed. - Note that flight blocks are expected to be sandboxed later. - Keep `HOMEBREW_NO_SANDBOX_CASK` as a temporary escape hatch. - Preserve completion output by discarding stderr unless requested.
1 parent 4d7be53 commit bc6d9cb

8 files changed

Lines changed: 195 additions & 15 deletions

File tree

Library/Homebrew/cask/artifact/abstract_artifact.rb

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@
22
# frozen_string_literal: true
33

44
require "extend/object/deep_dup"
5+
require "env_config"
6+
require "sandbox"
7+
require "tempfile"
58
require "utils/output"
69

710
module Cask
@@ -19,7 +22,6 @@ class AbstractArtifact
1922
# T.anything or the union of all possible argument types would be better choice, but it's convenient to be
2023
# able to invoke `.inspect`, `.to_s`, etc. without the overhead of type guards.
2124
DirectivesType = T.type_alias { Object }
22-
2325
sig { overridable.returns(String) }
2426
def self.english_name
2527
@english_name ||= T.let(T.must(name).sub(/^.*:/, "").gsub(/(.)([A-Z])/, '\1 \2'), T.nilable(String))
@@ -183,6 +185,55 @@ def config
183185
cask.config
184186
end
185187

188+
sig { returns(T.nilable(Sandbox)) }
189+
def cask_sandbox
190+
return if Homebrew::EnvConfig.no_sandbox_cask?
191+
192+
Sandbox.ensure_sandbox_installed!
193+
return unless Sandbox.available?
194+
195+
Sandbox.new.tap do |sandbox|
196+
sandbox.allow_read(path: cask.staged_path, type: :subpath)
197+
sandbox.allow_write_temp_and_cache
198+
sandbox.deny_all_network
199+
end
200+
end
201+
202+
sig {
203+
params(
204+
env: T::Hash[String, T.any(String, T::Boolean, PATH)],
205+
args: T::Array[T.any(String, Pathname)],
206+
).returns(T::Array[T.any(String, Pathname)])
207+
}
208+
def cask_sandbox_command(env, args)
209+
["/usr/bin/env", *env.map { |key, value| "#{key}=#{value}" }, *args]
210+
end
211+
212+
sig {
213+
params(
214+
sandbox: Sandbox,
215+
args: T::Array[T.any(String, Pathname)],
216+
input: T.any(String, T::Array[String]),
217+
).void
218+
}
219+
def run_cask_sandbox(sandbox, args, input: [])
220+
return sandbox.run(*args) if Array(input).empty?
221+
222+
Tempfile.create("homebrew-cask-script-input", HOMEBREW_TEMP) do |input_file|
223+
input_file.write(Array(input).join)
224+
input_file.close
225+
sandbox.allow_read(path: input_file.path)
226+
sandbox.run(
227+
"/bin/sh",
228+
"-c",
229+
"input=$1; shift; exec \"$@\" < \"$input\"",
230+
"sh",
231+
input_file.path,
232+
*args,
233+
)
234+
end
235+
end
236+
186237
sig { returns(String) }
187238
def to_s
188239
"#{summarize} (#{self.class.english_name})"

Library/Homebrew/cask/artifact/generated_completion.rb

Lines changed: 34 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
require "cask/artifact/fishcompletion"
77
require "cask/artifact/zshcompletion"
88
require "extend/hash/keys"
9+
require "tempfile"
910
require "utils/shell_completion"
1011

1112
module Cask
@@ -100,11 +101,7 @@ def install_phase(**_options)
100101

101102
script_path = completion_script_path(shell)
102103
script_path.dirname.mkpath
103-
script_path.write(::Utils::ShellCompletion.generate_completion_output(
104-
[executable, *commands[1..]],
105-
shell_parameter,
106-
popen_read_env,
107-
))
104+
script_path.write(generate_completion_output([executable, *commands[1..]], shell_parameter, popen_read_env))
108105
rescue => e
109106
opoo "Failed to generate #{shell} completions from #{executable}: #{e}"
110107
end
@@ -124,6 +121,38 @@ def uninstall_phase(command: SystemCommand, **_options)
124121

125122
private
126123

124+
sig {
125+
params(
126+
completion_commands: T::Array[T.any(Pathname, String)],
127+
shell_parameter: T.nilable(T.any(String, T::Array[String])),
128+
env: T::Hash[String, String],
129+
).returns(String)
130+
}
131+
def generate_completion_output(completion_commands, shell_parameter, env)
132+
sandbox = cask_sandbox
133+
unless sandbox
134+
return ::Utils::ShellCompletion.generate_completion_output(completion_commands, shell_parameter,
135+
env)
136+
end
137+
138+
Tempfile.create("homebrew-cask-completions", HOMEBREW_TEMP) do |output|
139+
sandbox.run(
140+
*cask_sandbox_command(
141+
env,
142+
[
143+
"/bin/sh",
144+
"-c",
145+
"output=$1; shift; exec \"$@\" > \"$output\"#{" 2>/dev/null" unless ENV["HOMEBREW_STDERR"]}",
146+
"sh",
147+
output.path,
148+
*(completion_commands + Array(shell_parameter)),
149+
],
150+
),
151+
)
152+
output.read
153+
end
154+
end
155+
127156
sig { returns(String) }
128157
def resolved_base_name
129158
@resolved_base_name ||= T.let(begin

Library/Homebrew/env_config.rb

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -474,6 +474,11 @@ module EnvConfig
474474
"shadowed by other commands earlier on `$PATH`.",
475475
boolean: true,
476476
},
477+
HOMEBREW_NO_SANDBOX_CASK: {
478+
# odeprecated: make cask executable sandboxing mandatory in a future release.
479+
description: "If set, disable sandboxing for cask artifacts that generate files by running executables.",
480+
boolean: true,
481+
},
477482
HOMEBREW_NO_SANDBOX_LINUX: {
478483
description: "If set, disable the Linux sandbox.",
479484
boolean: true,

Library/Homebrew/sorbet/rbi/dsl/homebrew/env_config.rbi

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Library/Homebrew/test/cask/artifact/generated_completion_spec.rb

Lines changed: 69 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,16 @@
3535
it "generates completion scripts for default shells" do
3636
artifact = cask.artifacts.grep(klass).first
3737

38-
allow(Utils).to receive(:safe_popen_read) do |env, *_args, **_opts|
39-
"#{env.fetch("SHELL")} completion output"
38+
allow(Sandbox).to receive_messages(ensure_sandbox_installed!: nil, available?: true)
39+
allow(Sandbox).to receive(:new) do
40+
instance_double(Sandbox).tap do |sandbox|
41+
allow(sandbox).to receive(:allow_read)
42+
allow(sandbox).to receive(:allow_write_temp_and_cache)
43+
allow(sandbox).to receive(:deny_all_network)
44+
allow(sandbox).to receive(:run) do |*args|
45+
Pathname(args.fetch(6)).write("#{args.fetch(1).delete_prefix("SHELL=")} completion output")
46+
end
47+
end
4048
end
4149

4250
artifact.install_phase
@@ -49,14 +57,57 @@
4957
expect((fish_dir/"foo.fish").read).to eq("fish completion output")
5058
end
5159

60+
it "sandboxes completion generation" do
61+
artifact = cask.artifacts.grep(klass).first
62+
sandboxes = []
63+
64+
allow(Sandbox).to receive_messages(ensure_sandbox_installed!: nil, available?: true)
65+
allow(Sandbox).to receive(:new) do
66+
instance_double(Sandbox).tap do |sandbox|
67+
expect(sandbox).to receive(:allow_read).with(path: staged_path, type: :subpath)
68+
expect(sandbox).to receive(:allow_write_temp_and_cache)
69+
expect(sandbox).to receive(:deny_all_network)
70+
allow(sandbox).to receive(:run) { |*args| Pathname(args.fetch(6)).write("completion") }
71+
sandboxes << sandbox
72+
end
73+
end
74+
75+
artifact.install_phase
76+
77+
expect(sandboxes.length).to eq(3)
78+
end
79+
80+
it "does not sandbox when HOMEBREW_NO_SANDBOX_CASK is set" do
81+
artifact = cask.artifacts.grep(klass).first
82+
83+
ENV["HOMEBREW_NO_SANDBOX_CASK"] = "1"
84+
allow(Sandbox).to receive(:available?).and_return(true)
85+
allow(Utils).to receive(:safe_popen_read) { |env, *_args, **_opts| "#{env.fetch("SHELL")} completion" }
86+
expect(Sandbox).not_to receive(:new)
87+
88+
artifact.install_phase
89+
90+
expect((bash_dir/"foo").read).to eq("bash completion")
91+
ensure
92+
ENV["HOMEBREW_NO_SANDBOX_CASK"] = nil
93+
end
94+
5295
context "when generation fails for one shell" do
5396
it "warns and continues generating other shells" do
5497
artifact = cask.artifacts.grep(klass).first
5598

56-
allow(Utils).to receive(:safe_popen_read) do |env, *_args, **_opts|
57-
raise "boom" if env.fetch("SHELL") == "bash"
58-
59-
"zsh completion"
99+
allow(Sandbox).to receive_messages(ensure_sandbox_installed!: nil, available?: true)
100+
allow(Sandbox).to receive(:new) do
101+
instance_double(Sandbox).tap do |sandbox|
102+
allow(sandbox).to receive(:allow_read)
103+
allow(sandbox).to receive(:allow_write_temp_and_cache)
104+
allow(sandbox).to receive(:deny_all_network)
105+
allow(sandbox).to receive(:run) do |*args|
106+
raise "boom" if args.fetch(1) == "SHELL=bash"
107+
108+
Pathname(args.fetch(6)).write("zsh completion")
109+
end
110+
end
60111
end
61112

62113
expect { artifact.install_phase }
@@ -103,14 +154,23 @@
103154
artifact = cask.artifacts.grep(klass).first
104155
captured_args = nil
105156

106-
allow(Utils).to receive(:safe_popen_read) do |_env, *args, **_opts|
107-
captured_args = args
108-
"zsh completion"
157+
allow(Sandbox).to receive_messages(ensure_sandbox_installed!: nil, available?: true)
158+
allow(Sandbox).to receive(:new) do
159+
instance_double(Sandbox).tap do |sandbox|
160+
allow(sandbox).to receive(:allow_read)
161+
allow(sandbox).to receive(:allow_write_temp_and_cache)
162+
allow(sandbox).to receive(:deny_all_network)
163+
allow(sandbox).to receive(:run) do |*args|
164+
captured_args = args
165+
Pathname(args.fetch(6)).write("zsh completion")
166+
end
167+
end
109168
end
110169

111170
artifact.install_phase
112171

113172
expect(captured_args).to include("--shell=zsh")
173+
expect(captured_args.fetch(4)).to end_with(" 2>/dev/null")
114174
expect(zsh_dir/"_bar").to be_a_file
115175
expect(bash_dir/"bar").not_to exist
116176
expect(fish_dir/"bar.fish").not_to exist

Library/Homebrew/test/cask/artifact/installer_spec.rb

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,14 @@
3939

4040
installer.install_phase(command:)
4141
end
42+
43+
it "does not sandbox the executable" do
44+
allow(Sandbox).to receive(:available?).and_return(true)
45+
expect(Sandbox).not_to receive(:new)
46+
expect(command).to receive(:run!)
47+
48+
installer.install_phase(command:)
49+
end
4250
end
4351
end
4452
end

Library/Homebrew/test/env_config_spec.rb

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,15 @@
179179
end
180180
end
181181

182+
describe ".no_sandbox_cask?" do
183+
it "returns true if HOMEBREW_NO_SANDBOX_CASK is set" do
184+
ENV["HOMEBREW_NO_SANDBOX_CASK"] = "1"
185+
expect(env_config.no_sandbox_cask?).to be(true)
186+
ensure
187+
ENV["HOMEBREW_NO_SANDBOX_CASK"] = nil
188+
end
189+
end
190+
182191
describe ".use_internal_api?" do
183192
it "returns true if HOMEBREW_USE_INTERNAL_API is set" do
184193
ENV["HOMEBREW_USE_INTERNAL_API"] = "1"

docs/Cask-Cookbook.md

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,7 @@ Each cask must declare one or more [artifacts](/rubydoc/Cask/Artifact.html) (i.e
142142
| `bash_completion` | yes | Relative path to a Bash completion file that should be linked into the `$(brew --prefix)/etc/bash_completion.d` folder on installation. |
143143
| `fish_completion` | yes | Relative path to a fish completion file that should be linked into the `$(brew --prefix)/share/fish/vendor_completions.d` folder on installation. |
144144
| `zsh_completion` | yes | Relative path to a Zsh completion file that should be linked into the `$(brew --prefix)/share/zsh/site-functions` folder on installation. |
145+
| `generate_completions_from_executable` | yes | Command and arguments used to generate shell completions from an executable at installation time. |
145146
| `colorpicker` | yes | Relative path to a ColorPicker plugin that should be moved into the `~/Library/ColorPickers` folder on installation. |
146147
| `dictionary` | yes | Relative path to a Dictionary that should be moved into the `~/Library/Dictionaries` folder on installation. |
147148
| `font` | yes | Relative path to a Font that should be moved into the `~/Library/Fonts` folder on installation. |
@@ -158,6 +159,16 @@ Each cask must declare one or more [artifacts](/rubydoc/Cask/Artifact.html) (i.e
158159
| `artifact` | yes | Relative path to an arbitrary path that should be moved on installation. Must provide an absolute path as a `target`. (Example: [free-gpgmail.rb](https://github.com/Homebrew/homebrew-cask/blob/b3c438d608d9702380edf10d5495e0727cf17108/Casks/f/free-gpgmail.rb#L44)) This is only for unusual cases; the `app` stanza is strongly preferred when moving `.app` bundles. |
159160
| `stage_only` | no | `true`. Asserts that the cask contains no activatable artifacts. |
160161

162+
### Cask artifact trust and sandboxing
163+
164+
Homebrew treats cask installation artifacts as trusted vendor installation actions once the cask has been accepted. Artifact stanzas such as [`app`](#stanza-app), [`pkg`](#stanza-pkg) and [`installer script`](#installer-script) are expected to install software and may write outside the Caskroom through Homebrew-managed moves, macOS installer services or vendor installer code.
165+
166+
Generated completion artifacts are different: `generate_completions_from_executable` runs an installed executable only to produce shell completion text. That execution is sandboxed where Homebrew has an available sandbox. The sandbox allows reading the staged cask, writing temporary/cache files and blocks network access. This limits side effects from commands that should only print completion data.
167+
168+
`installer script:` is not sandboxed. Many installer scripts are vendor installers that require broad filesystem writes, macOS services or `sudo`; macOS sandboxing does not work for root processes, and narrowing the write allowlist to the Caskroom plus uninstall or zap paths would break installers that legitimately write elsewhere. It would also change documented `SystemCommand` behaviours such as `sudo:`, `must_succeed:` and output handling.
169+
170+
`pkg` artifacts are not run in the cask sandbox either. They are installed by macOS `/usr/sbin/installer`, which applies package payloads, scripts and receipts according to the package metadata.
171+
161172
### Optional stanzas
162173

163174
| name | multiple occurrences allowed? | value |
@@ -544,6 +555,8 @@ Refer to [Deprecating, Disabling and Removing](Deprecating-Disabling-and-Removin
544555

545556
The stanzas `preflight`, `postflight`, `uninstall_preflight`, and `uninstall_postflight` define operations to be run before or after installation or uninstallation.
546557

558+
Flight blocks are not currently run in the cask sandbox. They should be written as though they may be sandboxed in the future: prefer the mini-DSL helpers below and keep filesystem writes limited to paths owned by the cask.
559+
547560
#### Evaluation of blocks is always deferred
548561

549562
The Ruby blocks defined by these stanzas are not evaluated until install time or uninstall time. Within a block you may refer to the `@cask` instance variable, and invoke [any method available on `@cask`](/rubydoc/Cask/Cask.html).
@@ -589,6 +602,8 @@ installer manual: "RubyMotion Installer.app"
589602
| `print_stdout:` | set to `false` to suppress `stdout` output |
590603
| `sudo:` | set to `true` if the script needs *sudo* |
591604

605+
Installer scripts run without the cask sandbox. Use them only when the vendor provides an installer command that cannot be represented by a more specific artifact stanza such as [`app`](#stanza-app) or [`pkg`](#stanza-pkg).
606+
592607
The path may be absolute, or relative to the cask. Example (from [miniforge.rb](https://github.com/Homebrew/homebrew-cask/blob/864f623e2cd17dbde5987a7b3923fdb0b4ac9ee5/Casks/m/miniforge.rb#L23-L26)):
593608

594609
```ruby

0 commit comments

Comments
 (0)