From 3e0db1febf96c6b814693426db07a7ee85a3c8d2 Mon Sep 17 00:00:00 2001 From: Scott Werner Date: Wed, 1 Jul 2026 18:05:25 -0400 Subject: [PATCH 1/9] feat: grant Dependabot alerts read/write to auto-created GitHub Apps (#543) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## What Adds the `vulnerability_alerts: write` fine-grained permission to the GitHub App manifest used when Fabro auto-creates a GitHub App, in **both** install flows: - `lib/crates/fabro-server/src/install.rs` (web-UI install) - `lib/crates/fabro-cli/src/commands/install.rs` (CLI install) `write` on `vulnerability_alerts` grants both read and write of Dependabot alerts (write implies read for fine-grained permissions). The two manifest builders are byte-for-byte identical by design, so both are updated together. A test assertion in the CLI install tests guards the new permission. ## Why We need auto-created Fabro apps to be able to read and manage Dependabot alerts. ## Note on rollout Manifest `default_permissions` are applied at **app-creation time**, so this only affects **newly** auto-created apps. Any app already created won't pick this up automatically — the owner must add the permission in the app's settings, and each existing installation must approve the new permission request. ## Test - `cargo nextest run -p fabro-cli -- manifest_includes_callback_urls_and_setup_url` passes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.8 (1M context) --- lib/crates/fabro-cli/src/commands/install.rs | 7 ++++++- lib/crates/fabro-server/src/install.rs | 3 ++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/crates/fabro-cli/src/commands/install.rs b/lib/crates/fabro-cli/src/commands/install.rs index d5c2145e4..b8fabff08 100644 --- a/lib/crates/fabro-cli/src/commands/install.rs +++ b/lib/crates/fabro-cli/src/commands/install.rs @@ -948,7 +948,8 @@ fn build_github_app_manifest(app_name: &str, port: u16, web_url: &str) -> serde_ "pull_requests": "write", "checks": "write", "issues": "write", - "emails": "read" + "emails": "read", + "vulnerability_alerts": "write" }, "default_events": [] }) @@ -2662,6 +2663,10 @@ client_id = "client-id" manifest["setup_url"], serde_json::json!("https://app.example.com/setup"), ); + assert_eq!( + manifest["default_permissions"]["vulnerability_alerts"], + serde_json::json!("write"), + ); } #[tokio::test] diff --git a/lib/crates/fabro-server/src/install.rs b/lib/crates/fabro-server/src/install.rs index aae91785a..6416c340e 100644 --- a/lib/crates/fabro-server/src/install.rs +++ b/lib/crates/fabro-server/src/install.rs @@ -2053,7 +2053,8 @@ fn build_github_app_manifest( "pull_requests": "write", "checks": "write", "issues": "write", - "emails": "read" + "emails": "read", + "vulnerability_alerts": "write" }, "default_events": [] }) From ec0a08afb3325a20b1d9cbf5b5b75b987202e546 Mon Sep 17 00:00:00 2001 From: Scott Werner Date: Wed, 1 Jul 2026 18:13:26 -0400 Subject: [PATCH 2/9] fix: grant organization_projects to auto-created GitHub Apps for Projects V2 (#544) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## What Adds the `organization_projects: write` permission to the GitHub App manifest used when Fabro auto-creates a GitHub App, in **both** install flows: - `lib/crates/fabro-server/src/install.rs` (web-UI install) - `lib/crates/fabro-cli/src/commands/install.rs` (CLI install) A test assertion in the CLI install tests guards the new permission. ## Why The GitHub Projects V2 tracker mints a scoped installation token requesting `{ "issues": "write", "organization_projects": "write" }` (`create_installation_access_token_for_projects`, `fabro-github/src/lib.rs`). GitHub only lets an installation token request a **subset** of the permissions the app was granted at install time — and `organization_projects` was never in the manifest. So on any auto-created Fabro app, the token request comes back **422** and the tracker fails before it can make a single GraphQL call. `issues: write` (also requested by that helper) is already covered by the manifest; `organization_projects` was the missing piece. ## Note on rollout Manifest `default_permissions` are applied at **app-creation time**, so this only affects **newly** auto-created apps. Existing apps need the permission added manually in their settings, and each installation must approve it. ## Follow-up (not in this PR) The `422` branch in `mint_installation_token_with_jwt` reports "GitHub App does not have access to repository {repo}" — which misattributes a missing-permission failure to repository access. Worth softening the message to mention permissions too; left out here to keep this PR focused on the scope change. ## Test - `cargo nextest run -p fabro-cli -- manifest_includes_callback_urls_and_setup_url` passes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.8 (1M context) --- lib/crates/fabro-cli/src/commands/install.rs | 7 ++++++- lib/crates/fabro-server/src/install.rs | 3 ++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/crates/fabro-cli/src/commands/install.rs b/lib/crates/fabro-cli/src/commands/install.rs index b8fabff08..6c74b31a4 100644 --- a/lib/crates/fabro-cli/src/commands/install.rs +++ b/lib/crates/fabro-cli/src/commands/install.rs @@ -949,7 +949,8 @@ fn build_github_app_manifest(app_name: &str, port: u16, web_url: &str) -> serde_ "checks": "write", "issues": "write", "emails": "read", - "vulnerability_alerts": "write" + "vulnerability_alerts": "write", + "organization_projects": "write" }, "default_events": [] }) @@ -2667,6 +2668,10 @@ client_id = "client-id" manifest["default_permissions"]["vulnerability_alerts"], serde_json::json!("write"), ); + assert_eq!( + manifest["default_permissions"]["organization_projects"], + serde_json::json!("write"), + ); } #[tokio::test] diff --git a/lib/crates/fabro-server/src/install.rs b/lib/crates/fabro-server/src/install.rs index 6416c340e..a24fd5b3b 100644 --- a/lib/crates/fabro-server/src/install.rs +++ b/lib/crates/fabro-server/src/install.rs @@ -2054,7 +2054,8 @@ fn build_github_app_manifest( "checks": "write", "issues": "write", "emails": "read", - "vulnerability_alerts": "write" + "vulnerability_alerts": "write", + "organization_projects": "write" }, "default_events": [] }) From 0e30ae30ba0fd049b4d2f60cf8b7b394d08a9c05 Mon Sep 17 00:00:00 2001 From: "fabro-releases[bot]" Date: Thu, 2 Jul 2026 10:17:41 +0000 Subject: [PATCH 3/9] Bump version to 0.282.0-nightly.0 --- Cargo.lock | 102 ++++++++++++++++++++++++++--------------------------- Cargo.toml | 2 +- 2 files changed, 52 insertions(+), 52 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5b3704880..d75649aa2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2173,7 +2173,7 @@ dependencies = [ [[package]] name = "fabro-acp" -version = "0.281.0-nightly.0" +version = "0.282.0-nightly.0" dependencies = [ "agent-client-protocol", "agent-client-protocol-tokio", @@ -2192,7 +2192,7 @@ dependencies = [ [[package]] name = "fabro-agent" -version = "0.281.0-nightly.0" +version = "0.282.0-nightly.0" dependencies = [ "anyhow", "async-trait", @@ -2234,7 +2234,7 @@ dependencies = [ [[package]] name = "fabro-api" -version = "0.281.0-nightly.0" +version = "0.282.0-nightly.0" dependencies = [ "chrono", "fabro-automation", @@ -2257,7 +2257,7 @@ dependencies = [ [[package]] name = "fabro-auth" -version = "0.281.0-nightly.0" +version = "0.282.0-nightly.0" dependencies = [ "anyhow", "async-trait", @@ -2281,7 +2281,7 @@ dependencies = [ [[package]] name = "fabro-automation" -version = "0.281.0-nightly.0" +version = "0.282.0-nightly.0" dependencies = [ "croner", "hex", @@ -2296,11 +2296,11 @@ dependencies = [ [[package]] name = "fabro-build-support" -version = "0.281.0-nightly.0" +version = "0.282.0-nightly.0" [[package]] name = "fabro-checkpoint" -version = "0.281.0-nightly.0" +version = "0.282.0-nightly.0" dependencies = [ "chrono", "fabro-config", @@ -2316,7 +2316,7 @@ dependencies = [ [[package]] name = "fabro-cli" -version = "0.281.0-nightly.0" +version = "0.282.0-nightly.0" dependencies = [ "anyhow", "assert_cmd", @@ -2418,7 +2418,7 @@ dependencies = [ [[package]] name = "fabro-client" -version = "0.281.0-nightly.0" +version = "0.282.0-nightly.0" dependencies = [ "anyhow", "bytes", @@ -2447,7 +2447,7 @@ dependencies = [ [[package]] name = "fabro-config" -version = "0.281.0-nightly.0" +version = "0.282.0-nightly.0" dependencies = [ "anyhow", "chrono", @@ -2476,7 +2476,7 @@ dependencies = [ [[package]] name = "fabro-core" -version = "0.281.0-nightly.0" +version = "0.282.0-nightly.0" dependencies = [ "async-trait", "fabro-types", @@ -2491,7 +2491,7 @@ dependencies = [ [[package]] name = "fabro-db" -version = "0.281.0-nightly.0" +version = "0.282.0-nightly.0" dependencies = [ "anyhow", "sqlx", @@ -2501,7 +2501,7 @@ dependencies = [ [[package]] name = "fabro-dev" -version = "0.281.0-nightly.0" +version = "0.282.0-nightly.0" dependencies = [ "anyhow", "assert_cmd", @@ -2520,7 +2520,7 @@ dependencies = [ [[package]] name = "fabro-dump" -version = "0.281.0-nightly.0" +version = "0.282.0-nightly.0" dependencies = [ "anyhow", "bytes", @@ -2534,7 +2534,7 @@ dependencies = [ [[package]] name = "fabro-environment" -version = "0.281.0-nightly.0" +version = "0.282.0-nightly.0" dependencies = [ "anyhow", "chrono", @@ -2556,7 +2556,7 @@ dependencies = [ [[package]] name = "fabro-github" -version = "0.281.0-nightly.0" +version = "0.282.0-nightly.0" dependencies = [ "anyhow", "base64", @@ -2578,7 +2578,7 @@ dependencies = [ [[package]] name = "fabro-graphviz" -version = "0.281.0-nightly.0" +version = "0.282.0-nightly.0" dependencies = [ "anyhow", "fabro-types", @@ -2592,7 +2592,7 @@ dependencies = [ [[package]] name = "fabro-hooks" -version = "0.281.0-nightly.0" +version = "0.282.0-nightly.0" dependencies = [ "async-trait", "fabro-agent", @@ -2615,7 +2615,7 @@ dependencies = [ [[package]] name = "fabro-http" -version = "0.281.0-nightly.0" +version = "0.282.0-nightly.0" dependencies = [ "fabro-static", "http 1.4.0", @@ -2625,7 +2625,7 @@ dependencies = [ [[package]] name = "fabro-install" -version = "0.281.0-nightly.0" +version = "0.282.0-nightly.0" dependencies = [ "anyhow", "base64", @@ -2643,7 +2643,7 @@ dependencies = [ [[package]] name = "fabro-interview" -version = "0.281.0-nightly.0" +version = "0.282.0-nightly.0" dependencies = [ "async-trait", "dialoguer", @@ -2658,7 +2658,7 @@ dependencies = [ [[package]] name = "fabro-llm" -version = "0.281.0-nightly.0" +version = "0.282.0-nightly.0" dependencies = [ "anyhow", "async-trait", @@ -2699,7 +2699,7 @@ dependencies = [ [[package]] name = "fabro-macros" -version = "0.281.0-nightly.0" +version = "0.282.0-nightly.0" dependencies = [ "clap", "fabro-options-metadata", @@ -2710,7 +2710,7 @@ dependencies = [ [[package]] name = "fabro-manifest" -version = "0.281.0-nightly.0" +version = "0.282.0-nightly.0" dependencies = [ "anyhow", "fabro-api", @@ -2728,7 +2728,7 @@ dependencies = [ [[package]] name = "fabro-mcp" -version = "0.281.0-nightly.0" +version = "0.282.0-nightly.0" dependencies = [ "anyhow", "axum", @@ -2748,7 +2748,7 @@ dependencies = [ [[package]] name = "fabro-mcp-server" -version = "0.281.0-nightly.0" +version = "0.282.0-nightly.0" dependencies = [ "anyhow", "chrono", @@ -2775,7 +2775,7 @@ dependencies = [ [[package]] name = "fabro-mcp-store" -version = "0.281.0-nightly.0" +version = "0.282.0-nightly.0" dependencies = [ "fabro-types", "serde", @@ -2787,7 +2787,7 @@ dependencies = [ [[package]] name = "fabro-model" -version = "0.281.0-nightly.0" +version = "0.282.0-nightly.0" dependencies = [ "fabro-static", "http 1.4.0", @@ -2803,7 +2803,7 @@ dependencies = [ [[package]] name = "fabro-oauth" -version = "0.281.0-nightly.0" +version = "0.282.0-nightly.0" dependencies = [ "anyhow", "axum", @@ -2825,7 +2825,7 @@ dependencies = [ [[package]] name = "fabro-options-metadata" -version = "0.281.0-nightly.0" +version = "0.282.0-nightly.0" dependencies = [ "serde", "serde_json", @@ -2833,7 +2833,7 @@ dependencies = [ [[package]] name = "fabro-proc" -version = "0.281.0-nightly.0" +version = "0.282.0-nightly.0" dependencies = [ "cc", "libc", @@ -2842,7 +2842,7 @@ dependencies = [ [[package]] name = "fabro-redact" -version = "0.281.0-nightly.0" +version = "0.282.0-nightly.0" dependencies = [ "aho-corasick", "ref-cast", @@ -2858,7 +2858,7 @@ dependencies = [ [[package]] name = "fabro-sandbox" -version = "0.281.0-nightly.0" +version = "0.282.0-nightly.0" dependencies = [ "anyhow", "async-trait", @@ -2903,7 +2903,7 @@ dependencies = [ [[package]] name = "fabro-server" -version = "0.281.0-nightly.0" +version = "0.282.0-nightly.0" dependencies = [ "anyhow", "async-trait", @@ -2994,7 +2994,7 @@ dependencies = [ [[package]] name = "fabro-slack" -version = "0.281.0-nightly.0" +version = "0.282.0-nightly.0" dependencies = [ "fabro-http", "fabro-interview", @@ -3016,18 +3016,18 @@ dependencies = [ [[package]] name = "fabro-spa" -version = "0.281.0-nightly.0" +version = "0.282.0-nightly.0" dependencies = [ "rust-embed", ] [[package]] name = "fabro-static" -version = "0.281.0-nightly.0" +version = "0.282.0-nightly.0" [[package]] name = "fabro-store" -version = "0.281.0-nightly.0" +version = "0.282.0-nightly.0" dependencies = [ "async-trait", "bytes", @@ -3054,7 +3054,7 @@ dependencies = [ [[package]] name = "fabro-telemetry" -version = "0.281.0-nightly.0" +version = "0.282.0-nightly.0" dependencies = [ "anyhow", "base64", @@ -3080,7 +3080,7 @@ dependencies = [ [[package]] name = "fabro-template" -version = "0.281.0-nightly.0" +version = "0.282.0-nightly.0" dependencies = [ "anyhow", "fabro-types", @@ -3094,7 +3094,7 @@ dependencies = [ [[package]] name = "fabro-test" -version = "0.281.0-nightly.0" +version = "0.282.0-nightly.0" dependencies = [ "anyhow", "assert_cmd", @@ -3119,7 +3119,7 @@ dependencies = [ [[package]] name = "fabro-tool" -version = "0.281.0-nightly.0" +version = "0.282.0-nightly.0" dependencies = [ "anyhow", "async-trait", @@ -3140,7 +3140,7 @@ dependencies = [ [[package]] name = "fabro-tracker" -version = "0.281.0-nightly.0" +version = "0.282.0-nightly.0" dependencies = [ "anyhow", "async-trait", @@ -3154,7 +3154,7 @@ dependencies = [ [[package]] name = "fabro-types" -version = "0.281.0-nightly.0" +version = "0.282.0-nightly.0" dependencies = [ "chrono", "clap", @@ -3176,7 +3176,7 @@ dependencies = [ [[package]] name = "fabro-util" -version = "0.281.0-nightly.0" +version = "0.282.0-nightly.0" dependencies = [ "anyhow", "console 0.15.11", @@ -3197,7 +3197,7 @@ dependencies = [ [[package]] name = "fabro-validate" -version = "0.281.0-nightly.0" +version = "0.282.0-nightly.0" dependencies = [ "fabro-acp", "fabro-graphviz", @@ -3210,7 +3210,7 @@ dependencies = [ [[package]] name = "fabro-variable" -version = "0.281.0-nightly.0" +version = "0.282.0-nightly.0" dependencies = [ "anyhow", "chrono", @@ -3227,7 +3227,7 @@ dependencies = [ [[package]] name = "fabro-vault" -version = "0.281.0-nightly.0" +version = "0.282.0-nightly.0" dependencies = [ "chrono", "fabro-static", @@ -3240,7 +3240,7 @@ dependencies = [ [[package]] name = "fabro-workflow" -version = "0.281.0-nightly.0" +version = "0.282.0-nightly.0" dependencies = [ "anyhow", "assert_cmd", @@ -8381,7 +8381,7 @@ dependencies = [ [[package]] name = "twin-github" -version = "0.281.0-nightly.0" +version = "0.282.0-nightly.0" dependencies = [ "axum", "base64", @@ -8400,7 +8400,7 @@ dependencies = [ [[package]] name = "twin-openai" -version = "0.281.0-nightly.0" +version = "0.282.0-nightly.0" dependencies = [ "anyhow", "async-stream", diff --git a/Cargo.toml b/Cargo.toml index c1a17580c..8980f0239 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -5,7 +5,7 @@ resolver = "2" [workspace.package] edition = "2021" -version = "0.281.0-nightly.0" +version = "0.282.0-nightly.0" license = "MIT" [workspace.dependencies] From 91bd115d5395b996baecc830028217b3f6f20e9b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Mazoni?= Date: Thu, 2 Jul 2026 23:33:17 +0930 Subject: [PATCH 4/9] fix(web): Enabling scroll in the Stages sidebar on the run overview/stages tabs (#541) This change enables scrolling the stages sidebar on the run's overview/stages page. Without it, for long runs with lots of stages, the entire page scrolls, hiding the graph while it's running. https://github.com/user-attachments/assets/c5405a5b-8480-46f8-8d7c-4cd4914f6228 --------- Co-authored-by: Claude Opus 4.8 --- apps/fabro-web/app/routes/run-overview.tsx | 22 +++++++++++++--------- apps/fabro-web/app/routes/run-stages.tsx | 4 ++-- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/apps/fabro-web/app/routes/run-overview.tsx b/apps/fabro-web/app/routes/run-overview.tsx index 7abe80333..4234d2641 100644 --- a/apps/fabro-web/app/routes/run-overview.tsx +++ b/apps/fabro-web/app/routes/run-overview.tsx @@ -20,7 +20,7 @@ import { type RunGraphNodeHover, } from "../hooks/use-annotated-run-graph-svg"; -export const handle = { wide: true }; +export const handle = { wide: true, fullHeight: true }; type Direction = "LR" | "TB"; @@ -113,15 +113,19 @@ export default function RunOverview() { }, []); return ( -
- +
+
+ +
-
- +
+
+ +
{graphSvg === undefined && graphQuery.isLoading ? ( -
+
) : graphSvg ? ( -
+
diff --git a/apps/fabro-web/app/routes/run-stages.tsx b/apps/fabro-web/app/routes/run-stages.tsx index b2824d0cc..aa0ae43af 100644 --- a/apps/fabro-web/app/routes/run-stages.tsx +++ b/apps/fabro-web/app/routes/run-stages.tsx @@ -1920,7 +1920,7 @@ export default function RunStages() { return (
-
+
@@ -1933,7 +1933,7 @@ export default function RunStages() { {isAgentStage && ( <> -
+
Date: Fri, 3 Jul 2026 01:05:24 +0930 Subject: [PATCH 5/9] fix(web): respect workflow's rankdir on run overview graph (#549) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Fixes the graph that was always being rendered as `left-to-right` even when the workflow's `rankdir` is `top-to-bottom` ## Test plan - [x] `bun run typecheck` (fabro-web) - [x] `bun test` (fabro-web, full suite — 625 pass) - [x] Manually load a run whose workflow declares `rankdir TB` and confirm the graph renders top-to-bottom on first load, with the toolbar's LR/TB buttons still working as manual overrides 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Sonnet 5 --- apps/fabro-web/app/routes/run-overview.test.tsx | 1 + apps/fabro-web/app/routes/run-overview.tsx | 17 ++++++++++++++--- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/apps/fabro-web/app/routes/run-overview.test.tsx b/apps/fabro-web/app/routes/run-overview.test.tsx index 0601acf80..4d2e5c692 100644 --- a/apps/fabro-web/app/routes/run-overview.test.tsx +++ b/apps/fabro-web/app/routes/run-overview.test.tsx @@ -19,6 +19,7 @@ mock.module("../lib/queries", () => ({ isLoading: currentGraphLoading, mutate: graphMutateMock, }), + useRunGraphSource: () => ({ data: undefined }), useRunStageEvents: () => ({ data: [] }), })); diff --git a/apps/fabro-web/app/routes/run-overview.tsx b/apps/fabro-web/app/routes/run-overview.tsx index 4234d2641..45ef67061 100644 --- a/apps/fabro-web/app/routes/run-overview.tsx +++ b/apps/fabro-web/app/routes/run-overview.tsx @@ -1,7 +1,7 @@ import { useCallback, useMemo, useRef, useState } from "react"; import { useNavigate, useParams } from "react-router"; import { ApiError } from "../lib/api-client"; -import { useRun, useRunGraph, useRunStages } from "../lib/queries"; +import { useRun, useRunGraph, useRunGraphSource, useRunStages } from "../lib/queries"; import { FloatingTooltip } from "../components/floating-tooltip"; import { RunSummaryPanel } from "../components/run-summary-panel"; import { StagePopover } from "../components/stage-popover"; @@ -24,9 +24,20 @@ export const handle = { wide: true, fullHeight: true }; type Direction = "LR" | "TB"; +// Mirrors fabro-graphviz's RANKDIR_RE (lib/crates/fabro-graphviz/src/render.rs) — +// keep the accepted `rankdir=` syntax in sync with that regex. +const RANKDIR_RE = /rankdir\s*=\s*(\w+)/; + +function parseSourceDirection(source: string | undefined): Direction | undefined { + const value = source?.match(RANKDIR_RE)?.[1]; + return value === "LR" || value === "TB" ? value : undefined; +} + export default function RunOverview() { const { id } = useParams(); - const [direction, setDirection] = useState("LR"); + const [direction, setDirection] = useState(undefined); + const sourceQuery = useRunGraphSource(id, direction === undefined); + const activeDirection = direction ?? parseSourceDirection(sourceQuery.data ?? undefined) ?? "TB"; const stagesQuery = useRunStages(id); const graphQuery = useRunGraph(id, direction); const runQuery = useRun(id); @@ -127,7 +138,7 @@ export default function RunOverview() { ) : graphSvg ? (
Date: Thu, 2 Jul 2026 16:58:47 -0400 Subject: [PATCH 6/9] Fix web app load performance: caching, compression, and eager chunk loading (#550) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Problem Loading the web UI from a remote server took **~11 seconds to first render on every refresh**. A HAR capture against a remote deployment showed the page downloading **13.5 MB of JavaScript across 356 files, uncompressed, on every single page load** — even though the assets are content-hashed and served with `Cache-Control: immutable`. Four compounding causes: 1. **`Pragma: no-cache` defeated the browser cache.** The security-headers middleware stamped `Pragma: no-cache` onto every response, including hashed assets that set a year-long immutable `Cache-Control`. Browsers treat a response `Pragma: no-cache` as `Cache-Control: no-cache` and check it *before* `max-age` (Chromium zeroes freshness on it), and since assets carried no validators, "revalidate" degraded into a full re-download. Empirically visible in the HAR: Google-Fonts woff2s served from cache (`transfer = 0`) during the same page load where all 356 of our assets re-downloaded in full. 2. **No response compression.** The server had no compression layer; 13.5 MB of JS compresses to ~2.5 MB with brotli. 3. **The HTML force-loaded every chunk.** `writeIndexHtml` emitted a ``) + .filter((output) => output.kind === "entry-point" && output.path.endsWith(".js")) + .map((output) => ``) .join("\n "); const styles = [ "/assets/app.css", - ...outputs.filter((path) => path.endsWith(".css")).map((path) => `/${path.replaceAll("\\\\", "/")}`), + ...outputs + .filter((output) => output.path.endsWith(".css")) + .map((output) => `/${output.path.replaceAll("\\\\", "/")}`), ] .filter((value, index, array) => array.indexOf(value) === index) .map((path) => ``) diff --git a/lib/crates/fabro-server/Cargo.toml b/lib/crates/fabro-server/Cargo.toml index 8627bebb9..3d54fb216 100644 --- a/lib/crates/fabro-server/Cargo.toml +++ b/lib/crates/fabro-server/Cargo.toml @@ -62,7 +62,7 @@ cookie.workspace = true dirs.workspace = true globset.workspace = true tower = "0.5" -tower-http = { version = "0.6", features = ["trace"] } +tower-http = { version = "0.6", features = ["trace", "compression-br", "compression-gzip"] } tokio-stream = { workspace = true, features = ["sync"] } tokio-util.workspace = true base64.workspace = true diff --git a/lib/crates/fabro-server/src/install.rs b/lib/crates/fabro-server/src/install.rs index a24fd5b3b..7afb7a8b2 100644 --- a/lib/crates/fabro-server/src/install.rs +++ b/lib/crates/fabro-server/src/install.rs @@ -52,7 +52,7 @@ use zeroize::Zeroizing; use crate::error::ApiError; use crate::serve::{self, DEFAULT_TCP_PORT}; use crate::server_secrets::{ServerSecrets, process_env_snapshot}; -use crate::{security_headers, static_files}; +use crate::{security_headers, server, static_files}; #[derive(Clone)] pub struct InstallAppState { @@ -667,6 +667,10 @@ pub fn build_install_router(state: InstallAppState) -> Router { } } })) + // Install mode serves the same multi-megabyte SPA bundle as the main + // router; a first-run setup over a slow link needs compression just + // as much. + .layer(server::compression_layer()) .layer(middleware::from_fn(security_headers::layer)) } diff --git a/lib/crates/fabro-server/src/security_headers.rs b/lib/crates/fabro-server/src/security_headers.rs index 9963fa43d..4ca1e6a9e 100644 --- a/lib/crates/fabro-server/src/security_headers.rs +++ b/lib/crates/fabro-server/src/security_headers.rs @@ -72,9 +72,17 @@ fn apply_defaults(headers: &mut HeaderMap, is_https: bool) { // Conservative cache defaults. Routes that deliberately want to cache // (hashed static assets, public GETs) set their own Cache-Control before - // this middleware runs, which prevents the default from being applied. - set_default(headers, header::CACHE_CONTROL, "no-store"); - set_default(headers, header::PRAGMA, "no-cache"); + // this middleware runs. When they have, we must not also stamp the no-cache + // pair: `Pragma: no-cache` next to a long-lived `Cache-Control: immutable` + // is contradictory, and browsers resolve it by revalidating on every load. + // Since these assets carry no ETag/Last-Modified, that revalidation + // degrades into a full re-download each time. Apply the no-store/no-cache + // defaults only to responses that haven't opted into caching; a present + // Cache-Control is the signal that the handler chose its own policy. + if !headers.contains_key(header::CACHE_CONTROL) { + headers.insert(header::CACHE_CONTROL, HeaderValue::from_static("no-store")); + headers.insert(header::PRAGMA, HeaderValue::from_static("no-cache")); + } set_default(headers, header::VARY, "Accept-Encoding"); // HSTS is a no-op over plain HTTP per RFC 6797, but only emit it on @@ -202,7 +210,10 @@ mod tests { #[test] fn existing_cache_control_is_not_overridden() { // Static assets set their own cache-control with long immutability. - // The middleware default must not clobber it. + // The middleware default must not clobber it, and must not stamp a + // contradictory `Pragma: no-cache` alongside it — that combination + // forces browsers to revalidate (and, absent validators, re-download) + // supposedly-immutable assets on every load. let headers = headers_after(&req("/assets/app-abc.js", &[]), &[( "cache-control", "public, max-age=31536000, immutable", @@ -211,6 +222,10 @@ mod tests { headers.get("cache-control").unwrap(), "public, max-age=31536000, immutable" ); + assert!( + !headers.contains_key("pragma"), + "cacheable responses must not carry Pragma: no-cache" + ); } #[test] diff --git a/lib/crates/fabro-server/src/server.rs b/lib/crates/fabro-server/src/server.rs index 254dc9658..c246e396d 100644 --- a/lib/crates/fabro-server/src/server.rs +++ b/lib/crates/fabro-server/src/server.rs @@ -137,6 +137,7 @@ use tokio_stream::StreamExt; use tokio_stream::wrappers::{BroadcastStream, UnboundedReceiverStream}; use tokio_util::sync::CancellationToken; use tower::{ServiceExt, service_fn}; +use tower_http::compression::{CompressionLayer, CompressionLevel}; use tracing::{Instrument, debug, error, info, warn}; use ulid::Ulid; @@ -1852,6 +1853,10 @@ pub fn build_router_with_options( } router + // Innermost of the outer layers so every response body — static SPA + // assets and JSON API alike — is compressed before the header/log + // middlewares see it. + .layer(compression_layer()) .layer(middleware::from_fn_with_state( canonical_host::Config { state: state_for_canonical_host, @@ -1864,6 +1869,17 @@ pub fn build_router_with_options( .layer(middleware::from_fn(request_id::layer)) } +/// Response-compression layer shared by the main and install-mode routers. +/// +/// The default predicate skips streaming SSE (`text/event-stream`), gRPC, +/// images, and tiny bodies. The quality is pinned because tower-http's +/// default defers to each codec's own default, and brotli's is quality 11 — +/// seconds of CPU on a multi-megabyte asset. Level 4 keeps both codecs fast +/// at a near-optimal ratio. +pub(crate) fn compression_layer() -> CompressionLayer { + CompressionLayer::new().quality(CompressionLevel::Precise(4)) +} + async fn http_log_middleware(mut req: axum_extract::Request, next: Next) -> Response { let path = req.uri().path(); if path.starts_with("/assets/") || path.starts_with("/images/") { diff --git a/lib/crates/fabro-server/src/static_files.rs b/lib/crates/fabro-server/src/static_files.rs index 590a97f82..cb5c500a2 100644 --- a/lib/crates/fabro-server/src/static_files.rs +++ b/lib/crates/fabro-server/src/static_files.rs @@ -1,10 +1,12 @@ +use std::borrow::Cow; use std::path::{Path, PathBuf}; use std::sync::OnceLock; -use axum::body::Body; +use axum::body::{Body, Bytes}; use axum::http::{HeaderMap, HeaderValue, StatusCode, header}; use axum::response::{IntoResponse, Response}; use fabro_static::EnvVars; +use sha2::{Digest, Sha256}; use tokio::fs; use crate::csp; @@ -100,9 +102,8 @@ async fn load_injected_install_shell( asset_root: Option<&Path>, dev_disk_only: bool, ) -> Option> { - Some(inject_install_mode( - load_asset("index.html", asset_root, dev_disk_only).await?, - )) + let shell = load_asset("index.html", asset_root, dev_disk_only).await?; + Some(inject_install_mode(shell.bytes.into())) } #[derive(Clone, Copy, Debug, Eq, PartialEq)] @@ -125,7 +126,7 @@ async fn serve_with_mode( } if let Some(asset) = load_asset_for_mode(&normalized, mode, asset_root, dev_disk_only).await { - return asset_response(&normalized, asset); + return asset_response(&normalized, asset, headers); } // SPA fallback: serve index.html only for browser navigations that @@ -136,7 +137,7 @@ async fn serve_with_mode( if let Some(index) = load_asset_for_mode("index.html", mode, asset_root, dev_disk_only).await { - return asset_response("index.html", index); + return asset_response("index.html", index, headers); } if dev_disk_only { return build_in_progress_response(); @@ -187,7 +188,39 @@ fn normalize(path: &str) -> String { } } -async fn load_asset(path: &str, asset_root: Option<&Path>, dev_disk_only: bool) -> Option> { +/// An asset body plus, when the source precomputed it (the embedded SPA +/// snapshot), its SHA-256. Carrying the hash lets mutable-asset ETags reuse +/// rust-embed's compile-time digest instead of rehashing process-lifetime +/// bytes on every revalidation. +struct Asset { + bytes: Bytes, + sha256: Option<[u8; 32]>, +} + +impl Asset { + fn from_vec(bytes: Vec) -> Self { + Self { + bytes: bytes.into(), + sha256: None, + } + } + + fn from_embedded(asset: fabro_spa::AssetBytes) -> Self { + let sha256 = asset.sha256(); + let bytes = match asset.into_cow() { + // Release builds embed assets as statics; serve them without + // copying the (potentially multi-megabyte) body per request. + Cow::Borrowed(bytes) => Bytes::from_static(bytes), + Cow::Owned(bytes) => Bytes::from(bytes), + }; + Self { + bytes, + sha256: Some(sha256), + } + } +} + +async fn load_asset(path: &str, asset_root: Option<&Path>, dev_disk_only: bool) -> Option { if spa_assets_disabled_for_test() { return None; } @@ -196,11 +229,11 @@ async fn load_asset(path: &str, asset_root: Option<&Path>, dev_disk_only: bool) // workspace's live `dist/` fallback or test isolation breaks. if let Some(root) = asset_root { if let Some(bytes) = read_disk_asset_from_root(root, path).await { - return Some(bytes); + return Some(Asset::from_vec(bytes)); } } else if cfg!(debug_assertions) { if let Some(bytes) = read_disk_asset(path).await { - return Some(bytes); + return Some(Asset::from_vec(bytes)); } } @@ -211,7 +244,7 @@ async fn load_asset(path: &str, asset_root: Option<&Path>, dev_disk_only: bool) return None; } - fabro_spa::get(path).map(fabro_spa::AssetBytes::into_vec) + fabro_spa::get(path).map(Asset::from_embedded) } async fn load_asset_for_mode( @@ -219,9 +252,11 @@ async fn load_asset_for_mode( mode: SpaMode, asset_root: Option<&Path>, dev_disk_only: bool, -) -> Option> { +) -> Option { if mode == SpaMode::Install && path == "index.html" { - return cached_install_mode_shell(asset_root, dev_disk_only).await; + return cached_install_mode_shell(asset_root, dev_disk_only) + .await + .map(Asset::from_vec); } load_asset(path, asset_root, dev_disk_only).await } @@ -276,43 +311,113 @@ fn disk_asset_root() -> PathBuf { PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("../../../apps/fabro-web/dist") } -fn asset_response(path: &str, bytes: Vec) -> Response { +const IMMUTABLE_CACHE_CONTROL: &str = "public, max-age=31536000, immutable"; +const REVALIDATE_CACHE_CONTROL: &str = "no-cache"; + +fn asset_response(path: &str, asset: Asset, request_headers: &HeaderMap) -> Response { + let content_hashed = is_content_hashed(path); + let cache_control = cache_control(content_hashed); + // Mutable assets keep stable names across deploys, so their `no-cache` + // policy needs a validator to revalidate as a cheap 304 instead of a full + // body download on every use. Hashed immutable assets never revalidate, + // so an ETag would be dead weight. + let etag = (!content_hashed).then(|| asset_etag(&asset)); + + if let Some(etag) = &etag { + if if_none_match_matches(request_headers, etag) { + let mut response = Response::new(Body::empty()); + *response.status_mut() = StatusCode::NOT_MODIFIED; + apply_cache_headers(response.headers_mut(), cache_control, Some(etag)); + return response; + } + } + let mime = mime_guess::from_path(path).first_or_octet_stream(); - let mut response = Response::new(Body::from(bytes)); + let mut response = Response::new(Body::from(asset.bytes)); *response.status_mut() = StatusCode::OK; response.headers_mut().insert( header::CONTENT_TYPE, HeaderValue::from_str(mime.as_ref()) .unwrap_or_else(|_| HeaderValue::from_static("application/octet-stream")), ); - response.headers_mut().insert( - header::CACHE_CONTROL, - HeaderValue::from_static(cache_control(path)), - ); + apply_cache_headers(response.headers_mut(), cache_control, etag.as_deref()); response } -fn cache_control(path: &str) -> &'static str { - if path.contains("/assets/") || path.contains('-') && has_hashed_extension(path) { - "public, max-age=31536000, immutable" - } else { - "no-cache" +fn apply_cache_headers(headers: &mut HeaderMap, cache_control: &'static str, etag: Option<&str>) { + headers.insert( + header::CACHE_CONTROL, + HeaderValue::from_static(cache_control), + ); + if let Some(etag) = etag { + if let Ok(value) = HeaderValue::from_str(etag) { + headers.insert(header::ETAG, value); + } } } -fn has_hashed_extension(path: &str) -> bool { - Path::new(path) - .file_name() - .and_then(|name| name.to_str()) - .is_some_and(|name| { - let mut parts = name.split('.'); - let Some(stem) = parts.next() else { - return false; - }; - stem.split('-').count() > 1 +fn asset_etag(asset: &Asset) -> String { + let digest = asset + .sha256 + .unwrap_or_else(|| Sha256::digest(&asset.bytes).into()); + format!("\"{}\"", hex::encode(digest)) +} + +fn if_none_match_matches(headers: &HeaderMap, etag: &str) -> bool { + headers + .get(header::IF_NONE_MATCH) + .and_then(|value| value.to_str().ok()) + .is_some_and(|value| { + value.split(',').map(str::trim).any(|candidate| { + candidate == "*" || candidate.strip_prefix("W/").unwrap_or(candidate) == etag + }) }) } +fn cache_control(content_hashed: bool) -> &'static str { + if content_hashed { + IMMUTABLE_CACHE_CONTROL + } else { + REVALIDATE_CACHE_CONTROL + } +} + +/// True only for the bundler's content-hashed outputs: files directly under +/// `assets/` named `-.js|css` with an 8-char lowercase base-36 +/// hash (e.g. `assets/entry-0sv53bs3.js`). Only those names change whenever +/// their bytes change, which is what makes a year-long `immutable` policy +/// safe. +/// +/// Stable-named files must NOT match — `index.html`, `assets/app.css`, the +/// pierre-diffs worker under `assets/pierre-diffs-worker/`, images — because +/// caching those immutably pins stale copies in browsers across deploys. +/// When in doubt this classifier says "not hashed": the cost of a false +/// negative is one 304 revalidation, the cost of a false positive is a +/// wrongly-pinned asset for up to a year. +fn is_content_hashed(path: &str) -> bool { + let Some(file_name) = path.trim_start_matches('/').strip_prefix("assets/") else { + return false; + }; + if file_name.contains('/') { + // Subdirectories under assets/ (the pierre-diffs worker) hold + // stable-named files copied verbatim from their package. + return false; + } + let Some((stem, extension)) = file_name.rsplit_once('.') else { + return false; + }; + if !matches!(extension, "js" | "css") { + return false; + } + let Some((_, hash)) = stem.rsplit_once('-') else { + return false; + }; + hash.len() == 8 + && hash + .bytes() + .all(|byte| byte.is_ascii_lowercase() || byte.is_ascii_digit()) +} + fn is_source_map(path: &str) -> bool { Path::new(path) .extension() @@ -328,8 +433,8 @@ mod tests { use axum::http::{HeaderMap, HeaderValue, StatusCode, header}; use super::{ - accepts_html, cache_control, inject_install_mode, is_source_map, read_disk_asset_from_root, - serve_with_asset_root, + accepts_html, cache_control, inject_install_mode, is_content_hashed, is_source_map, + read_disk_asset_from_root, serve_with_asset_root, }; fn headers_with_accept(value: &str) -> HeaderMap { @@ -364,11 +469,126 @@ mod tests { #[test] fn hashed_assets_are_cached_immutably() { + for path in [ + "assets/entry-0sv53bs3.js", + "assets/chunk-4tr91ktd.js", + "assets/chunk-x912wb67.css", + ] { + assert!(is_content_hashed(path), "{path} should be content-hashed"); + } + assert_eq!(cache_control(true), "public, max-age=31536000, immutable"); + } + + #[test] + fn stable_named_assets_must_revalidate() { + // Files whose names do NOT change when their bytes change would be + // pinned stale in browsers for a year if marked immutable. + for path in [ + "index.html", + "assets/app.css", + "assets/pierre-diffs-worker/worker-portable.js", + "images/apple-touch-icon.png", + // Dash segment that isn't an 8-char lowercase base-36 hash. + "assets/entry-abc123.js", + // Right hash shape, but not a bundler output extension. + "assets/photo-a1b2c3d4.png", + ] { + assert!( + !is_content_hashed(path), + "{path} should not be content-hashed" + ); + } + assert_eq!(cache_control(false), "no-cache"); + } + + #[tokio::test] + async fn mutable_assets_serve_etag_and_conditional_304() { + let temp_dir = tempfile::tempdir().unwrap(); + let asset_path = temp_dir.path().join("assets/app.css"); + std::fs::create_dir_all(asset_path.parent().unwrap()).unwrap(); + std::fs::write(&asset_path, b"body { color: red }").unwrap(); + + let first = serve_with_asset_root( + "/assets/app.css", + &HeaderMap::new(), + Some(temp_dir.path()), + false, + ) + .await; + assert_eq!(first.status(), StatusCode::OK); assert_eq!( - cache_control("assets/entry-abc123.js"), - "public, max-age=31536000, immutable" + first.headers().get(header::CACHE_CONTROL).unwrap(), + "no-cache" + ); + let etag = first + .headers() + .get(header::ETAG) + .expect("mutable assets should carry an ETag validator") + .clone(); + + let mut conditional = HeaderMap::new(); + conditional.insert(header::IF_NONE_MATCH, etag.clone()); + let second = serve_with_asset_root( + "/assets/app.css", + &conditional, + Some(temp_dir.path()), + false, + ) + .await; + assert_eq!(second.status(), StatusCode::NOT_MODIFIED); + assert_eq!(second.headers().get(header::ETAG).unwrap(), &etag); + assert_eq!( + second.headers().get(header::CACHE_CONTROL).unwrap(), + "no-cache" + ); + let bytes = axum::body::to_bytes(second.into_body(), usize::MAX) + .await + .unwrap(); + assert!(bytes.is_empty(), "304 must not carry a body"); + } + + #[tokio::test] + async fn stale_if_none_match_gets_full_response() { + let temp_dir = tempfile::tempdir().unwrap(); + let asset_path = temp_dir.path().join("assets/app.css"); + std::fs::create_dir_all(asset_path.parent().unwrap()).unwrap(); + std::fs::write(&asset_path, b"body { color: red }").unwrap(); + + let mut conditional = HeaderMap::new(); + conditional.insert( + header::IF_NONE_MATCH, + HeaderValue::from_static("\"0000stale0000\""), + ); + let response = serve_with_asset_root( + "/assets/app.css", + &conditional, + Some(temp_dir.path()), + false, + ) + .await; + assert_eq!(response.status(), StatusCode::OK); + assert!(response.headers().contains_key(header::ETAG)); + } + + #[tokio::test] + async fn immutable_assets_skip_etag() { + let temp_dir = tempfile::tempdir().unwrap(); + let asset_path = temp_dir.path().join("assets/entry-0sv53bs3.js"); + std::fs::create_dir_all(asset_path.parent().unwrap()).unwrap(); + std::fs::write(&asset_path, b"console.log(1)").unwrap(); + + let response = serve_with_asset_root( + "/assets/entry-0sv53bs3.js", + &HeaderMap::new(), + Some(temp_dir.path()), + false, + ) + .await; + assert_eq!(response.status(), StatusCode::OK); + assert!( + !response.headers().contains_key(header::ETAG), + "immutable assets never revalidate, so a validator is dead weight" ); - assert_eq!(cache_control("index.html"), "no-cache"); } #[tokio::test] diff --git a/lib/crates/fabro-server/tests/it/api/compression.rs b/lib/crates/fabro-server/tests/it/api/compression.rs new file mode 100644 index 000000000..fe3872840 --- /dev/null +++ b/lib/crates/fabro-server/tests/it/api/compression.rs @@ -0,0 +1,202 @@ +//! Response compression on the outer router. +//! +//! The compression layer sits at the outermost edge of `build_router`, so +//! these tests exercise it through the full middleware stack rather than in +//! isolation. `/api/v1/openapi.json` is used as the probe response: it is a +//! multi-hundred-KB JSON body, comfortably above the compression size floor. + +#![expect( + clippy::disallowed_methods, + reason = "integration tests stage fixtures with sync std::fs; test infrastructure, not Tokio-hot path" +)] + +use std::net::SocketAddr; + +use axum::Router; +use axum::body::Body; +use axum::http::{Request, StatusCode, header}; +use fabro_server::server::RouterOptions; +use tempfile::TempDir; +use tower::ServiceExt; + +use crate::helpers::{api, test_app_state}; + +/// Router serving an SPA shell comfortably above the compression size floor, +/// through the same fallback service production uses for static assets. +fn spa_router_with_big_index() -> (Router, TempDir) { + let temp_dir = tempfile::tempdir().expect("SPA fixture tempdir should create"); + std::fs::write( + temp_dir.path().join("index.html"), + format!("spa{}", "x".repeat(8192)), + ) + .expect("SPA fixture index.html should write"); + let app = fabro_server::test_support::build_test_router_with_options( + test_app_state(), + RouterOptions { + web_enabled: true, + static_asset_root: Some(temp_dir.path().to_path_buf()), + ..RouterOptions::default() + }, + ); + (app, temp_dir) +} + +async fn serve_on_ephemeral_port(app: Router) -> SocketAddr { + let listener = tokio::net::TcpListener::bind("127.0.0.1:0") + .await + .expect("test TCP listener should bind"); + let addr = listener + .local_addr() + .expect("test TCP listener should have a local address"); + tokio::spawn(async move { + let _ = axum::serve(listener, app).await; + }); + addr +} + +fn openapi_request(accept_encoding: Option<&str>) -> Request { + let mut builder = Request::builder().method("GET").uri(api("/openapi.json")); + if let Some(encoding) = accept_encoding { + builder = builder.header(header::ACCEPT_ENCODING, encoding); + } + builder + .body(Body::empty()) + .expect("openapi request should build") +} + +#[tokio::test] +async fn responses_are_gzip_compressed_when_client_accepts_gzip() { + let app = fabro_server::test_support::build_test_router(test_app_state()); + let response = app.oneshot(openapi_request(Some("gzip"))).await.unwrap(); + + assert_eq!(response.status(), StatusCode::OK); + assert_eq!( + response + .headers() + .get(header::CONTENT_ENCODING) + .and_then(|v| v.to_str().ok()), + Some("gzip"), + "large JSON responses should be gzip-compressed when the client asks" + ); +} + +#[tokio::test] +async fn responses_are_brotli_compressed_when_client_prefers_br() { + let app = fabro_server::test_support::build_test_router(test_app_state()); + let response = app + .oneshot(openapi_request(Some("gzip, br"))) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::OK); + assert_eq!( + response + .headers() + .get(header::CONTENT_ENCODING) + .and_then(|v| v.to_str().ok()), + Some("br"), + "brotli should win encoding negotiation when offered" + ); +} + +#[tokio::test] +async fn spa_assets_are_compressed() { + // SPA assets are served by the router's fallback service, not a regular + // route — this test pins that compression covers that path too, since the + // multi-megabyte JS bundle is the single largest thing the server sends. + let (app, _temp_dir) = spa_router_with_big_index(); + let request = Request::builder() + .method("GET") + .uri("/") + .header(header::ACCEPT, "text/html") + .header(header::ACCEPT_ENCODING, "gzip") + .body(Body::empty()) + .expect("spa request should build"); + let response = app.oneshot(request).await.unwrap(); + + assert_eq!(response.status(), StatusCode::OK); + assert_eq!( + response + .headers() + .get(header::CONTENT_ENCODING) + .and_then(|v| v.to_str().ok()), + Some("gzip"), + "SPA shell served through the fallback must be compressed" + ); +} + +#[tokio::test] +async fn compression_applies_over_a_real_tcp_connection() { + // `oneshot` exercises the tower stack directly; this pins the same + // behavior through hyper's real connection handling, matching how the + // production server actually serves (`axum::serve`). + let app = fabro_server::test_support::build_test_router(test_app_state()); + let addr = serve_on_ephemeral_port(app).await; + + let response = fabro_test::test_http_client() + .get(format!("http://{addr}/api/v1/openapi.json")) + // Setting the header manually also disables reqwest's transparent + // decompression, so Content-Encoding stays visible on the response. + .header(header::ACCEPT_ENCODING.as_str(), "gzip") + .send() + .await + .expect("openapi request should succeed"); + + assert_eq!(response.status(), fabro_http::StatusCode::OK); + assert_eq!( + response + .headers() + .get(header::CONTENT_ENCODING.as_str()) + .and_then(|v| v.to_str().ok()), + Some("gzip"), + "compression must survive real hyper serving, not just oneshot" + ); +} + +#[tokio::test] +async fn spa_assets_compress_over_a_real_tcp_connection() { + use tokio::io::{AsyncReadExt, AsyncWriteExt}; + + let (app, _temp_dir) = spa_router_with_big_index(); + let addr = serve_on_ephemeral_port(app).await; + + // Raw HTTP/1.1 over the socket: no client-side redirect following or + // transparent decompression can distort what the server actually sent. + // Host matches the test state's canonical origin so the canonical-host + // redirect stays out of the way. + let mut stream = tokio::net::TcpStream::connect(addr).await.unwrap(); + stream + .write_all( + b"GET / HTTP/1.1\r\nHost: localhost:3000\r\nAccept: text/html\r\nAccept-Encoding: gzip\r\nConnection: close\r\n\r\n", + ) + .await + .unwrap(); + let mut raw = Vec::new(); + stream.read_to_end(&mut raw).await.unwrap(); + let head_len = raw + .windows(4) + .position(|w| w == b"\r\n\r\n") + .expect("response should have a header block"); + let head = String::from_utf8_lossy(&raw[..head_len]).to_lowercase(); + + assert!( + head.starts_with("http/1.1 200"), + "unexpected response: {head}" + ); + assert!( + head.contains("content-encoding: gzip"), + "fallback-served SPA shell must compress over real TCP; got:\n{head}" + ); +} + +#[tokio::test] +async fn responses_stay_identity_encoded_without_accept_encoding() { + let app = fabro_server::test_support::build_test_router(test_app_state()); + let response = app.oneshot(openapi_request(None)).await.unwrap(); + + assert_eq!(response.status(), StatusCode::OK); + assert!( + !response.headers().contains_key(header::CONTENT_ENCODING), + "clients that don't advertise Accept-Encoding must get identity bodies" + ); +} diff --git a/lib/crates/fabro-server/tests/it/api/mod.rs b/lib/crates/fabro-server/tests/it/api/mod.rs index 9fe4bcbc0..7fcdf254a 100644 --- a/lib/crates/fabro-server/tests/it/api/mod.rs +++ b/lib/crates/fabro-server/tests/it/api/mod.rs @@ -1,6 +1,7 @@ mod auth_sessions; mod automations; mod cli_auth_token; +mod compression; mod docs; mod environments; mod events; diff --git a/lib/crates/fabro-spa/src/lib.rs b/lib/crates/fabro-spa/src/lib.rs index 66defab64..dd050d0c9 100644 --- a/lib/crates/fabro-spa/src/lib.rs +++ b/lib/crates/fabro-spa/src/lib.rs @@ -8,24 +8,43 @@ use rust_embed::RustEmbed; #[exclude = "**/*.map"] struct EmbeddedAssets; -pub struct AssetBytes(Cow<'static, [u8]>); +pub struct AssetBytes { + data: Cow<'static, [u8]>, + sha256: [u8; 32], +} impl AssetBytes { #[must_use] pub fn into_vec(self) -> Vec { - self.0.into_owned() + self.data.into_owned() + } + + #[must_use] + pub fn into_cow(self) -> Cow<'static, [u8]> { + self.data + } + + /// SHA-256 of the asset bytes. rust-embed computes it at compile time in + /// release builds, so callers can use it as a validator without rehashing + /// the body per request. + #[must_use] + pub fn sha256(&self) -> [u8; 32] { + self.sha256 } } impl AsRef<[u8]> for AssetBytes { fn as_ref(&self) -> &[u8] { - self.0.as_ref() + self.data.as_ref() } } #[must_use] pub fn get(path: &str) -> Option { - EmbeddedAssets::get(path).map(|file| AssetBytes(file.data)) + EmbeddedAssets::get(path).map(|file| AssetBytes { + sha256: file.metadata.sha256_hash(), + data: file.data, + }) } #[cfg(test)] From c1ff4a3e338fa41ccf287fbf9a55794be142197f Mon Sep 17 00:00:00 2001 From: "fabro-sh-fabro[bot]" <296591931+fabro-sh-fabro[bot]@users.noreply.github.com> Date: Thu, 2 Jul 2026 16:59:41 -0400 Subject: [PATCH 7/9] fabro-redact: add SecretRedactor for per-run exact-value redaction (#542) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a `SecretRedactor` primitive to `fabro-redact` so that low-entropy secret values (e.g. environment names, short tokens) are redacted even when the existing content-based heuristics (`redact_string`, `redact_json_value`) would leave them alone. The type is a cheap, `Clone`-able handle backed by `Arc>>`, so a clone handed to another subsystem shares the same registry. `register` ignores empty/whitespace-only values to prevent a footgun that would blank all output. `redact_into` sorts and merges match regions before substituting, so a secret that is a prefix of another longer secret is handled correctly (longest wins via union). `redact_json` walks string leaves in objects and arrays; object keys are left intact. This is an inert library primitive — it changes no existing behavior and is wired up by Plan C. The existing `"REDACTED"` literal is extracted to a `pub(crate) REDACTION_MARKER` constant so both the old path and the new one stay in sync. ### Fabro Details
Ran 8 stages in 43m 24s for $5.69 | Stage | Duration | Cost | Retries | |---|---|---|---| | start | 0s | – | 0 | | toolchain | 1s | – | 0 | | preflight_compile | 2m 23s | – | 0 | | preflight_lint | 2m 33s | – | 0 | | implement | 20m 1s | $3.09 | 0 | | simplify_opus | 4m 13s | $1.27 | 0 | | simplify_gpt | 7m 29s | $1.33 | 0 | | verify | 6m 16s | – | 0 | | **Total** | **43m 24s** | **$5.69** | **0** |
Ran ImplementPlan.fabro (11 nodes and 14 edges) ```dot digraph ImplementPlan { graph [ goal="Implement and simplify", model_stylesheet=" * { model: claude-opus-4-8; } " ] rankdir=LR start [shape=Mdiamond, label="Start"] exit [shape=Msquare, label="Exit"] toolchain [label="Toolchain", shape=parallelogram, script="command -v cargo >/dev/null || { curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y && sudo ln -sf $HOME/.cargo/bin/* /usr/local/bin/; }; cargo --version 2>&1", max_retries=0] preflight_compile [label="Preflight Compile", shape=parallelogram, script="cargo check -q --workspace 2>&1", max_retries=0] preflight_lint [label="Preflight Lint", shape=parallelogram, script="cargo +nightly-2026-04-14 clippy -q --workspace --all-targets -- -D warnings 2>&1", max_retries=0] fix_lints [label="Fix Lints", prompt="The preflight lint step failed. Read the build output from context and fix all clippy lint warnings.", max_visits=3] implement [label="Implement", prompt="Read the plan file referenced in the goal and implement every step. Make all the code changes described in the plan. Use red/green TDD.", model="gpt-55", reasoning_effort="xhigh"] simplify_opus [label="Simplify (Opus)", prompt="@prompts/simplify.md"] simplify_gpt [label="Simplify (GPT-55)", prompt="@prompts/simplify.md", model="gpt-55"] verify [label="Verify", shape=parallelogram, timeout="1800s", script="git fetch origin main 2>&1 && git merge --no-edit --no-stat origin/main 2>&1 && cargo +nightly-2026-04-14 fmt --all 2>&1 && cargo dev docs refresh 2>&1 && cargo +nightly-2026-04-14 fmt --check --all 2>&1 && { command -v rg >/dev/null 2>&1 || { echo 'rg is required for verify'; exit 127; }; } && ! rg -n 'AuthMode::Disabled|RunAuthMethod|RunSubjectProvenance|\bActorRef\b|\bActorKind\b|AuthenticatedSubject|AuthenticatedService|AuthorizeRunScoped|AuthorizeRunBlob|AuthorizeStageArtifact|AuthorizeCommandLog|auth_method\s*==\s*\"disabled\"' lib/crates apps lib/packages docs/public/api-reference/fabro-api.yaml 2>&1 && cargo +nightly-2026-04-14 clippy --workspace --all-targets -- -D warnings 2>&1 && cargo nextest run --workspace --status-level slow --profile ci 2>&1 && cargo dev docs check 2>&1 && bun install --frozen-lockfile 2>&1 && (cd apps/fabro-web && bun run typecheck) 2>&1 && (cd apps/fabro-web && bun run test) 2>&1 && (cd lib/packages/fabro-api-client && bun run typecheck) 2>&1 && cargo dev build -- -p fabro-cli --release 2>&1", goal_gate=true, retry_target="fixup"] fixup [label="Fixup", prompt="The verify step failed. Read the build output from context and fix all format, clippy, Rust test, docs, TypeScript typecheck/test, and build failures.", max_visits=3] start -> toolchain toolchain -> preflight_compile [condition="outcome=succeeded"] toolchain -> exit preflight_compile -> preflight_lint [condition="outcome=succeeded"] preflight_compile -> exit preflight_lint -> implement [condition="outcome=succeeded"] preflight_lint -> fix_lints fix_lints -> preflight_lint implement -> simplify_opus -> simplify_gpt -> verify verify -> exit [condition="outcome=succeeded"] verify -> fixup fixup -> verify } ```
⚒️ Generated with [Fabro](https://fabro.sh) --------- Co-authored-by: Fabro --- lib/crates/fabro-redact/src/lib.rs | 13 +- .../fabro-redact/src/secret_registry.rs | 217 ++++++++++++++++++ 2 files changed, 229 insertions(+), 1 deletion(-) create mode 100644 lib/crates/fabro-redact/src/secret_registry.rs diff --git a/lib/crates/fabro-redact/src/lib.rs b/lib/crates/fabro-redact/src/lib.rs index 170cf7a81..dd5da867b 100644 --- a/lib/crates/fabro-redact/src/lib.rs +++ b/lib/crates/fabro-redact/src/lib.rs @@ -8,9 +8,13 @@ mod entropy; mod gitleaks; mod jsonl; mod safe_url; +mod secret_registry; pub use jsonl::{redact_json_value, redact_jsonl_line}; pub use safe_url::{DisplaySafeUrl, DisplaySafeUrlError}; +pub use secret_registry::SecretRedactor; + +pub(crate) const REDACTION_MARKER: &str = "REDACTED"; /// Redact a URL string for log or error output. /// @@ -41,7 +45,14 @@ pub struct Region { pub fn redact_string(s: &str) -> String { let mut regions = entropy::find_entropy_regions(s); regions.extend(gitleaks::find_gitleaks_regions(s)); + redact_regions(s, regions) +} +/// Replace each region of `s` with [`REDACTION_MARKER`]. +/// +/// Regions may be unsorted and overlapping; they are sorted by start and +/// overlapping regions are merged so the union is redacted as a single marker. +pub(crate) fn redact_regions(s: &str, mut regions: Vec) -> String { if regions.is_empty() { return s.to_string(); } @@ -65,7 +76,7 @@ pub fn redact_string(s: &str) -> String { let mut prev = 0; for r in &merged { result.push_str(&s[prev..r.start]); - result.push_str("REDACTED"); + result.push_str(REDACTION_MARKER); prev = r.end; } result.push_str(&s[prev..]); diff --git a/lib/crates/fabro-redact/src/secret_registry.rs b/lib/crates/fabro-redact/src/secret_registry.rs new file mode 100644 index 000000000..764eb0fe5 --- /dev/null +++ b/lib/crates/fabro-redact/src/secret_registry.rs @@ -0,0 +1,217 @@ +use std::sync::{Arc, PoisonError, RwLock, RwLockReadGuard, RwLockWriteGuard}; + +use serde_json::Value; + +use crate::Region; + +/// Per-run registry of exact secret values to redact from strings and JSON. +/// +/// This complements the crate's content-based redaction by redacting registered +/// values even when they do not look like credentials. Clones share the same +/// registry so callers can hand a redactor to another subsystem and continue to +/// register values through the original. Registered values are exact substring +/// matches and may be low-entropy strings such as environment names. +#[derive(Clone, Default)] +pub struct SecretRedactor { + values: Arc>>, +} + +impl SecretRedactor { + /// Register a secret value for exact substring redaction. + /// + /// Empty or whitespace-only values are ignored so an accidental empty + /// registration cannot redact every output boundary. + pub fn register(&self, value: impl Into) { + let value = value.into(); + if value.trim().is_empty() { + return; + } + + let mut values = self.write(); + if !values.contains(&value) { + values.push(value); + } + } + + /// Return `true` when no secret values have been registered. + pub fn is_empty(&self) -> bool { + self.read().is_empty() + } + + /// Redact all registered secret values from `s`. + pub fn redact_into(&self, s: &str) -> String { + let Some(values) = self.values_snapshot() else { + return s.to_string(); + }; + redact_string_values(s, &values) + } + + /// Redact registered secret values from every JSON string value. + /// + /// Object keys and non-string values are left unchanged. + pub fn redact_json(&self, mut value: Value) -> Value { + let Some(values) = self.values_snapshot() else { + return value; + }; + + redact_json_leaves(&mut value, &values); + value + } + + fn read(&self) -> RwLockReadGuard<'_, Vec> { + self.values.read().unwrap_or_else(PoisonError::into_inner) + } + + fn write(&self) -> RwLockWriteGuard<'_, Vec> { + self.values.write().unwrap_or_else(PoisonError::into_inner) + } + + fn values_snapshot(&self) -> Option> { + let values = self.read(); + if values.is_empty() { + return None; + } + Some(values.clone()) + } +} + +fn redact_json_leaves(value: &mut Value, values: &[String]) { + match value { + Value::Object(obj) => { + for child in obj.values_mut() { + redact_json_leaves(child, values); + } + } + Value::Array(arr) => { + for child in arr { + redact_json_leaves(child, values); + } + } + Value::String(text) => { + let redacted = redact_string_values(text, values); + if redacted != *text { + *text = redacted; + } + } + _ => {} + } +} + +/// Collect every match of each registered value and let +/// [`crate::redact_regions`] sort and merge overlaps, so a secret that overlaps +/// another is fully redacted. +/// +/// Assumes a small number of registered values (bounded by the run's declared +/// secrets), so the per-value scan is not optimized further. +fn redact_string_values(s: &str, values: &[String]) -> String { + let mut regions = Vec::new(); + for value in values { + for (start, _) in s.match_indices(value) { + regions.push(Region { + start, + end: start + value.len(), + }); + } + } + + if regions.is_empty() { + return s.to_string(); + } + + crate::redact_regions(s, regions) +} + +#[cfg(test)] +mod tests { + use serde_json::json; + + use super::SecretRedactor; + + #[test] + fn redacts_registered_low_entropy_value() { + let redactor = SecretRedactor::default(); + redactor.register("staging"); + + assert_eq!( + crate::redact_string("deploy to staging"), + "deploy to staging" + ); + assert_eq!( + redactor.redact_into("deploy to staging"), + "deploy to REDACTED" + ); + } + + #[test] + fn ignores_empty_and_whitespace_values() { + let redactor = SecretRedactor::default(); + redactor.register(""); + redactor.register(" "); + + assert_eq!( + redactor.redact_into("deploy to staging"), + "deploy to staging" + ); + } + + #[test] + fn redacts_overlapping_values_longest_first() { + let redactor = SecretRedactor::default(); + redactor.register("abc"); + redactor.register("abcdef"); + + assert_eq!(redactor.redact_into("token=abcdef"), "token=REDACTED"); + } + + #[test] + fn empty_registry_is_identity() { + let redactor = SecretRedactor::default(); + let value = json!({ + "env": "staging", + "items": ["staging", 42], + }); + + assert_eq!( + redactor.redact_into("deploy to staging"), + "deploy to staging" + ); + assert_eq!(redactor.redact_json(value.clone()), value); + assert!(redactor.is_empty()); + } + + #[test] + fn redact_json_redacts_nested_object_values_and_array_elements() { + let redactor = SecretRedactor::default(); + redactor.register("staging"); + let value = json!({ + "environment": "staging", + "items": [ + "keep", + "deploy staging now" + ], + "staging": "object keys are not redacted", + }); + + assert_eq!( + redactor.redact_json(value), + json!({ + "environment": "REDACTED", + "items": [ + "keep", + "deploy REDACTED now" + ], + "staging": "object keys are not redacted", + }) + ); + } + + #[test] + fn clones_share_registered_values() { + let redactor = SecretRedactor::default(); + let clone = redactor.clone(); + + redactor.register("staging"); + + assert_eq!(clone.redact_into("deploy to staging"), "deploy to REDACTED"); + } +} From 9008058eab1518eb8bedbebe87c33bf5548d39ca Mon Sep 17 00:00:00 2001 From: "fabro-sh-fabro[bot]" <296591931+fabro-sh-fabro[bot]@users.noreply.github.com> Date: Thu, 2 Jul 2026 17:00:20 -0400 Subject: [PATCH 8/9] Resolve secret tokens at the run boundary (worker-side, fail-closed) (#545) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary `{{ secrets.NAME }}` tokens in workflow config (MCP transport, prepare steps, run environment) now resolve from the server vault at the run boundary — the same late-binding point where `{{ env.* }}` tokens resolve. Secret values are never persisted and never left literal in resolved commands or env; a missing or non-Token secret aborts startup with a clear error. ## What changed **`fabro-types` — `run.rs`** - `resolve_env_string` (shared choke-point for MCP and prepare) gains a `secrets_lookup` parameter and routes through `ResolveCtx::new().with_env(...).with_secrets(...)` / `resolve_with`. - `McpServerSettings::resolve_transport_env` and `RunPrepareSettings::resolve_step_env` thread the new parameter through. - `RunEnvironmentSettings::resolve_env` becomes fallible (`Result, ResolveError>`). Per-value error handling preserves the historical env fallback for `Namespace::Env`-only errors while failing closed for `Namespace::Secrets` errors. The intentional `as_source()` fallback is gated behind its `#[expect(clippy::disallowed_methods)]` with an explicit reason. **`fabro-workflow` — `start.rs`** - A single vault read guard is acquired once at the top of `RunSession::new`, replacing the previous per-site reads (Daytona key, etc.). - `vault_token_lookup` wraps `fabro_auth::vault_get_token` — returning `Some(value)` only for `Token`-type secrets; `Oauth` and `File` secrets become `None` (fail-closed). - The shared `secret_lookup` closure is threaded into `runtime_mcp_server`, `runtime_setup_commands`, and `resolve_env`. `resolve_docker_config` gains the same parameter and now returns `Result`. **`fabro-sandbox` — `from_environment.rs`** - `docker_config_from_environment` (server-preflight path, no vault available) retains `resolve_or_source` behavior unchanged. - New `docker_config_from_environment_with_secrets` is the vault-backed variant used by `start.rs`. **`fabro-cli` — `exec.rs`** - `fabro exec` has no vault; passes `|_| None` for secrets, preserving existing behavior with updated call signature. ### Plan summary - **B.1** — Secret lookup threaded through `resolve_env_string` / `resolve_transport_env` / `resolve_step_env` / `resolve_env` in `fabro-types`. - **B.2** — Vault-backed `secret_lookup` closure built once in `RunSession::new` and passed to all boundary resolvers in `start.rs`. - **B.3** — Persistence invariant test: a created run's persisted `RunCreated` event still carries `{{ secrets.DEPLOY_TOKEN }}` in source form, not the resolved value. - **B.4** — Verification (fmt, clippy, nextest, release build) with hermetic temp-vault tests. ### Key design decisions - **Fail closed everywhere secrets are referenced** — no source fallback for secret tokens, even in `resolve_env` which otherwise keeps the env fallback. This is enforced by checking `value.references(Namespace::Secrets)` before the fallback branch. - **Token-only** — `vault_get_token` enforces this; `Oauth` and `File` secrets silently become `None` and then hard-error via the resolver, not a panic. - **Single vault read guard per `RunSession::new`** — acquired once, shared across MCP / prepare / env resolvers, then dropped before the struct is returned. Mirrors how the Daytona key was already read. - **`fabro exec` stays unchanged behaviorally** — the added `|_| None` secrets argument makes the new signature explicit about having no vault. ### Fabro Details
Ran 9 stages in 82m 55s for $24.43 | Stage | Duration | Cost | Retries | |---|---|---|---| | start | 0s | – | 0 | | toolchain | 1s | – | 0 | | preflight_compile | 2m 21s | – | 0 | | preflight_lint | 2m 34s | – | 0 | | implement | 46m 58s | $15.83 | 0 | | simplify_opus | 12m 43s | $4.79 | 0 | | simplify_gpt | 6m 43s | $3.18 | 0 | | verify | 6m 41s | – | 0 | | fixup | 4m 33s | $0.63 | 0 | | **Total** | **82m 55s** | **$24.43** | **0** |
Ran ImplementPlan.fabro (11 nodes and 14 edges) ```dot digraph ImplementPlan { graph [ goal="Implement and simplify", model_stylesheet=" * { model: claude-opus-4-8; } " ] rankdir=LR start [shape=Mdiamond, label="Start"] exit [shape=Msquare, label="Exit"] toolchain [label="Toolchain", shape=parallelogram, script="command -v cargo >/dev/null || { curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y && sudo ln -sf $HOME/.cargo/bin/* /usr/local/bin/; }; cargo --version 2>&1", max_retries=0] preflight_compile [label="Preflight Compile", shape=parallelogram, script="cargo check -q --workspace 2>&1", max_retries=0] preflight_lint [label="Preflight Lint", shape=parallelogram, script="cargo +nightly-2026-04-14 clippy -q --workspace --all-targets -- -D warnings 2>&1", max_retries=0] fix_lints [label="Fix Lints", prompt="The preflight lint step failed. Read the build output from context and fix all clippy lint warnings.", max_visits=3] implement [label="Implement", prompt="Read the plan file referenced in the goal and implement every step. Make all the code changes described in the plan. Use red/green TDD.", model="gpt-55", reasoning_effort="xhigh"] simplify_opus [label="Simplify (Opus)", prompt="@prompts/simplify.md"] simplify_gpt [label="Simplify (GPT-55)", prompt="@prompts/simplify.md", model="gpt-55"] verify [label="Verify", shape=parallelogram, timeout="1800s", script="git fetch origin main 2>&1 && git merge --no-edit --no-stat origin/main 2>&1 && cargo +nightly-2026-04-14 fmt --all 2>&1 && cargo dev docs refresh 2>&1 && cargo +nightly-2026-04-14 fmt --check --all 2>&1 && { command -v rg >/dev/null 2>&1 || { echo 'rg is required for verify'; exit 127; }; } && ! rg -n 'AuthMode::Disabled|RunAuthMethod|RunSubjectProvenance|\bActorRef\b|\bActorKind\b|AuthenticatedSubject|AuthenticatedService|AuthorizeRunScoped|AuthorizeRunBlob|AuthorizeStageArtifact|AuthorizeCommandLog|auth_method\s*==\s*\"disabled\"' lib/crates apps lib/packages docs/public/api-reference/fabro-api.yaml 2>&1 && cargo +nightly-2026-04-14 clippy --workspace --all-targets -- -D warnings 2>&1 && cargo nextest run --workspace --status-level slow --profile ci 2>&1 && cargo dev docs check 2>&1 && bun install --frozen-lockfile 2>&1 && (cd apps/fabro-web && bun run typecheck) 2>&1 && (cd apps/fabro-web && bun run test) 2>&1 && (cd lib/packages/fabro-api-client && bun run typecheck) 2>&1 && cargo dev build -- -p fabro-cli --release 2>&1", goal_gate=true, retry_target="fixup"] fixup [label="Fixup", prompt="The verify step failed. Read the build output from context and fix all format, clippy, Rust test, docs, TypeScript typecheck/test, and build failures.", max_visits=3] start -> toolchain toolchain -> preflight_compile [condition="outcome=succeeded"] toolchain -> exit preflight_compile -> preflight_lint [condition="outcome=succeeded"] preflight_compile -> exit preflight_lint -> implement [condition="outcome=succeeded"] preflight_lint -> fix_lints fix_lints -> preflight_lint implement -> simplify_opus -> simplify_gpt -> verify verify -> exit [condition="outcome=succeeded"] verify -> fixup fixup -> verify } ```
⚒️ Generated with [Fabro](https://fabro.sh) --------- Co-authored-by: Fabro --- Cargo.lock | 1 + lib/crates/fabro-cli/src/commands/exec.rs | 6 +- .../fabro-sandbox/src/from_environment.rs | 33 +- lib/crates/fabro-types/src/settings/run.rs | 390 ++++++++++++---- lib/crates/fabro-workflow/Cargo.toml | 1 + .../fabro-workflow/src/operations/create.rs | 91 +++- .../fabro-workflow/src/operations/start.rs | 431 ++++++++++++++++-- 7 files changed, 808 insertions(+), 145 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b62ab7465..789505b27 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3360,6 +3360,7 @@ dependencies = [ "serde", "serde_json", "sha2 0.10.9", + "shlex", "tempfile", "thiserror 2.0.18", "tokio", diff --git a/lib/crates/fabro-cli/src/commands/exec.rs b/lib/crates/fabro-cli/src/commands/exec.rs index 935e81e95..49316eacf 100644 --- a/lib/crates/fabro-cli/src/commands/exec.rs +++ b/lib/crates/fabro-cli/src/commands/exec.rs @@ -345,13 +345,13 @@ pub(crate) async fn execute(mut args: ExecArgs, ctx: &CommandContext) -> AnyResu // 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 and reserved secrets/inputs tokens surface loudly rather than - // leaking. + // error. `fabro exec` has no server vault, so secrets/inputs tokens surface + // loudly rather than leaking. let mcp_servers = mcp_servers .into_iter() .map(|settings| { settings - .resolve_transport_env(process_env_var) + .resolve_transport_env(process_env_var, |_| None) .with_context(|| format!("failed to resolve MCP server {:?}", settings.name)) }) .collect::>>()?; diff --git a/lib/crates/fabro-sandbox/src/from_environment.rs b/lib/crates/fabro-sandbox/src/from_environment.rs index 89e7e1b73..ac972f22c 100644 --- a/lib/crates/fabro-sandbox/src/from_environment.rs +++ b/lib/crates/fabro-sandbox/src/from_environment.rs @@ -5,6 +5,7 @@ use std::path::{Path, PathBuf}; +use fabro_types::settings::ResolveError; #[cfg(feature = "daytona")] use fabro_types::settings::run::DockerfileSource as ResolvedDockerfileSource; use fabro_types::settings::run::{EnvironmentNetworkMode, RunEnvironmentSettings}; @@ -70,8 +71,36 @@ pub fn docker_config_from_environment( settings: &RunEnvironmentSettings, skip_clone: bool, ) -> DockerSandboxOptions { - let mut env_vars = settings - .resolve_env(process_env_var) + // 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. + let env = settings + .env + .iter() + .map(|(key, value)| (key.clone(), value.resolve_or_source(process_env_var))) + .collect(); + docker_config_from_environment_env(settings, skip_clone, env) +} + +#[cfg(feature = "docker")] +pub fn docker_config_from_environment_with_secrets( + settings: &RunEnvironmentSettings, + skip_clone: bool, + secrets_lookup: impl FnMut(&str) -> Option, +) -> Result { + let env = settings.resolve_env(process_env_var, secrets_lookup)?; + Ok(docker_config_from_environment_env( + settings, skip_clone, env, + )) +} + +#[cfg(feature = "docker")] +fn docker_config_from_environment_env( + settings: &RunEnvironmentSettings, + skip_clone: bool, + env: std::collections::HashMap, +) -> DockerSandboxOptions { + let mut env_vars = env .into_iter() .map(|(key, value)| format!("{key}={value}")) .collect::>(); diff --git a/lib/crates/fabro-types/src/settings/run.rs b/lib/crates/fabro-types/src/settings/run.rs index efad179c8..95b3a2282 100644 --- a/lib/crates/fabro-types/src/settings/run.rs +++ b/lib/crates/fabro-types/src/settings/run.rs @@ -16,7 +16,7 @@ use serde::ser::SerializeStruct; use serde::{Deserialize, Serialize}; use super::duration::Duration; -use super::interp::{InterpString, Namespace, ResolveError}; +use super::interp::{InterpString, Namespace, ResolveCtx, ResolveError}; use super::model_ref::ModelRef; use super::size::Size; @@ -733,35 +733,37 @@ impl Default for RunPrepareSettings { } impl RunPrepareSettings { - /// Resolve `{{ env.* }}` tokens in every prepare step's runnable part and - /// per-step `env` values against `env_lookup`, 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 + /// 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). /// /// 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.* }}` resolve here — in whichever - /// process actually runs the steps (the run worker for `fabro run`). + /// earlier, server-side, while `{{ env.* }}` and `{{ secrets.* }}` resolve + /// here — 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). /// - /// A referenced env var that is unset is a hard error — no fallback to the - /// unresolved source. Reserved `secrets`/`inputs` tokens have no lookup - /// here and surface as a loud + /// 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( &self, mut env_lookup: impl FnMut(&str) -> Option, + mut secrets_lookup: impl FnMut(&str) -> Option, ) -> Result { 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) + resolve_env_string(value, &mut env_lookup, &mut secrets_lookup) })?; } Ok(resolved) @@ -1059,17 +1061,37 @@ impl RunEnvironmentSettings { } } - /// Resolve every environment value's `{{ env.* }}` tokens via `lookup`, - /// falling back to the original source string when resolution fails. - #[must_use] - pub fn resolve_env(&self, mut lookup: F) -> HashMap - where - F: FnMut(&str) -> Option, - { - self.env - .iter() - .map(|(name, value)| (name.clone(), value.resolve_or_source(&mut lookup))) - .collect() + /// 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. + pub fn resolve_env( + &self, + mut env_lookup: impl FnMut(&str) -> Option, + mut secrets_lookup: impl FnMut(&str) -> Option, + ) -> Result, ResolveError> { + let mut ctx = ResolveCtx::new() + .with_env(&mut env_lookup) + .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.value, + 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); + } + Ok(resolved) } } @@ -1079,9 +1101,23 @@ impl Default for RunEnvironmentSettings { } } +/// Build a lookup closure over a fixed list of name/value pairs for the +/// run-boundary resolver tests. Shared by the env, secret, prepare-step, and +/// MCP transport test modules. +#[cfg(test)] +fn pair_lookup( + pairs: &'static [(&'static str, &'static str)], +) -> impl Fn(&str) -> Option + Copy { + move |name| { + pairs + .iter() + .find_map(|(key, value)| (*key == name).then(|| (*value).to_string())) + } +} + #[cfg(test)] mod run_environment_settings_tests { - use super::{HashMap, InterpString, RunEnvironmentSettings}; + use super::{HashMap, InterpString, RunEnvironmentSettings, pair_lookup as lookup}; fn settings(env: &[(&str, &str)]) -> RunEnvironmentSettings { RunEnvironmentSettings { @@ -1096,10 +1132,9 @@ 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(|name| match name { - "NODE_ENV" => Some("test".to_string()), - _ => None, - }); + let resolved = s + .resolve_env(lookup(&[("NODE_ENV", "test")]), lookup(&[])) + .unwrap(); assert_eq!(resolved.get("NODE_ENV"), Some(&"test".to_string())); assert_eq!(resolved.get("STATIC"), Some(&"value".to_string())); @@ -1108,7 +1143,7 @@ mod run_environment_settings_tests { #[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(|_| None); + let resolved = s.resolve_env(lookup(&[]), lookup(&[])).unwrap(); assert_eq!( resolved.get("NODE_ENV"), @@ -1116,9 +1151,49 @@ mod run_environment_settings_tests { ); } + #[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")])) + .unwrap(); + + assert_eq!( + resolved.get("API_TOKEN"), + Some(&"Bearer vault-token".to_string()) + ); + } + + #[test] + 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(); + + assert_eq!(err.namespace, super::Namespace::Secrets); + assert_eq!(err.name, "MISSING_TOKEN"); + } + + #[test] + fn resolve_env_does_not_source_fallback_mixed_values_that_reference_secrets() { + let s = settings(&[( + "API_TOKEN", + "{{ env.MISSING_PREFIX }} {{ secrets.API_TOKEN }}", + )]); + + let err = s + .resolve_env(lookup(&[]), lookup(&[("API_TOKEN", "vault-token")])) + .unwrap_err(); + + assert_eq!(err.namespace, super::Namespace::Env); + assert_eq!(err.name, "MISSING_PREFIX"); + } + #[test] fn resolve_env_is_empty_for_empty_settings() { - let s: HashMap = settings(&[]).resolve_env(|_| None); + let s: HashMap = + settings(&[]).resolve_env(lookup(&[]), lookup(&[])).unwrap(); assert!(s.is_empty()); } } @@ -1503,45 +1578,50 @@ impl McpServerSettings { StdDuration::from_secs(self.tool_timeout_secs) } - /// Resolve `{{ env.* }}` tokens in this server's transport strings - /// (`command`/`args`/`url`/`env`/`headers`) against `env_lookup`, - /// returning a copy with the tokens replaced and every other field - /// preserved. + /// 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. /// /// This is the late, use-time half of MCP interpolation, the counterpart /// to [`substitute_mcp_transport`]: `{{ vars.* }}` are substituted - /// earlier, server-side, while `{{ env.* }}` 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 `{{ 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). /// - /// A referenced env var that is unset is a hard error — no fallback to the - /// unresolved source. Reserved `secrets`/`inputs` tokens have no lookup - /// here and surface as a loud [`ResolveErrorKind::Unavailable`] error - /// rather than passing through as literal text. + /// 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( &self, mut env_lookup: impl FnMut(&str) -> Option, + mut secrets_lookup: impl FnMut(&str) -> Option, ) -> Result { let mut resolved = self.clone(); visit_mcp_transport_strings(&mut resolved.transport, &mut |value| { - resolve_env_string(value, &mut env_lookup) + resolve_env_string(value, &mut env_lookup, &mut secrets_lookup) })?; Ok(resolved) } } -/// Resolve `{{ env.* }}` tokens in one MCP transport string. A literal value -/// (no tokens) round-trips unchanged. +/// Resolve `{{ env.* }}` and `{{ secrets.* }}` tokens in one run-boundary +/// string. A literal value (no tokens) round-trips unchanged. fn resolve_env_string( value: &mut String, env_lookup: &mut impl FnMut(&str) -> Option, + secrets_lookup: &mut impl FnMut(&str) -> Option, ) -> Result<(), ResolveError> { if !value.contains("{{") { return Ok(()); } - *value = InterpString::parse(value).resolve(&mut *env_lookup)?.value; + let mut ctx = ResolveCtx::new() + .with_env(&mut *env_lookup) + .with_secrets(&mut *secrets_lookup); + *value = InterpString::parse(value).resolve_with(&mut ctx)?.value; Ok(()) } @@ -1550,17 +1630,10 @@ mod resolve_transport_env_tests { use std::collections::HashMap; use super::super::interp::ResolveErrorKind; - use super::{McpHttpProtocol, McpServerSettings, McpTransport, Namespace}; - - fn env_lookup( - pairs: &'static [(&'static str, &'static str)], - ) -> impl Fn(&str) -> Option + Copy { - move |name| { - pairs - .iter() - .find_map(|(key, value)| (*key == name).then(|| (*value).to_string())) - } - } + use super::{ + McpHttpProtocol, McpServerSettings, McpTransport, Namespace, pair_lookup as env_lookup, + pair_lookup as secret_lookup, + }; #[test] fn literal_transport_passes_through() { @@ -1573,7 +1646,9 @@ mod resolve_transport_env_tests { ..McpServerSettings::default() }; - let resolved = settings.resolve_transport_env(env_lookup(&[])).unwrap(); + let resolved = settings + .resolve_transport_env(env_lookup(&[]), secret_lookup(&[])) + .unwrap(); let McpTransport::Stdio { command, env } = resolved.transport else { panic!("expected stdio transport"); @@ -1597,10 +1672,13 @@ mod resolve_transport_env_tests { }; let resolved = settings - .resolve_transport_env(env_lookup(&[ - ("SERVER_PATH", "/srv/mcp.py"), - ("GEMINI_API_KEY", "real-key"), - ])) + .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 { @@ -1632,10 +1710,10 @@ mod resolve_transport_env_tests { }; let resolved = settings - .resolve_transport_env(env_lookup(&[ - ("MCP_HOST", "mcp.example"), - ("MCP_TOKEN", "abc123"), - ])) + .resolve_transport_env( + env_lookup(&[("MCP_HOST", "mcp.example"), ("MCP_TOKEN", "abc123")]), + secret_lookup(&[]), + ) .unwrap(); let McpTransport::Http { url, headers, .. } = resolved.transport else { @@ -1662,7 +1740,9 @@ mod resolve_transport_env_tests { ..McpServerSettings::default() }; - let err = settings.resolve_transport_env(env_lookup(&[])).unwrap_err(); + let err = settings + .resolve_transport_env(env_lookup(&[]), secret_lookup(&[])) + .unwrap_err(); assert_eq!(err.namespace, Namespace::Env); assert_eq!(err.name, "GEMINI_API_KEY"); @@ -1670,7 +1750,78 @@ mod resolve_transport_env_tests { } #[test] - fn reserved_secret_token_is_unavailable_not_leaked() { + fn stdio_command_and_env_resolve_secret_tokens() { + let settings = McpServerSettings { + name: "vaulted".to_string(), + transport: McpTransport::Stdio { + command: vec![ + "{{ secrets.SERVER_BIN }}".to_string(), + "--token".to_string(), + "{{ secrets.API_TOKEN }}".to_string(), + ], + env: HashMap::from([( + "API_TOKEN".to_string(), + "{{ secrets.API_TOKEN }}".to_string(), + )]), + }, + ..McpServerSettings::default() + }; + + let resolved = settings + .resolve_transport_env( + env_lookup(&[]), + secret_lookup(&[("SERVER_BIN", "/srv/mcp"), ("API_TOKEN", "vault-token")]), + ) + .unwrap(); + + let McpTransport::Stdio { command, env } = resolved.transport else { + panic!("expected stdio transport"); + }; + assert_eq!(command, vec![ + "/srv/mcp".to_string(), + "--token".to_string(), + "vault-token".to_string() + ]); + assert_eq!( + env.get("API_TOKEN").map(String::as_str), + Some("vault-token") + ); + } + + #[test] + fn http_url_and_headers_resolve_secret_tokens() { + let settings = McpServerSettings { + name: "remote".to_string(), + transport: McpTransport::Http { + protocol: McpHttpProtocol::default(), + url: "https://{{ secrets.MCP_HOST }}/mcp".to_string(), + headers: HashMap::from([( + "Authorization".to_string(), + "Bearer {{ secrets.MCP_TOKEN }}".to_string(), + )]), + }, + ..McpServerSettings::default() + }; + + let resolved = settings + .resolve_transport_env( + env_lookup(&[]), + secret_lookup(&[("MCP_HOST", "mcp.example"), ("MCP_TOKEN", "vault-token")]), + ) + .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 vault-token") + ); + } + + #[test] + fn missing_secret_token_is_secret_error() { let settings = McpServerSettings { name: "vaulted".to_string(), transport: McpTransport::Stdio { @@ -1683,10 +1834,13 @@ mod resolve_transport_env_tests { ..McpServerSettings::default() }; - let err = settings.resolve_transport_env(env_lookup(&[])).unwrap_err(); + let err = settings + .resolve_transport_env(env_lookup(&[]), secret_lookup(&[])) + .unwrap_err(); assert_eq!(err.namespace, Namespace::Secrets); - assert_eq!(err.kind, ResolveErrorKind::Unavailable); + assert_eq!(err.name, "API_KEY"); + assert_eq!(err.kind, ResolveErrorKind::Missing); } } @@ -1695,17 +1849,10 @@ mod resolve_step_env_tests { use std::collections::HashMap; use super::super::interp::ResolveErrorKind; - use super::{Namespace, PreparedStep, PreparedStepRun, RunPrepareSettings}; - - fn env_lookup( - pairs: &'static [(&'static str, &'static str)], - ) -> impl Fn(&str) -> Option + Copy { - move |name| { - pairs - .iter() - .find_map(|(key, value)| (*key == name).then(|| (*value).to_string())) - } - } + use super::{ + Namespace, PreparedStep, PreparedStepRun, RunPrepareSettings, pair_lookup as env_lookup, + pair_lookup as secret_lookup, + }; fn script_step(script: &str, env: HashMap) -> PreparedStep { PreparedStep { @@ -1735,7 +1882,9 @@ mod resolve_step_env_tests { timeout_ms: 1_000, }; - let resolved = settings.resolve_step_env(env_lookup(&[])).unwrap(); + let resolved = settings + .resolve_step_env(env_lookup(&[]), secret_lookup(&[])) + .unwrap(); assert_eq!(resolved.steps[0].to_shell_command(), "echo hello"); assert_eq!( @@ -1758,7 +1907,7 @@ mod resolve_step_env_tests { }; let resolved = settings - .resolve_step_env(env_lookup(&[("REGION", "us-east-1")])) + .resolve_step_env(env_lookup(&[("REGION", "us-east-1")]), secret_lookup(&[])) .unwrap(); assert_eq!( @@ -1778,10 +1927,10 @@ mod resolve_step_env_tests { }; let resolved = settings - .resolve_step_env(env_lookup(&[ - ("REGION", "us-east-1"), - ("DEPLOY_TOKEN", "secret-token"), - ])) + .resolve_step_env( + env_lookup(&[("REGION", "us-east-1"), ("DEPLOY_TOKEN", "secret-token")]), + secret_lookup(&[]), + ) .unwrap(); assert_eq!(resolved.steps[0].to_shell_command(), "deploy us-east-1"); @@ -1801,7 +1950,10 @@ mod resolve_step_env_tests { }; let resolved = settings - .resolve_step_env(env_lookup(&[("MESSAGE", "hello world")])) + .resolve_step_env( + env_lookup(&[("MESSAGE", "hello world")]), + secret_lookup(&[]), + ) .unwrap(); let shell = resolved.steps[0].to_shell_command(); @@ -1826,7 +1978,10 @@ mod resolve_step_env_tests { }; let resolved = settings - .resolve_step_env(|name| (name == "USER_INPUT").then(|| malicious.to_string())) + .resolve_step_env( + |name| (name == "USER_INPUT").then(|| malicious.to_string()), + secret_lookup(&[]), + ) .unwrap(); let shell = resolved.steps[0].to_shell_command(); @@ -1856,7 +2011,9 @@ mod resolve_step_env_tests { timeout_ms: 1_000, }; - let err = settings.resolve_step_env(env_lookup(&[])).unwrap_err(); + let err = settings + .resolve_step_env(env_lookup(&[]), secret_lookup(&[])) + .unwrap_err(); assert_eq!(err.namespace, Namespace::Env); assert_eq!(err.name, "REGION"); @@ -1873,7 +2030,9 @@ mod resolve_step_env_tests { timeout_ms: 1_000, }; - let err = settings.resolve_step_env(env_lookup(&[])).unwrap_err(); + let err = settings + .resolve_step_env(env_lookup(&[]), secret_lookup(&[])) + .unwrap_err(); assert_eq!(err.namespace, Namespace::Env); assert_eq!(err.name, "DEPLOY_TOKEN"); @@ -1881,7 +2040,45 @@ mod resolve_step_env_tests { } #[test] - fn reserved_secret_token_is_unavailable_not_leaked() { + fn script_command_and_env_resolve_secret_tokens() { + let settings = RunPrepareSettings { + steps: vec![ + script_step( + "deploy {{ secrets.REGION }} && echo done", + HashMap::from([( + "TOKEN".to_string(), + "{{ secrets.DEPLOY_TOKEN }}".to_string(), + )]), + ), + command_step(&["notify", "{{ secrets.MESSAGE }}"], HashMap::new()), + ], + timeout_ms: 1_000, + }; + + let resolved = settings + .resolve_step_env( + env_lookup(&[]), + secret_lookup(&[ + ("REGION", "us-east-1"), + ("DEPLOY_TOKEN", "vault-token"), + ("MESSAGE", "hello world"), + ]), + ) + .unwrap(); + + assert_eq!( + resolved.steps[0].to_shell_command(), + "deploy us-east-1 && echo done" + ); + assert_eq!( + resolved.steps[0].env.get("TOKEN").map(String::as_str), + Some("vault-token") + ); + assert_eq!(resolved.steps[1].to_shell_command(), "notify 'hello world'"); + } + + #[test] + fn missing_secret_token_is_secret_error() { let settings = RunPrepareSettings { steps: vec![script_step( "echo hi", @@ -1890,10 +2087,13 @@ mod resolve_step_env_tests { timeout_ms: 1_000, }; - let err = settings.resolve_step_env(env_lookup(&[])).unwrap_err(); + let err = settings + .resolve_step_env(env_lookup(&[]), secret_lookup(&[])) + .unwrap_err(); assert_eq!(err.namespace, Namespace::Secrets); - assert_eq!(err.kind, ResolveErrorKind::Unavailable); + assert_eq!(err.name, "API_KEY"); + assert_eq!(err.kind, ResolveErrorKind::Missing); } } diff --git a/lib/crates/fabro-workflow/Cargo.toml b/lib/crates/fabro-workflow/Cargo.toml index 0389c7e99..ca74a4878 100644 --- a/lib/crates/fabro-workflow/Cargo.toml +++ b/lib/crates/fabro-workflow/Cargo.toml @@ -85,3 +85,4 @@ httpmock = "0.8" fabro-macros = { path = "../fabro-macros" } fabro-test = { workspace = true } fabro-types = { path = "../fabro-types", features = ["test-support"] } +shlex = "1" diff --git a/lib/crates/fabro-workflow/src/operations/create.rs b/lib/crates/fabro-workflow/src/operations/create.rs index 02d870379..7d42f2c35 100644 --- a/lib/crates/fabro-workflow/src/operations/create.rs +++ b/lib/crates/fabro-workflow/src/operations/create.rs @@ -432,14 +432,14 @@ mod tests { use chrono::{Local, TimeZone, Utc}; use fabro_config::{ - ReplaceMap, RunExecutionLayer, RunGoalLayer, RunLayer, RunModelLayer, RunPullRequestLayer, - WorkflowSettingsBuilder, + PrepareStep, ReplaceMap, RunExecutionLayer, RunGoalLayer, RunLayer, RunModelLayer, + RunPrepareLayer, RunPullRequestLayer, WorkflowSettingsBuilder, }; use fabro_graphviz::graph::AttrValue; use fabro_store::Database; use fabro_types::settings::InterpString; use fabro_types::settings::run::RunMode; - use fabro_types::{WorkflowSettings, fixtures, test_support}; + use fabro_types::{EventBody, WorkflowSettings, fixtures, test_support}; use fabro_util::error::collect_chain; use fabro_validate::Severity; use object_store::local::LocalFileSystem; @@ -1389,6 +1389,91 @@ mod tests { assert!(created.run_dir.is_dir()); } + #[tokio::test] + async fn create_persists_secret_tokens_in_run_created_settings_source_form() { + let dir = tempfile::tempdir().unwrap(); + let storage_root = dir.path().join("storage"); + let store = memory_store(); + let created = create( + &store, + CreateRunInput { + workflow: WorkflowInput::DotSource { + source: MINIMAL_DOT.to_string(), + base_dir: None, + }, + settings: settings_from_run_layer(RunLayer { + prepare: Some(RunPrepareLayer { + steps: vec![PrepareStep { + script: None, + command: Some(vec![ + InterpString::parse("deploy"), + InterpString::parse("{{ secrets.DEPLOY_TOKEN }}"), + ]), + env: HashMap::from([( + "DEPLOY_TOKEN".to_string(), + InterpString::parse("{{ secrets.DEPLOY_TOKEN }}"), + )]), + }], + timeout: None, + }), + execution: Some(RunExecutionLayer { + mode: Some(RunMode::DryRun), + ..RunExecutionLayer::default() + }), + ..RunLayer::default() + }), + vars: HashMap::new(), + cwd: dir.path().to_path_buf(), + workflow_slug: Some("secret-source".to_string()), + workflow_path: None, + workflow_bundle: None, + submitted_manifest_bytes: None, + run_id: Some(fixtures::RUN_1), + title: None, + automation: None, + git: None, + fork_source_ref: None, + parent_id: None, + provenance: test_support::test_run_provenance(), + configured_providers: Vec::new(), + web_url: None, + }, + storage_root, + test_catalog(), + ) + .await + .unwrap(); + + let run_store = store.open_run(&created.run_id).await.unwrap(); + let events = run_store.list_events().await.unwrap(); + let run_created = events + .iter() + .find_map(|event| match &event.event.body { + EventBody::RunCreated(props) => Some(props), + _ => None, + }) + .expect("run.created event should be persisted"); + let step = run_created + .settings + .run + .prepare + .steps + .first() + .expect("prepare step should be persisted"); + + let fabro_types::settings::run::PreparedStepRun::Command { command } = &step.run else { + panic!("expected command prepare step"); + }; + assert_eq!(command, &vec![ + "deploy".to_string(), + "{{ secrets.DEPLOY_TOKEN }}".to_string() + ]); + assert_eq!( + step.env.get("DEPLOY_TOKEN").map(String::as_str), + Some("{{ secrets.DEPLOY_TOKEN }}") + ); + } + #[tokio::test] async fn create_persists_submitter_source_directory_from_request_cwd() { let dir = tempfile::tempdir().unwrap(); diff --git a/lib/crates/fabro-workflow/src/operations/start.rs b/lib/crates/fabro-workflow/src/operations/start.rs index f24ec8d43..7fa47e938 100644 --- a/lib/crates/fabro-workflow/src/operations/start.rs +++ b/lib/crates/fabro-workflow/src/operations/start.rs @@ -10,7 +10,7 @@ use fabro_mcp::config::McpServerSettings; use fabro_model::{Catalog, FallbackTarget, ProviderId}; use fabro_sandbox::daytona::DaytonaConfig; use fabro_sandbox::from_environment::{ - daytona_config_from_environment, docker_config_from_environment, + daytona_config_from_environment, docker_config_from_environment_with_secrets, local_working_directory_from_environment, }; use fabro_sandbox::{DockerSandboxOptions, SandboxSpec}; @@ -373,12 +373,22 @@ impl RunSession { let configured = configured_providers_for_start(services.vault.as_ref(), Arc::clone(&catalog)).await; 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, + }; + // 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 mcp_servers = resolved .agent .mcps .iter() .map(|(key, entry)| match entry { - ResolvedMcpEntry::Resolved(server) => runtime_mcp_server(server, process_env_var), + ResolvedMcpEntry::Resolved(server) => { + runtime_mcp_server(server, process_env_var, 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 @@ -409,19 +419,15 @@ impl RunSession { SandboxSpec::Local { working_directory } } SandboxProviderKind::Docker => SandboxSpec::Docker { - config: resolve_docker_config(resolved), + config: resolve_docker_config(resolved, secret_lookup)?, github_app: services.github_app.clone(), run_id: Some(record.run_id), clone_origin_url: record.repo_origin_url().map(str::to_string), clone_branch: record.base_branch().map(str::to_string), }, SandboxProviderKind::Daytona => { - let api_key = match &services.vault { - Some(v) => v - .read() - .await - .get(EnvVars::DAYTONA_API_KEY) - .map(str::to_string), + let api_key = match vault_guard.as_deref() { + Some(vault) => vault.get(EnvVars::DAYTONA_API_KEY).map(str::to_string), None => None, }; SandboxSpec::Daytona { @@ -435,7 +441,10 @@ impl RunSession { } }; - let toml_env = resolved.environment.resolve_env(process_env_var); + let toml_env = resolved + .environment + .resolve_env(process_env_var, secret_lookup) + .map_err(|err| Error::engine_with_source("failed to resolve run environment", err))?; let github_permissions: Option> = (!services.github_permissions.is_empty()).then(|| services.github_permissions.clone()); let sandbox_env = SandboxEnvSpec { @@ -452,6 +461,9 @@ impl RunSession { }; let pr_config = resolved.pull_request.clone(); + let setup_commands = + runtime_setup_commands(&resolved.prepare, process_env_var, secret_lookup)?; + drop(vault_guard); Ok(Self { cancel_token: services.cancel_token, @@ -471,7 +483,7 @@ impl RunSession { steering_hub: services.steering_hub, on_node: services.on_node, lifecycle: LifecycleOptions { - setup_commands: runtime_setup_commands(&resolved.prepare)?, + setup_commands, setup_command_timeout_ms: resolved.prepare.timeout_ms, }, hooks: fabro_hooks::HookSettings { @@ -550,6 +562,10 @@ fn process_env_var(name: &str) -> Option { std::env::var(name).ok() } +fn vault_token_lookup(vault: Option<&Vault>, name: &str) -> Option { + vault.and_then(|vault| fabro_auth::vault_get_token(vault, name).ok().flatten()) +} + async fn load_accepted_run_definition( run_store: &RunStoreHandle, blob_id: fabro_types::RunBlobId, @@ -574,8 +590,16 @@ fn resolve_daytona_config(settings: &ResolvedRunSettings) -> DaytonaConfig { daytona_config_from_environment(&settings.environment, !settings.clone.enabled) } -fn resolve_docker_config(settings: &ResolvedRunSettings) -> DockerSandboxOptions { - docker_config_from_environment(&settings.environment, !settings.clone.enabled) +fn resolve_docker_config( + settings: &ResolvedRunSettings, + secrets_lookup: impl FnMut(&str) -> Option, +) -> Result { + docker_config_from_environment_with_secrets( + &settings.environment, + !settings.clone.enabled, + secrets_lookup, + ) + .map_err(|err| Error::engine_with_source("failed to resolve Docker environment config", err)) } fn resolve_start_llm( @@ -680,46 +704,51 @@ impl ModelRegistry for CatalogModelRegistry<'_> { } /// Build the launch-time MCP config from resolved settings, resolving any -/// `{{ env.* }}` tokens in the transport (`command`/`url`/`env`/`headers`) -/// against the worker process environment — the run boundary where the MCP is -/// actually launched. +/// `{{ 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. /// /// The resolution itself lives on the type /// ([`McpServerSettings::resolve_transport_env`]) 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 that is unset is a hard error — -/// no fallback to the unresolved source. +/// env to be set), and a referenced env var or secret that is unset is a hard +/// error — no fallback to the unresolved source. fn runtime_mcp_server( settings: &ResolvedMcpServerSettings, env_lookup: impl FnMut(&str) -> Option, + secrets_lookup: impl FnMut(&str) -> Option, ) -> Result { - settings.resolve_transport_env(env_lookup).map_err(|err| { - Error::engine_with_source( - format!("failed to resolve MCP server {:?}", settings.name), - err, - ) - }) + settings + .resolve_transport_env(env_lookup, secrets_lookup) + .map_err(|err| { + Error::engine_with_source( + format!("failed to resolve MCP server {:?}", settings.name), + err, + ) + }) } /// Build the launch-time setup (prepare) commands from resolved settings, -/// resolving any `{{ env.* }}` tokens in each step's command and per-step env -/// against the worker process environment — the run boundary where the steps -/// actually run. +/// 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. /// /// The resolution itself lives on the type /// ([`ResolvedRunPrepareSettings::resolve_step_env`]) so prepare-step env /// 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 that is unset is a hard -/// error — no fallback to the unresolved source. +/// 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. fn runtime_setup_commands( prepare: &ResolvedRunPrepareSettings, + env_lookup: impl FnMut(&str) -> Option, + secrets_lookup: impl FnMut(&str) -> Option, ) -> Result, Error> { let resolved = prepare - .resolve_step_env(process_env_var) + .resolve_step_env(env_lookup, secrets_lookup) .map_err(|err| Error::engine_with_source("failed to resolve prepare step", err))?; Ok(resolved .steps @@ -1095,11 +1124,15 @@ mod tests { RunEnvironmentLayer, RunExecutionLayer, RunLayer, StickyMap, WorkflowSettingsBuilder, }; use fabro_store::Database; - use fabro_types::settings::run::{McpTransport as ResolvedMcpTransport, RunMode}; + use fabro_types::settings::run::{ + McpTransport as ResolvedMcpTransport, PreparedStep, PreparedStepRun, RunMode, + RunPrepareSettings, + }; use fabro_types::settings::{InterpString, ModelRef}; use fabro_types::{ BilledModelUsage, ManifestPath, StageTiming, WorkflowSettings, fixtures, test_support, }; + use fabro_vault::SecretType; use object_store::memory::InMemory; use super::*; @@ -1284,7 +1317,11 @@ reasoning = false ..RunLayer::default() }); - assert!(resolve_docker_config(&settings.run).skip_clone); + assert!( + resolve_docker_config(&settings.run, |_| None) + .unwrap() + .skip_clone + ); assert!(resolve_daytona_config(&settings.run).skip_clone); } @@ -1302,7 +1339,7 @@ reasoning = false ..ResolvedMcpServerSettings::default() }; - let err = runtime_mcp_server(&settings, |_| None).unwrap_err(); + let err = runtime_mcp_server(&settings, |_| None, |_| None).unwrap_err(); assert_eq!( err.to_string(), @@ -1313,6 +1350,261 @@ reasoning = false assert!(causes[0].contains("GEMINI_API_KEY")); } + #[test] + fn runtime_setup_command_env_resolves_secret_from_vault() { + let vault = token_vault("DEPLOY_TOKEN", "vault-token"); + let prepare = prepare_with_step(script_step( + "echo ready", + HashMap::from([( + "DEPLOY_TOKEN".to_string(), + "{{ secrets.DEPLOY_TOKEN }}".to_string(), + )]), + )); + + let commands = + runtime_setup_commands(&prepare, |_| None, vault_secret_lookup(&vault)).unwrap(); + + assert_eq!(commands.len(), 1); + assert_eq!( + commands[0].env.get("DEPLOY_TOKEN").map(String::as_str), + Some("vault-token") + ); + } + + #[test] + fn runtime_setup_command_secret_argv_is_resolved_before_shell_quoting() { + let malicious = "x'; touch PWNED; echo '"; + let vault = token_vault("USER_INPUT", malicious); + let prepare = prepare_with_step(command_step( + &["echo", "{{ secrets.USER_INPUT }}"], + HashMap::new(), + )); + + let commands = + runtime_setup_commands(&prepare, |_| None, vault_secret_lookup(&vault)).unwrap(); + let tokens = + shlex::split(&commands[0].command).expect("resolved command should remain valid shell"); + + assert_eq!(tokens, vec!["echo".to_string(), malicious.to_string()]); + assert_eq!( + tokens.len(), + 2, + "injected shell syntax leaked extra tokens: {}", + commands[0].command + ); + } + + #[test] + fn runtime_mcp_server_env_resolves_secret_from_vault() { + let vault = token_vault("MCP_TOKEN", "vault-token"); + let settings = ResolvedMcpServerSettings { + name: "vaulted".to_string(), + transport: ResolvedMcpTransport::Stdio { + command: vec!["mcp-server".to_string()], + env: HashMap::from([( + "MCP_TOKEN".to_string(), + "{{ secrets.MCP_TOKEN }}".to_string(), + )]), + }, + ..ResolvedMcpServerSettings::default() + }; + + let resolved = + runtime_mcp_server(&settings, |_| None, vault_secret_lookup(&vault)).unwrap(); + + let ResolvedMcpTransport::Stdio { env, .. } = resolved.transport else { + panic!("expected stdio transport"); + }; + assert_eq!( + env.get("MCP_TOKEN").map(String::as_str), + Some("vault-token") + ); + } + + #[test] + fn runtime_setup_command_missing_secret_fails_closed() { + let vault = temp_vault(&[]); + let prepare = prepare_with_step(command_step( + &["deploy", "{{ secrets.DEPLOY_TOKEN }}"], + HashMap::new(), + )); + + let Err(err) = runtime_setup_commands(&prepare, |_| None, vault_secret_lookup(&vault)) + else { + panic!("missing secret should fail setup command resolution"); + }; + + assert_eq!( + err.to_string(), + "Engine error: failed to resolve prepare step" + ); + let causes = err.causes(); + assert_eq!(causes.len(), 1); + assert!(causes[0].contains("DEPLOY_TOKEN")); + } + + #[test] + fn runtime_setup_command_oauth_secret_fails_closed() { + let vault = temp_vault(&[("DEPLOY_TOKEN", "{}", SecretType::Oauth)]); + let prepare = prepare_with_step(script_step( + "echo ready", + HashMap::from([( + "DEPLOY_TOKEN".to_string(), + "{{ secrets.DEPLOY_TOKEN }}".to_string(), + )]), + )); + + let Err(err) = runtime_setup_commands(&prepare, |_| None, vault_secret_lookup(&vault)) + else { + panic!("OAuth secret should fail setup command resolution"); + }; + + assert_eq!( + err.to_string(), + "Engine error: failed to resolve prepare step" + ); + assert!(err.causes()[0].contains("DEPLOY_TOKEN")); + } + + #[test] + fn runtime_setup_command_file_secret_fails_closed() { + let vault = temp_vault(&[(EnvVars::GITHUB_APP_PRIVATE_KEY, "pem", SecretType::File)]); + let prepare = prepare_with_step(script_step( + "echo ready", + HashMap::from([( + "GITHUB_APP_PRIVATE_KEY".to_string(), + "{{ secrets.GITHUB_APP_PRIVATE_KEY }}".to_string(), + )]), + )); + + let Err(err) = runtime_setup_commands(&prepare, |_| None, vault_secret_lookup(&vault)) + else { + panic!("file secret should fail setup command resolution"); + }; + + assert_eq!( + err.to_string(), + "Engine error: failed to resolve prepare step" + ); + assert!(err.causes()[0].contains("GITHUB_APP_PRIVATE_KEY")); + } + + #[tokio::test] + async fn run_session_new_resolves_secret_tokens_from_vault_at_boundary() { + let temp = tempfile::tempdir().unwrap(); + let (storage_root, _run_dir) = storage_root_and_run_dir(&temp); + let mut settings = settings_from_run_layer(RunLayer { + execution: Some(RunExecutionLayer { + mode: Some(RunMode::DryRun), + ..RunExecutionLayer::default() + }), + ..RunLayer::default() + }); + settings.run.environment.env.insert( + "API_TOKEN".to_string(), + InterpString::parse("{{ secrets.DEPLOY_TOKEN }}"), + ); + settings.run.prepare = prepare_with_step(command_step( + &["deploy", "{{ secrets.DEPLOY_TOKEN }}"], + HashMap::from([( + "DEPLOY_TOKEN".to_string(), + "{{ secrets.DEPLOY_TOKEN }}".to_string(), + )]), + )); + settings.run.agent.mcps.insert( + "vaulted".to_string(), + ResolvedMcpEntry::Resolved(ResolvedMcpServerSettings { + name: "vaulted".to_string(), + transport: ResolvedMcpTransport::Stdio { + command: vec!["mcp-server".to_string()], + env: HashMap::from([( + "MCP_TOKEN".to_string(), + "{{ secrets.DEPLOY_TOKEN }}".to_string(), + )]), + }, + ..ResolvedMcpServerSettings::default() + }), + ); + let (persisted, store) = + persisted_workflow_with_settings(MINIMAL_DOT, &storage_root, settings).await; + let emitter = Arc::new(Emitter::new(fixtures::RUN_1)); + let registry = Arc::new(test_registry()); + let vault = Arc::new(AsyncRwLock::new(token_vault("DEPLOY_TOKEN", "vault-token"))); + + let session = RunSession::new(&persisted, StartServices { + vault: Some(vault), + ..test_start_services(&store, &storage_root, emitter, registry).await + }) + .await + .unwrap(); + + assert_eq!( + session + .sandbox_env + .toml_env + .get("API_TOKEN") + .map(String::as_str), + Some("vault-token") + ); + assert_eq!( + session.lifecycle.setup_commands[0] + .env + .get("DEPLOY_TOKEN") + .map(String::as_str), + Some("vault-token") + ); + let setup_command = &session.lifecycle.setup_commands[0].command; + assert!(!setup_command.contains("{{ secrets.DEPLOY_TOKEN }}")); + assert_eq!( + shlex::split(setup_command).expect("setup command should be valid shell"), + vec!["deploy".to_string(), "vault-token".to_string()] + ); + let ResolvedMcpTransport::Stdio { env, .. } = &session.llm.mcp_servers[0].transport else { + panic!("expected stdio MCP transport"); + }; + assert_eq!( + env.get("MCP_TOKEN").map(String::as_str), + Some("vault-token") + ); + } + + #[tokio::test] + async fn run_session_new_missing_secret_fails_startup() { + let temp = tempfile::tempdir().unwrap(); + let (storage_root, _run_dir) = storage_root_and_run_dir(&temp); + let mut settings = settings_from_run_layer(RunLayer { + execution: Some(RunExecutionLayer { + mode: Some(RunMode::DryRun), + ..RunExecutionLayer::default() + }), + ..RunLayer::default() + }); + settings.run.prepare = prepare_with_step(command_step( + &["deploy", "{{ secrets.DEPLOY_TOKEN }}"], + HashMap::new(), + )); + let (persisted, store) = + persisted_workflow_with_settings(MINIMAL_DOT, &storage_root, settings).await; + let emitter = Arc::new(Emitter::new(fixtures::RUN_1)); + let registry = Arc::new(test_registry()); + let vault = Arc::new(AsyncRwLock::new(temp_vault(&[]))); + + let Err(err) = RunSession::new(&persisted, StartServices { + vault: Some(vault), + ..test_start_services(&store, &storage_root, emitter, registry).await + }) + .await + else { + panic!("missing secret should fail run startup"); + }; + + assert_eq!( + err.to_string(), + "Engine error: failed to resolve prepare step" + ); + assert!(err.causes()[0].contains("DEPLOY_TOKEN")); + } + #[test] fn runtime_docker_config_maps_environment_hints() { let settings = settings_from_run_layer(RunLayer { @@ -1339,7 +1631,7 @@ reasoning = false ..RunLayer::default() }); - let config = resolve_docker_config(&settings.run); + let config = resolve_docker_config(&settings.run, |_| None).unwrap(); assert_eq!(config.image, "ubuntu:24.04"); assert_eq!(config.cpu_quota, Some(400_000)); @@ -1381,7 +1673,11 @@ reasoning = false assert_eq!(git.meta_branch, None); } - async fn persisted_workflow(dot: &str, storage_root: &Path) -> (Persisted, Arc) { + async fn persisted_workflow_with_settings( + dot: &str, + storage_root: &Path, + settings: WorkflowSettings, + ) -> (Persisted, Arc) { let store = memory_store(); let created = crate::operations::create( &store, @@ -1390,13 +1686,7 @@ reasoning = false source: dot.to_string(), base_dir: None, }, - settings: settings_from_run_layer(RunLayer { - execution: Some(RunExecutionLayer { - mode: Some(RunMode::DryRun), - ..RunExecutionLayer::default() - }), - ..RunLayer::default() - }), + settings, vars: std::collections::HashMap::new(), cwd: storage_root .parent() @@ -1424,6 +1714,21 @@ reasoning = false (created.persisted, store) } + async fn persisted_workflow(dot: &str, storage_root: &Path) -> (Persisted, Arc) { + persisted_workflow_with_settings( + dot, + storage_root, + settings_from_run_layer(RunLayer { + execution: Some(RunExecutionLayer { + mode: Some(RunMode::DryRun), + ..RunExecutionLayer::default() + }), + ..RunLayer::default() + }), + ) + .await + } + fn test_registry() -> HandlerRegistry { let mut registry = HandlerRegistry::new(Box::new(StartHandler)); registry.register("start", Box::new(StartHandler)); @@ -1459,6 +1764,48 @@ reasoning = false } } + fn temp_vault(entries: &[(&str, &str, SecretType)]) -> Vault { + let dir = tempfile::tempdir().unwrap(); + let mut vault = Vault::load(dir.path().join("secrets.json")).unwrap(); + for (name, value, secret_type) in entries { + vault.set(name, value, *secret_type, None).unwrap(); + } + vault + } + + fn token_vault(name: &str, value: &str) -> Vault { + temp_vault(&[(name, value, SecretType::Token)]) + } + + fn vault_secret_lookup(vault: &Vault) -> impl FnMut(&str) -> Option + '_ { + move |name| vault_token_lookup(Some(vault), name) + } + + fn prepare_with_step(step: PreparedStep) -> RunPrepareSettings { + RunPrepareSettings { + steps: vec![step], + timeout_ms: 1_000, + } + } + + fn script_step(script: &str, env: HashMap) -> PreparedStep { + PreparedStep { + run: PreparedStepRun::Script { + script: script.to_string(), + }, + env, + } + } + + fn command_step(command: &[&str], env: HashMap) -> PreparedStep { + PreparedStep { + run: PreparedStepRun::Command { + command: command.iter().map(|value| (*value).to_string()).collect(), + }, + env, + } + } + use crate::test_support::{mark_run_running, test_usage}; async fn append_completed_stage( From 8c7d5dc7d0375fb8e7d1d3f47e7e6639979d8517 Mon Sep 17 00:00:00 2001 From: "fabro-releases[bot]" Date: Fri, 3 Jul 2026 10:17:50 +0000 Subject: [PATCH 9/9] Bump version to 0.283.0-nightly.0 --- Cargo.lock | 102 ++++++++++++++++++++++++++--------------------------- Cargo.toml | 2 +- 2 files changed, 52 insertions(+), 52 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 789505b27..30f0bd0e2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2239,7 +2239,7 @@ dependencies = [ [[package]] name = "fabro-acp" -version = "0.282.0-nightly.0" +version = "0.283.0-nightly.0" dependencies = [ "agent-client-protocol", "agent-client-protocol-tokio", @@ -2258,7 +2258,7 @@ dependencies = [ [[package]] name = "fabro-agent" -version = "0.282.0-nightly.0" +version = "0.283.0-nightly.0" dependencies = [ "anyhow", "async-trait", @@ -2300,7 +2300,7 @@ dependencies = [ [[package]] name = "fabro-api" -version = "0.282.0-nightly.0" +version = "0.283.0-nightly.0" dependencies = [ "chrono", "fabro-automation", @@ -2323,7 +2323,7 @@ dependencies = [ [[package]] name = "fabro-auth" -version = "0.282.0-nightly.0" +version = "0.283.0-nightly.0" dependencies = [ "anyhow", "async-trait", @@ -2347,7 +2347,7 @@ dependencies = [ [[package]] name = "fabro-automation" -version = "0.282.0-nightly.0" +version = "0.283.0-nightly.0" dependencies = [ "croner", "hex", @@ -2362,11 +2362,11 @@ dependencies = [ [[package]] name = "fabro-build-support" -version = "0.282.0-nightly.0" +version = "0.283.0-nightly.0" [[package]] name = "fabro-checkpoint" -version = "0.282.0-nightly.0" +version = "0.283.0-nightly.0" dependencies = [ "chrono", "fabro-config", @@ -2382,7 +2382,7 @@ dependencies = [ [[package]] name = "fabro-cli" -version = "0.282.0-nightly.0" +version = "0.283.0-nightly.0" dependencies = [ "anyhow", "assert_cmd", @@ -2484,7 +2484,7 @@ dependencies = [ [[package]] name = "fabro-client" -version = "0.282.0-nightly.0" +version = "0.283.0-nightly.0" dependencies = [ "anyhow", "bytes", @@ -2513,7 +2513,7 @@ dependencies = [ [[package]] name = "fabro-config" -version = "0.282.0-nightly.0" +version = "0.283.0-nightly.0" dependencies = [ "anyhow", "chrono", @@ -2542,7 +2542,7 @@ dependencies = [ [[package]] name = "fabro-core" -version = "0.282.0-nightly.0" +version = "0.283.0-nightly.0" dependencies = [ "async-trait", "fabro-types", @@ -2557,7 +2557,7 @@ dependencies = [ [[package]] name = "fabro-db" -version = "0.282.0-nightly.0" +version = "0.283.0-nightly.0" dependencies = [ "anyhow", "sqlx", @@ -2567,7 +2567,7 @@ dependencies = [ [[package]] name = "fabro-dev" -version = "0.282.0-nightly.0" +version = "0.283.0-nightly.0" dependencies = [ "anyhow", "assert_cmd", @@ -2586,7 +2586,7 @@ dependencies = [ [[package]] name = "fabro-dump" -version = "0.282.0-nightly.0" +version = "0.283.0-nightly.0" dependencies = [ "anyhow", "bytes", @@ -2600,7 +2600,7 @@ dependencies = [ [[package]] name = "fabro-environment" -version = "0.282.0-nightly.0" +version = "0.283.0-nightly.0" dependencies = [ "anyhow", "chrono", @@ -2622,7 +2622,7 @@ dependencies = [ [[package]] name = "fabro-github" -version = "0.282.0-nightly.0" +version = "0.283.0-nightly.0" dependencies = [ "anyhow", "base64", @@ -2644,7 +2644,7 @@ dependencies = [ [[package]] name = "fabro-graphviz" -version = "0.282.0-nightly.0" +version = "0.283.0-nightly.0" dependencies = [ "anyhow", "fabro-types", @@ -2658,7 +2658,7 @@ dependencies = [ [[package]] name = "fabro-hooks" -version = "0.282.0-nightly.0" +version = "0.283.0-nightly.0" dependencies = [ "async-trait", "fabro-agent", @@ -2681,7 +2681,7 @@ dependencies = [ [[package]] name = "fabro-http" -version = "0.282.0-nightly.0" +version = "0.283.0-nightly.0" dependencies = [ "fabro-static", "http 1.4.0", @@ -2691,7 +2691,7 @@ dependencies = [ [[package]] name = "fabro-install" -version = "0.282.0-nightly.0" +version = "0.283.0-nightly.0" dependencies = [ "anyhow", "base64", @@ -2709,7 +2709,7 @@ dependencies = [ [[package]] name = "fabro-interview" -version = "0.282.0-nightly.0" +version = "0.283.0-nightly.0" dependencies = [ "async-trait", "dialoguer", @@ -2724,7 +2724,7 @@ dependencies = [ [[package]] name = "fabro-llm" -version = "0.282.0-nightly.0" +version = "0.283.0-nightly.0" dependencies = [ "anyhow", "async-trait", @@ -2765,7 +2765,7 @@ dependencies = [ [[package]] name = "fabro-macros" -version = "0.282.0-nightly.0" +version = "0.283.0-nightly.0" dependencies = [ "clap", "fabro-options-metadata", @@ -2776,7 +2776,7 @@ dependencies = [ [[package]] name = "fabro-manifest" -version = "0.282.0-nightly.0" +version = "0.283.0-nightly.0" dependencies = [ "anyhow", "fabro-api", @@ -2794,7 +2794,7 @@ dependencies = [ [[package]] name = "fabro-mcp" -version = "0.282.0-nightly.0" +version = "0.283.0-nightly.0" dependencies = [ "anyhow", "axum", @@ -2814,7 +2814,7 @@ dependencies = [ [[package]] name = "fabro-mcp-server" -version = "0.282.0-nightly.0" +version = "0.283.0-nightly.0" dependencies = [ "anyhow", "chrono", @@ -2841,7 +2841,7 @@ dependencies = [ [[package]] name = "fabro-mcp-store" -version = "0.282.0-nightly.0" +version = "0.283.0-nightly.0" dependencies = [ "fabro-types", "serde", @@ -2853,7 +2853,7 @@ dependencies = [ [[package]] name = "fabro-model" -version = "0.282.0-nightly.0" +version = "0.283.0-nightly.0" dependencies = [ "fabro-static", "http 1.4.0", @@ -2869,7 +2869,7 @@ dependencies = [ [[package]] name = "fabro-oauth" -version = "0.282.0-nightly.0" +version = "0.283.0-nightly.0" dependencies = [ "anyhow", "axum", @@ -2891,7 +2891,7 @@ dependencies = [ [[package]] name = "fabro-options-metadata" -version = "0.282.0-nightly.0" +version = "0.283.0-nightly.0" dependencies = [ "serde", "serde_json", @@ -2899,7 +2899,7 @@ dependencies = [ [[package]] name = "fabro-proc" -version = "0.282.0-nightly.0" +version = "0.283.0-nightly.0" dependencies = [ "cc", "libc", @@ -2908,7 +2908,7 @@ dependencies = [ [[package]] name = "fabro-redact" -version = "0.282.0-nightly.0" +version = "0.283.0-nightly.0" dependencies = [ "aho-corasick", "ref-cast", @@ -2924,7 +2924,7 @@ dependencies = [ [[package]] name = "fabro-sandbox" -version = "0.282.0-nightly.0" +version = "0.283.0-nightly.0" dependencies = [ "anyhow", "async-trait", @@ -2969,7 +2969,7 @@ dependencies = [ [[package]] name = "fabro-server" -version = "0.282.0-nightly.0" +version = "0.283.0-nightly.0" dependencies = [ "anyhow", "async-trait", @@ -3060,7 +3060,7 @@ dependencies = [ [[package]] name = "fabro-slack" -version = "0.282.0-nightly.0" +version = "0.283.0-nightly.0" dependencies = [ "fabro-http", "fabro-interview", @@ -3082,18 +3082,18 @@ dependencies = [ [[package]] name = "fabro-spa" -version = "0.282.0-nightly.0" +version = "0.283.0-nightly.0" dependencies = [ "rust-embed", ] [[package]] name = "fabro-static" -version = "0.282.0-nightly.0" +version = "0.283.0-nightly.0" [[package]] name = "fabro-store" -version = "0.282.0-nightly.0" +version = "0.283.0-nightly.0" dependencies = [ "async-trait", "bytes", @@ -3120,7 +3120,7 @@ dependencies = [ [[package]] name = "fabro-telemetry" -version = "0.282.0-nightly.0" +version = "0.283.0-nightly.0" dependencies = [ "anyhow", "base64", @@ -3146,7 +3146,7 @@ dependencies = [ [[package]] name = "fabro-template" -version = "0.282.0-nightly.0" +version = "0.283.0-nightly.0" dependencies = [ "anyhow", "fabro-types", @@ -3160,7 +3160,7 @@ dependencies = [ [[package]] name = "fabro-test" -version = "0.282.0-nightly.0" +version = "0.283.0-nightly.0" dependencies = [ "anyhow", "assert_cmd", @@ -3185,7 +3185,7 @@ dependencies = [ [[package]] name = "fabro-tool" -version = "0.282.0-nightly.0" +version = "0.283.0-nightly.0" dependencies = [ "anyhow", "async-trait", @@ -3206,7 +3206,7 @@ dependencies = [ [[package]] name = "fabro-tracker" -version = "0.282.0-nightly.0" +version = "0.283.0-nightly.0" dependencies = [ "anyhow", "async-trait", @@ -3220,7 +3220,7 @@ dependencies = [ [[package]] name = "fabro-types" -version = "0.282.0-nightly.0" +version = "0.283.0-nightly.0" dependencies = [ "chrono", "clap", @@ -3242,7 +3242,7 @@ dependencies = [ [[package]] name = "fabro-util" -version = "0.282.0-nightly.0" +version = "0.283.0-nightly.0" dependencies = [ "anyhow", "console 0.15.11", @@ -3263,7 +3263,7 @@ dependencies = [ [[package]] name = "fabro-validate" -version = "0.282.0-nightly.0" +version = "0.283.0-nightly.0" dependencies = [ "fabro-acp", "fabro-graphviz", @@ -3276,7 +3276,7 @@ dependencies = [ [[package]] name = "fabro-variable" -version = "0.282.0-nightly.0" +version = "0.283.0-nightly.0" dependencies = [ "anyhow", "chrono", @@ -3293,7 +3293,7 @@ dependencies = [ [[package]] name = "fabro-vault" -version = "0.282.0-nightly.0" +version = "0.283.0-nightly.0" dependencies = [ "chrono", "fabro-static", @@ -3306,7 +3306,7 @@ dependencies = [ [[package]] name = "fabro-workflow" -version = "0.282.0-nightly.0" +version = "0.283.0-nightly.0" dependencies = [ "anyhow", "assert_cmd", @@ -8469,7 +8469,7 @@ dependencies = [ [[package]] name = "twin-github" -version = "0.282.0-nightly.0" +version = "0.283.0-nightly.0" dependencies = [ "axum", "base64", @@ -8488,7 +8488,7 @@ dependencies = [ [[package]] name = "twin-openai" -version = "0.282.0-nightly.0" +version = "0.283.0-nightly.0" dependencies = [ "anyhow", "async-stream", diff --git a/Cargo.toml b/Cargo.toml index 8980f0239..415dc26b3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -5,7 +5,7 @@ resolver = "2" [workspace.package] edition = "2021" -version = "0.282.0-nightly.0" +version = "0.283.0-nightly.0" license = "MIT" [workspace.dependencies]