From 5b32049998ad491566154c037ce0d2d4b9273965 Mon Sep 17 00:00:00 2001
From: Chris Olszewski <chris.olszewski@vercel.com>
Date: Thu, 23 Mar 2023 16:31:59 -0700
Subject: [PATCH] fix: make turbo command regex stricter (#4325)

### Description

With #4223, we started checking and throwing on commands that look like
they invoke turbo. The regex is fairly permissive and would match any
command that contained a path that ended in `turbo` e.g. `rm ~/.turbo`
would get marked as invoking turbo.

This PR restricts the regex so it only matches commands that contain
`turbo` surrounded by whitespace. This means that we now won't throw if
the user directly invokes turbo via a path e.g.
`./node_modules/turbo/bin/turbo`. I think this is the correct tradeoff
as it's much more likely a user will invoke turbo via either `turbo foo`
or `pnpm turbo foo`.

Another possibility would be to attempt parse the scripts and do more
exact analysis, but to do this and correctly handle Windows would be
*fun*.

### Testing Instructions

See new unit tests
---
 cli/internal/graph/graph.go      |  2 +-
 cli/internal/graph/graph_test.go | 50 ++++++++++++++++++++++++++++++++
 2 files changed, 51 insertions(+), 1 deletion(-)
 create mode 100644 cli/internal/graph/graph_test.go

diff --git a/cli/internal/graph/graph.go b/cli/internal/graph/graph.go
index 7876f8567e52c..214df0f50f89d 100644
--- a/cli/internal/graph/graph.go
+++ b/cli/internal/graph/graph.go
@@ -238,7 +238,7 @@ func (g *CompleteGraph) getTaskGraphDescendants(taskGraph *dag.AcyclicGraph, tas
 	return stringDescendents, nil
 }
 
-var _isTurbo = regexp.MustCompile(fmt.Sprintf("(?:^|%v|\\s)turbo(?:$|\\s)", regexp.QuoteMeta(string(filepath.Separator))))
+var _isTurbo = regexp.MustCompile(`(?:^|\s)turbo(?:$|\s)`)
 
 func commandLooksLikeTurbo(command string) bool {
 	return _isTurbo.MatchString(command)
diff --git a/cli/internal/graph/graph_test.go b/cli/internal/graph/graph_test.go
new file mode 100644
index 0000000000000..9323e19465444
--- /dev/null
+++ b/cli/internal/graph/graph_test.go
@@ -0,0 +1,50 @@
+package graph
+
+import (
+	"testing"
+
+	"gotest.tools/v3/assert"
+)
+
+func Test_CommandsInvokingTurbo(t *testing.T) {
+	type testCase struct {
+		command string
+		match   bool
+	}
+	testCases := []testCase{
+		{
+			"turbo run foo",
+			true,
+		},
+		{
+			"rm -rf ~/Library/Caches/pnpm && turbo run foo && rm -rf ~/.npm",
+			true,
+		},
+		{
+			"FLAG=true turbo run foo",
+			true,
+		},
+		{
+			"npx turbo run foo",
+			true,
+		},
+		{
+			"echo starting; turbo foo; echo done",
+			true,
+		},
+		// We don't catch this as if people are going to try to invoke the turbo
+		// binary directly, they'll always be able to work around us.
+		{
+			"./node_modules/.bin/turbo foo",
+			false,
+		},
+		{
+			"rm -rf ~/Library/Caches/pnpm && rm -rf ~/Library/Caches/turbo && rm -rf ~/.npm && rm -rf ~/.pnpm-store && rm -rf ~/.turbo",
+			false,
+		},
+	}
+
+	for _, tc := range testCases {
+		assert.Equal(t, commandLooksLikeTurbo(tc.command), tc.match, tc.command)
+	}
+}