Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Spec out API for apps to set further CSP restrictions. #3432

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

zenhack
Copy link
Collaborator

@zenhack zenhack commented Sep 17, 2020

Per #3410

Nothing is actually implemented yet; I'd like feedback on the design.

@ocdtrekkie ocdtrekkie added ready-for-review We think this is ready for review security Security issues or improvements app-platform App/Sandstorm integration features labels Sep 17, 2020
@kentonv
Copy link
Member

kentonv commented Sep 26, 2020

Realistically what kinds of restrictions do we think apps are going to want to apply to themselves? I can see the use case for disallowing inline and eval, but beyond that, is there a strong use case for other restrictions? It seems like only those two bits are interesting -- I'm not sure what legitimate security concerns are solved by restricting the others (beyond what Sandstorm already intends to do by default).

If apps are only really likely to use those two bits then I'd say we should define two flags and leave the rest out. Otherwise I think the logic to merge the app's CSP with Sandstorm's policy is going to be sort of complicated without adding that much value.

@zenhack
Copy link
Collaborator Author

zenhack commented Sep 26, 2020

@mnutt, have an opinion (since IIRC you suggested this on IRC)?

I don't feel that strongly; unless others see a reason to keep the full set I'm fine removing it -- it'll be easy to add later if we want it.

@ocdtrekkie ocdtrekkie removed the ready-for-review We think this is ready for review label Oct 24, 2020
@ocdtrekkie
Copy link
Collaborator

Whatever magical algorithm decides who I can assign to what has said I can't assign @mnutt to this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
app-platform App/Sandstorm integration features security Security issues or improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants