fix(core,cli,mcp): repository .env is a credential sidecar, never environment (ADR-062) by tachyon-beep · Pull Request #155 · foundryside-dev/loomweave

tachyon-beep · 2026-09-01T19:25:17Z

Closes the class behind Codex #143 / #149 (the session hook handing a loaded .env to its analyze child).

Problem

serve — launched automatically by agent harnesses on any checkout the operator opens — loaded <cwd>/.env into its process environment and then:

spawned loomweave analyze (analyze_start), the linked-worktree bootstrap, git, and the Filigree / Warpline MCP launchers with that environment;
read LOOMWEAVE_FILIGREE_MCP_COMMAND / LOOMWEAVE_WARPLINE_MCP_COMMAND from it to decide which program to execute.

dotenvy only sets variables that are unset, so exactly the normally-unset ones — those overrides, LD_PRELOAD, PYTHONPATH, HTTPS_PROXY — were attacker-fillable. A committed .env chose the program serve ran.

Fix (ADR-062)

loomweave_core::dotenv::{load_sidecar, var}: .env is parsed once into a private OnceLock<HashMap>; var(name) returns the process value if set, else the sidecar value. Nothing is ever set_var'd.
Every config-named credential lookup (api_key_env, token_env, identity_token_env, the select_provider_with_env / build_embedding_provider / resolve_filigree_url_with_roots / validate_auth_trust closures in serve, doctor, config, guidance, sarif, http_read, and the MCP server) calls dotenv::var. init_tracing builds its filter from var("RUST_LOG"), so a .env-supplied RUST_LOG still works.
Launcher/path overrides stay on std::env::var on purpose and can no longer be supplied by a checkout. Children inherit nothing from .env.
The command exclusion list (analyze, worktree analyze, hook, editor-spawning guidance) survives as defence in depth with its rationale rewritten.
dotenvy moves from loomweave-cli to loomweave-core.

Behaviour change

Variables that only third-party code read from .env (HTTPS_PROXY, SSL_CERT_FILE, …) must now be exported in the shell or the MCP server env block. Recorded in ADR-062 and the 1.6.1 changelog.

Tests

New core test: .env naming LOOMWEAVE_FILIGREE_MCP_COMMAND and LD_PRELOAD → visible to var, absent from std::env::var_os, absent in a spawned sh child; real env wins; idempotent load.
Existing CLI integration tests dotenv_in_cwd_is_loaded_before_tracing_setup and explicit_env_var_wins_over_dotenv pass unchanged.

Gates run locally: fmt, clippy -D warnings (core + cli + mcp), nextest (1554 across the three crates, bins built first), cargo doc -D warnings, cargo deny.

🤖 Generated with Claude Code

…ironment (ADR-062) `serve` — launched automatically by agent harnesses on any checkout the operator opens — loaded `<cwd>/.env` into its process environment, then (a) spawned `loomweave analyze` (analyze_start), the linked-worktree bootstrap, `git`, and the Filigree/Warpline MCP launchers with that environment, and (b) read LOOMWEAVE_FILIGREE_MCP_COMMAND / LOOMWEAVE_WARPLINE_MCP_COMMAND from it to decide which program to execute. dotenvy only sets variables that are unset, so exactly the normally-unset ones (those overrides, LD_PRELOAD, PYTHONPATH, HTTPS_PROXY) were attacker-fillable: a committed .env chose the program serve ran. #149 closed the hook's instance of this; this closes the class. `.env` is now parsed once into a private map (loomweave_core::dotenv) that only Loomweave's own config-named credential lookups consult — provider api_key_env, Filigree token_env / identity_token_env, RUST_LOG — and the real environment always wins. It never enters the process environment, so no child inherits it and no std::env::var launcher override can be supplied by a checkout, by construction rather than by per-site scrubbing (remove_var is unsafe in edition 2024; the workspace denies unsafe). Behaviour change: variables that only third-party code read from .env (HTTPS_PROXY, SSL_CERT_FILE, …) must now be exported in the shell or the MCP server env block. Pinned by a core test that writes a .env naming a launcher override and LD_PRELOAD, loads the sidecar, and asserts the values are visible to var() but absent from the process environment and from a spawned child; the existing CLI integration tests for .env-supplied RUST_LOG and never-clobber precedence pass unchanged. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0183ccEmYnCh5Nx2Y6weYfHy

chatgpt-codex-connector · 2026-09-01T19:25:20Z

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

Copilot

Pull request overview

This PR hardens Loomweave’s trust boundary around repository-controlled .env files by converting .env from a process environment mutator into a read-only “credential sidecar” consulted only for Loomweave config-named secrets (and RUST_LOG), preventing repo checkouts from influencing child-process environments or launcher overrides.

