toastdotdev/toast

process.env.* replacement should scope down to only used vars

ChristopherBiscardi opened this issue · 6 comments

The environment variable handling for process.env.* string replacements implemented here is extremely broad and results in errors like the following if an "impossible" env var is encountered. This is a cross platform issue (reports on Mac and Windows).

failed to parse global variable ITERM_ORIG_PS1=`'\[\e[33m\]\u\[\e[m\]@\[\e[32m\]\h\[\e[m\]:\W `parse_git_branch`$ '` as module

I'm not sure what the best fix is. We could safelist anything with a TOAST_ prefix and then we still need a solution for rando process.envs. Possibly a config option in toast.js to safelist more env vars.

reported by @jbolda and @lannonbr

The env vars are restricted to TOAST_* which works. We could probably use a safelist in toast.js as well. The esinstall config will also be going in toast.js

this seems like the right idea since you should deliberately opt in to inject an env var into client code. I do like the idea of a config setting to explicitly allow vs. prefixing, though

@jlengstorf you think we should remove the TOAST_* prefix? I've never loved it myself in other frameworks. Makes some code "non-portable" when it doesn't need to be. I think the explicit config setting to safelist is solid though.

noting that the env vars were scoped down in a recent PR, and that the TOAST_ prefix is already working while the manual safelisting is not

I think supporting a safelist is a great idea

maybe it could be a toast.js export?

export const exportEnvToClient = ['PUBLIC_KEY', 'CLOUDINARY_CLOUD_NAME'];

// or a function

export const exportEnvToClient = () => Object.keys(process.env).filter((k) => k.startsWith('PUBLIC_'));

a function could be enumerated at instantiation time, so I don't think we need to offer that as API surface.

export const exportEnvToClient = () => Object.keys(process.env).filter((k) => k.startsWith('PUBLIC_'));

would do the same as this, but need an additional function call.

export const exportEnvToClient = Object.keys(process.env).filter((k) => k.startsWith('PUBLIC_'));