Remove process-environment interpolation from config

[brynary]

Two commits. The first removes a dead credential source and makes an invariant explicit; the second builds on it to drop {{ env.* }} from interpolated config entirely.

Net −1203 lines.

1. refactor(auth): remove EnvCredentialSource, make the run vault required

EnvCredentialSource resolved provider credentials from the process environment. It had no entry point of its own — it was only reachable as the None arm of an Option<Vault> in build_llm_source, configured_providers_for_start, and configured_providers_from_process_env.

That optional vault isn't a state the product can be in: every run has a server, the server always spawns workers with --storage-dir (worker_runtime.rs:84), and SqlVaultCredentialSource backs both server and CLI. The fallback only served to silently degrade credential resolution to whatever the worker process happened to have in its environment.

The vault is now required across RunOptions, StartServices, build_llm_source, tool_secrets_from_configured_sources, vault_token_lookup, and the CLI GitHub helpers — so the invariant is enforced by types rather than assumed. A worker spawned without --storage-dir fails with a clear message instead of quietly continuing without a vault.

Also deleted: configured_providers_from_process_env (no callers anywhere) and AgentApiBackend::new_from_env (public, only its own tests called it). Test-only credential sources moved to a feature-gated fabro_auth::test_support.

Making the vault required surfaced a real gap: four CLI tests were spawning __run-worker without --storage-dir, so those runs resolved credentials from the process environment rather than a vault. I fixed the harness to match what the server does, rather than adding a storage-dir inference fallback to production — attach.rs's infer_storage_dir is #[cfg(test)], and a second silent resolution path would work against the point.

2. refactor(config): stop resolving {{ env.* }}

The process environment is no longer a configuration source. {{ vars.NAME }} (non-sensitive, server-stored) and {{ secrets.NAME }} (vault-backed) cover both cases, and reading the worker's ambient environment made a run's inputs depend on how its process happened to be launched.

Design call worth reviewing: Namespace::Env is kept but wired to nothing. Deleting the variant would make {{ env.X }} silently become literal text everywhere — the worst outcome. Instead it fails with a message naming the replacement:

{{ env.API_KEY }} is not supported: the process environment is not a configuration
source. Use {{ vars.API_KEY }} for a non-sensitive value (`fabro variable set`) or
{{ secrets.API_KEY }} for a credential (`fabro secret set`)

ResolveCtx::with_env is gone, so no call site can opt back in.

Both remaining interpolation warts were env-only and went with it

• InterpString::resolve_or_source — the "fall back to the raw template source on failure" path that let an unresolved token reach a sandbox or the GitHub API as literal {{ ... }} text. Its own comment said it was slated for hard-error semantics.
• RunEnvironmentSettings::resolve_env's matching source fallback.

Both carried #[expect(clippy::disallowed_methods)] escape hatches. Every run-boundary resolver now fails closed: sandbox env, prepare steps, MCP transports, GitHub permissions, Slack channels, run goal files, provider extra_headers.

Hooks

Lost allowed_env_vars, resolve_header, HeaderResolveError, and the E: Env generic threaded through the executor (~335 lines net). They keep {{ vars.* }} — RunSettings::substitute_variables already substitutes it server-side at run creation, so hooks aren't left without interpolation and no vault plumbing was needed.

Breaking changes to review

• {{ env.* }} in any interpolated config field now errors instead of resolving. This is the intended behavior change.
• allowed_env_vars is removed from fabro-api.yaml and the generated TypeScript client — a wire-contract change.
• Config values that previously fell back to source form on a missing env var now fail closed.

Incidental finds

• The docs showed bucket = "{{ env.SLATEDB_BUCKET }}" under [server.slatedb.s3], but that field is a plain String and never interpolated — the example was already wrong. Now a literal.
• Two MCP transport tests became exact duplicates of the pre-existing *_secret_tokens versions once env was gone, so they're deleted rather than kept as near-identical pairs.

Testing

• 7386 tests pass; clippy -D warnings and rustfmt clean; bun run typecheck clean.
• The 13 fabro-web test failures are pre-existing — confirmed the identical count on main with these changes stashed.

Note

