Unused server actions have endpoints created for them

[RhysSullivan]

Link to the code that reproduces this issue

https://github.com/RhysSullivan/next-unused-server-actions

To Reproduce

1. run next dev
2. go to localhost:3000
3. replace "getSignedInUser" with "getAllUsersPrivateDoNotLeak" in page.tsx
4. open dev tools to the network tab
5. press the button, copy the next action header value
6. revert step 3, changing page.tsx back to just using "getSignedInUser"
7. ensure "getAllUsersPrivateDataDoNotLeak" is not used anywhere in the app
8. run next build && next start
9. make a POST request to localhost:3000, with a json body of an empty array, and the copied next-action header
10. notice that despite being an unused function on the client, getAllUsersPrivateDoNotLeak still has an endpoint created for it resutling in data leaking

Current vs. Expected behavior

Current behavior is for all exported functions in a file with "use server" at the top of it to be turned into endpoints.

While this is called out in the docs

A Server Action can be defined with the React "use server" directive. You can place the directive at the top of an async function to mark the function as a Server Action, or at the top of a separate file to mark all exports of that file as Server Actions.

This can lead to developers accidentally exposing endpoints that they didn't mean to and don't ever consume on the client.

A scenario where this can occur: A developer not familiar with this functionality adds a utility function to their actions file, and exports it to test it / use in other locations throughout the app. Now they've accidentally created an endpoint anyone can hit without realizing it.

It's also trivial to detect these unintentionally exposed functions.

For open source projects, all someone has to do is write a script to go through open source NextJS apps and find all exported functions in files with "use server" that aren't consumed on the client

For closed source projects, attackers just have to look at the JS payload to find all server actions which aren't assigned to a variable

Out of curiosity, I did an initial look to see if any open source projects hit this footgun and have already found one, so I think this needs to be addressed.

The expected behavior would be if an exported function from a file with "use server" is not consumed on the client, it should not be turned into an endpoint that can be hit

This shouldn't impact existing functionality at all, and will help accidentally exposing internal functions that are written in a file with the "use server" directive

Provide environment information

Operating System:
  Platform: darwin
  Arch: arm64
  Version: Darwin Kernel Version 23.0.0: Fri Sep 15 14:42:57 PDT 2023; root:xnu-10002.1.13~1/RELEASE_ARM64_T8112
  Available memory (MB): 16384
  Available CPU cores: 8
Binaries:
  Node: 18.17.0
  npm: 9.6.7
  Yarn: 1.22.22
  pnpm: 8.15.4
Relevant Packages:
  next: 14.2.0-canary.46 // Latest available version is detected (14.2.0-canary.46).
  eslint-config-next: N/A
  react: 18.2.0
  react-dom: 18.2.0
  typescript: 5.1.3
Next.js Config:
  output: N/A

Which area(s) are affected? (Select all that apply)

App Router

Which stage(s) are affected? (Select all that apply)

next dev (local), next build (local), next start (local)

Additional context

To help visualize, here is a screenshot. Top left you can see the actions, top right you can see the only action being used on the client is the public action. Bottom left you can see prod Next running. Bottom right you can see the request succeeding to the endpoint

[shuding]

Hey! There're two issues with this, one is that the unused export wasn't properly tree-shaken where we will try to address.

This can lead to developers accidentally exposing endpoints that they didn't mean to and don't ever consume on the client.

Totally understand the concern but this is not exactly the case here. In the reproduction you copied the header of the request to the previous function (which has a cryptographically hashed id), and then switched to the new function in the code. This is only achievable on development because once you run it on production mode, that previous id won't be exposed anywhere if unused.

We're also adding hash salt rotation to production build, so rebuilds will be protected in this case and all ids will change. I think that will fully address the concern here.

[RhysSullivan]

This is only achievable on development because once you run it on production mode, that previous id won't be exposed anywhere if unused.

@shuding are you sure the id won't be exposed anywhere if unused in a production build? I'm able to see it in the JS that's sent to the client when doing npm run build && npm run start

repro steps:

1. npm run build
2. npm run start
3. Open dev tools to page-{}.js
4. Look in dev tools and see it's included, I've attached a screenshot here with the line highlighted where the id is exposed

[shuding]

@RhysSullivan That id you saw was for the exposed action (the one you are using inside <button onClick={...}>). The other one's id, which isn't used by the client, won't be seen in the client JS files.

[RhysSullivan]

@shuding I deployed the repo to Vercel for further verification https://next-unused-server-actions.vercel.app/

The JS payload is https://next-unused-server-actions.vercel.app/_next/static/chunks/app/page-eab1e6231173d707.js

Inside of that payload, there are 2 action ids listed

6fd2c8e1175bb0510320a3360aea26160ec5e722 - getSignedInUser which is called from the client

4c92e15aecb3e95ebc647ac038a361814d41cb48 - getAllUsersPrivateDoNotLeak which is never referenced on the client

Included in this screenshot to hopefully make it clearer, labels are as follows:

1 - Function which isn't meant to be exposed return body 2 - Only place actions are being called on the client, it's just calling getSignedInUser 3 - Action id of getAllUsersPrivateDoNotLeak 4 - Return value of calling that action, validating it's the same value as 1 5 - JS bundle on the client that has the action ids 6 - Action id of getSignedInUser 7 - Action id of getAllUsersPrivateDoNotLeak

[RhysSullivan]

Hey @shuding am I able to provide any more information to help debug / repro this?

[RhysSullivan]

Hey @shuding, do the repro steps I listed make sense for seeing the unused action id in the client side bundle? Let me know if I can provide more info, thanks

[shuding]

Yes @RhysSullivan thanks that’s very helpful, I’ll take a look!

But also keep in mind that Server Actions are public API endpoints even though they feel like internal function calls. So you’ll still need authentication layer for them.

[renchris]

Server Actions are public API endpoints

This needs to be explicitly stated in the documentation.

The Server Action Documentation states "You should treat Server Actions as you would public-facing API endpoints, and ensure that the user is authorized to perform the action."

The implication taken from this statement is that authorization on Server Actions is needed for application logic user permission roles. Such that a 'logged-out' user role can't invoke a 'logged-in' user action or a 'non-admin' user role can't invoke an 'admin' user action.

The statement should be "Server Actions are public-facing API endpoints, and you should ensure that the user is authorized to perform the action."

[huozhi]

Hey everyone, we've had a fix for this issue on latest canary, and plan to backport it to 14.3. So later when you can upgrade to the new minor once it's out 🙏

[github-actions[bot]]

This closed issue has been automatically locked because it had no new activity for 2 weeks. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

[huozhi]

We had another PR about this issue to avoid unused action references to appear on client side