mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-10-06 02:48:25 +00:00
Merge pull request #665 from fabro-sh/refactor/remove-env-interpolation
Some checks are pending
TypeScript / Build (push) Waiting to run
Rust / Format (push) Waiting to run
Rust / Clippy (push) Waiting to run
Rust / Generated Docs (push) Waiting to run
Rust / Test (Linux) (push) Waiting to run
Rust / Test (macOS) (push) Waiting to run
TypeScript / Typecheck (push) Waiting to run
TypeScript / Test (push) Waiting to run
Some checks are pending
TypeScript / Build (push) Waiting to run
Rust / Format (push) Waiting to run
Rust / Clippy (push) Waiting to run
Rust / Generated Docs (push) Waiting to run
Rust / Test (Linux) (push) Waiting to run
Rust / Test (macOS) (push) Waiting to run
TypeScript / Typecheck (push) Waiting to run
TypeScript / Test (push) Waiting to run
Remove process-environment interpolation from config
This commit is contained in:
commit
826c8be519
58 changed files with 904 additions and 1436 deletions
|
|
@ -218,7 +218,7 @@ provider = "s3"
|
|||
disk_cache = true
|
||||
|
||||
[server.slatedb.s3]
|
||||
bucket = "{{ env.SLATEDB_BUCKET }}"
|
||||
bucket = "fabro-production"
|
||||
region = "us-east-1"
|
||||
```
|
||||
|
||||
|
|
@ -466,7 +466,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 }}"
|
||||
X-Deployment-Environment = "{{ vars.DEPLOY_ENV }}"
|
||||
```
|
||||
|
||||
| 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. Use variables only for non-sensitive metadata. Do not store tokens, API keys, or other credentials in variables or literal hook configuration. `{{ 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.
|
||||
Interpolation applies to stdio and sandbox commands and env values, plus HTTP URLs and headers. Variable tokens are replaced in the created run configuration. Secret expressions remain in persisted configuration, while resolved secret values do not. A missing or non-token secret fails MCP startup instead of passing an unresolved token to the transport. `{{ env.* }}` is unsupported and also fails before launch.
|
||||
|
||||
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
|
||||
|
||||
|
|
|
|||
|
|
@ -14126,7 +14126,7 @@ components:
|
|||
$ref: "#/components/schemas/RunNamespace"
|
||||
|
||||
InterpString:
|
||||
description: Resolved config string that may contain env interpolation tokens.
|
||||
description: Config string that can contain typed interpolation tokens.
|
||||
type: string
|
||||
|
||||
StringMap:
|
||||
|
|
@ -14344,7 +14344,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]
|
||||
|
|
@ -14749,17 +14749,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]
|
||||
|
|
@ -206,13 +206,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]`
|
||||
|
||||
|
|
@ -309,13 +309,13 @@ When `provider = "local"`, Fabro runs directly in the resolved working
|
|||
directory. If you want local isolation, create or enter a separate clone or Git
|
||||
worktree yourself.
|
||||
|
||||
Environment variable values can combine literal text with server variables, worker environment variables, and token secrets:
|
||||
Environment variable values can combine literal text with server variables and token secrets:
|
||||
|
||||
```toml title="run.toml"
|
||||
[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 }}"
|
||||
```
|
||||
|
||||
|
|
@ -323,11 +323,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]`
|
||||
|
||||
|
|
@ -363,13 +362,13 @@ 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.
|
||||
|
||||
`run.failed` is emitted only when the run terminally fails. A failed stage that routes onward to a normal completion path produces `run.completed`, not `run.failed`.
|
||||
|
||||
If a Slack route's channel is missing, empty, or references an unresolved environment variable, Fabro logs a warning and skips that route. Delivery failures are logged and never fail or alter the run.
|
||||
If a Slack route's channel is missing, empty, or contains an unsupported interpolation token, Fabro logs a warning and skips that route. Delivery failures are logged and never fail or alter the run.
|
||||
|
||||
### `[run.checkpoint]`
|
||||
|
||||
|
|
@ -503,7 +502,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.
|
||||
|
||||
|
|
|
|||
|
|
@ -113,23 +113,34 @@ digraph Example {
|
|||
|
||||
## Direct SDK environment credentials
|
||||
|
||||
The built-in Modal provider reads its two headers from the Fabro vault. Direct SDK code that uses `EnvCredentialSource` must explicitly change those header sources to environment variables:
|
||||
The built-in Modal provider reads its two headers from the Fabro vault. `EnvCredentialSource` does not configure Modal automatically because Modal uses two headers instead of one API-key reference.
|
||||
|
||||
For direct SDK use, enable Modal and set its endpoint URL in the catalog:
|
||||
|
||||
```toml title="settings.toml"
|
||||
[llm.providers.modal]
|
||||
enabled = true
|
||||
base_url = "https://your-endpoint.modal.run/v1"
|
||||
|
||||
[llm.providers.modal.extra_headers]
|
||||
"Modal-Key" = "{{ env.MODAL_TOKEN_ID }}"
|
||||
"Modal-Secret" = "{{ env.MODAL_TOKEN_SECRET }}"
|
||||
```
|
||||
|
||||
Then export both values before starting the process:
|
||||
Then read both environment variables explicitly and create a typed credential after constructing `catalog` from those settings:
|
||||
|
||||
```bash
|
||||
export MODAL_TOKEN_ID=wk-...
|
||||
export MODAL_TOKEN_SECRET=ws-...
|
||||
```rust
|
||||
use fabro_auth::ApiCredential;
|
||||
use fabro_llm::client::Client;
|
||||
use std::collections::HashMap;
|
||||
|
||||
let credential = ApiCredential::with_extra_headers(
|
||||
"modal",
|
||||
HashMap::from([
|
||||
("Modal-Key".to_string(), std::env::var("MODAL_TOKEN_ID")?),
|
||||
(
|
||||
"Modal-Secret".to_string(),
|
||||
std::env::var("MODAL_TOKEN_SECRET")?,
|
||||
),
|
||||
]),
|
||||
);
|
||||
let client = Client::from_credentials(vec![credential], catalog).await?;
|
||||
```
|
||||
|
||||
## Costs
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
||||
|
|
|
|||
|
|
@ -372,20 +372,35 @@ For env-backed usage, `EnvCredentialSource` checks for API key environment varia
|
|||
| `INCEPTION_API_KEY` | Inception |
|
||||
| `POOLSIDE_API_KEY` | Poolside |
|
||||
| `OPENROUTER_API_KEY` | OpenRouter, when enabled in settings |
|
||||
| `MODAL_TOKEN_ID` and `MODAL_TOKEN_SECRET` | Modal, when enabled and configured as below |
|
||||
|
||||
The first provider registered becomes the default. Provider base URLs come from the model catalog. For vault-backed usage inside Fabro, use `fabro_auth::VaultCredentialSource` instead.
|
||||
|
||||
The built-in Modal definition reads proxy-token headers from the vault. To use `EnvCredentialSource` directly, enable Modal, set its endpoint URL, and override both header sources:
|
||||
The built-in Modal definition reads two proxy-token headers from the vault, so `EnvCredentialSource` does not configure it automatically. For direct SDK use, enable Modal and set its endpoint URL in the catalog:
|
||||
|
||||
```toml
|
||||
[llm.providers.modal]
|
||||
enabled = true
|
||||
base_url = "https://your-endpoint.modal.run/v1"
|
||||
```
|
||||
|
||||
[llm.providers.modal.extra_headers]
|
||||
"Modal-Key" = "{{ env.MODAL_TOKEN_ID }}"
|
||||
"Modal-Secret" = "{{ env.MODAL_TOKEN_SECRET }}"
|
||||
Then read the two environment variables explicitly and create a typed credential after constructing `catalog` from those settings:
|
||||
|
||||
```rust
|
||||
use fabro_auth::ApiCredential;
|
||||
use fabro_llm::client::Client;
|
||||
use std::collections::HashMap;
|
||||
|
||||
let credential = ApiCredential::with_extra_headers(
|
||||
"modal",
|
||||
HashMap::from([
|
||||
("Modal-Key".to_string(), std::env::var("MODAL_TOKEN_ID")?),
|
||||
(
|
||||
"Modal-Secret".to_string(),
|
||||
std::env::var("MODAL_TOKEN_SECRET")?,
|
||||
),
|
||||
]),
|
||||
);
|
||||
let client = Client::from_credentials(vec![credential], catalog).await?;
|
||||
```
|
||||
|
||||
#### Creating manually
|
||||
|
|
|
|||
|
|
@ -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 literal text or `{{ secrets.NAME }}` interpolation strings. Put credentials in a secret and reference them with a 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
|
||||
|
||||
|
|
|
|||
|
|
@ -1036,7 +1036,7 @@ pub(crate) struct RunWorkerArgs {
|
|||
|
||||
/// Fabro storage directory for loading worker-visible secrets
|
||||
#[arg(long, hide = true)]
|
||||
pub(crate) storage_dir: Option<PathBuf>,
|
||||
pub(crate) storage_dir: PathBuf,
|
||||
|
||||
/// Run scratch directory
|
||||
#[arg(long)]
|
||||
|
|
|
|||
|
|
@ -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>> {
|
||||
|
|
@ -341,17 +333,14 @@ pub(crate) async fn execute(mut args: ExecArgs, ctx: &CommandContext) -> AnyResu
|
|||
.transpose()?
|
||||
.unwrap_or_default(),
|
||||
};
|
||||
// Resolve `{{ env.* }}` in MCP transport config at the exec boundary,
|
||||
// against the CLI process env — the mirror of the `fabro run` worker
|
||||
// boundary in `fabro_workflow::operations::start::runtime_mcp_server`.
|
||||
// Both consumers read the same source-form settings; missing env is a hard
|
||||
// error. `fabro exec` has no server vault, so secrets/inputs tokens surface
|
||||
// loudly rather than leaking.
|
||||
// Fully validate MCP transport config at the exec boundary. `fabro exec`
|
||||
// has no server vault, so secret and unsupported tokens fail instead of
|
||||
// reaching the transport.
|
||||
let mcp_servers = mcp_servers
|
||||
.into_iter()
|
||||
.map(|settings| {
|
||||
settings
|
||||
.resolve_transport_env(process_env_var, |_| None)
|
||||
.resolve_transport_secrets(|_| None)
|
||||
.with_context(|| format!("failed to resolve MCP server {:?}", settings.name))
|
||||
})
|
||||
.collect::<AnyResult<Vec<_>>>()?;
|
||||
|
|
|
|||
|
|
@ -76,7 +76,7 @@ enum WorkerTitlePhase {
|
|||
pub(crate) async fn execute(
|
||||
run_id: RunId,
|
||||
server: String,
|
||||
storage_dir: Option<PathBuf>,
|
||||
storage_dir: PathBuf,
|
||||
run_dir: PathBuf,
|
||||
mode: RunWorkerMode,
|
||||
worker_token: &str,
|
||||
|
|
@ -136,13 +136,10 @@ pub(crate) async fn execute(
|
|||
if let Some(control_manager) = &mut control_manager {
|
||||
control_manager.wait_for_first_connection().await?;
|
||||
}
|
||||
let vault = load_worker_vault(storage_dir.as_deref()).await?;
|
||||
let vault = load_worker_vault(&storage_dir).await?;
|
||||
let github_app = {
|
||||
let vault_guard = match &vault {
|
||||
Some(arc) => Some(arc.read().await),
|
||||
None => None,
|
||||
};
|
||||
maybe_build_github_credentials(&run_spec.settings, vault_guard.as_deref())?
|
||||
let vault_guard = vault.read().await;
|
||||
maybe_build_github_credentials(&run_spec.settings, &vault_guard)?
|
||||
};
|
||||
let services = StartServices {
|
||||
run_id,
|
||||
|
|
@ -169,7 +166,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,
|
||||
|
|
@ -263,11 +261,11 @@ impl fabro_tool::RunManifestBuilder for WorkerRunManifestBuilder {
|
|||
}
|
||||
}
|
||||
|
||||
async fn load_worker_vault(storage_dir: Option<&Path>) -> Result<Option<Arc<AsyncRwLock<Vault>>>> {
|
||||
let Some(storage_dir) = storage_dir else {
|
||||
return Ok(None);
|
||||
};
|
||||
|
||||
/// Load the worker's secret vault from the run's storage root.
|
||||
///
|
||||
/// A worker always receives the server storage root so it can load the same
|
||||
/// secret vault as the server.
|
||||
async fn load_worker_vault(storage_dir: &Path) -> Result<Arc<AsyncRwLock<Vault>>> {
|
||||
let storage = Storage::new(storage_dir);
|
||||
let vault = SecretStore::open_snapshot(storage.sqlite_path(), storage.secrets_path())
|
||||
.await
|
||||
|
|
@ -278,7 +276,7 @@ async fn load_worker_vault(storage_dir: Option<&Path>) -> Result<Option<Arc<Asyn
|
|||
)
|
||||
})?
|
||||
.into_vault();
|
||||
Ok(Some(Arc::new(AsyncRwLock::new(vault))))
|
||||
Ok(Arc::new(AsyncRwLock::new(vault)))
|
||||
}
|
||||
|
||||
const WORKER_CONTROL_RECONNECT_INITIAL_BACKOFF: Duration = Duration::from_millis(100);
|
||||
|
|
@ -1103,7 +1101,7 @@ fn stamp_system_worker(mut event: RunEvent) -> RunEvent {
|
|||
|
||||
fn maybe_build_github_credentials(
|
||||
settings: &WorkflowSettings,
|
||||
vault: Option<&fabro_vault::Vault>,
|
||||
vault: &fabro_vault::Vault,
|
||||
) -> Result<Option<fabro_github::GitHubCredentials>> {
|
||||
let resolved_run = &settings.run;
|
||||
let resolved_server = ServerSettingsBuilder::load_default().ok();
|
||||
|
|
@ -1134,14 +1132,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
|
||||
|
|
@ -1739,7 +1729,7 @@ mod tests {
|
|||
.set("ANTHROPIC_API_KEY", "vault-key", SecretType::Token, None)
|
||||
.unwrap();
|
||||
|
||||
let loaded = load_worker_vault(Some(temp.path())).await.unwrap().unwrap();
|
||||
let loaded = load_worker_vault(temp.path()).await.unwrap();
|
||||
let guard = loaded.read().await;
|
||||
let credential = guard.get("ANTHROPIC_API_KEY").unwrap();
|
||||
|
||||
|
|
|
|||
|
|
@ -496,7 +496,7 @@ async fn pre_tracing_bootstrap(command: &Commands) -> Result<PreTracingBootstrap
|
|||
.await
|
||||
}
|
||||
Commands::RunCmd(RunCommands::RunWorker(args)) => {
|
||||
prepare_run_worker_bootstrap(args.storage_dir.as_deref(), &args.run_dir)
|
||||
prepare_run_worker_bootstrap(&args.storage_dir, &args.run_dir)
|
||||
}
|
||||
_ => Ok(PreTracingBootstrap::cli()),
|
||||
}
|
||||
|
|
@ -541,10 +541,10 @@ async fn prepare_server_bootstrap(
|
|||
}
|
||||
|
||||
fn prepare_run_worker_bootstrap(
|
||||
storage_dir: Option<&std::path::Path>,
|
||||
storage_dir: &std::path::Path,
|
||||
run_dir: &std::path::Path,
|
||||
) -> Result<PreTracingBootstrap> {
|
||||
let local_config = local_server::LocalServerConfig::load_with_storage_dir(storage_dir)?;
|
||||
let local_config = local_server::LocalServerConfig::load_with_storage_dir(Some(storage_dir))?;
|
||||
let runtime_directory = fabro_config::RuntimeDirectory::new(local_config.storage_dir());
|
||||
let log_destination = fabro_config::resolve_log_destination(
|
||||
local_config.config_log_destination().unwrap_or_default(),
|
||||
|
|
@ -1515,6 +1515,8 @@ destination = "{destination}"
|
|||
"__run-worker",
|
||||
"--server",
|
||||
"/tmp/fabro.sock",
|
||||
"--storage-dir",
|
||||
"/tmp/storage",
|
||||
"--run-dir",
|
||||
"/tmp/run",
|
||||
"--run-id",
|
||||
|
|
@ -1526,6 +1528,7 @@ destination = "{destination}"
|
|||
match *cli.command.unwrap() {
|
||||
Commands::RunCmd(RunCommands::RunWorker(args)) => {
|
||||
assert_eq!(args.server, "/tmp/fabro.sock");
|
||||
assert_eq!(args.storage_dir, std::path::PathBuf::from("/tmp/storage"));
|
||||
assert_eq!(args.run_dir, std::path::PathBuf::from("/tmp/run"));
|
||||
assert_eq!(args.run_id, "01ARZ3NDEKTSV4RRFFQ69G5FAV".parse().unwrap());
|
||||
assert!(matches!(args.mode, args::RunWorkerMode::Start));
|
||||
|
|
@ -1541,6 +1544,8 @@ destination = "{destination}"
|
|||
"__run-worker",
|
||||
"--server",
|
||||
"http://127.0.0.1:3000",
|
||||
"--storage-dir",
|
||||
"/tmp/storage",
|
||||
"--run-dir",
|
||||
"/tmp/run",
|
||||
"--run-id",
|
||||
|
|
@ -1552,6 +1557,7 @@ destination = "{destination}"
|
|||
match *cli.command.unwrap() {
|
||||
Commands::RunCmd(RunCommands::RunWorker(args)) => {
|
||||
assert_eq!(args.server, "http://127.0.0.1:3000");
|
||||
assert_eq!(args.storage_dir, std::path::PathBuf::from("/tmp/storage"));
|
||||
assert_eq!(args.run_dir, std::path::PathBuf::from("/tmp/run"));
|
||||
assert_eq!(args.run_id, "01ARZ3NDEKTSV4RRFFQ69G5FAV".parse().unwrap());
|
||||
assert!(matches!(args.mode, args::RunWorkerMode::Resume));
|
||||
|
|
|
|||
|
|
@ -8,7 +8,7 @@ pub(crate) fn build_github_credentials(
|
|||
strategy: GithubIntegrationStrategy,
|
||||
app_id: Option<&str>,
|
||||
app_slug: Option<&str>,
|
||||
vault: Option<&Vault>,
|
||||
vault: &Vault,
|
||||
) -> anyhow::Result<Option<GitHubCredentials>> {
|
||||
match strategy {
|
||||
GithubIntegrationStrategy::App => {
|
||||
|
|
@ -31,7 +31,7 @@ pub(crate) fn build_github_credentials(
|
|||
|
||||
/// Look up GitHub token: GITHUB_TOKEN env -> vault GITHUB_TOKEN -> GH_TOKEN env
|
||||
/// -> vault GH_TOKEN
|
||||
fn lookup_github_token(vault: Option<&Vault>) -> Option<String> {
|
||||
fn lookup_github_token(vault: &Vault) -> Option<String> {
|
||||
lookup_env_or_vault(EnvVars::GITHUB_TOKEN, vault)
|
||||
.or_else(|| lookup_env_or_vault(EnvVars::GH_TOKEN, vault))
|
||||
}
|
||||
|
|
@ -40,10 +40,10 @@ fn lookup_github_token(vault: Option<&Vault>) -> Option<String> {
|
|||
clippy::disallowed_methods,
|
||||
reason = "GitHub credential resolution intentionally falls back from vault to documented process-env names."
|
||||
)]
|
||||
fn lookup_env_or_vault(name: &str, vault: Option<&Vault>) -> Option<String> {
|
||||
fn lookup_env_or_vault(name: &str, vault: &Vault) -> Option<String> {
|
||||
std::env::var(name)
|
||||
.ok()
|
||||
.or_else(|| vault.and_then(|v| v.get(name).map(str::to_string)))
|
||||
.or_else(|| vault.get(name).map(str::to_string))
|
||||
.map(|t| t.trim().to_string())
|
||||
.filter(|t| !t.is_empty())
|
||||
}
|
||||
|
|
|
|||
|
|
@ -69,19 +69,17 @@ fn spawn_worker_process(
|
|||
"FABRO_WORKER_TOKEN",
|
||||
issue_test_worker_jwt(&context.storage_dir, run_id),
|
||||
);
|
||||
cmd.args([
|
||||
"__run-worker",
|
||||
"--server",
|
||||
server,
|
||||
"--run-dir",
|
||||
run_dir
|
||||
.to_str()
|
||||
.expect("run directory path should be valid UTF-8"),
|
||||
"--run-id",
|
||||
run_id,
|
||||
"--mode",
|
||||
mode,
|
||||
]);
|
||||
cmd.arg("__run-worker")
|
||||
.arg("--storage-dir")
|
||||
.arg(&context.storage_dir)
|
||||
.arg("--server")
|
||||
.arg(server)
|
||||
.arg("--run-dir")
|
||||
.arg(run_dir)
|
||||
.arg("--run-id")
|
||||
.arg(run_id)
|
||||
.arg("--mode")
|
||||
.arg(mode);
|
||||
cmd.stdin(Stdio::piped());
|
||||
cmd.stdout(Stdio::piped());
|
||||
cmd.stderr(Stdio::piped());
|
||||
|
|
@ -124,8 +122,16 @@ fn child_output(mut child: Child, status: ExitStatus) -> Output {
|
|||
}
|
||||
}
|
||||
|
||||
fn worker_command(context: &fabro_test::TestContext, run_id: &str) -> assert_cmd::Command {
|
||||
fn worker_base_command(context: &fabro_test::TestContext) -> assert_cmd::Command {
|
||||
let mut cmd = context.command();
|
||||
cmd.arg("__run-worker")
|
||||
.arg("--storage-dir")
|
||||
.arg(&context.storage_dir);
|
||||
cmd
|
||||
}
|
||||
|
||||
fn worker_command(context: &fabro_test::TestContext, run_id: &str) -> assert_cmd::Command {
|
||||
let mut cmd = worker_base_command(context);
|
||||
cmd.env(
|
||||
"FABRO_WORKER_TOKEN",
|
||||
issue_test_worker_jwt(&context.storage_dir, run_id),
|
||||
|
|
@ -189,7 +195,7 @@ fn help() {
|
|||
----- stdout -----
|
||||
Internal: execute a single workflow run locally
|
||||
|
||||
Usage: fabro __run-worker [OPTIONS] --server <SERVER> --run-dir <RUN_DIR> --run-id <RUN_ID> --mode <MODE>
|
||||
Usage: fabro __run-worker [OPTIONS] --server <SERVER> --storage-dir <STORAGE_DIR> --run-dir <RUN_DIR> --run-id <RUN_ID> --mode <MODE>
|
||||
|
||||
Options:
|
||||
--json Output as JSON [env: FABRO_JSON=]
|
||||
|
|
@ -211,10 +217,8 @@ fn worker_requires_fabro_worker_token_env() {
|
|||
let context = auth_context();
|
||||
let run_dir = tempfile::tempdir().unwrap();
|
||||
let run_id = unique_run_id();
|
||||
let output = context
|
||||
.command()
|
||||
let output = worker_base_command(&context)
|
||||
.args([
|
||||
"__run-worker",
|
||||
"--server",
|
||||
"http://127.0.0.1:32276",
|
||||
"--run-dir",
|
||||
|
|
@ -272,7 +276,6 @@ digraph CachedGraph {
|
|||
|
||||
let output = worker_command(&context, run_id.as_str())
|
||||
.args([
|
||||
"__run-worker",
|
||||
"--server",
|
||||
server.as_str(),
|
||||
"--run-dir",
|
||||
|
|
@ -341,7 +344,6 @@ digraph GitHubApp {
|
|||
let mut cmd = worker_command(&context, run_id.as_str());
|
||||
cmd.env("GITHUB_APP_PRIVATE_KEY", "%%%not-base64%%%");
|
||||
cmd.args([
|
||||
"__run-worker",
|
||||
"--server",
|
||||
server.as_str(),
|
||||
"--run-dir",
|
||||
|
|
@ -390,7 +392,6 @@ digraph DetachedStoreOnly {
|
|||
let server = server_target(&context.storage_dir);
|
||||
let output = worker_command(&context, run_id.as_str())
|
||||
.args([
|
||||
"__run-worker",
|
||||
"--server",
|
||||
server.as_str(),
|
||||
"--run-dir",
|
||||
|
|
@ -608,7 +609,6 @@ digraph Test {
|
|||
|
||||
let mut cmd = worker_command(&context, &run_id);
|
||||
cmd.args([
|
||||
"__run-worker",
|
||||
"--server",
|
||||
&server,
|
||||
"--run-dir",
|
||||
|
|
@ -673,7 +673,6 @@ fn runner_reports_malformed_run_state_without_prefetching_events() {
|
|||
|
||||
let output = worker_command(&context, &run_id)
|
||||
.args([
|
||||
"__run-worker",
|
||||
"--server",
|
||||
&format!("{}/api/v1", server.base_url()),
|
||||
"--run-dir",
|
||||
|
|
|
|||
|
|
@ -393,17 +393,17 @@ fn runner_rejects_bogus_worker_token_against_github_only_server() {
|
|||
cmd.env("FABRO_HOME", &worker_home);
|
||||
cmd.env("FABRO_AUTH_FILE", &auth_file);
|
||||
cmd.env("FABRO_WORKER_TOKEN", bogus_token);
|
||||
cmd.args([
|
||||
"__run-worker",
|
||||
"--server",
|
||||
&target,
|
||||
"--run-dir",
|
||||
run_dir.to_str().unwrap(),
|
||||
"--run-id",
|
||||
&run_id,
|
||||
"--mode",
|
||||
"start",
|
||||
]);
|
||||
cmd.arg("__run-worker")
|
||||
.arg("--server")
|
||||
.arg(&target)
|
||||
.arg("--storage-dir")
|
||||
.arg(&server.storage_dir)
|
||||
.arg("--run-dir")
|
||||
.arg(&run_dir)
|
||||
.arg("--run-id")
|
||||
.arg(&run_id)
|
||||
.arg("--mode")
|
||||
.arg("start");
|
||||
cmd.stdin(Stdio::null());
|
||||
cmd.stdout(Stdio::piped());
|
||||
cmd.stderr(Stdio::piped());
|
||||
|
|
|
|||
|
|
@ -109,6 +109,7 @@ fabro-build-support = { path = "../../foundation/build-support" }
|
|||
chrono = { workspace = true }
|
||||
|
||||
[dev-dependencies]
|
||||
fabro-auth = { path = "../../foundation/fabro-auth", features = ["test-support"] }
|
||||
tokio = { workspace = true, features = ["test-util", "macros"] }
|
||||
tower = "0.5"
|
||||
http-body-util = "0.1"
|
||||
|
|
|
|||
|
|
@ -45,7 +45,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;
|
||||
|
||||
|
|
@ -590,9 +589,10 @@ async fn build_preflight_report(
|
|||
llm_result,
|
||||
)
|
||||
.await;
|
||||
run_github_token_check(&mut checks, prepared, &resolved_run, github_app).await;
|
||||
let github_token_ok =
|
||||
run_github_token_check(&mut checks, prepared, &resolved_run, github_app).await;
|
||||
|
||||
let checks_ok = sandbox_ok && repository_access_ok && llm_ok;
|
||||
let checks_ok = sandbox_ok && repository_access_ok && llm_ok && github_token_ok;
|
||||
|
||||
Ok((
|
||||
CheckReport {
|
||||
|
|
@ -1210,48 +1210,63 @@ async fn run_github_token_check(
|
|||
prepared: &PreparedManifest,
|
||||
resolved_run: &RunNamespace,
|
||||
github_app: Option<fabro_github::GitHubCredentials>,
|
||||
) {
|
||||
) -> bool {
|
||||
if !resolved_run.integrations.github.is_token_requested() {
|
||||
return;
|
||||
return true;
|
||||
}
|
||||
|
||||
// Resolve InterpString permission values eagerly for token minting and
|
||||
// for display in the preflight report.
|
||||
let github_permissions = resolved_run
|
||||
.integrations
|
||||
.github
|
||||
.resolve_permissions(process_env_var);
|
||||
let github_permissions = match resolved_run.integrations.github.resolve_permissions() {
|
||||
Ok(permissions) => permissions,
|
||||
Err(err) => {
|
||||
checks.push(CheckResult {
|
||||
name: "GitHub Token".into(),
|
||||
status: CheckStatus::Error,
|
||||
summary: "invalid permissions".into(),
|
||||
details: vec![],
|
||||
remediation: Some(format!("Failed to resolve GitHub permissions: {err}")),
|
||||
});
|
||||
return false;
|
||||
}
|
||||
};
|
||||
|
||||
let perm_details = github_permissions
|
||||
.iter()
|
||||
.map(|(key, value)| CheckDetail::new(format!("{key}: {value}")))
|
||||
.collect::<Vec<_>>();
|
||||
match (&github_app, prepared.git.as_ref()) {
|
||||
(Some(creds), Some(git)) => {
|
||||
match mint_github_token(creds, &git.origin_url, &github_permissions).await {
|
||||
Ok(_) => checks.push(CheckResult {
|
||||
if let (Some(creds), Some(git)) = (&github_app, prepared.git.as_ref()) {
|
||||
match mint_github_token(creds, &git.origin_url, &github_permissions).await {
|
||||
Ok(_) => {
|
||||
checks.push(CheckResult {
|
||||
name: "GitHub Token".into(),
|
||||
status: CheckStatus::Pass,
|
||||
summary: "minted".into(),
|
||||
details: perm_details,
|
||||
remediation: None,
|
||||
}),
|
||||
Err(err) => checks.push(CheckResult {
|
||||
});
|
||||
true
|
||||
}
|
||||
Err(err) => {
|
||||
checks.push(CheckResult {
|
||||
name: "GitHub Token".into(),
|
||||
status: CheckStatus::Error,
|
||||
summary: "failed".into(),
|
||||
details: perm_details,
|
||||
remediation: Some(format!("Failed to mint GitHub token: {err}")),
|
||||
}),
|
||||
});
|
||||
false
|
||||
}
|
||||
}
|
||||
_ => checks.push(CheckResult {
|
||||
} else {
|
||||
checks.push(CheckResult {
|
||||
name: "GitHub Token".into(),
|
||||
status: CheckStatus::Warning,
|
||||
summary: "skipped".into(),
|
||||
details: perm_details,
|
||||
remediation: Some("No GitHub credentials or origin URL available".to_string()),
|
||||
}),
|
||||
});
|
||||
true
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -2266,6 +2281,55 @@ issues = "read"
|
|||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn preflight_rejects_unresolved_github_permissions() {
|
||||
let state = crate::test_support::test_app_state();
|
||||
let mut manifest = minimal_manifest();
|
||||
manifest.workflows.get_mut("workflow.fabro").unwrap().config =
|
||||
Some(types::ManifestWorkflowConfig {
|
||||
path: "workflow.toml".to_string(),
|
||||
source: r#"_version = 1
|
||||
|
||||
[run.environment]
|
||||
id = "local"
|
||||
|
||||
[run.integrations.github.permissions]
|
||||
issues = "{{ env.GITHUB_ISSUES_PERMISSION }}"
|
||||
"#
|
||||
.to_string(),
|
||||
});
|
||||
|
||||
let prepared = prepare_manifest(
|
||||
&manifest_run_defaults(Some(&default_settings_fixture())),
|
||||
&manifest,
|
||||
)
|
||||
.unwrap();
|
||||
let validated = validate_prepared_manifest(&prepared, test_catalog()).unwrap();
|
||||
assert!(!validated.has_errors());
|
||||
|
||||
let (response, ok) = resolve_and_run_preflight(state.as_ref(), &prepared, &validated)
|
||||
.await
|
||||
.unwrap();
|
||||
let github_token_check = response.checks.sections[0]
|
||||
.checks
|
||||
.iter()
|
||||
.find(|check| check.name == "GitHub Token")
|
||||
.expect("GitHub Token check should report invalid permissions");
|
||||
|
||||
assert!(!ok);
|
||||
assert_eq!(
|
||||
github_token_check.status,
|
||||
types::PreflightCheckResultStatus::Error
|
||||
);
|
||||
assert_eq!(github_token_check.summary, "invalid permissions");
|
||||
assert!(
|
||||
github_token_check
|
||||
.remediation
|
||||
.as_deref()
|
||||
.is_some_and(|message| message.contains("GITHUB_ISSUES_PERMISSION"))
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn preflight_allows_pull_request_enabled_without_github_credentials() {
|
||||
let state = crate::test_support::test_app_state();
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
@ -874,13 +873,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!(
|
||||
|
|
@ -1060,7 +1054,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,
|
||||
|
|
@ -1080,7 +1073,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!(
|
||||
|
|
@ -4085,13 +4081,31 @@ async fn execute_run_in_process(state: Arc<AppState>, run_id: RunId) {
|
|||
return;
|
||||
}
|
||||
};
|
||||
let github_permissions = persisted
|
||||
let github_permissions = match persisted
|
||||
.run_spec()
|
||||
.settings
|
||||
.run
|
||||
.integrations
|
||||
.github
|
||||
.resolve_permissions(process_env_var);
|
||||
.resolve_permissions()
|
||||
{
|
||||
Ok(permissions) => permissions,
|
||||
Err(err) => {
|
||||
tracing::error!(
|
||||
run_id = %run_id,
|
||||
error = %err,
|
||||
"GitHub permission interpolation failed"
|
||||
);
|
||||
fail_run_before_execution(
|
||||
&state,
|
||||
run_id,
|
||||
FailureReason::WorkflowError,
|
||||
format!("Failed to resolve GitHub permissions: {err}"),
|
||||
)
|
||||
.await;
|
||||
return;
|
||||
}
|
||||
};
|
||||
let vault = match state.stores.vault.snapshot().await {
|
||||
Ok(vault) => vault,
|
||||
Err(err) => {
|
||||
|
|
@ -4118,7 +4132,7 @@ async fn execute_run_in_process(state: Arc<AppState>, run_id: RunId) {
|
|||
run_control: None,
|
||||
github_app,
|
||||
github_permissions,
|
||||
vault: Some(Arc::new(AsyncRwLock::new(vault.into_vault()))),
|
||||
vault: Arc::new(AsyncRwLock::new(vault.into_vault())),
|
||||
catalog: state.catalog(),
|
||||
on_node: None,
|
||||
registry_override,
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
|
|
|||
|
|
@ -2,7 +2,7 @@ use std::sync::Arc;
|
|||
|
||||
use axum::body::Body;
|
||||
use axum::http::{Request, StatusCode};
|
||||
use fabro_auth::EnvCredentialSource;
|
||||
use fabro_auth::test_support;
|
||||
use fabro_model::{Catalog, ProviderId};
|
||||
use fabro_static::EnvVars;
|
||||
use fabro_test::{TwinScenario, TwinScenarios, twin_openai};
|
||||
|
|
@ -43,12 +43,11 @@ fn test_app_with_openai_agent_backend(openai_base_url: String, api_key: String)
|
|||
);
|
||||
let source_api_key = api_key.clone();
|
||||
let env_api_key = api_key.clone();
|
||||
let llm_source: Arc<dyn fabro_auth::CredentialSource> = Arc::new(
|
||||
EnvCredentialSource::with_env_lookup(Arc::new(move |name| match name {
|
||||
let llm_source: Arc<dyn fabro_auth::CredentialSource> =
|
||||
test_support::env_credential_source(move |name| match name {
|
||||
"OPENAI_API_KEY" => Some(source_api_key.clone()),
|
||||
_ => None,
|
||||
})),
|
||||
);
|
||||
});
|
||||
let state = fabro_server::test_support::TestAppStateBuilder::new()
|
||||
.runtime_settings(settings.server_settings, settings.manifest_run_defaults)
|
||||
.max_concurrent_runs(5)
|
||||
|
|
|
|||
|
|
@ -59,6 +59,7 @@ htmd = "0.5"
|
|||
libc = "0.2"
|
||||
|
||||
[dev-dependencies]
|
||||
fabro-auth = { path = "../../foundation/fabro-auth", features = ["test-support"] }
|
||||
insta.workspace = true
|
||||
tokio = { workspace = true, features = ["test-util", "macros"] }
|
||||
tempfile = "3"
|
||||
|
|
|
|||
|
|
@ -30,6 +30,7 @@ tracing.workspace = true
|
|||
tokio-util.workspace = true
|
||||
|
||||
[dev-dependencies]
|
||||
fabro-auth = { path = "../../foundation/fabro-auth", features = ["test-support"] }
|
||||
httpmock = "0.8"
|
||||
tokio = { workspace = true, features = ["test-util", "macros"] }
|
||||
toml.workspace = true
|
||||
|
|
|
|||
|
|
@ -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,99 +54,34 @@ 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(
|
||||
clippy::disallowed_methods,
|
||||
reason = "hook HTTP logs use the unresolved token source, not the resolved URL, so env-sourced \
|
||||
URL material is not logged; redacted_url_for_log masks literal credentials in \
|
||||
parseable source URLs and replaces unparseable sources with a placeholder"
|
||||
reason = "hook HTTP logs use the unresolved token source, not the resolved URL; \
|
||||
redacted_url_for_log masks literal credentials in parseable source URLs and \
|
||||
replaces unparseable sources with a placeholder"
|
||||
)]
|
||||
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,22 +277,18 @@ 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");
|
||||
tracing::error!(error = %error, "prompt hook interpolation failed, not firing");
|
||||
return HookDecision::Block {
|
||||
reason: Some(error.to_string()),
|
||||
};
|
||||
|
|
@ -421,24 +340,20 @@ 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");
|
||||
tracing::error!(error = %error, "agent hook interpolation failed, not firing");
|
||||
return HookDecision::Block {
|
||||
reason: Some(error.to_string()),
|
||||
};
|
||||
|
|
@ -566,26 +481,21 @@ 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!(
|
||||
url_source = %safe_url_source_for_log(url),
|
||||
error = %error,
|
||||
"HTTP hook URL env resolution failed, not firing"
|
||||
"HTTP hook URL interpolation failed, not firing"
|
||||
);
|
||||
return HookDecision::Block {
|
||||
reason: Some(error.to_string()),
|
||||
|
|
@ -611,18 +521,14 @@ 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!(
|
||||
url_source = %safe_url_source_for_log(url),
|
||||
header = %key,
|
||||
error = %error,
|
||||
"HTTP hook header env resolution failed, not firing"
|
||||
"HTTP hook header interpolation failed, not firing"
|
||||
);
|
||||
return HookDecision::Block {
|
||||
reason: Some(error.to_string()),
|
||||
|
|
@ -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),
|
||||
)
|
||||
|
|
@ -836,9 +728,9 @@ impl HookExecutor for HookExecutorImpl {
|
|||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use fabro_auth::{CredentialSource, EnvCredentialSource};
|
||||
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;
|
||||
|
|
@ -855,7 +747,7 @@ mod tests {
|
|||
}
|
||||
|
||||
fn test_llm_source() -> Arc<dyn CredentialSource> {
|
||||
Arc::new(EnvCredentialSource::new())
|
||||
test_support::vault_only_credential_source()
|
||||
}
|
||||
|
||||
fn test_catalog() -> Arc<Catalog> {
|
||||
|
|
@ -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;
|
||||
|
||||
|
|
@ -1525,12 +1336,9 @@ mod tests {
|
|||
&client,
|
||||
&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 +1357,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 +1373,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 +1397,9 @@ mod tests {
|
|||
&client,
|
||||
&interp(&server.url("/hook")),
|
||||
None,
|
||||
&[],
|
||||
&TlsMode::Off,
|
||||
&make_context(),
|
||||
std::time::Duration::from_secs(5),
|
||||
&test_env(&[]),
|
||||
)
|
||||
.await;
|
||||
|
||||
|
|
@ -1621,10 +1423,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 +1460,6 @@ mod tests {
|
|||
&make_context(),
|
||||
&sandbox,
|
||||
&HookExecutionContext::default(),
|
||||
&test_env(&[]),
|
||||
)
|
||||
.await;
|
||||
|
||||
|
|
@ -1675,7 +1475,6 @@ mod tests {
|
|||
&interp("{{ env.MISSING_HOOK_VALUE }}"),
|
||||
None,
|
||||
&make_context(),
|
||||
&test_env(&[]),
|
||||
test_llm_source().as_ref(),
|
||||
test_catalog(),
|
||||
)
|
||||
|
|
@ -1704,7 +1503,6 @@ mod tests {
|
|||
Some(1),
|
||||
&make_context(),
|
||||
make_sandbox(),
|
||||
&test_env(&[]),
|
||||
test_llm_source().as_ref(),
|
||||
test_catalog(),
|
||||
)
|
||||
|
|
|
|||
|
|
@ -4,7 +4,7 @@ use std::sync::Arc;
|
|||
use fabro_agent::Sandbox;
|
||||
use fabro_auth::CredentialSource;
|
||||
#[cfg(test)]
|
||||
use fabro_auth::EnvCredentialSource;
|
||||
use fabro_auth::test_support;
|
||||
use fabro_model::Catalog;
|
||||
|
||||
use crate::config::{HookDefinition, HookSettings};
|
||||
|
|
@ -46,7 +46,7 @@ impl HookRunner {
|
|||
Self {
|
||||
config,
|
||||
executor,
|
||||
llm_source: Arc::new(EnvCredentialSource::new()),
|
||||
llm_source: test_support::vault_only_credential_source(),
|
||||
catalog: Arc::new(Catalog::from_builtin().expect("default catalog should build")),
|
||||
compiled_matchers,
|
||||
}
|
||||
|
|
@ -238,7 +238,6 @@ impl HookRunner {
|
|||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use fabro_auth::EnvCredentialSource;
|
||||
use fabro_types::fixtures;
|
||||
|
||||
use super::*;
|
||||
|
|
@ -279,7 +278,7 @@ mod tests {
|
|||
}
|
||||
|
||||
fn test_llm_source() -> Arc<dyn CredentialSource> {
|
||||
Arc::new(EnvCredentialSource::new())
|
||||
test_support::vault_only_credential_source()
|
||||
}
|
||||
|
||||
fn test_catalog() -> Arc<Catalog> {
|
||||
|
|
|
|||
|
|
@ -2,7 +2,7 @@ use std::path::Path;
|
|||
use std::sync::Arc;
|
||||
|
||||
use fabro_agent::{LocalSandbox, Sandbox};
|
||||
use fabro_auth::{CredentialSource, EnvCredentialSource};
|
||||
use fabro_auth::{CredentialSource, test_support};
|
||||
use fabro_hooks::{
|
||||
HookContext, HookDecision, HookDefinition, HookEvent, HookExecutionContext, HookRunner,
|
||||
HookSettings, InterpString,
|
||||
|
|
@ -12,7 +12,7 @@ use fabro_types::RunId;
|
|||
use tokio::fs;
|
||||
|
||||
fn test_llm_source() -> Arc<dyn CredentialSource> {
|
||||
Arc::new(EnvCredentialSource::new())
|
||||
test_support::vault_only_credential_source()
|
||||
}
|
||||
|
||||
fn test_catalog() -> Arc<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;
|
||||
|
|
|
|||
|
|
@ -14,7 +14,7 @@ readme = "README.md"
|
|||
doctest = false
|
||||
|
||||
[features]
|
||||
test-support = []
|
||||
test-support = ["fabro-auth/test-support"]
|
||||
|
||||
[lints]
|
||||
workspace = true
|
||||
|
|
@ -75,6 +75,7 @@ tempfile = "3"
|
|||
toml.workspace = true
|
||||
fabro-vault = { path = "../../foundation/fabro-vault" }
|
||||
[dev-dependencies]
|
||||
fabro-auth = { path = "../../foundation/fabro-auth", features = ["test-support"] }
|
||||
base64.workspace = true
|
||||
fabro-acp = { path = "../fabro-acp", features = ["test-support"] }
|
||||
fabro-workflow = { path = ".", features = ["test-support"] }
|
||||
|
|
|
|||
|
|
@ -10,7 +10,7 @@ use fabro_agent::{
|
|||
Sandbox, Session, SessionOptions, SessionShutdownReason, StaticEnvProvider, ToolEnvProvider,
|
||||
ToolSecrets, WebFetchSummarizer, canonical_tool_name, register_question_tools,
|
||||
};
|
||||
use fabro_auth::{CredentialSource, EnvCredentialSource};
|
||||
use fabro_auth::CredentialSource;
|
||||
use fabro_graphviz::graph::{AttrValue, Node};
|
||||
use fabro_llm::client::Client;
|
||||
use fabro_llm::types::{
|
||||
|
|
@ -695,22 +695,6 @@ impl AgentApiBackend {
|
|||
}
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub fn new_from_env(
|
||||
model: String,
|
||||
provider_id: impl Into<ProviderId>,
|
||||
fallback_chain: Vec<FallbackTarget>,
|
||||
steering_hub: Arc<SteeringHub>,
|
||||
) -> Self {
|
||||
Self::new(
|
||||
model,
|
||||
provider_id,
|
||||
fallback_chain,
|
||||
Arc::new(EnvCredentialSource::new()),
|
||||
steering_hub,
|
||||
)
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub fn with_env(mut self, env: HashMap<String, String>) -> Self {
|
||||
self.tool_env = Some(Arc::new(StaticEnvProvider(env)));
|
||||
|
|
@ -1729,7 +1713,7 @@ mod tests {
|
|||
use fabro_agent::subagent::SessionFactory;
|
||||
use fabro_agent::{AgentProfile, LocalSandbox, ToolRegistry};
|
||||
use fabro_api::types;
|
||||
use fabro_auth::{EnvCredentialSource, VaultCredentialSource};
|
||||
use fabro_auth::{VaultCredentialSource, test_support as auth_test_support};
|
||||
use fabro_llm::provider::{ProviderAdapter, StreamEventStream};
|
||||
use fabro_llm::{Error as LlmError, ProviderErrorDetail, ProviderErrorKind};
|
||||
use fabro_tool::FabroToolBackend;
|
||||
|
|
@ -1906,18 +1890,18 @@ reasoning = false
|
|||
}
|
||||
|
||||
fn mock_api_backend(server: &MockServer) -> AgentApiBackend {
|
||||
let source = EnvCredentialSource::with_env_lookup(Arc::new(|name| {
|
||||
let source = auth_test_support::env_credential_source(|name| {
|
||||
if name == "MOCK_API_KEY" {
|
||||
Some("sk-test".to_string())
|
||||
} else {
|
||||
None
|
||||
}
|
||||
}));
|
||||
});
|
||||
AgentApiBackend::new_with_catalog(
|
||||
"mock-model".to_string(),
|
||||
ProviderId::from("mock"),
|
||||
Vec::new(),
|
||||
Arc::new(source),
|
||||
source,
|
||||
SteeringHub::for_tests(),
|
||||
mock_llm_catalog(server),
|
||||
)
|
||||
|
|
@ -2015,10 +1999,11 @@ reasoning = false
|
|||
|
||||
#[test]
|
||||
fn agent_backend_stores_config() {
|
||||
let backend = AgentApiBackend::new_from_env(
|
||||
let backend = AgentApiBackend::new(
|
||||
"claude-opus-4-6".to_string(),
|
||||
ProviderId::openai(),
|
||||
Vec::new(),
|
||||
auth_test_support::vault_only_credential_source(),
|
||||
SteeringHub::for_tests(),
|
||||
);
|
||||
assert_eq!(backend.model, "claude-opus-4-6");
|
||||
|
|
@ -2027,10 +2012,11 @@ reasoning = false
|
|||
|
||||
#[test]
|
||||
fn agent_backend_initializes_empty_sessions() {
|
||||
let backend = AgentApiBackend::new_from_env(
|
||||
let backend = AgentApiBackend::new(
|
||||
"claude-opus-4-6".to_string(),
|
||||
ProviderId::anthropic(),
|
||||
Vec::new(),
|
||||
auth_test_support::vault_only_credential_source(),
|
||||
SteeringHub::for_tests(),
|
||||
);
|
||||
assert!(backend.sessions.lock().unwrap().is_empty());
|
||||
|
|
@ -2733,7 +2719,7 @@ enabled = true
|
|||
"gpt-5.4".to_string(),
|
||||
ProviderId::from("openrouter"),
|
||||
Vec::new(),
|
||||
Arc::new(EnvCredentialSource::new()),
|
||||
auth_test_support::vault_only_credential_source(),
|
||||
SteeringHub::for_tests(),
|
||||
Arc::new(Catalog::from_builtin_with_overrides(&settings).unwrap()),
|
||||
);
|
||||
|
|
@ -2749,7 +2735,7 @@ enabled = true
|
|||
"gpt-5.4".to_string(),
|
||||
ProviderId::from("openrouter"),
|
||||
Vec::new(),
|
||||
Arc::new(EnvCredentialSource::new()),
|
||||
auth_test_support::vault_only_credential_source(),
|
||||
SteeringHub::for_tests(),
|
||||
Arc::new(Catalog::from_builtin().unwrap()),
|
||||
);
|
||||
|
|
@ -2796,7 +2782,7 @@ reasoning = false
|
|||
"acme-llama".to_string(),
|
||||
ProviderId::from("acme"),
|
||||
Vec::new(),
|
||||
Arc::new(EnvCredentialSource::new()),
|
||||
auth_test_support::vault_only_credential_source(),
|
||||
SteeringHub::for_tests(),
|
||||
catalog,
|
||||
);
|
||||
|
|
@ -2843,7 +2829,7 @@ reasoning = false
|
|||
"acme-claude".to_string(),
|
||||
ProviderId::from("acme"),
|
||||
Vec::new(),
|
||||
Arc::new(EnvCredentialSource::new()),
|
||||
auth_test_support::vault_only_credential_source(),
|
||||
SteeringHub::for_tests(),
|
||||
catalog,
|
||||
);
|
||||
|
|
@ -2860,7 +2846,7 @@ reasoning = false
|
|||
"claude-sonnet-5".to_string(),
|
||||
ProviderId::anthropic(),
|
||||
Vec::new(),
|
||||
Arc::new(EnvCredentialSource::new()),
|
||||
auth_test_support::vault_only_credential_source(),
|
||||
SteeringHub::for_tests(),
|
||||
Arc::new(Catalog::from_builtin().unwrap()),
|
||||
);
|
||||
|
|
@ -2887,7 +2873,7 @@ enabled = true
|
|||
"openai/gpt-5.4".to_string(),
|
||||
ProviderId::from("openrouter"),
|
||||
Vec::new(),
|
||||
Arc::new(EnvCredentialSource::new()),
|
||||
auth_test_support::vault_only_credential_source(),
|
||||
SteeringHub::for_tests(),
|
||||
catalog,
|
||||
);
|
||||
|
|
@ -2902,10 +2888,11 @@ enabled = true
|
|||
|
||||
#[test]
|
||||
fn run_model_controls_apply_when_node_omits_controls() {
|
||||
let backend = AgentApiBackend::new_from_env(
|
||||
let backend = AgentApiBackend::new(
|
||||
"gpt-5.4".to_string(),
|
||||
ProviderId::openai(),
|
||||
Vec::new(),
|
||||
auth_test_support::vault_only_credential_source(),
|
||||
SteeringHub::for_tests(),
|
||||
)
|
||||
.with_run_model_controls(fabro_types::settings::run::RunModelControls {
|
||||
|
|
@ -2922,10 +2909,11 @@ enabled = true
|
|||
|
||||
#[test]
|
||||
fn node_controls_override_run_model_controls() {
|
||||
let backend = AgentApiBackend::new_from_env(
|
||||
let backend = AgentApiBackend::new(
|
||||
"gpt-5.4".to_string(),
|
||||
ProviderId::openai(),
|
||||
Vec::new(),
|
||||
auth_test_support::vault_only_credential_source(),
|
||||
SteeringHub::for_tests(),
|
||||
)
|
||||
.with_run_model_controls(fabro_types::settings::run::RunModelControls {
|
||||
|
|
@ -2950,10 +2938,11 @@ enabled = true
|
|||
|
||||
#[test]
|
||||
fn omitted_reasoning_effort_stays_unset() {
|
||||
let backend = AgentApiBackend::new_from_env(
|
||||
let backend = AgentApiBackend::new(
|
||||
"gpt-5.4".to_string(),
|
||||
ProviderId::openai(),
|
||||
Vec::new(),
|
||||
auth_test_support::vault_only_credential_source(),
|
||||
SteeringHub::for_tests(),
|
||||
);
|
||||
let node = Node::new("work");
|
||||
|
|
@ -2999,10 +2988,11 @@ enabled = true
|
|||
provider: "openai".to_string(),
|
||||
model: "gpt-5.5".to_string(),
|
||||
}];
|
||||
let backend = AgentApiBackend::new_from_env(
|
||||
let backend = AgentApiBackend::new(
|
||||
"claude-fable-5".to_string(),
|
||||
ProviderId::anthropic(),
|
||||
fallback_chain.clone(),
|
||||
auth_test_support::vault_only_credential_source(),
|
||||
SteeringHub::for_tests(),
|
||||
);
|
||||
let mut providers = HashMap::new();
|
||||
|
|
@ -3272,10 +3262,11 @@ enabled = true
|
|||
|
||||
#[tokio::test]
|
||||
async fn api_backend_shutdown_closes_cached_sessions_once() {
|
||||
let backend = AgentApiBackend::new_from_env(
|
||||
let backend = AgentApiBackend::new(
|
||||
"gpt-5.4".to_string(),
|
||||
ProviderId::openai(),
|
||||
Vec::new(),
|
||||
auth_test_support::vault_only_credential_source(),
|
||||
SteeringHub::for_tests(),
|
||||
);
|
||||
let emitter = Arc::new(Emitter::new(fabro_types::RunId::new()));
|
||||
|
|
|
|||
|
|
@ -605,6 +605,7 @@ mod tests {
|
|||
use anyhow::Result;
|
||||
use async_trait::async_trait;
|
||||
use bytes::Bytes;
|
||||
use fabro_auth::test_support as auth_test_support;
|
||||
use fabro_core::graph::Graph as CoreGraph;
|
||||
use fabro_core::lifecycle::RunLifecycle;
|
||||
use fabro_core::state::ExecutionState;
|
||||
|
|
@ -1273,7 +1274,7 @@ mod tests {
|
|||
tokio_util::sync::CancellationToken::new(),
|
||||
fabro_model::ProviderId::anthropic(),
|
||||
"claude-sonnet-4-6".to_string(),
|
||||
Arc::new(fabro_auth::EnvCredentialSource::new()),
|
||||
auth_test_support::vault_only_credential_source(),
|
||||
Arc::new(Catalog::from_builtin().expect("default catalog should build")),
|
||||
Arc::new(SandboxGitRuntime::new()),
|
||||
Arc::clone(&lifecycle.metadata_runtime),
|
||||
|
|
|
|||
|
|
@ -3,7 +3,7 @@ use std::path::Path;
|
|||
use std::sync::{Arc, Mutex};
|
||||
use std::time::{Duration, Instant};
|
||||
|
||||
use fabro_auth::{CredentialSource, EnvCredentialSource, VaultCredentialSource};
|
||||
use fabro_auth::{CredentialSource, VaultCredentialSource};
|
||||
use fabro_interview::{AutoApproveInterviewer, Interviewer};
|
||||
use fabro_llm::client::Client as LlmClient;
|
||||
use fabro_mcp::config::McpServerSettings;
|
||||
|
|
@ -81,7 +81,7 @@ struct RunSession {
|
|||
workflow_path: Option<ManifestPath>,
|
||||
workflow_bundle: Option<Arc<WorkflowBundle>>,
|
||||
run_control: Option<Arc<RunControlState>>,
|
||||
vault: Option<Arc<AsyncRwLock<Vault>>>,
|
||||
vault: Arc<AsyncRwLock<Vault>>,
|
||||
catalog: Arc<Catalog>,
|
||||
fabro_run_tools: Option<FabroRunToolServices>,
|
||||
}
|
||||
|
|
@ -106,7 +106,7 @@ pub struct StartServices {
|
|||
/// Server-resolved GitHub integration permissions to inject into the
|
||||
/// sandbox env. Empty when github integration has no permissions.
|
||||
pub github_permissions: HashMap<String, String>,
|
||||
pub vault: Option<Arc<AsyncRwLock<Vault>>>,
|
||||
pub vault: Arc<AsyncRwLock<Vault>>,
|
||||
pub catalog: Arc<Catalog>,
|
||||
pub on_node: crate::OnNodeCallback,
|
||||
pub registry_override: Option<Arc<HandlerRegistry>>,
|
||||
|
|
@ -374,7 +374,7 @@ impl RunSession {
|
|||
resolve_sandbox_provider(resolved).effective_for(resolved.execution.mode);
|
||||
let catalog = Arc::clone(&services.catalog);
|
||||
let configured =
|
||||
configured_providers_for_start(services.vault.as_ref(), Arc::clone(&catalog)).await;
|
||||
configured_providers_for_start(&services.vault, Arc::clone(&catalog)).await;
|
||||
#[cfg(feature = "test-support")]
|
||||
let configured = workflow_test_support::test_configured_provider_ids(
|
||||
catalog.as_ref(),
|
||||
|
|
@ -383,22 +383,17 @@ impl RunSession {
|
|||
.is_some_and(|value| !matches!(value.as_str(), "" | "0" | "false" | "no")),
|
||||
);
|
||||
let llm = resolve_start_llm(catalog.as_ref(), &configured, resolved)?;
|
||||
let vault_guard = match services.vault.as_ref() {
|
||||
Some(vault) => Some(vault.read().await),
|
||||
None => None,
|
||||
};
|
||||
let vault_guard = services.vault.read().await;
|
||||
// Token-only secrets lookup over the vault read guard, shared across
|
||||
// every run-boundary resolver. A missing or non-Token secret becomes
|
||||
// `None`, so resolution fails closed with a secret error.
|
||||
let secret_lookup = |name: &str| vault_token_lookup(vault_guard.as_deref(), name);
|
||||
let secret_lookup = |name: &str| vault_token_lookup(&vault_guard, name);
|
||||
let mcp_servers = resolved
|
||||
.agent
|
||||
.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
|
||||
|
|
@ -436,10 +431,9 @@ impl RunSession {
|
|||
clone_branch: record.base_branch().map(str::to_string),
|
||||
},
|
||||
SandboxProviderKind::Daytona => {
|
||||
let api_key = match vault_guard.as_deref() {
|
||||
Some(vault) => vault.get(EnvVars::DAYTONA_API_KEY).map(str::to_string),
|
||||
None => None,
|
||||
};
|
||||
let api_key = vault_guard
|
||||
.get(EnvVars::DAYTONA_API_KEY)
|
||||
.map(str::to_string);
|
||||
SandboxSpec::Daytona {
|
||||
config: Box::new(resolve_daytona_config(resolved)),
|
||||
github_app: services.github_app.clone(),
|
||||
|
|
@ -453,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());
|
||||
|
|
@ -471,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 {
|
||||
|
|
@ -522,16 +515,13 @@ impl RunSession {
|
|||
}
|
||||
|
||||
async fn configured_providers_for_start(
|
||||
vault: Option<&Arc<AsyncRwLock<Vault>>>,
|
||||
vault: &Arc<AsyncRwLock<Vault>>,
|
||||
catalog: Arc<Catalog>,
|
||||
) -> Vec<ProviderId> {
|
||||
let source: Arc<dyn CredentialSource> = match vault {
|
||||
Some(vault) => Arc::new(VaultCredentialSource::with_env_lookup(
|
||||
Arc::clone(vault),
|
||||
process_env_var,
|
||||
)),
|
||||
None => Arc::new(EnvCredentialSource::new()),
|
||||
};
|
||||
let source: Arc<dyn CredentialSource> = Arc::new(VaultCredentialSource::with_env_lookup(
|
||||
Arc::clone(vault),
|
||||
process_env_var,
|
||||
));
|
||||
match LlmClient::from_source_report(source.as_ref(), catalog).await {
|
||||
Ok(report) => report
|
||||
.client
|
||||
|
|
@ -566,14 +556,14 @@ fn git_checkpoint_options_from_start(
|
|||
|
||||
#[expect(
|
||||
clippy::disallowed_methods,
|
||||
reason = "Run startup interpolation owns a process-env lookup facade for {{ env.* }} values."
|
||||
reason = "Run startup reads process env only for explicit provider credential refs and test mode."
|
||||
)]
|
||||
fn process_env_var(name: &str) -> Option<String> {
|
||||
std::env::var(name).ok()
|
||||
}
|
||||
|
||||
fn vault_token_lookup(vault: Option<&Vault>, name: &str) -> Option<String> {
|
||||
vault.and_then(|vault| fabro_auth::vault_get_token(vault, name).ok().flatten())
|
||||
fn vault_token_lookup(vault: &Vault, name: &str) -> Option<String> {
|
||||
fabro_auth::vault_get_token(vault, name).ok().flatten()
|
||||
}
|
||||
|
||||
async fn load_accepted_run_definition(
|
||||
|
|
@ -717,25 +707,22 @@ fn canonical_provider_id(catalog: &Catalog, provider_name: &str) -> ProviderId {
|
|||
.map_or(provider_id, |provider| provider.id.clone())
|
||||
}
|
||||
|
||||
/// Build the launch-time MCP config from resolved settings, resolving any
|
||||
/// `{{ env.* }}` and `{{ secrets.* }}` tokens in the transport
|
||||
/// (`command`/`url`/`env`/`headers`) against the worker process environment and
|
||||
/// vault — the run boundary where the MCP is actually launched.
|
||||
/// Build the launch-time MCP config from resolved settings. Secret tokens in
|
||||
/// the transport (`command`/`url`/`env`/`headers`) resolve from the vault at
|
||||
/// the run boundary. Unsupported tokens fail.
|
||||
///
|
||||
/// The resolution itself lives on the type
|
||||
/// ([`McpServerSettings::resolve_transport_env`]) so `fabro run` (here) and
|
||||
/// ([`McpServerSettings::resolve_transport_secrets`]) so `fabro run` (here) and
|
||||
/// `fabro exec` share one resolver; this wrapper just adds the server name to
|
||||
/// the error. MCP transport strings are carried in source form out of the
|
||||
/// config resolve layer so `fabro validate` stays portable (it never requires
|
||||
/// env to be set), and a referenced env var or secret that is unset is a hard
|
||||
/// error — no fallback to the unresolved source.
|
||||
/// config resolve layer so `fabro validate` stays portable. A missing or
|
||||
/// non-token secret is a hard error.
|
||||
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_secrets(secrets_lookup)
|
||||
.map_err(|err| {
|
||||
Error::engine_with_source(
|
||||
format!("failed to resolve MCP server {:?}", settings.name),
|
||||
|
|
@ -744,25 +731,22 @@ fn runtime_mcp_server(
|
|||
})
|
||||
}
|
||||
|
||||
/// Build the launch-time setup (prepare) commands from resolved settings,
|
||||
/// resolving any `{{ env.* }}` and `{{ secrets.* }}` tokens in each step's
|
||||
/// command and per-step env against the worker process environment and vault —
|
||||
/// the run boundary where the steps actually run.
|
||||
/// Build the launch-time setup (prepare) commands from resolved settings.
|
||||
/// Secret tokens in each step's command and per-step env resolve from the vault
|
||||
/// at the run boundary. Unsupported tokens fail.
|
||||
///
|
||||
/// The resolution itself lives on the type
|
||||
/// ([`ResolvedRunPrepareSettings::resolve_step_env`]) so prepare-step env
|
||||
/// ([`ResolvedRunPrepareSettings::resolve_step_secrets`]) so prepare-step
|
||||
/// resolution shares one resolver with the rest of the run-boundary
|
||||
/// interpolation. Prepare-step commands and env are carried in source form out
|
||||
/// of the config resolve layer so `fabro validate` stays portable (it never
|
||||
/// requires env to be set), and a referenced env var or secret that is unset is
|
||||
/// a hard error — no fallback to the unresolved source.
|
||||
/// of the config resolve layer so `fabro validate` stays portable. A missing or
|
||||
/// non-token secret is a hard error.
|
||||
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_secrets(secrets_lookup)
|
||||
.map_err(|err| Error::engine_with_source("failed to resolve prepare step", err))?;
|
||||
Ok(resolved
|
||||
.steps
|
||||
|
|
@ -1651,7 +1635,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(),
|
||||
|
|
@ -1673,8 +1657,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!(
|
||||
|
|
@ -1692,8 +1675,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");
|
||||
|
||||
|
|
@ -1721,8 +1703,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");
|
||||
|
|
@ -1741,8 +1722,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");
|
||||
};
|
||||
|
||||
|
|
@ -1766,8 +1746,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");
|
||||
};
|
||||
|
||||
|
|
@ -1789,8 +1768,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");
|
||||
};
|
||||
|
||||
|
|
@ -1848,7 +1826,7 @@ reasoning = false
|
|||
)])));
|
||||
|
||||
let session = RunSession::new(&persisted, StartServices {
|
||||
vault: Some(vault),
|
||||
vault,
|
||||
..test_start_services(&store, &storage_root, emitter, registry).await
|
||||
})
|
||||
.await
|
||||
|
|
@ -1906,7 +1884,7 @@ reasoning = false
|
|||
let vault = Arc::new(AsyncRwLock::new(start_vault(&[])));
|
||||
|
||||
let Err(err) = RunSession::new(&persisted, StartServices {
|
||||
vault: Some(vault),
|
||||
vault,
|
||||
..test_start_services(&store, &storage_root, emitter, registry).await
|
||||
})
|
||||
.await
|
||||
|
|
@ -2072,7 +2050,7 @@ reasoning = false
|
|||
run_control: None,
|
||||
github_app: None,
|
||||
github_permissions: HashMap::new(),
|
||||
vault: Some(Arc::new(AsyncRwLock::new(start_vault(&[])))),
|
||||
vault: Arc::new(AsyncRwLock::new(start_vault(&[]))),
|
||||
catalog: test_catalog(),
|
||||
on_node: None,
|
||||
registry_override: Some(registry),
|
||||
|
|
@ -2100,7 +2078,7 @@ reasoning = false
|
|||
}
|
||||
|
||||
fn vault_secret_lookup(vault: &Vault) -> impl FnMut(&str) -> Option<String> + '_ {
|
||||
move |name| vault_token_lookup(Some(vault), name)
|
||||
move |name| vault_token_lookup(vault, name)
|
||||
}
|
||||
|
||||
fn prepare_with_step(step: PreparedStep) -> RunPrepareSettings {
|
||||
|
|
|
|||
|
|
@ -12,6 +12,7 @@ use std::time::Duration;
|
|||
|
||||
use async_trait::async_trait;
|
||||
use fabro_agent::Sandbox;
|
||||
use fabro_auth::test_support as auth_test_support;
|
||||
use fabro_graphviz::graph::{AttrValue, Edge, Graph, Node};
|
||||
use fabro_hooks::HookSettings;
|
||||
use fabro_interview::AutoApproveInterviewer;
|
||||
|
|
@ -287,7 +288,7 @@ async fn execute_test_run_with_options(
|
|||
github_permissions: None,
|
||||
origin_url: None,
|
||||
},
|
||||
vault: None,
|
||||
vault: auth_test_support::empty_vault(),
|
||||
git: git_options,
|
||||
run_control: None,
|
||||
registry_override,
|
||||
|
|
@ -349,7 +350,7 @@ async fn execute_runs_start_to_exit_and_returns_final_context() {
|
|||
github_permissions: None,
|
||||
origin_url: None,
|
||||
},
|
||||
vault: None,
|
||||
vault: auth_test_support::empty_vault(),
|
||||
git: None,
|
||||
run_control: None,
|
||||
registry_override: None,
|
||||
|
|
@ -488,7 +489,7 @@ async fn resumed_in_flight_node_starts_a_new_stage_execution() {
|
|||
github_permissions: None,
|
||||
origin_url: None,
|
||||
},
|
||||
vault: None,
|
||||
vault: auth_test_support::empty_vault(),
|
||||
git: None,
|
||||
run_control: None,
|
||||
registry_override: Some(Arc::new(make_registry())),
|
||||
|
|
@ -599,7 +600,7 @@ async fn run_with_lifecycle(
|
|||
github_permissions: None,
|
||||
origin_url: None,
|
||||
},
|
||||
vault: None,
|
||||
vault: auth_test_support::empty_vault(),
|
||||
git: None,
|
||||
run_control: None,
|
||||
registry_override: Some(Arc::new(registry)),
|
||||
|
|
|
|||
|
|
@ -677,6 +677,7 @@ mod tests {
|
|||
use anyhow::Result;
|
||||
use async_trait::async_trait;
|
||||
use bytes::Bytes;
|
||||
use fabro_auth::test_support as auth_test_support;
|
||||
use fabro_graphviz::graph::Graph;
|
||||
use fabro_model::Catalog;
|
||||
use fabro_sandbox::test_support::MockSandbox;
|
||||
|
|
@ -1088,7 +1089,7 @@ mod tests {
|
|||
tokio_util::sync::CancellationToken::new(),
|
||||
fabro_model::ProviderId::anthropic(),
|
||||
"claude-sonnet-4-6".to_string(),
|
||||
Arc::new(fabro_auth::EnvCredentialSource::new()),
|
||||
auth_test_support::vault_only_credential_source(),
|
||||
Arc::new(Catalog::from_builtin().expect("default catalog should build")),
|
||||
Arc::new(SandboxGitRuntime::new()),
|
||||
metadata_runtime,
|
||||
|
|
@ -1121,7 +1122,7 @@ mod tests {
|
|||
tokio_util::sync::CancellationToken::new(),
|
||||
fabro_model::ProviderId::anthropic(),
|
||||
"claude-sonnet-4-6".to_string(),
|
||||
Arc::new(fabro_auth::EnvCredentialSource::new()),
|
||||
auth_test_support::vault_only_credential_source(),
|
||||
Arc::new(Catalog::from_builtin().expect("default catalog should build")),
|
||||
Arc::new(SandboxGitRuntime::new()),
|
||||
Arc::new(RunMetadataRuntime::new()),
|
||||
|
|
|
|||
|
|
@ -5,8 +5,7 @@ use std::time::Instant;
|
|||
|
||||
use fabro_agent::{Sandbox, ToolSecrets};
|
||||
use fabro_auth::{
|
||||
CredentialSource, EnvCredentialSource, ExtraHeadersCredentialSource, VaultCredentialSource,
|
||||
auth_issue_message,
|
||||
CredentialSource, ExtraHeadersCredentialSource, VaultCredentialSource, auth_issue_message,
|
||||
};
|
||||
use fabro_graphviz::graph;
|
||||
use fabro_hooks::{HookContext, HookDecision, HookEvent, HookExecutionContext, HookRunner};
|
||||
|
|
@ -237,21 +236,12 @@ async fn build_registry(
|
|||
}
|
||||
}
|
||||
|
||||
#[expect(
|
||||
clippy::disallowed_methods,
|
||||
reason = "CLI/library workflow runs without a vault explicitly pass the Brave Search process-env credential into tool configuration; server runs pass a vault."
|
||||
)]
|
||||
async fn tool_secrets_from_configured_sources(
|
||||
vault: Option<&Arc<AsyncRwLock<Vault>>>,
|
||||
) -> ToolSecrets {
|
||||
let brave_search_api_key = match vault {
|
||||
Some(vault) => vault
|
||||
.read()
|
||||
.await
|
||||
.get(EnvVars::BRAVE_SEARCH_API_KEY)
|
||||
.map(str::to_string),
|
||||
None => std::env::var(EnvVars::BRAVE_SEARCH_API_KEY).ok(),
|
||||
};
|
||||
async fn tool_secrets_from_configured_sources(vault: &Arc<AsyncRwLock<Vault>>) -> ToolSecrets {
|
||||
let brave_search_api_key = vault
|
||||
.read()
|
||||
.await
|
||||
.get(EnvVars::BRAVE_SEARCH_API_KEY)
|
||||
.map(str::to_string);
|
||||
ToolSecrets {
|
||||
brave_search_api_key,
|
||||
}
|
||||
|
|
@ -267,15 +257,11 @@ fn graph_needs_api_backend(graph: &graph::Graph) -> bool {
|
|||
const SESSION_ID_HEADER: &str = "x-session-id";
|
||||
|
||||
fn build_llm_source(
|
||||
vault: Option<Arc<AsyncRwLock<Vault>>>,
|
||||
vault: Arc<AsyncRwLock<Vault>>,
|
||||
run_id: fabro_types::RunId,
|
||||
) -> Arc<dyn CredentialSource> {
|
||||
let inner: Arc<dyn CredentialSource> = match vault {
|
||||
Some(vault) => Arc::new(VaultCredentialSource::new(vault)),
|
||||
None => Arc::new(EnvCredentialSource::new()),
|
||||
};
|
||||
Arc::new(ExtraHeadersCredentialSource::new(
|
||||
inner,
|
||||
Arc::new(VaultCredentialSource::new(vault)),
|
||||
HashMap::from([(SESSION_ID_HEADER.to_string(), run_id.to_string())]),
|
||||
))
|
||||
}
|
||||
|
|
@ -298,7 +284,7 @@ pub async fn initialize(
|
|||
options.run_options.git = options.git.clone();
|
||||
|
||||
let llm_source = build_llm_source(options.vault.clone(), options.run_options.run_id);
|
||||
let tool_secrets = tool_secrets_from_configured_sources(options.vault.as_ref()).await;
|
||||
let tool_secrets = tool_secrets_from_configured_sources(&options.vault).await;
|
||||
let catalog = Arc::clone(&options.catalog);
|
||||
let sandbox_git = Arc::new(SandboxGitRuntime::new());
|
||||
let metadata_runtime = Arc::new(RunMetadataRuntime::new());
|
||||
|
|
@ -356,14 +342,12 @@ pub async fn initialize(
|
|||
let instance = record.instance().ok_or_else(|| {
|
||||
Error::Precondition("cannot resume run: run sandbox was not initialized".to_string())
|
||||
})?;
|
||||
let daytona_api_key = match &options.vault {
|
||||
Some(vault) => vault
|
||||
.read()
|
||||
.await
|
||||
.get(EnvVars::DAYTONA_API_KEY)
|
||||
.map(str::to_string),
|
||||
None => None,
|
||||
};
|
||||
let daytona_api_key = options
|
||||
.vault
|
||||
.read()
|
||||
.await
|
||||
.get(EnvVars::DAYTONA_API_KEY)
|
||||
.map(str::to_string);
|
||||
let sandbox = reconnect_for_run_with_callback(
|
||||
instance,
|
||||
daytona_api_key,
|
||||
|
|
@ -665,6 +649,7 @@ mod tests {
|
|||
use std::time::Duration;
|
||||
|
||||
use fabro_acp::test_support::fake_acp_agent_script;
|
||||
use fabro_auth::test_support as auth_test_support;
|
||||
use fabro_graphviz::graph::{AttrValue, Edge, Graph, Node};
|
||||
use fabro_interview::AutoApproveInterviewer;
|
||||
use fabro_sandbox::SandboxSpec;
|
||||
|
|
@ -866,7 +851,7 @@ mod tests {
|
|||
github_permissions: None,
|
||||
origin_url: None,
|
||||
},
|
||||
vault: None,
|
||||
vault: auth_test_support::empty_vault(),
|
||||
git: None,
|
||||
run_control: None,
|
||||
registry_override: None,
|
||||
|
|
@ -947,7 +932,7 @@ mod tests {
|
|||
github_permissions: None,
|
||||
origin_url: None,
|
||||
},
|
||||
vault: None,
|
||||
vault: auth_test_support::empty_vault(),
|
||||
git: None,
|
||||
run_control: None,
|
||||
registry_override: None,
|
||||
|
|
@ -1055,7 +1040,7 @@ mod tests {
|
|||
let run_id = test_run_id();
|
||||
let expected_session_id = run_id.to_string();
|
||||
|
||||
let source = build_llm_source(Some(vault), run_id);
|
||||
let source = build_llm_source(vault, run_id);
|
||||
let resolved = source.resolve(test_catalog().as_ref()).await.unwrap();
|
||||
|
||||
assert!(!resolved.credentials.is_empty());
|
||||
|
|
@ -1138,13 +1123,13 @@ mod tests {
|
|||
let store = memory_store();
|
||||
let run_store = store.create_run(&test_run_id()).await.unwrap();
|
||||
let initialized = initialize(test_persisted(graph, source, &run_dir), InitOptions {
|
||||
run_store: run_store.into(),
|
||||
dry_run: false,
|
||||
emitter: emitter.clone(),
|
||||
sandbox: SandboxSpec::Local {
|
||||
run_store: run_store.into(),
|
||||
dry_run: false,
|
||||
emitter: emitter.clone(),
|
||||
sandbox: SandboxSpec::Local {
|
||||
working_directory: temp.path().to_path_buf(),
|
||||
},
|
||||
llm: LlmSpec {
|
||||
llm: LlmSpec {
|
||||
model: "fake-acp".to_string(),
|
||||
provider_id: fabro_model::ProviderId::openai(),
|
||||
fallback_chain: Vec::new(),
|
||||
|
|
@ -1152,30 +1137,30 @@ mod tests {
|
|||
model_controls: RunModelControls::default(),
|
||||
dry_run: false,
|
||||
},
|
||||
interviewer: Arc::new(AutoApproveInterviewer::engine()),
|
||||
steering_hub: Arc::new(crate::steering_hub::SteeringHub::new(emitter)),
|
||||
catalog: test_catalog(),
|
||||
lifecycle: crate::run_options::LifecycleOptions {
|
||||
interviewer: Arc::new(AutoApproveInterviewer::engine()),
|
||||
steering_hub: Arc::new(crate::steering_hub::SteeringHub::new(emitter)),
|
||||
catalog: test_catalog(),
|
||||
lifecycle: crate::run_options::LifecycleOptions {
|
||||
setup_commands: Vec::new(),
|
||||
setup_command_timeout_ms: 1_000,
|
||||
},
|
||||
run_options: test_settings(&run_dir),
|
||||
workflow_path: None,
|
||||
workflow_bundle: None,
|
||||
hooks: fabro_hooks::HookSettings { hooks: vec![] },
|
||||
sandbox_env: SandboxEnvSpec {
|
||||
run_options: test_settings(&run_dir),
|
||||
workflow_path: None,
|
||||
workflow_bundle: None,
|
||||
hooks: fabro_hooks::HookSettings { hooks: vec![] },
|
||||
sandbox_env: SandboxEnvSpec {
|
||||
toml_env: HashMap::new(),
|
||||
github_permissions: None,
|
||||
origin_url: None,
|
||||
},
|
||||
vault: Some(vault),
|
||||
git: None,
|
||||
run_control: None,
|
||||
vault,
|
||||
git: None,
|
||||
run_control: None,
|
||||
registry_override: None,
|
||||
artifact_sink: None,
|
||||
resume: None,
|
||||
seed_context: None,
|
||||
fabro_run_tools: None,
|
||||
artifact_sink: None,
|
||||
resume: None,
|
||||
seed_context: None,
|
||||
fabro_run_tools: None,
|
||||
})
|
||||
.await
|
||||
.unwrap();
|
||||
|
|
@ -1263,7 +1248,7 @@ mod tests {
|
|||
github_permissions: None,
|
||||
origin_url: None,
|
||||
},
|
||||
vault: None,
|
||||
vault: auth_test_support::empty_vault(),
|
||||
git: None,
|
||||
run_control: None,
|
||||
registry_override: None,
|
||||
|
|
@ -1405,7 +1390,7 @@ mod tests {
|
|||
github_permissions: None,
|
||||
origin_url: None,
|
||||
},
|
||||
vault: None,
|
||||
vault: auth_test_support::empty_vault(),
|
||||
git: None,
|
||||
run_control: None,
|
||||
registry_override: None,
|
||||
|
|
|
|||
|
|
@ -299,7 +299,7 @@ pub struct InitOptions {
|
|||
pub workflow_bundle: Option<Arc<WorkflowBundle>>,
|
||||
pub hooks: fabro_hooks::HookSettings,
|
||||
pub sandbox_env: SandboxEnvSpec,
|
||||
pub vault: Option<Arc<AsyncRwLock<Vault>>>,
|
||||
pub vault: Arc<AsyncRwLock<Vault>>,
|
||||
pub git: Option<GitCheckpointOptions>,
|
||||
pub registry_override: Option<Arc<HandlerRegistry>>,
|
||||
pub artifact_sink: Option<ArtifactSink>,
|
||||
|
|
|
|||
|
|
@ -5,7 +5,7 @@ use std::sync::Arc;
|
|||
use std::time::Duration;
|
||||
|
||||
use fabro_agent::Sandbox;
|
||||
use fabro_auth::{CredentialSource, EnvCredentialSource};
|
||||
use fabro_auth::{CredentialSource, test_support as auth_test_support};
|
||||
use fabro_graphviz::graph::Graph as GvGraph;
|
||||
use fabro_interview::AutoApproveInterviewer;
|
||||
use fabro_model::Catalog;
|
||||
|
|
@ -249,7 +249,7 @@ async fn initialized(
|
|||
"claude-sonnet-4-6".to_string(),
|
||||
options
|
||||
.llm_source
|
||||
.unwrap_or_else(|| Arc::new(EnvCredentialSource::new())),
|
||||
.unwrap_or_else(auth_test_support::vault_only_credential_source),
|
||||
Arc::new(Catalog::from_builtin().expect("default catalog should build")),
|
||||
Arc::new(SandboxGitRuntime::new()),
|
||||
Arc::new(RunMetadataRuntime::new()),
|
||||
|
|
|
|||
|
|
@ -2168,7 +2168,6 @@ async fn smoke_test_with_mock_codergen_backend() {
|
|||
|
||||
#[tokio::test]
|
||||
async fn shared_thread_compaction_before_routing_audit_succeeds() {
|
||||
use fabro_auth::EnvCredentialSource;
|
||||
use fabro_workflow::steering_hub::SteeringHub;
|
||||
use httpmock::Method::POST;
|
||||
use httpmock::MockServer;
|
||||
|
|
@ -2295,9 +2294,9 @@ reasoning = false
|
|||
))
|
||||
.expect("test catalog should parse");
|
||||
let catalog = Arc::new(Catalog::from_builtin_with_overrides(&settings).unwrap());
|
||||
let source = Arc::new(EnvCredentialSource::with_env_lookup(Arc::new(|name| {
|
||||
let source = auth_test_support::env_credential_source(|name| {
|
||||
(name == "COMPACT_API_KEY").then(|| "sk-test".to_string())
|
||||
})));
|
||||
});
|
||||
let backend = AgentApiBackend::new_with_catalog(
|
||||
"compact-model".to_string(),
|
||||
ProviderId::from("compact"),
|
||||
|
|
@ -2400,7 +2399,6 @@ reasoning = false
|
|||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn workflow_persists_authoritative_openrouter_cost_for_agent_stage() {
|
||||
use fabro_auth::EnvCredentialSource;
|
||||
use fabro_workflow::steering_hub::SteeringHub;
|
||||
use httpmock::Method::POST;
|
||||
use httpmock::MockServer;
|
||||
|
|
@ -2451,9 +2449,9 @@ base_url = "{}"
|
|||
))
|
||||
.expect("test catalog should parse");
|
||||
let catalog = Arc::new(Catalog::from_builtin_with_overrides(&settings).unwrap());
|
||||
let source = Arc::new(EnvCredentialSource::with_env_lookup(Arc::new(|name| {
|
||||
let source = auth_test_support::env_credential_source(|name| {
|
||||
(name == "OPENROUTER_API_KEY").then(|| "sk-test".to_string())
|
||||
})));
|
||||
});
|
||||
let backend = AgentApiBackend::new_with_catalog(
|
||||
"openai/gpt-5.4".to_string(),
|
||||
ProviderId::from("openrouter"),
|
||||
|
|
@ -6918,6 +6916,7 @@ mod real_llm {
|
|||
use std::sync::Arc;
|
||||
|
||||
use async_trait::async_trait;
|
||||
use fabro_auth::EnvCredentialSource;
|
||||
use fabro_graphviz::graph::Node;
|
||||
use fabro_llm::client::Client;
|
||||
use fabro_llm::providers::OpenAiAdapter;
|
||||
|
|
@ -7011,7 +7010,7 @@ mod real_llm {
|
|||
}
|
||||
|
||||
fabro_test::require_env("ANTHROPIC_API_KEY")?;
|
||||
let source = fabro_auth::EnvCredentialSource::new();
|
||||
let source = EnvCredentialSource::new();
|
||||
Some(Arc::new(
|
||||
Client::from_source(&source, super::default_catalog())
|
||||
.await
|
||||
|
|
@ -8421,7 +8420,7 @@ fn subgraph_without_label_no_class_derived() {
|
|||
fn hook_runner_from_defs(hooks: Vec<fabro_hooks::HookDefinition>) -> Arc<fabro_hooks::HookRunner> {
|
||||
Arc::new(fabro_hooks::HookRunner::new(
|
||||
fabro_hooks::HookSettings { hooks },
|
||||
Arc::new(fabro_auth::EnvCredentialSource::new()),
|
||||
auth_test_support::vault_only_credential_source(),
|
||||
default_catalog(),
|
||||
))
|
||||
}
|
||||
|
|
@ -10332,6 +10331,7 @@ async fn node_dir_uses_visit_count_on_revisit() {
|
|||
// Git checkpoint e2e (Local)
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
use fabro_auth::test_support as auth_test_support;
|
||||
use fabro_workflow::handler::fan_in::FanInHandler;
|
||||
use fabro_workflow::handler::parallel::ParallelHandler;
|
||||
|
||||
|
|
|
|||
|
|
@ -9,6 +9,9 @@ description = "Typed provider credential storage and resolution for Fabro"
|
|||
[lints]
|
||||
workspace = true
|
||||
|
||||
[features]
|
||||
test-support = []
|
||||
|
||||
[dependencies]
|
||||
anyhow.workspace = true
|
||||
async-trait.workspace = true
|
||||
|
|
|
|||
|
|
@ -2,17 +2,22 @@ use std::collections::HashMap;
|
|||
use std::sync::Arc;
|
||||
|
||||
use async_trait::async_trait;
|
||||
use fabro_model::catalog::CatalogProvider;
|
||||
use fabro_model::{Catalog, CredentialRef, ProviderId};
|
||||
use fabro_model::{Catalog, ProviderId};
|
||||
use fabro_static::EnvVars;
|
||||
use fabro_types::settings::ResolveCtx;
|
||||
use fabro_vault::Vault;
|
||||
use tokio::sync::RwLock as AsyncRwLock;
|
||||
|
||||
use crate::credential_source::{CredentialSource, ResolvedCredentials};
|
||||
use crate::resolve::{apply_openai_api_env_context, apply_openai_codex_api_context};
|
||||
use crate::{ApiCredential, EnvLookup, ResolveError, build_api_key_header, resolve};
|
||||
use crate::resolve::apply_openai_codex_api_context;
|
||||
use crate::{CredentialSource, EnvLookup, ResolvedCredentials, VaultCredentialSource};
|
||||
|
||||
/// A credential source for provider credentials declared as `env:<NAME>`.
|
||||
///
|
||||
/// This public SDK facade does not resolve `{{ env.NAME }}` settings
|
||||
/// interpolation. Provider extra headers can use literals, but secret
|
||||
/// interpolation requires a vault-backed source.
|
||||
#[derive(Clone)]
|
||||
pub struct EnvCredentialSource {
|
||||
inner: VaultCredentialSource,
|
||||
env_lookup: EnvLookup,
|
||||
}
|
||||
|
||||
|
|
@ -20,7 +25,7 @@ impl EnvCredentialSource {
|
|||
#[must_use]
|
||||
#[expect(
|
||||
clippy::disallowed_methods,
|
||||
reason = "EnvCredentialSource is the provider API-key process-env facade."
|
||||
reason = "EnvCredentialSource is the provider credential process-env facade."
|
||||
)]
|
||||
pub fn new() -> Self {
|
||||
Self::with_env_lookup(Arc::new(|name| std::env::var(name).ok()))
|
||||
|
|
@ -28,61 +33,15 @@ impl EnvCredentialSource {
|
|||
|
||||
#[must_use]
|
||||
pub fn with_env_lookup(env_lookup: EnvLookup) -> Self {
|
||||
Self { env_lookup }
|
||||
let vault = Arc::new(AsyncRwLock::new(Vault::from_entries(HashMap::new())));
|
||||
let inner_lookup = Arc::clone(&env_lookup);
|
||||
let inner = VaultCredentialSource::with_env_lookup(vault, move |name| inner_lookup(name));
|
||||
Self { inner, env_lookup }
|
||||
}
|
||||
|
||||
fn lookup(&self, name: &str) -> Option<String> {
|
||||
(self.env_lookup)(name)
|
||||
}
|
||||
|
||||
fn credential_for(
|
||||
&self,
|
||||
provider: &CatalogProvider,
|
||||
) -> Result<Option<ApiCredential>, ResolveError> {
|
||||
let (auth_header, extra_headers) = match &provider.auth {
|
||||
Some(auth) => {
|
||||
let Some(key) = auth.credentials.iter().find_map(|credential_ref| {
|
||||
let CredentialRef::Env(name) = credential_ref else {
|
||||
return None;
|
||||
};
|
||||
self.lookup(name)
|
||||
}) else {
|
||||
return Ok(None);
|
||||
};
|
||||
(
|
||||
Some(build_api_key_header(auth.header.clone(), key)),
|
||||
self.resolved_extra_headers(provider)?,
|
||||
)
|
||||
}
|
||||
None => (None, self.resolved_extra_headers(provider)?),
|
||||
};
|
||||
|
||||
let mut cred = ApiCredential {
|
||||
provider: provider.id.clone(),
|
||||
auth_header,
|
||||
extra_headers,
|
||||
base_url: provider.base_url.clone(),
|
||||
codex_mode: false,
|
||||
org_id: None,
|
||||
project_id: None,
|
||||
};
|
||||
if provider.id == ProviderId::openai() && cred.auth_header.is_some() {
|
||||
if let Some(account_id) = self.lookup(EnvVars::CHATGPT_ACCOUNT_ID) {
|
||||
apply_openai_codex_api_context(&mut cred, Some(&account_id), &*self.env_lookup);
|
||||
} else {
|
||||
apply_openai_api_env_context(&mut cred, &*self.env_lookup);
|
||||
}
|
||||
}
|
||||
Ok(Some(cred))
|
||||
}
|
||||
|
||||
fn resolved_extra_headers(
|
||||
&self,
|
||||
provider: &CatalogProvider,
|
||||
) -> Result<HashMap<String, String>, ResolveError> {
|
||||
let mut ctx = ResolveCtx::new().with_env(|env_name| self.lookup(env_name));
|
||||
resolve::resolve_extra_headers(&provider.id, &provider.extra_headers, &mut ctx)
|
||||
}
|
||||
}
|
||||
|
||||
impl std::fmt::Debug for EnvCredentialSource {
|
||||
|
|
@ -101,37 +60,21 @@ impl Default for EnvCredentialSource {
|
|||
#[async_trait]
|
||||
impl CredentialSource for EnvCredentialSource {
|
||||
async fn resolve(&self, catalog: &Catalog) -> anyhow::Result<ResolvedCredentials> {
|
||||
let mut credentials = Vec::new();
|
||||
let mut auth_issues = Vec::new();
|
||||
|
||||
for provider in catalog.providers() {
|
||||
match self.credential_for(provider) {
|
||||
Ok(Some(credential)) => credentials.push(credential),
|
||||
Ok(None) => {}
|
||||
Err(ResolveError::NotConfigured(_) | ResolveError::Interpolation { .. })
|
||||
if provider.auth.is_some() => {}
|
||||
Err(err) => auth_issues.push((provider.id.clone(), err)),
|
||||
}
|
||||
let mut resolved = self.inner.resolve(catalog).await?;
|
||||
if let (Some(account_id), Some(credential)) = (
|
||||
self.lookup(EnvVars::CHATGPT_ACCOUNT_ID),
|
||||
resolved
|
||||
.credentials
|
||||
.iter_mut()
|
||||
.find(|credential| credential.provider == ProviderId::openai()),
|
||||
) {
|
||||
apply_openai_codex_api_context(credential, Some(&account_id), self.env_lookup.as_ref());
|
||||
}
|
||||
|
||||
Ok(ResolvedCredentials {
|
||||
credentials,
|
||||
auth_issues,
|
||||
})
|
||||
Ok(resolved)
|
||||
}
|
||||
|
||||
async fn configured_providers(&self, catalog: &Catalog) -> Vec<ProviderId> {
|
||||
catalog
|
||||
.providers()
|
||||
.iter()
|
||||
.filter(|provider| match &provider.auth {
|
||||
Some(auth) => auth.credentials.iter().any(|credential_ref| {
|
||||
matches!(credential_ref, CredentialRef::Env(name) if self.lookup(name).is_some())
|
||||
}),
|
||||
None => self.resolved_extra_headers(provider).is_ok(),
|
||||
})
|
||||
.map(|provider| provider.id.clone())
|
||||
.collect()
|
||||
self.inner.configured_providers(catalog).await
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -142,6 +85,7 @@ mod tests {
|
|||
|
||||
use fabro_model::catalog::LlmCatalogSettings;
|
||||
use fabro_model::{Catalog, ProviderId};
|
||||
use fabro_types::settings::interp::Namespace;
|
||||
|
||||
use super::EnvCredentialSource;
|
||||
use crate::CredentialSource;
|
||||
|
|
@ -154,68 +98,16 @@ mod tests {
|
|||
EnvCredentialSource::with_env_lookup(Arc::new(move |name| entries.get(name).cloned()))
|
||||
}
|
||||
|
||||
fn catalog_with(overrides: &str) -> Catalog {
|
||||
let settings: LlmCatalogSettings = toml::from_str(overrides).unwrap();
|
||||
Catalog::from_builtin_with_overrides(&settings).unwrap()
|
||||
}
|
||||
|
||||
fn default_catalog() -> Catalog {
|
||||
catalog_with("")
|
||||
}
|
||||
|
||||
/// A no-auth portkey provider whose only variation is its `extra_headers`
|
||||
/// TOML lines.
|
||||
fn portkey_catalog(extra_headers: &str) -> Catalog {
|
||||
catalog_with(&format!(
|
||||
r#"
|
||||
[providers.portkey]
|
||||
display_name = "Portkey Bedrock"
|
||||
adapter = "anthropic"
|
||||
agent_profile = "anthropic"
|
||||
base_url = "https://api.portkey.ai/v1"
|
||||
|
||||
[providers.portkey.extra_headers]
|
||||
{extra_headers}
|
||||
|
||||
[models."portkey-claude"]
|
||||
provider = "portkey"
|
||||
display_name = "Portkey Claude"
|
||||
family = "claude"
|
||||
default = true
|
||||
|
||||
[models."portkey-claude".limits]
|
||||
context_window = 200000
|
||||
|
||||
[models."portkey-claude".features]
|
||||
tools = true
|
||||
vision = true
|
||||
reasoning = true
|
||||
reasoning_effort = "levels"
|
||||
"#
|
||||
))
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn configured_providers_reads_injected_env() {
|
||||
async fn configured_providers_reads_injected_provider_env() {
|
||||
let source = test_source(&[("ANTHROPIC_API_KEY", "anthropic-key")]);
|
||||
let catalog = default_catalog();
|
||||
let catalog = Catalog::from_builtin().unwrap();
|
||||
|
||||
assert_eq!(source.configured_providers(&catalog).await, vec![
|
||||
ProviderId::anthropic()
|
||||
]);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn resolve_returns_empty_when_no_keys_are_configured() {
|
||||
let source = test_source(&[]);
|
||||
let catalog = default_catalog();
|
||||
|
||||
let resolved = source.resolve(&catalog).await.unwrap();
|
||||
|
||||
assert!(resolved.credentials.is_empty());
|
||||
assert!(resolved.auth_issues.is_empty());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn resolve_builds_openai_codex_env_credential() {
|
||||
let source = test_source(&[
|
||||
|
|
@ -223,7 +115,7 @@ reasoning_effort = "levels"
|
|||
("CHATGPT_ACCOUNT_ID", "acct_123"),
|
||||
("OPENAI_PROJECT_ID", "project_123"),
|
||||
]);
|
||||
let catalog = default_catalog();
|
||||
let catalog = Catalog::from_builtin().unwrap();
|
||||
|
||||
let resolved = source.resolve(&catalog).await.unwrap();
|
||||
let credential = resolved.credentials.first().unwrap();
|
||||
|
|
@ -242,23 +134,8 @@ reasoning_effort = "levels"
|
|||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn resolve_uses_catalog_credentials_and_base_url_for_openai_compatible_providers() {
|
||||
let source = test_source(&[("KIMI_API_KEY", "kimi-key")]);
|
||||
let catalog = default_catalog();
|
||||
|
||||
let resolved = source.resolve(&catalog).await.unwrap();
|
||||
let credential = resolved.credentials.first().unwrap();
|
||||
|
||||
assert_eq!(credential.provider, ProviderId::new("kimi"));
|
||||
assert_eq!(
|
||||
credential.base_url.as_deref(),
|
||||
Some("https://api.moonshot.ai/v1")
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn resolve_registers_custom_env_backed_provider() {
|
||||
let catalog = catalog_with(
|
||||
async fn env_settings_interpolation_remains_unsupported() {
|
||||
let settings: LlmCatalogSettings = toml::from_str(
|
||||
r#"
|
||||
[providers.acme]
|
||||
display_name = "Acme"
|
||||
|
|
@ -269,158 +146,66 @@ base_url = "https://api.acme.test/v1"
|
|||
[providers.acme.auth]
|
||||
credentials = ["env:ACME_API_KEY"]
|
||||
|
||||
[models."acme-large"]
|
||||
provider = "acme"
|
||||
display_name = "Acme Large"
|
||||
family = "acme"
|
||||
default = true
|
||||
|
||||
[models."acme-large".limits]
|
||||
context_window = 128000
|
||||
|
||||
[models."acme-large".features]
|
||||
tools = true
|
||||
vision = false
|
||||
reasoning = false
|
||||
[providers.acme.extra_headers]
|
||||
x-account = "{{ env.ACME_ACCOUNT }}"
|
||||
"#,
|
||||
);
|
||||
let source = test_source(&[("ACME_API_KEY", "acme-key")]);
|
||||
)
|
||||
.unwrap();
|
||||
let catalog = Catalog::from_builtin_with_overrides(&settings).unwrap();
|
||||
let source = test_source(&[("ACME_API_KEY", "acme-key"), ("ACME_ACCOUNT", "account-id")]);
|
||||
|
||||
let resolved = source.resolve(&catalog).await.unwrap();
|
||||
let credential = resolved
|
||||
.credentials
|
||||
.iter()
|
||||
.find(|credential| credential.provider == ProviderId::new("acme"))
|
||||
.expect("custom provider should resolve from the supplied catalog");
|
||||
|
||||
assert_eq!(
|
||||
credential.auth_header.as_ref().unwrap(),
|
||||
&crate::ApiKeyHeader::Bearer("acme-key".to_string(),)
|
||||
);
|
||||
assert_eq!(
|
||||
credential.base_url.as_deref(),
|
||||
Some("https://api.acme.test/v1")
|
||||
assert!(
|
||||
resolved
|
||||
.credentials
|
||||
.iter()
|
||||
.all(|credential| credential.provider != ProviderId::new("acme"))
|
||||
);
|
||||
assert!(resolved.auth_issues.iter().any(|(provider, issue)| {
|
||||
provider == &ProviderId::new("acme")
|
||||
&& matches!(
|
||||
issue,
|
||||
crate::ResolveError::Interpolation { source, .. }
|
||||
if source.namespace == Namespace::Env
|
||||
)
|
||||
}));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn env_source_resolves_literal_and_env_header_tokens() {
|
||||
let catalog = portkey_catalog(
|
||||
r#"
|
||||
x-portkey-api-key = "{{ env.PORTKEY_API_KEY }}"
|
||||
x-portkey-provider = "@bedrock-prod"
|
||||
"#,
|
||||
);
|
||||
let source = test_source(&[("PORTKEY_API_KEY", "pk-live")]);
|
||||
|
||||
let resolved = source.resolve(&catalog).await.unwrap();
|
||||
let credential = resolved
|
||||
.credentials
|
||||
.iter()
|
||||
.find(|credential| credential.provider == ProviderId::new("portkey"))
|
||||
.expect("no-auth provider should register when extra headers resolve");
|
||||
|
||||
assert!(credential.auth_header.is_none());
|
||||
assert_eq!(
|
||||
credential.extra_headers.get("x-portkey-api-key"),
|
||||
Some(&"pk-live".to_string())
|
||||
);
|
||||
assert_eq!(
|
||||
credential.extra_headers.get("x-portkey-provider"),
|
||||
Some(&"@bedrock-prod".to_string())
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn env_source_resolves_modal_proxy_headers_when_explicitly_configured() {
|
||||
// The shipped `modal.toml` reads `{{ secrets.* }}`, which this source
|
||||
// cannot resolve. Operators who want env-backed Modal credentials must
|
||||
// override both header sources, as documented in `reference/sdk.mdx`.
|
||||
let catalog = catalog_with(
|
||||
async fn modal_env_vars_do_not_replace_vault_secrets() {
|
||||
let settings: LlmCatalogSettings = toml::from_str(
|
||||
r#"
|
||||
[providers.modal]
|
||||
enabled = true
|
||||
base_url = "https://example--kimi-k3.modal.run/v1"
|
||||
|
||||
[providers.modal.extra_headers]
|
||||
"Modal-Key" = "{{ env.MODAL_TOKEN_ID }}"
|
||||
"Modal-Secret" = "{{ env.MODAL_TOKEN_SECRET }}"
|
||||
"#,
|
||||
);
|
||||
)
|
||||
.unwrap();
|
||||
let catalog = Catalog::from_builtin_with_overrides(&settings).unwrap();
|
||||
let source = test_source(&[
|
||||
("MODAL_TOKEN_ID", "wk-test"),
|
||||
("MODAL_TOKEN_SECRET", "ws-test"),
|
||||
]);
|
||||
let modal = ProviderId::new("modal");
|
||||
|
||||
assert!(source.configured_providers(&catalog).await.contains(&modal));
|
||||
|
||||
let resolved = source.resolve(&catalog).await.unwrap();
|
||||
let credential = resolved
|
||||
.credentials
|
||||
.iter()
|
||||
.find(|credential| credential.provider == modal)
|
||||
.expect("Modal should resolve from the explicit environment header settings");
|
||||
|
||||
assert!(credential.auth_header.is_none());
|
||||
assert_eq!(
|
||||
credential.extra_headers,
|
||||
HashMap::from([
|
||||
("Modal-Key".to_string(), "wk-test".to_string()),
|
||||
("Modal-Secret".to_string(), "ws-test".to_string()),
|
||||
])
|
||||
);
|
||||
assert_eq!(
|
||||
credential.base_url.as_deref(),
|
||||
Some("https://example--kimi-k3.modal.run/v1")
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn env_source_secrets_header_token_is_unavailable() {
|
||||
let catalog = portkey_catalog(r#"x-team-secret = "{{ secrets.gateway_team_secret }}""#);
|
||||
let source = test_source(&[]);
|
||||
assert!(!source.configured_providers(&catalog).await.contains(&modal));
|
||||
|
||||
let resolved = source.resolve(&catalog).await.unwrap();
|
||||
|
||||
assert!(
|
||||
!resolved
|
||||
resolved
|
||||
.credentials
|
||||
.iter()
|
||||
.any(|credential| credential.provider == ProviderId::new("portkey"))
|
||||
.all(|credential| credential.provider != modal)
|
||||
);
|
||||
let (_, issue) = resolved
|
||||
.auth_issues
|
||||
.iter()
|
||||
.find(|(provider, _)| provider == &ProviderId::new("portkey"))
|
||||
.expect("secrets token should surface as an auth issue");
|
||||
assert!(matches!(
|
||||
issue,
|
||||
crate::ResolveError::Interpolation { provider, .. }
|
||||
if provider == &ProviderId::new("portkey")
|
||||
));
|
||||
assert!(issue.to_string().contains("gateway_team_secret"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn env_source_reports_missing_env_header_for_no_auth_provider() {
|
||||
let catalog = portkey_catalog(r#"x-portkey-api-key = "{{ env.PORTKEY_API_KEY }}""#);
|
||||
let source = test_source(&[]);
|
||||
|
||||
let resolved = source.resolve(&catalog).await.unwrap();
|
||||
|
||||
assert!(
|
||||
!resolved
|
||||
.credentials
|
||||
.iter()
|
||||
.any(|credential| credential.provider == ProviderId::new("portkey"))
|
||||
);
|
||||
let (_, issue) = resolved
|
||||
.auth_issues
|
||||
.iter()
|
||||
.find(|(provider, _)| provider == &ProviderId::new("portkey"))
|
||||
.expect("missing env header should surface as an auth issue");
|
||||
assert!(matches!(issue, crate::ResolveError::Interpolation { .. }));
|
||||
assert!(issue.to_string().contains("PORTKEY_API_KEY"));
|
||||
assert!(resolved.auth_issues.iter().any(|(provider, issue)| {
|
||||
provider == &modal
|
||||
&& matches!(
|
||||
issue,
|
||||
crate::ResolveError::Interpolation { source, .. }
|
||||
if source.namespace == Namespace::Secrets
|
||||
)
|
||||
}));
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -7,6 +7,8 @@ mod refresh;
|
|||
mod resolve;
|
||||
mod sql_vault_source;
|
||||
mod strategy;
|
||||
#[cfg(any(test, feature = "test-support"))]
|
||||
pub mod test_support;
|
||||
mod vault_ext;
|
||||
mod vault_source;
|
||||
|
||||
|
|
@ -21,7 +23,6 @@ pub use refresh::refresh_oauth_credential;
|
|||
pub use resolve::{
|
||||
ApiCredential, CredentialResolver, CredentialUsage, EnvLookup, ResolveError,
|
||||
ResolvedCredential, auth_issue_message, build_api_key_header,
|
||||
configured_providers_from_process_env,
|
||||
};
|
||||
pub use sql_vault_source::SqlVaultCredentialSource;
|
||||
pub use strategy::{
|
||||
|
|
|
|||
|
|
@ -10,8 +10,6 @@ use tokio::sync::RwLock as AsyncRwLock;
|
|||
use tokio::task::spawn_blocking;
|
||||
|
||||
use crate::credential::{ApiKeyHeader, OAuthCredential};
|
||||
use crate::credential_source::CredentialSource;
|
||||
use crate::env_source::EnvCredentialSource;
|
||||
use crate::refresh::refresh_oauth_credential;
|
||||
use crate::vault_ext::{
|
||||
VaultLookupError, vault_get_oauth, vault_get_token, vault_set_oauth, vault_token_lookup,
|
||||
|
|
@ -238,8 +236,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 = {
|
||||
|
|
@ -332,9 +329,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)
|
||||
|
|
@ -372,10 +367,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)
|
||||
|
|
@ -383,7 +374,6 @@ impl CredentialResolver {
|
|||
}
|
||||
|
||||
fn resolved_extra_headers_for_catalog(
|
||||
&self,
|
||||
vault: &Vault,
|
||||
provider: &ProviderId,
|
||||
catalog: &Catalog,
|
||||
|
|
@ -391,9 +381,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)
|
||||
}
|
||||
|
||||
|
|
@ -411,7 +400,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,
|
||||
|
|
@ -429,7 +418,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,
|
||||
|
|
@ -448,7 +437,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,
|
||||
|
|
@ -471,7 +460,6 @@ impl CredentialResolver {
|
|||
}
|
||||
|
||||
fn api_credential_from_provider_auth(
|
||||
&self,
|
||||
vault: &Vault,
|
||||
provider: &CatalogProvider,
|
||||
catalog: &Catalog,
|
||||
|
|
@ -479,8 +467,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,
|
||||
|
|
@ -533,23 +520,6 @@ fn vault_lookup_error(provider: &ProviderId, name: &str, err: VaultLookupError)
|
|||
}
|
||||
}
|
||||
|
||||
pub async fn configured_providers_from_process_env(
|
||||
vault: Option<&Arc<AsyncRwLock<Vault>>>,
|
||||
catalog: &Catalog,
|
||||
) -> Vec<ProviderId> {
|
||||
match vault {
|
||||
Some(vault_arc) => {
|
||||
let resolver = CredentialResolver::new(Arc::clone(vault_arc));
|
||||
let guard = vault_arc.read().await;
|
||||
resolver.configured_providers(&guard, catalog)
|
||||
}
|
||||
None => {
|
||||
EnvCredentialSource::new()
|
||||
.configured_providers(catalog)
|
||||
.await
|
||||
}
|
||||
}
|
||||
}
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use std::error::Error as _;
|
||||
|
|
|
|||
41
lib/foundation/fabro-auth/src/test_support.rs
Normal file
41
lib/foundation/fabro-auth/src/test_support.rs
Normal file
|
|
@ -0,0 +1,41 @@
|
|||
//! Test-only credential sources.
|
||||
//!
|
||||
//! Feature-gated so they never link into production builds. Production code
|
||||
//! resolves credentials through [`VaultCredentialSource`] over a real vault;
|
||||
//! these helpers exist so tests can supply a source without one.
|
||||
|
||||
use std::collections::HashMap;
|
||||
use std::sync::Arc;
|
||||
|
||||
use fabro_vault::Vault;
|
||||
use tokio::sync::RwLock as AsyncRwLock;
|
||||
|
||||
use crate::credential_source::CredentialSource;
|
||||
use crate::vault_source::VaultCredentialSource;
|
||||
|
||||
/// A detached in-memory vault holding no secrets.
|
||||
#[must_use]
|
||||
pub fn empty_vault() -> Arc<AsyncRwLock<Vault>> {
|
||||
Arc::new(AsyncRwLock::new(Vault::from_entries(HashMap::new())))
|
||||
}
|
||||
|
||||
/// A vault-backed source whose credentials come only from `env_lookup`.
|
||||
///
|
||||
/// Tests that inject fake provider keys use this instead of reading the real
|
||||
/// process environment, which would make them order-dependent.
|
||||
#[must_use]
|
||||
pub fn env_credential_source<F>(env_lookup: F) -> Arc<dyn CredentialSource>
|
||||
where
|
||||
F: Fn(&str) -> Option<String> + Send + Sync + 'static,
|
||||
{
|
||||
Arc::new(VaultCredentialSource::with_env_lookup(
|
||||
empty_vault(),
|
||||
env_lookup,
|
||||
))
|
||||
}
|
||||
|
||||
/// A vault-backed source over an empty vault with no process-env fallback.
|
||||
#[must_use]
|
||||
pub fn vault_only_credential_source() -> Arc<dyn CredentialSource> {
|
||||
Arc::new(VaultCredentialSource::vault_only(empty_vault()))
|
||||
}
|
||||
|
|
@ -76,10 +76,9 @@ pub struct ProviderSettings {
|
|||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub base_url: Option<String>,
|
||||
/// Extra HTTP headers attached to every outgoing provider request after
|
||||
/// credential resolution. Values are interpolation strings: literal text,
|
||||
/// `{{ env.NAME }}`, or `{{ secrets.NAME }}`. Put credentials in a secret
|
||||
/// and reference them with a `{{ secrets.NAME }}` token, not a bare
|
||||
/// literal.
|
||||
/// credential resolution. Values are literal text or
|
||||
/// `{{ secrets.NAME }}` interpolation strings. Put credentials in a secret
|
||||
/// and reference them with a token, not a bare literal.
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub extra_headers: Option<HashMap<String, InterpString>>,
|
||||
/// Higher wins; missing → `0`; ties broken by canonical provider ID.
|
||||
|
|
|
|||
|
|
@ -733,40 +733,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,
|
||||
})
|
||||
);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -156,8 +156,9 @@ fn resolve_git(git: Option<&RunGitLayer>) -> RunGitSettings {
|
|||
#[expect(
|
||||
clippy::disallowed_methods,
|
||||
reason = "intentional source preservation: prepare step commands and per-step env are carried \
|
||||
in source form so `fabro validate` stays portable; their {{ env.* }} tokens resolve \
|
||||
at the run boundary in fabro_types::settings::run::RunPrepareSettings::resolve_step_env"
|
||||
in source form so `fabro validate` stays portable; secret tokens resolve and \
|
||||
unsupported tokens fail at the run boundary in \
|
||||
fabro_types::settings::run::RunPrepareSettings::resolve_step_secrets"
|
||||
)]
|
||||
fn resolve_prepare(
|
||||
prepare: Option<&RunPrepareLayer>,
|
||||
|
|
@ -168,18 +169,16 @@ fn resolve_prepare(
|
|||
let mut steps = Vec::new();
|
||||
for (index, step) in prepare.steps.iter().enumerate() {
|
||||
let run = match (&step.script, &step.command) {
|
||||
// A `script` is a raw shell snippet: carry it verbatim so the shell
|
||||
// interprets it. Its `{{ env.* }}` tokens resolve at the run
|
||||
// boundary.
|
||||
// A `script` is a raw shell snippet. Carry it verbatim so the shell
|
||||
// interprets it after late secret resolution.
|
||||
(Some(script), None) => PreparedStepRun::Script {
|
||||
script: script.as_source(),
|
||||
},
|
||||
// A `command` is an argv: carry it as a vector of element source
|
||||
// strings — neither pre-joined nor shell-quoted here. Each element's
|
||||
// `{{ env.* }}` token resolves at the run boundary, and only the
|
||||
// resolved value is shell-quoted (resolve-then-quote), so an
|
||||
// interpolated value can never break out of its argument and inject
|
||||
// shell syntax.
|
||||
// strings — neither pre-joined nor shell-quoted here. Secret tokens
|
||||
// resolve at the run boundary, and only the resolved value is
|
||||
// shell-quoted (resolve-then-quote), so an interpolated value can
|
||||
// never break out of its argument and inject shell syntax.
|
||||
(None, Some(argv)) => PreparedStepRun::Command {
|
||||
command: argv.iter().map(InterpString::as_source).collect(),
|
||||
},
|
||||
|
|
@ -386,8 +385,9 @@ pub(crate) fn resolve_enabled_mcps(
|
|||
#[expect(
|
||||
clippy::disallowed_methods,
|
||||
reason = "intentional source preservation: MCP transport strings are carried in source form \
|
||||
so `fabro validate` stays portable; their {{ env.* }} tokens resolve at the run \
|
||||
boundary in fabro_workflow::operations::start::runtime_mcp_server"
|
||||
so `fabro validate` stays portable; secret tokens resolve and unsupported tokens \
|
||||
fail at the run boundary in \
|
||||
fabro_workflow::operations::start::runtime_mcp_server"
|
||||
)]
|
||||
pub(crate) fn resolve_mcp_entry(name: &str, entry: &McpEntryLayer) -> McpServerSettings {
|
||||
let transport = match entry {
|
||||
|
|
@ -482,8 +482,9 @@ pub(crate) fn resolve_mcp_entry(name: &str, entry: &McpEntryLayer) -> McpServerS
|
|||
#[expect(
|
||||
clippy::disallowed_methods,
|
||||
reason = "intentional source preservation: MCP transport strings are carried in source form \
|
||||
so `fabro validate` stays portable; their {{ env.* }} tokens resolve at the run \
|
||||
boundary in fabro_workflow::operations::start::runtime_mcp_server"
|
||||
so `fabro validate` stays portable; secret tokens resolve and unsupported tokens \
|
||||
fail at the run boundary in \
|
||||
fabro_workflow::operations::start::runtime_mcp_server"
|
||||
)]
|
||||
fn resolve_mcp_command(
|
||||
script: Option<&InterpString>,
|
||||
|
|
@ -595,7 +596,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,
|
||||
});
|
||||
}
|
||||
|
|
@ -706,8 +706,8 @@ mod resolve_prepare_tests {
|
|||
assert_eq!(steps[0].run, PreparedStepRun::Script {
|
||||
script: "setup".to_string(),
|
||||
});
|
||||
// The per-step env survives resolution in source form (its {{ env.* }}
|
||||
// token resolves later, at the run boundary).
|
||||
// The per-step env survives config resolution in source form. The
|
||||
// unsupported token fails later at the run boundary.
|
||||
assert_eq!(
|
||||
steps[0].env.get("TOKEN").map(String::as_str),
|
||||
Some("{{ env.DEPLOY_TOKEN }}")
|
||||
|
|
@ -732,9 +732,9 @@ mod resolve_prepare_tests {
|
|||
let steps = resolve(&layer);
|
||||
|
||||
// Argv elements are carried as separate source strings, NOT joined and
|
||||
// NOT shell-quoted here. Quoting happens at the run boundary, after
|
||||
// `{{ env.* }}` resolution, so the resolved value (not the source
|
||||
// token) is what gets quoted.
|
||||
// NOT shell-quoted here. Quoting happens after late interpolation at
|
||||
// the run boundary, so a resolved value (not the source token) is what
|
||||
// gets quoted.
|
||||
assert_eq!(steps[0].run, PreparedStepRun::Command {
|
||||
command: vec![
|
||||
"echo".to_string(),
|
||||
|
|
|
|||
|
|
@ -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 \
|
||||
|
|
|
|||
|
|
@ -231,7 +231,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 }}"
|
||||
```
|
||||
|
|
@ -246,7 +246,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 literal text or `{{ secrets.NAME }}` interpolation strings. Put credentials in a secret and reference them with a 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. |
|
||||
|
|
|
|||
|
|
@ -2,26 +2,29 @@
|
|||
//!
|
||||
//! An [`InterpString`] field may contain narrow `{{ <namespace>.NAME }}`
|
||||
//! tokens — no template logic. Which [`Namespace`]s resolve is
|
||||
//! scope-determined by the caller through [`ResolveCtx`]: server-scope
|
||||
//! settings provide `env` (and eventually `secrets`), run-scope settings
|
||||
//! additionally provide `vars`, and a command node `script` provides `inputs`
|
||||
//! and `vars` plus the bare `goal` value. A token whose namespace has no lookup
|
||||
//! in the resolution context fails loudly rather than passing through as
|
||||
//! literal text.
|
||||
//! scope-determined by the caller through [`ResolveCtx`]. Run settings provide
|
||||
//! `vars` during run creation and `secrets` at consumption time. Workflow
|
||||
//! rendering additionally provides `inputs` and the bare `goal` value for
|
||||
//! supported fields. A token whose namespace has no lookup in the resolution
|
||||
//! context fails loudly rather than passing through as literal text.
|
||||
//!
|
||||
//! Most call sites do not wire `inputs`: it is bound where a run's typed
|
||||
//! `[run.inputs]` values are in scope, which is the workflow graph, not
|
||||
//! general config. Everywhere else an `{{ inputs.* }}` token still parses, so
|
||||
//! it fails with a clear message instead of reaching a consumer as literal
|
||||
//! text.
|
||||
//! Most call sites do not wire `inputs` or `goal`. They are bound where the
|
||||
//! run's typed values are in scope, which is the workflow graph rather than
|
||||
//! general config. Elsewhere their tokens still parse, so they fail with a
|
||||
//! clear message instead of reaching a consumer 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
|
||||
//! content-based (entropy analysis plus credential patterns), applied where
|
||||
//! output is serialized.
|
||||
//! `env` parses but has no [`ResolveCtx`] lookup. The process environment is
|
||||
//! not a configuration source. Use `{{ vars.NAME }}` for non-sensitive
|
||||
//! server-owned values or `{{ secrets.NAME }}` for vault-backed values.
|
||||
//! Keeping the namespace parseable lets it fail with a useful migration
|
||||
//! message.
|
||||
//!
|
||||
//! Resolution timing is split. `vars` substitute during run creation, and
|
||||
//! `inputs` and `goal` substitute during workflow rendering. `secrets` resolve
|
||||
//! late, at consumption time in the process that owns the value. 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.
|
||||
|
||||
use std::borrow::Cow;
|
||||
use std::fmt;
|
||||
|
|
@ -56,7 +59,8 @@ const GOAL_TOKEN: &str = "goal";
|
|||
)]
|
||||
#[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,
|
||||
|
|
@ -141,7 +145,6 @@ impl Namespace {
|
|||
/// [`InterpString::substitute_with`].
|
||||
#[derive(Default)]
|
||||
pub struct ResolveCtx<'a> {
|
||||
env: Option<LookupFn<'a>>,
|
||||
vars: Option<LookupFn<'a>>,
|
||||
secrets: Option<LookupFn<'a>>,
|
||||
inputs: Option<LookupFn<'a>>,
|
||||
|
|
@ -156,12 +159,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));
|
||||
|
|
@ -200,7 +197,10 @@ impl<'a> ResolveCtx<'a> {
|
|||
|
||||
fn lookup_for(&mut self, namespace: Namespace) -> Option<&mut LookupFn<'a>> {
|
||||
match namespace {
|
||||
Namespace::Env => self.env.as_mut(),
|
||||
// The process environment is not a configuration source. Keep the
|
||||
// namespace parseable so full resolution reports the migration
|
||||
// error instead of passing the token through as literal text.
|
||||
Namespace::Env => None,
|
||||
Namespace::Vars => self.vars.as_mut(),
|
||||
Namespace::Secrets => self.secrets.as_mut(),
|
||||
Namespace::Inputs => self.inputs.as_mut(),
|
||||
|
|
@ -360,9 +360,11 @@ impl InterpString {
|
|||
/// for the namespaces it does not — their resolution happens later,
|
||||
/// possibly in a different process.
|
||||
///
|
||||
/// This is the early, server-side pass (`vars`/`inputs`/`goal`); late-bound
|
||||
/// namespaces (`env`/`secrets`) survive in token form for their
|
||||
/// consumption-time [`InterpString::resolve_with`].
|
||||
/// Callers substitute the namespaces available at their boundary: `vars`
|
||||
/// during run creation, then `inputs` and `goal` during workflow rendering.
|
||||
/// `secrets` survive for consumption-time [`InterpString::resolve_with`].
|
||||
/// Unsupported `env` tokens also survive so the next full-resolution
|
||||
/// boundary can reject them with a migration message.
|
||||
pub fn substitute_with(&self, ctx: &mut ResolveCtx<'_>) -> Result<Self, ResolveError> {
|
||||
let mut segments = Vec::new();
|
||||
for seg in &self.segments {
|
||||
|
|
@ -385,35 +387,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>
|
||||
|
|
@ -527,6 +500,16 @@ impl fmt::Display for ResolveError {
|
|||
"{{{{ goal }}}} is only available in prompts and command node `script` \
|
||||
attributes, not in other config fields"
|
||||
),
|
||||
// `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 \
|
||||
|
|
@ -623,56 +606,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]
|
||||
|
|
@ -687,8 +681,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);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -729,14 +722,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();
|
||||
|
||||
|
|
@ -748,11 +741,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");
|
||||
|
|
@ -765,10 +754,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);
|
||||
|
|
@ -781,10 +770,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);
|
||||
|
|
@ -796,13 +785,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();
|
||||
|
|
@ -925,7 +914,7 @@ mod tests {
|
|||
assert_eq!(wired, "7");
|
||||
|
||||
let err = s
|
||||
.resolve_with(&mut ResolveCtx::new().with_env(lookup_from(&[("id", "7")])))
|
||||
.resolve_with(&mut ResolveCtx::new().with_vars(lookup_from(&[("id", "7")])))
|
||||
.unwrap_err();
|
||||
assert_eq!(err.kind, ResolveErrorKind::Unavailable);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -269,9 +269,9 @@ where
|
|||
/// value. Both interpolation passes route through this one traversal so they
|
||||
/// cannot drift as `PreparedStepRun` or `PreparedStep` grow fields: the
|
||||
/// `{{ vars.* }}` pass ([`RunNamespace::substitute_variables`]) passes a
|
||||
/// `substitute_string` visitor, the `{{ env.* }}` pass
|
||||
/// ([`RunPrepareSettings::resolve_step_env`]) passes a `resolve_env_string`
|
||||
/// visitor. Mirrors [`visit_mcp_transport_strings`].
|
||||
/// `substitute_string` visitor, and the late secret pass
|
||||
/// ([`RunPrepareSettings::resolve_step_secrets`]) passes a
|
||||
/// `resolve_secret_string` visitor. Mirrors [`visit_mcp_transport_strings`].
|
||||
fn visit_prepared_step_strings<F>(
|
||||
step: &mut PreparedStep,
|
||||
visitor: &mut F,
|
||||
|
|
@ -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());
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -744,37 +735,32 @@ impl Default for RunPrepareSettings {
|
|||
}
|
||||
|
||||
impl RunPrepareSettings {
|
||||
/// Resolve `{{ env.* }}` and `{{ secrets.* }}` tokens in every prepare
|
||||
/// step's runnable part and per-step `env` values against the supplied
|
||||
/// lookups, returning a copy with the tokens replaced and every other field
|
||||
/// preserved. A `script` step's snippet resolves in place; a `command`
|
||||
/// step's argv resolves per element (each element is shell-quoted later, in
|
||||
/// [`PreparedStep::to_shell_command`], so quoting applies to the resolved
|
||||
/// value rather than the source token).
|
||||
/// Resolve `{{ secrets.* }}` tokens in every prepare step's runnable part
|
||||
/// and per-step `env` values against the supplied lookup. Unsupported
|
||||
/// tokens fail instead of reaching the process. A `script` step's snippet
|
||||
/// resolves in place; a `command` step's argv resolves per element (each
|
||||
/// element is shell-quoted later, in [`PreparedStep::to_shell_command`], so
|
||||
/// quoting applies to the resolved value rather than the source token).
|
||||
///
|
||||
/// This is the late, use-time half of prepare-step interpolation, the
|
||||
/// counterpart to the server-side `{{ vars.* }}` substitution in
|
||||
/// [`RunNamespace::substitute_variables`]: `{{ vars.* }}` are substituted
|
||||
/// earlier, server-side, while `{{ env.* }}` and `{{ secrets.* }}` resolve
|
||||
/// here — in whichever process actually runs the steps (the run worker for
|
||||
/// `fabro run`).
|
||||
/// earlier, server-side, while `{{ secrets.* }}` resolves in whichever
|
||||
/// process actually runs the steps (the run worker for `fabro run`).
|
||||
/// Carrying the source form out of the config resolve layer keeps
|
||||
/// `fabro validate` portable (it never requires env to be set).
|
||||
/// `fabro validate` portable.
|
||||
///
|
||||
/// 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
|
||||
/// passing through as literal text.
|
||||
pub fn resolve_step_env(
|
||||
/// A missing or non-token secret is a hard error. Unsupported `env` and
|
||||
/// template-only `inputs` tokens surface as
|
||||
/// [`ResolveErrorKind::Unavailable`] errors.
|
||||
pub fn resolve_step_secrets(
|
||||
&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_secret_string(value, &mut secrets_lookup)
|
||||
})?;
|
||||
}
|
||||
Ok(resolved)
|
||||
|
|
@ -784,9 +770,9 @@ impl RunPrepareSettings {
|
|||
/// A single resolved prepare step: the thing to run plus the per-step
|
||||
/// environment variables it should see. The runnable part keeps the
|
||||
/// script-vs-argv distinction (see [`PreparedStepRun`]), and every string is
|
||||
/// carried in source form out of the config resolve layer; their `{{ env.* }}`
|
||||
/// tokens resolve at the run boundary via
|
||||
/// [`RunPrepareSettings::resolve_step_env`].
|
||||
/// carried in source form out of the config resolve layer. Secret tokens
|
||||
/// resolve at the run boundary via
|
||||
/// [`RunPrepareSettings::resolve_step_secrets`].
|
||||
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
|
||||
pub struct PreparedStep {
|
||||
#[serde(flatten)]
|
||||
|
|
@ -803,10 +789,10 @@ pub struct PreparedStep {
|
|||
/// for the shell to interpret.
|
||||
/// - [`Command`](PreparedStepRun::Command) is an argv: a vector of element
|
||||
/// source strings, neither pre-joined nor shell-quoted at config time. Its
|
||||
/// `{{ env.* }}` tokens resolve per element at the run boundary, and only
|
||||
/// then is each *resolved* element shell-quoted and joined. Resolving before
|
||||
/// quoting is what stops an interpolated env value from breaking out of its
|
||||
/// argument and injecting shell syntax.
|
||||
/// `{{ secrets.* }}` tokens resolve per element at the run boundary, and only
|
||||
/// then is each resolved element shell-quoted and joined. Resolving before
|
||||
/// quoting stops an interpolated secret from breaking out of its argument and
|
||||
/// injecting shell syntax.
|
||||
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
|
||||
#[serde(tag = "type", rename_all = "snake_case")]
|
||||
pub enum PreparedStepRun {
|
||||
|
|
@ -821,8 +807,8 @@ impl PreparedStep {
|
|||
/// For a script, the snippet is returned verbatim. For an argv `command`,
|
||||
/// each element is shell-quoted and joined with spaces so an argument that
|
||||
/// contains spaces or shell metacharacters survives as a single token. This
|
||||
/// must run *after* [`RunPrepareSettings::resolve_step_env`] so the quoting
|
||||
/// applies to the resolved values, not the `{{ env.* }}` source.
|
||||
/// must run *after* [`RunPrepareSettings::resolve_step_secrets`] so the
|
||||
/// quoting applies to the resolved values, not the secret token source.
|
||||
pub fn to_shell_command(&self) -> String {
|
||||
match &self.run {
|
||||
PreparedStepRun::Script { script } => script.clone(),
|
||||
|
|
@ -1094,35 +1080,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 +1119,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 +1132,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 +1158,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());
|
||||
}
|
||||
}
|
||||
|
|
@ -1611,61 +1564,53 @@ impl McpServerSettings {
|
|||
StdDuration::from_secs(self.tool_timeout_secs)
|
||||
}
|
||||
|
||||
/// Resolve `{{ env.* }}` and `{{ secrets.* }}` tokens in this server's
|
||||
/// transport strings (`command`/`args`/`url`/`env`/`headers`) against the
|
||||
/// supplied lookups, returning a copy with the tokens replaced and every
|
||||
/// other field preserved.
|
||||
/// Resolve `{{ secrets.* }}` tokens in this server's transport strings
|
||||
/// (`command`/`args`/`url`/`env`/`headers`) against the supplied lookup.
|
||||
/// Unsupported tokens fail instead of reaching the transport.
|
||||
///
|
||||
/// This is the late, use-time half of MCP interpolation, the counterpart
|
||||
/// to [`substitute_mcp_transport`]: `{{ vars.* }}` are substituted
|
||||
/// earlier, server-side, while `{{ env.* }}` and `{{ secrets.* }}` resolve
|
||||
/// here — in whichever process actually launches the server (the run worker
|
||||
/// for `fabro run`, the CLI process for `fabro exec`). Carrying the source
|
||||
/// form out of the config resolve layer keeps `fabro validate` portable (it
|
||||
/// never requires env to be set).
|
||||
/// earlier, server-side, while `{{ secrets.* }}` resolves in whichever
|
||||
/// process actually launches the server (the run worker for `fabro run`,
|
||||
/// the CLI process for `fabro exec`). Carrying the source form out of the
|
||||
/// config resolve layer keeps `fabro validate` portable.
|
||||
///
|
||||
/// 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 [`ResolveErrorKind::Unavailable`]
|
||||
/// error rather than passing through as literal text.
|
||||
pub fn resolve_transport_env(
|
||||
/// A missing or non-token secret is a hard error. Unsupported `env` and
|
||||
/// template-only `inputs` tokens surface as
|
||||
/// [`ResolveErrorKind::Unavailable`] errors.
|
||||
pub fn resolve_transport_secrets(
|
||||
&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_secret_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(
|
||||
fn resolve_secret_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(())
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod resolve_transport_env_tests {
|
||||
mod resolve_transport_secrets_tests {
|
||||
use std::collections::HashMap;
|
||||
|
||||
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]
|
||||
|
|
@ -1680,7 +1625,7 @@ mod resolve_transport_env_tests {
|
|||
};
|
||||
|
||||
let resolved = settings
|
||||
.resolve_transport_env(env_lookup(&[]), secret_lookup(&[]))
|
||||
.resolve_transport_secrets(secret_lookup(&[]))
|
||||
.unwrap();
|
||||
|
||||
let McpTransport::Stdio { command, env } = resolved.transport else {
|
||||
|
|
@ -1691,93 +1636,24 @@ mod resolve_transport_env_tests {
|
|||
}
|
||||
|
||||
#[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() {
|
||||
fn missing_secret_is_hard_error() {
|
||||
let settings = McpServerSettings {
|
||||
name: "gemini".to_string(),
|
||||
transport: McpTransport::Stdio {
|
||||
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_secrets(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 +1677,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_secrets(secret_lookup(&[
|
||||
("SERVER_BIN", "/srv/mcp"),
|
||||
("API_TOKEN", "vault-token"),
|
||||
]))
|
||||
.unwrap();
|
||||
|
||||
let McpTransport::Stdio { command, env } = resolved.transport else {
|
||||
|
|
@ -1837,10 +1713,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_secrets(secret_lookup(&[
|
||||
("MCP_HOST", "mcp.example"),
|
||||
("MCP_TOKEN", "vault-token"),
|
||||
]))
|
||||
.unwrap();
|
||||
|
||||
let McpTransport::Http { url, headers, .. } = resolved.transport else {
|
||||
|
|
@ -1868,7 +1744,7 @@ mod resolve_transport_env_tests {
|
|||
};
|
||||
|
||||
let err = settings
|
||||
.resolve_transport_env(env_lookup(&[]), secret_lookup(&[]))
|
||||
.resolve_transport_secrets(secret_lookup(&[]))
|
||||
.unwrap_err();
|
||||
|
||||
assert_eq!(err.namespace, Namespace::Secrets);
|
||||
|
|
@ -1878,13 +1754,12 @@ mod resolve_transport_env_tests {
|
|||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod resolve_step_env_tests {
|
||||
mod resolve_step_secrets_tests {
|
||||
use std::collections::HashMap;
|
||||
|
||||
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 +1818,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_secrets(secret_lookup(&[])).unwrap();
|
||||
|
||||
assert_eq!(resolved.steps[0].to_shell_command(), "echo hello");
|
||||
assert_eq!(
|
||||
|
|
@ -1956,19 +1829,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_secrets(secret_lookup(&[("REGION", "us-east-1")]))
|
||||
.unwrap();
|
||||
|
||||
assert_eq!(
|
||||
|
|
@ -1978,20 +1850,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_secrets(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 +1881,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_secrets(secret_lookup(&[("MESSAGE", "hello world")]))
|
||||
.unwrap();
|
||||
|
||||
let shell = resolved.steps[0].to_shell_command();
|
||||
|
|
@ -2024,7 +1899,7 @@ mod resolve_step_env_tests {
|
|||
|
||||
#[test]
|
||||
fn command_arg_resolving_to_shell_metacharacters_is_not_injected() {
|
||||
// Regression test for the command-injection defect: an `{{ env.* }}`
|
||||
// Regression test for the command-injection defect: an interpolated
|
||||
// value containing a single quote and `;` must be resolved THEN quoted
|
||||
// so it stays a single argument and cannot break out to inject extra
|
||||
// shell commands. Quoting the source token *before* resolving (the old
|
||||
|
|
@ -2032,17 +1907,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_secrets(|name| (name == "USER_INPUT").then(|| malicious.to_string()))
|
||||
.unwrap();
|
||||
|
||||
let shell = resolved.steps[0].to_shell_command();
|
||||
|
|
@ -2063,39 +1935,42 @@ 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(&[]))
|
||||
.resolve_step_secrets(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(&[]))
|
||||
.resolve_step_secrets(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 +1992,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_secrets(secret_lookup(&[
|
||||
("REGION", "us-east-1"),
|
||||
("DEPLOY_TOKEN", "vault-token"),
|
||||
("MESSAGE", "hello world"),
|
||||
]))
|
||||
.unwrap();
|
||||
|
||||
assert_eq!(
|
||||
|
|
@ -2149,7 +2021,7 @@ mod resolve_step_env_tests {
|
|||
};
|
||||
|
||||
let err = settings
|
||||
.resolve_step_env(env_lookup(&[]), secret_lookup(&[]))
|
||||
.resolve_step_secrets(secret_lookup(&[]))
|
||||
.unwrap_err();
|
||||
|
||||
assert_eq!(err.namespace, Namespace::Secrets);
|
||||
|
|
@ -2232,12 +2104,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;
|
||||
|
|
|
|||
|
|
@ -17,7 +17,7 @@
|
|||
export interface RunGoalFile {
|
||||
'type': RunGoalFileTypeEnum;
|
||||
/**
|
||||
* Resolved config string that may contain env interpolation tokens.
|
||||
* Config string that can contain typed interpolation tokens.
|
||||
*/
|
||||
'value': string;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -17,7 +17,7 @@
|
|||
export interface RunGoalInline {
|
||||
'type': RunGoalInlineTypeEnum;
|
||||
/**
|
||||
* Resolved config string that may contain env interpolation tokens.
|
||||
* Config string that can contain typed interpolation tokens.
|
||||
*/
|
||||
'value': string;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -71,7 +71,7 @@ import type { RunScmSettings } from './run-scm-settings';
|
|||
export interface RunNamespace {
|
||||
'goal': RunGoal | null;
|
||||
/**
|
||||
* Resolved config string that may contain env interpolation tokens.
|
||||
* Config string that can contain typed interpolation tokens.
|
||||
*/
|
||||
'working_dir': string | null;
|
||||
'metadata': { [key: string]: string; };
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue