NextJS 'use server' Trap (Accidentally Leaking Data)
ฝัง
- เผยแพร่เมื่อ 3 ก.ค. 2024
- Ever since React moved onto the server, the new directives "use client" and "use server" have caused some confusion. Lets look at how the top level directive in an actions.ts file may leak some data you may not want, and why this is more an issue of developer confusion rather than an issue with NextJS.
Rhys Tweet: x.com/RhysSullivan/status/180...
-------
🐦 Twitter (X): jollycod.ing/x
🤓 Personal Site: jollycod.ing/me
💻 GitHub: jollycod.ing/git
JollyUI: jollycod.ing/ui
Security threat starts from 9:30
Sorry, considering the mistake is geared more to beginners I think an explainer of the concepts is needed first
Auth for every server action functions. I've got it.
Thank you very much. Maybe you just saved my life.
That's basic javascript. You're exporting 2 separate functions. Even if they weren't separate endpoints the authentication check of one function is clearly outside the scope of the second. Another mistake is fetching data from server actions. Next have explicitly stated in their docs that server actions are only to be used for mutations. They get queued and can't run in parallel so if you start fetching with them and you have 2 different server actions for fetching data inside 1 component one has to wait for the other to complete. Instead fetching in server components can be done with simple helper functions that then get imported at the top level of the server component and fetching in client components (if you want it to be secure) still has to be done through api endpoints so the cookie can be send to the route handler and checked there.
I wish people would read the docs. Server Actions are not meant for fetching data. Regardless, you should always check for auth in any exported server action function and you should always check auth before fetching data.
In an ideal world we would all read the docs!
It is an interesting one, as I said it’s a trap for beginners and experts will be aware of this. In terms of reading the docs I think it’s up to educators to make sure this point is clear now.
At the end of the day beginners will probably find a way to leak data somehow and learn from that, I just hope they have experts up the chain to teach them.
@@JollyCoding I saw a very popular next course on Udemy teaching to use server actions for data fetching. The instructor had no idea they were meant to be used for mutations.
@@michaelfriezewhos the instructor
@@michaelfrieze saw a udemy course teaching people to use floats for money too, oh lord
Server actions can be used for data fetching no problem. There is even several doc examples showing use with react/tanstack query etc. You should just not think that theres any connection between two different function that only has being in the same file in common, it’s literally just a way of defining endpoints
Thanks for covering this. Making clear what the API is - and therefore where the security audit should happen - is really important. I worry about tools that hide this for the sake of convenience. I want this boundary to scream loudly at me so I know what calls can fail due to network partitions or leak data through their parameters or return values. This makes it look like I could maybe understand what the attack surface is with server components, with some effort.
Is there some kind of middleware for server actions? Middlewares can potentially solve this problem.
Informative
SvelteKit handles that separation of concern so much better
Man, when react added hooks I seen a few arguments complaining they added footguns, but to me every "proof" felt like wasnt an actual use-case. Sadly this time I'm on the oposite camp, server actions feels so cool, but feels like there are oopsies that could happen and no one notice that makes me avoid it completely.
Maybe it has something to do with experience, I know that a year down the line most we'll all just gotten used to its quirks, but it makes me scared because I know there are projects being started now, without knowledge of any of those quirks, and that will fall onto someone to sift trhough all of it...
There’s a good tweet from Ryan Florence that says “I’ve seen people leak server stuff my entire career”. Just seems another area that we need to educate people on and be very careful with. Think the issue here is how fast this server stuff has moved and how much of a change it is for frontend devs.
x.com/ryanflorence/status/1808538800005730612?s=46&t=5NfomTO38hUdG85aPtK2SA
Or just store helper functions in functions.js file. It should work too, right?
thanks for the great info!
Great! Thank you.
how could server side rendering leaks data? it is for rendering, nothing more
the issue is: how the heck react became this complicated ?
A growing demand for performance and a move to a full stack framework. Really good talk by Ryan Florence here on the journey of it all and what reacts new role is. th-cam.com/video/zqhE-CepH2g/w-d-xo.htmlsi=ZreHb7iXEBAIDFeA
@@JollyCoding oh thanks, i'll be sure to have a look
I don't use it here, I've a util function, I don't add everything under 'use server'
can't middleware solve this?
Just put auth in middleware like a chad
exactly !!!
Drinking game suggestion - whenever he says "go ahead", you take a shot. Bye bye liver.
Reminds me of PHP
This is so easy to mess up!
There's a fix: use the npm package called astro
Good choice, each has their own strengths!
i don;t see how server actions are a trap.
if you are not check auth when fetching data its your mistake clearly
Everything that I said would happen is happening 😂😂
Things JS devs would do not to split server and client code. I'm a golang backend, JS frontend guy, never mixing code, easy to secure, easy to code, easy to manage.
If you going to use server actions you better instead use tRPC
I’ve found the use zsa very helpful on that matter. The pattern it introduced makes it almost impossible to make these kind of mistakes. Take a look at it as it’s game changing when working with server actions in NextJs
Will have to check that out thanks!
That is why i use authentication in middleware. Problem solved.
I dont get it, why use this server stuff anyway?
You can pick up FastAPI in less than an hour and make a real backend that supports tons of new ai libraries, which your are definately making a wrapper around 😅
This is not called a TRAP, please read the docs and dont ever use something without knowing exactly what it does.
Server actions are one hell of a cool feature and I love them. The thing here is that you are explictly exporting two functions within a single "use server" directive.
Which makes NextJS thinks that you are aware of what you are doing and proceeds to give you exactly what you are looking for. If you don't want to fall for that, then just export one single function and mark it as a default export instead. What I mean, is instead of exporting multiple action in the module, you can export only one default per module/file and that automatically solve your (issuem eventhough it is not an issue).
One more thing you can do, is add your authentication in the middleware and you are GUCCI, no more similiar data leaking.
Conclusion: Please stop blaming the framework for your mistakes, read the docs carefully and only when something behaves differently that it's described you can blame them.
To be clear I wasn’t blaming the framework, I was explaining a technique that it seems people have done that isn’t correct. This video educates them on that. I hope I was clear in the video this isn’t NextJS fault.
The only thing they could change is tree shake the unused functions which will be a quality of life improvement
that's just people using the tool tge wrong way
Yes correct, but it is easy to see how people can fall into this. It’s that typical battle of being beginner friendly and stopping dangerous errors where we can, but at the same time not holding your hand too much that it limits advanced developers doing cool things and customisability
This should've been just a 1-2 minute video!!!!
Sorry, considering the mistake is geared more to beginners I think an explainer of the concepts is needed first
@@JollyCoding yeah not 1-2 minute but the beginning of the video is unnecessary, you could have directly made the application which *has* the problem!!! The statement for the problem is really simple...
@@JollyCoding btw can we use server actions to replace the /APIs ?
I've seen this suggestion on many yt videos and tried and it works fine...
I mean the page isn't server rendered, we call server actions on click or some other events (not only on form submition)
And how is this different from looking the damn Network Tab? Come on. Have y people build real apps? I don’t get the security issues with Server Actions. People put their Google API Keys in their code plain. Y can see them.. what’s the fuss about..?
Well, the problem is solved from just using the inline “use server”, and then inline the function. Problem solved! Drama again!😂
Regardless human stupidity this is actually a really stupid API design. Instead of supporting multi exports as different endpoints, Nextjs server actions should only accept a specifically named function. Like export function action() {} and ignore anything else in the file. It's the same concept as Vue's one component per file.
This is needlessly complicated and less readable
NestJS does this using decorators. Does remind me a lot of .Net controllers though!
React nerds leaking data? What's new about programmers without basic understanding of underlying protocols and technologies doing crappy programming?
It’s always happened and will continue too. All part of the learning process, you just hope that someone experienced doesn’t let it get to production haha
That has absolutely nothing to do with a particular programming language, compiler, or framework.
Absurd comment.
Yea if they are not planning and writing in machine language I aint hiring
Always one of those people 😅
This is a job for decorators. It should be closed by default and opened with an explicit decorator.
```
@serverAction
@auth(USER)
export const getUser = async () => {}
export const getAllUsers = async () => {}
```