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

Prevent js assets from being added into the public assets when targeting server #363

Closed
SeanCassiere opened this issue Sep 3, 2024 · 3 comments · Fixed by #370
Closed

Comments

@SeanCassiere
Copy link
Contributor

SeanCassiere commented Sep 3, 2024

From TanStack Start discussion thread: https://discord.com/channels/719702312431386674/1280228822104674334


Currently, routers with target: "server" and type: "http", have ALL their "assets" output into the "public" directory.

Builds with shared assets therefore cause outputs in the public folder resembling a structure similar to this. In this example, the router name is "api" with its base set to "/api".

public/
  assets/
    regular-client-side-bundles.js
  api/
    assets/
      index-B-8U_3IY.js
      json-Bxz_4eXb.js

See full TanStack Start build-output: https://github.com/SeanCassiere/vigilant-journey/tree/master/.vercel/output/static

Unless "non-built js" assets are found, the "public/api/" directory could probably be omitted during build. Therefore, just outputting:

public/
  assets/
    regular-client-side-bundles.js

From @nksaraf

I think we started doing this when people were importing asset stuff in their server side and were pushing those urls to the client.. those cases we needed to make the sure the assets on the server are available from the client.. but we shouldn’t do this for the js .. just the assets probably
So yeah it’s probably a bug we should raise in Vinxi

@alfredomariamilano
Copy link

Yeah, I just realised I leaked some API keys this way. I think I'm tapping out. Using vinxi/solidstart is a mess and they are not very responsive. I had hoped that having solid start and tanstack start as users of vinxi would create a better ecosystem, but alas ti hasn't happened. I am grateful for all the awesome work people are pouring into these amazing technologies, but at this point in time they're not safe for production.

@SeanCassiere
Copy link
Contributor Author

The shared JS assets being leaked into the public folder are not currently a security concern for TanStack Start, since what's being leaked is our handler implementation and our util for easily returning json. Especially since it is common practice to store and read sensitive variables either from an external service or environment variables.

@nksaraf had solid reasoning for why the "assets" were being sent into the public folder was necessary (#52 issue originating in SolidStart), as well as acknowledging that the built js probably doesn't need to be shipped to be into the public folder.

I'd try implementing a "fix" myself, but I'm just too unfamiliar with this codebase to make any changes with confidence 😅

@alfredomariamilano
Copy link

The shared JS assets being leaked into the public folder are not currently a security concern for TanStack Start, since what's being leaked is our handler implementation and our util for easily returning json. Especially since it is common practice to store and read sensitive variables either from an external service or environment variables.

@nksaraf had solid reasoning for why the "assets" were being sent into the public folder was necessary (#52 issue originating in SolidStart), as well as acknowledging that the built js probably doesn't need to be shipped to be into the public folder.

I'd try implementing a "fix" myself, but I'm just too unfamiliar with this codebase to make any changes with confidence 😅

I may have found the issue in one file in which I dynamically import another file, so that might be the cause of it, but still, the issue is that it was leaked in the public folder when it shouldn't, especially since it's duplicated in server and public. Duplication is also another issue, since most JS files are duplicated, like file.js, file2.js, file3.js, file32.js (!) for some reason.
I'm considering a move to either Astro or Vike. I would love to contribute to the project and help fix these things, but I'm close to launching my SaaS, so time is tight.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants