Skip to content

Commit 5f59f92

Browse files
committed
Filter env during Ruby loading
- Hide token-like environment variables while formulae and casks are evaluated so top-level DSL code cannot read them. - Restore the caller environment after metadata loading and command parser requires so command execution keeps its normal environment. - Keep the filtering extension lightweight so command parser loading does not require slow startup dependencies. - Use `requires_ancestor` for the ENV module typing rather than `T.unsafe(self)`. - Cover formula, cask and external command require paths.
1 parent 03684f7 commit 5f59f92

12 files changed

Lines changed: 132 additions & 27 deletions

File tree

AGENTS.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,5 +58,6 @@ When running Ruby directly (e.g. `ruby -e ...`, `gem`, profiling tools), never u
5858
8. Prefer shelling out via `HOMEBREW_BREW_FILE` instead of requiring `cmd/` or `dev-cmd` when composing brew commands.
5959
9. Inline new or existing methods as methods or local variables unless they are reused 2+ times or needed for unit tests.
6060
10. Avoid `T.must`; prefer explicit nil checks or APIs that return non-nil values.
61-
11. Keep `extend/os/*` prepends as thin as possible; put the `prepend` in the OS-specific `linux` or `macos` file rather than the shared `extend/os/*` loader with an inline `if`, and prefer putting substantive logic in shared code outside `extend/` when practical so it can be tested on all platforms instead of relying on `:needs_linux` or `:needs_macos` specs.
62-
12. When Bash logic mirrors Ruby logic, keep both implementations in sync and add two-way comments naming the matching Ruby and Bash locations; keep matching helper filenames aligned where practical.
61+
11. Avoid `T.unsafe(self)` whenever possible; prefer `requires_ancestor` or similar typed module patterns.
62+
12. Keep `extend/os/*` prepends as thin as possible; put the `prepend` in the OS-specific `linux` or `macos` file rather than the shared `extend/os/*` loader with an inline `if`, and prefer putting substantive logic in shared code outside `extend/` when practical so it can be tested on all platforms instead of relying on `:needs_linux` or `:needs_macos` specs.
63+
13. When Bash logic mirrors Ruby logic, keep both implementations in sync and add two-way comments naming the matching Ruby and Bash locations; keep matching helper filenames aligned where practical.

Library/Homebrew/cask/cask_loader.rb

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
require "utils/output"
99
require "utils/path"
1010
require "extend/hash/keys"
11+
require "extend/ENV/sensitive"
1112
require "api"
1213

1314
module Cask
@@ -102,7 +103,9 @@ def initialize(content, tap: T.unsafe(nil))
102103
def load(config:)
103104
@config = config
104105

105-
instance_eval(content, __FILE__, __LINE__)
106+
ENV.clear_sensitive_environment! do
107+
instance_eval(content, __FILE__, __LINE__)
108+
end
106109
end
107110
end
108111

@@ -187,8 +190,10 @@ def load(config:)
187190
end
188191

189192
begin
190-
instance_eval(content, path.to_s).tap do |cask|
191-
raise CaskUnreadableError.new(token, "'#{path}' does not contain a cask.") unless cask.is_a?(Cask)
193+
ENV.clear_sensitive_environment! do
194+
instance_eval(content, path.to_s).tap do |cask|
195+
raise CaskUnreadableError.new(token, "'#{path}' does not contain a cask.") unless cask.is_a?(Cask)
196+
end
192197
end
193198
rescue NameError, ArgumentError, ScriptError => e
194199
error = CaskUnreadableError.new(token, e.message)

Library/Homebrew/cli/parser.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
require "cli/args"
88
require "cli/error"
99
require "commands"
10+
require "extend/ENV/sensitive"
1011
require "optparse"
1112
require "utils/tty"
1213
require "utils/formatter"
@@ -57,7 +58,7 @@ def self.from_cmd_path(cmd_path)
5758
cmd_name = cmd_args_method_name.to_s.delete_suffix("_args").tr("_", "-")
5859

5960
begin
60-
if Homebrew.require?(cmd_path)
61+
if ENV.clear_sensitive_environment! { Homebrew.require?(cmd_path) }
6162
cmd = Homebrew::AbstractCommand.command(cmd_name)
6263
if cmd
6364
cmd.parser

Library/Homebrew/commands.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
require "homebrew"
55
require "cli/parser"
6+
require "extend/ENV/sensitive"
67

78
# Helper functions for commands.
89
module Commands
@@ -88,7 +89,7 @@ def self.internal_dev_cmd_path(cmd)
8889
sig { params(cmd: String).returns(T.nilable(Pathname)) }
8990
def self.external_ruby_v2_cmd_path(cmd)
9091
path = which("#{cmd}.rb", tap_cmd_directories)
91-
path if Homebrew.require?(path)
92+
path if ENV.clear_sensitive_environment! { Homebrew.require?(path) }
9293
end
9394

9495
# Ruby commands which are run by being `require`d.

Library/Homebrew/extend/ENV.rb

Lines changed: 3 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
require "hardware"
55
require "diagnostic"
6+
require "extend/ENV/sensitive"
67
require "extend/ENV/shared"
78
require "extend/ENV/std"
89
require "extend/ENV/super"
@@ -18,6 +19,8 @@
1819
# <!-- vale on -->
1920

