[engine] Support static env var pass-throughs

Allow users to selectively poke holes in the subprocess sandbox by
specifying `pass_through_env` in shac.textproto. `pass_through_env` is a
list of environment variables that should be passed through the sandbox,
along with bits indicating whether the variable's value represents a
file that should also be mounted and, if so, whether it should be
writeable by subprocesses.

We can immediately use this feature to grant Go checks in this
repository access to $HOME so they can share the same go cache with the
rest of the system (ideally we could use $GOCACHE, but it's not
guaranteed to be set, and if it's not set it's inferred from $HOME).
Same for tests that run `go run` since they can make use of the cache
instead of doing a full recompile on every test run; as a result, the
runtime of `go test ./internal/engine` went from ~4.2 seconds to ~1.2
seconds on my workstation.

This is a medium-term workaround until we support declaring
pass-throughs in Starlark, which would allow things like running `go env
GOCACHE` to obtain and pass through just the $GOCACHE directory, while
omitting the rest of $HOME.

Change-Id: I0bcc9956c4c4e2e9cca292925c66f6aed6f07524

README.md

@@ -48,6 +48,8 @@ Planned features/changes, in descending order by priority:
 * [ ] Built-in formatting of Starlark files
 * [ ] Configurable "pass-throughs" - non-default environment variables and
   mounts that can optionally be passed through to the sandbox
+  * [x] Passed-through environment variables statically declared in
+    shac.textproto
 * [ ] Add `glob` arguments to `ctx.scm.{all,affected}_files()` functions for
   easier filtering
 * [ ] Filesystem sandboxing on MacOS

checks/common.star

@@ -14,21 +14,27 @@
 
 def go_install(ctx, pkg, version):
     """Runs `go install`."""
-    tool_name = pkg.split("/")[-1]
-    env = go_env(ctx, tool_name)
+
+    env = go_env()
+
+    # TODO(olivernewman): Stop using a separate GOPATH for each tool, and instead
+    # install the tools sequentially. Multiple concurrent `go install` runs on the
+    # same GOPATH results in race conditions.
+    env["GOBIN"] = ctx.scm.root + "/.tools/gobin"
+
+    # TODO(olivernewman): Stop using a separate GOPATH for each tool, and instead
+    # install the tools sequentially. Multiple concurrent `go install` runs on the
+    # same GOPATH results in race conditions.
     ctx.os.exec(
         ["go", "install", "%s@%s" % (pkg, version)],
         allow_network = True,
         env = env,
     ).wait()
 
+    tool_name = pkg.split("/")[-1]
     return "%s/%s" % (env["GOBIN"], tool_name)
 
-def go_env(ctx, key):
-    # TODO(olivernewman): Stop using a separate GOPATH for each tool, and instead
-    # install the tools sequentially. Multiple concurrent `go install` runs on the
-    # same GOPATH results in race conditions.
-    gopath = "%s/.tools/gopath/%s" % (ctx.scm.root, key)
+def go_env():
     return {
         # Disable cgo as it's not necessary and not all development platforms have
         # the necessary headers.
@@ -38,11 +44,6 @@ def go_env(ctx, key):
             # to fail on some machines.
             "-buildvcs=false",
         ]),
-        "GOPATH": gopath,
-        "GOBIN": "%s/bin" % gopath,
-        # Cache within the directory to avoid writing to $HOME/.cache.
-        # TODO(olivernewman): Implement named caches.
-        "GOCACHE": "%s/.tools/gocache" % ctx.scm.root,
         # TODO(olivernewman): The default gopackagesdriver is broken within an
         # nsjail.
         "GOPACKAGESDRIVER": "off",

checks/go.star

@@ -52,26 +52,34 @@ def _gosec(ctx, version = "v2.15.0", level = "error"):
         be rolled from time to time.
       level: level at which issues should be emitted.
     """
+    affected_files = set(ctx.scm.affected_files())
     exe = go_install(ctx, "github.com/securego/gosec/v2/cmd/gosec", version)
     res = ctx.os.exec([exe, "-fmt=json", "-quiet", "-exclude=G304", "-exclude-dir=.tools", "./..."], raise_on_failure = False).wait()
     if res.retcode:
         # Schema is https://pkg.go.dev/github.com/securego/gosec/v2#ReportInfo
         d = json.decode(res.stdout)
         o = len(ctx.scm.root) + 1
-        for file, data in d["Golang errors"]:
-            ctx.emit.finding(
-                level = "error",
-                message = i["error"],
-                filepath = file[o:],
-                line = int(i["line"]),
-                col = int(i["column"]),
-            )
+        for file, errs in d["Golang errors"].items():
+            filepath = file[o:]
+            if filepath not in affected_files:
+                continue
+            for e in errs:
+                ctx.emit.finding(
+                    level = "error",
+                    message = e["error"],
+                    filepath = filepath,
+                    line = int(e["line"]),
+                    col = int(e["column"]),
+                )
         for i in d["Issues"]:
             line = i["line"].split("-")[0]
+            filepath = i["file"][o:]
+            if filepath not in affected_files:
+                continue
             ctx.emit.finding(
                 level = level,
                 message = i["rule_id"] + ": " + i["details"],
-                filepath = i["file"][o:],
+                filepath = filepath,
                 line = int(line),
                 col = int(i["column"]),
             )
@@ -91,7 +99,7 @@ def _ineffassign(ctx, version = "v0.0.0-20230107090616-13ace0543b28"):
     exe = go_install(ctx, "github.com/gordonklaus/ineffassign", version)
     res = ctx.os.exec(
         [exe, "./..."],
-        env = go_env(ctx, "ineffassign"),
+        env = go_env(),
         # ineffassign's README claims that it emits a retcode of 1 if it returns any
         # findings, but it actually emits a retcode of 3.
         # https://github.com/gordonklaus/ineffassign/blob/4cc7213b9bc8b868b2990c372f6fa057fa88b91c/ineffassign.go#L70
@@ -122,8 +130,7 @@ def _staticcheck(ctx, version = "v0.4.3"):
       will be rolled from time to time.
     """
     exe = go_install(ctx, "honnef.co/go/tools/cmd/staticcheck", version)
