Support for opt-in network isolation in build/test sandboxes

[alebcay]

• Have you followed the guidelines in our Contributing document?
• Have you checked to ensure there aren't other open Pull Requests for the same change?
• Have you added an explanation of what your changes do and why you'd like us to include them?
• Have you written new tests for your changes? Here's an example.
• Have you successfully run brew style with your changes locally?
• Have you successfully run brew typecheck with your changes locally?
• Have you successfully run brew tests with your changes locally?

---

This is probably going to need a lot of iterating, but I think it's in a state that's ready for some eyes and feedback now.

Overview

This PR introduces opt-in (disabled by default) support for network isolation during build, test, and postinstall. A formula can opt-in like so:

class Hello < Formula
  url "[...]"
  sha256 "[...]"
  [...]
  # For enabling on a single phase:
  enable_offline_phase :build
  # For enabling on multiple phases:
  enable_offline_phase [:build, :postinstall]
  # Without arguments, enables on all phases:
  enable_offline_phase

  bottle do
    [...]
  end
  [...]

Suppose we opt-in a formula for offline build phase like so:

class Hello < Formula
  desc "Program providing model for GNU coding standards and practices"
  homepage "https://www.gnu.org/software/hello/"
  url "https://ftp.gnu.org/gnu/hello/hello-2.12.1.tar.gz"
  sha256 "8d99142afd92576f30b0cd7cb42a8dc6809998bc5d607d88761f512e26c7db20"
  license "GPL-3.0-or-later"
  enable_offline_phase :build
  [...]

and perhaps there's a pesky curl call in the install block:

def install
    system "curl", "example.org"
    ENV.append "LDFLAGS", "-liconv" if OS.mac?
[...]

When we attempt to install this formula:

==> curl example.org
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0curl: (6) Could not resolve host: example.org
[...]
Error: hello 2.12.1 did not build
Logs:
     /Users/caleb/Library/Logs/Homebrew/hello/00.options.out
     /Users/caleb/Library/Logs/Homebrew/hello/01.curl
READ THIS: https://docs.brew.sh/Troubleshooting

Things still to be (re-)considered

• I refer to "phases" here since it was the first term that came to mind. Any opinions on what to call "build", "test", "postinstall", etc.? This naming would be reflected in the method naming.
  • Open generally to feedback on the method/DSL naming.
• Going in a different direction, is the option to choose phases for which to enable this too granular? Maybe we just settle for one big on/off toggle. But then there's a lot of network-related tools/libraries that probably don't need connectivity to build but might need it for a meaningful test.
  • Compromise: combine :build and :postinstall into :build, while leaving :test as its own
  • Maybe we can open some exceptions for localhost in the sandbox, at least when running test block
• Running enable_offline_phase without arguments currently enables for all. Would it be less surprising for the no-arguments case to enable for build only? build + postinstall? Or maybe require an argument
  • Consider providing :all as an easy way to enable for everything, if we decide to require an argument

[alebcay]

An 'offline do' block for use when only part(s) of an 'install' (build,) 'postinstall,' or 'test do' block need network isolation;

For this point in particular, to achieve this would require more significant re-design of the formula installation/postinstall/test sandbox setup code. Currently, the sandbox is set up before the respective block is run, so the decision/logic to enable this has to be outside the install/test/postinstall blocks; trying to change it inside the block is too late since everything inside e.g. def install is already inside the sandbox.

An ':offline' 'Symbol' argument that formula authors can pass to Formula's 'install,' 'postinstall,' and 'test' methods before their block argument that causes the entire block to get evaluated as if all of its author-supplied contents were written inside of such an 'offline do' block

I personally like this idea but am not sure how it (def install :offline) could be achieved syntactically.

[MikeMcQuaid]

This PR introduces opt-in (disabled by default) support for network isolation during build, test, and postinstall. A formula can opt-in like so:

@alebcay AMAZING work here 👏🏻. Seen many attempts over the years and this seems by far the most likely to actually get adopted.

I refer to "phases" here since it was the first term that came to mind. Any opinions on what to call "build", "test", "postinstall", etc.? This naming would be reflected in the method naming.

• Open generally to feedback on the method/DSL naming.

Given there's only three: I wonder about just having them all be separate calls? I guess "methods" would work instead of "phases". I don't think we really describe these to users/contributors anywhere 🤔

• But then there's a lot of network-related tools/libraries that probably don't need connectivity to build but might need it for a meaningful test.

Yeh, I think different phases makes sense.

• Maybe we can open some exceptions for localhost in the sandbox, at least when running test block

This makes sense to me 👍🏻

---

The main question I have here isn't actually about the implementation but about formulae: how many formulae have you tried this on already? I'd be interested to see at least 3+ where it's working for (each of) build, postinstall, test before this gets merged.

Also I think it'd make sense pretty quickly to have some sort of environment variable to enable this without the DSL. It'd be great to be able to play around locally or with test-bot to find out which formulae can have this DSL used. We also probably want to e.g. require it unconditionally for new formulae at some point.

Relatedly, based on similar DSL addition work like this, it might be nice to have effectively a no-op reverse DSL e.g. disable_offline_phase. This might seem odd but: if this merges, eventually we'll end up flipping the default on formulae to disabling network access unless specifically enabled (and maybe in ~5-10 years: we remove the ability to disable here entirely).

This DSL would also make it easier to have a triage process (perhaps with the environment variable above) for formulae where:

