-
Notifications
You must be signed in to change notification settings - Fork 39
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
feat: add support for passing a nonce to the rehydration script #160
feat: add support for passing a nonce to the rehydration script #160
Conversation
Rehydration currently does not work in an app with a CSP that disables unsafe-inline scripts. With this change, you can now follow the [NextJS guide](https://nextjs.org/docs/app/building-your-application/configuring/content-security-policy) on CSP, and in `layout.tsx` pass the `nonce` from headers down into the `ApolloWrapper` to prevent `unsafe-inline` CSP errors.
@josh-feldman: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Apollo Contributor License Agreement here: https://contribute.apollographql.com/ |
@josh-feldman thanks a ton for the PR! I've tweaked this a bit to now accept a Generally, this is probably not optimal because it means passing the At the same time, it's probably also the best we can get at this point, since Next.js has not native way of having SSR-specific secrets. Could you take another look if these changes still work for your use case? |
Thank you for the more generic solution @phryneas, this should work for us. As far as I'm aware, the serialized props that get passed to a client component come along in the GET request for the Document itself embedded in the Let me know if there is a security concern I'm missing though, I'd like to better understand. |
@josh-feldman The difference is that JavaScript (not even the DevTools) can actually read the Try self.__next_f.map(x=>x[1]).filter(x => x?.includes('nonce')).map(x=>/nonce=(.*?),/.exec(x)[1]) that will extract the nonce props for you in a very unsophisticated & crude way. That said, this is still a lot more safe than without the nonce - but it probably wouldn't help against a very targeted attack. We have a similar problem with token values if they are passed from a server component to a client component. There is a potential mitigation against this to be found in this comment, but it's not nice either and a lot of manual work - as far as I know, nobody has made a library out of this yet. |
@phryneas thank you for the explanation, that makes total sense and I'd agree with your conclusion. I just wanted to make sure we weren't introducing security theater with this PR or encouraging an anti pattern. I'm happy with this PR, let me know if you need anything else from me to get it merged 👍. As you've stated, the nonce is now exposed on the client side JavaScript and perhaps a very targeted attack could find a way around other protections. That being said, a CSP nonce is just a mechanism to prevent arbitrary JavaScript execution on any given page load. I'd imagine in many cases if an attacker has the means to execute code to retrieve the nonce from the RSC payload, then they've already bypassed most protection the nonce would provide. I can see how providing access tokens or auth cookies to the client props (as discussed in your linked comment) could raise a larger concern due to the doors it may open for an attacker. This PR should help mitigate those types of attacks by allowing users of Apollo to set stricter script-src CSP. Let me know if there's any gap in my understanding above, and thank you again for the updates to the PR! |
I agree that this is a low-risk problem in this specific case, but as this came up with a second use case now, I've gone and created a library for this: https://www.npmjs.com/package/ssr-only-secrets I've also added it to the integration tests here, so these should serve as an example how to use it for now. I'll probably get to publishing this PR tomorrow :) |
Rehydration currently does not work in an app with a CSP that disables unsafe-inline scripts.
With this change, you can now follow the NextJS guide on CSP, and in
layout.tsx
orpage.tsx
pass thenonce
from headers down into theApolloWrapper
to preventunsafe-inline
CSP errors.