fix(security)!: path-based app sharing changes #5772
Conversation
This commit disables path-based app sharing by default. It is possible for a workspace app on a path (not a subdomain) to make API requests to the Coder API. When accessing your own workspace, this is not much of a problem. When accessing a shared workspace app, the workspace owner could include malicious javascript in the page that makes requests to the Coder API on behalf of the visitor. This vulnerability does not affect subdomain apps. - Disables path-based app sharing by default. Previous behavior can be restored using the `--dangerous-allow-path-app-sharing` flag which is not recommended. - Disables users with the site "owner" role from accessing path-based apps from workspaces they do not own. Previous behavior can be restored using the `--dangerous-allow-path-app-site-owner-access` flag which is not recommended. - Adds a flag `--disable-path-apps` which can be used by security-conscious admins to disable all path-based apps across the entire deployment. This check is enforced at app-access time, not at template-ingest time.
cli/deployment/config.go
Outdated
| }, | ||
| DisablePathApps: &codersdk.DeploymentConfigField[bool]{ | ||
| Name: "Disable Path Apps", | ||
| Usage: "Disable workspace apps that are not served from subdomains. Path-based apps can make requests to the Coder API and pose a security risk when the workspace serves malicious Javascript. This is recommended for security purposes if a --wildcard-access-url is configured.", |
There was a problem hiding this comment.
| Usage: "Disable workspace apps that are not served from subdomains. Path-based apps can make requests to the Coder API and pose a security risk when the workspace serves malicious Javascript. This is recommended for security purposes if a --wildcard-access-url is configured.", | |
| Usage: "Disable workspace apps that are not served from subdomains. Path-based apps can make requests to the Coder API and pose a security risk when the workspace serves malicious JavaScript. This is recommended for security purposes if a --wildcard-access-url is configured.", |
Should this be default if --wildcard-access-url is enabled? I think it'd be preferable if we were secure-by-default in most configurations.
There was a problem hiding this comment.
Maybe we could make that change in the future, but the other changes in this PR of disabling sharing for path-based apps will mitigate this problem in almost all cases.
There was a problem hiding this comment.
I approve the way we interact with authz.
Unfortunately to do this entirely in authz RBAC checks would require dynamic permissions , scopes, or roles.
Dynamic roles is not something we should really get into prematurely, so doing a static check if the workspace.OwnerID == roles.ID is just something we have to live with right now.
Another opther option looked into was making a new resource for Path based apps, but still has the issue of making dynamic permissions based on mutable flags 😢. If we did this, we could implement the permissions such that there is no security vulnerability (by not giving perms for path based apps), but then users cannot share path based apps which is a great feature for small deployments and demos 🤷.
UX vs security is always a fun battle.
👍
| type DangerousConfig struct { | ||
| AllowPathAppSharing *DeploymentConfigField[bool] `json:"allow_path_app_sharing" typescript:",notnull"` | ||
| AllowPathAppSiteOwnerAccess *DeploymentConfigField[bool] `json:"allow_path_app_site_owner_access" typescript:",notnull"` | ||
| } |
There was a problem hiding this comment.
I think it would be nice to include the dangerous ratelimits flag in this config.
This commit disables path-based app sharing by default. It is possible for a workspace app on a path (not a subdomain) to make API requests to the Coder API. When accessing your own workspace, this is not much of a problem. When accessing a shared workspace app, the workspace owner could include malicious javascript in the page that makes requests to the Coder API on behalf of the visitor.
This vulnerability does not affect subdomain apps.
Disables path-based app sharing by default. Previous behavior can be restored using the
--dangerous-allow-path-app-sharingflag which is not recommended.Disables users with the site "owner" role from accessing path-based apps from workspaces they do not own. Previous behavior can be restored using the
--dangerous-allow-path-app-site-owner-accessflag which is not recommended.Adds a flag
--disable-path-appswhich can be used by security-conscious admins to disable all path-based apps across the entire deployment. This check is enforced at app-access time, not at template-ingest time.Note: After this is merged we will immediately release a minor release.
Updating the tests required a lot of massaging, since the workspaces were being made by the site owner in almost all of them.
https://github.com/coder/security/issues/3