Prevent using symlinks to starlark files

Symlinks are frequently confusing / a source of bugs. They also don't
provide much utility over just loading the other file and re-exporting
its symbols, so recommend doing that instead.

Test: Presubmits
Change-Id: Ie3052ebc0add77f1746d6321fbdf7bc15df9819b

3 files changed, 37 insertions, 0 deletions

diff --git a/tools/rbcrun/host.go b/tools/rbcrun/host.go
index 7f5e3327c0..f36553e746 100644
--- a/tools/rbcrun/host.go
+++ b/tools/rbcrun/host.go
@@ -63,6 +63,14 @@ var sclBuiltins starlark.StringDict = starlark.StringDict{
 	"json": starlarkjson.Module,
 }
 
+func isSymlink(filepath string) (bool, error) {
+	if info, err := os.Lstat(filepath); err == nil {
+		return info.Mode() & os.ModeSymlink != 0, nil
+	} else {
+		return false, err
+	}
+}
+
 // Takes a module name (the first argument to the load() function) and returns the path
 // it's trying to load, stripping out leading //, and handling leading :s.
 func cleanModuleName(moduleName string, callerDir string, allowExternalPaths bool) (string, error) {
@@ -158,6 +166,13 @@ func loader(thread *starlark.Thread, module string) (starlark.StringDict, error)
 			if strings.HasSuffix(modulePath, ".scl") {
 				mode = ExecutionModeScl
 			}
+
+			if sym, err := isSymlink(modulePath); sym && err == nil {
+				return nil, fmt.Errorf("symlinks to starlark files are not allowed. Instead, load the target file and re-export its symbols: %s", modulePath)
+			} else if err != nil {
+				return nil, err
+			}
+
 			childThread := &starlark.Thread{Name: "exec " + module, Load: thread.Load}
 			// Cheating for the sake of testing:
 			// propagate starlarktest's Reporter key, otherwise testing
@@ -368,6 +383,12 @@ func Run(filename string, src interface{}, mode ExecutionMode, allowExternalEntr
 		return nil, nil, err
 	}
 
+	if sym, err := isSymlink(filename); sym && err == nil {
+		return nil, nil, fmt.Errorf("symlinks to starlark files are not allowed. Instead, load the target file and re-export its symbols: %s", filename)
+	} else if err != nil {
+		return nil, nil, err
+	}
+
 	// Add top-level file to cache for cycle detection purposes
 	moduleCache[filename] = nil
 
diff --git a/tools/rbcrun/host_test.go b/tools/rbcrun/host_test.go
index 468a620b51..7cfeb14463 100644
--- a/tools/rbcrun/host_test.go
+++ b/tools/rbcrun/host_test.go
@@ -186,6 +186,21 @@ func TestSclLoadsBzl(t *testing.T) {
 	}
 }
 
+func TestCantLoadSymlink(t *testing.T) {
+	moduleCache = make(map[string]*modentry)
+	dir := dataDir()
+	if err := os.Chdir(filepath.Dir(dir)); err != nil {
+		t.Fatal(err)
+	}
+	_, _, err := Run("testdata/test_scl_symlink.scl", nil, ExecutionModeScl, false)
+	if err == nil {
+		t.Fatal("Expected failure")
+	}
+	if !strings.Contains(err.Error(), "symlinks to starlark files are not allowed") {
+		t.Fatalf("Expected error to contain \"symlinks to starlark files are not allowed\": %q", err.Error())
+	}
+}
+
 func TestShell(t *testing.T) {
 	exerciseStarlarkTestFile(t, "testdata/shell.star")
 }
diff --git a/tools/rbcrun/testdata/test_scl_symlink.scl b/tools/rbcrun/testdata/test_scl_symlink.scl
new file mode 120000
index 0000000000..3f5aef488e
--- /dev/null
+++ b/tools/rbcrun/testdata/test_scl_symlink.scl
@@ -0,0 +1 @@
+test_scl.scl
\ No newline at end of file