Changes:

Introduces loomweave_core::dotenv with load_sidecar() + var() and routes credential lookups through it instead of std::env::var.
Updates CLI tracing initialization to honor .env-provided RUST_LOG via the sidecar without ever calling set_var.
Moves the dotenvy dependency from loomweave-cli to loomweave-core and updates documentation/ADR inventory accordingly.

Reviewed changes

Copilot reviewed 16 out of 17 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
docs/loomweave/adr/README.md	Adds ADR-062 to the ADR index.
docs/loomweave/adr/ADR-062-dotenv-credential-sidecar.md	New ADR documenting the sidecar-only dotenv posture and consequences.
crates/loomweave-mcp/src/tools/status.rs	Switches env lookup closure to `loomweave_core::dotenv::var`.
crates/loomweave-mcp/src/lib.rs	Routes provider/key presence checks through `loomweave_core::dotenv::var`.
crates/loomweave-core/src/lib.rs	Exposes new `dotenv` module from `loomweave-core`.
crates/loomweave-core/src/dotenv.rs	Implements sidecar parsing, lookup rules, and a core regression test.
crates/loomweave-core/Cargo.toml	Adds `dotenvy` dependency to `loomweave-core`.
crates/loomweave-cli/src/serve.rs	Routes provider/token lookups through `loomweave_core::dotenv::var`.
crates/loomweave-cli/src/sarif.rs	Uses `loomweave_core::dotenv::var` for Filigree client credential resolution.
crates/loomweave-cli/src/main.rs	Loads sidecar (not env) pre-tracing; builds `EnvFilter` from sidecar-aware `RUST_LOG`.
crates/loomweave-cli/src/http_read.rs	Routes env lookup closure through `loomweave_core::dotenv::var`.
crates/loomweave-cli/src/guidance.rs	Uses `loomweave_core::dotenv::var` for Filigree client credential resolution.
crates/loomweave-cli/src/doctor.rs	Routes env checks/closures through `loomweave_core::dotenv::var`.
crates/loomweave-cli/src/config.rs	Routes provider/key presence checks through `loomweave_core::dotenv::var`.
crates/loomweave-cli/src/analyze.rs	Routes env lookup closures through `loomweave_core::dotenv::var` (sidecar only when loaded).
crates/loomweave-cli/Cargo.toml	Removes `dotenvy` dependency from `loomweave-cli`.
Cargo.lock	Updates dependency graph to reflect `dotenvy` moving to `loomweave-core`.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+pub fn var(name: &str) -> Option<String> {
+    if let Ok(value) = std::env::var(name) {
+        return Some(value);
+    }
+    SIDECAR.get().and_then(|map| map.get(name).cloned())
+}


+    /// Sets the process CWD for the test and restores it on drop (including on
+    /// panic). The sidecar is a process-global `OnceLock`, so this module's
+    /// tests are written as ONE test: under `cargo test` they would share the
+    /// binary; under nextest each test is its own process either way.
+    struct CwdGuard(std::path::PathBuf);
+    impl CwdGuard {
+        fn enter(dir: &std::path::Path) -> Self {
+            let prev = std::env::current_dir().unwrap();
+            std::env::set_current_dir(dir).unwrap();
+            Self(prev)
+        }
+    }
+    impl Drop for CwdGuard {
+        fn drop(&mut self) {
+            let _ = std::env::set_current_dir(&self.0);
+        }
+    }


Copilot AI lite review requested due to automatic review settings September 1, 2026 19:25

Copilot started reviewing on behalf of tachyon-beep September 1, 2026 19:25 View session

Copilot AI reviewed Sep 1, 2026

View reviewed changes

tachyon-beep merged commit bd5b0a5 into release/1.6.0 Sep 1, 2026
6 checks passed

This was referenced Sep 1, 2026

chore(release): bump to 1.6.1 and cut CHANGELOG 1.6.1 — 2026-09-02 #156

Merged

Release 1.6.1: sync release/1.6.0 into main #157

Merged

tachyon-beep deleted the fix/dotenv-credential-sidecar branch September 1, 2026 20:04

Navigation Menu

Uh oh!

fix(core,cli,mcp): repository .env is a credential sidecar, never environment (ADR-062) - #155

fix(core,cli,mcp): repository .env is a credential sidecar, never environment (ADR-062)#155
tachyon-beep merged 1 commit into
release/1.6.0from
fix/dotenv-credential-sidecar

tachyon-beep commented Sep 1, 2026

Uh oh!

chatgpt-codex-connector Bot commented Sep 1, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

tachyon-beep commented Sep 1, 2026

Problem

Fix (ADR-062)

Behaviour change

Tests

Uh oh!

chatgpt-codex-connector Bot commented Sep 1, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants