[engine] Don't try to mount invalid PATH elements

Skip trying to mount any elements of $PATH that are not valid absolute paths. This makes shac resilient to weirdly/improperly configured environments. Change-Id: Ib6ba47798433c68c1b19e07e2afc49e172f4ee98 Reviewed-on: https://fuchsia-review.googlesource.com/c/shac-project/shac/+/926132 Commit-Queue: Auto-Submit <[email protected]> Fuchsia-Auto-Submit: Oliver Newman <[email protected]> Reviewed-by: Anthony Fandrianto <[email protected]>
shac-project · Oct 3, 2023 · 3cab7b9 · 3cab7b9
1 parent 34074c1
commit 3cab7b9
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 0 deletions.
diff --git a/internal/engine/run_test.go b/internal/engine/run_test.go
@@ -635,6 +635,36 @@ func TestRun_PassthroughEnv(t *testing.T) {
 	}
 }
 
+func TestRun_Exec_InvalidPATHElements(t *testing.T) {
+	root := t.TempDir()
+
+	writeFile(t, root, "file.txt", "")
+
+	invalidPathElements := []string{
+		filepath.Join(t.TempDir(), "does-not-exist"),
+		"relative/path",
+		"./dot/relative/path",
+		filepath.Join(root, "file.txt"),
+	}
+
+	t.Setenv("PATH", strings.Join(
+		append(invalidPathElements, os.Getenv("PATH")),
+		string(os.PathListSeparator)))
+
+	// No need for a complicated test case, just make sure that the subprocess
+	// launching succeeds. If shac doesn't handle invalid PATH elements
+	// correctly, launching any subprocess should fail, at least on platforms
+	// where filesystem sandboxing is supported.
+	writeFile(t, root, "shac.star", ""+
+		"def cb(ctx):\n"+
+		"    ctx.os.exec([\"true\"]).wait()\n"+
+		"    print(\"success!\")\n"+
+		"shac.register_check(cb)\n")
+
+	want := "[//shac.star:3] success!\n"
+	testStarlarkPrint(t, root, "shac.star", false, false, want)
+}
+
 func TestRun_SCM_Raw(t *testing.T) {
 	t.Parallel()
 	root := t.TempDir()

diff --git a/internal/engine/runtime_ctx_os.go b/internal/engine/runtime_ctx_os.go
@@ -371,8 +371,14 @@ func ctxOsExec(ctx context.Context, s *shacState, name string, args starlark.Tup
 		// Mount all directories listed in $PATH.
 		for _, p := range strings.Split(env["PATH"], string(os.PathListSeparator)) {
 			// $PATH may contain invalid elements. Filter them out.
+			if !filepath.IsAbs(p) {
+				// Relative paths in $PATH are not allowed.
+				continue
+			}
 			var fi os.FileInfo
 			if fi, err = os.Stat(p); err != nil || !fi.IsDir() {
+				// Skip $PATH elements that don't exist or point to
+				// non-directories.
 				continue
 			}
 			config.Mounts = append(config.Mounts, sandbox.Mount{Path: p})