-    env = go_env(ctx, "staticcheck")
-    env["STATICCHECK_CACHE"] = env["GOCACHE"]
+    env = go_env()
     res = ctx.os.exec(
         [exe, "-f=json", "./..."],
         ok_retcodes = [0, 1],
@@ -172,7 +179,7 @@ def _shadow(ctx, version = "v0.7.0"):
             "-json",
             "./...",
         ],
-        env = go_env(ctx, "shadow"),
+        env = go_env(),
     ).wait()
 
     # Example output:
@@ -221,7 +228,7 @@ def no_fork_without_lock(ctx):
             "-json",
             "./...",
         ],
-        env = go_env(ctx, "no_fork_without_lock"),
+        env = go_env(),
     ).wait().stdout)
 
     # Skip the "execsupport" package since it contains the wrappers around Run()

internal/engine/run.go

@@ -311,18 +311,19 @@ func runInner(ctx context.Context, o *Options, tmpdir string) error {
 			scm = &subdirSCM{s: scm, subdir: normalized}
 		}
 		return &shacState{
-			allowNetwork: doc.AllowNetwork,
-			env:          &env,
-			filter:       o.Filter,
-			entryPoint:   entryPoint,
-			r:            o.Report,
-			root:         root,
-			sandbox:      sb,
-			scm:          scm,
-			subdir:       subdir,
-			tmpdir:       filepath.Join(tmpdir, strconv.Itoa(idx)),
-			writableRoot: doc.WritableRoot,
-			vars:         vars,
+			allowNetwork:   doc.AllowNetwork,
+			env:            &env,
+			filter:         o.Filter,
+			entryPoint:     entryPoint,
+			r:              o.Report,
+			root:           root,
+			sandbox:        sb,
+			scm:            scm,
+			subdir:         subdir,
+			tmpdir:         filepath.Join(tmpdir, strconv.Itoa(idx)),
+			writableRoot:   doc.WritableRoot,
+			vars:           vars,
+			passThroughEnv: doc.PassThroughEnv,
 		}
 	}
 	var shacStates []*shacState
