mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-08-28 05:27:41 +00:00
refactor(config): stop resolving {{ env.* }} in interpolated config
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.
`Namespace::Env` is kept but wired to nothing, so `{{ env.NAME }}` still
parses and fails with a message naming its replacement rather than reaching
a consumer as literal text. `ResolveCtx::with_env` is gone, so no call site
can opt back in.
Two long-standing warts were env-only and go with it:
- `InterpString::resolve_or_source`, the "fall back to the raw template
source on failure" path, which let an unresolved token reach a sandbox or
the GitHub API as literal `{{ ... }}` text. Its own comment noted it was
slated for hard-error semantics.
- `RunEnvironmentSettings::resolve_env`'s matching source fallback for
env-only values.
Both carried `#[expect(clippy::disallowed_methods)]` escape hatches. Every
run-boundary resolver — sandbox env, prepare steps, MCP transports, GitHub
permissions, Slack channels, run goal files, provider extra_headers — now
fails closed instead.
Hooks lose their `allowed_env_vars` allowlist, `resolve_header`, and
`HeaderResolveError` along with the `E: Env` generic threaded through the
executor. They keep `{{ vars.* }}`, which `RunSettings::substitute_variables`
already substitutes server-side at run creation.
`allowed_env_vars` is removed from the OpenAPI spec and the generated
TypeScript client. The docs example showing `{{ env.* }}` in
`[server.slatedb.s3].bucket` was already wrong — that field is a plain
String and never interpolated — and is now a literal.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Bryan Helmkamp
parent
e54fef760a
commit
f0a7423b51
27 changed files with 369 additions and 788 deletions
|
|
@ -218,7 +218,7 @@ provider = "s3"
|
|||
disk_cache = true
|
||||
|
||||
[server.slatedb.s3]
|
||||
bucket = "{{ env.SLATEDB_BUCKET }}"
|
||||
bucket = "fabro-production"
|
||||
region = "us-east-1"
|
||||
```
|
||||
|
||||
|
|
@ -463,7 +463,7 @@ GitHub App mode stores these secrets in the vault. `fabro install` writes them a
|
|||
|
||||
### Slack integration (optional)
|
||||
|
||||
Slack credentials are server-level secrets. Add `[server.integrations.slack]` to enable one Slack connection that is shared by human interview prompts and run lifecycle notifications. `server.integrations.slack.default_channel` is an optional literal channel name used only as the default destination for interview prompts; it does not interpolate `{{ env.* }}`. Lifecycle notifications use `[run.notifications.<name>.slack].channel` in run or workflow configuration.
|
||||
Slack credentials are server-level secrets. Add `[server.integrations.slack]` to enable one Slack connection that is shared by human interview prompts and run lifecycle notifications. `server.integrations.slack.default_channel` is an optional literal channel name used only as the default destination for interview prompts; it does not interpolate. Lifecycle notifications use `[run.notifications.<name>.slack].channel` in run or workflow configuration.
|
||||
|
||||
Fabro resolves these from the vault only. When `[server.integrations.slack]` is present and both credentials are present, startup logs `Slack integration enabled` and then the Slack Socket Mode connection status. If the Slack config table is absent or `enabled = false`, startup logs `Slack integration disabled by server configuration`. If the table is present but either credential is missing or empty, startup logs `Slack integration disabled; missing credentials` with the missing variable names.
|
||||
|
||||
|
|
|
|||
|
|
@ -28,19 +28,19 @@ POST the event context as JSON to an HTTP endpoint. Useful for webhooks, externa
|
|||
event = "run_complete"
|
||||
type = "http"
|
||||
url = "https://hooks.example.com/done"
|
||||
allowed_env_vars = ["API_KEY"]
|
||||
|
||||
[hooks.headers]
|
||||
Authorization = "Bearer {{ env.API_KEY }}"
|
||||
Authorization = "Bearer {{ vars.WEBHOOK_TOKEN }}"
|
||||
```
|
||||
|
||||
| Field | Description |
|
||||
|---|---|
|
||||
| `url` | The endpoint to POST to. Must use `https://` unless `tls = "off"`. Supports `{{ env.NAME }}` interpolation. |
|
||||
| `headers` | Optional HTTP headers. Values support `{{ env.NAME }}` interpolation, scoped to the names in `allowed_env_vars`. A token for any other env var fails to resolve and the hook blocks (fail-closed). |
|
||||
| `allowed_env_vars` | Allowlist of environment variable names a header may read via `{{ env.NAME }}`. Empty (the default) means no env vars may be interpolated into headers. |
|
||||
| `url` | The endpoint to POST to. Must use `https://` unless `tls = "off"`. Supports `{{ vars.NAME }}` interpolation. |
|
||||
| `headers` | Optional HTTP headers. Values support `{{ vars.NAME }}` interpolation. A token that is still unresolved when the hook fires blocks it (fail-closed), so a header is never sent half-rendered. |
|
||||
| `tls` | TLS mode: `"verify"` (default), `"no_verify"`, or `"off"`. |
|
||||
|
||||
`{{ vars.NAME }}` is substituted when the run is created. `{{ env.NAME }}` and `{{ secrets.NAME }}` are not available in hooks.
|
||||
|
||||
### Prompt
|
||||
|
||||
A single-turn LLM call that evaluates the event context and returns an `ok`/`block` decision. The model responds with structured JSON.
|
||||
|
|
|
|||
|
|
@ -149,12 +149,11 @@ Inline transport fields can interpolate values at the run boundary:
|
|||
| Syntax | Resolution time |
|
||||
|---|---|
|
||||
| `{{ vars.NAME }}` | When the server creates the run, using that run's variable snapshot |
|
||||
| `{{ env.NAME }}` | When the worker launches the MCP transport |
|
||||
| `{{ secrets.NAME }}` | When the worker launches the MCP transport, using a token secret from the server vault |
|
||||
|
||||
Interpolation applies to stdio and sandbox commands and env values, plus HTTP URLs and headers. Variable tokens are replaced in the created run configuration. Worker-time environment and secret expressions remain in persisted configuration, while resolved secret values do not. A missing environment variable, missing secret, or non-token secret fails MCP startup instead of passing an unresolved token to the transport.
|
||||
|
||||
Standalone `fabro exec` can resolve `{{ env.* }}` from its process environment, but it has no server vault. A `{{ secrets.* }}` reference therefore fails with an explicit error in standalone execution.
|
||||
Standalone `fabro exec` has no server vault, so a `{{ secrets.* }}` reference fails with an explicit error in standalone execution.
|
||||
|
||||
## Transports
|
||||
|
||||
|
|
|
|||
|
|
@ -14169,7 +14169,7 @@ components:
|
|||
script-vs-argv distinction via the `type` discriminator: a `script`
|
||||
is a raw shell snippet kept verbatim, while a `command` is an argv
|
||||
whose elements are shell-quoted and joined at the run boundary (after
|
||||
`{{ env.* }}` resolution) so an interpolated value cannot inject shell
|
||||
`{{ secrets.* }}` resolution) so an interpolated value cannot inject shell
|
||||
syntax. Optional per-step `env` is shared by both shapes.
|
||||
type: object
|
||||
required: [type]
|
||||
|
|
@ -14574,17 +14574,9 @@ components:
|
|||
- type: "null"
|
||||
description: >-
|
||||
Optional HTTP headers for an http hook. Values support
|
||||
`{{ env.NAME }}` interpolation, scoped to the names listed in
|
||||
`allowed_env_vars`; a token for any other env var fails to resolve
|
||||
and the hook blocks (fail-closed).
|
||||
allowed_env_vars:
|
||||
type: array
|
||||
items:
|
||||
type: string
|
||||
description: >-
|
||||
Allowlist of environment variable names that an http hook header may
|
||||
read via `{{ env.NAME }}`. An empty list (the default) permits no env
|
||||
vars in headers.
|
||||
`{{ vars.NAME }}` interpolation, substituted when the run is
|
||||
created; a token left unresolved at fire time blocks the hook
|
||||
(fail-closed).
|
||||
tls:
|
||||
$ref: "#/components/schemas/TlsMode"
|
||||
prompt:
|
||||
|
|
|
|||
|
|
@ -84,7 +84,7 @@ aliases = ["gateway"]
|
|||
credentials = ["env:ACME_GATEWAY_API_KEY", "vault:ACME_GATEWAY_API_KEY"]
|
||||
|
||||
[llm.providers.proxy.extra_headers]
|
||||
x-portkey-api-key = "{{ env.PORTKEY_API_KEY }}"
|
||||
x-portkey-api-key = "{{ secrets.PORTKEY_API_KEY }}"
|
||||
x-portkey-config = "@bedrock-prod"
|
||||
|
||||
[llm.providers.proxy.models."team-code-large"]
|
||||
|
|
@ -153,7 +153,7 @@ Historical built-in catalog keys that exposed provider API IDs remain accepted a
|
|||
|
||||
Model roles are separate: `default = true` controls normal model selection for workflow execution, while `small_default = true` marks the provider's small/cheap utility model for metadata tasks such as generated run titles. If a provider has no small default, Fabro falls back to that provider's normal default.
|
||||
|
||||
Provider auth is declared in `[llm.providers.<id>.auth]` with ordered `env:<NAME>` or `vault:<NAME>` refs. The primary auth header defaults to `bearer`; override with `header = { custom = "Header-Name" }` for providers like Anthropic that use `x-api-key`. Omit the `[llm.providers.<id>.auth]` block entirely for providers that need no API key (e.g. Ollama). Custom headers for any provider — including providers that need only interpolation headers and no API-key auth — go in `extra_headers` as literal text, `{{ env.NAME }}` tokens, or `{{ secrets.NAME }}` tokens. Put credentials in secrets and reference them with `{{ secrets.NAME }}` instead of a bare literal.
|
||||
Provider auth is declared in `[llm.providers.<id>.auth]` with ordered `env:<NAME>` or `vault:<NAME>` refs. The primary auth header defaults to `bearer`; override with `header = { custom = "Header-Name" }` for providers like Anthropic that use `x-api-key`. Omit the `[llm.providers.<id>.auth]` block entirely for providers that need no API key (e.g. Ollama). Custom headers for any provider — including providers that need only interpolation headers and no API-key auth — go in `extra_headers` as literal text or `{{ secrets.NAME }}` tokens. Put credentials in secrets and reference them with `{{ secrets.NAME }}` instead of a bare literal.
|
||||
|
||||
Workflow runs also add `x-session-id: <run-id>` to every LLM request so compatible gateways can group requests from the same run. An explicitly configured `x-session-id` in provider `extra_headers` takes precedence.
|
||||
|
||||
|
|
|
|||
|
|
@ -152,16 +152,16 @@ preserve = true
|
|||
|
||||
## Environment value interpolation
|
||||
|
||||
Environment `env` values can mix literal text with `{{ vars.NAME }}`, `{{ env.NAME }}`, and `{{ secrets.NAME }}` tokens:
|
||||
Environment `env` values can mix literal text with `{{ vars.NAME }}` and `{{ secrets.NAME }}` tokens:
|
||||
|
||||
```toml title="workflow.toml"
|
||||
[environments.fabro-dev.env]
|
||||
DEPLOY_ENV = "{{ vars.DEPLOY_ENV }}"
|
||||
SERVICE_URL = "https://api.{{ env.REGION }}.example.com"
|
||||
SERVICE_URL = "https://api.{{ vars.REGION }}.example.com"
|
||||
SERVICE_TOKEN = "{{ secrets.SERVICE_TOKEN }}"
|
||||
```
|
||||
|
||||
Server-managed variables resolve when the run is created. Worker environment variables and token secrets resolve immediately before the sandbox starts, so resolved secret values are not persisted in the run definition. A missing or non-token secret fails closed. For backward compatibility, a value containing only missing `{{ env.* }}` references is passed through in source form.
|
||||
Server-managed variables resolve when the run is created. Token secrets resolve immediately before the sandbox starts, so resolved secret values are not persisted in the run definition. A missing or non-token secret fails closed, as does any `{{ env.* }}` reference: the process environment is not a configuration source.
|
||||
|
||||
## Selecting an environment from the CLI
|
||||
|
||||
|
|
|
|||
|
|
@ -79,7 +79,7 @@ memory = "8GB"
|
|||
disk = "20GB"
|
||||
|
||||
[environments.cloud.env]
|
||||
API_KEY = "{{ env.MY_API_KEY }}"
|
||||
API_KEY = "{{ secrets.MY_API_KEY }}"
|
||||
NODE_ENV = "production"
|
||||
|
||||
[run.integrations.github.permissions]
|
||||
|
|
@ -192,13 +192,13 @@ env = { NPM_TOKEN = "{{ secrets.NPM_TOKEN }}" }
|
|||
|
||||
| Field | Description |
|
||||
|---|---|
|
||||
| `script` | Bash source, evaluated by the sandbox's non-login Bash (`bash -c`). Supports `{{ vars.* }}`, `{{ env.* }}`, and `{{ secrets.* }}` interpolation. |
|
||||
| `script` | Bash source, evaluated by the sandbox's non-login Bash (`bash -c`). Supports `{{ vars.* }}` and `{{ secrets.* }}` interpolation. |
|
||||
| `command` | Argv-style command, mutually exclusive with `script`. Each resolved element is shell-quoted as one argument. |
|
||||
| `env` | Additional environment variables for this step. Values support the same interpolation as `script` and `command`. |
|
||||
|
||||
Each step must exit with status 0. If any step fails, the run aborts before the workflow starts. Prepare steps replace across layers — the higher-precedence layer wins wholesale.
|
||||
|
||||
Fabro substitutes `{{ vars.* }}` when the server creates the run, then resolves `{{ env.* }}` from the worker process and `{{ secrets.* }}` from token entries in the server vault immediately before the worker executes the steps. Worker-time environment and secret expressions remain in the persisted run definition; resolved secret values are not persisted. A missing environment variable, missing secret, or non-token secret aborts startup with the affected step and token named in the error.
|
||||
Fabro substitutes `{{ vars.* }}` when the server creates the run, then resolves `{{ secrets.* }}` from token entries in the server vault immediately before the worker executes the steps. Secret expressions remain in the persisted run definition; resolved secret values are not persisted. A missing or non-token secret aborts startup with the affected step and token named in the error.
|
||||
|
||||
### `[run.clone]`
|
||||
|
||||
|
|
@ -301,7 +301,7 @@ Environment variable values can combine literal text with server variables, work
|
|||
[environments.ci.env]
|
||||
API_KEY = "{{ secrets.SERVICE_API_KEY }}"
|
||||
NODE_ENV = "production"
|
||||
SERVICE_URL = "https://api.{{ env.REGION }}.example.com"
|
||||
SERVICE_URL = "https://api.{{ vars.REGION }}.example.com"
|
||||
RELEASE_CHANNEL = "{{ vars.RELEASE_CHANNEL }}"
|
||||
```
|
||||
|
||||
|
|
@ -309,11 +309,10 @@ RELEASE_CHANNEL = "{{ vars.RELEASE_CHANNEL }}"
|
|||
|---|---|
|
||||
| `"literal"` | Static value passed as-is |
|
||||
| `"{{ vars.NAME }}"` | Server-managed variable substituted when the run is created |
|
||||
| `"{{ env.VARNAME }}"` | Worker process environment value resolved when the run starts |
|
||||
| `"{{ secrets.NAME }}"` | Token secret resolved from the server vault when the run starts |
|
||||
| `"prefix-{{ env.X }}-suffix"` | Substring interpolation; multiple supported tokens per string are allowed |
|
||||
| `"prefix-{{ vars.X }}-suffix"` | Substring interpolation; multiple supported tokens per string are allowed |
|
||||
|
||||
Missing or non-token secret references fail closed before sandbox startup. For backward compatibility, an environment value that references only a missing `{{ env.* }}` value is passed through in source form; use preflight or prepare-step interpolation when an absent worker variable must be a hard error.
|
||||
Missing or non-token secret references fail closed before sandbox startup. `{{ env.* }}` is not supported: the process environment is not a configuration source. Use `{{ vars.NAME }}` for a non-sensitive value or `{{ secrets.NAME }}` for a credential.
|
||||
|
||||
### `[run.integrations.github.permissions]`
|
||||
|
||||
|
|
@ -349,7 +348,7 @@ channel = "#deploys"
|
|||
| `enabled` | Enables this route. Defaults to `false`. |
|
||||
| `provider` | Notification provider. Use `"slack"` for Slack lifecycle notifications. Other provider names may be parsed but are not delivered by the server yet. |
|
||||
| `events` | Raw Fabro event names that trigger this route, such as `run.started`, `run.completed`, and `run.failed`. |
|
||||
| `[run.notifications.<name>.slack].channel` | Required for Slack lifecycle notifications. Literal channel names and `{{ env.VAR }}` interpolation are supported. |
|
||||
| `[run.notifications.<name>.slack].channel` | Required for Slack lifecycle notifications. Literal channel names and `{{ vars.NAME }}` interpolation are supported. |
|
||||
|
||||
Each enabled Slack route posts once for each matching lifecycle event. Messages include the run ID, an Open in Fabro link when available, workflow label, terminal result, duration, and pull request details when those are already present in the run event stream.
|
||||
|
||||
|
|
@ -489,7 +488,7 @@ id = "sentry"
|
|||
| `startup_timeout` | Max duration for server startup + MCP handshake (e.g. `"10s"`, `"1m"`). | `"10s"` |
|
||||
| `tool_timeout` | Max duration for a single tool call. | `"60s"` |
|
||||
|
||||
Inline transport commands, URLs, env values, and headers support `{{ vars.* }}`, `{{ env.* }}`, and `{{ secrets.* }}` interpolation. As with prepare steps, server variables resolve at run creation and worker env/token secrets resolve at launch; missing values fail closed. See [MCP runtime interpolation](/agents/mcp#runtime-interpolation) for the standalone `fabro exec` difference.
|
||||
Inline transport commands, URLs, env values, and headers support `{{ vars.* }}` and `{{ secrets.* }}` interpolation. As with prepare steps, server variables resolve at run creation and token secrets resolve at launch; missing values fail closed. See [MCP runtime interpolation](/agents/mcp#runtime-interpolation) for the standalone `fabro exec` difference.
|
||||
|
||||
The `sandbox` transport runs the MCP server inside the workflow's sandbox. This is useful for tools that need access to the sandbox environment, such as browser automation with Playwright. See [MCP](/agents/mcp#sandbox) for details.
|
||||
|
||||
|
|
|
|||
|
|
@ -117,7 +117,7 @@ enabled = true
|
|||
default_channel = "#fabro-reviews"
|
||||
```
|
||||
|
||||
`default_channel` is a literal channel name used only for human-in-the-loop interview prompts. Fabro does not interpolate `{{ env.* }}` in this server setting. Run lifecycle notifications use per-run or per-workflow `[run.notifications]` routes instead, whose channel values can use environment interpolation.
|
||||
`default_channel` is a literal channel name used only for human-in-the-loop interview prompts; Fabro does not interpolate it. Run lifecycle notifications use per-run or per-workflow `[run.notifications]` routes instead, whose channel values support `{{ vars.NAME }}` interpolation.
|
||||
|
||||
### 8. Invite the bot
|
||||
|
||||
|
|
@ -182,7 +182,7 @@ Each enabled route posts one message when a matching event is emitted. Lifecycle
|
|||
|
||||
`run.failed` is a terminal run event. A stage can fail and still be followed by another graph edge that lets the run complete; in that case a route listening for `run.completed` fires, not `run.failed`.
|
||||
|
||||
The route-level Slack channel is required for lifecycle notifications. The channel may be a literal (`"#deploys"`) or an environment interpolation (`"{{ env.DEPLOYS_SLACK_CHANNEL }}"`). If the channel is missing, empty, or cannot be resolved, Fabro logs a warning and skips that route without affecting the run or other notification routes.
|
||||
The route-level Slack channel is required for lifecycle notifications. The channel may be a literal (`"#deploys"`) or a server variable (`"{{ vars.DEPLOYS_SLACK_CHANNEL }}"`). If the channel is missing, empty, or cannot be resolved, Fabro logs a warning and skips that route without affecting the run or other notification routes.
|
||||
|
||||
Lifecycle notifications are one-way and fire-and-forget. They never accept answers, register reply threads, update prior messages, or interact with interview state.
|
||||
|
||||
|
|
|
|||
|
|
@ -94,7 +94,7 @@ aliases = ["gateway"]
|
|||
credentials = ["env:ACME_GATEWAY_API_KEY", "vault:ACME_GATEWAY_API_KEY"]
|
||||
|
||||
[llm.providers.proxy.extra_headers]
|
||||
x-portkey-api-key = "{{ env.PORTKEY_API_KEY }}"
|
||||
x-portkey-api-key = "{{ secrets.PORTKEY_API_KEY }}"
|
||||
x-portkey-config = "@bedrock-prod"
|
||||
|
||||
[llm.providers.proxy.models."team-code-large"]
|
||||
|
|
@ -184,7 +184,7 @@ aliases = ["gateway"]
|
|||
credentials = ["env:ACME_GATEWAY_API_KEY", "vault:ACME_GATEWAY_API_KEY"]
|
||||
|
||||
[llm.providers.proxy.extra_headers]
|
||||
x-portkey-api-key = "{{ env.PORTKEY_API_KEY }}"
|
||||
x-portkey-api-key = "{{ secrets.PORTKEY_API_KEY }}"
|
||||
x-portkey-config = "@bedrock-prod"
|
||||
x-team-secret = "{{ secrets.gateway_team_secret }}"
|
||||
```
|
||||
|
|
@ -199,7 +199,7 @@ x-team-secret = "{{ secrets.gateway_team_secret }}"
|
|||
| `auth` | table | omitted | API-key auth config. Omit the table entirely for providers that need no API key; any `extra_headers` are still attached. |
|
||||
| `auth.credentials` | array<string> | required when `auth` present | Ordered credential refs. Accepted forms are `vault:<NAME>`, `env:<NAME>`, and `aws_sigv4` (sign requests from the AWS default credential chain — Bedrock). Literal secret strings are rejected. |
|
||||
| `auth.header` | `"bearer"` or `{ custom = "Header-Name" }` | `"bearer"` | Primary API-key header policy. Omit when the provider uses a standard bearer token. |
|
||||
| `extra_headers` | table | `{}` | Additional headers attached to provider requests. Values are interpolation strings: literal text, an `{{ env.NAME }}` token, or a `{{ secrets.NAME }}` token. Put credentials in a secret and reference them with a `{{ secrets.NAME }}` token, not a bare literal. |
|
||||
| `extra_headers` | table | `{}` | Additional headers attached to provider requests. Values are interpolation strings: literal text or a `{{ secrets.NAME }}` token. Put credentials in a secret and reference them with a `{{ secrets.NAME }}` token, not a bare literal. |
|
||||
| `priority` | integer | `0` | Higher-priority ready providers win unqualified model and default selection; ties use canonical provider ID. |
|
||||
| `enabled` | boolean | `true` | Set `false` to disable a provider after lower-precedence layers define it. |
|
||||
| `aliases` | array<string> | `[]` | Additional provider names accepted by model routing and fallback config. |
|
||||
|
|
|
|||
|
|
@ -15,7 +15,7 @@ Goal templates can reference inputs and server-managed variables. Prompt templat
|
|||
| `{{ inputs.name }}` | A value from `[run.inputs]`, optionally overridden by CLI input flags |
|
||||
| `{{ vars.NAME }}` | A server-managed variable snapshotted when the run is created |
|
||||
|
||||
Environment variables and secrets are **not** available in goal or prompt templates. Use `{{ env.NAME }}` and `{{ secrets.NAME }}` only in the configuration fields that support run-boundary interpolation.
|
||||
Secrets are **not** available in goal or prompt templates. Use `{{ secrets.NAME }}` only in the configuration fields that support run-boundary interpolation.
|
||||
|
||||
## Run config inputs
|
||||
|
||||
|
|
|
|||
|
|
@ -282,14 +282,6 @@ impl ProviderAdapter for AuthenticatedFabroServerAdapter {
|
|||
}
|
||||
}
|
||||
|
||||
#[expect(
|
||||
clippy::disallowed_methods,
|
||||
reason = "exec-boundary MCP transport InterpString resolution facade for {{ env.* }} values."
|
||||
)]
|
||||
fn process_env_var(name: &str) -> Option<String> {
|
||||
std::env::var(name).ok()
|
||||
}
|
||||
|
||||
fn run_mcp_servers_for_exec(
|
||||
mcps: &HashMap<String, ResolvedMcpEntry>,
|
||||
) -> AnyResult<Vec<McpServerSettings>> {
|
||||
|
|
@ -351,7 +343,7 @@ pub(crate) async fn execute(mut args: ExecArgs, ctx: &CommandContext) -> AnyResu
|
|||
.into_iter()
|
||||
.map(|settings| {
|
||||
settings
|
||||
.resolve_transport_env(process_env_var, |_| None)
|
||||
.resolve_transport_env(|_| None)
|
||||
.with_context(|| format!("failed to resolve MCP server {:?}", settings.name))
|
||||
})
|
||||
.collect::<AnyResult<Vec<_>>>()?;
|
||||
|
|
|
|||
|
|
@ -167,7 +167,8 @@ pub(crate) async fn execute(
|
|||
.run
|
||||
.integrations
|
||||
.github
|
||||
.resolve_permissions(process_env_var),
|
||||
.resolve_permissions()
|
||||
.context("failed to resolve github permissions")?,
|
||||
vault,
|
||||
catalog,
|
||||
on_node: None,
|
||||
|
|
@ -1153,14 +1154,6 @@ fn maybe_build_github_credentials(
|
|||
Ok(None)
|
||||
}
|
||||
|
||||
#[expect(
|
||||
clippy::disallowed_methods,
|
||||
reason = "CLI worker InterpString resolution facade for {{ env.* }} values."
|
||||
)]
|
||||
fn process_env_var(name: &str) -> Option<String> {
|
||||
std::env::var(name).ok()
|
||||
}
|
||||
|
||||
/// Hard-gate for the CLI worker path: a run-level token is requested, or
|
||||
/// a clone-based sandbox in non-dry-run mode will need credentials to
|
||||
/// pull the repository. Pull-request-driven credential acquisition is
|
||||
|
|
|
|||
|
|
@ -44,7 +44,6 @@ use futures_util::stream::{self, StreamExt};
|
|||
use tokio::process::Command;
|
||||
use tokio::time;
|
||||
|
||||
use crate::interp::process_env_var;
|
||||
use crate::server::AppState;
|
||||
use crate::server_secrets::LlmClientResult;
|
||||
|
||||
|
|
@ -1214,7 +1213,8 @@ async fn run_github_token_check(
|
|||
let github_permissions = resolved_run
|
||||
.integrations
|
||||
.github
|
||||
.resolve_permissions(process_env_var);
|
||||
.resolve_permissions()
|
||||
.unwrap_or_default();
|
||||
|
||||
let perm_details = github_permissions
|
||||
.iter()
|
||||
|
|
|
|||
|
|
@ -152,7 +152,6 @@ use crate::git_checkout::GitRepoCache;
|
|||
use crate::github_webhooks::{
|
||||
WEBHOOK_ROUTE, WEBHOOK_SECRET_ENV, parse_event_metadata, verify_signature,
|
||||
};
|
||||
use crate::interp::process_env_var;
|
||||
use crate::jwt_auth::{self, AuthMode};
|
||||
use crate::principal_middleware::{
|
||||
AuthContextSlot, RequestAuth, RequestAuthContext, RequireRunBlob, RequireRunManagementTarget,
|
||||
|
|
@ -873,13 +872,8 @@ impl SlackService {
|
|||
|
||||
let blocks = &blocks;
|
||||
let posts = routes.into_iter().filter_map(|(route_name, route)| {
|
||||
let channel = resolve_slack_lifecycle_route_channel(
|
||||
state,
|
||||
event.run_id,
|
||||
route_name,
|
||||
route,
|
||||
event_name,
|
||||
)?;
|
||||
let channel =
|
||||
resolve_slack_lifecycle_route_channel(event.run_id, route_name, route, event_name)?;
|
||||
Some(async move {
|
||||
if let Err(err) = self.client.post_message(&channel, blocks, None).await {
|
||||
warn!(
|
||||
|
|
@ -1059,7 +1053,6 @@ fn slack_lifecycle_pull_request_from_link(link: &PullRequestLink) -> SlackLifecy
|
|||
}
|
||||
|
||||
fn resolve_slack_lifecycle_route_channel(
|
||||
state: &AppState,
|
||||
run_id: RunId,
|
||||
route_name: &str,
|
||||
route: &NotificationRouteSettings,
|
||||
|
|
@ -1079,7 +1072,10 @@ fn resolve_slack_lifecycle_route_channel(
|
|||
return None;
|
||||
};
|
||||
|
||||
let resolved = match channel.resolve(|name| (state.env_lookup)(name)) {
|
||||
// `{{ vars.* }}` is substituted at run creation, so the channel is literal
|
||||
// here; anything still unresolved skips the route rather than sending to a
|
||||
// half-rendered channel name.
|
||||
let resolved = match channel.resolve_with(&mut fabro_types::settings::ResolveCtx::new()) {
|
||||
Ok(resolved) => resolved,
|
||||
Err(err) => {
|
||||
warn!(
|
||||
|
|
@ -4088,7 +4084,11 @@ async fn execute_run_in_process(state: Arc<AppState>, run_id: RunId) {
|
|||
.run
|
||||
.integrations
|
||||
.github
|
||||
.resolve_permissions(process_env_var);
|
||||
.resolve_permissions()
|
||||
.unwrap_or_else(|err| {
|
||||
tracing::warn!(error = %err, "github permission interpolation failed");
|
||||
std::collections::HashMap::new()
|
||||
});
|
||||
let vault = match state.stores.vault.snapshot().await {
|
||||
Ok(vault) => vault,
|
||||
Err(err) => {
|
||||
|
|
|
|||
|
|
@ -4587,19 +4587,11 @@ async fn slack_lifecycle_missing_channel_is_skipped_without_blocking_other_route
|
|||
"100.5",
|
||||
)
|
||||
.await;
|
||||
let state = test_app_state_with_env_lookup(
|
||||
default_test_server_settings(),
|
||||
fabro_config::RunLayer::default(),
|
||||
5,
|
||||
|name| match name {
|
||||
"SLACK_ROUTE_CHANNEL" => Some("#ops".to_string()),
|
||||
_ => None,
|
||||
},
|
||||
);
|
||||
let state = test_app_state();
|
||||
let service = slack_lifecycle_service(server.base_url(), None);
|
||||
let run_id = fixtures::RUN_1;
|
||||
let settings = workflow_settings_with_run_notifications(
|
||||
r#"
|
||||
r##"
|
||||
[run.notifications.missing]
|
||||
enabled = true
|
||||
provider = "slack"
|
||||
|
|
@ -4619,8 +4611,8 @@ provider = "slack"
|
|||
events = ["run.started"]
|
||||
|
||||
[run.notifications.valid.slack]
|
||||
channel = "{{ env.SLACK_ROUTE_CHANNEL }}"
|
||||
"#,
|
||||
channel = "#ops"
|
||||
"##,
|
||||
Some("Deploy workflow"),
|
||||
);
|
||||
let run_store = create_slack_notification_run(&state, run_id, settings, "deploy", None).await;
|
||||
|
|
|
|||
|
|
@ -1,6 +1,5 @@
|
|||
use std::borrow::Cow;
|
||||
use std::collections::HashMap;
|
||||
use std::fmt;
|
||||
use std::sync::{Arc, LazyLock};
|
||||
use std::time::Instant;
|
||||
|
||||
|
|
@ -13,9 +12,7 @@ use fabro_llm::generate::{GenerateParams, generate_object};
|
|||
use fabro_llm::types::{Message, Request, ToolResult};
|
||||
use fabro_model::Catalog;
|
||||
use fabro_redact::redacted_url_for_log;
|
||||
use fabro_types::settings::interp::Namespace;
|
||||
use fabro_types::settings::{InterpString, ResolveError};
|
||||
use fabro_util::env::{Env, SystemEnv};
|
||||
use fabro_types::settings::{InterpString, ResolveCtx, ResolveError};
|
||||
use tokio::process::Command as TokioCommand;
|
||||
use tokio::time::timeout as tokio_timeout;
|
||||
use tokio_util::sync::CancellationToken;
|
||||
|
|
@ -57,27 +54,22 @@ pub trait HookExecutor: Send + Sync {
|
|||
) -> HookResult;
|
||||
}
|
||||
|
||||
/// Resolve a typed [`InterpString`] hook segment at fire time, looking up
|
||||
/// `{{ env.* }}` tokens against `env`.
|
||||
/// Resolve a typed [`InterpString`] hook segment at fire time.
|
||||
///
|
||||
/// Only the `env` namespace is wired here; `{{ secrets.* }}`, `{{ vars.* }}`,
|
||||
/// and `{{ inputs.* }}` tokens have no lookup in this context and resolve as
|
||||
/// `Unavailable`, which is a hard error — so a hook that references one fails
|
||||
/// closed rather than firing with a half-resolved value.
|
||||
/// No namespace is wired here. `{{ vars.* }}` is already substituted
|
||||
/// server-side when the run is created, so a literal value resolves unchanged
|
||||
/// and any remaining token — `secrets`, `inputs`, `env` — surfaces as
|
||||
/// `Unavailable`. That is a hard error, so a hook referencing one fails closed
|
||||
/// rather than firing with a half-resolved value.
|
||||
///
|
||||
/// The value stays typed end-to-end: it is carried as an `InterpString`
|
||||
/// through the config resolve layer and resolved here from its segments —
|
||||
/// there is no `InterpString -> String -> InterpString` re-parse. A missing or
|
||||
/// out-of-scope token is a hard error (fail-closed); there is no fallback to
|
||||
/// the unresolved source.
|
||||
/// there is no `InterpString -> String -> InterpString` re-parse.
|
||||
///
|
||||
/// Returns the typed [`ResolveError`] so callers keep the source until the
|
||||
/// decision boundary renders it; do not flatten it to a `String` here.
|
||||
fn resolve_interp<E>(value: &InterpString, env: &E) -> Result<String, ResolveError>
|
||||
where
|
||||
E: Env + ?Sized,
|
||||
{
|
||||
value.resolve(|name| env.var(name).ok())
|
||||
fn resolve_interp(value: &InterpString) -> Result<String, ResolveError> {
|
||||
value.resolve_with(&mut ResolveCtx::new())
|
||||
}
|
||||
|
||||
#[expect(
|
||||
|
|
@ -90,66 +82,6 @@ fn safe_url_source_for_log(url: &InterpString) -> String {
|
|||
redacted_url_for_log(&url.as_source())
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
enum HeaderResolveError {
|
||||
NotAllowed { name: String },
|
||||
Resolve(ResolveError),
|
||||
}
|
||||
|
||||
impl fmt::Display for HeaderResolveError {
|
||||
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
|
||||
match self {
|
||||
Self::NotAllowed { name } => write!(
|
||||
f,
|
||||
"environment variable {name:?} referenced by an HTTP hook header is not listed in \
|
||||
allowed_env_vars"
|
||||
),
|
||||
Self::Resolve(error) => error.fmt(f),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl std::error::Error for HeaderResolveError {
|
||||
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
|
||||
match self {
|
||||
Self::NotAllowed { .. } => None,
|
||||
Self::Resolve(error) => Some(error),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Resolve an HTTP-hook **header** value at fire time, scoping its
|
||||
/// `{{ env.* }}` lookups to `allowed_env_vars`.
|
||||
///
|
||||
/// Headers carry credentials, so unlike every other hook field they read env
|
||||
/// through an allowlist: a `{{ env.NAME }}` token resolves only when `NAME` is
|
||||
/// listed in the hook's `allowed_env_vars`. A name outside the allowlist fails
|
||||
/// with a distinct error before any lookup, while an allowlisted-but-unset name
|
||||
/// still surfaces as the normal `Missing` error. An empty `allowed_env_vars`
|
||||
/// therefore permits no env vars in headers at all. This mirrors the previous
|
||||
/// template-based `with_env_lookup_allowed` behavior without reviving any
|
||||
/// template engine.
|
||||
fn resolve_header<E>(
|
||||
value: &InterpString,
|
||||
allowed_env_vars: &[String],
|
||||
env: &E,
|
||||
) -> Result<String, HeaderResolveError>
|
||||
where
|
||||
E: Env + ?Sized,
|
||||
{
|
||||
if let Some(name) = value.names(Namespace::Env).into_iter().find(|name| {
|
||||
!allowed_env_vars
|
||||
.iter()
|
||||
.any(|allowed| allowed.as_str() == *name)
|
||||
}) {
|
||||
return Err(HeaderResolveError::NotAllowed {
|
||||
name: name.to_string(),
|
||||
});
|
||||
}
|
||||
|
||||
resolve_interp(value, env).map_err(HeaderResolveError::Resolve)
|
||||
}
|
||||
|
||||
/// Executes hooks via shell commands or HTTP POST.
|
||||
pub struct HookExecutorImpl;
|
||||
|
||||
|
|
@ -179,36 +111,27 @@ impl HookExecutorImpl {
|
|||
|
||||
/// Resolve the prompt and optional model segments at fire time.
|
||||
///
|
||||
/// Fail-closed: only `{{ env.* }}` is wired here; a missing env token (or a
|
||||
/// token in any other, unavailable namespace) is a hard error so the hook
|
||||
/// never fires with a half-resolved value. The caller turns the error into
|
||||
/// a `Block` decision, matching the command-hook behavior.
|
||||
fn resolve_prompt_and_model<E>(
|
||||
/// Fail-closed: an unresolved token is a hard error so the hook never
|
||||
/// fires with a half-resolved value. The caller turns the error into a
|
||||
/// `Block` decision, matching the command-hook behavior.
|
||||
fn resolve_prompt_and_model(
|
||||
prompt: &InterpString,
|
||||
model: Option<&InterpString>,
|
||||
env: &E,
|
||||
) -> Result<(String, Option<String>), ResolveError>
|
||||
where
|
||||
E: Env + ?Sized,
|
||||
{
|
||||
let prompt = resolve_interp(prompt, env)?;
|
||||
let model = model.map(|model| resolve_interp(model, env)).transpose()?;
|
||||
) -> Result<(String, Option<String>), ResolveError> {
|
||||
let prompt = resolve_interp(prompt)?;
|
||||
let model = model.map(resolve_interp).transpose()?;
|
||||
Ok((prompt, model))
|
||||
}
|
||||
|
||||
/// Execute a command hook (sandbox or host).
|
||||
async fn execute_command<E>(
|
||||
async fn execute_command(
|
||||
definition: &HookDefinition,
|
||||
command: &InterpString,
|
||||
context: &HookContext,
|
||||
sandbox: &Arc<dyn Sandbox>,
|
||||
execution_context: &HookExecutionContext,
|
||||
env: &E,
|
||||
) -> HookDecision
|
||||
where
|
||||
E: Env + ?Sized,
|
||||
{
|
||||
let command = match resolve_interp(command, env) {
|
||||
) -> HookDecision {
|
||||
let command = match resolve_interp(command) {
|
||||
Ok(command) => command,
|
||||
Err(error) => {
|
||||
return HookDecision::Block {
|
||||
|
|
@ -354,19 +277,15 @@ impl HookExecutorImpl {
|
|||
}
|
||||
|
||||
/// Execute a prompt hook: single-turn LLM call returning ok/block.
|
||||
async fn execute_prompt<E>(
|
||||
async fn execute_prompt(
|
||||
definition: &HookDefinition,
|
||||
prompt: &InterpString,
|
||||
model: Option<&InterpString>,
|
||||
context: &HookContext,
|
||||
env: &E,
|
||||
llm_source: &dyn CredentialSource,
|
||||
catalog: Arc<Catalog>,
|
||||
) -> HookDecision
|
||||
where
|
||||
E: Env + ?Sized,
|
||||
{
|
||||
let (prompt, model) = match Self::resolve_prompt_and_model(prompt, model, env) {
|
||||
) -> HookDecision {
|
||||
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");
|
||||
|
|
@ -421,21 +340,17 @@ impl HookExecutorImpl {
|
|||
/// Reuses the core `ToolRegistry` from `fabro_agent` so the agent hook has
|
||||
/// the same tools (read_file, write_file, shell, grep, glob, etc.) as
|
||||
/// a normal agent session.
|
||||
async fn execute_agent<E>(
|
||||
async fn execute_agent(
|
||||
definition: &HookDefinition,
|
||||
prompt: &InterpString,
|
||||
model: Option<&InterpString>,
|
||||
max_tool_rounds: Option<u32>,
|
||||
context: &HookContext,
|
||||
sandbox: Arc<dyn Sandbox>,
|
||||
env: &E,
|
||||
llm_source: &dyn CredentialSource,
|
||||
catalog: Arc<Catalog>,
|
||||
) -> HookDecision
|
||||
where
|
||||
E: Env + ?Sized,
|
||||
{
|
||||
let (prompt, model) = match Self::resolve_prompt_and_model(prompt, model, env) {
|
||||
) -> HookDecision {
|
||||
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");
|
||||
|
|
@ -566,20 +481,15 @@ impl HookExecutorImpl {
|
|||
/// half-resolved URL or an empty credential header. Transport outcomes
|
||||
/// (non-2xx, connection errors, unparseable body) stay fail-open and
|
||||
/// return `Proceed`.
|
||||
async fn execute_http<E>(
|
||||
async fn execute_http(
|
||||
client: &fabro_http::HttpClient,
|
||||
url: &InterpString,
|
||||
headers: Option<&HashMap<String, InterpString>>,
|
||||
allowed_env_vars: &[String],
|
||||
tls: &TlsMode,
|
||||
context: &HookContext,
|
||||
timeout: std::time::Duration,
|
||||
env: &E,
|
||||
) -> HookDecision
|
||||
where
|
||||
E: Env + ?Sized,
|
||||
{
|
||||
let resolved_url = match resolve_interp(url, env) {
|
||||
) -> HookDecision {
|
||||
let resolved_url = match resolve_interp(url) {
|
||||
Ok(url) => url,
|
||||
Err(error) => {
|
||||
tracing::error!(
|
||||
|
|
@ -611,11 +521,7 @@ impl HookExecutorImpl {
|
|||
|
||||
if let Some(hdrs) = headers {
|
||||
for (key, value) in hdrs {
|
||||
// Headers resolve through the per-hook env allowlist: a
|
||||
// `{{ env.NAME }}` not in `allowed_env_vars` blocks before any
|
||||
// lookup, while an allowlisted-but-unset name still fails as
|
||||
// missing.
|
||||
let interpolated = match resolve_header(value, allowed_env_vars, env) {
|
||||
let interpolated = match resolve_interp(value) {
|
||||
Ok(rendered) => rendered,
|
||||
Err(error) => {
|
||||
tracing::error!(
|
||||
|
|
@ -730,34 +636,24 @@ impl HookExecutor for HookExecutorImpl {
|
|||
static HTTP_CLIENTS: OnceLock<HttpClientCache> = OnceLock::new();
|
||||
|
||||
let start = Instant::now();
|
||||
let env = SystemEnv;
|
||||
|
||||
let decision = match definition.resolved_hook_type() {
|
||||
Some(
|
||||
Cow::Borrowed(HookType::Command { ref command })
|
||||
| Cow::Owned(HookType::Command { ref command }),
|
||||
) => {
|
||||
Self::execute_command(
|
||||
definition,
|
||||
command,
|
||||
context,
|
||||
&sandbox,
|
||||
execution_context,
|
||||
&env,
|
||||
)
|
||||
.await
|
||||
Self::execute_command(definition, command, context, &sandbox, execution_context)
|
||||
.await
|
||||
}
|
||||
Some(
|
||||
Cow::Borrowed(HookType::Http {
|
||||
ref url,
|
||||
ref headers,
|
||||
ref allowed_env_vars,
|
||||
ref tls,
|
||||
})
|
||||
| Cow::Owned(HookType::Http {
|
||||
ref url,
|
||||
ref headers,
|
||||
ref allowed_env_vars,
|
||||
ref tls,
|
||||
}),
|
||||
) => {
|
||||
|
|
@ -766,11 +662,9 @@ impl HookExecutor for HookExecutorImpl {
|
|||
clients.get(*tls),
|
||||
url,
|
||||
headers.as_ref(),
|
||||
allowed_env_vars,
|
||||
tls,
|
||||
context,
|
||||
definition.timeout(),
|
||||
&env,
|
||||
)
|
||||
.await
|
||||
}
|
||||
|
|
@ -789,7 +683,6 @@ impl HookExecutor for HookExecutorImpl {
|
|||
prompt,
|
||||
model.as_ref(),
|
||||
context,
|
||||
&env,
|
||||
llm_source,
|
||||
Arc::clone(&catalog),
|
||||
)
|
||||
|
|
@ -814,7 +707,6 @@ impl HookExecutor for HookExecutorImpl {
|
|||
*max_tool_rounds,
|
||||
context,
|
||||
sandbox,
|
||||
&env,
|
||||
llm_source,
|
||||
Arc::clone(&catalog),
|
||||
)
|
||||
|
|
@ -838,7 +730,7 @@ impl HookExecutor for HookExecutorImpl {
|
|||
mod tests {
|
||||
use fabro_auth::{CredentialSource, test_support};
|
||||
use fabro_types::fixtures;
|
||||
use fabro_util::env::TestEnv;
|
||||
use fabro_types::settings::ResolveErrorKind;
|
||||
|
||||
use super::*;
|
||||
use crate::config::HookType;
|
||||
|
|
@ -1144,14 +1036,6 @@ mod tests {
|
|||
|
||||
// --- hook segment resolution helpers ---
|
||||
|
||||
fn test_env(vars: &[(&str, &str)]) -> TestEnv {
|
||||
TestEnv(
|
||||
vars.iter()
|
||||
.map(|(k, v)| (k.to_string(), v.to_string()))
|
||||
.collect(),
|
||||
)
|
||||
}
|
||||
|
||||
fn interp(value: &str) -> InterpString {
|
||||
InterpString::parse(value)
|
||||
}
|
||||
|
|
@ -1175,81 +1059,31 @@ mod tests {
|
|||
assert_eq!(safe, "<invalid url>");
|
||||
}
|
||||
|
||||
// Headers resolve `{{ env.NAME }}` tokens through the per-hook
|
||||
// `allowed_env_vars` allowlist: an allowlisted name resolves, anything else
|
||||
// fails closed before lookup.
|
||||
/// Hook values are resolved from their typed segments at fire time, never
|
||||
/// via a String -> InterpString re-parse. `{{ vars.* }}` is already
|
||||
/// substituted server-side, so a literal value passes straight through.
|
||||
#[test]
|
||||
fn header_resolves_allowlisted_var() {
|
||||
let env = test_env(&[("FABRO_TEST_KEY_1", "secret123")]);
|
||||
let result = resolve_header(
|
||||
&interp("Bearer {{ env.FABRO_TEST_KEY_1 }}"),
|
||||
&["FABRO_TEST_KEY_1".to_string()],
|
||||
&env,
|
||||
)
|
||||
.unwrap();
|
||||
assert_eq!(result, "Bearer secret123");
|
||||
}
|
||||
|
||||
// Fail-closed: a header may not read an env var that is set in the process
|
||||
// but missing from `allowed_env_vars`. This is distinct from an unset
|
||||
// allowlisted variable, so the block reason points at the allowlist.
|
||||
#[test]
|
||||
fn header_rejects_unlisted_var() {
|
||||
let env = test_env(&[("FABRO_TEST_KEY_3", "should_not_appear")]);
|
||||
let err = resolve_header(
|
||||
&interp("prefix-{{ env.FABRO_TEST_KEY_3 }}-suffix"),
|
||||
&[],
|
||||
&env,
|
||||
)
|
||||
.unwrap_err();
|
||||
assert_eq!(err, HeaderResolveError::NotAllowed {
|
||||
name: "FABRO_TEST_KEY_3".to_string(),
|
||||
});
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn header_missing_token_is_hard_error() {
|
||||
let env = test_env(&[]);
|
||||
let err = resolve_header(
|
||||
&interp("prefix-{{ env.FABRO_TEST_KEY_3 }}-suffix"),
|
||||
&["FABRO_TEST_KEY_3".to_string()],
|
||||
&env,
|
||||
)
|
||||
.unwrap_err();
|
||||
match err {
|
||||
HeaderResolveError::Resolve(error) => assert_eq!(error.name, "FABRO_TEST_KEY_3"),
|
||||
HeaderResolveError::NotAllowed { .. } => {
|
||||
panic!("expected missing token resolve error, got {err:?}")
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// The value stays a typed `InterpString`: it resolves at fire time from its
|
||||
// segments, never via a String -> InterpString re-parse.
|
||||
#[test]
|
||||
fn resolve_interp_resolves_embedded_token_from_typed_value() {
|
||||
let env = test_env(&[("FABRO_TEST_KEY_2", "val")]);
|
||||
let value = interp("x{{ env.FABRO_TEST_KEY_2 }}y");
|
||||
let result = resolve_interp(&value, &env).unwrap();
|
||||
assert_eq!(result, "xvaly");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resolve_interp_errors_on_missing_var() {
|
||||
let env = test_env(&[]);
|
||||
let err = resolve_interp(&interp("a{{ env.FABRO_TEST_NOEXIST }}-b"), &env).unwrap_err();
|
||||
assert_eq!(err.name, "FABRO_TEST_NOEXIST");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resolve_interp_without_tokens_passes_through() {
|
||||
let env = test_env(&[]);
|
||||
fn resolve_interp_passes_through_literal_values() {
|
||||
assert_eq!(resolve_interp(&interp("plain text")).unwrap(), "plain text");
|
||||
assert_eq!(
|
||||
resolve_interp(&interp("plain text"), &env).unwrap(),
|
||||
"plain text"
|
||||
resolve_interp(&interp("Bearer already-substituted")).unwrap(),
|
||||
"Bearer already-substituted"
|
||||
);
|
||||
}
|
||||
|
||||
/// Fail-closed: a token that survived to fire time can never resolve, so a
|
||||
/// hook referencing one blocks rather than sending a half-rendered header.
|
||||
#[test]
|
||||
fn resolve_interp_errors_on_an_unresolved_token() {
|
||||
let err = resolve_interp(&interp("a{{ env.FABRO_TEST_NOEXIST }}-b")).unwrap_err();
|
||||
assert_eq!(err.name, "FABRO_TEST_NOEXIST");
|
||||
assert_eq!(err.kind, ResolveErrorKind::Unavailable);
|
||||
|
||||
let err = resolve_interp(&interp("Bearer {{ secrets.API_KEY }}")).unwrap_err();
|
||||
assert_eq!(err.name, "API_KEY");
|
||||
assert_eq!(err.kind, ResolveErrorKind::Unavailable);
|
||||
}
|
||||
|
||||
// --- HTTP hook execution tests ---
|
||||
|
||||
#[tokio::test]
|
||||
|
|
@ -1270,11 +1104,9 @@ mod tests {
|
|||
&client,
|
||||
&interp(&server.url("/hook")),
|
||||
None,
|
||||
&[],
|
||||
&TlsMode::Off,
|
||||
&make_context(),
|
||||
std::time::Duration::from_secs(5),
|
||||
&test_env(&[]),
|
||||
)
|
||||
.await;
|
||||
|
||||
|
|
@ -1299,11 +1131,9 @@ mod tests {
|
|||
&client,
|
||||
&interp(&server.url("/hook")),
|
||||
None,
|
||||
&[],
|
||||
&TlsMode::Off,
|
||||
&make_context(),
|
||||
std::time::Duration::from_secs(5),
|
||||
&test_env(&[]),
|
||||
)
|
||||
.await;
|
||||
|
||||
|
|
@ -1326,11 +1156,9 @@ mod tests {
|
|||
&client,
|
||||
&interp(&server.url("/hook")),
|
||||
None,
|
||||
&[],
|
||||
&TlsMode::Off,
|
||||
&make_context(),
|
||||
std::time::Duration::from_secs(5),
|
||||
&test_env(&[]),
|
||||
)
|
||||
.await;
|
||||
|
||||
|
|
@ -1345,21 +1173,19 @@ mod tests {
|
|||
&client,
|
||||
&interp("http://127.0.0.1:1"),
|
||||
None,
|
||||
&[],
|
||||
&TlsMode::Off,
|
||||
&make_context(),
|
||||
std::time::Duration::from_secs(1),
|
||||
&test_env(&[]),
|
||||
)
|
||||
.await;
|
||||
|
||||
assert_eq!(decision, HookDecision::Proceed);
|
||||
}
|
||||
|
||||
/// `{{ vars.* }}` is substituted server-side, so a header arrives literal
|
||||
/// and is sent as-is.
|
||||
#[tokio::test]
|
||||
async fn http_hook_sends_interpolated_headers() {
|
||||
let env = test_env(&[("FABRO_TEST_TOKEN", "my-secret")]);
|
||||
|
||||
async fn http_hook_sends_substituted_headers() {
|
||||
let server = httpmock::MockServer::start_async().await;
|
||||
let mock = server
|
||||
.mock_async(|when, then| {
|
||||
|
|
@ -1370,21 +1196,16 @@ mod tests {
|
|||
})
|
||||
.await;
|
||||
|
||||
let headers = HashMap::from([(
|
||||
"Authorization".to_string(),
|
||||
interp("Bearer {{ env.FABRO_TEST_TOKEN }}"),
|
||||
)]);
|
||||
let headers = HashMap::from([("Authorization".to_string(), interp("Bearer my-secret"))]);
|
||||
|
||||
let client = test_http_client();
|
||||
let decision = HookExecutorImpl::execute_http(
|
||||
&client,
|
||||
&interp(&server.url("/hook")),
|
||||
Some(&headers),
|
||||
&["FABRO_TEST_TOKEN".to_string()],
|
||||
&TlsMode::Off,
|
||||
&make_context(),
|
||||
std::time::Duration::from_secs(5),
|
||||
&env,
|
||||
)
|
||||
.await;
|
||||
|
||||
|
|
@ -1392,12 +1213,10 @@ mod tests {
|
|||
assert_eq!(decision, HookDecision::Proceed);
|
||||
}
|
||||
|
||||
// Fail-closed: a header that references an env var set in the process but
|
||||
// absent from `allowed_env_vars` must block and never fire the request.
|
||||
/// Fail-closed: `{{ env.* }}` no longer resolves anywhere, so a header
|
||||
/// referencing one must block rather than send a half-rendered credential.
|
||||
#[tokio::test]
|
||||
async fn http_hook_unlisted_header_var_blocks_without_firing() {
|
||||
let env = test_env(&[("FABRO_TEST_TOKEN", "my-secret")]);
|
||||
|
||||
async fn http_hook_env_header_token_blocks_without_firing() {
|
||||
let server = httpmock::MockServer::start_async().await;
|
||||
let mock = server
|
||||
.mock_async(|when, then| {
|
||||
|
|
@ -1416,12 +1235,9 @@ mod tests {
|
|||
&client,
|
||||
&interp(&server.url("/hook")),
|
||||
Some(&headers),
|
||||
// Empty allowlist: the env var is set, but headers may read nothing.
|
||||
&[],
|
||||
&TlsMode::Off,
|
||||
&make_context(),
|
||||
std::time::Duration::from_secs(5),
|
||||
&env,
|
||||
)
|
||||
.await;
|
||||
|
||||
|
|
@ -1432,15 +1248,15 @@ mod tests {
|
|||
reason
|
||||
.as_deref()
|
||||
.is_some_and(|reason| reason.contains("FABRO_TEST_TOKEN")),
|
||||
"block reason should name the unlisted token, got: {reason:?}"
|
||||
"block reason should name the token, got: {reason:?}"
|
||||
);
|
||||
}
|
||||
other => panic!("expected Block on unlisted header var, got {other:?}"),
|
||||
other => panic!("expected Block on env header token, got {other:?}"),
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn http_hook_resolves_url_before_dispatch() {
|
||||
async fn http_hook_dispatches_a_substituted_url() {
|
||||
let server = httpmock::MockServer::start_async().await;
|
||||
let mock = server
|
||||
.mock_async(|when, then| {
|
||||
|
|
@ -1450,16 +1266,13 @@ mod tests {
|
|||
.await;
|
||||
|
||||
let client = test_http_client();
|
||||
let env = test_env(&[("FABRO_TEST_URL", &server.url("/hook"))]);
|
||||
let decision = HookExecutorImpl::execute_http(
|
||||
&client,
|
||||
&interp("{{ env.FABRO_TEST_URL }}"),
|
||||
&interp(&server.url("/hook")),
|
||||
None,
|
||||
&[],
|
||||
&TlsMode::Off,
|
||||
&make_context(),
|
||||
std::time::Duration::from_secs(5),
|
||||
&env,
|
||||
)
|
||||
.await;
|
||||
|
||||
|
|
@ -1482,11 +1295,9 @@ mod tests {
|
|||
&client,
|
||||
&interp("{{ env.FABRO_TEST_MISSING_URL }}/hook"),
|
||||
None,
|
||||
&[],
|
||||
&TlsMode::Off,
|
||||
&make_context(),
|
||||
std::time::Duration::from_secs(5),
|
||||
&test_env(&[]),
|
||||
)
|
||||
.await;
|
||||
|
||||
|
|
@ -1526,11 +1337,9 @@ mod tests {
|
|||
&interp(&server.url("/hook")),
|
||||
Some(&headers),
|
||||
// Allowlisted but unset: still blocks on the Missing lookup.
|
||||
&["FABRO_TEST_MISSING_HEADER".to_string()],
|
||||
&TlsMode::Off,
|
||||
&make_context(),
|
||||
std::time::Duration::from_secs(5),
|
||||
&test_env(&[]),
|
||||
)
|
||||
.await;
|
||||
|
||||
|
|
@ -1549,11 +1358,9 @@ mod tests {
|
|||
&client,
|
||||
&interp("http://example.com/hook"),
|
||||
None,
|
||||
&[],
|
||||
&TlsMode::Verify,
|
||||
&make_context(),
|
||||
std::time::Duration::from_secs(5),
|
||||
&test_env(&[]),
|
||||
)
|
||||
.await;
|
||||
|
||||
|
|
@ -1567,11 +1374,9 @@ mod tests {
|
|||
&client,
|
||||
&interp("http://example.com/hook"),
|
||||
None,
|
||||
&[],
|
||||
&TlsMode::NoVerify,
|
||||
&make_context(),
|
||||
std::time::Duration::from_secs(5),
|
||||
&test_env(&[]),
|
||||
)
|
||||
.await;
|
||||
|
||||
|
|
@ -1593,11 +1398,9 @@ mod tests {
|
|||
&client,
|
||||
&interp(&server.url("/hook")),
|
||||
None,
|
||||
&[],
|
||||
&TlsMode::Off,
|
||||
&make_context(),
|
||||
std::time::Duration::from_secs(5),
|
||||
&test_env(&[]),
|
||||
)
|
||||
.await;
|
||||
|
||||
|
|
@ -1621,10 +1424,9 @@ mod tests {
|
|||
event: HookEvent::StageStart,
|
||||
command: None,
|
||||
hook_type: Some(HookType::Http {
|
||||
url: interp(&server.url("/hook")),
|
||||
headers: None,
|
||||
allowed_env_vars: vec![],
|
||||
tls: TlsMode::Off,
|
||||
url: interp(&server.url("/hook")),
|
||||
headers: None,
|
||||
tls: TlsMode::Off,
|
||||
}),
|
||||
matcher: None,
|
||||
blocking: None,
|
||||
|
|
@ -1659,7 +1461,6 @@ mod tests {
|
|||
&make_context(),
|
||||
&sandbox,
|
||||
&HookExecutionContext::default(),
|
||||
&test_env(&[]),
|
||||
)
|
||||
.await;
|
||||
|
||||
|
|
@ -1675,7 +1476,6 @@ mod tests {
|
|||
&interp("{{ env.MISSING_HOOK_VALUE }}"),
|
||||
None,
|
||||
&make_context(),
|
||||
&test_env(&[]),
|
||||
test_llm_source().as_ref(),
|
||||
test_catalog(),
|
||||
)
|
||||
|
|
@ -1704,7 +1504,6 @@ mod tests {
|
|||
Some(1),
|
||||
&make_context(),
|
||||
make_sandbox(),
|
||||
&test_env(&[]),
|
||||
test_llm_source().as_ref(),
|
||||
test_catalog(),
|
||||
)
|
||||
|
|
|
|||
|
|
@ -71,13 +71,18 @@ pub fn docker_config_from_environment(
|
|||
settings: &RunEnvironmentSettings,
|
||||
skip_clone: bool,
|
||||
) -> DockerSandboxOptions {
|
||||
// No vault is available on this path (server preflight / manifest), so
|
||||
// resolve `{{ env.* }}` against the process environment and let every other
|
||||
// token (including `{{ secrets.* }}`) fall back to its source form.
|
||||
// No vault is available on this path (server preflight / manifest), so a
|
||||
// `{{ secrets.* }}` value keeps its source form. Nothing else is left to
|
||||
// resolve: `{{ vars.* }}` is substituted at run creation.
|
||||
#[expect(
|
||||
clippy::disallowed_methods,
|
||||
reason = "preflight has no vault, so an unresolved secret token is carried in source \
|
||||
form; the real value is resolved by docker_config_from_environment_with_secrets"
|
||||
)]
|
||||
let env = settings
|
||||
.env
|
||||
.iter()
|
||||
.map(|(key, value)| (key.clone(), value.resolve_or_source(process_env_var)))
|
||||
.map(|(key, value)| (key.clone(), value.as_source()))
|
||||
.collect();
|
||||
docker_config_from_environment_env(settings, skip_clone, env)
|
||||
}
|
||||
|
|
@ -88,7 +93,7 @@ pub fn docker_config_from_environment_with_secrets(
|
|||
skip_clone: bool,
|
||||
secrets_lookup: impl FnMut(&str) -> Option<String>,
|
||||
) -> Result<DockerSandboxOptions, ResolveError> {
|
||||
let env = settings.resolve_env(process_env_var, secrets_lookup)?;
|
||||
let env = settings.resolve_env(secrets_lookup)?;
|
||||
Ok(docker_config_from_environment_env(
|
||||
settings, skip_clone, env,
|
||||
))
|
||||
|
|
@ -158,14 +163,6 @@ pub fn local_working_directory_from_environment(
|
|||
}
|
||||
|
||||
#[cfg(feature = "docker")]
|
||||
#[expect(
|
||||
clippy::disallowed_methods,
|
||||
reason = "Environment interpolation owns a process-env lookup facade for {{ env.* }} values."
|
||||
)]
|
||||
fn process_env_var(name: &str) -> Option<String> {
|
||||
std::env::var(name).ok()
|
||||
}
|
||||
|
||||
#[cfg(feature = "daytona")]
|
||||
fn duration_to_minutes_i32(duration: std::time::Duration) -> i32 {
|
||||
let minutes = duration.as_secs() / 60;
|
||||
|
|
|
|||
|
|
@ -393,9 +393,7 @@ impl RunSession {
|
|||
.mcps
|
||||
.iter()
|
||||
.map(|(key, entry)| match entry {
|
||||
ResolvedMcpEntry::Resolved(server) => {
|
||||
runtime_mcp_server(server, process_env_var, secret_lookup)
|
||||
}
|
||||
ResolvedMcpEntry::Resolved(server) => runtime_mcp_server(server, secret_lookup),
|
||||
// References must be resolved to concrete servers before the run
|
||||
// spec is persisted (server-side run-preparation pass). Reaching
|
||||
// worker startup with an unresolved reference is an invariant
|
||||
|
|
@ -449,7 +447,7 @@ impl RunSession {
|
|||
|
||||
let toml_env = resolved
|
||||
.environment
|
||||
.resolve_env(process_env_var, secret_lookup)
|
||||
.resolve_env(secret_lookup)
|
||||
.map_err(|err| Error::engine_with_source("failed to resolve run environment", err))?;
|
||||
let github_permissions: Option<HashMap<String, String>> =
|
||||
(!services.github_permissions.is_empty()).then(|| services.github_permissions.clone());
|
||||
|
|
@ -467,8 +465,7 @@ impl RunSession {
|
|||
};
|
||||
|
||||
let pr_config = resolved.pull_request.clone();
|
||||
let setup_commands =
|
||||
runtime_setup_commands(&resolved.prepare, process_env_var, secret_lookup)?;
|
||||
let setup_commands = runtime_setup_commands(&resolved.prepare, secret_lookup)?;
|
||||
drop(vault_guard);
|
||||
|
||||
Ok(Self {
|
||||
|
|
@ -739,11 +736,10 @@ impl ModelRegistry for CatalogModelRegistry<'_> {
|
|||
/// error — no fallback to the unresolved source.
|
||||
fn runtime_mcp_server(
|
||||
settings: &ResolvedMcpServerSettings,
|
||||
env_lookup: impl FnMut(&str) -> Option<String>,
|
||||
secrets_lookup: impl FnMut(&str) -> Option<String>,
|
||||
) -> Result<McpServerSettings, Error> {
|
||||
settings
|
||||
.resolve_transport_env(env_lookup, secrets_lookup)
|
||||
.resolve_transport_env(secrets_lookup)
|
||||
.map_err(|err| {
|
||||
Error::engine_with_source(
|
||||
format!("failed to resolve MCP server {:?}", settings.name),
|
||||
|
|
@ -766,11 +762,10 @@ fn runtime_mcp_server(
|
|||
/// a hard error — no fallback to the unresolved source.
|
||||
fn runtime_setup_commands(
|
||||
prepare: &ResolvedRunPrepareSettings,
|
||||
env_lookup: impl FnMut(&str) -> Option<String>,
|
||||
secrets_lookup: impl FnMut(&str) -> Option<String>,
|
||||
) -> Result<Vec<SetupCommand>, Error> {
|
||||
let resolved = prepare
|
||||
.resolve_step_env(env_lookup, secrets_lookup)
|
||||
.resolve_step_env(secrets_lookup)
|
||||
.map_err(|err| Error::engine_with_source("failed to resolve prepare step", err))?;
|
||||
Ok(resolved
|
||||
.steps
|
||||
|
|
@ -1576,7 +1571,7 @@ reasoning = false
|
|||
..ResolvedMcpServerSettings::default()
|
||||
};
|
||||
|
||||
let err = runtime_mcp_server(&settings, |_| None, |_| None).unwrap_err();
|
||||
let err = runtime_mcp_server(&settings, |_| None).unwrap_err();
|
||||
|
||||
assert_eq!(
|
||||
err.to_string(),
|
||||
|
|
@ -1598,8 +1593,7 @@ reasoning = false
|
|||
)]),
|
||||
));
|
||||
|
||||
let commands =
|
||||
runtime_setup_commands(&prepare, |_| None, vault_secret_lookup(&vault)).unwrap();
|
||||
let commands = runtime_setup_commands(&prepare, vault_secret_lookup(&vault)).unwrap();
|
||||
|
||||
assert_eq!(commands.len(), 1);
|
||||
assert_eq!(
|
||||
|
|
@ -1617,8 +1611,7 @@ reasoning = false
|
|||
HashMap::new(),
|
||||
));
|
||||
|
||||
let commands =
|
||||
runtime_setup_commands(&prepare, |_| None, vault_secret_lookup(&vault)).unwrap();
|
||||
let commands = runtime_setup_commands(&prepare, vault_secret_lookup(&vault)).unwrap();
|
||||
let tokens =
|
||||
shlex::split(&commands[0].command).expect("resolved command should remain valid shell");
|
||||
|
||||
|
|
@ -1646,8 +1639,7 @@ reasoning = false
|
|||
..ResolvedMcpServerSettings::default()
|
||||
};
|
||||
|
||||
let resolved =
|
||||
runtime_mcp_server(&settings, |_| None, vault_secret_lookup(&vault)).unwrap();
|
||||
let resolved = runtime_mcp_server(&settings, vault_secret_lookup(&vault)).unwrap();
|
||||
|
||||
let ResolvedMcpTransport::Stdio { env, .. } = resolved.transport else {
|
||||
panic!("expected stdio transport");
|
||||
|
|
@ -1666,8 +1658,7 @@ reasoning = false
|
|||
HashMap::new(),
|
||||
));
|
||||
|
||||
let Err(err) = runtime_setup_commands(&prepare, |_| None, vault_secret_lookup(&vault))
|
||||
else {
|
||||
let Err(err) = runtime_setup_commands(&prepare, vault_secret_lookup(&vault)) else {
|
||||
panic!("missing secret should fail setup command resolution");
|
||||
};
|
||||
|
||||
|
|
@ -1691,8 +1682,7 @@ reasoning = false
|
|||
)]),
|
||||
));
|
||||
|
||||
let Err(err) = runtime_setup_commands(&prepare, |_| None, vault_secret_lookup(&vault))
|
||||
else {
|
||||
let Err(err) = runtime_setup_commands(&prepare, vault_secret_lookup(&vault)) else {
|
||||
panic!("OAuth secret should fail setup command resolution");
|
||||
};
|
||||
|
||||
|
|
@ -1714,8 +1704,7 @@ reasoning = false
|
|||
)]),
|
||||
));
|
||||
|
||||
let Err(err) = runtime_setup_commands(&prepare, |_| None, vault_secret_lookup(&vault))
|
||||
else {
|
||||
let Err(err) = runtime_setup_commands(&prepare, vault_secret_lookup(&vault)) else {
|
||||
panic!("file secret should fail setup command resolution");
|
||||
};
|
||||
|
||||
|
|
|
|||
|
|
@ -218,8 +218,7 @@ impl CredentialResolver {
|
|||
};
|
||||
if catalog_provider.auth.is_none() {
|
||||
let vault = self.vault.read().await;
|
||||
return self
|
||||
.api_credential_from_provider_auth(&vault, catalog_provider, catalog)
|
||||
return Self::api_credential_from_provider_auth(&vault, catalog_provider, catalog)
|
||||
.map(ResolvedCredential::Api);
|
||||
}
|
||||
let initial_secret = {
|
||||
|
|
@ -312,9 +311,7 @@ impl CredentialResolver {
|
|||
catalog: &Catalog,
|
||||
) -> bool {
|
||||
let Some(auth) = &provider.auth else {
|
||||
return self
|
||||
.resolved_extra_headers_for_catalog(vault, &provider.id, catalog)
|
||||
.is_ok();
|
||||
return Self::resolved_extra_headers_for_catalog(vault, &provider.id, catalog).is_ok();
|
||||
};
|
||||
auth.credentials.iter().any(|credential_ref| {
|
||||
self.credential_from_ref(vault, &provider.id, credential_ref)
|
||||
|
|
@ -352,10 +349,6 @@ impl CredentialResolver {
|
|||
}
|
||||
}
|
||||
|
||||
fn lookup_env(&self, name: &str) -> Option<String> {
|
||||
(self.env_lookup)(name)
|
||||
}
|
||||
|
||||
fn provider_base_url_for_catalog(provider: &ProviderId, catalog: &Catalog) -> Option<String> {
|
||||
catalog
|
||||
.provider(provider)
|
||||
|
|
@ -363,7 +356,6 @@ impl CredentialResolver {
|
|||
}
|
||||
|
||||
fn resolved_extra_headers_for_catalog(
|
||||
&self,
|
||||
vault: &Vault,
|
||||
provider: &ProviderId,
|
||||
catalog: &Catalog,
|
||||
|
|
@ -371,9 +363,8 @@ impl CredentialResolver {
|
|||
let Some(catalog_provider) = catalog.provider(provider) else {
|
||||
return Ok(HashMap::new());
|
||||
};
|
||||
let mut ctx = ResolveCtx::new()
|
||||
.with_env(|env_name| self.lookup_env(env_name))
|
||||
.with_secrets(|secret_name| vault_token_lookup(vault, secret_name));
|
||||
let mut ctx =
|
||||
ResolveCtx::new().with_secrets(|secret_name| vault_token_lookup(vault, secret_name));
|
||||
resolve_extra_headers(provider, &catalog_provider.extra_headers, &mut ctx)
|
||||
}
|
||||
|
||||
|
|
@ -391,7 +382,7 @@ impl CredentialResolver {
|
|||
ResolvedSecret::AwsSigv4 => Ok(ApiCredential {
|
||||
provider: provider_id.clone(),
|
||||
auth_header: Some(ApiKeyHeader::AwsSigv4),
|
||||
extra_headers: self.resolved_extra_headers_for_catalog(
|
||||
extra_headers: Self::resolved_extra_headers_for_catalog(
|
||||
vault,
|
||||
provider_id,
|
||||
catalog,
|
||||
|
|
@ -409,7 +400,7 @@ impl CredentialResolver {
|
|||
let mut cred = ApiCredential {
|
||||
provider: provider_id.clone(),
|
||||
auth_header: Some(auth_header),
|
||||
extra_headers: self.resolved_extra_headers_for_catalog(
|
||||
extra_headers: Self::resolved_extra_headers_for_catalog(
|
||||
vault,
|
||||
provider_id,
|
||||
catalog,
|
||||
|
|
@ -428,7 +419,7 @@ impl CredentialResolver {
|
|||
let mut api_credential = ApiCredential {
|
||||
provider: provider_id.clone(),
|
||||
auth_header: Some(ApiKeyHeader::Bearer(credential.tokens.access_token.clone())),
|
||||
extra_headers: self.resolved_extra_headers_for_catalog(
|
||||
extra_headers: Self::resolved_extra_headers_for_catalog(
|
||||
vault,
|
||||
provider_id,
|
||||
catalog,
|
||||
|
|
@ -451,7 +442,6 @@ impl CredentialResolver {
|
|||
}
|
||||
|
||||
fn api_credential_from_provider_auth(
|
||||
&self,
|
||||
vault: &Vault,
|
||||
provider: &CatalogProvider,
|
||||
catalog: &Catalog,
|
||||
|
|
@ -459,8 +449,7 @@ impl CredentialResolver {
|
|||
if provider.auth.is_some() {
|
||||
return Err(ResolveError::NotConfigured(provider.id.clone()));
|
||||
}
|
||||
let extra_headers =
|
||||
self.resolved_extra_headers_for_catalog(vault, &provider.id, catalog)?;
|
||||
let extra_headers = Self::resolved_extra_headers_for_catalog(vault, &provider.id, catalog)?;
|
||||
Ok(ApiCredential {
|
||||
provider: provider.id.clone(),
|
||||
auth_header: None,
|
||||
|
|
|
|||
|
|
@ -730,40 +730,38 @@ impl<'de> Deserialize<'de> for McpEntryLayer {
|
|||
pub struct HookEntry {
|
||||
/// Optional merge identity. Hooks with the same `id` replace in place.
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub id: Option<String>,
|
||||
pub id: Option<String>,
|
||||
/// Display-only human name.
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub name: Option<String>,
|
||||
pub event: HookEvent,
|
||||
pub name: Option<String>,
|
||||
pub event: HookEvent,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub matcher: Option<String>,
|
||||
pub matcher: Option<String>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub blocking: Option<bool>,
|
||||
pub blocking: Option<bool>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub timeout: Option<Duration>,
|
||||
pub timeout: Option<Duration>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub sandbox: Option<bool>,
|
||||
pub sandbox: Option<bool>,
|
||||
// Exactly one of the following groups is expected:
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub script: Option<InterpString>,
|
||||
pub script: Option<InterpString>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub command: Option<Vec<InterpString>>,
|
||||
pub command: Option<Vec<InterpString>>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub url: Option<InterpString>,
|
||||
pub url: Option<InterpString>,
|
||||
#[serde(default, skip_serializing_if = "HashMap::is_empty")]
|
||||
pub headers: HashMap<String, InterpString>,
|
||||
#[serde(default, skip_serializing_if = "Vec::is_empty")]
|
||||
pub allowed_env_vars: Vec<String>,
|
||||
pub headers: HashMap<String, InterpString>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub tls: Option<HookTlsMode>,
|
||||
pub tls: Option<HookTlsMode>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub prompt: Option<InterpString>,
|
||||
pub prompt: Option<InterpString>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub model: Option<InterpString>,
|
||||
pub model: Option<InterpString>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub max_tool_rounds: Option<u32>,
|
||||
pub max_tool_rounds: Option<u32>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub agent: Option<HookAgentMarker>,
|
||||
pub agent: Option<HookAgentMarker>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Serialize, Deserialize)]
|
||||
|
|
|
|||
|
|
@ -202,13 +202,12 @@ Authorization = "Bearer {{ env.HOOK_TOKEN }}"
|
|||
assert_eq!(
|
||||
hook.resolved_hook_type().as_deref(),
|
||||
Some(&HookType::Http {
|
||||
url: InterpString::parse("https://hooks.example.com"),
|
||||
headers: Some(HashMap::from([(
|
||||
url: InterpString::parse("https://hooks.example.com"),
|
||||
headers: Some(HashMap::from([(
|
||||
"Authorization".to_string(),
|
||||
InterpString::parse("Bearer {{ env.HOOK_TOKEN }}"),
|
||||
)])),
|
||||
allowed_env_vars: Vec::new(),
|
||||
tls: TlsMode::Verify,
|
||||
tls: TlsMode::Verify,
|
||||
})
|
||||
);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -595,7 +595,6 @@ fn resolve_hook_type(hook: &HookEntry) -> Option<HookType> {
|
|||
return Some(HookType::Http {
|
||||
url: url.clone(),
|
||||
headers,
|
||||
allowed_env_vars: hook.allowed_env_vars.clone(),
|
||||
tls,
|
||||
});
|
||||
}
|
||||
|
|
|
|||
|
|
@ -11,8 +11,8 @@
|
|||
|
||||
use std::path::{Path, PathBuf};
|
||||
|
||||
use fabro_types::settings::InterpString;
|
||||
use fabro_types::settings::run::{ResolvedGoalSource, ResolvedRunGoal, RunGoal, RunNamespace};
|
||||
use fabro_types::settings::{InterpString, ResolveCtx, ResolveError};
|
||||
|
||||
use crate::load::{load_settings_path, resolve_goal_file_path};
|
||||
use crate::parse::{SettingsSource, validate_settings_source};
|
||||
|
|
@ -60,8 +60,8 @@ pub fn resolve_graph_path(workflow_toml: &Path, graph_relative: &str) -> PathBuf
|
|||
|
||||
#[derive(Debug)]
|
||||
pub enum ResolveRunGoalError {
|
||||
EnvLookup {
|
||||
var: String,
|
||||
Interpolation {
|
||||
source: ResolveError,
|
||||
},
|
||||
Io {
|
||||
path: PathBuf,
|
||||
|
|
@ -72,10 +72,9 @@ pub enum ResolveRunGoalError {
|
|||
impl std::fmt::Display for ResolveRunGoalError {
|
||||
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
|
||||
match self {
|
||||
Self::EnvLookup { var } => write!(
|
||||
f,
|
||||
"run.goal.file references env var `{var}` which is not set"
|
||||
),
|
||||
Self::Interpolation { source } => {
|
||||
write!(f, "run.goal.file interpolation failed: {source}")
|
||||
}
|
||||
Self::Io { path, source } => {
|
||||
write!(f, "failed to read goal file {}: {source}", path.display())
|
||||
}
|
||||
|
|
@ -86,7 +85,7 @@ impl std::fmt::Display for ResolveRunGoalError {
|
|||
impl std::error::Error for ResolveRunGoalError {
|
||||
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
|
||||
match self {
|
||||
Self::EnvLookup { .. } => None,
|
||||
Self::Interpolation { source } => Some(source),
|
||||
Self::Io { source, .. } => Some(source),
|
||||
}
|
||||
}
|
||||
|
|
@ -118,9 +117,11 @@ fn resolve_goal_file(
|
|||
file: &InterpString,
|
||||
base_dir: &Path,
|
||||
) -> std::result::Result<ResolvedRunGoal, ResolveRunGoalError> {
|
||||
// `{{ vars.* }}` is substituted server-side at run creation, so the path is
|
||||
// literal by this point; anything left unresolved fails closed.
|
||||
let resolved = file
|
||||
.resolve(process_env_var)
|
||||
.map_err(|err| ResolveRunGoalError::EnvLookup { var: err.name })?;
|
||||
.resolve_with(&mut ResolveCtx::new())
|
||||
.map_err(|source| ResolveRunGoalError::Interpolation { source })?;
|
||||
let path = resolve_goal_file_path(&resolved, base_dir);
|
||||
let text = std::fs::read_to_string(&path).map_err(|source| ResolveRunGoalError::Io {
|
||||
path: path.clone(),
|
||||
|
|
@ -132,14 +133,6 @@ fn resolve_goal_file(
|
|||
})
|
||||
}
|
||||
|
||||
#[expect(
|
||||
clippy::disallowed_methods,
|
||||
reason = "Run config interpolation owns a process-env lookup facade for {{ env.* }} values."
|
||||
)]
|
||||
fn process_env_var(name: &str) -> Option<String> {
|
||||
std::env::var(name).ok()
|
||||
}
|
||||
|
||||
#[expect(
|
||||
clippy::disallowed_methods,
|
||||
reason = "goal text intentionally passes through in source form; goals become importable \
|
||||
|
|
|
|||
|
|
@ -1,21 +1,25 @@
|
|||
//! Interpolation for config strings.
|
||||
//!
|
||||
//! An [`InterpString`] field may contain narrow `{{ <namespace>.NAME }}`
|
||||
//! tokens — no template logic. Three [`Namespace`]s resolve here: `env`,
|
||||
//! `vars`, and `secrets`. `inputs` is **template-only**: it is a
|
||||
//! recognized namespace so an `{{ inputs.* }}` token fails loudly with a clear
|
||||
//! message instead of passing through as literal text, but it never resolves
|
||||
//! in an `InterpString` field — it belongs in prompts and goals. Which of the
|
||||
//! resolvable namespaces actually apply is scope-determined by the caller
|
||||
//! through [`ResolveCtx`]: server-scope settings provide `env` (and eventually
|
||||
//! `secrets`), run-scope settings additionally provide `vars`. A token whose
|
||||
//! namespace is not available in the resolution context fails loudly.
|
||||
//! tokens — no template logic. Which [`Namespace`]s resolve is
|
||||
//! scope-determined by the caller through [`ResolveCtx`]: run-scope settings
|
||||
//! provide `vars` and `secrets`, a command node `script` provides `inputs`,
|
||||
//! `vars`, and `goal`. A token whose namespace has no lookup in the resolution
|
||||
//! context fails loudly rather than passing through as literal text.
|
||||
//!
|
||||
//! Resolution timing is split: `vars` substitutes early (server-side, at run
|
||||
//! creation) via [`InterpString::substitute_with`], while `env`/`secrets`
|
||||
//! resolve late, at consumption time in the process that owns
|
||||
//! the value, via [`InterpString::resolve_with`]. Resolved secret values are
|
||||
//! plain strings; sensitivity is not tracked. Redaction of run output is
|
||||
//! Two namespaces parse but never resolve. `inputs` and `goal` are bound only
|
||||
//! where a run's values are in scope, which is the workflow graph rather than
|
||||
//! general config. `env` resolves nowhere at all: the process environment is
|
||||
//! not a configuration source, and `{{ vars.NAME }}` (non-sensitive, stored on
|
||||
//! the server) or `{{ secrets.NAME }}` (vault-backed) replaces it. Keeping
|
||||
//! them parseable is what lets an out-of-scope token fail with a message that
|
||||
//! names the alternative instead of reaching a consumer as literal text.
|
||||
//!
|
||||
//! Resolution timing is split: `vars` and `inputs` substitute early
|
||||
//! (server-side, at run creation) via [`InterpString::substitute_with`], while
|
||||
//! `secrets` resolves late, at consumption time in the process that owns the
|
||||
//! value, via [`InterpString::resolve_with`]. Resolved secret values are plain
|
||||
//! strings; sensitivity is not tracked. Redaction of run output is
|
||||
//! content-based (entropy analysis plus credential patterns), applied where
|
||||
//! output is serialized.
|
||||
|
||||
|
|
@ -49,7 +53,8 @@ enum Segment {
|
|||
)]
|
||||
#[strum(serialize_all = "lowercase")]
|
||||
pub enum Namespace {
|
||||
/// `{{ env.NAME }}` — process environment, resolved at consumption time.
|
||||
/// `{{ env.NAME }}` — the process environment. Parses so the token fails
|
||||
/// loudly, but resolves nowhere: use `vars` or `secrets` instead.
|
||||
Env,
|
||||
/// `{{ vars.NAME }}` — non-sensitive run variables, substituted early.
|
||||
Vars,
|
||||
|
|
@ -115,7 +120,6 @@ impl Namespace {
|
|||
/// [`InterpString::substitute_with`].
|
||||
#[derive(Default)]
|
||||
pub struct ResolveCtx<'a> {
|
||||
env: Option<LookupFn<'a>>,
|
||||
vars: Option<LookupFn<'a>>,
|
||||
secrets: Option<LookupFn<'a>>,
|
||||
}
|
||||
|
|
@ -128,12 +132,6 @@ impl<'a> ResolveCtx<'a> {
|
|||
Self::default()
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub fn with_env(mut self, lookup: impl FnMut(&str) -> Option<String> + 'a) -> Self {
|
||||
self.env = Some(Box::new(lookup));
|
||||
self
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub fn with_vars(mut self, lookup: impl FnMut(&str) -> Option<String> + 'a) -> Self {
|
||||
self.vars = Some(Box::new(lookup));
|
||||
|
|
@ -148,14 +146,15 @@ impl<'a> ResolveCtx<'a> {
|
|||
|
||||
fn lookup_for(&mut self, namespace: Namespace) -> Option<&mut LookupFn<'a>> {
|
||||
match namespace {
|
||||
Namespace::Env => self.env.as_mut(),
|
||||
Namespace::Vars => self.vars.as_mut(),
|
||||
Namespace::Secrets => self.secrets.as_mut(),
|
||||
// `inputs` is template-only: an `InterpString` resolve context
|
||||
// never provides it, so an `{{ inputs.* }}` token is always
|
||||
// unavailable here. `substitute_with` still preserves the token so a
|
||||
// goal (an `InterpString` that feeds a template) can forward it.
|
||||
Namespace::Inputs => None,
|
||||
// Neither is ever wired. `env` has no lookup because the process
|
||||
// environment is not a configuration source; `inputs` is
|
||||
// template-only. Both variants exist so the token fails with a
|
||||
// message naming where the value belongs, and `substitute_with`
|
||||
// still preserves them so a goal (an `InterpString` that feeds a
|
||||
// template) can forward `{{ inputs.* }}` to the template layer.
|
||||
Namespace::Env | Namespace::Inputs => None,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -334,35 +333,6 @@ impl InterpString {
|
|||
Ok(Self { segments })
|
||||
}
|
||||
|
||||
/// Resolve in an env-only context, e.g. server-scope settings.
|
||||
///
|
||||
/// `lookup` should return the current value for a given env var name (or
|
||||
/// `None` if unset). Tokens in any other namespace fail with
|
||||
/// [`ResolveErrorKind::Unavailable`].
|
||||
pub fn resolve<F>(&self, lookup: F) -> Result<String, ResolveError>
|
||||
where
|
||||
F: FnMut(&str) -> Option<String>,
|
||||
{
|
||||
self.resolve_with(&mut ResolveCtx::new().with_env(lookup))
|
||||
}
|
||||
|
||||
/// Resolve in an env-only context, falling back to the raw template
|
||||
/// source when resolution fails so a missing env var surfaces as a
|
||||
/// recognizable diagnostic instead of a silently dropped value.
|
||||
#[expect(
|
||||
clippy::disallowed_methods,
|
||||
reason = "intentional raw-source fallback so a missing env var surfaces as a \
|
||||
recognizable diagnostic; slated for hard-error semantics in the \
|
||||
interpolation cleanup"
|
||||
)]
|
||||
#[must_use]
|
||||
pub fn resolve_or_source<F>(&self, lookup: F) -> String
|
||||
where
|
||||
F: FnMut(&str) -> Option<String>,
|
||||
{
|
||||
self.resolve(lookup).unwrap_or_else(|_| self.as_source())
|
||||
}
|
||||
|
||||
/// Substitute only `{{ vars.* }}` tokens while preserving all other
|
||||
/// namespaces for their consumption-time resolution.
|
||||
pub fn substitute_variables<F>(&self, lookup: F) -> Result<Self, ResolveError>
|
||||
|
|
@ -471,6 +441,16 @@ impl fmt::Display for ResolveError {
|
|||
config fields",
|
||||
self.name
|
||||
),
|
||||
// `env` resolves nowhere. Name the replacement rather than
|
||||
// reporting a generic out-of-scope error.
|
||||
Namespace::Env => write!(
|
||||
f,
|
||||
"{{{{ env.{} }}}} is not supported: the process environment is not a \
|
||||
configuration source. Use {{{{ vars.{} }}}} for a non-sensitive value \
|
||||
(`fabro variable set`) or {{{{ secrets.{} }}}} for a credential \
|
||||
(`fabro secret set`)",
|
||||
self.name, self.name, self.name
|
||||
),
|
||||
_ => write!(
|
||||
f,
|
||||
"{noun} {:?} referenced by {{{{ {namespace}.{} }}}} is not supported in \
|
||||
|
|
@ -567,56 +547,67 @@ mod tests {
|
|||
assert_eq!(s.names(Namespace::Env), vec!["USER", "HOST", "PORT"]);
|
||||
}
|
||||
|
||||
fn resolve_vars(s: &InterpString, pairs: &[(&str, &str)]) -> Result<String, ResolveError> {
|
||||
s.resolve_with(&mut ResolveCtx::new().with_vars(lookup_from(pairs)))
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resolve_literal_string() {
|
||||
let s = InterpString::parse("static");
|
||||
let resolved = s.resolve(lookup_from(&[])).unwrap();
|
||||
assert_eq!(resolved, "static");
|
||||
assert_eq!(resolve_vars(&s, &[]).unwrap(), "static");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resolve_whole_value() {
|
||||
let s = InterpString::parse("{{ env.API_KEY }}");
|
||||
let resolved = s
|
||||
.resolve(lookup_from(&[("API_KEY", "secret-123")]))
|
||||
.unwrap();
|
||||
assert_eq!(resolved, "secret-123");
|
||||
let s = InterpString::parse("{{ vars.API_KEY }}");
|
||||
assert_eq!(
|
||||
resolve_vars(&s, &[("API_KEY", "secret-123")]).unwrap(),
|
||||
"secret-123"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resolve_substring() {
|
||||
let s = InterpString::parse("Bearer {{ env.TOKEN }}");
|
||||
let resolved = s.resolve(lookup_from(&[("TOKEN", "abc")])).unwrap();
|
||||
assert_eq!(resolved, "Bearer abc");
|
||||
let s = InterpString::parse("Bearer {{ vars.TOKEN }}");
|
||||
assert_eq!(resolve_vars(&s, &[("TOKEN", "abc")]).unwrap(), "Bearer abc");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resolve_multiple_tokens() {
|
||||
let s = InterpString::parse("{{ env.USER }}@{{ env.HOST }}");
|
||||
let resolved = s
|
||||
.resolve(lookup_from(&[("USER", "root"), ("HOST", "example.com")]))
|
||||
.unwrap();
|
||||
assert_eq!(resolved, "root@example.com");
|
||||
let s = InterpString::parse("{{ vars.USER }}@{{ vars.HOST }}");
|
||||
assert_eq!(
|
||||
resolve_vars(&s, &[("USER", "root"), ("HOST", "example.com")]).unwrap(),
|
||||
"root@example.com"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resolve_missing_env_fails_with_name() {
|
||||
let s = InterpString::parse("{{ env.MISSING }}");
|
||||
let err = s.resolve(lookup_from(&[])).unwrap_err();
|
||||
fn resolve_missing_var_fails_with_name() {
|
||||
let s = InterpString::parse("{{ vars.MISSING }}");
|
||||
let err = resolve_vars(&s, &[]).unwrap_err();
|
||||
assert_eq!(err.name, "MISSING");
|
||||
assert_eq!(err.namespace, Namespace::Env);
|
||||
assert_eq!(err.namespace, Namespace::Vars);
|
||||
assert_eq!(err.kind, ResolveErrorKind::Missing);
|
||||
assert_eq!(
|
||||
err.to_string(),
|
||||
"environment variable \"MISSING\" referenced by {{ env.MISSING }} is not set"
|
||||
);
|
||||
}
|
||||
|
||||
/// `env` still parses so the token fails loudly, but it resolves nowhere
|
||||
/// and the message names its replacements.
|
||||
#[test]
|
||||
fn env_token_parses_but_never_resolves() {
|
||||
let s = InterpString::parse("{{ env.API_KEY }}");
|
||||
let err = resolve_vars(&s, &[("API_KEY", "ignored")]).unwrap_err();
|
||||
|
||||
assert_eq!(err.namespace, Namespace::Env);
|
||||
assert_eq!(err.kind, ResolveErrorKind::Unavailable);
|
||||
let message = err.to_string();
|
||||
assert!(message.contains("vars.API_KEY"), "{message}");
|
||||
assert!(message.contains("secrets.API_KEY"), "{message}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn unterminated_token_treated_as_literal() {
|
||||
let s = InterpString::parse("{{ env.OPEN");
|
||||
let resolved = s.resolve(lookup_from(&[])).unwrap();
|
||||
assert_eq!(resolved, "{{ env.OPEN");
|
||||
assert_eq!(resolve_vars(&s, &[]).unwrap(), "{{ env.OPEN");
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
|
@ -631,8 +622,7 @@ mod tests {
|
|||
] {
|
||||
let s = InterpString::parse(raw);
|
||||
assert!(s.is_literal(), "{raw} should stay literal");
|
||||
let resolved = s.resolve(lookup_from(&[])).unwrap();
|
||||
assert_eq!(resolved, raw);
|
||||
assert_eq!(resolve_vars(&s, &[]).unwrap(), raw);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -672,14 +662,14 @@ mod tests {
|
|||
}
|
||||
|
||||
#[test]
|
||||
fn resolve_with_substitutes_env_and_var_tokens() {
|
||||
let s = InterpString::parse("https://{{ env.REGION }}.{{ vars.DOMAIN }}");
|
||||
fn resolve_with_substitutes_secret_and_var_tokens() {
|
||||
let s = InterpString::parse("https://{{ vars.REGION }}.{{ secrets.DOMAIN }}");
|
||||
|
||||
let resolved = s
|
||||
.resolve_with(
|
||||
&mut ResolveCtx::new()
|
||||
.with_env(lookup_from(&[("REGION", "us-east-1")]))
|
||||
.with_vars(lookup_from(&[("DOMAIN", "example.com")])),
|
||||
.with_vars(lookup_from(&[("REGION", "us-east-1")]))
|
||||
.with_secrets(lookup_from(&[("DOMAIN", "example.com")])),
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
|
|
@ -691,11 +681,7 @@ mod tests {
|
|||
let s = InterpString::parse("{{ vars.MISSING }}");
|
||||
|
||||
let err = s
|
||||
.resolve_with(
|
||||
&mut ResolveCtx::new()
|
||||
.with_env(lookup_from(&[]))
|
||||
.with_vars(lookup_from(&[])),
|
||||
)
|
||||
.resolve_with(&mut ResolveCtx::new().with_vars(lookup_from(&[])))
|
||||
.unwrap_err();
|
||||
|
||||
assert_eq!(err.name, "MISSING");
|
||||
|
|
@ -708,10 +694,10 @@ mod tests {
|
|||
}
|
||||
|
||||
#[test]
|
||||
fn env_only_resolution_rejects_vars_reference() {
|
||||
fn empty_context_rejects_vars_reference() {
|
||||
let s = InterpString::parse("{{ vars.RUNTIME_TOKEN }}");
|
||||
|
||||
let err = s.resolve(lookup_from(&[])).unwrap_err();
|
||||
let err = s.resolve_with(&mut ResolveCtx::new()).unwrap_err();
|
||||
|
||||
assert_eq!(err.name, "RUNTIME_TOKEN");
|
||||
assert_eq!(err.namespace, Namespace::Vars);
|
||||
|
|
@ -724,10 +710,10 @@ mod tests {
|
|||
}
|
||||
|
||||
#[test]
|
||||
fn env_only_resolution_rejects_secrets_reference() {
|
||||
fn empty_context_rejects_secrets_reference() {
|
||||
let s = InterpString::parse("{{ secrets.API_KEY }}");
|
||||
|
||||
let err = s.resolve(lookup_from(&[])).unwrap_err();
|
||||
let err = s.resolve_with(&mut ResolveCtx::new()).unwrap_err();
|
||||
|
||||
assert_eq!(err.namespace, Namespace::Secrets);
|
||||
assert_eq!(err.kind, ResolveErrorKind::Unavailable);
|
||||
|
|
@ -739,13 +725,13 @@ mod tests {
|
|||
}
|
||||
|
||||
#[test]
|
||||
fn resolve_with_substitutes_secrets_and_env() {
|
||||
let s = InterpString::parse("Bearer {{ secrets.API_KEY }} via {{ env.PROXY }}");
|
||||
fn resolve_with_substitutes_secrets_and_vars() {
|
||||
let s = InterpString::parse("Bearer {{ secrets.API_KEY }} via {{ vars.PROXY }}");
|
||||
|
||||
let resolved = s
|
||||
.resolve_with(
|
||||
&mut ResolveCtx::new()
|
||||
.with_env(lookup_from(&[("PROXY", "proxy.internal")]))
|
||||
.with_vars(lookup_from(&[("PROXY", "proxy.internal")]))
|
||||
.with_secrets(lookup_from(&[("API_KEY", "vault-value")])),
|
||||
)
|
||||
.unwrap();
|
||||
|
|
|
|||
|
|
@ -422,13 +422,12 @@ mod run_namespace_variable_substitution_tests {
|
|||
event: HookEvent::RunComplete,
|
||||
command: None,
|
||||
hook_type: Some(HookType::Http {
|
||||
url: InterpString::parse("https://hooks.example/{{ vars.ENV }}"),
|
||||
headers: Some(HashMap::from([(
|
||||
url: InterpString::parse("https://hooks.example/{{ vars.ENV }}"),
|
||||
headers: Some(HashMap::from([(
|
||||
"X-Env".to_string(),
|
||||
InterpString::parse("{{ vars.ENV }}"),
|
||||
)])),
|
||||
allowed_env_vars: Vec::new(),
|
||||
tls: super::TlsMode::Verify,
|
||||
tls: super::TlsMode::Verify,
|
||||
}),
|
||||
matcher: None,
|
||||
blocking: None,
|
||||
|
|
@ -617,26 +616,22 @@ impl RunIntegrationsGithubSettings {
|
|||
!self.permissions.is_empty()
|
||||
}
|
||||
|
||||
/// Resolve every `permissions` value's `{{ env.* }}` tokens via
|
||||
/// `lookup`, falling back to the raw template source when resolution
|
||||
/// fails so callers see a recognizable diagnostic instead of a
|
||||
/// silently dropped key. The `lookup` seam keeps tests free of
|
||||
/// process-env coupling; production callers pass a thin wrapper over
|
||||
/// `std::env::var`.
|
||||
pub fn resolve_permissions<F>(&self, mut lookup: F) -> HashMap<String, String>
|
||||
where
|
||||
F: FnMut(&str) -> Option<String>,
|
||||
{
|
||||
/// Resolve every `permissions` value. `{{ vars.* }}` is substituted
|
||||
/// server-side at run creation, so values are literal by this point; a
|
||||
/// still-unresolved token fails closed rather than reaching the GitHub API
|
||||
/// as literal text.
|
||||
pub fn resolve_permissions(&self) -> Result<HashMap<String, String>, ResolveError> {
|
||||
let mut ctx = ResolveCtx::new();
|
||||
self.permissions
|
||||
.iter()
|
||||
.map(|(name, value)| (name.clone(), value.resolve_or_source(&mut lookup)))
|
||||
.map(|(name, value)| Ok((name.clone(), value.resolve_with(&mut ctx)?)))
|
||||
.collect()
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod run_integrations_github_tests {
|
||||
use super::{HashMap, InterpString, RunIntegrationsGithubSettings};
|
||||
use super::{InterpString, Namespace, RunIntegrationsGithubSettings};
|
||||
|
||||
fn settings(permissions: &[(&str, &str)]) -> RunIntegrationsGithubSettings {
|
||||
RunIntegrationsGithubSettings {
|
||||
|
|
@ -654,30 +649,26 @@ mod run_integrations_github_tests {
|
|||
}
|
||||
|
||||
#[test]
|
||||
fn resolve_permissions_substitutes_env_tokens_via_lookup() {
|
||||
let s = settings(&[("issues", "{{ env.GH_PERM_LEVEL }}"), ("contents", "read")]);
|
||||
let resolved = s.resolve_permissions(|name| match name {
|
||||
"GH_PERM_LEVEL" => Some("write".to_string()),
|
||||
_ => None,
|
||||
});
|
||||
fn resolve_permissions_passes_through_literal_values() {
|
||||
let s = settings(&[("issues", "write"), ("contents", "read")]);
|
||||
let resolved = s.resolve_permissions().unwrap();
|
||||
assert_eq!(resolved.get("issues"), Some(&"write".to_string()));
|
||||
assert_eq!(resolved.get("contents"), Some(&"read".to_string()));
|
||||
}
|
||||
|
||||
/// `{{ vars.* }}` is substituted at run creation, so a token still present
|
||||
/// here can never resolve and must fail rather than reach the GitHub API
|
||||
/// as literal text.
|
||||
#[test]
|
||||
fn resolve_permissions_falls_back_to_source_when_lookup_fails() {
|
||||
let s = settings(&[("issues", "{{ env.GH_PERM_MISSING }}")]);
|
||||
let resolved = s.resolve_permissions(|_| None);
|
||||
assert_eq!(
|
||||
resolved.get("issues"),
|
||||
Some(&"{{ env.GH_PERM_MISSING }}".to_string())
|
||||
);
|
||||
fn resolve_permissions_fails_on_an_unresolved_token() {
|
||||
let s = settings(&[("issues", "{{ env.GH_PERM_LEVEL }}")]);
|
||||
let err = s.resolve_permissions().unwrap_err();
|
||||
assert_eq!(err.namespace, Namespace::Env);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resolve_permissions_is_empty_for_empty_settings() {
|
||||
let s: HashMap<String, String> = settings(&[]).resolve_permissions(|_| None);
|
||||
assert!(s.is_empty());
|
||||
assert!(settings(&[]).resolve_permissions().unwrap().is_empty());
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -764,17 +755,16 @@ impl RunPrepareSettings {
|
|||
/// A referenced env var or secret that is unset is a hard error — no
|
||||
/// fallback to the unresolved source. Reserved `inputs` tokens have no
|
||||
/// lookup here and surface as a loud
|
||||
/// [`super::interp::ResolveErrorKind::Unavailable`] error rather than
|
||||
/// [`ResolveErrorKind::Unavailable`] error rather than
|
||||
/// passing through as literal text.
|
||||
pub fn resolve_step_env(
|
||||
&self,
|
||||
mut env_lookup: impl FnMut(&str) -> Option<String>,
|
||||
mut secrets_lookup: impl FnMut(&str) -> Option<String>,
|
||||
) -> Result<Self, ResolveError> {
|
||||
let mut resolved = self.clone();
|
||||
for step in &mut resolved.steps {
|
||||
visit_prepared_step_strings(step, &mut |value| {
|
||||
resolve_env_string(value, &mut env_lookup, &mut secrets_lookup)
|
||||
resolve_env_string(value, &mut secrets_lookup)
|
||||
})?;
|
||||
}
|
||||
Ok(resolved)
|
||||
|
|
@ -1094,35 +1084,17 @@ impl RunEnvironmentSettings {
|
|||
}
|
||||
}
|
||||
|
||||
/// Resolve every environment value's `{{ env.* }}` and `{{ secrets.* }}`
|
||||
/// tokens via the supplied lookups. Missing env vars retain the historical
|
||||
/// fallback to the original source string for env-only values; values that
|
||||
/// reference secrets fail closed instead of preserving a secret token.
|
||||
/// Resolve every environment value's `{{ secrets.* }}` tokens via
|
||||
/// `secrets_lookup`. `{{ vars.* }}` is already substituted server-side at
|
||||
/// run creation, so anything still unresolved here fails closed.
|
||||
pub fn resolve_env(
|
||||
&self,
|
||||
mut env_lookup: impl FnMut(&str) -> Option<String>,
|
||||
mut secrets_lookup: impl FnMut(&str) -> Option<String>,
|
||||
) -> Result<HashMap<String, String>, ResolveError> {
|
||||
let mut ctx = ResolveCtx::new()
|
||||
.with_env(&mut env_lookup)
|
||||
.with_secrets(&mut secrets_lookup);
|
||||
let mut ctx = ResolveCtx::new().with_secrets(&mut secrets_lookup);
|
||||
let mut resolved = HashMap::with_capacity(self.env.len());
|
||||
for (name, value) in &self.env {
|
||||
let references_secrets = value.references(Namespace::Secrets);
|
||||
let resolved_value = match value.resolve_with(&mut ctx) {
|
||||
Ok(resolved) => resolved,
|
||||
Err(err) if err.namespace == Namespace::Env && !references_secrets => {
|
||||
#[expect(
|
||||
clippy::disallowed_methods,
|
||||
reason = "intentional raw-source fallback preserves existing \
|
||||
environment variable behavior for env-only run environment values"
|
||||
)]
|
||||
let source = value.as_source();
|
||||
source
|
||||
}
|
||||
Err(err) => return Err(err),
|
||||
};
|
||||
resolved.insert(name.clone(), resolved_value);
|
||||
resolved.insert(name.clone(), value.resolve_with(&mut ctx)?);
|
||||
}
|
||||
Ok(resolved)
|
||||
}
|
||||
|
|
@ -1151,6 +1123,7 @@ fn pair_lookup(
|
|||
#[cfg(test)]
|
||||
mod run_environment_settings_tests {
|
||||
use super::{HashMap, InterpString, RunEnvironmentSettings, pair_lookup as lookup};
|
||||
use crate::settings::ResolveErrorKind;
|
||||
|
||||
fn settings(env: &[(&str, &str)]) -> RunEnvironmentSettings {
|
||||
RunEnvironmentSettings {
|
||||
|
|
@ -1163,33 +1136,20 @@ mod run_environment_settings_tests {
|
|||
}
|
||||
|
||||
#[test]
|
||||
fn resolve_env_substitutes_env_tokens_via_lookup() {
|
||||
let s = settings(&[("NODE_ENV", "{{ env.NODE_ENV }}"), ("STATIC", "value")]);
|
||||
let resolved = s
|
||||
.resolve_env(lookup(&[("NODE_ENV", "test")]), lookup(&[]))
|
||||
.unwrap();
|
||||
fn resolve_env_passes_through_literal_values() {
|
||||
let s = settings(&[("NODE_ENV", "production"), ("STATIC", "value")]);
|
||||
let resolved = s.resolve_env(lookup(&[])).unwrap();
|
||||
|
||||
assert_eq!(resolved.get("NODE_ENV"), Some(&"test".to_string()));
|
||||
assert_eq!(resolved.get("NODE_ENV"), Some(&"production".to_string()));
|
||||
assert_eq!(resolved.get("STATIC"), Some(&"value".to_string()));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resolve_env_falls_back_to_source_when_lookup_fails() {
|
||||
let s = settings(&[("NODE_ENV", "{{ env.MISSING_NODE_ENV }}")]);
|
||||
let resolved = s.resolve_env(lookup(&[]), lookup(&[])).unwrap();
|
||||
|
||||
assert_eq!(
|
||||
resolved.get("NODE_ENV"),
|
||||
Some(&"{{ env.MISSING_NODE_ENV }}".to_string())
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resolve_env_substitutes_secret_tokens_via_lookup() {
|
||||
let s = settings(&[("API_TOKEN", "Bearer {{ secrets.API_TOKEN }}")]);
|
||||
|
||||
let resolved = s
|
||||
.resolve_env(lookup(&[]), lookup(&[("API_TOKEN", "vault-token")]))
|
||||
.resolve_env(lookup(&[("API_TOKEN", "vault-token")]))
|
||||
.unwrap();
|
||||
|
||||
assert_eq!(
|
||||
|
|
@ -1202,31 +1162,28 @@ mod run_environment_settings_tests {
|
|||
fn resolve_env_returns_secret_error_without_source_fallback() {
|
||||
let s = settings(&[("API_TOKEN", "{{ secrets.MISSING_TOKEN }}")]);
|
||||
|
||||
let err = s.resolve_env(lookup(&[]), lookup(&[])).unwrap_err();
|
||||
let err = s.resolve_env(lookup(&[])).unwrap_err();
|
||||
|
||||
assert_eq!(err.namespace, super::Namespace::Secrets);
|
||||
assert_eq!(err.name, "MISSING_TOKEN");
|
||||
}
|
||||
|
||||
/// `{{ env.* }}` no longer resolves anywhere. It fails closed rather than
|
||||
/// falling back to source form, which previously let an unresolved token
|
||||
/// reach the sandbox as literal text.
|
||||
#[test]
|
||||
fn resolve_env_does_not_source_fallback_mixed_values_that_reference_secrets() {
|
||||
let s = settings(&[(
|
||||
"API_TOKEN",
|
||||
"{{ env.MISSING_PREFIX }} {{ secrets.API_TOKEN }}",
|
||||
)]);
|
||||
fn resolve_env_fails_closed_on_an_env_token() {
|
||||
let s = settings(&[("NODE_ENV", "{{ env.NODE_ENV }}")]);
|
||||
|
||||
let err = s
|
||||
.resolve_env(lookup(&[]), lookup(&[("API_TOKEN", "vault-token")]))
|
||||
.unwrap_err();
|
||||
let err = s.resolve_env(lookup(&[])).unwrap_err();
|
||||
|
||||
assert_eq!(err.namespace, super::Namespace::Env);
|
||||
assert_eq!(err.name, "MISSING_PREFIX");
|
||||
assert_eq!(err.kind, ResolveErrorKind::Unavailable);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resolve_env_is_empty_for_empty_settings() {
|
||||
let s: HashMap<String, String> =
|
||||
settings(&[]).resolve_env(lookup(&[]), lookup(&[])).unwrap();
|
||||
let s: HashMap<String, String> = settings(&[]).resolve_env(lookup(&[])).unwrap();
|
||||
assert!(s.is_empty());
|
||||
}
|
||||
}
|
||||
|
|
@ -1630,30 +1587,26 @@ impl McpServerSettings {
|
|||
/// error rather than passing through as literal text.
|
||||
pub fn resolve_transport_env(
|
||||
&self,
|
||||
mut env_lookup: impl FnMut(&str) -> Option<String>,
|
||||
mut secrets_lookup: impl FnMut(&str) -> Option<String>,
|
||||
) -> Result<Self, ResolveError> {
|
||||
let mut resolved = self.clone();
|
||||
visit_mcp_transport_strings(&mut resolved.transport, &mut |value| {
|
||||
resolve_env_string(value, &mut env_lookup, &mut secrets_lookup)
|
||||
resolve_env_string(value, &mut secrets_lookup)
|
||||
})?;
|
||||
Ok(resolved)
|
||||
}
|
||||
}
|
||||
|
||||
/// Resolve `{{ env.* }}` and `{{ secrets.* }}` tokens in one run-boundary
|
||||
/// Resolve `{{ secrets.* }}` tokens in one run-boundary
|
||||
/// string. A literal value (no tokens) round-trips unchanged.
|
||||
fn resolve_env_string(
|
||||
value: &mut String,
|
||||
env_lookup: &mut impl FnMut(&str) -> Option<String>,
|
||||
secrets_lookup: &mut impl FnMut(&str) -> Option<String>,
|
||||
) -> Result<(), ResolveError> {
|
||||
if !value.contains("{{") {
|
||||
return Ok(());
|
||||
}
|
||||
let mut ctx = ResolveCtx::new()
|
||||
.with_env(&mut *env_lookup)
|
||||
.with_secrets(&mut *secrets_lookup);
|
||||
let mut ctx = ResolveCtx::new().with_secrets(&mut *secrets_lookup);
|
||||
*value = InterpString::parse(value).resolve_with(&mut ctx)?;
|
||||
Ok(())
|
||||
}
|
||||
|
|
@ -1664,8 +1617,7 @@ mod resolve_transport_env_tests {
|
|||
|
||||
use super::super::interp::ResolveErrorKind;
|
||||
use super::{
|
||||
McpHttpProtocol, McpServerSettings, McpTransport, Namespace, pair_lookup as env_lookup,
|
||||
pair_lookup as secret_lookup,
|
||||
McpHttpProtocol, McpServerSettings, McpTransport, Namespace, pair_lookup as secret_lookup,
|
||||
};
|
||||
|
||||
#[test]
|
||||
|
|
@ -1679,9 +1631,7 @@ mod resolve_transport_env_tests {
|
|||
..McpServerSettings::default()
|
||||
};
|
||||
|
||||
let resolved = settings
|
||||
.resolve_transport_env(env_lookup(&[]), secret_lookup(&[]))
|
||||
.unwrap();
|
||||
let resolved = settings.resolve_transport_env(secret_lookup(&[])).unwrap();
|
||||
|
||||
let McpTransport::Stdio { command, env } = resolved.transport else {
|
||||
panic!("expected stdio transport");
|
||||
|
|
@ -1690,75 +1640,6 @@ mod resolve_transport_env_tests {
|
|||
assert_eq!(env.get("TOKEN").map(String::as_str), Some("literal-value"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn stdio_command_and_env_resolve() {
|
||||
let settings = McpServerSettings {
|
||||
name: "gemini".to_string(),
|
||||
transport: McpTransport::Stdio {
|
||||
command: vec!["python".to_string(), "{{ env.SERVER_PATH }}".to_string()],
|
||||
env: HashMap::from([(
|
||||
"GEMINI_API_KEY".to_string(),
|
||||
"{{ env.GEMINI_API_KEY }}".to_string(),
|
||||
)]),
|
||||
},
|
||||
..McpServerSettings::default()
|
||||
};
|
||||
|
||||
let resolved = settings
|
||||
.resolve_transport_env(
|
||||
env_lookup(&[
|
||||
("SERVER_PATH", "/srv/mcp.py"),
|
||||
("GEMINI_API_KEY", "real-key"),
|
||||
]),
|
||||
secret_lookup(&[]),
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
let McpTransport::Stdio { command, env } = resolved.transport else {
|
||||
panic!("expected stdio transport");
|
||||
};
|
||||
assert_eq!(command, vec![
|
||||
"python".to_string(),
|
||||
"/srv/mcp.py".to_string()
|
||||
]);
|
||||
assert_eq!(
|
||||
env.get("GEMINI_API_KEY").map(String::as_str),
|
||||
Some("real-key")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn http_url_and_headers_resolve() {
|
||||
let settings = McpServerSettings {
|
||||
name: "remote".to_string(),
|
||||
transport: McpTransport::Http {
|
||||
protocol: McpHttpProtocol::default(),
|
||||
url: "https://{{ env.MCP_HOST }}/mcp".to_string(),
|
||||
headers: HashMap::from([(
|
||||
"Authorization".to_string(),
|
||||
"Bearer {{ env.MCP_TOKEN }}".to_string(),
|
||||
)]),
|
||||
},
|
||||
..McpServerSettings::default()
|
||||
};
|
||||
|
||||
let resolved = settings
|
||||
.resolve_transport_env(
|
||||
env_lookup(&[("MCP_HOST", "mcp.example"), ("MCP_TOKEN", "abc123")]),
|
||||
secret_lookup(&[]),
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
let McpTransport::Http { url, headers, .. } = resolved.transport else {
|
||||
panic!("expected http transport");
|
||||
};
|
||||
assert_eq!(url, "https://mcp.example/mcp");
|
||||
assert_eq!(
|
||||
headers.get("Authorization").map(String::as_str),
|
||||
Some("Bearer abc123")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn missing_env_is_hard_error() {
|
||||
let settings = McpServerSettings {
|
||||
|
|
@ -1767,17 +1648,17 @@ mod resolve_transport_env_tests {
|
|||
command: vec!["python".to_string()],
|
||||
env: HashMap::from([(
|
||||
"GEMINI_API_KEY".to_string(),
|
||||
"{{ env.GEMINI_API_KEY }}".to_string(),
|
||||
"{{ secrets.GEMINI_API_KEY }}".to_string(),
|
||||
)]),
|
||||
},
|
||||
..McpServerSettings::default()
|
||||
};
|
||||
|
||||
let err = settings
|
||||
.resolve_transport_env(env_lookup(&[]), secret_lookup(&[]))
|
||||
.resolve_transport_env(secret_lookup(&[]))
|
||||
.unwrap_err();
|
||||
|
||||
assert_eq!(err.namespace, Namespace::Env);
|
||||
assert_eq!(err.namespace, Namespace::Secrets);
|
||||
assert_eq!(err.name, "GEMINI_API_KEY");
|
||||
assert_eq!(err.kind, ResolveErrorKind::Missing);
|
||||
}
|
||||
|
|
@ -1801,10 +1682,10 @@ mod resolve_transport_env_tests {
|
|||
};
|
||||
|
||||
let resolved = settings
|
||||
.resolve_transport_env(
|
||||
env_lookup(&[]),
|
||||
secret_lookup(&[("SERVER_BIN", "/srv/mcp"), ("API_TOKEN", "vault-token")]),
|
||||
)
|
||||
.resolve_transport_env(secret_lookup(&[
|
||||
("SERVER_BIN", "/srv/mcp"),
|
||||
("API_TOKEN", "vault-token"),
|
||||
]))
|
||||
.unwrap();
|
||||
|
||||
let McpTransport::Stdio { command, env } = resolved.transport else {
|
||||
|
|
@ -1837,10 +1718,10 @@ mod resolve_transport_env_tests {
|
|||
};
|
||||
|
||||
let resolved = settings
|
||||
.resolve_transport_env(
|
||||
env_lookup(&[]),
|
||||
secret_lookup(&[("MCP_HOST", "mcp.example"), ("MCP_TOKEN", "vault-token")]),
|
||||
)
|
||||
.resolve_transport_env(secret_lookup(&[
|
||||
("MCP_HOST", "mcp.example"),
|
||||
("MCP_TOKEN", "vault-token"),
|
||||
]))
|
||||
.unwrap();
|
||||
|
||||
let McpTransport::Http { url, headers, .. } = resolved.transport else {
|
||||
|
|
@ -1868,7 +1749,7 @@ mod resolve_transport_env_tests {
|
|||
};
|
||||
|
||||
let err = settings
|
||||
.resolve_transport_env(env_lookup(&[]), secret_lookup(&[]))
|
||||
.resolve_transport_env(secret_lookup(&[]))
|
||||
.unwrap_err();
|
||||
|
||||
assert_eq!(err.namespace, Namespace::Secrets);
|
||||
|
|
@ -1883,8 +1764,7 @@ mod resolve_step_env_tests {
|
|||
|
||||
use super::super::interp::ResolveErrorKind;
|
||||
use super::{
|
||||
Namespace, PreparedStep, PreparedStepRun, RunPrepareSettings, pair_lookup as env_lookup,
|
||||
pair_lookup as secret_lookup,
|
||||
Namespace, PreparedStep, PreparedStepRun, RunPrepareSettings, pair_lookup as secret_lookup,
|
||||
};
|
||||
|
||||
fn script_step(script: &str, env: HashMap<String, String>) -> PreparedStep {
|
||||
|
|
@ -1943,9 +1823,7 @@ mod resolve_step_env_tests {
|
|||
timeout_ms: 1_000,
|
||||
};
|
||||
|
||||
let resolved = settings
|
||||
.resolve_step_env(env_lookup(&[]), secret_lookup(&[]))
|
||||
.unwrap();
|
||||
let resolved = settings.resolve_step_env(secret_lookup(&[])).unwrap();
|
||||
|
||||
assert_eq!(resolved.steps[0].to_shell_command(), "echo hello");
|
||||
assert_eq!(
|
||||
|
|
@ -1956,19 +1834,18 @@ mod resolve_step_env_tests {
|
|||
|
||||
#[test]
|
||||
fn script_resolves_verbatim() {
|
||||
// A script is a raw shell snippet: its `{{ env.* }}` token resolves but
|
||||
// the result is NOT shell-quoted — the shell interprets the snippet as
|
||||
// written.
|
||||
// A script is a raw shell snippet: its token resolves but the result is
|
||||
// NOT shell-quoted — the shell interprets the snippet as written.
|
||||
let settings = RunPrepareSettings {
|
||||
steps: vec![script_step(
|
||||
"deploy {{ env.REGION }} && echo done",
|
||||
"deploy {{ secrets.REGION }} && echo done",
|
||||
HashMap::new(),
|
||||
)],
|
||||
timeout_ms: 1_000,
|
||||
};
|
||||
|
||||
let resolved = settings
|
||||
.resolve_step_env(env_lookup(&[("REGION", "us-east-1")]), secret_lookup(&[]))
|
||||
.resolve_step_env(secret_lookup(&[("REGION", "us-east-1")]))
|
||||
.unwrap();
|
||||
|
||||
assert_eq!(
|
||||
|
|
@ -1978,20 +1855,23 @@ mod resolve_step_env_tests {
|
|||
}
|
||||
|
||||
#[test]
|
||||
fn command_and_env_resolve() {
|
||||
fn command_and_env_resolve_secret_tokens() {
|
||||
let settings = RunPrepareSettings {
|
||||
steps: vec![command_step(
|
||||
&["deploy", "{{ env.REGION }}"],
|
||||
HashMap::from([("TOKEN".to_string(), "{{ env.DEPLOY_TOKEN }}".to_string())]),
|
||||
&["deploy", "{{ secrets.REGION }}"],
|
||||
HashMap::from([(
|
||||
"TOKEN".to_string(),
|
||||
"{{ secrets.DEPLOY_TOKEN }}".to_string(),
|
||||
)]),
|
||||
)],
|
||||
timeout_ms: 1_000,
|
||||
};
|
||||
|
||||
let resolved = settings
|
||||
.resolve_step_env(
|
||||
env_lookup(&[("REGION", "us-east-1"), ("DEPLOY_TOKEN", "secret-token")]),
|
||||
secret_lookup(&[]),
|
||||
)
|
||||
.resolve_step_env(secret_lookup(&[
|
||||
("REGION", "us-east-1"),
|
||||
("DEPLOY_TOKEN", "secret-token"),
|
||||
]))
|
||||
.unwrap();
|
||||
|
||||
assert_eq!(resolved.steps[0].to_shell_command(), "deploy us-east-1");
|
||||
|
|
@ -2006,15 +1886,15 @@ mod resolve_step_env_tests {
|
|||
// A resolved argv element that contains a space must survive as a
|
||||
// single shell word, not re-split into two.
|
||||
let settings = RunPrepareSettings {
|
||||
steps: vec![command_step(&["echo", "{{ env.MESSAGE }}"], HashMap::new())],
|
||||
steps: vec![command_step(
|
||||
&["echo", "{{ secrets.MESSAGE }}"],
|
||||
HashMap::new(),
|
||||
)],
|
||||
timeout_ms: 1_000,
|
||||
};
|
||||
|
||||
let resolved = settings
|
||||
.resolve_step_env(
|
||||
env_lookup(&[("MESSAGE", "hello world")]),
|
||||
secret_lookup(&[]),
|
||||
)
|
||||
.resolve_step_env(secret_lookup(&[("MESSAGE", "hello world")]))
|
||||
.unwrap();
|
||||
|
||||
let shell = resolved.steps[0].to_shell_command();
|
||||
|
|
@ -2032,17 +1912,14 @@ mod resolve_step_env_tests {
|
|||
let malicious = "x'; touch PWNED; echo '";
|
||||
let settings = RunPrepareSettings {
|
||||
steps: vec![command_step(
|
||||
&["echo", "{{ env.USER_INPUT }}"],
|
||||
&["echo", "{{ secrets.USER_INPUT }}"],
|
||||
HashMap::new(),
|
||||
)],
|
||||
timeout_ms: 1_000,
|
||||
};
|
||||
|
||||
let resolved = settings
|
||||
.resolve_step_env(
|
||||
|name| (name == "USER_INPUT").then(|| malicious.to_string()),
|
||||
secret_lookup(&[]),
|
||||
)
|
||||
.resolve_step_env(|name| (name == "USER_INPUT").then(|| malicious.to_string()))
|
||||
.unwrap();
|
||||
|
||||
let shell = resolved.steps[0].to_shell_command();
|
||||
|
|
@ -2063,39 +1940,38 @@ mod resolve_step_env_tests {
|
|||
}
|
||||
|
||||
#[test]
|
||||
fn missing_env_in_command_is_hard_error() {
|
||||
fn missing_secret_in_command_is_hard_error() {
|
||||
let settings = RunPrepareSettings {
|
||||
steps: vec![command_step(
|
||||
&["deploy", "{{ env.REGION }}"],
|
||||
&["deploy", "{{ secrets.REGION }}"],
|
||||
HashMap::new(),
|
||||
)],
|
||||
timeout_ms: 1_000,
|
||||
};
|
||||
|
||||
let err = settings
|
||||
.resolve_step_env(env_lookup(&[]), secret_lookup(&[]))
|
||||
.unwrap_err();
|
||||
let err = settings.resolve_step_env(secret_lookup(&[])).unwrap_err();
|
||||
|
||||
assert_eq!(err.namespace, Namespace::Env);
|
||||
assert_eq!(err.namespace, Namespace::Secrets);
|
||||
assert_eq!(err.name, "REGION");
|
||||
assert_eq!(err.kind, ResolveErrorKind::Missing);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn missing_env_in_step_env_value_is_hard_error() {
|
||||
fn missing_secret_in_step_env_value_is_hard_error() {
|
||||
let settings = RunPrepareSettings {
|
||||
steps: vec![script_step(
|
||||
"echo hi",
|
||||
HashMap::from([("TOKEN".to_string(), "{{ env.DEPLOY_TOKEN }}".to_string())]),
|
||||
HashMap::from([(
|
||||
"TOKEN".to_string(),
|
||||
"{{ secrets.DEPLOY_TOKEN }}".to_string(),
|
||||
)]),
|
||||
)],
|
||||
timeout_ms: 1_000,
|
||||
};
|
||||
|
||||
let err = settings
|
||||
.resolve_step_env(env_lookup(&[]), secret_lookup(&[]))
|
||||
.unwrap_err();
|
||||
let err = settings.resolve_step_env(secret_lookup(&[])).unwrap_err();
|
||||
|
||||
assert_eq!(err.namespace, Namespace::Env);
|
||||
assert_eq!(err.namespace, Namespace::Secrets);
|
||||
assert_eq!(err.name, "DEPLOY_TOKEN");
|
||||
assert_eq!(err.kind, ResolveErrorKind::Missing);
|
||||
}
|
||||
|
|
@ -2117,14 +1993,11 @@ mod resolve_step_env_tests {
|
|||
};
|
||||
|
||||
let resolved = settings
|
||||
.resolve_step_env(
|
||||
env_lookup(&[]),
|
||||
secret_lookup(&[
|
||||
("REGION", "us-east-1"),
|
||||
("DEPLOY_TOKEN", "vault-token"),
|
||||
("MESSAGE", "hello world"),
|
||||
]),
|
||||
)
|
||||
.resolve_step_env(secret_lookup(&[
|
||||
("REGION", "us-east-1"),
|
||||
("DEPLOY_TOKEN", "vault-token"),
|
||||
("MESSAGE", "hello world"),
|
||||
]))
|
||||
.unwrap();
|
||||
|
||||
assert_eq!(
|
||||
|
|
@ -2148,9 +2021,7 @@ mod resolve_step_env_tests {
|
|||
timeout_ms: 1_000,
|
||||
};
|
||||
|
||||
let err = settings
|
||||
.resolve_step_env(env_lookup(&[]), secret_lookup(&[]))
|
||||
.unwrap_err();
|
||||
let err = settings.resolve_step_env(secret_lookup(&[])).unwrap_err();
|
||||
|
||||
assert_eq!(err.namespace, Namespace::Secrets);
|
||||
assert_eq!(err.name, "API_KEY");
|
||||
|
|
@ -2232,12 +2103,10 @@ pub enum HookType {
|
|||
command: InterpString,
|
||||
},
|
||||
Http {
|
||||
url: InterpString,
|
||||
headers: Option<HashMap<String, InterpString>>,
|
||||
url: InterpString,
|
||||
headers: Option<HashMap<String, InterpString>>,
|
||||
#[serde(default)]
|
||||
allowed_env_vars: Vec<String>,
|
||||
#[serde(default)]
|
||||
tls: TlsMode,
|
||||
tls: TlsMode,
|
||||
},
|
||||
Prompt {
|
||||
prompt: InterpString,
|
||||
|
|
|
|||
|
|
@ -27,10 +27,6 @@ export interface HookDefinition {
|
|||
'type'?: HookDefinitionTypeEnum | null;
|
||||
'url'?: string | null;
|
||||
'headers'?: { [key: string]: string; } | null;
|
||||
/**
|
||||
* Allowlist of environment variable names that an http hook header may read via `{{ env.NAME }}`. An empty list (the default) permits no env vars in headers.
|
||||
*/
|
||||
'allowed_env_vars'?: Array<string>;
|
||||
'tls'?: TlsMode;
|
||||
'prompt'?: string | null;
|
||||
'model'?: string | null;
|
||||
|
|
|
|||
|
|
@ -22,6 +22,6 @@ import type { PreparedScriptStep } from './prepared-script-step';
|
|||
|
||||
/**
|
||||
* @type PreparedStep
|
||||
* A single resolved prepare step. The runnable part preserves the script-vs-argv distinction via the `type` discriminator: a `script` is a raw shell snippet kept verbatim, while a `command` is an argv whose elements are shell-quoted and joined at the run boundary (after `{{ env.* }}` resolution) so an interpolated value cannot inject shell syntax. Optional per-step `env` is shared by both shapes.
|
||||
* A single resolved prepare step. The runnable part preserves the script-vs-argv distinction via the `type` discriminator: a `script` is a raw shell snippet kept verbatim, while a `command` is an argv whose elements are shell-quoted and joined at the run boundary (after `{{ secrets.* }}` resolution) so an interpolated value cannot inject shell syntax. Optional per-step `env` is shared by both shapes.
|
||||
*/
|
||||
export type PreparedStep = { type: 'command' } & PreparedCommandStep | { type: 'script' } & PreparedScriptStep;
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue