Skip to content

Commit d7774e5

Browse files
github-actions[bot]zedkipp
github-actions[bot]
and
zedkipp
authored
fix: prevent session token exfiltration via external app URLs (#26146) (#26302)
Backport of #26146 Original PR: #26146 — fix: prevent session token exfiltration via external app URLs Merge commit: 9b550cb Requested by: @f0ssel Co-authored-by: Zach <3724288+zedkipp@users.noreply.github.com>
1 parent 3c46473 commit d7774e5

3 files changed

Lines changed: 121 additions & 37 deletions

File tree

‎cli/open.go‎

Lines changed: 24 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,11 @@ func (r *RootCmd) open() *serpent.Command {
3939

4040
const vscodeDesktopName = "VS Code Desktop"
4141

42+
// externalSessionTokenPlaceholder is the literal substring in an external
43+
// workspace-app URL that the CLI replaces with the user's session token
44+
// when the app belongs to a trusted (top-level) agent.
45+
const externalSessionTokenPlaceholder = "$SESSION_TOKEN"
46+
4247
func (r *RootCmd) openVSCode() *serpent.Command {
4348
var (
4449
generateToken bool
@@ -387,8 +392,13 @@ func (r *RootCmd) openApp() *serpent.Command {
387392
pathAppURL := strings.TrimPrefix(region.PathAppURL, baseURL.String())
388393
appURL := buildAppLinkURL(baseURL, ws, agt, foundApp, region.WildcardHostname, pathAppURL)
389394

390-
if foundApp.External {
391-
appURL = replacePlaceholderExternalSessionTokenString(client, appURL)
395+
externalSubAgentApp := foundApp.External && agt.ParentID.Valid
396+
if foundApp.External && !agt.ParentID.Valid {
397+
// Template-defined apps run on a top-level agent and are
398+
// admin-authored, so their URLs are trusted. Substitute the
399+
// session token placeholder so the OS open handler receives
400+
// a usable URL.
401+
appURL = strings.ReplaceAll(appURL, externalSessionTokenPlaceholder, client.SessionToken())
392402
}
393403

394404
// Check if we're inside a workspace. Generally, we know
@@ -399,6 +409,18 @@ func (r *RootCmd) openApp() *serpent.Command {
399409
_, _ = fmt.Fprintf(inv.Stdout, "%s\n", appURL)
400410
return nil
401411
}
412+
413+
// Sub-agent external app URLs are set at runtime. Only open
414+
// sub-agent URLs that don't contain the placeholder to prevent
415+
// token exfiltration.
416+
if externalSubAgentApp && strings.Contains(appURL, externalSessionTokenPlaceholder) {
417+
cliui.Warnf(inv.Stderr,
418+
"This app was registered from inside the workspace rather than from the workspace template. "+
419+
"Inspect the URL below carefully and, if you trust the source, substitute the $SESSION_TOKEN placeholder "+
420+
"with your session token and manually open it:")
421+
_, _ = fmt.Fprintf(inv.Stdout, "%s\n", appURL)
422+
return nil
423+
}
402424
_, _ = fmt.Fprintf(inv.Stderr, "Opening %s\n", appURL)
403425

404426
if !testOpenError {
@@ -664,15 +686,3 @@ func buildAppLinkURL(baseURL *url.URL, workspace codersdk.Workspace, agent coder
664686
}
665687
return u.String()
666688
}
667-
668-
// replacePlaceholderExternalSessionTokenString replaces any $SESSION_TOKEN
669-
// strings in the URL with the actual session token.
670-
// This is consistent behavior with the frontend. See: site/src/modules/resources/AppLink/AppLink.tsx
671-
func replacePlaceholderExternalSessionTokenString(client *codersdk.Client, appURL string) string {
672-
if !strings.Contains(appURL, "$SESSION_TOKEN") {
673-
return appURL
674-
}
675-
676-
// We will just re-use the existing session token we're already using.
677-
return strings.ReplaceAll(appURL, "$SESSION_TOKEN", client.SessionToken())
678-
}

‎cli/open_test.go‎

Lines changed: 97 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
package cli_test
22

33
import (
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) {
719724
w.RequireContains("region not found")
720725
})
721726

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

730+
// Apps on the top-level (template-defined) agent are trusted, so the
731+
// CLI substitutes $SESSION_TOKEN regardless of scheme.
725732
client, ws, _ := setupWorkspaceForAgent(t, func(agents []*proto.Agent) []*proto.Agent {
726733
agents[0].Apps = []*proto.App{
727734
{
728735
Slug: "app1",
729-
Url: "https://example.com/app1?token=$SESSION_TOKEN",
736+
Url: "vscode://coder.coder-remote/open?token=$SESSION_TOKEN",
730737
External: true,
731738
},
732739
}
@@ -743,4 +750,92 @@ func TestOpenApp(t *testing.T) {
743750
w.RequireContains("test.open-error")
744751
w.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
}

‎docs/user-guides/devcontainers/customizing-dev-containers.md‎

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -247,27 +247,6 @@ Standard dev container variables are also available:
247247
| `${containerWorkspaceFolder}` | Workspace folder path inside the container |
248248
| `${localWorkspaceFolder}` | Workspace folder path on the host |
249249

250-
### Session token
251-
252-
Use `$SESSION_TOKEN` in external app URLs to include the user's session token:
253-
254-
```json
255-
{
256-
"customizations": {
257-
"coder": {
258-
"apps": [
259-
{
260-
"slug": "custom-ide",
261-
"displayName": "Custom IDE",
262-
"url": "custom-ide://open?token=$SESSION_TOKEN&folder=${containerWorkspaceFolder}",
263-
"external": true
264-
}
265-
]
266-
}
267-
}
268-
}
269-
```
270-
271250
## Feature options as environment variables
272251

273252
When your dev container uses features, Coder exposes feature options as

0 commit comments

Comments
 (0)