2021
module EnvActivation
22+
include EnvSensitive
23+
2124
sig { params(env: T.nilable(String)).void }
2225
def activate_extensions!(env: nil)
2326
if superenv?(env)
@@ -52,21 +55,6 @@ def with_build_environment(env: nil, cc: nil, build_bottle: false, bottle_arch:
5255
replace(old_env)
5356
end
5457
end
55-
56-
sig { params(key: T.any(String, Symbol)).returns(T::Boolean) }
57-
def sensitive?(key)
58-
key.match?(/(cookie|key|token|password|passphrase|auth)/i)
59-
end
60-
61-
sig { returns(T::Hash[String, String]) }
62-
def sensitive_environment
63-
select { |key, _| sensitive?(key) }
64-
end
65-
66-
sig { void }
67-
def clear_sensitive_environment!
68-
each_key { |key| delete key if sensitive?(key) }
69-
end
7058
end
7159

7260
ENV.extend(EnvActivation)

Library/Homebrew/extend/ENV.rbi

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
# typed: strict
22

3+
module EnvSensitive; end
4+
35
module EnvActivation
6+
include EnvSensitive
47
include SharedEnvExtension
58
end
69

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
# typed: strict
2+
# frozen_string_literal: true
3+
4+
module EnvSensitive
5+
extend T::Helpers
6+
7+
requires_ancestor { Sorbet::Private::Static::ENVClass }
8+
9+
sig { params(key: T.any(String, Symbol)).returns(T::Boolean) }
10+
def sensitive?(key)
11+
key.match?(/(cookie|key|token|password|passphrase|auth)/i)
12+
end
13+
14+
sig { returns(T::Hash[String, String]) }
15+
def sensitive_environment
16+
select { |key, _| sensitive?(key) }
17+
end
18+
19+
sig { params(block: T.nilable(T.proc.returns(T.untyped))).returns(T.untyped) }
20+
def clear_sensitive_environment!(&block)
21+
unless block
22+
each_key { |key| delete key if sensitive?(key) }
23+
return
24+
end
25+
26+
old_env = to_hash.dup
27+
begin
28+
clear_sensitive_environment!
29+
yield
30+
ensure
31+
replace(old_env)
32+
end
33+
end
34+
end
35+
36+
ENV.extend(EnvSensitive)

Library/Homebrew/formulary.rb

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
require "utils/curl"
1414
require "extend/hash/deep_transform_values"
1515
require "extend/hash/keys"
16+
require "extend/ENV/sensitive"
1617
require "tap"
1718

1819
# The {Formulary} is responsible for creating instances of {Formula}.
@@ -151,10 +152,12 @@ def self.load_formula(name, path, contents, namespace, flags:, ignore_errors:)
151152
raise FormulaUnreadableError.new(name, e)
152153
end
153154
end
154-
if ignore_errors
155-
Ignorable.hook_raise(&eval_formula)
156-
else
157-
eval_formula.call
155+
ENV.clear_sensitive_environment! do
156+
if ignore_errors
157+
Ignorable.hook_raise(&eval_formula)
158+
else
159+
eval_formula.call
160+
end
158161
end
159162

160163
class_name = class_s(name)

Library/Homebrew/test/ENV_spec.rb

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,23 @@
164164
subject.clear_sensitive_environment!
165165
expect(subject["FOO"]).to eq "bar"
166166
end
167+
168+
it "restores the environment after yielding" do
169+
subject["SECRET_TOKEN"] = "password"
170+
subject["FOO"] = "bar"
171+
172+
result = subject.clear_sensitive_environment! do
173+
subject["FOO"] = "baz"
174+
subject["OTHER_TOKEN"] = "secret"
175+
176+
[subject["SECRET_TOKEN"], subject["FOO"]]
177+
end
178+
179+
expect(result).to eq([nil, "baz"])
180+
expect(subject["SECRET_TOKEN"]).to eq("password")
181+
expect(subject["FOO"]).to eq("bar")
182+
expect(subject).not_to include("OTHER_TOKEN")
183+
end
167184
end
168185
end
169186

Library/Homebrew/test/cask/cask_loader_spec.rb

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,31 @@
252252
end
253253

254254
describe "FromPathLoader" do
255+
it "clears sensitive environment variables while evaluating casks" do
256+
cask_token = "sensitive-env"
257+
cask_file = mktmpdir/"#{cask_token}.rb"
258+
cask_file.write <<~RUBY
259+
cask "#{cask_token}" do
260+
version "1.0.0"
261+
sha256 "1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef"
262+
263+
url "https://example.com/app.dmg"
264+
name "Sensitive Env"
265+
desc ENV.key?("SECRET_TOKEN") ? "Secret present" : "Secret absent"
266+
homepage "https://example.com"
267+
268+
app "App.app"
269+
end
270+
RUBY
271+
272+
with_env(SECRET_TOKEN: "password") do
273+
cask = Cask::CaskLoader::FromPathLoader.new(cask_file).load(config: nil)
274+
275+
expect(cask.desc).to eq("Secret absent")
276+
expect(ENV.fetch("SECRET_TOKEN", nil)).to eq("password")
277+
end
278+
end
279+
255280
describe "loading a cask with a removed DSL method" do
256281
let(:tmpdir) { mktmpdir }
257282
let(:cask_token) { "removed-method-cask" }

0 commit comments

Comments
 (0)