fix: prevent session token exfiltration via external app URLs (#26146… · coder/coder@91d1865
11package cli_test
2233import (
4+"bytes"
45"context"
6+"database/sql"
57"net/url"
68"os"
79"path"
@@ -21,6 +23,9 @@ import (
2123"github.com/coder/coder/v2/agent/agenttest"
2224"github.com/coder/coder/v2/cli/clitest"
2325"github.com/coder/coder/v2/coderd/coderdtest"
26+"github.com/coder/coder/v2/coderd/database"
27+"github.com/coder/coder/v2/coderd/database/dbfake"
28+"github.com/coder/coder/v2/coderd/database/dbgen"
2429"github.com/coder/coder/v2/coderd/database/dbtime"
2530"github.com/coder/coder/v2/codersdk"
2631"github.com/coder/coder/v2/provisionersdk/proto"
@@ -654,14 +659,16 @@ func TestOpenApp(t *testing.T) {
654659w.RequireContains("region not found")
655660 })
656661657-t.Run("ExternalAppSessionToken", func(t *testing.T) {
662+t.Run("ExternalAppOnTopLevelAgentSubstitutes", func(t *testing.T) {
658663t.Parallel()
659664665+// Apps on the top-level (template-defined) agent are trusted, so the
666+// CLI substitutes $SESSION_TOKEN regardless of scheme.
660667client, ws, _ := setupWorkspaceForAgent(t, func(agents []*proto.Agent) []*proto.Agent {
661668agents[0].Apps = []*proto.App{
662669 {
663670Slug: "app1",
664-Url: "https://example.com/app1?token=$SESSION_TOKEN",
671+Url: "vscode://coder.coder-remote/open?token=$SESSION_TOKEN",
665672External: true,
666673 },
667674 }
@@ -678,4 +685,92 @@ func TestOpenApp(t *testing.T) {
678685w.RequireContains("test.open-error")
679686w.RequireContains(client.SessionToken())
680687 })
688+689+t.Run("ExternalAppOnSubAgentWithPlaceholderPrintsURLAndDoesNotOpen", func(t *testing.T) {
690+t.Parallel()
691+692+// Sub-agent app URLs are attacker-influenceable through workspace
693+// configuration and runtime registration. The CLI must not
694+// substitute the session token, and must not hand the URL to the
695+// OS open handler. The URL is printed to stdout so a user who
696+// trusts the source can substitute and open it manually.
697+ownerClient, store := coderdtest.NewWithDatabase(t, nil)
698+ownerClient.SetLogger(testutil.Logger(t).Named("client"))
699+first := coderdtest.CreateFirstUser(t, ownerClient)
700+userClient, user := coderdtest.CreateAnotherUserMutators(t, ownerClient, first.OrganizationID, nil, func(r *codersdk.CreateUserRequestWithOrgs) {
701+r.Username = "subagentowner"
702+ })
703+r := dbfake.WorkspaceBuild(t, store, database.WorkspaceTable{
704+Name: "subagentws",
705+OrganizationID: first.OrganizationID,
706+OwnerID: user.ID,
707+ }).WithAgent().Do()
708+709+require.NotEmpty(t, r.Agents, "expected at least one workspace agent")
710+mainAgent := r.Agents[0]
711+712+subAgent := dbgen.WorkspaceSubAgent(t, store, mainAgent, database.WorkspaceAgent{
713+Name: "devcontainer",
714+ })
715+_ = dbgen.WorkspaceApp(t, store, database.WorkspaceApp{
716+AgentID: subAgent.ID,
717+Slug: "subapp",
718+External: true,
719+Url: sql.NullString{Valid: true, String: "vscode://coder.coder-remote/open?token=$SESSION_TOKEN"},
720+ })
721+722+inv, root := clitest.New(t, "open", "app", r.Workspace.Name+".devcontainer", "subapp", "--test.open-error")
723+clitest.SetupConfig(t, userClient, root)
724+var stdout, stderr bytes.Buffer
725+inv.Stdout = &stdout
726+inv.Stderr = &stderr
727+728+w := clitest.StartWithWaiter(t, inv)
729+w.RequireSuccess()
730+require.NotContains(t, stderr.String(), "test.open-error")
731+require.NotContains(t, stdout.String(), "test.open-error")
732+require.Contains(t, stdout.String(), "vscode://coder.coder-remote/open?token=$SESSION_TOKEN")
733+require.NotContains(t, stdout.String(), userClient.SessionToken())
734+require.Contains(t, stderr.String(), "substitute")
735+ })
736+737+t.Run("ExternalAppOnSubAgentWithoutPlaceholderOpensAsIs", func(t *testing.T) {
738+t.Parallel()
739+740+// Sub-agent app URLs that don't reference $SESSION_TOKEN carry no
741+// token to leak. The CLI auto-opens them like any other external
742+// app; only placeholder-bearing URLs are gated.
743+ownerClient, store := coderdtest.NewWithDatabase(t, nil)
744+ownerClient.SetLogger(testutil.Logger(t).Named("client"))
745+first := coderdtest.CreateFirstUser(t, ownerClient)
746+userClient, user := coderdtest.CreateAnotherUserMutators(t, ownerClient, first.OrganizationID, nil, func(r *codersdk.CreateUserRequestWithOrgs) {
747+r.Username = "subagentowner2"
748+ })
749+r := dbfake.WorkspaceBuild(t, store, database.WorkspaceTable{
750+Name: "subagentws2",
751+OrganizationID: first.OrganizationID,
752+OwnerID: user.ID,
753+ }).WithAgent().Do()
754+755+require.NotEmpty(t, r.Agents, "expected at least one workspace agent")
756+mainAgent := r.Agents[0]
757+758+subAgent := dbgen.WorkspaceSubAgent(t, store, mainAgent, database.WorkspaceAgent{
759+Name: "devcontainer",
760+ })
761+_ = dbgen.WorkspaceApp(t, store, database.WorkspaceApp{
762+AgentID: subAgent.ID,
763+Slug: "subapp",
764+External: true,
765+Url: sql.NullString{Valid: true, String: "https://example.com/some/path"},
766+ })
767+768+inv, root := clitest.New(t, "open", "app", r.Workspace.Name+".devcontainer", "subapp", "--test.open-error")
769+clitest.SetupConfig(t, userClient, root)
770+771+w := clitest.StartWithWaiter(t, inv)
772+w.RequireError()
773+w.RequireContains("test.open-error")
774+w.RequireContains("https://example.com/some/path")
775+ })
681776}