Turbopack: Add TransientEnvMap to avoid persisting env vars in cache

turbopack/crates/turbo-tasks-env/src/command_line.rs

@@ -1,7 +1,7 @@
 use turbo_rcstr::RcStr;
 use turbo_tasks::{FxIndexMap, Vc, mark_session_dependent};
 
-use crate::{EnvMap, GLOBAL_ENV_LOCK, ProcessEnv, sorted_env_vars};
+use crate::{GLOBAL_ENV_LOCK, ProcessEnv, TransientEnvMap, sorted_env_vars};
 
 /// Load the environment variables defined via command line.
 #[turbo_tasks::value]
@@ -24,7 +24,7 @@ fn env_snapshot() -> FxIndexMap<RcStr, RcStr> {
 #[turbo_tasks::value_impl]
 impl ProcessEnv for CommandLineProcessEnv {
     #[turbo_tasks::function]
-    fn read_all(&self) -> Vc<EnvMap> {
+    fn read_all(&self) -> Vc<TransientEnvMap> {
         mark_session_dependent();
         Vc::cell(env_snapshot())
     }

turbopack/crates/turbo-tasks-env/src/custom.rs

@@ -2,7 +2,7 @@ use anyhow::Result;
 use turbo_rcstr::RcStr;
 use turbo_tasks::{ResolvedVc, Vc};
 
-use crate::{EnvMap, ProcessEnv, case_insensitive_read};
+use crate::{EnvMap, ProcessEnv, TransientEnvMap, case_insensitive_read};
 
 /// Allows providing any custom env values that you'd like, deferring the prior
 /// envs if a key is not overridden.
@@ -23,7 +23,7 @@ impl CustomProcessEnv {
 #[turbo_tasks::value_impl]
 impl ProcessEnv for CustomProcessEnv {
     #[turbo_tasks::function]
-    async fn read_all(&self) -> Result<Vc<EnvMap>> {
+    async fn read_all(&self) -> Result<Vc<TransientEnvMap>> {
         let prior = self.prior.read_all().owned().await?;
         let custom = self.custom.owned().await?;
 
@@ -34,7 +34,8 @@ impl ProcessEnv for CustomProcessEnv {
 
     #[turbo_tasks::function]
     async fn read(&self, name: RcStr) -> Result<Vc<Option<RcStr>>> {
-        let custom = case_insensitive_read(*self.custom, name.clone());
+        let custom_transient: Vc<TransientEnvMap> = Vc::cell((*self.custom.await?).clone());
+        let custom = case_insensitive_read(custom_transient, name.clone());
         match &*custom.await? {
             Some(_) => Ok(custom),
             None => Ok(self.prior.read(name)),

turbopack/crates/turbo-tasks-env/src/dotenv.rs

@@ -5,7 +5,7 @@ use turbo_rcstr::RcStr;
 use turbo_tasks::{FxIndexMap, ReadRef, ResolvedVc, Vc};
 use turbo_tasks_fs::{FileContent, FileSystemPath};
 
-use crate::{EnvMap, GLOBAL_ENV_LOCK, ProcessEnv, sorted_env_vars};
+use crate::{GLOBAL_ENV_LOCK, ProcessEnv, TransientEnvMap, sorted_env_vars};
 
 /// Load the environment variables defined via a dotenv file, with an
 /// optional prior state that we can lookup already defined variables
@@ -24,15 +24,18 @@ impl DotenvProcessEnv {
     }
 
     #[turbo_tasks::function]
-    pub fn read_prior(&self) -> Vc<EnvMap> {
+    pub fn read_prior(&self) -> Vc<TransientEnvMap> {
         match self.prior {
-            None => EnvMap::empty(),
+            None => TransientEnvMap::empty(),
             Some(p) => p.read_all(),
         }
     }
 
     #[turbo_tasks::function]
-    pub async fn read_all_with_prior(&self, prior: Vc<EnvMap>) -> Result<Vc<EnvMap>> {
+    pub async fn read_all_with_prior(
+        &self,
+        prior: Vc<TransientEnvMap>,
+    ) -> Result<Vc<TransientEnvMap>> {
         let prior = prior.await?;
 
         let file = self.path.read().await?;
@@ -77,7 +80,7 @@ impl DotenvProcessEnv {
 #[turbo_tasks::value_impl]
 impl ProcessEnv for DotenvProcessEnv {
     #[turbo_tasks::function]
-    fn read_all(self: Vc<Self>) -> Vc<EnvMap> {
+    fn read_all(self: Vc<Self>) -> Vc<TransientEnvMap> {
         let prior = self.read_prior();
         self.read_all_with_prior(prior)
     }

turbopack/crates/turbo-tasks-env/src/filter.rs

@@ -2,7 +2,7 @@ use anyhow::Result;
 use turbo_rcstr::RcStr;
 use turbo_tasks::{FxIndexMap, ResolvedVc, Vc};
 
-use crate::{EnvMap, ProcessEnv};
+use crate::{ProcessEnv, TransientEnvMap};
 
 /// Filters env variables by some prefix. Casing of the env vars is ignored for
 /// filtering.
@@ -30,7 +30,7 @@ impl FilterProcessEnv {
 #[turbo_tasks::value_impl]
 impl ProcessEnv for FilterProcessEnv {
     #[turbo_tasks::function]
-    async fn read_all(&self) -> Result<Vc<EnvMap>> {
+    async fn read_all(&self) -> Result<Vc<TransientEnvMap>> {
         let prior = self.prior.read_all().await?;
         let mut filtered = FxIndexMap::default();
         for (key, value) in &*prior {

turbopack/crates/turbo-tasks-env/src/lib.rs

@@ -17,6 +17,19 @@ pub use self::{
     filter::FilterProcessEnv,
 };
 
+/// Like [`EnvMap`], but with `serialization = "none"` to avoid storing
+/// environment variables (which may contain secrets) in the persistent cache.
+#[turbo_tasks::value(transparent, serialization = "none")]
+pub struct TransientEnvMap(#[turbo_tasks(trace_ignore)] FxIndexMap<RcStr, RcStr>);
+
+#[turbo_tasks::value_impl]
+impl TransientEnvMap {
+    #[turbo_tasks::function]
+    pub fn empty() -> Vc<Self> {
+        TransientEnvMap(FxIndexMap::default()).cell()
+    }
+}
+
 #[turbo_tasks::value(transparent)]
 pub struct EnvMap(
     #[turbo_tasks(trace_ignore)]
@@ -35,26 +48,21 @@ impl EnvMap {
 #[turbo_tasks::value_impl]
 impl ProcessEnv for EnvMap {
     #[turbo_tasks::function]
-    fn read_all(self: Vc<Self>) -> Vc<EnvMap> {
-        self
+    async fn read_all(self: Vc<Self>) -> Result<Vc<TransientEnvMap>> {
+        Ok(Vc::cell((*self.await?).clone()))
     }
 
     #[turbo_tasks::function]
     fn read(self: Vc<Self>, name: RcStr) -> Vc<Option<RcStr>> {
-        case_insensitive_read(self, name)
+        case_insensitive_read(self.read_all(), name)
     }
 }
 
 #[turbo_tasks::value_trait]
 pub trait ProcessEnv {
-    // TODO SECURITY: From security perspective it's not good that we read *all* env
-    // vars into the cache. This might store secrects into the filesystem cache
-    // which we want to avoid.
-    // Instead we should use only `read_prefix` to read all env vars with a specific
-    // prefix.
     /// Reads all env variables into a Map
     #[turbo_tasks::function]
-    fn read_all(self: Vc<Self>) -> Vc<EnvMap>;
+    fn read_all(self: Vc<Self>) -> Vc<TransientEnvMap>;
 
     /// Reads a single env variable. Ignores casing.
     #[turbo_tasks::function]
@@ -72,7 +80,10 @@ pub fn sorted_env_vars() -> FxIndexMap<RcStr, RcStr> {
 }
 
 #[turbo_tasks::function]
-pub async fn case_insensitive_read(map: Vc<EnvMap>, name: RcStr) -> Result<Vc<Option<RcStr>>> {
+pub async fn case_insensitive_read(
+    map: Vc<TransientEnvMap>,
+    name: RcStr,
+) -> Result<Vc<Option<RcStr>>> {
     Ok(Vc::cell(
         to_uppercase_map(map)
             .await?
@@ -82,7 +93,7 @@ pub async fn case_insensitive_read(map: Vc<EnvMap>, name: RcStr) -> Result<Vc<Op
 }
 
 #[turbo_tasks::function]
-async fn to_uppercase_map(map: Vc<EnvMap>) -> Result<Vc<EnvMap>> {
+async fn to_uppercase_map(map: Vc<TransientEnvMap>) -> Result<Vc<TransientEnvMap>> {
     let map = &*map.await?;
     let mut new = FxIndexMap::with_capacity_and_hasher(map.len(), Default::default());
     for (k, v) in map {

turbopack/crates/turbopack-env/src/embeddable.rs

@@ -1,7 +1,7 @@
 use anyhow::Result;
 use turbo_rcstr::RcStr;
 use turbo_tasks::{ResolvedVc, Vc};
-use turbo_tasks_env::{EnvMap, ProcessEnv};
+use turbo_tasks_env::{ProcessEnv, TransientEnvMap};
 use turbopack_ecmascript::utils::StringifyJs;
 
 /// Encodes values as JS strings so that they can be safely injected into a JS
@@ -22,7 +22,7 @@ impl EmbeddableProcessEnv {
 #[turbo_tasks::value_impl]
 impl ProcessEnv for EmbeddableProcessEnv {
     #[turbo_tasks::function]
-    async fn read_all(&self) -> Result<Vc<EnvMap>> {
+    async fn read_all(&self) -> Result<Vc<TransientEnvMap>> {
         let prior = self.prior.read_all().await?;
 
         let encoded = prior

turbopack/crates/turbopack-env/src/try_env.rs

@@ -1,6 +1,6 @@
 use anyhow::Result;
 use turbo_tasks::{ResolvedVc, Vc};
-use turbo_tasks_env::{DotenvProcessEnv, EnvMap, ProcessEnv};
+use turbo_tasks_env::{DotenvProcessEnv, ProcessEnv, TransientEnvMap};
 use turbo_tasks_fs::FileSystemPath;
 use turbopack_core::issue::{IssueExt, StyledString};
 
@@ -35,7 +35,7 @@ impl TryDotenvProcessEnv {
 #[turbo_tasks::value_impl]
 impl ProcessEnv for TryDotenvProcessEnv {
     #[turbo_tasks::function]
-    async fn read_all(&self) -> Result<Vc<EnvMap>> {
+    async fn read_all(&self) -> Result<Vc<TransientEnvMap>> {
         let dotenv = self.dotenv;
         let prior = dotenv.read_prior();