Conflicts with #664 ({{ goal }} / inputs / vars in command node scripts). Both touch interp.rs, and #664 adds a Namespace::Goal arm to the same Display match edited here. Whichever lands second needs a rebase.

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR removes process-environment ({{ env.* }}) interpolation from configuration and eliminates the env-only credential fallback path, enforcing “vault is always present” as a type-level invariant across the workflow runtime, CLI worker, and server. It updates interpolation semantics to fail closed, removes the hooks header env allowlist from the wire contract, and aligns tests/docs with the new model.

Changes:

• Require a vault throughout run execution paths (server/worker/CLI), removing EnvCredentialSource and env-fallback credential resolution.
• Drop {{ env.* }} from InterpString resolution entirely (kept parseable for better errors), and remove “fallback to source” behavior so unresolved tokens fail closed.
• Remove hook header allowed_env_vars from OpenAPI + generated clients, and refactor hooks/MCP/prepare/env resolution to secrets-only at run boundaries.

Reviewed changes

Copilot reviewed 50 out of 50 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
lib/packages/fabro-api-client/src/models/prepared-step.ts	Update model doc for secrets resolution
lib/packages/fabro-api-client/src/models/hook-definition.ts	Remove allowed_env_vars field
lib/foundation/fabro-types/src/settings/run.rs	Make env/secret resolution fail-closed; remove allowlist
lib/foundation/fabro-types/src/settings/interp.rs	Remove env lookup wiring; env token becomes always-unavailable
lib/foundation/fabro-config/src/run.rs	Remove env interpolation from goal-file paths
lib/foundation/fabro-config/src/resolve/run.rs	Stop threading allowed_env_vars into resolved hooks
lib/foundation/fabro-config/src/resolve/mod.rs	Update hook resolve test expectations formatting
lib/foundation/fabro-config/src/layers/run.rs	Remove hook allowed_env_vars from config layer
lib/foundation/fabro-auth/src/test_support.rs	Add feature-gated test credential helpers
lib/foundation/fabro-auth/src/resolve.rs	Remove env-only provider config path; secrets-only headers
lib/foundation/fabro-auth/src/lib.rs	Gate test_support; stop exporting env source helpers
lib/foundation/fabro-auth/src/env_source.rs	Delete EnvCredentialSource implementation
lib/foundation/fabro-auth/Cargo.toml	Add test-support feature flag
lib/components/fabro-workflow/tests/it/integration.rs	Switch tests to auth test_support sources
lib/components/fabro-workflow/src/test_support.rs	Default test LLM source uses vault-only helper
lib/components/fabro-workflow/src/pipeline/types.rs	Make vault required in init options
lib/components/fabro-workflow/src/pipeline/pull_request.rs	Update tests to vault-only credential sources
lib/components/fabro-workflow/src/pipeline/initialize.rs	Require vault; remove env-backed tool secret fallback
lib/components/fabro-workflow/src/pipeline/finalize.rs	Update tests to vault-only credential sources
lib/components/fabro-workflow/src/pipeline/execute/tests.rs	Ensure tests provide an empty vault
lib/components/fabro-workflow/src/operations/start.rs	Require vault; remove env interpolation from runtime resolvers
lib/components/fabro-workflow/src/lifecycle/git.rs	Update tests to vault-only credential sources
lib/components/fabro-workflow/src/handler/llm/api.rs	Remove new_from_env; switch tests to test_support
lib/components/fabro-workflow/Cargo.toml	Wire workflow test-support to auth test-support
lib/components/fabro-sandbox/src/from_environment.rs	Preflight docker env now preserves token source strings
lib/components/fabro-hooks/tests/host_command_hooks.rs	Use vault-only test credential source
lib/components/fabro-hooks/src/runner.rs	Use vault-only test LLM source in test helper constructor
lib/components/fabro-hooks/src/executor.rs	Remove env-based hook interpolation; fail closed on tokens
lib/components/fabro-hooks/Cargo.toml	Enable fabro-auth/test-support for tests
lib/components/fabro-agent/tests/it/parity_matrix.rs	Replace env-backed source with stub test source
lib/components/fabro-agent/Cargo.toml	Enable fabro-auth/test-support for tests
lib/apps/fabro-server/tests/it/scenario/run_completion.rs	Use test support env-credential source helper
lib/apps/fabro-server/src/server/tests.rs	Remove env-interpolated slack channel usage in test
lib/apps/fabro-server/src/server.rs	Remove env usage for GitHub perms; require vault in start services
lib/apps/fabro-server/src/run_manifest.rs	Remove env usage for GitHub perms in preflight
lib/apps/fabro-server/Cargo.toml	Enable fabro-auth/test-support for tests
lib/apps/fabro-cli/tests/it/cmd/runner.rs	Ensure worker tests pass --storage-dir
lib/apps/fabro-cli/src/shared/github.rs	Require vault reference; keep env fallback for GitHub tokens
lib/apps/fabro-cli/src/commands/run/runner.rs	Make worker vault loading mandatory; remove env-based perms
lib/apps/fabro-cli/src/commands/exec.rs	Stop resolving MCP {{ env.* }} in standalone exec
docs/public/workflows/variables.mdx	Remove mention of env in templates guidance
docs/public/reference/user-configuration.mdx	Update provider header interpolation docs to secrets-only
docs/public/integrations/slack.mdx	Update slack channel interpolation docs to vars-only
docs/public/execution/run-configuration.mdx	Update run-boundary interpolation docs to vars+secrets
docs/public/execution/environments.mdx	Remove env interpolation docs; clarify env unsupported
docs/public/core-concepts/models.mdx	Update provider extra_headers docs to secrets-only
docs/public/api-reference/fabro-api.yaml	Remove hook allowed_env_vars; adjust prepared-step wording
docs/public/agents/mcp.mdx	Remove {{ env.* }} row; clarify exec secrets behavior
docs/public/agents/hooks.mdx	Update hooks http header guidance to vars-only
docs/public/administration/server-configuration.mdx	Remove env interpolation from server config example