• neither DSL used: formula hasn't been checked yet
• enable_* used: formula has been checked and offline works (for at least one method)
• disable_* used: formula has been checked and offline is broken (for at least one method)

Again: great work here and looking forward to seeing how this evolves! ❤️

[MikeMcQuaid]

@RandomDSdevel I appreciate the enthusiasm but I'm not sure your code review on PRs like this is helpful given your knowledge of Homebrew internals and Ruby, sorry ❤️

[carlocab]

Minor bikeshedding comment, but otherwise seems reasonable to me

[alebcay]

Thanks for the feedback everyone, made some adjustments:

• DSL is now {allow,deny}_network_access_for_{build,postinstall,test} - total 6 methods
• Added environment variables HOMEBREW_FORMULA_{BUILD,POSTINSTALL,TEST}_NETWORK. Set to allow or deny to force allow or deny, respectively; otherwise if unset will default to the value(s) specified via DSL in the formula(e). Open to further bikeshedding on the name of these env vars.

One thing I noticed - tests that use resources currently cannot run offline because, unlike formula build time, the resource fetch seems to be executed in the sandbox (vs before the sandbox).

Todo - consider opening up localhost in test sandbox when network access is denied.

---

Some formulae that already seem to be working offline - or not - with this. Might update with some more in the next day or two.

Formula	Build succeeds?	Postinstall succeeds?	Test succeeds?
cmake	✅	✅	✅
ca-certificates	✅	✅	✅
xz	✅	✅	✅
gh	❌1	✅	✅

¹ - go build needs network access to pull modules/dependencies

In the long term, I think there will be formulae (like gh above) that will never be able to build offline with our current processes/policies - we would have to do a lot of vendoring to control downloading stuff ourselves (kind of like we do for some Python formulae).

[MikeMcQuaid]

In the long term, I think there will be formulae (like gh above) that will never be able to build offline with our current processes/policies - we would have to do a lot of vendoring to control downloading stuff ourselves (kind of like we do for some Python formulae).

I don't think we should ever do that vendoring. I think better to make some DSL changes/additions. Most of these dependency download systems use checksummed and/or version pinned downloads here and have their own (used at comparable scale to Homebrew) security around this. Instead we should isolate what they do so they aren't able to e.g. arbitrarily write and perhaps restrict which hosts they can download from.

I think there's probably a place for a preinstall do or fetch do and/or pretest do that has different sandboxing:

• it can download arbitrary stuff off the internet
• it can only write to the sandboxed temporary directory
• maybe even: it can only read from the sandboxed temporary directory
• this temporary directory is then copied into the install directory to be used by an offline-only installation

To be clear: this should not block this PR being merged/DSL being added but will be worth thinking about as time goes on and we want to get 100% of homebrew-core formulae offline.

---

Alternatively, for tests: there was also talk in other issues about having resources assigned as "test resources" (much like how "normal" resources are implicitly "build resources") and then they could be fetched by brew fetch --test or similar. Again: not in this PR but an approach that will let us move forwards here.

[MikeMcQuaid]

That's the last feedback I have to add here.

@RandomDSdevel I asked you earlier so I'll be a little more firm here: until you're willing to step up contributions such that you're asking to be a Homebrew maintainer: please stop commenting on PRs, thanks.

[carlocab]

In the long term, I think there will be formulae (like gh above) that will never be able to build offline with our current processes/policies - we would have to do a lot of vendoring to control downloading stuff ourselves (kind of like we do for some Python formulae).

I don't think we should ever do that vendoring. I think better to make some DSL changes/additions.

Agreed. It would probably be better to run something like go mod vendor (perhaps automatically?) in the source directory before sandboxing happens or something like that. (Which admittedly may be complex, since I'm not sure whether source extraction happens inside or outside the sandbox).

[alebcay]

Made some adjustments:

• DSL is now reduced back to two methods, {allow,deny}_network_access, accepting a single symbol or array of symbols. If no argument is passed, the method applies to all phases.
• Added method deny_all_network_except_pipe to combine the current usages into one method.
• Added typing for the new methods in sandbox. It's a bit messy since I tried to limit changes to only the new methods, when it would probably be less jarring once some of the existing methods are also typed. I have another branch with more of those changes, if we would prefer to have that in a PR first and then add this.

[MikeMcQuaid]

Looking good @alebcay, almost ready to merge here! Might be worth a homebrew/core PR to modify a few formulae so we can see what the syntax might look like there.

[MikeMcQuaid]

One question and then should be good to go!

[MikeMcQuaid]

@alebcay Would love to see an example PR in homebrew-core!

[alebcay]

@alebcay Would love to see an example PR in homebrew-core!

Opened Homebrew/homebrew-core#169720. A few more things I thought of as I opened that:

• Do we need to enforce block/statement order for this? (If so I need to figure out how to write a cop for this). I intuitively, without any particular reason, inserted it before def install in this case but open to thoughts on if there's some place that we think makes more sense.
• Any considerations needed for this to work when building from source using API? Not something I have tested (and not quite sure how to test this).

[MikeMcQuaid]

• Do we need to enforce block/statement order for this? (If so I need to figure out how to write a cop for this).

There's already an existing cop so you should be able to add it to that 👍🏻

• inserted it before def install in this case

This makes sense to me.

• Any considerations needed for this to work when building from source using API?

Feels like it should be the same building from source/building bottles with/without API.

[MikeMcQuaid]

Thanks again @alebcay, great work here!