@@ -542,7 +543,8 @@ type shacState struct {
 	// mutated. They run checks and emit results (results and comments).
 	checks []registeredCheck
 	// filter controls which checks run. If nil, all checks will run.
-	filter CheckFilter
+	filter         CheckFilter
+	passThroughEnv []*PassThroughEnv
 
 	// Set when fail() is called. This happens only during the first phase, thus
 	// no mutex is needed.

internal/engine/run_test.go

@@ -16,6 +16,7 @@ package engine
 
 import (
 	"context"
+	"crypto/md5"
 	"errors"
 	"fmt"
 	"io"
@@ -551,6 +552,89 @@ func TestRun_Vars(t *testing.T) {
 	}
 }
 
+func TestRun_PassThroughEnv(t *testing.T) {
+	hash := fmt.Sprintf("%x", md5.Sum([]byte(t.Name())))
+
+	varPrefix := "TEST_" + hash + "_"
+	nonFileVarname := varPrefix + "VAR"
+	readOnlyDirVarname := varPrefix + "RO_DIR"
+	writeableDirVarname := varPrefix + "WRITEABLE_DIR"
+	env := map[string]string{
+		nonFileVarname:      "this is not a file",
+		readOnlyDirVarname:  filepath.Join(t.TempDir(), "readonly"),
+		writeableDirVarname: filepath.Join(t.TempDir(), "writeable"),
+	}
+	mkdirAll(t, env[readOnlyDirVarname])
+	mkdirAll(t, env[writeableDirVarname])
+
+	for k, v := range env {
+		t.Setenv(k, v)
+	}
+
+	config := &Document{
+		PassThroughEnv: []*PassThroughEnv{
+			{
+				Name: nonFileVarname,
+			},
+			{
+				Name:   readOnlyDirVarname,
+				IsFile: true,
+			},
+			{
+				Name:      writeableDirVarname,
+				IsFile:    true,
+				Writeable: true,
+			},
+			{
+				// Additionally give access to HOME, which contains the
+				// Go cache, so checks that run `go run` can use cached
+				// artifacts.
+				Name:      "HOME",
+				IsFile:    true,
+				Writeable: true,
+			},
+		},
+		Vars: []*Var{{Name: "VAR_PREFIX"}},
+	}
+	root := t.TempDir()
+	writeFile(t, root, "shac.textproto", prototext.Format(config))
+
+	main := "ctx-os-exec-pass_through_env.star"
+	copyFile(t, root, filepath.Join("testdata", main))
+	copyFile(t, root, filepath.Join("testdata", "ctx-os-exec-pass_through_env.go"))
+
+	r := reportPrint{reportNoPrint: reportNoPrint{t: t}}
+	o := Options{
+		Report:     &r,
+		Dir:        root,
+		EntryPoint: main,
+		Vars:       map[string]string{"VAR_PREFIX": varPrefix},
+	}
+
+	const filesystemSandboxed = runtime.GOOS == "linux" && (runtime.GOARCH == "amd64" || runtime.GOARCH == "arm64")
+
+	wantLines := []string{
+		"[//ctx-os-exec-pass_through_env.star:25] non-file env var: this is not a file",
+		"read-only dir env var: " + env[readOnlyDirVarname],
+		"writeable dir env var: " + env[writeableDirVarname],
+		"able to write to writeable dir",
+	}
+	if filesystemSandboxed {
+		wantLines = append(wantLines, fmt.Sprintf(
+			"error writing to read-only dir: open %s: read-only file system",
+			filepath.Join(env[readOnlyDirVarname], "foo.txt")))
+	} else {
+		wantLines = append(wantLines, "able to write to read-only dir")
+	}
+
+	if err := Run(context.Background(), &o); err != nil {
+		t.Fatal(err)
+	}
+	if diff := cmp.Diff(strings.Join(wantLines, "\n")+"\n", r.b.String()); diff != "" {
+		t.Fatalf("mismatch (-want +got):\n%s", diff)
+	}
+}
+
 func TestRun_SCM_Raw(t *testing.T) {
 	t.Parallel()
 	root := t.TempDir()
@@ -1686,6 +1770,17 @@ func TestTestDataFailOrThrow(t *testing.T) {
 		i := i
 		t.Run(data[i].name, func(t *testing.T) {
 			t.Parallel()
+			writeFile(t, root, "shac.textproto", prototext.Format(&Document{
+				PassThroughEnv: []*PassThroughEnv{
+					{
+						// Give access to HOME, which contains the Go cache, so
+						// checks that run `go run` can use cached artifacts.
+						Name:      "HOME",
+						IsFile:    true,
+						Writeable: true,
+					},
+				},
+			}))
 			o := Options{Report: &reportNoPrint{t: t}, Dir: root, EntryPoint: data[i].name}
 			err := Run(context.Background(), &o)
 			if err == nil {
@@ -2096,6 +2191,17 @@ func TestTestDataPrint(t *testing.T) {
 		i := i
 		t.Run(data[i].name, func(t *testing.T) {
 			t.Parallel()
+			writeFile(t, p, "shac.textproto", prototext.Format(&Document{
+				PassThroughEnv: []*PassThroughEnv{
+					{
+						// Give access to HOME, which contains the Go cache, so
+						// checks that run `go run` can use cached artifacts.
+						Name:      "HOME",
+						IsFile:    true,
+						Writeable: true,
+					},
+				},
+			}))
 			testStarlarkPrint(t, p, data[i].name, false, data[i].ignoreOrder, data[i].want)
 		})
 	}

internal/engine/runtime_ctx_os.go

@@ -237,6 +237,22 @@ func ctxOsExec(ctx context.Context, s *shacState, name string, args starlark.Tup
 			env["PATH"],
 		}, string(os.PathListSeparator))
 	}
+
+	var passThroughMounts []sandbox.Mount
+	for _, pte := range s.passThroughEnv {
+		val, ok := os.LookupEnv(pte.Name)
+		if !ok {
+			continue
+		}
+		env[pte.Name] = val
+		if pte.IsFile {
+			passThroughMounts = append(passThroughMounts, sandbox.Mount{
+				Path:     val,
+				Writable: pte.Writeable,
+			})
+		}
+	}
+
 	for _, item := range argenv.Items() {
 		k, ok := item[0].(starlark.String)
 		if !ok {
@@ -342,6 +358,7 @@ func ctxOsExec(ctx context.Context, s *shacState, name string, args starlark.Tup
 			// this executable.
 			{Path: filepath.Dir(tempDir), Writable: true},
 		}
+		config.Mounts = append(config.Mounts, passThroughMounts...)
 
 		// TODO(olivernewman): This is necessary because checks for shac itself
 		// assume Go is pre-installed. Switch to a hermetic Go installation that