- Notifications
You must be signed in to change notification settings - Fork913
feat: add csp headers for embedded apps#18374
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Conversation
| proxies := []*proxyhealth.ProxyHost{ | ||
| { | ||
| Host: api.AccessURL.Host, | ||
| AppHost: appurl.ConvertAppHostForCSP(api.AccessURL.String(), api.AppHostname), | ||
| }, | ||
| } |
code-asherJun 14, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I tacked on the primary here, but we could instead add the primary as part of theWorkspaceProxyHostsFn function.
The current method here also includes the primary for AGPL, so if we do that we would also need to make some changes for the AGPL version (assuming wildcard apps and tasks even work with AGPL).
| // is returned from. | ||
| u, _ := url.Parse(p.Url) | ||
| status = proxyhealth.ProxyStatus{ | ||
| Proxy: p, | ||
| ProxyHost: u.Host, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Rather than convertProxyHost to the new format, I deleted it as it is not used in this function or returned in any way. Also any non-primary proxystatus we pass is already missingProxyHost. We should probably use a narrower type or something, idk.
| @@ -0,0 +1,8 @@ | |||
| package proxyhealth | |||
code-asherJun 14, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Felt kinda weird to make a whole file for a single struct but idk where else to put it. Needs to be used in the AGPL code so I am not able to import it directly from enterprise.
I modified the proxy host cache we already had and were using for websocket csp headers to also include the wildcard app host, then used those for frame-src policies.
I did not add frame-ancestors, since if I understand correctly, those would go on the app, and this middleware does not come into play there. Maybe we will want to add it on workspace apps like we do with cors, if we find apps are setting it to
noneor something.Closescoder/internal#684