Improved env handling

[florian-lefebvre]

• Accepted Date: 2024-02-09
• Reference Issues/Discussions: Better 1st party env handling #822
• Author: @florian-lefebvre
• Champion(s): @florian-lefebvre (with some help!)
• Implementation PR: N/A

Summary

This RFC aims to improve DX (and eventually security) around env variables.

Background & Motivation

Env variables are an important part of any web application. You need to store sensitive data (think API secrets, tokens etc) without being leaked inside your git repo. But that's only the 1st part of the story. It's easy to leak this data by importing in the wrong place, eg. the frontend like Resend a few weeks ago.

Other JS frameworks (eg. SvelteKit) are handling env pretty well. From my understanding, the env story is currently a bit tricky in Astro. According to the docs, here is how env variables are currently handled:

• Astro uses Vite’s built-in support for environment variables
• Static variables (ie. replaced statically at build time) are accessible via import.meta.env
• import.meta.env includes some default variables like SSR, BASE_URL...
• A public variable key has to be prefixed by PUBLIC_
• A non public variable accessed using import.meta.env on the client side will be undefined (value will be accessible server side)
• Env variables can be loaded through .env (or .env.production, .env.development) and CLI
• Any non built-in variable can be manually typed
• Runtime variables should be access using process.env, or following the used runtime (eg. Deno.env.get() for the deno adapter)
• I'm not sure if process.env has any protection against client-side usage (like import.meta.env), but I guess it doesn't

Goals

• Provide a fully type-safe experience for environment variables, without manual type definitions
• Allow users to mark environment variables as required, providing hints when a variable is missing
• Reduce user confusion between inlined, static variables and dynamic, runtime variables
• Allow adapters to specify how runtime env variables should be handled

Non-Goals

• Complex validation or type casting of env variables. We might want to enable this at some point, but there is likely a performance cost for this at runtime. We should punt on this if possible!
• Allowing integrations to define environment variable constraints. This is a great idea, but until we know what this feature looks like, we should consider it out of scope.
• Future: allow adapters to customize build-time variable handling. Better runtime handling is the most important problem this proposal aims to solve, but if we find an API that enables this as well, that's great.

[natemoo-re]

Great work @florian-lefebvre! I’m very excited to finally be tackling this problem.

[florian-lefebvre]

We discussed a little bit already about about those features could look. Here are some ideas:

1. Static variables can be made available though a virtual module and defined in the Astro config. Type safety is not an issue using codegen (depends on feat: injectDts and codegenDir astro#9952).

export default defineConfig({
	experimental: {
		env: {
			variables: ["FOO", "BAR"]
		}
	}
})

import { FOO, BAR } from "astro:env"
// or import * as env from "astro:env"

2. Static variables validation can be easily achieved from the variables list. We could allow users to choose how strict they want the check to be:

export default defineConfig({
	experimental: {
		env: {
			variables: ["FOO", "BAR"],
			checkLevel: "warn" // "error"
		}
	}
})

3. Public static variables would still be using the PUBLIC_ prefix
4. Dynamic variables would be accessible through a virtual module as well. I know @lilnasy suggested using something like Astro.env.KEY but I think the issue is that it makes it hard to use env variables outside of .astro files and endpoints
5. Dynamic variables "security" could be improved. Currently, using process.env does not prevent from using a secret dynamic variable on the client-side. Since we'll have an abstraction for this, maybe we could use a proxy and depending on the key being accessed (ie. starting with the PUBLIC_ prefix), we could check if it's being access on the client-side (!import.meta.env.SSR) and return undefined
6. I'm not used to the adapter API so I don't have much ideas about how it should look

[bholmesdev]

I appreciate these changes! Looking at the config, it seems odd to me that I can't have an "optional" check for variables. I agree Zod is overkill here, but primitive options per-variable, instead of a global "warn vs. error" property, would be nice.

[florian-lefebvre]

what do you think about something like this?

env: {
	variables: ({ string }) => ({
		FOO: string(),
		BAR: string({ optional: true })
	})
}

This way, we can set optional variables + it prepares the ground for more advanced schema

[NuroDev]

Or if it was to be even simpler something along these lines could work?

export default defineConfig({
	experimental: {
		env: {
			variables: {
				FOO: true, // Or possibly `null` instead? Some other kind of truthy value
				BAR: { optional: true, }
			},
			checkLevel: "warn" // "error"
		}
	}
});

[florian-lefebvre]

could be! the advantage of having string({ optional: true }) is that in the future we could have stuff like this

env: {
	variables: ({ string, number, boolean, enum }) => ({
		FOO: string({ minLength: 5 }),
		BAR: string({ optional: true }),
		TRUE: boolean(),
		ENV: enum(["dev", "prod"])
	})
}

[NuroDev]

IMO if the plan is to offer that level of validation control then it may make more sense to just expose Zod like is already done in content collection configs?

[florian-lefebvre]

I think it's better to avoid zod here for 2 reasons

1. It's hard to serialize and deserialize (we'll need to pass stuff to a virtual import, I already tried and it requires at least 2 3rd party libs + some hacks; or a distinct entrypoint but it feels like it's a bit too much). It's better if we own format so that we can generate a schema ourselves
2. zod contains a lot of stuff not relevant to env (eg. functions, objects, void etc) and not everything will work as expected. For example:
   • z.number won't work, you need to use z.coerce.number
   • z.boolean is even trickier (see https://env.t3.gg/docs/recipes#booleans). By having our own format we can abstract those for the users and improve the DX

[florian-lefebvre]

Maybe related to this RFC withastro/docs#5328

[alexanderniebuhr]

I've been giving it some thought, and I'm leaning towards not reinventing the wheel when it comes to validation logic. Considering the existing solutions available, and the familiarity that Astro users have with zod, I'd warmly recommend that we either skip validation altogether or opt to integrate zod into our implementation.

[florian-lefebvre]

I'm not suggesting to not use zod at all, just not in the public interface. I've been working on it tonight for astro-env and as soon as I have something to show (probably this week), I'll post here

[florian-lefebvre]

Well I got distracted by other contributions since then 😅 but here is the PR! Useful links are on the PR description to see how the API looks: florian-lefebvre/astro-env#4

[matthewp]

@florian-lefebvre have you thought about runtime environment variables much? this is something that's come up recently with @astrojs/db which needs to read runtime variables but can't easily do so while working in various non-Node environments.

Here's the start of an idea (maybe we can improve)

// Default export
import getEnv from "astro:env"

getEnv('FOO'); // always retrieved at runtime

adapter.js

'astro:config:done': ({ setAdapter }) => {
  setAdapter({
    name: '@matthewp/my-adapter',
    serverEntrypoint: '@matthewp/my-adapter/server.js',
    envEntrypoint: '@matthewp/my-adapter/env.js'
  });
},

env.js

export default function(key) {{
  return Deno.getEnv(key);
}

cc @alexanderniebuhr not sure if this works for Cloudflare or not.

[florian-lefebvre]

@matthewp I suggested this and this is kinda what I have in mind! But apparently it has to be tied to the request because of cloudflare (so it would most likely be available as Astro.env.FOO)

[alexanderniebuhr]

So for Cloudflare there is no runtime without a request, so once you have the runtime you already have the request chain and then the env is accessible inside the request handler in Cloudflare. We do write the env to Astro.locals.runtime.env, but we can also write to to something else.

So as long as env.js has access to something, which is writable inside the request handler exported by serverEntrypoint, e.g. the lines of app.render. I don't see any blocks with Cloudflare.. But once we have a POC, I'm more than happy to test it with Cloudflare

[florian-lefebvre]

I'm not familiar enough with the adapters api to know what's possible tbh!

[matthewp]

Ok, I figured this was probably a constraint. The only issue with the Astro.env idea is that's only accessible by Astro components. I think we'll want to figure out a solution that works for any module, Astro, JS or otherwise.

[lilnasy]

The solution could be AsyncContext or AsyncLocalStorage - users would call getEnv() wherever and it will retrieve from the environment associated with currently "active" request. It would still have to error at the top-level on cloudflare because there is active request and no global environment.

AsyncLocalStorage is a WinterCG standard, but requires enabling node compat on cloudflare, which isn't straightforward from what I understand, and that may be a blocker. AsyncContext is a stage 2 tc39 proposal.

[alexanderniebuhr]

It would still have to error at the top-level on cloudflare because there is active request and no global environment.

Correct I think that is important to understand that there is a "global" context, but it is not recommended to use it for env, and we need to think about env per request.. However this is not a Cloudflare only issue, other runtimes with a isolate approach have the same logic.

AsyncLocalStorage is a WinterCG standard, but requires enabling node compat on cloudflare, which isn't straightforward from what I understand, and that may be a blocker.

There is some nuances to this, so AsyncLocalStorage wouldn't work out-of-the-box with Cloudflare Pages, however it does work with the nodejs_compat flag (which is different to the node_compat flag).
Additionally the import would have to look like import { AsyncLocalStorage } from 'node:async_hooks';
ref: https://developers.cloudflare.com/workers/runtime-apis/nodejs/asynclocalstorage/

Enabling nodejs_compat in theory doesn't have any downsides for the user (node_compat has downsides), users need to configure it manually to make sure their Astro project works though. I don't know if that is something we want, looking at UX?
There are plans to use wrangler.toml configuration file for Cloudflare Pages, once Cloudflare ships that, we could add a base wrangler.toml with the correct flag settings when using astro add cloudflare and document it when users choose to install manually.

In addition to that, we would have to decide if we want to add a check and tell users they forgot the flag or not?
ref: https://github.com/cloudflare/next-on-pages/blob/9900517223d34612d503b0489d3383c3b4453cd2/packages/next-on-pages/templates/_worker.js/index.ts#L34-L37

[ematipico]

@alexanderniebuhr out of curiosity, how do you set and read environment variables with Cloudflare? Code wise.

[alexanderniebuhr]

So I'll try to answer this as detailed as possible, but still as consise as possible.

The whole message only talks about "runtime" environment variables, build/compile time environment variables, are a completly different story.

You do set environment variables using the Cloudflare Dashboard, for Workers you can also use wrangler.toml and .dev.vars files, Pages will get config file support in the future, but it doesn't have it yet.

Pages/Workers (I'll try to make sure to highlight any differences if needed) do have a concept of environment variables on a per-Page/Worker and per-request basis. These are not accessible automatically via the process.env API. It is possible to manually copy these values into process.env if you need to, and those will be globally persistent for all Workers running in the same isolate and context. Be aware, however, that means env variables could leak inside the same isolate and context. In addition any value on process.env will coerce that value into a string.

Cloudflare recommends strongly that you/framworks do not replace the entire process.env object with the request env object. It will cause unexpected bahviour for other Workers running in the same isolate.

Cloudflare has some docs around this.

---

In general you can access the environment variables in the request handler. This looks a little different for Workers and Pages, but the concept is general the same. Pages (Astro) uses the Module Worker Syntax.

export default {
  async fetch(request, env, ctx) {
    // env is the object that contains your environment variables and bindings.
  },
};

We (Astro) currently overwrite the whole process.env completly (reference), which I elaborated on above, is not the correct way. The next major version of the Adapter v10, will remove this. (However that means right now users should be able to access the environment variables via process.env as long as they only need a string value. This wouldn't work for any Cloudflare Bindings)

The other options for Users to access the request based environment variables is via Astro.locals.runtime.env (reference). However this has the downside that it doesn't work in .js or .ts files.

---

So that means there is no "global" way for a Astro User to access env variables, because we handle the request handler as serverEntrypoint and need to provide a way for users to access the env object which is a parameter of the request handler, we can decide how that should look like. (I don't know how other frameworks handle it, but we can explore that)

[matthewp]

@florian-lefebvre is it a goal that you can use this package within your Astro config? Many users ask for the ability to use import.meta.env in their Astro config but they cannot. Not saying this proposal needs to solve this problem, just asking.

[florian-lefebvre]

No this is out of scope, actually I didn't even know people wanted that!

[ematipico]

No this is out of scope, actually I didn't even know people wanted that!

Yeah it's something that a lot of people started to do since JS config files started to be a thing: ability to tweak it based on environment variables.

I would personally discourage this kind of pattern

[alexanderniebuhr]

I very good piece of context, why it also makes sense for Cloudflare to have request based env variables:

https://blog.cloudflare.com/workers-environment-live-object-bindings#why-is-env-a-parameter-to-fetch-not-global

[bluwy]

Awesome work! I quite like the direction for the validation.

I don't mind the separation between static and dynamic fields. That makes it clear what environment variables can be dynamically set in the deployment platform. If we combine it, maybe we could also support something like envField.static() and envField.dynamic()?

I also think that we should limit the env values to primitives like strings and numbers for now.

Also, do we want to tackle prevention of leaking sensitive environment variables? Unlike SvelteKit with its separate dimension of public/private, I think in Astro we can assume static = public and dynamic = private. And we make sure that dynamic env vars are not used in the client side.

Lastly, I'm a bit concerned with how dynamic env var will be implemented. Would we have to do the validation in runtime instead? And does that brings in a lot of dependency?

[florian-lefebvre]

About separating static/dynamic, here is what I shared at the end of the cloudflare env thread:

Actually, reading this convo reminded me why I think we need 1 schema per env variables type (static/dynamic). In the case of a cloudflare binding, that's a runtime thing but people would be able to try to access it through astro:env/static, and that would fail. Maybe there's a more elegant way than having 2 schemas, like adding a property on each field

I think Bjorn's idea is pretty elegant: we could have something like this:

// or envField.dynamic
envField.static({
	type: "string",
	optional: false
})
// OR the opposite
envField.string({
	context: "static", // "dynamic"
	optional: false
})

I don't mind having getEnv for static usage as well. We just need to see if there's a single getEnv for both static and dynamic values.

Regarding validation of dynamic env, I think it will be done at runtime (when calling getEnv, maybe we have a way to add caching within the same request) using zod since it's already included in astro. It should not require any other library afaik

Also, do we want to tackle prevention of leaking sensitive environment variables? Unlike SvelteKit with its separate dimension of public/private, I think in Astro we can assume static = public and dynamic = private. And we make sure that dynamic env vars are not used in the client side.

I think static variables are already protected because they'll use import.meta.env under the hood. I don't know how we should do it for dynamic variables tho. I guess we could ship a different virtual module on the client that doesn't even import sensitive data, idk

[bluwy]

Regarding validation of dynamic env, I think it will be done at runtime (when calling getEnv, maybe we have a way to add caching within the same request) using zod since it's already included in astro. It should not require any other library afaik

I'm not sure if zod is part of the bundle, IIRC it's only used during build-time, so if someone use dynamic env vars, that includes around ~50kb of code to the bundle. There's also the handling to serialize the dynamic schema from astro.config.js to the bundle runtime.

I don't know how we should do it for dynamic variables tho. I guess we could ship a different virtual module on the client that doesn't even import sensitive data, idk

If it's a virtual module, it should be simple to check so, example. And we could decide to stub/warn/error something.

[florian-lefebvre]

If zod is problematic, I think it's fine to do some validation ourselves. I mean, given that we are going to only support a tiny subset of zod's api, it will be easy (and lightweight) to implement

[bholmesdev]

Of the APIs I've seen presented, I'd prefer an extension function like envField.static(...) and envField.dynamic(...). These match the type conventions for Astro DB columns.

That said, I'm not very comfortable with the "static" vs. "dynamic" naming. I haven't seen this convention in other frameworks, and it isn't immediately clear as a user what these names mean. I also noticed Bjorn mention this above:

Unlike SvelteKit with its separate dimension of public/private, I think in Astro we can assume static = public and dynamic = private.

If this is the case, would it make sense to use a more standard convention like envField.variable(...) and envField.secret(...), or envField.public(...) and envField.private(...)? Having used platforms like Vercel, GitHub, and Cloudflare, I understand the difference between variables and secrets as a user. The encoding mechanism (static vs. dynamic) becomes an implementation detail users don't have to think about.

[florian-lefebvre]

I don't think saying "static = public and dynamic = private" is true. If I understand correctly, we currently have:

	Build time	Runtime
Public	import.meta.env.PUBLIC_FOO	process.env.*
Private	import.meta.env.FOO	N/A

I think public/private could still be kept defined by the PUBLIC_ prefix, but having a way to distinct build time / runtime variables is really important imo. I think that's the nice part about sveltekit, they have clear imports for each of these 4 cases (see docs).

[bluwy]

(I think you have runtime-public and runtime-private flipped)

I don't think supporting runtime-public makes a lot of sense. For buildtime-private, yeah I was thinking that maybe we don't support it and force users to runtime-private instead. If it was sensitive, maybe it shouldn't be inlined in the first place. And that users will always have to set the private env var on their deployment platform.

If we still want to support it, I also don't mind, just that I would prefer a chain API like:

env.static().optional() // implicitly private by default?
env.static().public().default("")
env.dynamic().private().optional()
env.dynamic().public().optional()

If the API is like envField.static({ type: "string", optional: false }), we could directly accept an object instead? { type: "string", scope: "static", optional: false }

[florian-lefebvre]

I quite like this API tbh! I don't especially mind supporting or not public dynamic variables either

I see a few issues with chaining tho. What happens if we have this case?

schema: {
	FOO: env.static().public()
}

Since it doesn't have the PUBLIC_ prefix, should we error and explain it's required? What about this case if we don't support public dynamic vars?

schema: {
	PUBLIC_FOO: env.dynamic()
}

Should we error saying it can't have this prefix?

[bluwy]

Yeah I think we should error in those two cases and treat it as a config validation error. We currently have zod validate the Astro config, and we could do a nested validation within it too.

[matthewp]

What is the definition of static and dynamic? What do these terms mean (in the context of environment variables)?

[florian-lefebvre]

Static means available at build time through import.meta.env. Dynamic means available at runtime and depends on the adapter, eg process.env or Deno.env.get()

[alexanderniebuhr]

I also struggled with those terms at the beginning, maybe we should refer to them as build/compile-time vs runtime variables?

[florian-lefebvre]

Yeah it's a bit longer but way clearer

[ematipico]

Good suggestion @alexanderniebuhr, I prefer this naming convention!

[florian-lefebvre]

So to recap since last time I asked for feedback

How do we want to call the 2 types of env variables? I went for static/dynamic but I'm open to anything

Let's go for buildTime / runtime

How do we feel about having 2 distinct schemas for static/dynamic vars?
What do you think about envField? Just to clarify, envField.string(options) returns { type: "string", ...options } under the hood

Instead of having 2 schemas, it will be part of envField as suggested by Bjorn

What do you think about having 2 APIs for dynamic variables? I don't see any other possibilities to remain cloudflare-friendly

We are going to use ALS + virtual import only.

---

More questions now!

1. Ema expressed concerns about overriding constants exported from astro:env/static. I still think it improves the DX (it's also done by sveltekit) but I'd like your opinions. If we don't do this, what should we do? I think we need 2 distinct imports anyway at least?
2. Do we want to support public runtime variables? Or are we fine with only keeping them private (ie. always undefined on the client even with the PUBLIC_ prefix)?

[bluwy]

1. buildTime and runtime reads weird to me 😄 Personally I prefer static/dynamic but not a hill I'd die on.
2. I think overriding constants is fine as it's only an implementation detail.
3. How would we support public runtime variables though? For example, how would PUBLIC_FOO be referenced in runtime? If the variable is already public, they can switch to buildtime instead.

[florian-lefebvre]

buildTime and runtime reads weird to me 😄 Personally I prefer static/dynamic but not a hill I'd die on.

I'm comfortable with both namings tbh but idk what's the best way to gather feedback as some kind of poll on gh ngl

How would we support public runtime variables though? For example, how would PUBLIC_FOO be referenced in runtime? If the variable is already public, they can switch to buildtime instead.

That wouldn't change anything on the server side, but it would require extending the window with the public runtime values and use those in the client side version of the virtual import. This adds some little JS to every page and tbh, I don't see the benefit much either

[ematipico]

About the naming, I think we should reach out to docs for an opinion.

We keep naming things based on what are internally, but we have to consider how docs teach concepts to our users. For example they try to push the term "on demand" instead of dynamic/SSR.

To echo what @bluwy said, naming is hard and subjective, so I would feel more comfortable to use any name that docs suggests, because at the end they will teach the narrative to our users.

[florian-lefebvre]

A few comments from @FredKSchott on Discord

Fred

SST calls these "Secrets" (runtime) vs. "Parameters" (static) which I always liked, although it implies that runtime things are always secret which may not always be neccessarily true.

However, if the big thing you need to know about static is that they get bundled into your build itself, then a name should definitely make that distinction clear

aka the distinction I would care about is that it's not really about runtime vs. buildtime, but more about "included in your build = insecure" vs. "read from the server at runtime = secure" (which gets back to why I think "Secrets vs. Parameter" is actually pretty good, all things considered)

no strong objections to the other names though, at the end of the day. Static vs. Dynamic is pretty good as well.

Florian

the thing is, "included in your build" is not necessarily unsecure. I mean, if you prefix it with PUBLIC_ then it means it's safe to use client side right?

Fred

Anything in the deployed code (not read from ENV) should be considered insecure as a default, yes. For ex:

• Deploying it to your own machine = the secret is readable to any user account with SSH + file read access
• Deploying it to Vercel = the secret is readable from the "source" tab (possibly Netlify, CF, etc. as well?)

[bholmesdev]

That's some good perspective! I notice Cloudflare and GitHub also use "secret" vs "variable" (or parameter in SST's case). It seems like we're not agreed on whether buildtime vs. runtime is synonymous with variable vs. secret. I think static vs. dynamic would be more agreeable thinking it over

[alexanderniebuhr]

The only issue I have with static vs. dynamic is, that those variables are not really "static" or "dynamic". Both types can be changed/overriden by the user in the runtime. Both types can be updated with a config update (for some hosts that needs a redeployment for both types).
I associate a changing value as "dynamic", e.g. timestamp

[klizter]

Built in support for dynamic PUBLIC environment variables would be great. We just recently ran into the issue of getting the same build to work in several different environments, because public variables are baked in at build time. I hope this will work in the future, for now we have to handle this issue outside Astro.

[bluwy]

@klizter If we support dynamic public env vars, how would you envision the API to pass in the different env var value dynamically? (specifically client-side / browsers). On the server-side, it's simpler with process.env or Deno.env, but on the client-side, it's not clear to me how that's being passed. Hence, I suggested above to not support it.

[florian-lefebvre]

Stage 3 RFC available in #894

[nemanjam]

How do I access env variables in the astro.config.ts itself? Can they be defined and used in the same file? For example i store site url as env variable site: SITE_URL.

import { envSchema } from './src/utils/env';
const { SITE_URL } = CONFIG;

export default defineConfig({
  site: SITE_URL, // I still must use process.env.SITE_URL here?
  experimental: {
    env: {
      schema: envSchema,
    }
  },
...
});

[florian-lefebvre]

Hey @nemanjam thanks for bringing this up! It's out of scope of this RFC but we're not against it! That's something often asked and source of confusion so I think that's something we could tackle. Would you mind creating a new roadmap discussion for this with as much info/context as you can? Then we can share it to gather more feedback and usecases