Comments suppressed due to low confidence (4)

lib/components/fabro-hooks/src/executor.rs:293

• This log message still says "env resolution failed", but hooks no longer wire env lookups. Updating the wording will avoid confusion when debugging interpolation failures.

This issue also appears on line 353 of the same file.

        let (prompt, model) = match Self::resolve_prompt_and_model(prompt, model) {
            Ok(resolved) => resolved,
            Err(error) => {
                tracing::error!(error = %error, "prompt hook env resolution failed, not firing");
                return HookDecision::Block {
                    reason: Some(error.to_string()),

lib/components/fabro-hooks/src/executor.rs:358

• This log message still says "env resolution failed", but hooks no longer wire env lookups. Updating the wording will avoid confusion when debugging interpolation failures.

        let (prompt, model) = match Self::resolve_prompt_and_model(prompt, model) {
            Ok(resolved) => resolved,
            Err(error) => {
                tracing::error!(error = %error, "agent hook env resolution failed, not firing");
                return HookDecision::Block {
                    reason: Some(error.to_string()),

lib/components/fabro-hooks/src/executor.rs:532

• The HTTP hook logs refer to "env resolution", but failures here are general interpolation/token resolution failures (env/secrets/inputs tokens are all unavailable at fire time). Renaming these messages will make logs accurate.

This issue also appears on line 522 of the same file.

        let resolved_url = match resolve_interp(url) {
            Ok(url) => url,
            Err(error) => {
                tracing::error!(
                    url_source = %safe_url_source_for_log(url),
                    error = %error,
                    "HTTP hook URL env resolution failed, not firing"
                );
                return HookDecision::Block {
                    reason: Some(error.to_string()),
                };
            }
        };

        // Enforce URL scheme based on TLS mode
        match tls {
            TlsMode::Verify | TlsMode::NoVerify => {
                if !resolved_url.starts_with("https://") {
                    return HookDecision::Block {
                        reason: Some(format!(
                            "HTTP hook URL must use https:// (tls mode is {tls:?})"
                        )),
                    };
                }
            }
            TlsMode::Off => {}
        }

        let mut request = client.post(&resolved_url).timeout(timeout).json(context);

        if let Some(hdrs) = headers {
            for (key, value) in hdrs {
                let interpolated = match resolve_interp(value) {
                    Ok(rendered) => rendered,
                    Err(error) => {
                        tracing::error!(
                            url_source = %safe_url_source_for_log(url),
                            header = %key,
                            error = %error,
                            "HTTP hook header env resolution failed, not firing"
                        );

lib/components/fabro-hooks/src/executor.rs:535

• The HTTP hook logs refer to "env resolution", but failures here are general interpolation/token resolution failures (env/secrets/inputs tokens are all unavailable at fire time). Renaming these messages will make logs accurate.

        if let Some(hdrs) = headers {
            for (key, value) in hdrs {
                let interpolated = match resolve_interp(value) {
                    Ok(rendered) => rendered,
                    Err(error) => {
                        tracing::error!(
                            url_source = %safe_url_source_for_log(url),
                            header = %key,
                            error = %error,
                            "HTTP hook header env resolution failed, not firing"
                        );
                        return HookDecision::Block {
                            reason: Some(error.to_string()),
                        };

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[brynary]

Review cleanup is pushed in 6226c8c51.

Changes:

• Restored the documented SDK EnvCredentialSource facade without restoring any worker or run fallback to ambient environment credentials. Live E2E tests now use real provider credentials again.
• Made unresolved GitHub permissions fail preflight and in-process execution instead of silently becoming an empty permission map.
• Made worker --storage-dir required at the CLI boundary and removed repeated test command setup.
• Renamed late interpolation helpers from env to secrets and corrected stale comments, public docs, OpenAPI text, generated TypeScript comments, and generated configuration reference content.
• Removed the shared no-op credential source that only masked live test setup.

I did not apply the efficiency suggestion to replace full LLM client construction with configured_providers. Client construction also validates adapter readiness. A credential-only check could select a provider whose adapter cannot initialize, which would change routing behavior.

Validation:

• cargo build --workspace
• cargo +nightly-2026-04-14 fmt --check --all
• cargo +nightly-2026-04-14 clippy --workspace --all-targets -- -D warnings
• cargo nextest run --workspace --no-fail-fast: 7,389 passed, 200 skipped
• Final affected-crate rerun: 1,873 passed, 124 skipped; final fabro-auth rerun: 42 passed
• cargo dev docs check
• bun run typecheck in apps/fabro-web
• TypeScript API client regeneration completed from the updated OpenAPI spec
• No pending insta snapshots

[Copilot]

Pull request overview

Copilot reviewed 57 out of 57 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (3)

docs/public/reference/user-configuration.mdx:188

• The examples use two different secret names for the same Portkey API key ("{{ secrets.PORTKEY_API_KEY }}" above vs "{{ secrets.portkey_api_key }}" here). Since secret keys are case-sensitive, this is likely to confuse users and lead to unresolved-token errors. Consider using one canonical name consistently (e.g. PORTKEY_API_KEY) or explicitly calling out that the name is arbitrary.

[llm.providers.proxy.extra_headers]
x-portkey-api-key = "{{ secrets.portkey_api_key }}"
x-portkey-config = "@bedrock-prod"

lib/foundation/fabro-dev/src/commands/docs_options_reference.rs:235

• This generated docs snippet uses a lowercase secret name ("{{ secrets.portkey_api_key }}"), while other docs/examples use "{{ secrets.PORTKEY_API_KEY }}". Because secret keys are case-sensitive, this inconsistency can mislead users. Consider standardizing on a single example name.
  docs/public/administration/server-configuration.mdx:222
• The example S3 bucket name is set to a specific value ("fabro-production"), which reads like a real/internal bucket rather than a placeholder. For public docs, it’s safer/clearer to use an obviously user-supplied placeholder name.

[server.slatedb.s3]
bucket = "fabro-production"
region = "us-east-1"

[brynary]

Follow-up: current origin/main is merged in 9d828a868 with a normal merge commit. The only content conflict was a stale workflow pull-request test helper; I resolved it against the current main test structure.

Post-merge validation:

• cargo build --locked --workspace
• cargo +nightly-2026-04-14 fmt --check --all
• cargo +nightly-2026-04-14 clippy --locked --workspace --all-targets -- -D warnings
• Rust CI-profile suite: 7,497 passed, 200 skipped
• Twin ignored shard: 118 passed
• Web suite: 785 passed
• Both TypeScript type checks passed
• Generated docs and insta snapshot checks passed

[Copilot]

Pull request overview

Copilot reviewed 56 out of 56 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

docs/public/reference/user-configuration.mdx:187

• This doc uses two different secret keys for the same Portkey header (PORTKEY_API_KEY earlier in the file vs portkey_api_key here). Since secret keys are case-sensitive, this is likely to confuse users copying the example. Consider using a single key consistently (e.g. PORTKEY_API_KEY).

x-portkey-api-key = "{{ secrets.portkey_api_key }}"

lib/foundation/fabro-dev/src/commands/docs_options_reference.rs:234

• The reference example uses {{ secrets.portkey_api_key }}, but other docs/examples use {{ secrets.PORTKEY_API_KEY }} for the Portkey API key. Since secret keys are case-sensitive, it’s better to keep one canonical key name across the docs to avoid copy/paste failures.

[brynary]

Final base update: PR #664 landed while the previous merge was validating, so current origin/main is now merged in 00228383d.

The expected settings/interp.rs conflict is resolved by keeping #664 support for scoped {{ inputs.* }} and {{ goal }} values while retaining #665 behavior: {{ secrets.* }} stays late-bound and {{ env.* }} remains unsupported with the migration error.

Validation on this final tree:

• Affected fabro-types and workflow suites: 1,681 passed
• Full Rust CI-profile suite: 7,529 passed, 200 skipped
• Twin ignored shard: 118 passed
• Workspace Clippy, formatting, generated docs, and snapshot checks passed

[Copilot]

Pull request overview

Copilot reviewed 56 out of 56 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

docs/public/reference/user-configuration.mdx:187

• This page uses two different secret names for the same Portkey API key ({{ secrets.PORTKEY_API_KEY }} earlier vs {{ secrets.portkey_api_key }} here). Secret names are case-sensitive, so this inconsistency can cause copy/paste configs to fail at runtime. Consider standardizing on the same secret name (e.g. PORTKEY_API_KEY, matching the common env var name).

x-portkey-api-key = "{{ secrets.portkey_api_key }}"

lib/foundation/fabro-dev/src/commands/docs_options_reference.rs:234

• The generated options reference uses {{ secrets.portkey_api_key }} here, but another snippet in this same file uses {{ secrets.PORTKEY_API_KEY }}. Since secret keys are case-sensitive, the docs should consistently use one name to avoid confusing users.

[Copilot]

Pull request overview

Copilot reviewed 56 out of 56 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

docs/public/reference/user-configuration.mdx:188

• The secret name used for x-portkey-api-key is inconsistent within this page ({{ secrets.PORTKEY_API_KEY }} earlier vs {{ secrets.portkey_api_key }} here). Since secret identifiers are case-sensitive in most systems, this inconsistency can confuse users and lead to unresolved-token errors. Prefer one canonical name across examples (e.g., PORTKEY_API_KEY to match the credential ref style).

[llm.providers.proxy.extra_headers]
x-portkey-api-key = "{{ secrets.portkey_api_key }}"
x-portkey-config = "@bedrock-prod"

[Copilot]

Pull request overview

Copilot reviewed 58 out of 58 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

docs/public/reference/user-configuration.mdx:189

• The Portkey header secret name is inconsistent within this page: earlier the example uses {{ secrets.PORTKEY_API_KEY }}, but this example uses {{ secrets.portkey_api_key }}. Since secret names are case-sensitive, this is likely to confuse users and cause copy/paste misconfiguration. Please use one name consistently (preferably PORTKEY_API_KEY to match the prior snippet and env-style credential refs).

[llm.providers.proxy.extra_headers]
x-portkey-api-key = "{{ secrets.portkey_api_key }}"
x-portkey-config = "@bedrock-prod"
x-team-secret = "{{ secrets.gateway_team_secret }}"

lib/foundation/fabro-dev/src/commands/docs_options_reference.rs:236

• This generated options-reference snippet uses {{ secrets.portkey_api_key }} while other docs/examples use {{ secrets.PORTKEY_API_KEY }}. Since secret names are case-sensitive, please align the example with the canonical name used elsewhere to avoid copy/paste failures.