fix: prevent session token exfiltration via external app URLs (#26146… · coder/coder@d7774e5
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"
@@ -719,14 +724,16 @@ func TestOpenApp(t *testing.T) {
719724w.RequireContains("region not found")
720725 })
721726722-t.Run("ExternalAppSessionToken", func(t *testing.T) {
727+t.Run("ExternalAppOnTopLevelAgentSubstitutes", func(t *testing.T) {
723728t.Parallel()
724729730+// Apps on the top-level (template-defined) agent are trusted, so the
731+// CLI substitutes $SESSION_TOKEN regardless of scheme.
725732client, ws, _ := setupWorkspaceForAgent(t, func(agents []*proto.Agent) []*proto.Agent {
726733agents[0].Apps = []*proto.App{
727734 {
728735Slug: "app1",
729-Url: "https://example.com/app1?token=$SESSION_TOKEN",
736+Url: "vscode://coder.coder-remote/open?token=$SESSION_TOKEN",
730737External: true,
731738 },
732739 }
@@ -743,4 +750,92 @@ func TestOpenApp(t *testing.T) {
743750w.RequireContains("test.open-error")
744751w.RequireContains(client.SessionToken())
745752 })
753+754+t.Run("ExternalAppOnSubAgentWithPlaceholderPrintsURLAndDoesNotOpen", func(t *testing.T) {
755+t.Parallel()
756+757+// Sub-agent app URLs are attacker-influenceable through workspace
758+// configuration and runtime registration. The CLI must not
759+// substitute the session token, and must not hand the URL to the
760+// OS open handler. The URL is printed to stdout so a user who
761+// trusts the source can substitute and open it manually.
762+ownerClient, store := coderdtest.NewWithDatabase(t, nil)
763+ownerClient.SetLogger(testutil.Logger(t).Named("client"))
764+first := coderdtest.CreateFirstUser(t, ownerClient)
765+userClient, user := coderdtest.CreateAnotherUserMutators(t, ownerClient, first.OrganizationID, nil, func(r *codersdk.CreateUserRequestWithOrgs) {
766+r.Username = "subagentowner"
767+ })
768+r := dbfake.WorkspaceBuild(t, store, database.WorkspaceTable{
769+Name: "subagentws",
770+OrganizationID: first.OrganizationID,
771+OwnerID: user.ID,
772+ }).WithAgent().Do()
773+774+require.NotEmpty(t, r.Agents, "expected at least one workspace agent")
775+mainAgent := r.Agents[0]
776+777+subAgent := dbgen.WorkspaceSubAgent(t, store, mainAgent, database.WorkspaceAgent{
778+Name: "devcontainer",
779+ })
780+_ = dbgen.WorkspaceApp(t, store, database.WorkspaceApp{
781+AgentID: subAgent.ID,
782+Slug: "subapp",
783+External: true,
784+Url: sql.NullString{Valid: true, String: "vscode://coder.coder-remote/open?token=$SESSION_TOKEN"},
785+ })
786+787+inv, root := clitest.New(t, "open", "app", r.Workspace.Name+".devcontainer", "subapp", "--test.open-error")
788+clitest.SetupConfig(t, userClient, root)
789+var stdout, stderr bytes.Buffer
790+inv.Stdout = &stdout
791+inv.Stderr = &stderr
792+793+w := clitest.StartWithWaiter(t, inv)
794+w.RequireSuccess()
795+require.NotContains(t, stderr.String(), "test.open-error")
796+require.NotContains(t, stdout.String(), "test.open-error")
797+require.Contains(t, stdout.String(), "vscode://coder.coder-remote/open?token=$SESSION_TOKEN")
798+require.NotContains(t, stdout.String(), userClient.SessionToken())
799+require.Contains(t, stderr.String(), "substitute")
800+ })
801+802+t.Run("ExternalAppOnSubAgentWithoutPlaceholderOpensAsIs", func(t *testing.T) {
803+t.Parallel()
804+805+// Sub-agent app URLs that don't reference $SESSION_TOKEN carry no
806+// token to leak. The CLI auto-opens them like any other external
807+// app; only placeholder-bearing URLs are gated.
808+ownerClient, store := coderdtest.NewWithDatabase(t, nil)
809+ownerClient.SetLogger(testutil.Logger(t).Named("client"))
810+first := coderdtest.CreateFirstUser(t, ownerClient)
811+userClient, user := coderdtest.CreateAnotherUserMutators(t, ownerClient, first.OrganizationID, nil, func(r *codersdk.CreateUserRequestWithOrgs) {
812+r.Username = "subagentowner2"
813+ })
814+r := dbfake.WorkspaceBuild(t, store, database.WorkspaceTable{
815+Name: "subagentws2",
816+OrganizationID: first.OrganizationID,
817+OwnerID: user.ID,
818+ }).WithAgent().Do()
819+820+require.NotEmpty(t, r.Agents, "expected at least one workspace agent")
821+mainAgent := r.Agents[0]
822+823+subAgent := dbgen.WorkspaceSubAgent(t, store, mainAgent, database.WorkspaceAgent{
824+Name: "devcontainer",
825+ })
826+_ = dbgen.WorkspaceApp(t, store, database.WorkspaceApp{
827+AgentID: subAgent.ID,
828+Slug: "subapp",
829+External: true,
830+Url: sql.NullString{Valid: true, String: "https://example.com/some/path"},
831+ })
832+833+inv, root := clitest.New(t, "open", "app", r.Workspace.Name+".devcontainer", "subapp", "--test.open-error")
834+clitest.SetupConfig(t, userClient, root)
835+836+w := clitest.StartWithWaiter(t, inv)
837+w.RequireError()
838+w.RequireContains("test.open-error")
839+w.RequireContains("https://example.com/some/path")
840+ })
746841}