Merge remote-tracking branch 'origin/main'

# Conflicts:
#	lib/crates/fabro-cli/src/commands/dump.rs
#	lib/crates/fabro-cli/src/commands/store/mod.rs
#	lib/crates/fabro-cli/src/main.rs
This commit is contained in:
Bryan Helmkamp 2026-04-23 08:42:16 -04:00
commit 4ad4d8fd36
No known key found for this signature in database
66 changed files with 2088 additions and 540 deletions

View file

@ -0,0 +1,39 @@
---
status: ready
priority: p1
issue_id: "001"
tags: [rust, clippy, async-io, std-fs]
dependencies: []
---
## Problem Statement
Several Rust crates still contain `FOLLOW-UP:` markers related to blocking `std::fs` or sync I/O on async paths. The requested work is to execute the implementation plan in `~/.claude/plans/we-ll-feal-with-std-fs-jaunty-feigenbaum.md` and finish the refactors or tighten the remaining sync justifications.
## Findings
- The repo is currently on `main`, and the user explicitly approved proceeding there.
- `docs/solutions/` is not present, so there are no repo learnings to consult for this task.
- The current code matches the plan buckets across `fabro-agent`, `fabro-devcontainer`, `fabro-llm`, and `fabro-workflow`.
## Proposed Solutions
- Execute the plan in bucket order, using targeted failing checks before each production change where feasible.
- Prefer async propagation for truly async paths and `spawn_blocking` only at natural async boundaries.
- Remove or narrow `#[expect(clippy::disallowed_methods)]` annotations once the production sites are fixed.
## Recommended Action
Implement the plan directly, verify each bucket with crate-level tests or lint checks, then run the final formatting, clippy, workspace tests, and `FOLLOW-UP` sweep.
## Acceptance Criteria
- All `FOLLOW-UP:` markers under `lib/crates/` are removed.
- The planned async refactors and `spawn_blocking` boundary changes are implemented.
- Formatting and workspace clippy pass.
- Relevant crate tests pass during incremental verification.
## Work Log
- 2026-04-19: Created execution todo, confirmed branch choice with the user, and started inspecting the planned call sites.

View file

@ -47,6 +47,7 @@ jobs:
with:
persist-credentials: false
- run: bin/dev/check-boundary.sh
- run: bin/dev/check-env-mutation.sh
fmt:
name: Format

View file

@ -100,6 +100,7 @@ When working on Rust crates, read the relevant strategy doc **before** making ch
- **`docs-internal/logging-strategy.md`** — read when adding `tracing` calls (`info!`, `debug!`, `warn!`, `error!`), working on error handling paths, or adding new operations that should be observable
- **`docs-internal/events-strategy.md`** — read when adding or modifying `Event` variants, touching `Emitter`/`emit()`, changing `progress.jsonl` output, or adding new workflow stage types
- **`files-internal/testing-strategy.md`** — read when adding or reorganizing tests, choosing between unit vs `tests/it`, deciding whether a test belongs in `cmd` vs `workflow` vs `scenario`, or deciding how to structure snapshots and fixtures
- **`docs-internal/server-secrets-strategy.md`** — read when adding or changing server-level secrets, startup validation, install-time secret persistence, or subprocess env inheritance/scrubbing
## Shell quoting in sandbox code

43
bin/dev/check-env-mutation.sh Executable file
View file

@ -0,0 +1,43 @@
#!/usr/bin/env bash
set -euo pipefail
cd "$(dirname "$0")/../.."
if command -v rg >/dev/null 2>&1; then
matches=$(rg -n 'std::env::(set_var|remove_var)' --glob '*.rs' || true)
else
matches=$(grep -R -n -E 'std::env::(set_var|remove_var)' . --include='*.rs' --exclude-dir=target --exclude-dir=.git || true)
fi
fail=0
while IFS= read -r match; do
[[ -z "$match" ]] && continue
path=${match%%:*}
rest=${match#*:}
line=${rest#*:}
line=${line#"${line%%[![:space:]]*}"}
case "$path:$line" in
"lib/crates/fabro-telemetry/src/spawn.rs:std::env::set_var(key, value);" | \
"lib/crates/fabro-telemetry/src/spawn.rs:std::env::remove_var(key);" | \
'lib/crates/fabro-server/src/install.rs:std::env::set_var("FABRO_TEST_IN_MEMORY_STORE", "1");')
continue
;;
esac
echo "process env mutation check failed: $match" >&2
fail=1
done <<< "$matches"
if [[ $fail -ne 0 ]]; then
cat >&2 <<'EOF'
Do not mutate process-wide env with std::env::set_var/remove_var.
Inject env at construction time or on child-process Command values instead.
See docs-internal/server-secrets-strategy.md.
EOF
exit 1
fi
echo "Process env mutation checks passed."

View file

@ -20,6 +20,8 @@ disallowed-methods = [
{ path = "std::fs::File::create", reason = "Blocking open; prefer tokio::fs::File::create on Tokio paths. Document intentional sync I/O with #[expect(clippy::disallowed_methods, reason = \"...\")]" },
{ path = "std::fs::File::create_new", reason = "Blocking open; prefer tokio::fs::File::create_new on Tokio paths. Document intentional sync I/O with #[expect(clippy::disallowed_methods, reason = \"...\")]" },
{ path = "std::fs::OpenOptions::open", reason = "Blocking open; prefer tokio::fs::OpenOptions::open on Tokio paths. OS file-lock semantics may require spawn_blocking instead. Document intentional sync I/O with #[expect(clippy::disallowed_methods, reason = \"...\")]" },
{ path = "std::env::set_var", reason = "Server/process env must be injected at construction or child-process spawn time, not mutated globally. See docs-internal/server-secrets-strategy.md" },
{ path = "std::env::remove_var", reason = "Server/process env must be injected at construction or child-process spawn time, not mutated globally. See docs-internal/server-secrets-strategy.md" },
{ path = "reqwest::Client::new", reason = "Use fabro_http::http_client() or fabro_http::test_http_client()", allow-invalid = true },
{ path = "reqwest::Client::builder", reason = "Use fabro_http::HttpClientBuilder::new()", allow-invalid = true },
{ path = "reqwest::blocking::Client::new", reason = "Use fabro_http::blocking_http_client() or fabro_http::blocking_test_http_client()", allow-invalid = true },

View file

@ -34,7 +34,7 @@
| Path | Direct dependency | Why it still exists | Suggested handling |
| --- | --- | --- | --- |
| `lib/crates/fabro-cli/src/commands/store/dump.rs` test module | `event::{Event, append_event}` | Unit tests synthesize workflow events directly. | Low priority; keep until a lighter-weight event fixture helper exists. |
| `lib/crates/fabro-cli/src/commands/dump.rs` test module | `event::{Event, append_event}` | Unit tests synthesize workflow events directly. | Low priority; keep until a lighter-weight event fixture helper exists. |
| `lib/crates/fabro-cli/src/commands/run/wait.rs` test module | `outcome::StageStatus`, `records::Conclusion`, `run_status::RunStatusRecord` | Output tests construct workflow-owned records directly. | Replace with shared fixture builders once status/conclusion DTOs move out. |
| `lib/crates/fabro-cli/src/commands/run/run_progress/mod.rs` test module | `event::{Event, RunNoticeLevel, to_run_event, to_run_event_at}`, `outcome::billed_model_usage_from_llm` | Progress tests build engine events directly. | Replace with shared event fixture helpers after event DTO extraction. |
| `lib/crates/fabro-cli/src/commands/run/run_progress/event.rs` test module | `event::{Event, to_run_event}` | Event rendering tests depend on engine event constructors. | Replace with shared event fixture helpers after event DTO extraction. |

View file

@ -33,7 +33,7 @@ These paths are local runtime state, not canonical event projections.
These names are still real, but they are no longer live scratch files by default:
- Metadata branch files such as `run.json`, `start.json`, `checkpoint.json`, and `retro.json`
- `fabro store dump` exports such as `run.json`, `start.json`, `status.json`, `checkpoint.json`, `conclusion.json`, `retro.json`, `events.jsonl`, and per-node prompt/response/status/stdout/stderr files
- `fabro dump` exports such as `run.json`, `start.json`, `status.json`, `checkpoint.json`, `conclusion.json`, `retro.json`, `events.jsonl`, and per-node prompt/response/status/stdout/stderr files
- Retro-agent temp uploads named `progress.jsonl`, `checkpoint.json`, `run.json`, and `start.json` inside the retro sandbox
## Notes

View file

@ -0,0 +1,68 @@
# Server Secrets Strategy
This document defines how Fabro handles server-level secrets.
## Core Rules
- `ServerSecrets` is the canonical server-secret reader.
- It reads from `process env` and `<storage>/server.env`.
- Resolution is snapshot-based: env and file are read once at construction, then treated as immutable for the life of the process.
- `process env` wins over `server.env` on conflicts.
- `fabro server start` never generates secrets. Missing required secrets are a startup error.
- `std::env::set_var` and `std::env::remove_var` are banned workspace-wide. Tests are not exempt. CI enforces this with `bin/dev/check-env-mutation.sh` so broad clippy suppressions cannot bypass it.
## Active Server Secrets
These values belong to the server runtime and are read via `state.server_secret(...)`:
| Secret | Used by |
|---|---|
| `SESSION_SECRET` | Cookie encryption and JWT signing derivation |
| `FABRO_DEV_TOKEN` | Dev-token auth for worker/server interactions |
| `GITHUB_APP_PRIVATE_KEY` | GitHub App credentials |
| `GITHUB_APP_WEBHOOK_SECRET` | GitHub webhook verification |
| `GITHUB_APP_CLIENT_SECRET` | GitHub OAuth login |
`FABRO_JWT_PRIVATE_KEY` and `FABRO_JWT_PUBLIC_KEY` are removed. `SESSION_SECRET` is the single auth root.
## Startup
- Foreground and daemon startup use the same validation path.
- Required-at-startup secrets are:
- `SESSION_SECRET`
- `FABRO_DEV_TOKEN` when dev-token auth is enabled
- `GITHUB_APP_CLIENT_SECRET` when GitHub auth is enabled
- Other server secrets remain lazy/feature-specific rather than universal boot blockers.
## Provisioning
Server secrets come from one of two sources:
- Platform env for 12-factor deployments
- `server.env` written by install flows
There is no compatibility layer for removed secrets and no startup-time secret generation.
## Subprocess Boundaries
- Worker and render-graph subprocesses start from `env_clear()` and re-add only explicit allowlisted variables.
- Authority-bearing values are re-injected intentionally.
- The daemon child inherits the parent env unchanged except for output-format hygiene (`FABRO_JSON` removal).
## Tests
- In-process tests must inject server secrets with construction-time stubs (`EnvSource`, `StubEnv`) or by writing `server.env`.
- Subprocess tests must set child env with `Command::env`.
- Tests must not mutate the process-wide environment.
## Rotation
- Secret rotation requires restart.
- Live rotation is intentionally unsupported.
## Adding A New Server Secret
1. Provision it through platform env or install-written `server.env`.
2. Read it through `state.server_secret(...)`.
3. Decide explicitly whether startup should fail when it is absent.
4. If a worker or render subprocess needs it, re-inject it explicitly rather than broadening inheritance casually.

View file

@ -30,7 +30,7 @@ Fabro is single-tenant software designed for small, trusted teams. The following
- **Enable authentication.** Fabro supports `dev-token` and GitHub OAuth. Do not disable auth outside of local development or controlled demos.
- **Configure a username allowlist for GitHub OAuth.** `[server.auth.github].allowed_usernames` should contain the exact GitHub users allowed to log in. An empty list rejects everyone.
- **Configure the session secret used by the web flow.** `SESSION_SECRET` should be provisioned with a strong value on long-lived deployments. If you also provision `FABRO_JWT_PRIVATE_KEY` and `FABRO_JWT_PUBLIC_KEY`, treat them as server runtime secrets, but they are not what currently gates browser auth.
- **Configure the session secret used by the web flow.** `SESSION_SECRET` should be provisioned with a strong value on long-lived deployments.
- **Terminate HTTPS or mTLS upstream when needed.** Fabro's listener is plain HTTP/Unix only. If CI, scripts, or a browser must connect over HTTPS, terminate TLS at a reverse proxy or load balancer and keep the Fabro listener on a private network.
### Secrets

View file

@ -299,7 +299,6 @@ For the auth model above, the main server runtime secrets are:
- `SESSION_SECRET` when the web UI is enabled
- `FABRO_DEV_TOKEN` when `"dev-token"` auth is enabled
- `GITHUB_APP_CLIENT_SECRET` when `"github"` auth is enabled
- `FABRO_JWT_PRIVATE_KEY` / `FABRO_JWT_PUBLIC_KEY`, provisioned during install for future CLI login flows
- `AWS_ACCESS_KEY_ID` / `AWS_SECRET_ACCESS_KEY` when the install wizard or a manual config uses
static S3 object-store credentials
@ -332,8 +331,6 @@ Fabro resolves these from `process env -> server.env`.
| Variable | Description |
|---|---|
| `FABRO_JWT_PRIVATE_KEY` | Ed25519 private key (base64-encoded PEM) for JWT signing |
| `FABRO_JWT_PUBLIC_KEY` | Ed25519 public key (base64-encoded PEM) for JWT verification |
| `SESSION_SECRET` | Session encryption secret (64-character hex string) |
### Object store runtime secrets (optional)

View file

@ -7,7 +7,7 @@ When an agent or prompt node finishes, Fabro captures its response text and prod
## Response capture
After an agent or prompt node completes, Fabro captures the full response text and persists it to `stages/{node_id}@{visit}/response.md` in metadata snapshots and `fabro store dump` output. It also writes the final outcome (status, context updates, routing directives) to `stages/{node_id}@{visit}/status.json`.
After an agent or prompt node completes, Fabro captures the full response text and persists it to `stages/{node_id}@{visit}/response.md` in metadata snapshots and `fabro dump` output. It also writes the final outcome (status, context updates, routing directives) to `stages/{node_id}@{visit}/status.json`.
## Context updates
@ -92,7 +92,7 @@ review -> approve [label="Approve"]
## Output logging
Fabro writes several files per stage to `stages/{node_id}@{visit}/` in metadata snapshots and `fabro store dump` output:
Fabro writes several files per stage to `stages/{node_id}@{visit}/` in metadata snapshots and `fabro dump` output:
| File | Contents |
|---|---|

View file

@ -295,4 +295,4 @@ Use prompt nodes for analysis, classification, and summarization tasks where too
## Prompt logging
Fabro persists the assembled prompt to `stages/{node_id}@{visit}/prompt.md` in metadata snapshots and `fabro store dump` output for every agent and prompt stage. This includes the preamble (if any) and the expanded prompt text. Use these files for debugging when an agent behaves unexpectedly.
Fabro persists the assembled prompt to `stages/{node_id}@{visit}/prompt.md` in metadata snapshots and `fabro dump` output for every agent and prompt stage. This includes the preamble (if any) and the expanded prompt text. Use these files for debugging when an agent behaves unexpectedly.

View file

@ -47,5 +47,4 @@ If something is misconfigured, `fabro doctor` tells you exactly what's wrong and
- New indicatif-based progress display for `fabro run start` shows real-time stage progress, tool calls, model names, and timing
- Mercury provider updated to `mercury-2`; estimated output speed (tok/s) added to `fabro model list`
- Run defaults in `server.toml` are inherited by all workflow runs, so you don't have to repeat sandbox, model, or concurrency settings
- `FABRO_JWT_PUBLIC_KEY` and `FABRO_JWT_PRIVATE_KEY` accept base64-encoded PEM strings for containerized deployments
</Accordion>

View file

@ -1,14 +1,14 @@
---
title: "Store dump export command"
title: "Dump export command"
date: "2026-03-29"
---
## `fabro store dump`
## `fabro dump`
A new `fabro store dump` command exports the contents of the run store to a human-readable format for debugging and inspection. This is useful for diagnosing issues with run state, verifying data integrity after migrations, or extracting run data for external analysis.
A new `fabro dump` command exports the contents of the run store to a human-readable format for debugging and inspection. This is useful for diagnosing issues with run state, verifying data integrity after migrations, or extracting run data for external analysis.
```bash
fabro store dump <run-id>
fabro dump <run-id>
```
## More

View file

@ -81,7 +81,7 @@ jq '{from: .properties.from_node, to: .properties.to_node, label: .properties.la
<(fabro logs 01JKXYZ...) | head
```
If you need files on disk for offline analysis, `fabro store dump` exports `events.jsonl` plus run-state projections.
If you need files on disk for offline analysis, `fabro dump` exports `events.jsonl` plus run-state projections.
## Event categories
@ -132,6 +132,6 @@ Post-run analysis surfaces include:
|---|---|
| `fabro logs <RUN>` | Full event envelope stream as NDJSON |
| `fabro inspect <RUN>` | Current durable run state, including run/start/checkpoint/conclusion records |
| `fabro store dump --output <DIR> <RUN>` | Exported `events.jsonl` plus reconstructed JSON and node files |
| `fabro dump --output <DIR> <RUN>` | Exported `events.jsonl` plus reconstructed JSON and node files |
See [retros](/execution/retros), [stages](/api-reference/run-internals/list-run-stages), and [turns](/api-reference/run-internals/list-stage-turns) for higher-level analysis views built on top of this event stream.

View file

@ -143,4 +143,4 @@ Retros are also available via the REST API. See the [list retros](/api-reference
## Storage
Retros are stored in durable run state. If you need files on disk, `fabro store dump` materializes retro text under `stages/retro/` alongside `run.json`, stage files, and the rest of the exported run data.
Retros are stored in durable run state. If you need files on disk, `fabro dump` materializes retro text under `stages/retro/` alongside `run.json`, stage files, and the rest of the exported run data.

View file

@ -0,0 +1,483 @@
---
title: "refactor: Lock down server-level secrets handling"
type: refactor
status: active
date: 2026-04-22
deepened: 2026-04-23
---
# Lock down server-level secrets handling
## Overview
Server-level secrets (SESSION_SECRET, FABRO_DEV_TOKEN, GitHub App credentials, JWT keypair) flow through three independent paths today:
- `ServerSecrets::get` reads process env *live* on every call, with on-disk envfile fallback.
- `load_or_create_local_session_secret` reads env first, then envfile, then auto-generates if missing.
- `execute_foreground` mutates parent process env via `std::env::set_var` so the in-process server's live env reads see the resolved value.
Worker subprocess scrubs `FABRO_DEV_TOKEN`; daemon spawn does not — accidental inconsistency.
This refactor:
- **Makes `ServerSecrets` snapshot-based.** Reads env and envfile *once* at construction, exposes the merged result. Both env and file remain valid sources (env wins on conflict — 12-factor convention). The bug was *live* env reads coupled with parent-process mutation, not env reads themselves.
- **Eliminates parent-process env mutation.** With snapshot semantics, the foreground `set_var` becomes unnecessary; the daemon `cmd.env(SESSION_SECRET)` becomes redundant.
- **CLI install and web install share a common orchestration path for `server.env` writes.** Coordinated install flows may update server-level secrets while a server is running, but they must also own restart/handoff (web install already does this via `/install/finish`).
- **Worker and render-graph subprocesses use `env_clear` + strict fail-closed allowlists.** Worker is the trust boundary — it dispatches user-supplied workflow stages via `Sandbox`. Daemon child inherits parent env unchanged (12-factor pattern works).
- **Foreground and daemon startup share one validation path.** Daemon preflight builds the same `ServerSecrets` snapshot and runs the same server-side auth/startup validation foreground does — no separate parent-CLI validator with drift risk.
- **Legacy `FABRO_JWT_PRIVATE_KEY` / `FABRO_JWT_PUBLIC_KEY` drift is removed.** SESSION_SECRET remains the sole auth master (HKDF source for cookie key + JWT signing key per `2026-04-19-003-feat-cli-auth-login-plan.md`); install stops generating those keys and actively removes them from `server.env` on subsequent runs.
- **`clippy::disallowed_methods` denies `std::env::set_var` / `remove_var`** workspace-wide *including tests*; narrow documented post-fork/pre-exec exceptions only.
## Problem Frame
- **Live env reads + parent-process mutation is unsound.** `std::env::set_var` in `execute_foreground` mutates global process state inside an async runtime. Rust 2024 marks this `unsafe` because it races concurrent readers. Workspace is on 2021; moving to 2024 forces the issue. The set_var is load-bearing because `ServerSecrets::get` reads env *live*, not at construction.
- **Worker subprocess env leakage.** Workers run user-supplied workflow stages (via `Sandbox`). Today they inherit the operator's full process env — any credential the operator exported leaks to user-controlled commands.
- **`SESSION_SECRET` autogeneration is the real startup bypass.** `load_or_create_local_session_secret` mints a value and writes the envfile if missing — a server can boot with a freshly-minted secret that bypassed the install flow's full setup. Same precedent already removed for `FABRO_DEV_TOKEN` (commit `7cb6c65d5`); SESSION_SECRET is the leftover.
- **`FABRO_JWT_*` is stale config drift.** The CLI auth migration (`2026-04-19-003-feat-cli-auth-login-plan.md`) consolidated cookie + JWT signing into a single HKDF chain rooted at SESSION_SECRET. The legacy `FABRO_JWT_PRIVATE_KEY` / `FABRO_JWT_PUBLIC_KEY` envfile entries no longer participate in the runtime auth model but install still generates them and diagnostics still inspects them — they obscure the actual auth model and risk operator confusion.
- **CLI install and web install have drifted into separate persistence/restart behaviors.** They write to `server.env` via parallel code paths today; the bug-surface of "what does install actually persist" is twice as large as it should be. They should reconverge on a shared orchestration with consistent contracts.
## Requirements Trace
- R1. `ServerSecrets` reads env and envfile *once* at boot and exposes the merged snapshot. No live env reads from `ServerSecrets::get` or its callers on the resolution path. Env wins on conflict.
- R2. `fabro server start` does not generate any server-level secret. Missing → fail fast pointing at the install flows (CLI or web) or direct env-set.
- R3. `execute_foreground` does not mutate parent process env.
- R4. `execute_daemon` does not pass server-level secrets via `cmd.env(...)` (no longer needed; daemon child reads env+file at its own boot).
- R5. Worker and render-graph subprocesses use `env_clear` + strict fail-closed allowlists. New inherited env vars require demonstrated need plus tests. Authority-bearing values re-injected explicitly.
- R6. Tests use construction-time env stubs (via a small `EnvSource` trait) or explicit child-process `Command::env`. No test mutates process-wide env.
- R7. Foreground and daemon startup use one shared validation path; required secrets mirror current auth rules (SESSION_SECRET always; FABRO_DEV_TOKEN when dev-token auth enabled; GITHUB_APP_CLIENT_SECRET when GitHub auth enabled).
- R8. CLI install and web install share orchestration for coordinated `server.env` writes and restart/handoff.
- R9. `FABRO_JWT_PRIVATE_KEY` / `FABRO_JWT_PUBLIC_KEY` are removed from install output, diagnostics, docs, and existing `server.env` files during install flows.
- R10. Behavior changes are documented in a strategy doc and enforced via workspace-wide `clippy::disallowed_methods` — including tests — with narrow documented exceptions only.
## Scope Boundaries
**In scope, active** — server-level secrets read via `state.server_secret(...)`, sourced from process env or `<storage>/server.env`:
| Secret | Consumer | Sources |
|---|---|---|
| `SESSION_SECRET` | `AppState::session_key` (`server.rs:713`); HKDF source for cookie key + JWT signing key | env (12-factor) or `server.env` (install) |
| `FABRO_DEV_TOKEN` | `worker_command` (`server.rs:3825`) | env or `server.env` |
| `GITHUB_APP_PRIVATE_KEY` | `AppState::github_credentials` (`server.rs:726`) | env or `server.env` |
| `GITHUB_APP_WEBHOOK_SECRET` | `github_webhook_routes` (`server.rs:983`) | env or `server.env` |
| `GITHUB_APP_CLIENT_SECRET` | `web_auth.rs:536` | env or `server.env` |
Both install flows (CLI `fabro install` and web install via `/install/finish`) write the envfile; neither mutates process env. Operators on 12-factor PaaS set secrets in platform env; operators on local/server installs use one of the install flows.
**In scope, removal targets:**
- `FABRO_JWT_PRIVATE_KEY`, `FABRO_JWT_PUBLIC_KEY` — legacy entries that no longer participate in the runtime auth model. Install stops generating them; diagnostics stops inspecting them; existing entries are actively removed from `server.env` on subsequent install flows.
**Startup-critical subset:** SESSION_SECRET (always), FABRO_DEV_TOKEN (when dev-token auth is enabled), and GITHUB_APP_CLIENT_SECRET (when GitHub auth is enabled). The other in-scope active secrets (GITHUB_APP_PRIVATE_KEY, GITHUB_APP_WEBHOOK_SECRET) are not boot blockers — they continue to surface where they did before (conditional route mounts, per-request errors). The plan does not promote every server secret to a startup gate.
**Out of scope:**
- **Vault + `ProviderCredentials`** (`secrets.json`, REST-managed, runtime-mutable). Different lifecycle.
- **`vault_or_env` for run-level credentials** (`GITHUB_TOKEN`, `DAYTONA_API_KEY`). Same env-as-source pattern, different track.
- **`{{ env.FOO }}` config interpolation.** Operator-supplied templating, different feature.
- **Workflow-stage env (Sandbox).** Stages run inside `Sandbox` (local/Docker/Daytona); their env is configured by the workflow definition + Sandbox config. Anything a workflow stage needs to execute (e.g. `git push` requiring `GITHUB_TOKEN`) routes via Vault → Sandbox, not subprocess inheritance.
- **`validate_api_key` `set_var` in `provider_auth.rs`.** Vault-side smell, deferred.
- **Tailscale and `bun --watch-web` spawns.** Different surfaces.
## Context & Research
### Relevant code
- `lib/crates/fabro-server/src/server_secrets.rs` — `ServerSecrets` definition; today's live `env_lookup` closure is the bug.
- `lib/crates/fabro-server/src/server.rs:697-699` — `state.server_secret(name)` wrapper.
- `lib/crates/fabro-server/src/server.rs:703,712-715` and `jwt_auth.rs:84-99` — `resolve_auth_mode_with_lookup`, currently routed through the live `env_lookup`.
- `lib/crates/fabro-server/src/server.rs:3776-3834` (`worker_command`) — worker spawn site.
- `lib/crates/fabro-server/src/server.rs:7232-7235` — `__render-graph` spawn site.
- `lib/crates/fabro-cli/src/commands/server/start.rs:229-274` — `load_or_create_local_session_secret` and `execute_foreground` `set_var` block.
- `lib/crates/fabro-cli/src/commands/server/start.rs:319-362` — `execute_daemon` spawn flow.
- `lib/crates/fabro-cli/src/commands/install.rs:1816,1856,1864` — install SESSION_SECRET generation and persistence.
- `lib/crates/fabro-config/src/envfile.rs:204-256` — `write_env_entries`, already atomic via tmp + fsync + rename.
- `Cargo.toml:111` and `clippy.toml` — existing `disallowed_methods` mechanism.
### Institutional learnings
- `docs/plans/2026-04-05-server-canonical-secrets-doctor-repo-plan.md` — architectural anchor: server is canonical, install provisions, request-time reads from store.
- `docs/plans/2026-04-19-003-feat-cli-auth-login-plan.md` — `SESSION_SECRET` is now HKDF master for JWT signing + cookie key. Higher stakes.
- `docs/plans/2026-04-18-001-feat-webhook-strategy-plan.md` — webhook secret already plumbed via `AppState` from the resolved store.
- Commit `7cb6c65d5` — "Remove dev-token minting from server start." SESSION_SECRET autogenerate is the leftover.
## Key Technical Decisions
- **`ServerSecrets` is a snapshot, not a live reader.** A small `EnvSource` trait exists *only at construction time* and immediately materializes into an owned `HashMap<String, String>`. No trait object or callback survives into runtime lookup. `get(name)` returns from the merged (env ∪ file) snapshot; env wins on conflict (12-factor). Role: the *resolved-secrets API*; the source mix is the operator's choice.
- **Production uses a real process-env `EnvSource`; tests use a map-backed stub.** No live env reads anywhere on the secret resolution path. No `EnvLookup` closure surviving into runtime.
- **`resolve_auth_mode_with_lookup` migrates to read from `server_secrets`, not raw env.** Closure passed in delegates to `self.server_secrets.get(...)`. Without this, auth-mode resolution remains a live env reader.
- **Worker and render-graph allowlists are strict fail-closed.** Worker is the trust boundary (dispatches user stages via `Sandbox`); render-graph is hygiene. New inherited env vars require demonstrated need (failing test) and intentional addition. Proxy/cert env vars are not auto-inherited.
- Worker list: `PATH`, `HOME`, `TMPDIR`, `USER`, `RUST_LOG`, `RUST_BACKTRACE`, `FABRO_HOME`, `FABRO_STORAGE_ROOT`. Plus explicit `FABRO_DEV_TOKEN` re-injection when dev-token auth is enabled.
- Render-graph list: `PATH`, `HOME`, `TMPDIR`. Plus explicit `FABRO_TELEMETRY=off`.
- **Daemon child spawn inherits parent env unchanged** (modulo existing `cmd.env_remove("FABRO_JSON")` for output-format hygiene). The daemon is fabro's own server code, not a security boundary against itself. 12-factor env vars (SESSION_SECRET, AWS_*, RAILWAY_*) flow through naturally to the daemon child's snapshot.
- **Daemon preflight reuses the same shared auth/startup validation as foreground startup.** Both modes construct the same `ServerSecrets` snapshot and call the same server-side validation logic (today's `jwt_auth::resolve_auth_mode_with_lookup` already encodes "what's required for this auth mode" — that's the shared path). No separate parent-CLI validator with drift risk. Required secrets remain exactly the current startup-critical set: SESSION_SECRET (always), FABRO_DEV_TOKEN (when dev-token auth enabled), GITHUB_APP_CLIENT_SECRET (when GitHub auth enabled). GITHUB_APP_PRIVATE_KEY and GITHUB_APP_WEBHOOK_SECRET remain in-scope server secrets but are not new universal boot blockers.
- **Install coordination policy lives in shared install orchestration, not in low-level envfile helpers.** Coordinated install flows (CLI and web) may write to `server.env` while a server is running — but only when they also own restart/handoff. Web install already does this via `/install/finish`. CLI install joins the same orchestration. Manual edits to `server.env` outside the orchestration still require operator restart discipline.
- **`pub(crate) server_secrets` field tightened to `pub(super)`.** No production code legitimately bypasses the wrapper.
- **Workspace-wide clippy ban including tests.** Add `std::env::set_var`/`remove_var` to `clippy.toml` `disallowed-methods` — applies to production AND test code. Tests must use construction-time `EnvSource` stubs or explicit child-process `Command::env`. Narrow documented exceptions only (`fabro-telemetry/src/spawn.rs:73-76` post-fork pre-execvp).
- **Legacy `FABRO_JWT_*` is removed, not preserved.** Install stops generating; diagnostics stops reading; existing envfile entries are actively cleaned up on subsequent install flows. SESSION_SECRET is the sole auth master.
## Open Questions
### Resolved during planning
- **`ServerSecrets` reads env AND file, snapshots, env wins.** Process env is a legitimate source (12-factor). The bug was *live* env reads + parent mutation, not env-as-source.
- **`EnvSource` exists only at construction time.** A small trait used to materialize an owned `HashMap` once; no callback or trait object survives into runtime lookup. Production uses a real process-env source; tests use a map-backed stub.
- **Daemon child inherits parent env; only worker/render-graph scrub.** Daemon is fabro's own server code and must see whatever the operator/platform set in env. Worker is the trust boundary (dispatches user stages via `Sandbox`).
- **No carve-out for AWS / cloud-platform credentials.** Daemon inherits parent env, so AWS env names reach the object-store layer naturally.
- **Active scope is 5 server secrets plus 2 legacy removal targets.** SESSION_SECRET, FABRO_DEV_TOKEN, GITHUB_APP_PRIVATE_KEY, GITHUB_APP_WEBHOOK_SECRET, GITHUB_APP_CLIENT_SECRET are active. FABRO_JWT_PRIVATE_KEY and FABRO_JWT_PUBLIC_KEY are legacy — install stops generating them; diagnostics stops reading them; existing entries are actively removed.
- **Workflow stage env is the Sandbox's job.** This plan does not address workflow `bash`/`git`/etc. stage env — that's per-workflow Sandbox config. Workflows requiring credentials route them through Vault.
- **Web install keeps its existing restart-handoff contract.** `/install/finish` still returns the restart URL and the SPA still waits for the server to come back. No new `restart_required` flag, no manual-restart message proposal, no API surface change.
- **CLI and web install share orchestration for coordinated writes while running.** Blanket "refuse-while-running" is out. The orchestration is responsible for restart/handoff; both flows call into it.
- **Test migration shape:** full migration in Unit 1. `ServerSecrets::with_env_lookup` is deleted; tests use either `ServerSecrets::load(path, env_source)` with a map-backed `EnvSource` stub, or `provision_server_secrets(env_path, &[(name,value)])` helper (file-based injection). No `#[cfg(test)]` backdoor.
- **Visibility tightening:** `pub(crate) server_secrets` at `server.rs:579` → `pub(super)` as part of Unit 1.
- **Install lock window for interactive flows:** install orchestration's job; `gather inputs → persist + restart-handoff`. Serialization window is sub-second regardless of OAuth duration.
- **Dev-loop friction:** no `--quick` flag. Contributors run `fabro install` or set secrets directly via env (`SESSION_SECRET=$(openssl rand -hex 32) cargo run -- server start` works under the env-as-source model).
- **Compliance-driven rotation:** deferred. Captured as a known limitation in the strategy doc.
## Implementation Units
- [ ] **Unit 1: `ServerSecrets` becomes a snapshot built from `EnvSource` (construction-time only)**
**Goal:** `ServerSecrets` reads env and envfile *once* at construction via a small `EnvSource` trait, materializes both into owned `HashMap`s, and exposes the merged snapshot. No trait object or closure survives into runtime lookup. Auth-mode resolution stops being a live env reader.
**Requirements:** R1, R6
**Dependencies:** None.
**Files:**
- Modify: `lib/crates/fabro-server/src/server_secrets.rs` — replace `env_lookup` closure field with `env_entries: HashMap<String, String>` (owned). Introduce a small `EnvSource` trait used *only* at construction:
- `pub trait EnvSource { fn snapshot(&self) -> HashMap<String, String>; }`
- `pub struct ProcessEnv;` impls `snapshot` via `std::env::vars().collect()`.
- `pub struct StubEnv(pub HashMap<String, String>);` impls `snapshot` by cloning. (cfg-test or cfg(any(test, feature = "test-support")) — implementer picks based on existing patterns.)
- `pub fn load(path: PathBuf, env: &dyn EnvSource) -> Result<Self, Error>` — calls `env.snapshot()` once, stores the result, never references `env` again.
- `get(name)` returns `self.env_entries.get(name).cloned().or_else(|| self.file_entries.get(name).cloned())`.
- Modify: `lib/crates/fabro-server/src/server.rs` — `build_app_state` accepts pre-built `ServerSecrets` and `AuthMode` from the caller (no longer constructs `ServerSecrets` itself). The single production caller is `serve_command` (via `resolve_startup` from Unit 4); test helpers construct both in the same way. Tighten `pub(crate) server_secrets` field at `server.rs:579` to `pub(super)`. Migrate the `resolve_auth_mode_with_lookup` call (`server.rs:703`) to pass a closure delegating to `self.server_secrets.get(...)`.
- Modify: `lib/crates/fabro-server/src/serve.rs:512` and `install.rs:910,965,2108` — minimal API migration: `ServerSecrets::load(path, &ProcessEnv)` instead of `with_env_lookup` at any *non-startup* construction sites (e.g. install-time inspection, install_object_store_lookup test helper). The startup construction sites at `serve.rs:594-607` are NOT modified by Unit 1 — Unit 4 collapses them into a single `resolve_startup(...)` call. To keep the workspace compiling between Unit 1 and Unit 4, Unit 1 may leave a temporary `ServerSecrets::load(path, &ProcessEnv)` call at `serve.rs:594` if needed; Unit 4 deletes it. Sequencing is documented in Unit 4's Dependencies.
- Modify: `lib/crates/fabro-server/src/server.rs:7864-7885` — replace `server_secrets_resolve_process_env_before_server_env` with a snapshot test using `StubEnv` (assert env-wins-on-conflict from the snapshot).
- Modify: `lib/crates/fabro-server/tests/it/api/cli_auth_token.rs:34`, `routing.rs:21,35-36`, `tcp.rs:28,68,173-174`, `lib/crates/fabro-cli/tests/it/support/auth_harness.rs:39-86` — migrate from `env_lookup` injection (for *secret* values) to constructing the test's `ServerSecrets` with `&StubEnv(...)`, or writing to a temp `server.env` via the `provision_server_secrets(env_path, &[(name, value)])` helper. Tests that currently pass `env_lookup` only to feed secret values into `ServerSecrets` switch to the new mechanism.
- Modify: test helpers like `create_app_state_with_env_lookup` (`server.rs:2488` and similar) — once `build_app_state` (`server.rs:2631`) requires precomputed `ServerSecrets` and `AuthMode`, the helpers must produce both internally and pass them through. The `env_lookup` parameter on these helpers is *retained* (still consumed downstream by `resolve_canonical_origin` at `server.rs:708` and slack `{{ env.FOO }}` interpolation at `server.rs:2676`, both out of scope). New shape: helper accepts an additional `&dyn EnvSource` parameter (or constructs a `StubEnv`/`ProcessEnv` based on the call site's intent), internally calls `resolve_startup(...)` (or directly constructs `ServerSecrets` + computes `AuthMode` if the helper bypasses settings — auditor's choice per call site), and passes the resulting `ServerSecrets` and `AuthMode` into `build_app_state`. Tests that set up specific auth modes (e.g. dev-token enabled) now thread settings + `EnvSource` through the helper rather than relying on a single closure to drive both secret resolution and auth-mode computation.
- Test: existing files plus new snapshot-semantics tests in `server_secrets.rs`.
**Approach:**
- `EnvSource::snapshot()` is called exactly once during `ServerSecrets::load`. The trait reference is dropped immediately after; only the materialized `HashMap` is retained. No risk of live env reads via trait object dispatch.
- Object-store credential resolution at `serve.rs:340-401, 520-522, 540-542` uses `std::env::var(...)` directly (daemon child inherits parent env, so AWS names are present).
**Test scenarios:**
- Happy path: `ServerSecrets::load(path, &StubEnv([("SESSION_SECRET", "from-env")].into()))` returns "from-env" even when file has a different value.
- Edge: env empty, file has the value → file wins (single fallback path).
- Edge: neither env nor file has the value → `get` returns `None`.
- Edge: env has it, file does too, different values → env wins (12-factor).
- Edge: missing file path → empty `file_entries`, env-only resolution.
- Snapshot semantics: after construction, mutating the source `EnvSource` (or process env, if production source) does not affect `get` returns. The snapshot is owned and immutable.
- Auth-mode happy path: `resolve_auth_mode_with_lookup` resolves correctly when secrets come from env, file, or both.
- Migration: `cli_auth_token`, `routing`, `tcp`, `auth_harness` tests pass after migration to `EnvSource` stubs.
**Verification:**
- `cargo nextest run -p fabro-server -p fabro-cli` passes.
- `grep -rn 'std::env::var\|std::env::vars' lib/crates/fabro-server/src/ | grep -v 'serve.rs\|object_store\|spawn_env\|server_secrets.rs'` shows no live env reads on the server-secret resolution path (the only `std::env::vars()` is inside `ProcessEnv::snapshot`, called once at construction).
---
- [ ] **Unit 2: Drop foreground `set_var`; drop daemon `cmd.env(SESSION_SECRET)`**
**Goal:** Eliminate parent-process env mutation. Daemon parent stops passing SESSION_SECRET via `cmd.env`; daemon child reads env+file at its own boot.
**Requirements:** R3, R4
**Dependencies:** Unit 1.
**Files:**
- Modify: `lib/crates/fabro-cli/src/commands/server/start.rs:265-274` — delete the `set_var`/scopeguard block in `execute_foreground`.
- Modify: `lib/crates/fabro-cli/src/commands/server/start.rs:350,352` — `execute_daemon` removes `cmd.env("SESSION_SECRET", ...)`. Keep existing `cmd.env_remove("FABRO_JSON")` (output-format hygiene).
- Modify: `lib/crates/fabro-test/src/lib.rs:931` — drop `cmd.env("SESSION_SECRET", ...)` for spawned test servers; tests provision via `server.env` or by setting env on the spawned `Command` explicitly when the test simulates a 12-factor PaaS scenario.
- Test: existing `cmd/server_start.rs` tests cover both modes.
**Approach:**
- Unit 1's snapshot semantics make both `set_var` and `cmd.env(SESSION_SECRET)` unnecessary. The in-process foreground server's `ServerSecrets::load(path, &ProcessEnv)` snapshots whatever env the parent CLI had. The daemon child inherits parent env (today's behavior — unchanged) and snapshots it at its own boot.
- A `SESSION_SECRET` exported in the operator's parent shell does flow through to both modes — that's the 12-factor design intent.
**Test scenarios:**
Tests proving env behavior must use construction-time `EnvSource` stubs or explicit child-process `Command::env` — *not* `std::env::set_var`. Any old test using process-env mutation is migrated as part of this unit (or Unit 7's clippy enforcement will fail it).
- Happy path (foreground, env source): in-process server constructed with `&StubEnv([("SESSION_SECRET", "from-env")].into())`; running server uses that value.
- Happy path (foreground, file source): empty `EnvSource`, value in `server.env`; running server uses the file value.
- Happy path (foreground, env-wins): env and file have different values; env wins.
- Happy path (daemon, env source): test passes `SESSION_SECRET` to spawned daemon via explicit `Command::env`; daemon child inherits and snapshots it. NOT via `std::env::set_var` in the test process.
- Happy path (daemon, file source): no env on the spawned `Command`, value in `server.env`; daemon child uses the file value.
**Verification:**
- `grep -rn 'std::env::set_var\|remove_var' lib/crates/fabro-cli/src/` returns no hits in production code.
- `grep -rn 'cmd\.env."SESSION_SECRET"' lib/crates/fabro-cli/src/` returns no hits in production code (test code may still use `Command::env` for daemon spawn simulation; that's fine — it's setting child env, not parent env).
---
- [ ] **Unit 3: Worker and render-graph spawns use `env_clear` + strict fail-closed allowlists**
**Goal:** Worker and render-graph subprocesses inherit only an explicit allowlist. The lists are strict fail-closed — new env vars require demonstrated need (a failing test that proves a workflow execution path needs them) plus intentional addition. Authority-bearing values re-injected explicitly. Daemon child spawn is unchanged (inherits parent env per 12-factor).
Proxy and TLS env vars (`HTTPS_PROXY`, `SSL_CERT_FILE`, etc.) are *not* inherited unless usage-proven by a failing test and intentionally added — they were never part of the worker's documented contract.
**Requirements:** R5
**Dependencies:** None.
**Files:**
- Create: `lib/crates/fabro-server/src/spawn_env.rs` — defines two helpers:
- `apply_worker_env(&mut tokio::process::Command)` — `env_clear` + worker list.
- `apply_render_graph_env(&mut tokio::process::Command)` — `env_clear` + render-graph list.
- Modify: `lib/crates/fabro-server/src/server.rs:3776-3834` — `worker_command` calls `apply_worker_env(&mut cmd)` first, then existing `cmd.env("FABRO_DEV_TOKEN", token)` re-injection. Remove existing `env_remove("FABRO_JSON")` and `env_remove("FABRO_DEV_TOKEN")` (covered by `env_clear`).
- Modify: `lib/crates/fabro-server/src/server.rs:7232-7235` — `__render-graph` spawn calls `apply_render_graph_env(&mut cmd)`. Keep explicit `cmd.env("FABRO_TELEMETRY", "off")`.
- Test: new tests in `spawn_env.rs`.
**Approach:**
```text
WORKER list (each entry has // reason: ... in the source):
PATH, HOME, TMPDIR, USER // process essentials
RUST_LOG, RUST_BACKTRACE // diagnostics
FABRO_HOME, FABRO_STORAGE_ROOT // worker reads its own state
+ explicit cmd.env("FABRO_DEV_TOKEN", token) when dev-token auth enabled
RENDER_GRAPH list:
PATH, HOME, TMPDIR
+ explicit cmd.env("FABRO_TELEMETRY", "off")
DAEMON spawn: no helper. Inherits parent env. Keeps existing cmd.env_remove("FABRO_JSON").
```
The lists are constants in `spawn_env.rs`. Each entry is one named env var with a one-line `//` comment. A worker that needs a new env var must be amended in source — that's the entire mechanism.
**Test scenarios:**
- Happy path (worker): with allowlisted names in parent env, all reach the worker. Random names (`MY_API_KEY`, `NEW_RELIC_LICENSE_KEY`, `DATABASE_URL`, `SESSION_SECRET=leak`) do not.
- Happy path (worker, dev-token enabled): `FABRO_DEV_TOKEN` is set to the install-provisioned value via explicit re-injection.
- Negative (worker): parent's `FABRO_DEV_TOKEN=garbage` does not reach the worker; explicit re-injection sets the correct value.
- Happy path (render-graph): `PATH` and `HOME` reach the child; arbitrary parent vars do not.
- Integration (render-graph): with `FABRO_TELEMETRY=on` in parent env, child sees `off` (explicit override after `env_clear`).
- Integration: real worker subprocess executes a workflow end-to-end with leak probes (`MY_API_TOKEN=leak`, `NEW_RELIC_LICENSE_KEY=leak`) exported in parent env. Workflow completes; leak probes do not appear in worker logs or in stage env (Sandbox-configured).
**Verification:**
- `cargo nextest run -p fabro-server` passes.
- A test asserts the worker spawn sees *only* names in the allowlist plus the explicit `FABRO_DEV_TOKEN` — i.e. "no ambient inheritance except allowlisted names." Same for render-graph. The check operates by enumerating the child's actual env, not by greping `env_remove` calls.
- `grep -rn 'env_remove' lib/crates/fabro-server/src/` returns no hits for the worker (`server.rs:3776-3834`) or render-graph (`server.rs:7232-7235`) spawn sites — both routes are now via `env_clear` + the helpers.
- `grep -rn 'env_remove' lib/crates/fabro-cli/src/commands/server/start.rs` returns the single intentional `cmd.env_remove("FABRO_JSON")` on the daemon-spawn path (output-format hygiene; daemon child unchanged per Unit 2). No other `env_remove` calls in CLI server code.
---
- [ ] **Unit 4: Shared startup validation via snapshot-backed `ServerSecrets`**
**Goal:** Server start no longer auto-generates. Daemon preflight constructs the same `ServerSecrets` snapshot foreground startup uses and runs the *same* server-side auth/startup validation logic. No separate parent-CLI validator that could drift from the server-side rules.
**Requirements:** R1, R2, R7
**Dependencies:** Units 1-3.
**Files:**
- Create: `lib/crates/fabro-server/src/startup.rs` (or extend an existing module) — define the shared validation logic. Two entry points around it: a public preflight wrapper (CLI calls this; never sees `ServerSecrets`), and a crate-internal full-resolution function (`serve_command` calls this; consumes the snapshot directly). Both share a single internal implementation so there is no possibility of drift.
```text
// Crate-internal: returns full state for in-process consumption.
pub(crate) struct StartupResolution {
pub(crate) auth_mode: AuthMode,
pub(crate) server_secrets: ServerSecrets,
}
pub(crate) fn resolve_startup(
env_path: &Path,
env: &dyn EnvSource,
settings: &ResolvedServerSettings,
) -> Result<StartupResolution, StartupValidationError>
// Public: thin wrapper for CLI preflight. Calls resolve_startup, drops the
// StartupResolution after validating, returns just the success/failure.
pub fn validate_startup(
env_path: &Path,
env: &dyn EnvSource,
settings: &ResolvedServerSettings,
) -> Result<(), StartupValidationError>
```
Internally `resolve_startup` constructs `ServerSecrets::load(env_path, env)`, then runs the existing `jwt_auth::resolve_auth_mode_with_lookup` against a closure delegating to that snapshot, then returns both. `validate_startup` is `resolve_startup(...).map(|_| ())` — the literal sharing of code makes drift impossible.
Function takes `&ResolvedServerSettings` (not just `&ServerAuthSettings`) because validation already depends on `server.web.enabled` and `server.integrations.github.client_id` (`jwt_auth.rs:63`) — narrowing would silently weaken the validation surface.
`StartupValidationError` *wraps or re-uses the existing error type* returned by `jwt_auth::resolve_auth_mode_with_lookup` plus the secret-loading errors from `ServerSecrets::load`. It must cover the full surface that path rejects today: missing required secret (`SESSION_SECRET`, `FABRO_DEV_TOKEN` when dev-token auth enabled, `GITHUB_APP_CLIENT_SECRET` when GitHub auth enabled), invalid secret value (e.g., malformed `FABRO_DEV_TOKEN`), empty auth methods, GitHub auth configured with `server.web.enabled = false`, missing `server.integrations.github.client_id`. Implementer audits `jwt_auth.rs:67` to enumerate the full variant set; the plan does not invent a narrow new one.
CLI never sees `ServerSecrets`. `ServerSecrets` and `resolve_startup` stay `pub(crate)`. Only `validate_startup` + `EnvSource` + `ProcessEnv` + `StartupValidationError` cross the crate boundary.
- Modify: `lib/crates/fabro-server/src/lib.rs` — re-export `startup::{validate_startup, EnvSource, ProcessEnv, StartupValidationError}` from the crate root. **Not** `resolve_startup` or `StartupResolution`.
- Modify: `lib/crates/fabro-cli/src/commands/server/start.rs:229-250` — delete `load_or_create_local_session_secret`. Daemon preflight calls `fabro_server::validate_startup(runtime_directory.env_path(), &fabro_server::ProcessEnv, &resolved_settings)`. On `Err`: surface the error to stderr exactly as returned (text comes from the shared error type's `Display`).
- Modify: `lib/crates/fabro-cli/src/commands/server/start.rs:264` — `execute_foreground` does not run a separate validator. The in-process server's `serve_command` calls `resolve_startup` internally and threads the returned `(ServerSecrets, AuthMode)` into `build_app_state` (replacing today's separate `ServerSecrets::with_env_lookup` + `resolve_auth_mode_with_lookup` calls at `serve.rs:594-607`). Single source of truth: foreground's resolution and daemon's preflight share the same internal `resolve_startup`; only the calling surface differs.
- Modify: `lib/crates/fabro-server/src/serve.rs:594-607` — replace today's ad-hoc two-step construction with a single `resolve_startup(...)` call. The returned `ServerSecrets` and `AuthMode` are passed into `build_app_state` (per Unit 1's revised signature).
- Modify: `lib/crates/fabro-cli/src/commands/server/start.rs:350` — `execute_daemon` runs `validate_startup` before spawning the child.
- Test: `tests/it/cmd/server_start.rs` adds missing-secret tests for each required secret across both modes, plus tests demonstrating env-source success, file-source success, and each non-missing-key rejection (empty auth methods, web-disabled with GitHub auth, missing client_id, invalid dev token). A unit test in `fabro-server` asserts `validate_startup` and `resolve_startup` return identical accept/reject decisions for the same inputs.
**Approach:**
- Required secrets remain *exactly* the current startup-critical set, encoded once in the shared validation: SESSION_SECRET (always), FABRO_DEV_TOKEN (when dev-token auth is enabled), GITHUB_APP_CLIENT_SECRET (when GitHub auth is enabled). GITHUB_APP_PRIVATE_KEY and GITHUB_APP_WEBHOOK_SECRET remain in-scope server secrets but are NOT new universal boot blockers — they continue to surface where they did before (conditional route mounts, per-request errors).
- Daemon preflight and foreground startup run the same validation function with the same `ServerSecrets` snapshot type; "drift" is impossible by construction.
**Test scenarios:**
- Happy path: required secrets in env → boots (both modes).
- Happy path: required secrets in file → boots (both modes).
- Happy path: env wins when both present.
- Error path (each required secret): missing in both → fail fast naming the secret and both sources (both modes get identical error text because they call the same function).
- Negative regression: post-Unit-1 snapshot is built once; mutating env after boot doesn't change the resolved value.
**Verification:**
- `fabro server start` against an empty env and uninstalled storage exits non-zero with the same error message in both `--foreground` and daemon modes.
- `fabro server start` with secrets in env (12-factor simulation) boots without any install flow having run.
- `grep -rn 'generate_session_secret' lib/crates/fabro-cli/src/commands/server/` returns no hits.
- `grep -rn 'ServerSecrets\|server_secrets::\|resolve_startup\|StartupResolution' lib/crates/fabro-cli/` returns no hits — CLI never sees `ServerSecrets` or the internal resolution. Only `validate_startup` is reachable from outside `fabro-server`.
- The public secret-related surface from `fabro-server`'s crate root is exactly: `validate_startup`, `EnvSource`, `ProcessEnv`, `StartupValidationError`. Nothing else.
- Foreground startup at `serve.rs:594-607` makes exactly one call to compute `(ServerSecrets, AuthMode)` — `resolve_startup(...)` — not a separate `ServerSecrets::with_env_lookup` followed by `resolve_auth_mode_with_lookup`. Both values are passed into `build_app_state`.
- `build_app_state` no longer constructs `ServerSecrets`; it accepts pre-built `ServerSecrets` and `AuthMode` from the caller. Verified by reading the signature.
---
- [ ] **Unit 5: Shared install orchestration for coordinated `server.env` writes and restart handoff**
**Goal:** CLI install and web install share a single orchestration layer for `server.env` persistence. Both flows take the same path through generation, persistence, removal of legacy entries, and restart handoff. Web install keeps its existing `/install/finish` restart-handoff contract; CLI install joins the same orchestration. No blanket refuse-while-running policy.
**Requirements:** R8
**Dependencies:** Pairs naturally with Unit 6 (legacy `FABRO_JWT_*` removal hooks into the same orchestration). Either order works.
**Files:**
- Modify: `lib/crates/fabro-install/src/lib.rs` — establish the shared orchestration entry points. Both CLI install and web install call into these. Orchestration owns:
- input gathering (no `server.env` write, no serialization)
- persistence (atomic `server.env` write via existing `envfile::merge_env_file` at `envfile.rs:204-256`)
- legacy entry removal (Unit 6 hook for `FABRO_JWT_*`)
- restart/handoff (web install via `/install/finish`'s existing restart URL contract; CLI install determines its own handoff — for the in-process case, exit cleanly; for daemon mode, respect today's `fabro server stop` + restart pattern)
- Modify: `lib/crates/fabro-cli/src/commands/install.rs:1864, 1213` — route `server.env` writes through the shared orchestration in `fabro-install`. CLI-specific UX (prompts, progress display) remains in `commands/install.rs`; persistence does not.
- Modify: `lib/crates/fabro-server/src/install.rs:1313, 1343-1353, 1380` — server-side `/install/finish` handlers route through the same orchestration. The existing `/install/finish` API contract (returns restart URL; SPA polls until server returns) is preserved exactly — no `restart_required` flag, no API surface change.
- Test: parity tests in `lib/crates/fabro-install/tests/` prove CLI install and web install take the same persistence/removal path with the same outputs given the same inputs.
**Approach:**
- The shared orchestration in `fabro-install` is the single chokepoint for `server.env` writes from install flows. Manual edits to `server.env` outside the orchestration still require operator restart discipline (documented).
- Coordinated install flows MAY write while a server is running — they own restart/handoff. Web install already has the handoff via `/install/finish`; CLI install gets the same primitives.
- Two-phase shape: `gather inputs (no serialization)` → `persist + restart-handoff (atomic)`. Serialization window is sub-second regardless of OAuth duration.
- No PID-comparison logic, no refuse-while-running predicate, no new chokepoint helper in `fabro-config`. The earlier `write_server_env_serialized` proposal is dropped.
**Test scenarios:**
- Parity: identical install inputs produce identical `server.env` contents and identical removed entries via CLI and web paths.
- Happy path (CLI): install on stopped server succeeds; `server.env` updated atomically.
- Happy path (web): install on running server completes via `/install/finish`; restart URL returned; SPA reconnects after restart.
- Legacy removal: install (CLI or web) on a `server.env` containing `FABRO_JWT_*` entries leaves the file without them (Unit 6 hook).
- Negative regression: install-then-start works in the common single-shell CLI sequence.
**Verification:**
- `cargo nextest run -p fabro-install -p fabro-cli` passes including parity tests.
- `grep -rn 'envfile::write_env_entries\|envfile::merge_env_file' lib/crates/fabro-cli/src lib/crates/fabro-server/src` shows `server.env` writes routed through `fabro-install` orchestration; no direct calls outside it.
- `/install/finish` API contract unchanged (existing OpenAPI schema and SPA reconnect tests pass without modification).
---
- [ ] **Unit 6: Remove legacy `FABRO_JWT_*` drift**
**Goal:** Stop generating `FABRO_JWT_PRIVATE_KEY` / `FABRO_JWT_PUBLIC_KEY`, stop reading them in diagnostics, remove operator/docs references that describe them as auth inputs, and actively remove existing entries from `server.env` during install flows. SESSION_SECRET is the sole auth master.
**Requirements:** R9
**Dependencies:** None for code shape; the active-removal hook plugs into Unit 5's shared install orchestration.
**Files:**
- Modify: `lib/crates/fabro-cli/src/commands/install.rs:1853-1855` — delete `generate_jwt_keypair()` call and the corresponding `("FABRO_JWT_PRIVATE_KEY", ...)` / `("FABRO_JWT_PUBLIC_KEY", ...)` entries from `generated_server_env_pairs`.
- Modify: `lib/crates/fabro-server/src/install.rs` (whichever lines mirror the CLI side, surfaced in research) — same deletion on the web install side.
- Modify: `lib/crates/fabro-install/src/lib.rs` (Unit 5's shared orchestration) — add `FABRO_JWT_PRIVATE_KEY` and `FABRO_JWT_PUBLIC_KEY` to a `legacy_keys_to_remove` set; the persistence step writes these as removals on every install run.
- Modify: `lib/crates/fabro-server/src/diagnostics.rs:540, 552` — remove the `state.server_secret("FABRO_JWT_PUBLIC_KEY")` and `state.server_secret("FABRO_JWT_PRIVATE_KEY")` checks. Adjust diagnostics output and tests accordingly.
- Modify: **all** operator-facing references to `FABRO_JWT_PRIVATE_KEY` / `FABRO_JWT_PUBLIC_KEY` across `docs/`, `apps/marketing/`, README, and any in-repo runbook. The criterion is "any mention," not "mention as auth input." Concrete known-stale targets:
- `docs/administration/server-configuration.mdx:297` — remove the "future CLI login flows" reference and any surrounding prose treating these as install/runtime secrets.
- `docs/administration/server-configuration.mdx:331` — remove the row(s) from the "Server authentication" table.
- Implementer must also `grep -rn 'FABRO_JWT_PRIVATE_KEY\|FABRO_JWT_PUBLIC_KEY' docs/ apps/ README*` and either delete or rewrite every hit. Surviving mentions should appear only in (a) the new strategy doc's "removed" section or (b) historical plans under `docs/plans/`.
- Modify: install snapshot tests, server.env fixture files, and any insta snapshots that assert on `FABRO_JWT_*` lines — regenerate.
- Test: install run against a `server.env` pre-seeded with `FABRO_JWT_*` entries — assert they are absent after install completes.
**Approach:**
- Deletion is the entire change. No deprecation period; no compatibility shim. The keys haven't participated in runtime auth since the CLI auth login migration (`2026-04-19-003`).
- Operators upgrading run install once; legacy entries are silently cleaned up. The strategy doc records the cleanup for any operator who notices.
**Test scenarios:**
- Happy path: install on a fresh storage dir produces a `server.env` with no `FABRO_JWT_*` entries.
- Happy path (cleanup): install on a `server.env` containing `FABRO_JWT_PRIVATE_KEY=...` and `FABRO_JWT_PUBLIC_KEY=...` produces an updated `server.env` without those keys, with other entries preserved.
- Diagnostics: `fabro doctor` (or whichever command surfaces diagnostics) does not mention `FABRO_JWT_*`.
**Verification:**
- `grep -rn 'FABRO_JWT_PRIVATE_KEY\|FABRO_JWT_PUBLIC_KEY\|generate_jwt_keypair' lib/crates apps/ docs/ README*` returns hits *only* in (a) the new strategy doc's "removed" section, (b) historical plan files under `docs/plans/`. No hits in operator-facing docs (`docs/administration/`, `docs/quickstart/`, marketing) or production crate code.
- `cargo nextest run -p fabro-cli -p fabro-server` passes with regenerated snapshots.
- Mintlify docs build succeeds with the removed entries (no broken anchors/links from other pages that referenced the JWT-key sections).
---
- [ ] **Unit 7: Strategy doc + workspace-wide clippy enforcement**
**Goal:** Document the design and encode the rules as compile-time enforcement applied to *all* code including tests.
**Requirements:** R10
**Dependencies:** Units 1-6.
**Files:**
- Create: `docs-internal/server-secrets-strategy.md`.
- Modify: `clippy.toml` — add `std::env::set_var` and `std::env::remove_var` to `disallowed-methods` with reason text pointing at the strategy doc. The ban is workspace-wide and applies to tests as well as production code.
- Modify: `lib/crates/fabro-telemetry/src/spawn.rs:73-76` — add `#[expect(clippy::disallowed_methods, reason = "post-fork pre-execvp env mutation; safe because grandchild is single-threaded and about to be replaced via exec")]`.
- Modify: any other production caller surfaced during implementation (the count is small per Unit 1's grep) — same `#[expect]` pattern with documented reason.
- Modify: `CLAUDE.md` (and `AGENTS.md` if separate) — add "Strategy docs" entry pointing at the new doc.
**Approach:**
Strategy doc covers:
- **`ServerSecrets` is the resolved-secrets API.** Sources are process env (12-factor) and `<storage>/server.env` (install/local convenience). Snapshotted at boot via `EnvSource` trait that materializes immediately into owned `HashMap`. Env wins on conflict.
- **The five active server-level secrets** and their consumers (table from this plan). `FABRO_JWT_*` is removed (Unit 6); SESSION_SECRET is the sole auth master via HKDF.
- **Provisioning paths:** CLI install, web install (shared orchestration in `fabro-install`), or platform env. Server start does not auto-generate.
- **Tests must not mutate process env.** Enforced workspace-wide by clippy. Tests inject via construction-time `EnvSource` stubs (e.g. `StubEnv`) for in-process resolution, or via explicit child-process `Command::env` for subprocess simulation.
- **Worker and render-graph env:** `env_clear` + strict fail-closed allowlist. Daemon child inherits parent env (12-factor pattern). New worker env entries require demonstrated need + intentional addition.
- **Install while running:** allowed only through the shared install orchestration which owns restart/handoff. Manual `server.env` edits still require restart discipline.
- **Rotation:** restart required. Live rotation intentionally not supported. Compliance-driven N+1 rotation (overlap windows) is a known limitation tracked as follow-up.
- **Out of scope (with reasons):** Vault + `ProviderCredentials`, `vault_or_env` for run-level credentials (different track), `{{ env.FOO }}` config interpolation, workflow-stage env (Sandbox's job), `validate_api_key` `set_var` smell, Tailscale spawns, `bun --watch-web`.
- **Adding a new server-level secret:** (1) provision via the install orchestration or platform env; (2) consume via `state.server_secret(...)`; (3) do not touch env in any other layer; (4) decide if it joins the startup-critical set (most don't).
- **Adding a new worker env var:** add to the worker list in `spawn_env.rs` with a one-line reason and a failing-without test that proves need.
**Verification:**
- `cargo +nightly-2026-04-14 clippy --workspace --all-targets -- -D warnings` passes.
- Adding a new `std::env::set_var(...)` in any production OR test file produces a clippy denial that names the strategy doc.
## System-Wide Impact
- **Interaction graph:** all active server-level secrets (the five remaining after Unit 6) converge through `state.server_secret(name)` after Unit 1, which returns from the env+file snapshot. `resolve_auth_mode_with_lookup` migrates in the same unit. Legacy `FABRO_JWT_*` is removed across install, diagnostics, and docs (Unit 6).
- **Error propagation:** daemon preflight (`validate_startup`) and foreground startup (`resolve_startup`, called from `serve_command`) both delegate to the same shared validation logic introduced in Unit 4 — the public preflight wrapper is literally `resolve_startup(...).map(|_| ())`, so error messages and accept/reject decisions are identical by construction. Unit 1 provides only the snapshot machinery (`ServerSecrets` + `EnvSource`) that Unit 4's validation consumes. Object-store credential failures still surface from the AWS SDK / object_store crate (unchanged — daemon inherits AWS env).
- **State lifecycle:** `ServerSecrets` snapshot built once at boot from env+file; both immutable for process lifetime. Rotating any in-scope secret requires restart. Install flows MAY update `server.env` while a server is running when they own restart/handoff via the shared install orchestration (Unit 5).
- **Subprocess env:** workers and render-graph processes inherit only an explicit fail-closed allowlist. Daemon child inherits parent env — that's how 12-factor SESSION_SECRET reaches the daemon.
- **API surface:** no public API change. `state.server_secret(...)` signature unchanged. `ServerSecrets::with_env_lookup` removed; replaced by `load(path, &dyn EnvSource)`. `/install/finish` API contract unchanged (no `restart_required` flag, no new fields).
- **Unchanged invariants:** `ProviderCredentials`, Vault REST API, `vault_or_env` for run-level credentials, `{{ env.FOO }}` interpolation, workflow stages (configured by `Sandbox`) all behave exactly as before.
## Risks & Dependencies
| Risk | Mitigation |
|---|---|
| Snapshot semantics surprise: operator changes env after boot, expects the running server to pick it up | Documented behavior. Live rotation is intentionally not supported; restart required. Strategy doc is explicit. |
| Worker list is missing something a workflow stage runner inside Sandbox needs | Worker process itself only does HTTP callbacks — Sandbox handles stage env. Real workflow integration test in Unit 3 verification catches false negatives. |
| Existing deployments without `server.env` AND without env-set secrets fail to start | Intended behavior; error message names both sources. Per repo policy, accept the breakage. |
| Test migration: replacing `std::env::set_var` calls with `EnvSource` stubs touches many files | `EnvSource` + `StubEnv` pattern is mechanical; one helper per pattern. Unit 7's clippy enforcement catches stragglers at compile time so nothing slips through. |
| Shared install orchestration regression breaks CLI/web parity | Parity tests in Unit 5 explicitly assert identical persistence behavior across CLI and web paths. Both flows route through the same `fabro-install` entry points. |
| Automatic `FABRO_JWT_*` removal surprises operators who believed those keys were authoritative | Strategy doc names the cleanup explicitly. Keys haven't participated in runtime auth since `2026-04-19-003`; cleanup is overdue, not novel. Operators upgrading run install once and the cleanup happens silently. |
| Rust 2024 edition migration is a separate effort | Removing `set_var` is a prerequisite; this work doesn't gate on the edition migration. Workspace-wide clippy enforcement (Unit 7) prevents reintroduction including in tests. |
## Documentation / Operational Notes
- **Operator-facing change:** `fabro server start` against an empty env AND uninstalled storage now fails fast naming both sources. Document in install/quickstart.
- **12-factor PaaS deployments (Railway, Heroku, Fly):** unchanged ergonomics — set secrets in platform env, run `fabro server start`. Works without `fabro install` having run on the platform.
- **Container/k8s deployments:** if secrets are mounted as files (e.g. via projected volume into `<storage>/server.env`) or set as env vars (k8s Secret → env), both work.
- **Cloud object-store (S3 / IRSA / ECS):** unchanged — daemon inherits AWS env from parent, object-store layer reads ambient credentials.
- **Install while server running:** install flows may update `server.env` on a running server only when they coordinate restart/handoff via the shared install orchestration (CLI install or web install via `/install/finish`). Manual edits to `server.env` outside the orchestration still require restart discipline.
- **`FABRO_JWT_*` removal:** `FABRO_JWT_PRIVATE_KEY` / `FABRO_JWT_PUBLIC_KEY` are removed from the runtime auth model; SESSION_SECRET is the sole auth master for cookie and JWT derivation. Existing legacy entries are cleaned up automatically on subsequent install flows.
- **Rotation:** edit env (and restart) or edit `server.env` (and restart). Live rotation not supported.
- **Logging:** the fail-fast error appears in operator log aggregators. Pair human-readable message with a structured error code (e.g., `error_code=missing_session_secret`) so log searches match without depending on the exact string.
## Sources & References
- Related plans:
- `docs/plans/2026-04-05-server-canonical-secrets-doctor-repo-plan.md` — architectural anchor
- `docs/plans/2026-04-19-003-feat-cli-auth-login-plan.md` — SESSION_SECRET as HKDF master
- `docs/plans/2026-04-18-001-feat-webhook-strategy-plan.md` — webhook secret precedent
- `docs/plans/2026-04-02-001-feat-server-daemon-management-plan.md` — origin of daemon/foreground split
- Related commits:
- `7cb6c65d5` — "Remove dev-token minting from server start" (precedent for Unit 4)
- Strategy doc precedent: `docs-internal/logging-strategy.md`, `docs-internal/events-strategy.md`

View file

@ -0,0 +1,518 @@
---
title: "refactor: Server-issued per-run JWT for worker subprocess auth"
type: refactor
status: active
date: 2026-04-22
deepened: 2026-04-22
---
# refactor: Server-issued per-run JWT for worker subprocess auth
## Overview
Server mints one per-run JWT (HS256, 72h, claims include `run_id`) at every worker subprocess spawn, passes it to the worker via the `FABRO_WORKER_TOKEN` env var, and every run-scoped route accepts it. Worker stops reading `~/.fabro/auth.json`. Existing artifact-upload-token mechanism is folded into the new worker token (one credential covers all run-scoped routes). End-user auth (dev-token / github) is now strictly orthogonal to worker auth.
## Problem Frame
Workers POST back to the server (events, state, blobs, stage artifacts). Today the worker only authenticates because it inherits the CLI user's OAuth session from `~/.fabro/auth.json` (`fabro-cli/src/server_client.rs:312` → `AuthStore::default()` at `fabro-client/src/auth_store.rs:107`). Side effects:
- (a) worker authenticates *as the user* — events emitted by the worker get user identity, not "system";
- (b) any deployment where the worker doesn't share a home dir with an authenticated CLI user (containerized server, `fabro` system user, remote worker, multi-tenant) silently fails;
- (c) worker auth is implicitly coupled to end-user auth strategy when conceptually independent.
Today's GitHub-only install (`auth.methods = ["github"]`) writes no `FABRO_DEV_TOKEN` and `worker_command` (`server.rs:3836-3845`) injects nothing — the worker has no documented credential at all. Only the home-dir steal makes it work.
The artifact-upload-token mechanism (`server.rs:758`, `server.rs:861`) already proves the right pattern for one route. Generalize it.
## Requirements Trace
- R1. Worker subprocess authenticates to server with a credential the server explicitly issued, not one stolen from the user's home directory.
- R2. Credential is per-run (claim `run_id` must match path `run_id`); cross-run reuse rejected.
- R3. Credential survives server restart up to its natural 72h expiry (no operational events shortening the ceiling).
- R4. Both `start` and `resume` spawn paths re-mint a fresh 72h credential.
- R5. Worker auth works in any deployment topology, including GitHub-only installs with no `~/.fabro/auth.json` on the worker host.
- R6. Worker-emitted events stamped as a system principal (`system:worker`); originator user identity remains discoverable on the run record.
- R7. Run-scoped routes still accept end-user JWTs for non-worker callers (CLI, web UI) — fall-through, not replacement.
## Scope Boundaries
- Out: refresh tokens for workers (decided: hard 72h ceiling per spawn, fresh mint on resume).
- Out: multi-host / remote worker spawning (this plan makes it *possible*, doesn't deliver it).
- Out: changes to end-user auth methods (`ServerAuthMethod::DevToken | Github`).
- Out: any new `RunAuthMethod` variant — worker token bypasses `AuthenticatedSubject` entirely.
- Out: extending `RunSummary` with provenance — originator already on `RunSpec.provenance.subject` and that's enough.
- Out: SSE attach routes (`/runs/{id}/attach`, `/attach`). These are end-user read paths (web UI / CLI tail). The worker is an event *producer*, not consumer; it never calls them. They keep `AuthenticatedService` (user-JWT-only).
- Out: lifecycle endpoints (`/runs/{id}/cancel`, `/pause`, `/unpause`, `/archive`, `/unarchive`, `DELETE /runs/{id}`). The worker has no business invoking these — they remain user-JWT-only and the worker token is explicitly rejected on them (Unit 3).
- Out: any `Display` impl on `Credential::Worker` payload. Plan keeps redacted `Debug` only; do not add `Display`. Token must never be `format!`-able as a side effect.
## Threat Model
State the assumptions explicitly so reviewers and operators can challenge them.
- **Trust boundary:** the server process and any worker subprocess running under the same OS user are mutually trusted. The plan does not provide isolation between workers running as the same UID — a same-UID attacker (or a workflow stage that compromises the worker process) can read `FABRO_WORKER_TOKEN` from `/proc/<pid>/environ` on Linux. Multi-tenant deployments must use per-tenant OS users, per-tenant containers, or per-tenant namespaces for tenant isolation. Cross-run isolation between same-UID workers is NOT a property of this design.
- **`SESSION_SECRET` is the master key.** It signs both user JWTs (via existing `derive_jwt_key` HKDF) and worker JWTs (via new `derive_worker_jwt_key` HKDF, distinct context label). Any leak vector — backup including `server.env`, environment dump in logs, ECS task definition exposure, accidental commit, breadcrumb capture — gives the attacker the ability to mint both kinds of tokens for any user / any run. Operational guidance: store `SESSION_SECRET` in a secrets manager, exclude from logs/Sentry, document a rotation procedure (rotation invalidates ALL outstanding worker tokens AND ALL user sessions — accept as the cost of compromise response). A separate `WORKER_JWT_SECRET` could narrow this — out of scope, called out in Open Questions.
- **Worker token compromise (single run):** an attacker who exfiltrates a single worker token gains read/write on that one run's events/blobs/state for up to 72h or until terminal-status revocation, whichever first. Run-id binding limits cross-run damage. Server-side revocation set narrows the post-completion window (with the caveat in Risks: in-memory revocation does not survive server restart).
- **`SESSION_SECRET` rotation as defense:** rotating `SESSION_SECRET` is the only operator-facing mechanism today to invalidate all outstanding worker tokens. Acceptable for emergency response; not a regular rotation cadence.
## Context & Research
### Relevant Code and Patterns
- `lib/crates/fabro-server/src/server.rs:291-293, 758-826, 828-869` — artifact-upload-token: claims struct, key generation (`OsRng` per boot), mint, "service token first, else user JWT" check (`authorize_artifact_upload`). This is the exact shape of the new worker token, generalized to all run-scoped routes.
- `lib/crates/fabro-server/src/server.rs:3797-3851` — `worker_command`: single spawn site for both `start` and `resume`. Already passes `--artifact-upload-token` via argv. Replace with `--worker-token`.
- `lib/crates/fabro-server/src/server.rs:4760` — `execute_run_subprocess` calls `worker_command` once per spawn; `RunExecutionMode` flows from `start_run` (`server.rs:4385`) and `create_run` (`server.rs:4156`). Single mint site covers both modes.
- `lib/crates/fabro-server/src/auth/keys.rs:41` — `derive_jwt_key(secret: &[u8])` — existing HKDF helper for the user-JWT key. Mirror for worker JWT with distinct context label `b"fabro-worker-jwt-v1"` so worker keys survive server restarts.
- `lib/crates/fabro-cli/src/commands/run/runner.rs:55-125, 239-295` — `__run-worker` entry, `HttpRunStore`, `HttpArtifactUploader`. Only seven server endpoints touched (catalogued below).
- `lib/crates/fabro-cli/src/server_client.rs:51-58, 133-137, 312-335` — `connect_server_target_direct` → `connect_target_api_client_bundle` → `resolve_target_credential` → `AuthStore::default()`. Sole worker caller is `runner.rs:66`. Can be replaced for worker only via a sibling constructor.
- `lib/crates/fabro-client/src/credential.rs:6-30` — `Credential` enum. Add `Worker(String)` variant; `bearer_token()` returns the string.
- `lib/crates/fabro-types/src/run_event/mod.rs:29-81` — `ActorRef`/`ActorKind { User | Agent | System }`. No new variant needed — stamp worker events with `ActorKind::System`.
- `lib/crates/fabro-types/src/run.rs:34-49, 68` — `RunProvenance`/`RunSubjectProvenance` already on `RunSpec`. Originator preserved at run-creation time; no schema change.
- `lib/crates/fabro-workflow/src/event.rs:1340-1493, 2580-2603` — `stored_event_fields`/`to_run_event_at`: where `actor` is set on emitted events. Today most worker events ship `actor: None`; lifecycle events get user actor server-side; agent events get `ActorKind::Agent`. Default-fill at conversion time is the surgical change.
### Worker → server endpoint surface (the surface that needs `authorize_run_scoped`)
| Worker call | HTTP | Path | Server handler | Auth today |
|---|---|---|---|---|
| `client.get_run_state` | GET | `/runs/{id}/state` | `get_run_state` (`server.rs:5179`) | `AuthenticatedService` |
| `client.list_run_events` | GET | `/runs/{id}/events` | `list_run_events` (`server.rs:5249`) | `AuthenticatedService` |
| `client.append_run_event` | POST | `/runs/{id}/events` | `append_run_event` (`server.rs:5199`) | `AuthenticatedService` |
| `client.write_run_blob` | POST | `/runs/{id}/blobs` | `write_run_blob` (`server.rs:5455`) | `AuthenticatedService` |
| `client.read_run_blob` | GET | `/runs/{id}/blobs/{blobId}` | `read_run_blob` (`server.rs:5482`) | `AuthenticatedService` |
| `client.upload_stage_artifact_file` | POST | `/runs/{id}/stages/{stageId}/artifacts` (octet-stream) | `put_stage_artifact` (`server.rs:5941`) | `authorize_artifact_upload` |
| `client.upload_stage_artifact_batch` | POST | same path (multipart) | same handler | same |
### Coordination with concurrent plans
- `docs/plans/2026-04-22-003-refactor-lock-down-server-secrets-plan.md` partially landed: `apply_worker_env` exists at `lib/crates/fabro-server/src/spawn_env.rs:22` and is already invoked from `worker_command` at `server.rs:3835`. The allowlist keeps `HOME`, so the env scrub alone does NOT fix the OAuth-from-disk steal. This plan is the worker-side fix. Per the user's "share" decision, both `apply_worker_env` (server-side) and the new `apply_sandbox_env` (worker-side) move into `fabro-util` so they share a single denylist constant — coordinate with whatever else of `2026-04-22-003` is still in flight.
- `docs/plans/2026-04-19-003-feat-cli-auth-login-plan.md` Unit 8 created `lib/crates/fabro-server/src/auth/jwt.rs` (`Claims`, `issue`, `verify`, `JwtError`) and `auth/keys.rs::derive_jwt_key`. Reuse the HKDF derivation pattern (distinct context label) and the `jsonwebtoken` primitives directly — do not route worker-token claims through user-`JwtSubject`.
- `docs/plans/2026-04-20-001-fix-cli-server-same-host-assumptions-plan.md` deliberately closed the "trust local files because same host" pattern. This plan preserves that closure — no new same-host exceptions; the worker uses an explicitly-passed token.
### Institutional Learnings
- No `docs/solutions/` directory exists. Prior decisions live in `docs/plans/` (see above).
- Artifact-upload-token TTL precedent is 24h (`server.rs:293`). New worker-token TTL is 72h (3× expansion) — justified because worker tokens must survive long human-in-the-loop pauses and there's no in-process refresh.
## Key Technical Decisions
| Decision | Rationale |
|---|---|
| Replace artifact-upload-token entirely; one JWT per run covers all run-scoped routes | Two parallel per-run JWTs is bookkeeping. Run-id binding gives the same blast-radius constraint without a separate scope. |
| Pass JWT to worker via env var `FABRO_WORKER_TOKEN`, NOT CLI arg | Symmetric with existing `FABRO_DEV_TOKEN` re-injection model (`worker_command` at `server.rs:3836-3845`). Plays naturally with the `env_clear` + explicit re-injection model in `2026-04-22-003`. Avoids token strings in `ps` output. |
| HS256, key derived from `SESSION_SECRET` via HKDF (context `b"fabro-worker-jwt-v1"`) | Workers must survive server restarts up to natural 72h expiry. `OsRng`-per-boot (artifact-upload precedent) defeats the long TTL. Distinct context label keeps it isolated from the user-JWT key. Operator rotation of `SESSION_SECRET` invalidates outstanding worker tokens — accepted. |
| 72h TTL, no refresh | Decided by user. Long-paused runs need a generous outer ceiling. Resume re-mints. Workers running > 72h continuously fail loudly — acceptable outer bound. |
| Per-run `run_id` claim, path-vs-claim check | Mirrors `maybe_authorize_artifact_upload_token`. Cross-run reuse → 403. |
| Add `Credential::Worker(String)` variant (not reuse `DevToken`) | Debug printing stays accurate; type lets us prove "worker code only constructs `Worker`" structurally. |
| New worker-only client constructor `connect_server_target_with_bearer(target, token)`, bypasses `AuthStore`/`OAuthSession` entirely | Worker should never read user OAuth. Surgical to fix at the worker callsite (one caller, `runner.rs:66`) rather than gating `resolve_target_credential` with a "are you a worker" flag. |
| Worker default-fills `actor` on emitted events to `ActorRef { kind: System, id: Some("worker"), display: Some("system:worker") }` only when variant doesn't already set it | Lifecycle events keep user actor (set server-side at the lifecycle endpoint, not by worker). Agent events keep `ActorKind::Agent`. Surgical change in `to_run_event_at`. |
| `authorize_run_scoped(parts, state, run_id)` is the single helper for all worker-touched routes | Replaces five `_auth: AuthenticatedService` extractors and the existing `authorize_artifact_upload`. One helper, one fall-through behavior. |
| Delete artifact-upload-token mechanism atomically (no transition period) | Single-server, single-codebase change. No external consumers of the old token shape. In-flight workers survive deploy via user-JWT fall-through (they hold OAuth from `auth.json`); only newly-spawned post-deploy workers exercise the new contract — those start cleanly. Atomic swap. |
| Server-only secrets (`SESSION_SECRET`, `FABRO_JWT_PRIVATE_KEY`, `GITHUB_APP_*`) must NOT leak to the worker process | **Already structurally mitigated**: `apply_worker_env` at `spawn_env.rs:22` does `env_clear` + an 8-name allowlist that excludes all of these. `worker_allowlist_is_fail_closed` test (`spawn_env.rs:80-113`) asserts `SESSION_SECRET` doesn't leak. Without this protection, an inherited `SESSION_SECRET` would let the worker derive `WorkerTokenKeys` locally and mint tokens for any run — defeating per-run binding. Verification: extend the existing fail-closed test to also assert `FABRO_JWT_PRIVATE_KEY`, `FABRO_JWT_PUBLIC_KEY`, `GITHUB_APP_PRIVATE_KEY`, `GITHUB_APP_CLIENT_SECRET`, `GITHUB_APP_WEBHOOK_SECRET` don't leak. |
| Server-side per-run revocation set populated when run reaches terminal status | 72h compromise window is severe under the multi-tenant threat model. In-memory `DashSet<RunId>` checked in `authorize_worker_token` cuts post-completion blast radius to zero without changing the TTL ceiling. ~10 lines. |
| Worker-side env scrubbing via single chokepoint helper, not per-call `env_remove` | Per-call rots: every new spawn site forgets. Codebase already uses chokepoint pattern in `LocalSandbox::execute` (allowlist + suffix filter). Mirror with `fabro-sandbox/src/spawn_env.rs::apply_sandbox_env(&mut Command)`, route remaining unscrubbed sites through it. `LocalSandbox::execute`'s incidental `_token`-suffix filter already catches `FABRO_WORKER_TOKEN` today — make that explicit (denylist entry, not coincidence). |
| Worker exits with distinct non-zero code (`78` / `EX_NOPERM`) on missing/rejected `FABRO_WORKER_TOKEN`; emits `tracing::error!(target = "worker_auth", ...)` | Lets operators distinguish auth failures from generic crashes during deploy windows. Today all worker exit codes look identical at the server. |
### Worker-token vs artifact-upload-token (delta)
| Property | Artifact-upload-token (today) | Worker-token (new) |
|---|---|---|
| Coverage | One route (`/runs/{id}/stages/{stageId}/artifacts`) | All run-scoped routes the worker hits |
| TTL | 24h | 72h |
| Signing key | `OsRng` at server boot, in-memory only | HKDF from `SESSION_SECRET`, context `b"fabro-worker-jwt-v1"` |
| Survives server restart | No | Yes (up to natural expiry) |
| Issuer string | `"fabro-server-artifact-upload"` | `"fabro-server-worker"` |
| Scope claim | `"stage_artifacts:upload"` | `"run:worker"` |
| Passed to worker | `--artifact-upload-token` argv | `FABRO_WORKER_TOKEN` env var |
| Worker uses it as | Per-call method arg on the client | Client's bearer for every server call |
## Open Questions
### Resolved During Planning
- TTL: 72h (user). Refresh: none in-process; fresh mint at every spawn (start AND resume).
- Key derivation: HKDF from `SESSION_SECRET` with context `b"fabro-worker-jwt-v1"` (user, this session).
- Replace artifact-upload-token entirely vs. keep both: replace (user, this session).
- New `RunAuthMethod::Worker` variant: no — worker token bypasses `AuthenticatedSubject` entirely.
- Stamp worker events server-side vs. worker-side: worker-side, in a dedicated `SystemWorkerActorSink` wrapper inside the worker's `RunEventSink::fanout` chain (NOT in the shared `to_run_event_at` converter, which is also called server-side). Keeps the converter pure (passthrough semantics preserved) and avoids mis-stamping server-emitted events.
### Deferred to Implementation
- Exact module name for new server-side worker-token machinery — likely `fabro-server/src/worker_token.rs`, decide at implementation.
- Exact name of the new `Credential::Worker` variant on `fabro-client` — `Worker` likely, confirm against existing naming when implementing.
- Whether to enforce "worker module never imports `AuthStore`" structurally (clippy `disallowed_types` on the `commands::run` module). Nice-to-have; defer.
- Mechanism for the compile-time "no `Display` for `Credential::Worker`" guard — `static_assertions::assert_not_impl_any!` is the natural fit, but choose at implementation time based on whether the crate already pulls that dep.
- Reaper for the in-memory revocation set — defer; entries are bounded by terminal-state runs and process lifetime is the natural reaper. Add only if real workloads show unbounded growth.
- Core dump disable (`setrlimit(RLIMIT_CORE, 0)`) on the worker process to prevent token capture in crash dumps. Same-UID attacker assumption holds today; nice-to-have, defer.
- Centralizing terminal-status writes through a single `mark_run_terminal(run_id, status)` chokepoint that both writes the status AND inserts into the revocation set (instead of grep-and-wire-by-hand). Cleaner, more auditable; defer pending implementation discovery of how dispersed the current writes are.
- Whether `WORKER_JWT_SECRET` should be a separate operator-rotated secret (narrower than `SESSION_SECRET`) — defer; the SESSION_SECRET-as-master-key design is acceptable for the current threat model per Threat Model section, but the gap is documented for follow-up.
### Decisions surfaced by review (resolved)
- **Revocation persistence:** **document the gap, accept for now (option c).** In-memory revocation set, lost on restart. Combined with HKDF-derived signing key surviving restart, this leaves a known re-enablement window for tokens of completed runs across restarts. Documented in Risks and System-Wide Impact. Follow-up plan can add persistence if multi-tenant production demand surfaces it.
- **Multi-token-per-run on rapid pause/resume:** **accept and document (option c).** Each resume mints a fresh 72h token; prior tokens stay valid up to natural `exp` or run terminal status. Multiplied compromise window is bounded by run-id (still scoped to one run). Documented in Risks. Follow-up plan can add per-spawn nonce if it becomes a real problem.
- **Clippy lint extension to `tokio::process::Command::new`:** **no.** Do not extend the lint. Tokio-Command sites rely on code-review discipline to use `apply_sandbox_env`. Plan keeps `clippy.toml`'s existing `std::process::Command::new` denial only.
- **Share `apply_sandbox_env` / `apply_worker_env`:** **share.** Place the helper in `fabro-util` (or a new dedicated crate if `fabro-util` becomes too dumping-ground), expose two thin wrappers — `fabro_util::process::apply_worker_env` (server-side, used by `worker_command`) and `fabro_util::process::apply_sandbox_env` (worker-side, used by all worker-reachable spawn sites). Both call the same underlying scrub function with the same denylist constant. Coordinate with `2026-04-22-003` Unit 3 — whichever plan lands first creates the helper; the other plan adds the second wrapper.
- **Line-number drift in plan references:** addressed in this revision pass.
## High-Level Technical Design
> *This illustrates the intended approach and is directional guidance for review, not implementation specification. The implementing agent should treat it as context, not code to reproduce.*
```mermaid
sequenceDiagram
participant Op as Operator
participant Srv as fabro-server
participant W as worker subprocess
participant Child as sandbox/agent child
Op->>Srv: start (SESSION_SECRET in env)
Note over Srv: HKDF-derive WorkerTokenKeys<br/>context "fabro-worker-jwt-v1"
Srv->>Srv: spawn scheduled (start or resume)
Note over Srv: issue_worker_token(run_id)<br/>HS256 + claims{run_id, scope, 72h}
Srv->>W: spawn with env_clear + FABRO_WORKER_TOKEN injected<br/>(SESSION_SECRET / JWT_PRIVATE_KEY / GITHUB_* removed)
W->>W: read FABRO_WORKER_TOKEN from env<br/>build Client with Credential::Worker(token)<br/>(no AuthStore, no OAuthSession)
W->>Srv: POST /runs/{id}/events (Authorization: Bearer ...)
Srv->>Srv: authorize_run_scoped:<br/>1) try worker token (run_id match + revocation check)<br/>2) else fall through to user-JWT extractor
Srv-->>W: 200 OK
W->>Child: spawn (apply_sandbox_env: scrub FABRO_WORKER_TOKEN)
Child-->>W: result (no token in env)
Note over W,Srv: ... run completes ...
W->>Srv: POST /runs/{id}/events (terminal)
Srv->>Srv: insert run_id into revocation set
Note over Srv: server restart: HKDF re-derives same key,<br/>outstanding tokens still verify (up to natural exp)
```
## Implementation Units
- [ ] **Unit 1: Worker JWT primitives (claims, keys, mint)**
**Goal:** Server can mint a per-run worker JWT signed with a key derived from `SESSION_SECRET`. No callers yet.
**Requirements:** R2, R3.
**Dependencies:** None.
**Files:**
- Create: `lib/crates/fabro-server/src/worker_token.rs`
- Modify: `lib/crates/fabro-server/src/auth/keys.rs` (add `derive_worker_jwt_key`)
- Modify: `lib/crates/fabro-server/src/lib.rs` (module declaration)
- Modify: `lib/crates/fabro-server/src/server.rs` (`AppState` field for `WorkerTokenKeys`, construction in `build_app_state`)
- Test: `lib/crates/fabro-server/src/worker_token.rs` (unit tests inline)
**Approach:**
- Constants: `WORKER_TOKEN_ISSUER = "fabro-server-worker"`, `WORKER_TOKEN_SCOPE = "run:worker"`, `WORKER_TOKEN_TTL_SECS = 72 * 60 * 60`.
- `WorkerTokenClaims { iss, iat, exp, run_id, scope, jti }` — mirrors `ArtifactUploadClaims` plus a `jti` (random 128-bit hex) claim. `jti` enables per-token audit correlation in the future and lets logs distinguish two tokens minted for the same run (start vs resume) without leaking the token itself.
- `WorkerTokenKeys { encoding, decoding, validation }` — mirrors `ArtifactUploadTokenKeys`. Built from a 32-byte HKDF output keyed by `SESSION_SECRET`, context `b"fabro-worker-jwt-v1"`.
- `pub fn issue_worker_token(keys: &WorkerTokenKeys, run_id: &RunId) -> Result<String, ApiError>` — `jsonwebtoken::encode` with HS256.
- Add `worker_tokens: WorkerTokenKeys` field on `AppState` next to `artifact_upload_tokens` (keep both during this unit; the artifact field is deleted in Unit 3).
- `derive_worker_jwt_key(secret: &[u8]) -> [u8; 32]` in `auth/keys.rs` — same HKDF construction as `derive_jwt_key`, distinct `info` parameter.
**Patterns to follow:**
- `server.rs:291-293, 322-329, 758-780, 813-826` (artifact-upload-token, end-to-end).
- `auth/keys.rs:41` (`derive_jwt_key` HKDF construction).
**Test scenarios:**
- Happy path: `issue_worker_token` produces a token; `jsonwebtoken::decode` with the same `WorkerTokenKeys` returns the expected `WorkerTokenClaims` (iss, scope, run_id).
- Edge case: token issued with `WorkerTokenKeys` derived from secret S verifies under a *fresh* `WorkerTokenKeys` derived from the same S — proves restart survival (R3).
- Edge case: token issued under secret S1 fails to verify under keys derived from secret S2 (rotation invalidation).
- Edge case: derivation context label `b"fabro-worker-jwt-v1"` produces a key materially different from `derive_jwt_key(secret)` (no accidental cross-acceptance with user JWTs).
**Verification:**
- All worker-token unit tests pass.
- `cargo build -p fabro-server` succeeds.
- No production callers of new symbols yet — Unit 1 is purely additive infrastructure.
---
- [ ] **Unit 2: Server `authorize_run_scoped` helper (with revocation) + client `Credential::Worker` variant**
**Goal:** Single server-side helper accepts worker token (run-id-bound, not revoked) OR falls back to user JWT. Client crate gains a typed worker credential with no `Display` and redacted `Debug`.
**Requirements:** R2, R7.
**Dependencies:** Unit 1.
**Files:**
- Modify: `lib/crates/fabro-server/src/worker_token.rs` (add `authorize_worker_token`, `authorize_run_scoped`, `RevokedRunSet`)
- Modify: `lib/crates/fabro-server/src/server.rs` (`AppState` field for `revoked_runs: RevokedRunSet`; populate in run-terminal-status sites)
- Modify: `lib/crates/fabro-server/src/lib.rs` (re-export `authorize_run_scoped` if needed)
- Modify: `lib/crates/fabro-client/src/credential.rs` (add `Worker(String)` variant — no `Display`)
- Test: `lib/crates/fabro-server/src/worker_token.rs` (authorize helper tests inline)
- Test: `lib/crates/fabro-client/src/credential.rs` (Debug + bearer_token tests inline)
**Approach:**
- `authorize_worker_token(parts: &Parts, state: &AppState, run_id: &RunId) -> Result<bool, ApiError>` — mirror `maybe_authorize_artifact_upload_token` (`server.rs:828-859`): on decode fail return `Ok(false)`; on scope mismatch / run_id mismatch / `state.revoked_runs.contains(run_id)` return `Err(ApiError::forbidden())`; on success `Ok(true)`.
- `pub fn authorize_run_scoped(parts: &Parts, state: &AppState, run_id: &RunId) -> Result<(), ApiError>` — mirror `authorize_artifact_upload` (`server.rs:861-869`): try worker token first, else `authenticate_service_parts(parts)`.
- `RevokedRunSet` — wraps `Arc<DashSet<RunId>>` (or equivalent concurrent set). Bounded by terminal-state runs; entries can age out via a periodic reaper or simply persist for the process lifetime (simpler; bounded memory unless workload is pathological — defer reaper to a follow-up).
- Wire revocation insertion at every site that transitions a run to a terminal status (find via grep for `RunStatus::Succeeded | Failed | Cancelled` writes in `fabro-server` and `fabro-workflow`-reduced events on the server side).
- `Credential::Worker(String)` — `bearer_token() -> &str` returns the string; `Debug` prints `Credential::Worker(<redacted>)`. **Do NOT implement `Display`.** Compile-time hardening.
- **Audit logging at authorize time:** on successful worker-token auth, emit `tracing::info!(target = "worker_auth", run_id = %run_id, jti = %claims.jti, "worker token accepted")`. On rejection (wrong run_id, wrong scope, revoked, bad signature), emit `tracing::warn!(target = "worker_auth", reason = %rejection_reason, "worker token rejected")`. Never log the token string itself — only `jti`. Supports incident response: operators can correlate accepted/rejected tokens to specific runs without a token database.
**Patterns to follow:**
- `server.rs:828-869` (artifact-upload-token authorize pattern).
- `fabro-client/src/credential.rs:6-40` (existing `DevToken`/`OAuth` variants).
**Test scenarios:**
- Happy path (server): valid worker token for path run_id → `authorize_run_scoped` returns `Ok(())`.
- Error path (server): worker token with `claims.run_id != path.run_id` → 403.
- Error path (server): worker token with wrong scope → 403.
- Error path (server): expired worker token → falls through to user-JWT path (returns false from worker check), and user-JWT extractor then rejects → 401.
- Error path (server): bad signature → falls through to user-JWT path; user-JWT also rejects → 401.
- Error path (server): `alg=none` JWT → rejected (validation requires HS256).
- Error path (server, revocation): valid token but `run_id` in `revoked_runs` set → 403. Insert run_id into set, then re-attempt with a fresh-issued token for the same run → 403.
- Integration (server): no `Authorization` header → falls through to user-JWT extractor → 401 (no implicit acceptance).
- Integration (server): valid user JWT, no worker token → user-JWT path accepts (R7).
- Happy path (client): `Credential::Worker(s).bearer_token()` returns `s`.
- Edge case (client): `Debug` impl prints `Credential::Worker(<redacted>)` — token string never appears in debug output.
- Compile-time guard (client): assert `Credential::Worker` does not implement `Display` (e.g. via a `static_assertions::assert_not_impl_any!` or equivalent — defer exact mechanism to implementation, but the test must exist).
**Verification:**
- All `authorize_run_scoped` tests pass with both branches and revocation exercised.
- `cargo nextest run -p fabro-server -p fabro-client` succeeds.
---
- [ ] **Unit 3: Wire all run-scoped routes through `authorize_run_scoped`; replace artifact-upload-token at the spawn**
**Goal:** Every endpoint the worker hits accepts the worker token. Server spawn passes `--worker-token`. Old artifact-upload-token machinery deleted atomically.
**Requirements:** R1, R2, R4, R7.
**Dependencies:** Unit 1, Unit 2.
**Files:**
- Modify: `lib/crates/fabro-server/src/server.rs`
- Replace `_auth: AuthenticatedService` with explicit `authorize_run_scoped(&parts, &state, &id)?` call inside handler bodies for: `get_run_state` (5162), `list_run_events` (5232), `append_run_event` (5182), `write_run_blob` (5438), `read_run_blob` (5465). Add `parts: Parts` extractor where needed.
- `put_stage_artifact` (5924): swap `authorize_artifact_upload` → `authorize_run_scoped`.
- `worker_command` (3797-3851): replace `state.issue_artifact_upload_token` → `state.issue_worker_token`. Drop the `--artifact-upload-token <jwt>` arg entirely. Set `cmd.env("FABRO_WORKER_TOKEN", token)` unconditionally (always, regardless of auth method). Also `cmd.env_remove("FABRO_WORKER_TOKEN")` before re-injection (defense against parent-env leakage). Delete the conditional `cmd.env("FABRO_DEV_TOKEN", token)` block (3836-3845) and the preceding `cmd.env_remove("FABRO_DEV_TOKEN")` (keep an unconditional `env_remove` if Unit 3 of `2026-04-22-003` hasn't landed yet — coordinate; once `apply_worker_env` lands, `FABRO_WORKER_TOKEN` becomes the only authority-bearing re-injection).
- `worker_command`: ensure `SESSION_SECRET`, `FABRO_JWT_PRIVATE_KEY`, `FABRO_JWT_PUBLIC_KEY`, `GITHUB_APP_PRIVATE_KEY`, `GITHUB_APP_CLIENT_SECRET`, `GITHUB_APP_WEBHOOK_SECRET` are all stripped from the worker env. The existing `apply_worker_env` at `spawn_env.rs:22` (already wired in at `server.rs:3835`) does `env_clear` + allowlist, so as long as the allowlist excludes these names (verify), they're already structurally absent. Add explicit `env_remove`s as belt-and-suspenders only if the audit reveals any leak. **Critical context**: without these strips, an inherited `SESSION_SECRET` lets the worker derive `WorkerTokenKeys` locally and mint tokens for any run. Verify allowlist excludes them — this is the highest-impact verification in the plan.
- **Lifecycle/admin routes**: enumerate the routes the worker token must NOT reach and confirm none of them call `authorize_run_scoped` (they keep `_auth: AuthenticatedSubject` / `AuthenticatedService` directly): `cancel_run`, `pause_run`, `unpause_run`, `archive_run`, `unarchive_run`, `delete_run`, `submit_answer` (interview answers are end-user actions), `start_run`/`create_run`. Add a one-line code comment at each such handler: "// Worker token intentionally not accepted; this is a user/admin action."
- Delete: `ARTIFACT_UPLOAD_TOKEN_*` constants (290-292), `ArtifactUploadClaims` (322-329), `ArtifactUploadTokenKeys` (315-320), `artifact_upload_token_keys` (813-826), `AppState::issue_artifact_upload_token` (758-780), `AppState::artifact_upload_tokens` field (568), `maybe_authorize_artifact_upload_token` (827-858), `authorize_artifact_upload` (860-869).
- Test: existing tests in `lib/crates/fabro-server/src/server.rs` test module (rename / replace `worker_command_injects_dev_token_only_when_enabled` at 7911).
**Approach:**
- All handler signature changes are mechanical: `_auth: AuthenticatedService` → take `parts: Parts` (or use `axum::extract::Request` + `into_parts`), call `authorize_run_scoped(&parts, &state, &id)?` near the top of the body.
- Atomic swap, no transition period — cargo + tests catch any missed callsite.
- **Pre-implementation audit (must run BEFORE coding starts):**
1. Grep all `client.*` and `api.*` callsites under `lib/crates/fabro-cli/src/commands/run/` (and any helpers it transitively uses) → confirm each resolves to a server handler in the worker-touched table above. Adds rows for any newly-discovered handlers (e.g. heartbeat, status report) and wires them through `authorize_run_scoped`.
2. Grep all internal repos and `docs/api-reference/fabro-api.yaml` for `artifact-upload-token`, `artifact_upload_token`, `--artifact-upload-token`, and `stage_artifacts:upload`. Document zero hits in the PR description before merging. If any external consumer exists, add a deprecation cycle instead of atomic delete.
- **Structural rule:** `authorize_run_scoped` is invoked ONLY from handlers whose path contains `{id}` (`RunId`). Never invoke from non-run-scoped routes (list endpoints, admin endpoints). Verify by grep: every `authorize_run_scoped` callsite must be preceded by a path-extracted `RunId`.
**Patterns to follow:**
- `put_stage_artifact` handler (`server.rs:5941`) is the existing model: takes `parts: Parts`, calls `authorize_artifact_upload(&parts, &state, &id)?`.
**Test scenarios:**
- Happy path: each of the 5 newly-wired routes accepts a worker token whose `claims.run_id` matches the path `id` → expected response.
- Error path: each route with worker token whose `claims.run_id` ≠ path `id` → 403.
- Integration: each route with valid user JWT and no worker token → still works (R7 fall-through).
- Replacement test for `worker_command_injects_dev_token_only_when_enabled`: rename to `worker_command_always_sets_worker_token_env`. Build `worker_command` for both `methods=["github"]` and `methods=["dev-token"]` settings; assert `FABRO_WORKER_TOKEN` env is set to a valid token in BOTH cases (no longer conditional on auth method). Assert `FABRO_DEV_TOKEN` env is NOT set in either case. Assert no `--artifact-upload-token` or `--worker-token` arg appears in argv (env-only).
- Edge case (server secrets stripped): extend the existing `worker_allowlist_is_fail_closed` test (`spawn_env.rs:80-113`, currently asserts `SESSION_SECRET` and `MY_API_KEY` don't leak) to ALSO assert `FABRO_JWT_PRIVATE_KEY`, `FABRO_JWT_PUBLIC_KEY`, `GITHUB_APP_PRIVATE_KEY`, `GITHUB_APP_CLIENT_SECRET`, `GITHUB_APP_WEBHOOK_SECRET` don't leak. Critical: prevents the worker-mints-any-token escalation. The existing structure (env_clear + allowlist) already provides this; the test just needs to enumerate the additional names so future allowlist edits can't silently add them back.
- Negative path (route enumeration): for each lifecycle/admin route (`cancel_run`, `pause_run`, `unpause_run`, `archive_run`, `unarchive_run`, `delete_run`, `submit_answer`), assert that presenting a valid worker token for the same `run_id` is rejected (handler still requires user JWT). Use the existing JWT-extractor test pattern (`jwt_auth.rs::valid_jwt_bearer_authenticates_with_identity` shape) as a model.
- Negative path (SSE attach + non-run-scoped): assert a worker token is rejected on `/runs/{id}/attach`, `/attach`, `GET /runs` (list), `GET /usage`, `GET /sessions`, and any other non-run-scoped route discovered by the structural-rule grep above. Documents that worker authority does not bleed into list/admin surfaces.
- Audit logging: assert that successful auth emits a `target = "worker_auth"` info span with `run_id` and `jti`; assert that rejection emits a `warn` span with `reason`. Use a tracing test subscriber. Confirms incident-response observability.
- Edge case: assert deleted symbols (`ArtifactUploadClaims`, `authorize_artifact_upload`, etc.) no longer exist — covered implicitly by `cargo build`.
**Verification:**
- `cargo build --workspace` succeeds (no references to deleted artifact-upload-token symbols).
- `cargo nextest run -p fabro-server` passes.
- Server test for `worker_command` confirms `--worker-token` always present, `FABRO_DEV_TOKEN` never set.
---
- [ ] **Unit 4: Worker (CLI) — use injected worker token from env, stop reading `AuthStore`**
**Goal:** Worker subprocess reads its credential from `FABRO_WORKER_TOKEN` env at startup, uses it as its sole bearer for every server call, never constructs `AuthStore::default()`. Artifact uploads use the same client credential.
**Requirements:** R1, R5.
**Dependencies:** Unit 2 (`Credential::Worker` variant), Unit 3 (server sets `FABRO_WORKER_TOKEN` env on the worker subprocess).
**Files:**
- Modify: `lib/crates/fabro-cli/src/args.rs:815` — DELETE the `artifact_upload_token: Option<String>` field on `RunWorkerArgs`. Do NOT add a replacement clap arg — the worker reads from env directly.
- Modify: `lib/crates/fabro-cli/src/commands/run/mod.rs:88-100` — drop the `artifact_upload_token` plumbing on the dispatch path.
- Modify: `lib/crates/fabro-cli/src/server_client.rs` — add `pub(crate) async fn connect_server_target_with_bearer(target: &ServerTarget, bearer: &str) -> Result<Client>`. Builds `Client` with `.credential(Credential::Worker(bearer.to_owned()))`, no `oauth_session`, no `resolve_target_credential` call, no `AuthStore` access.
- Modify: `lib/crates/fabro-cli/src/commands/run/runner.rs`
- At top of `execute`: read `FABRO_WORKER_TOKEN` from env via `std::env::var` and **immediately wrap in a `WorkerToken(SecretString)` newtype** (using `secrecy::SecretString` or a local equivalent: redacted `Debug`, no `Display`, no `Deref<Target=String>`). Never let the raw `String` escape the read site. Validate non-empty; bail with a clear error mentioning `FABRO_WORKER_TOKEN` if missing or empty. Drop `artifact_upload_token` from `execute`'s signature.
- Modify: `lib/crates/fabro-util/src/exit.rs` — add `ExitClass::WorkerAuthRequired` mapping to integer `78` (`EX_NOPERM`). The runner's missing-token error is classified as this variant via `.classify(ExitClass::WorkerAuthRequired)` so the existing `exit_code_for` mapper produces 78 cleanly. Avoids a divergent `std::process::exit` path that would bypass the runner's tracing/JSON-output flow.
- Line 66: replace `connect_server_target_direct(&server)` with `connect_server_target_with_bearer(&target, &worker_token)`.
- Lines 76-80: build the artifact uploader without a separate `bearer_token` field — the client already carries the credential.
- Delete `MissingArtifactUploadTokenUploader` (298-315) and the `match artifact_upload_token { Some/None }` fork (244-251); always construct `HttpArtifactUploader`.
- `HttpArtifactUploader`: drop the `bearer_token: String` field; `upload_stage_artifacts` no longer threads a per-call bearer.
- Modify: `lib/crates/fabro-client/src/client.rs:1053, 1091` — `upload_stage_artifact_*` no longer takes `bearer_token` parameter; uses the client's credential. (Confirm signature change is workable; if the client API forces per-call bearer for legacy reasons, leave the parameter and pass `client.credential().bearer_token()` from the worker side instead.)
- Modify: `runner::execute` exits with code `78` (`EX_NOPERM`) via `ExitClass::WorkerAuthRequired` when `FABRO_WORKER_TOKEN` is missing or invalid; emits `tracing::error!(target = "worker_auth", ...)` so operators can grep for it. Distinct from generic crash exits.
- Compile-time guards on `WorkerToken`: `static_assertions::assert_not_impl_any!(WorkerToken: Display, std::fmt::Display)` (or equivalent) — token cannot be `format!`-ed by accident. Test asserts `format!("{:?}", worker_token)` contains no substring of the actual token.
**Child-process env scrub (chokepoint pattern, NOT per-call):**
The codebase already has a chokepoint at `LocalSandbox::execute` (`lib/crates/fabro-sandbox/src/local.rs:223-242`): `env_clear` + `should_filter_env_var` heuristic with safelist + suffix denylist. `FABRO_WORKER_TOKEN` is incidentally caught by the `_token` suffix filter today — make this explicit (denylist entry, not coincidence). Then add a sibling helper for the unscrubbed sites.
- Modify: `lib/crates/fabro-sandbox/src/local.rs:43-66` — add `"FABRO_WORKER_TOKEN"` (and `SESSION_SECRET`, `FABRO_JWT_PRIVATE_KEY`, `FABRO_JWT_PUBLIC_KEY`, `GITHUB_APP_PRIVATE_KEY`, `GITHUB_APP_CLIENT_SECRET`, `GITHUB_APP_WEBHOOK_SECRET`) to an explicit denylist alongside the suffix heuristic. Comment why: future rename like `FABRO_WORKER_AUTH` would silently leak under the suffix heuristic alone.
- Create or extend: `lib/crates/fabro-util/src/process.rs` (new module) exposing both `pub fn apply_worker_env(cmd: &mut Command)` (server-side, used by `worker_command`) AND `pub fn apply_sandbox_env(cmd: &mut Command)` (worker-side, used by spawn sites below). Both call the same underlying scrub against the same denylist constant. **Shared with `2026-04-22-003`** — whichever plan lands first creates the module; the other adds the second wrapper. The denylist constant lives here too, single source of truth for "secret env names that must not leak across the worker boundary." Both `tokio::process::Command` and `std::process::Command` supported (the function takes a trait or has two overloads).
- Modify: route the following unscrubbed worker-reachable spawn sites through `apply_sandbox_env`:
- `lib/crates/fabro-sandbox/src/local.rs:334, 355, 504` — `rg`, `grep`, `uname`.
- `lib/crates/fabro-workflow/src/git.rs:35` (`git_cmd` helper — single chokepoint for git lines `342, 347, 414`).
- `lib/crates/fabro-workflow/src/transforms/file_inlining.rs:124, 129` — git invocations.
- `lib/crates/fabro-workflow/src/sandbox_git.rs` — production git sites (skip `#[cfg(test)]` blocks at 800-1053).
- `lib/crates/fabro-workflow/src/pipeline/initialize.rs:417` — devcontainer `initializeCommand` (`sh -c`).
- `lib/crates/fabro-devcontainer/src/features.rs:67, 80, 113, 150, 199` — `which`, `brew`, `sh -c curl|tar`, `tar`, `oras pull`.
- `lib/crates/fabro-mcp/src/client.rs:47` — stdio MCP server subprocess.
- `lib/crates/fabro-github/src/lib.rs:129` — `gh auth token`.
- `lib/crates/fabro-hooks/src/executor.rs:187` — non-sandbox hook `sh -c`.
- Note: `fabro-agent` constructs zero `Command`s (all exec routes through `Sandbox::exec_command`). Fixing `LocalSandbox` covers the entire agent surface.
- Note: existing `clippy.toml` denies `std::process::Command::new` workspace-wide but does NOT deny `tokio::process::Command::new`. Most worker-reachable spawn sites use tokio Command, so the lint nudge is partial — tokio sites rely on code-review discipline to use `apply_sandbox_env`. Decision: do not extend the lint here (out of scope; bigger workspace-wide audit). Code review enforces tokio-side hygiene.
**Approach:**
- `connect_server_target_with_bearer` is the smallest possible surface: it skips the `AuthStore`/`OAuthSession` machinery entirely. The user-facing `connect_server_target` and `connect_server_with_settings` are unchanged.
- The new constructor is the *only* path the worker takes; verify by grep that `commands::run::runner` and `commands::run::mod` are the only modules importing it.
**Patterns to follow:**
- `server_client.rs:80-110` (`connect_managed_unix_socket_api_client_bundle`) for the client-builder shape; new constructor is a stripped-down version.
**Test scenarios:**
- Happy path: `connect_server_target_with_bearer` builds a `Client` whose outgoing requests carry `Authorization: Bearer <worker-token>`.
- Edge case: `connect_server_target_with_bearer` does NOT call `AuthStore::default()` — verify by injecting a `FABRO_AUTH_FILE=/nonexistent` env override and confirming construction succeeds (the helper must not even attempt to read the file).
- Edge case: `connect_server_target_with_bearer` does NOT install an `OAuthSession` (no refresh attempts on 401).
- Integration: `runner::execute` with `FABRO_WORKER_TOKEN=<jwt>` set in env POSTs an event using only the injected token; no fallback to `auth.json`.
- Error path: `runner::execute` invoked without `FABRO_WORKER_TOKEN` in env returns a clear error mentioning the variable name; does NOT silently fall back to user OAuth.
- Edge case: `RunWorkerArgs` no longer has `artifact_upload_token` field — `cargo build` confirms.
- Integration (env hygiene, sandbox path): a workflow stage Bash command `env | grep -E "FABRO_WORKER_TOKEN|SESSION_SECRET|FABRO_JWT_PRIVATE_KEY"` prints empty output. Covers `LocalSandbox::execute` denylist correctness.
- Integration (env hygiene, non-sandbox path): a unit test on `apply_sandbox_env` builds a `Command`, sets the denylist names in the parent test env, calls the helper, then asserts each name is `Removed` in the resulting `Command`'s env overrides.
- Error path (exit code): `runner::execute` invoked with no `FABRO_WORKER_TOKEN` exits with status 78 and emits a `target = "worker_auth"` tracing line. Verify via process spawn in a small integration test.
**Verification:**
- `cargo build --workspace` succeeds.
- `cargo nextest run -p fabro-cli` passes.
- grep for `AuthStore` from `commands/run/` returns no hits.
---
- [ ] **Unit 5: Stamp `system:worker` actor on worker-emitted events**
**Goal:** Events the worker emits without a typed actor get a `system:worker` stamp at the *worker-side sink layer*, not in shared event-conversion infrastructure. User identity stays on the run record (`RunSpec.provenance.subject`), where it already lives.
**Requirements:** R6.
**Dependencies:** Should land WITH the auth changes (Units 1-4), not in isolation. Landing this alone produces a wire-visible behavior change (worker events flip from `actor: None` to `actor: System(worker)`) without the auth context that justifies it. Sequence: land Unit 5 in the same release as Units 1-4 to keep the rationale visible in git history.
**Files:**
- Modify: `lib/crates/fabro-cli/src/commands/run/runner.rs` — install a worker-only sink wrapper in the `RunEventSink::fanout` chain at `runner.rs:100` that default-fills `actor` for events emitted by the worker.
- Test: `lib/crates/fabro-cli/src/commands/run/runner.rs` (test module inline).
**Approach:**
- **Critical: do NOT add the default-fill to `lib/crates/fabro-workflow/src/event.rs::to_run_event_at` or `stored_event_fields`.** Those helpers are shared infrastructure called by both the server (e.g. `server.rs:6805` lifecycle/error event flush via `workflow_event::to_run_event`) AND the worker. Default-filling there mis-stamps server-emitted events as `system:worker`.
- Helper function (not `const` — `ActorRef::id`/`display` are `Option<String>`, heap-allocated, not `const`-constructible): `fn system_worker_actor() -> ActorRef { ActorRef { kind: ActorKind::System, id: Some("worker".to_string()), display: Some("system:worker".to_string()) } }` lives in `runner.rs` (worker-local).
- Wrap `RunEventSink::backend(http_store)` in a `SystemWorkerActorSink` adapter that, before forwarding to the inner sink, applies the rule: **if `event.actor.is_none()`, fill with `system_worker_actor()`** (value-based, not variant-based). Variants like `RunArchived { actor: Some(user_actor) }` keep their actor unchanged because `actor.is_some()`. Variants like worker self-cancel `RunCancelRequested { actor: None }` correctly get the system actor.
- Agent events (`AssistantMessage`) are constructed with `actor: Some(ActorRef::agent(...))` upstream — they retain `ActorKind::Agent` because `actor.is_some()`.
**Patterns to follow:**
- `RunEventSink::fanout` composition (`fabro-workflow/src/event.rs` sink layer) — sink wrappers are the existing pattern for per-deployment behavior.
- Existing actor-stamping for lifecycle events at the server endpoints (`actor_from_subject` at `server.rs:6278`) — server-side stamping for user actions; worker-side wrapper for worker events. Symmetric.
**Test scenarios:**
- Happy path: a stage-execution `RunEvent { actor: None, ... }` flows through `SystemWorkerActorSink` → forwarded event has `actor: Some(ActorRef { kind: System, id: Some("worker"), display: Some("system:worker") })`.
- Edge case: `RunEvent { actor: Some(user_actor), ... }` (e.g. lifecycle event mirrored back) → forwarded event retains the user actor (`actor.is_some()` → no-op).
- Edge case: agent message `RunEvent { actor: Some(ActorKind::Agent), ... }` → forwarded event retains `ActorKind::Agent`.
- Edge case: worker self-cancel `RunEvent { actor: None, body: RunCancelRequested { ... } }` → forwarded event gets `system:worker`. (Catches the variant-vs-value-based bug from earlier draft.)
- Regression: server-side event flush via `workflow_event::to_run_event` (`server.rs:6805`) is unchanged — `to_run_event_at` retains its passthrough semantics. Verify by leaving `to_run_event_at` tests untouched.
- Regression: `create_hydrates_provenance_into_store_state` (`fabro-workflow/src/operations/create.rs:1037`) still passes — originator user identity still on `RunSpec.provenance.subject`.
**Verification:**
- `cargo nextest run -p fabro-workflow` passes.
- New tests for the default-fill and the override-protections both pass.
---
- [ ] **Unit 6: End-to-end regression — github-only worker run with no `~/.fabro/auth.json`**
**Goal:** Lock in the bug fix: a github-only deployment can spawn a worker that completes a run, with no user OAuth artifact present on the worker host.
**Requirements:** R1, R5.
**Dependencies:** Units 1-5.
**Files:**
- Create: an integration test under `lib/crates/fabro-cli/tests/it/cmd/` (existing test scaffolding) — file name per local convention (e.g. `worker_auth.rs` or `cmd/run_worker_auth_test.rs`).
**Approach:**
- Test fixture: server configured with `auth.methods = ["github"]` only; no `FABRO_DEV_TOKEN`; `FABRO_HOME` redirected to a fresh tempdir with no `auth.json`.
- Run a tiny workflow end-to-end via the daemon → worker spawn path.
- Assert the worker successfully POSTs at least one `RunEvent` and the run reaches a terminal status.
- Assert no `auth.json` file is touched (timestamp / non-existence).
**Patterns to follow:**
- Existing integration tests in `lib/crates/fabro-cli/tests/it/cmd/` (per `support.rs` helpers like `daemon.bind.to_target()` at `support.rs:718`).
- CLAUDE.md note: tests must use `.no_proxy()` HTTP clients.
**Test scenarios:**
- Integration: github-only server + no `auth.json` + minimal workflow → run completes successfully; events visible via `GET /runs/{id}/events`.
- Integration: same setup but worker token tampered (e.g. spawn with a manually-replaced bogus token) → worker fails fast on first server call.
**Verification:**
- `cargo nextest run -p fabro-cli --test it` passes the new test.
- Test fails on `main` (pre-Units 1-5) — confirms it covers the regression.
## System-Wide Impact
- **Interaction graph:** Worker subprocess no longer reads `~/.fabro/auth.json`. CLI user-facing commands (`fabro run`, `fabro ps`, `fabro auth`, etc.) unchanged — they still go through `connect_server_target` / `connect_server_with_settings`. SSE attach endpoints (`/runs/{id}/attach`, `/attach`) unchanged: worker is a producer, not a consumer; user-JWT-only auth on those routes preserved.
- **Error propagation:** Worker token expiry mid-run → next server call returns 401, worker exits 78 (`EX_NOPERM`), server marks run failed via existing `pump_worker_*` paths (`server.rs:4880`+). Distinct exit code separates auth failures from generic crashes. Worker token rejected as revoked after run terminal status → 403, same exit path.
- **State lifecycle risks:** Server restart with HKDF-derived key: outstanding worker tokens remain valid up to natural expiry. Server restart with `SESSION_SECRET` rotated: outstanding workers fail at next call (acceptable; matches user-session invalidation). Server-side revocation set is in-memory and lost on restart — combined with token survival across restart, this creates a post-restart re-enablement window for tokens of completed runs. See Risks for mitigation options. The previous draft's claim that revocation "cuts blast radius to zero" was wrong; the honest claim is "cuts post-completion blast radius to zero **between server restarts**."
- **Event sink uniformity:** Worker `RunEventSink::fanout([Store(http), Callback])` (`runner.rs:100`) — default-fill of `actor` happens upstream in `to_run_event_at`, so all sinks (HTTP backend, local callback, future telemetry) see the same `system:worker` actor. No per-sink divergence.
- **API surface parity:** `RunEvent.actor` shape unchanged (`ActorRef` already has `ActorKind::System`); worker-emitted events newly carry `system:worker` instead of `None`. Web UI audit (`apps/fabro-web/app/`) found ZERO references to `actor` or `author` — there's nothing to break or filter today. Risk of UI breakage was overstated in earlier draft; confirmed safe.
- **Worker-process trust degradation:** A workflow stage that compromises the worker process (malicious shell, code injection) gains read access to `FABRO_WORKER_TOKEN` for that worker's lifetime + 72h until natural expiry, *or* until the run reaches terminal status (revocation). Blast radius bounded by run-id claim: only the compromised run's blobs/events/state are accessible. Not cross-run. `SESSION_SECRET` is NOT in the worker's env (Unit 3) so the worker cannot mint cross-run tokens.
- **Integration coverage:** Unit 6 covers github-only-no-auth-store regression; existing CLI integration tests cover dev-token deployments.
- **Unchanged invariants:** End-user auth (dev-token / github) unchanged. `RunAuthMethod` enum unchanged. `RunSpec.provenance` shape unchanged. Webhook auth unchanged. Artifact upload route's behavior from a worker's perspective unchanged (worker still authenticates and uploads — just via the unified token). `OpenAPI` / `fabro-api-client` (TypeScript) DTOs unchanged.
## Risks & Dependencies
| Risk | Mitigation |
|------|------------|
| Worker process inherits `SESSION_SECRET` → can mint tokens for any run, defeating per-run binding | Unit 3 explicitly `env_remove`s `SESSION_SECRET` (and other server-only secrets) on every worker spawn. Verified by `worker_command_strips_server_secrets_from_worker_env` test. **Highest-impact mitigation in the plan.** |
| Token leaks via `format!`/`Display`/`tracing` of `Credential::Worker` payload | `Credential::Worker` has redacted `Debug`, no `Display`. Compile-time guard test in Unit 2 asserts `!impl Display`. Code review must reject any `format!("... {token}")` pattern in worker code paths. |
| Sentry / panic capture serializes the worker's env or backtrace locals | Sentry panic hook (`fabro-telemetry/src/panic.rs`) captures only panic message + stacktrace, not env or frame variables. Confirmed safe today. New panics in worker code with token in the formatted message would leak — code review responsibility, no structural fix. |
| 72h token compromised mid-run, attacker uses it from any host on network | Run-id binding limits blast radius to one run. Server-side revocation set (Unit 2) cuts post-completion blast radius to zero **between server restarts** — see the restart re-enablement risk row below. If the threat model demands more (multi-tenant production), follow-up plan should add IP binding, proof-of-possession, or persisted revocation — out of scope here. |
| `client.upload_stage_artifact_*` API forces per-call bearer parameter | Fallback in Unit 4: keep parameter, pass `client.credential().bearer_token()` from the worker side. |
| `2026-04-22-003-refactor-lock-down-server-secrets-plan.md` lands first; its `apply_worker_env` allowlist must include `FABRO_WORKER_TOKEN` and EXCLUDE `SESSION_SECRET`/`FABRO_JWT_PRIVATE_KEY`/`GITHUB_APP_*` | Coordinate at land time. Both plans converge on env_clear + explicit re-injection. `FABRO_WORKER_TOKEN` replaces `FABRO_DEV_TOKEN` as the single re-injected secret. Verify the allowlist excludes server-only secrets — if not, this plan's `env_remove`s in `worker_command` are the safety net. |
| Server restart mid-run with `SESSION_SECRET` rotated → outstanding workers die | Accepted. Same UX as user sessions. Operators who rotate `SESSION_SECRET` already accept session invalidation. |
| `FABRO_AUTH_FILE` env var present in worker subprocess somehow re-introduces user OAuth | Worker explicitly does not call `AuthStore::default()`; the env var is only consulted by that constructor. Unit 4 removes the worker callsite. |
| Worker child processes (sandbox, agent, devcontainer) inherit `FABRO_WORKER_TOKEN` | Chokepoint helper `apply_sandbox_env` (Unit 4) + explicit denylist in `LocalSandbox::should_filter_env_var` covers all worker-reachable spawn sites. Note: `clippy.toml` denies `std::process::Command::new` only — extend to `tokio::process::Command::new` (Unit 4) so the lint nudge actually catches the dominant async spawn pattern. |
| `FABRO_WORKER_TOKEN` readable via `/proc/<pid>/environ` to same-UID processes on Linux | Documented in Threat Model: env-var transport does not protect against same-UID reads. Multi-tenant deployments must isolate per-tenant via separate UIDs / containers / namespaces. NOT a property of this design. |
| Revocation set is in-memory; lost on server restart, but worker tokens survive restart by design (HKDF key persists) — creates post-restart re-enablement window for tokens belonging to terminated runs | **Known limitation, called out honestly.** A token captured pre-completion can be replayed for up to 72h after a routine deploy if the run completed before the deploy and the attacker waits out the restart. Mitigation options: (a) persist revocation set to a small KV (Redis or a SlateDB key), (b) at authorize time, look up the run's terminal status from the run store and reject if `claims.iat < run.terminal_at`. **Pick at implementation time** — see Open Questions. The plan explicitly does NOT claim revocation "cuts post-completion blast radius to zero" anymore. |
| Same-run concurrent worker spawn (scheduler race, manual operator action) → two valid tokens for one `run_id` racing on event/state appends | Scheduler's at-most-one-worker-per-run guarantee is assumed but not verified by this plan. Verification step in Unit 3 audit must check `start_run` / `resume_run` / `unarchive_run` paths for race conditions. If not enforced today, follow-up plan adds a server-side spawn lock or a per-spawn nonce. Out of scope for this plan to fix the scheduler; in scope to flag the assumption. |
| Rapid pause/resume cycles leave multiple valid tokens per run (each resume mints fresh, prior tokens not revoked) | Each prior worker token remains valid up to its 72h `exp`. Multiplicative compromise window. **Pick at implementation time:** (a) revoke prior tokens on every new spawn (requires tracking active tokens per run), (b) bind via `spawn_id` nonce that the server replaces on each spawn, (c) accept and document. See Open Questions. |
| Web UI renders `ActorKind::System` poorly (assumed `User` only) | **Downgraded.** Audit of `apps/fabro-web/app/` found zero references to `actor`/`author` today — nothing to break. If future UI surfaces author display, that's additive work, not a regression. |
## Documentation / Operational Notes
- `docs-internal/` — if any internal doc describes worker auth (search before landing), update to reflect: "worker → server auth uses a server-issued per-run JWT, independent of end-user auth method."
- No external user-facing doc impact (no public API change; CLI args on `__run-worker` are internal-only, hidden via `#[command(hide = true)]`).
### Deploy story (atomic swap, no shim)
Verified: in-flight workers spawned pre-deploy continue working post-deploy via user-JWT fall-through (R7) — they still hold OAuth from `auth.json`, and `authorize_run_scoped` accepts user JWTs. The artifact-upload route (the only worker-touched route that today does NOT use bare `AuthenticatedService`) already implements the same fall-through pattern — `authorize_artifact_upload` at `lib/crates/fabro-server/src/server.rs:868` calls `authenticate_service_parts(parts)` after the upload-token check, so existing workers' user JWTs authenticate against artifact uploads post-deploy. Only newly-spawned workers post-deploy exercise the new contract; those start cleanly with `FABRO_WORKER_TOKEN`. Resumed runs re-spawn via `worker_command`, get a fresh token, no orphaning. **No drain or backwards-compat shim required.**
**Pre-deploy checklist:**
- Confirm `SESSION_SECRET` is set and stable across the restart (HKDF key derives from it).
- Verify `apply_worker_env` (if `2026-04-22-003` landed) excludes `SESSION_SECRET` and other server-only secrets from the allowlist.
- Grep all internal repos and `docs/api-reference/fabro-api.yaml` for `artifact-upload-token`, `artifact_upload_token`, `--artifact-upload-token`, `stage_artifacts:upload`. Document zero hits in the PR description before merging — the atomic-delete decision depends on no external consumer existing.
- Baseline: record `count(runs where status=running)`.
**Post-deploy (within 5 min):**
- `count(runs where status=running)` matches baseline ± natural completions.
- grep server logs for `target=worker_auth` — expect zero hits (any hit means token-injection bug).
- `WorkflowRunFailed` rate vs. 7-day baseline.
- Spawn one new run end-to-end; confirm completion.
- If any HITL-paused runs exist, resume one and confirm event emission.
**Rollback:** redeploy old binary. Workers spawned during the new-binary window fail at next call → marked failed → user re-runs. No data restoration needed; `FABRO_WORKER_TOKEN` is env-only, never persisted.
## Sources & References
- Worker auth surface inventory: `lib/crates/fabro-cli/src/commands/run/runner.rs:55-125`
- Artifact-upload-token model to generalize: `lib/crates/fabro-server/src/server.rs:291-293, 757-869`
- Worker spawn site: `lib/crates/fabro-server/src/server.rs:3797-3851`
- Existing HKDF key derivation: `lib/crates/fabro-server/src/auth/keys.rs:41`
- `Credential` variants: `lib/crates/fabro-client/src/credential.rs:6-30`
- `ActorRef` / `ActorKind`: `lib/crates/fabro-types/src/run_event/mod.rs:29-81`
- `RunProvenance`: `lib/crates/fabro-types/src/run.rs:34-49`
- Coordinated plans: `docs/plans/2026-04-22-003-refactor-lock-down-server-secrets-plan.md`, `docs/plans/2026-04-19-003-feat-cli-auth-login-plan.md`, `docs/plans/2026-04-20-001-fix-cli-server-same-host-assumptions-plan.md`
- Origin of artifact-upload-token pattern: `docs/plans/2026-04-06-object-backed-artifact-uploads.md:42-45`
- Worker subprocess history: `docs/plans/2026-04-06-subprocess-run-workers-signal-control-plan.md`, `docs/plans/2026-04-07-worker-http-only-run-store-migration-plan.md`

View file

@ -947,13 +947,13 @@ fabro secret rm ANTHROPIC_API_KEY
---
## `fabro store dump`
## `fabro dump`
Export the contents of a run's store-backed state to a directory for debugging and inspection.
Export the contents of a run's durable state to a directory for debugging and inspection.
```bash
fabro store dump <RUN>
fabro store dump abc123 -o ./debug-output
fabro dump <RUN>
fabro dump abc123 -o ./debug-output
```
| Argument / Flag | Description |

View file

@ -30,18 +30,18 @@ These paths are local runtime state and caches, not the canonical run state.
- **`runtime/`** — Local runtime files. Today this is mainly materialized blob payloads under `runtime/blobs/`.
- **`nodes/{manager_node}_{visit}/child/`** — Nested scratch directories for manager-loop child workflows.
Large durable values, event streams, checkpoints, diffs, conclusions, and retros are no longer projected into live scratch by default. Use `fabro logs`, `fabro inspect`, the API, or `fabro store dump` for those surfaces.
Large durable values, event streams, checkpoints, diffs, conclusions, and retros are no longer projected into live scratch by default. Use `fabro logs`, `fabro inspect`, the API, or `fabro dump` for those surfaces.
## Reconstructed and export-only layouts
Reconstructed metadata branches and `fabro store dump` exports now use the same core layout:
Reconstructed metadata branches and `fabro dump` exports now use the same core layout:
- `run.json` for the current projection snapshot, including the current checkpoint
- `graph.fabro` for workflow source
- `stages/retro/*.md` for retro prompt/response text
- `stages/{node_id}@{visit}/...` for per-stage prompt, response, status, diff, stdout, and stderr files
`fabro store dump` adds export-only history surfaces on top of that shared layout:
`fabro dump` adds export-only history surfaces on top of that shared layout:
- `events.jsonl` for the durable event stream
- `checkpoints/*.json` for checkpoint history snapshots

View file

@ -260,7 +260,7 @@ The web wizard produces **the same on-disk state** as the CLI install. The TOML-
Files written:
- `~/.fabro/settings.toml` — server config, auth methods, GitHub integration strategy.
- `<storage_dir>/server.env` — `FABRO_JWT_PRIVATE_KEY`, `FABRO_JWT_PUBLIC_KEY`, `SESSION_SECRET`, `FABRO_DEV_TOKEN`, plus GitHub App env pairs (`GITHUB_APP_PRIVATE_KEY`, `GITHUB_APP_CLIENT_SECRET`, `GITHUB_APP_WEBHOOK_SECRET`) if the App strategy was chosen.
- `<storage_dir>/server.env` — `SESSION_SECRET`, `FABRO_DEV_TOKEN`, plus GitHub App env pairs (`GITHUB_APP_PRIVATE_KEY`, `GITHUB_APP_CLIENT_SECRET`, `GITHUB_APP_WEBHOOK_SECRET`) if the App strategy was chosen.
- `<storage_dir>/vaults/default/secrets.json` — vault entries for LLM API key credentials and (if Token strategy) `GITHUB_TOKEN`. Path matches `Storage::secrets_path()` at `lib/crates/fabro-config/src/storage.rs:38`.
- `<storage_dir>/server.dev-token` — the per-storage dev token, written via `Storage::server_state().dev_token_path()` at `storage.rs:103`. The CLI install also writes a home-level mirror at `Home::from_env().dev_token_path()` (`install.rs:1994-1999`); the web flow does the same to keep parity, since the home-level file is what tooling outside the storage dir expects to find.
- Artifact store metadata stamped with `FABRO_VERSION` via `write_artifact_store_metadata` (`install.rs:1458`).

View file

@ -514,7 +514,7 @@ pub(crate) struct InspectArgs {
}
#[derive(Args)]
pub(crate) struct StoreDumpArgs {
pub(crate) struct DumpArgs {
#[command(flatten)]
pub(crate) server: ServerTargetArgs,
@ -1001,8 +1001,8 @@ pub(crate) enum Commands {
Parse(ParseArgs),
/// Inspect and copy run artifacts (screenshots, reports, traces)
Artifact(ArtifactNamespace),
/// Export store-backed run state for debugging
Store(StoreNamespace),
/// Export a run's durable state to a directory
Dump(DumpArgs),
#[command(flatten)]
RunsCmd(RunsCommands),
/// List and test LLM models
@ -1085,9 +1085,7 @@ impl Commands {
ArtifactCommand::List(_) => "artifact list",
ArtifactCommand::Cp(_) => "artifact cp",
},
Self::Store(ns) => match &ns.command {
StoreCommand::Dump(_) => "store dump",
},
Self::Dump(_) => "dump",
Self::Exec(_) => "exec",
Self::RunCmd(cmd) => cmd.name(),
Self::Preflight(_) => "preflight",
@ -1199,18 +1197,6 @@ pub(crate) enum ArtifactCommand {
Cp(ArtifactCpArgs),
}
#[derive(Args)]
pub(crate) struct StoreNamespace {
#[command(subcommand)]
pub(crate) command: StoreCommand,
}
#[derive(Subcommand)]
pub(crate) enum StoreCommand {
/// Export a run's durable state to a directory
Dump(StoreDumpArgs),
}
#[derive(Args)]
pub(crate) struct SecretNamespace {
#[command(flatten)]

View file

@ -1,6 +1,6 @@
#![expect(
clippy::disallowed_methods,
reason = "CLI `store dump` command: sync file I/O for dump outputs"
reason = "CLI `dump` command: sync file I/O for dump outputs"
)]
use std::io::ErrorKind;
@ -12,18 +12,18 @@ use bytes::Bytes;
use fabro_store::{ArtifactStore, RunDatabase};
use fabro_store::{EventEnvelope, RunProjection, StageId};
use fabro_types::{RunBlobId, RunId};
use fabro_workflow::run_dump::RunDump;
use futures::future::BoxFuture;
#[cfg(test)]
use serde::de::DeserializeOwned;
use tokio::task::spawn_blocking;
use super::run_export::StoreRunExport;
use crate::args::StoreDumpArgs;
use crate::args::DumpArgs;
use crate::command_context::CommandContext;
use crate::server_client::Client;
use crate::shared::{absolute_or_current, print_json_pretty};
pub(crate) async fn dump_command(args: &StoreDumpArgs, base_ctx: &CommandContext) -> Result<()> {
pub(crate) async fn run(args: &DumpArgs, base_ctx: &CommandContext) -> Result<()> {
let ctx = base_ctx.with_target(&args.server)?;
let printer = ctx.printer();
let client = ctx.server().await?;
@ -215,7 +215,7 @@ async fn export_run_from_source(
.with_context(|| format!("failed to create {}", staging_parent.display()))?;
let staging_dir = tempfile::Builder::new()
.prefix(".fabro-store-dump-")
.prefix(".fabro-dump-")
.tempdir_in(staging_parent)
.with_context(|| {
format!(
@ -241,7 +241,7 @@ async fn write_run_dump(
output_dir: &Path,
) -> Result<usize> {
let events = source.list_events().await?;
let mut dump = StoreRunExport::from_store_state_and_events(state, &events)?;
let mut dump = RunDump::from_store_state_and_events(state, &events)?;
dump.hydrate_referenced_blobs_with_reader(|blob_id| source.read_blob(blob_id))
.await?;

View file

@ -26,8 +26,8 @@ use fabro_config::daemon::ServerDaemon;
use fabro_config::user::{SETTINGS_CONFIG_FILENAME, default_storage_dir};
use fabro_config::{Storage, envfile};
use fabro_install::{
InstallListenConfig, generate_jwt_keypair, merge_server_settings as merge_server_settings_impl,
write_github_app_settings, write_token_settings,
InstallListenConfig, PendingSettingsWrite, merge_server_settings as merge_server_settings_impl,
persist_install_outputs_direct, write_github_app_settings, write_token_settings,
};
use fabro_model::Provider;
use fabro_server::serve;
@ -883,13 +883,6 @@ enum PendingGitHubSettings {
},
}
#[derive(Clone, Copy)]
struct PendingSettingsWrite<'a> {
path: &'a Path,
contents: &'a str,
previous_contents: Option<&'a str>,
}
async fn setup_github_app(
s: &Styles,
web_url: &str,
@ -1148,15 +1141,24 @@ fn credential_secret_request(credential: &AuthCredential) -> Result<CreateSecret
})
}
fn persist_server_env_secrets(storage_dir: &Path, secrets: &[(String, String)]) -> Result<()> {
if secrets.is_empty() {
return Ok(());
}
fn server_env_updates(secrets: &[(String, String)]) -> Vec<envfile::EnvFileUpdate> {
secrets
.iter()
.map(|(key, value)| envfile::EnvFileUpdate {
key: key.clone(),
value: value.clone(),
comment: None,
})
.collect()
}
let env_path = Storage::new(storage_dir).runtime_directory().env_path();
envfile::merge_env_file(&env_path, secrets.iter().cloned())
.with_context(|| format!("merging server env secrets into {}", env_path.display()))?;
Ok(())
fn server_env_removals(keys: &[&'static str]) -> Vec<envfile::EnvFileRemoval> {
keys.iter()
.map(|key| envfile::EnvFileRemoval {
key: (*key).to_string(),
comment: None,
})
.collect()
}
async fn persist_install_outputs(
@ -1221,20 +1223,15 @@ fn persist_github_install_changes(
let previous_vault = std::fs::read_to_string(&vault_path).ok();
let result = (|| -> Result<()> {
let mut server_env = envfile::read_env_file(&server_env_path)
.with_context(|| format!("reading env file {}", server_env_path.display()))?;
for key in &writes.server_env_remove {
server_env.remove(*key);
}
for (key, value) in &writes.server_env_set {
server_env.insert(key.clone(), value.clone());
}
if server_env.is_empty() {
restore_optional_file(&server_env_path, None)?;
} else {
envfile::write_env_file(&server_env_path, &server_env)
.with_context(|| format!("writing env file {}", server_env_path.display()))?;
}
let server_env_writes = server_env_updates(&writes.server_env_set);
let server_env_removals = server_env_removals(&writes.server_env_remove);
persist_install_outputs_direct(
storage_dir,
&server_env_writes,
&server_env_removals,
&[],
Some(&writes.settings_write),
)?;
let mut vault = Vault::load(vault_path.clone()).map_err(anyhow::Error::from)?;
for key in &writes.vault_remove {
@ -1249,14 +1246,6 @@ fn persist_github_install_changes(
.map_err(anyhow::Error::from)?;
}
std::fs::write(writes.settings_write.path, writes.settings_write.contents).with_context(
|| {
format!(
"writing settings file {}",
writes.settings_write.path.display()
)
},
)?;
Ok(())
})();
@ -1293,12 +1282,16 @@ async fn persist_install_outputs_with_settings(
connect_server: impl for<'a> Fn(&'a Path) -> BoxFuture<'a, Result<server_client::Client>>,
stop_server: impl for<'a> Fn(&'a Path, Duration) -> BoxFuture<'a, bool>,
) -> Result<()> {
persist_server_env_secrets(storage_dir, server_env_secrets)?;
if let Some(write) = settings_write {
std::fs::write(write.path, write.contents)
.with_context(|| format!("writing settings file {}", write.path.display()))?;
}
let server_env_path = Storage::new(storage_dir).runtime_directory().env_path();
let previous_server_env = std::fs::read_to_string(&server_env_path).ok();
let settings_write_ref = settings_write.as_ref();
persist_install_outputs_direct(
storage_dir,
&server_env_updates(server_env_secrets),
&[],
&[],
settings_write_ref,
)?;
let persist_result = persist_vault_secrets_with(
storage_dir,
@ -1310,6 +1303,7 @@ async fn persist_install_outputs_with_settings(
.await;
if let Err(err) = persist_result {
restore_optional_file(&server_env_path, previous_server_env.as_deref())?;
if let Some(write) = settings_write {
match write.previous_contents {
Some(previous) => std::fs::write(write.path, previous)
@ -1799,13 +1793,6 @@ async fn run_install_inner(args: &InstallArgs, ctx: &CommandContext) -> Result<(
s.green.apply_to("✔")
);
let (jwt_private_pem, jwt_public_pem) = generate_jwt_keypair()?;
fabro_util::printerr!(
printer,
" {} Ed25519 JWT keypair generated",
s.green.apply_to("✔")
);
let dev_token = if fabro_config::dev_token_auth_enabled(&install_settings) {
let token = dev_token::read_or_mint_dev_token_for_install(
&fabro_util::Home::from_env().dev_token_path(),
@ -1826,14 +1813,7 @@ async fn run_install_inner(args: &InstallArgs, ctx: &CommandContext) -> Result<(
None
};
let jwt_private_b64 = BASE64_STANDARD.encode(jwt_private_pem.as_bytes());
let jwt_public_b64 = BASE64_STANDARD.encode(jwt_public_pem.as_bytes());
let mut generated_server_env_pairs = vec![
("FABRO_JWT_PRIVATE_KEY".to_string(), jwt_private_b64),
("FABRO_JWT_PUBLIC_KEY".to_string(), jwt_public_b64),
("SESSION_SECRET".to_string(), session_secret),
];
let mut generated_server_env_pairs = vec![("SESSION_SECRET".to_string(), session_secret)];
if let Some(token) = dev_token {
generated_server_env_pairs.push(("FABRO_DEV_TOKEN".to_string(), token));
}
@ -2018,39 +1998,6 @@ mod tests {
assert!(secret.chars().all(|c| !c.is_ascii_uppercase()));
}
// -- JWT keypair --
#[tokio::test]
async fn jwt_keypair_private_pem_header() {
let (private, _) = generate_jwt_keypair().unwrap();
assert!(
private.starts_with("-----BEGIN PRIVATE KEY-----"),
"private PEM: {private}"
);
}
#[tokio::test]
async fn jwt_keypair_public_pem_header() {
let (_, public) = generate_jwt_keypair().unwrap();
assert!(
public.starts_with("-----BEGIN PUBLIC KEY-----"),
"public PEM: {public}"
);
}
#[tokio::test]
async fn jwt_keypair_public_parses() {
let (_, public) = generate_jwt_keypair().unwrap();
jsonwebtoken::DecodingKey::from_ed_pem(public.as_bytes()).expect("public key should parse");
}
#[tokio::test]
async fn jwt_keypair_private_parses() {
let (private, _) = generate_jwt_keypair().unwrap();
jsonwebtoken::EncodingKey::from_ed_pem(private.as_bytes())
.expect("private key should parse");
}
// -- Config TOML generation --
#[test]
@ -2503,11 +2450,8 @@ client_id = "client-id"
#[tokio::test]
async fn persist_install_outputs_persists_vault_secrets_via_server_when_autostarting() {
let dir = tempfile::tempdir().unwrap();
let server_env_pairs = vec![
("SESSION_SECRET".to_string(), "session".to_string()),
("FABRO_JWT_PUBLIC_KEY".to_string(), "public-key".to_string()),
];
let vault_secrets = vec![
let server_env_pairs = [("SESSION_SECRET".to_string(), "session".to_string())];
let vault_secrets = [
CreateSecretRequest {
name: "GITHUB_TOKEN".to_string(),
value: "gh-token".to_string(),
@ -2541,7 +2485,8 @@ client_id = "client-id"
.await;
let stop_called = Arc::new(AtomicBool::new(false));
persist_server_env_secrets(dir.path(), &server_env_pairs).unwrap();
let env_path = Storage::new(dir.path()).runtime_directory().env_path();
envfile::merge_env_file(&env_path, server_env_pairs.iter().cloned()).unwrap();
persist_vault_secrets_with(
dir.path(),
&vault_secrets,
@ -2568,7 +2513,6 @@ client_id = "client-id"
std::fs::read_to_string(Storage::new(dir.path()).runtime_directory().env_path())
.unwrap();
assert!(server_env.contains("SESSION_SECRET=session"));
assert!(server_env.contains("FABRO_JWT_PUBLIC_KEY=public-key"));
assert_eq!(created.calls_async().await, 2);
assert!(stop_called.load(Ordering::SeqCst));
assert!(!Storage::new(dir.path()).secrets_path().exists());
@ -2577,7 +2521,7 @@ client_id = "client-id"
#[tokio::test]
async fn persist_vault_secrets_with_leaves_running_server_up() {
let dir = tempfile::tempdir().unwrap();
let vault_secrets = vec![CreateSecretRequest {
let vault_secrets = [CreateSecretRequest {
name: "GITHUB_TOKEN".to_string(),
value: "gh-token".to_string(),
type_: ApiSecretType::Environment,
@ -2785,10 +2729,10 @@ client_id = "client-id"
}
#[tokio::test]
async fn persist_install_outputs_with_settings_does_not_write_settings_on_secret_failure() {
async fn persist_install_outputs_with_settings_rolls_back_new_files_on_secret_failure() {
let dir = tempfile::tempdir().unwrap();
let server_env_pairs = vec![("SESSION_SECRET".to_string(), "session".to_string())];
let vault_secrets = vec![CreateSecretRequest {
let server_env_pairs = [("SESSION_SECRET".to_string(), "session".to_string())];
let vault_secrets = [CreateSecretRequest {
name: "GITHUB_CLI_TOKEN".to_string(),
value: "gh-token".to_string(),
type_: ApiSecretType::Environment,
@ -2823,7 +2767,7 @@ client_id = "client-id"
assert!(result.is_err());
assert!(
Storage::new(dir.path())
!Storage::new(dir.path())
.runtime_directory()
.env_path()
.exists()
@ -2835,8 +2779,8 @@ client_id = "client-id"
#[tokio::test]
async fn persist_install_outputs_with_settings_restores_previous_contents_on_secret_failure() {
let dir = tempfile::tempdir().unwrap();
let server_env_pairs = vec![("SESSION_SECRET".to_string(), "session".to_string())];
let vault_secrets = vec![CreateSecretRequest {
let server_env_pairs = [("SESSION_SECRET".to_string(), "session".to_string())];
let vault_secrets = [CreateSecretRequest {
name: "GITHUB_CLI_TOKEN".to_string(),
value: "gh-token".to_string(),
type_: ApiSecretType::Environment,

View file

@ -2,6 +2,7 @@ pub(crate) mod artifact;
pub(crate) mod auth;
pub(crate) mod config;
pub(crate) mod doctor;
pub(crate) mod dump;
pub(crate) mod exec;
pub(crate) mod graph;
pub(crate) mod install;
@ -10,6 +11,7 @@ pub(crate) mod parse;
pub(crate) mod pr;
pub(crate) mod preflight;
pub(crate) mod provider;
pub(crate) mod rebuild;
pub(crate) mod render_graph;
pub(crate) mod repo;
pub(crate) mod run;
@ -17,7 +19,6 @@ pub(crate) mod runs;
pub(crate) mod sandbox;
pub(crate) mod secret;
pub(crate) mod server;
pub(crate) mod store;
pub(crate) mod system;
pub(crate) mod uninstall;
pub(crate) mod upgrade;

View file

@ -13,7 +13,7 @@ use tracing::info;
use crate::args::PrCreateArgs;
use crate::command_context::CommandContext;
use crate::commands::store::rebuild::rebuild_run_store;
use crate::commands::rebuild::rebuild_run_store;
use crate::shared::print_json_pretty;
use crate::shared::repo::ensure_matching_repo_origin;
use crate::user_config;

View file

@ -6,7 +6,7 @@ use git2::Repository;
use crate::args::ForkArgs;
use crate::command_context::CommandContext;
use crate::commands::store::rebuild::rebuild_run_store;
use crate::commands::rebuild::rebuild_run_store;
use crate::shared::print_json_pretty;
use crate::shared::repo::ensure_matching_repo_origin;

View file

@ -15,7 +15,7 @@ use serde::Serialize;
use crate::args::RewindArgs;
use crate::command_context::CommandContext;
use crate::commands::store::rebuild::rebuild_run_store;
use crate::commands::rebuild::rebuild_run_store;
use crate::server_client::Client;
use crate::shared::repo::ensure_matching_repo_origin;
use crate::shared::{color_if, print_json_pretty};

View file

@ -7,15 +7,15 @@ use std::path::{Path, PathBuf};
use std::time::Duration;
use anyhow::{Context, Result, anyhow, bail};
use fabro_config::RuntimeDirectory;
use fabro_config::bind::{Bind, BindRequest};
use fabro_config::daemon::ServerDaemon;
use fabro_config::user::{FABRO_CONFIG_ENV, default_settings_path, load_settings_config};
use fabro_config::{RuntimeDirectory, envfile};
use fabro_server::jwt_auth::auth_method_name;
use fabro_server::serve::{DEFAULT_TCP_PORT, ServeArgs};
use fabro_server::serve::{DEFAULT_TCP_PORT, ServeArgs, resolve_runtime_server_settings_for_start};
use fabro_server::{process_env_snapshot, validate_startup};
use fabro_types::settings::ServerAuthMethod;
use fabro_util::printer::Printer;
use fabro_util::session_secret;
use fabro_util::terminal::Styles;
use tokio::net::{TcpStream, UnixStream};
use tokio::process::Command as TokioCommand;
@ -222,33 +222,6 @@ fn configured_auth_methods(config_path: Option<&Path>) -> Vec<ServerAuthMethod>
.unwrap_or_default()
}
fn valid_session_secret(secret: &str) -> bool {
session_secret::validate_session_secret(secret).is_ok()
}
fn load_or_create_local_session_secret(runtime_directory: &RuntimeDirectory) -> Result<String> {
if let Some(secret) = std::env::var("SESSION_SECRET")
.ok()
.filter(|secret| valid_session_secret(secret))
{
return Ok(secret);
}
let server_env_path = runtime_directory.env_path();
if let Some(secret) = envfile::read_env_file(&server_env_path)
.ok()
.and_then(|entries| entries.get("SESSION_SECRET").cloned())
.filter(|secret| valid_session_secret(secret))
{
return Ok(secret);
}
let secret = session_secret::generate_session_secret();
envfile::merge_env_file(&server_env_path, [("SESSION_SECRET", secret.as_str())])
.with_context(|| format!("merging session secret into {}", server_env_path.display()))?;
Ok(secret)
}
// ---------------------------------------------------------------------------
// Foreground mode
// ---------------------------------------------------------------------------
@ -261,18 +234,6 @@ async fn execute_foreground(
styles: &'static Styles,
_printer: Printer,
) -> Result<()> {
let session_secret = load_or_create_local_session_secret(&RuntimeDirectory::new(&storage_dir))?;
let prior_session_secret = std::env::var_os("SESSION_SECRET");
std::env::set_var("SESSION_SECRET", &session_secret);
let _env_guard =
scopeguard::guard(
prior_session_secret,
|prior_session_secret| match prior_session_secret {
Some(value) => std::env::set_var("SESSION_SECRET", value),
None => std::env::remove_var("SESSION_SECRET"),
},
);
super::foreground::serve_with_daemon_record(serve_args, bind, storage_dir, styles).await
}
@ -303,6 +264,13 @@ async fn execute_daemon(
return Ok(());
}
let resolved_settings = resolve_runtime_server_settings_for_start(serve_args, storage_dir)?;
validate_startup(
runtime_directory.env_path().as_path(),
process_env_snapshot(),
&resolved_settings,
)?;
let log_path = runtime_directory.log_path();
if let Some(parent) = log_path.parent() {
std::fs::create_dir_all(parent)
@ -347,9 +315,7 @@ async fn execute_daemon(
cmd.arg("--watch-web");
}
let session_secret = load_or_create_local_session_secret(&runtime_directory)?;
cmd.arg("--storage-dir").arg(storage_dir);
cmd.env("SESSION_SECRET", &session_secret);
cmd.env_remove("FABRO_JSON");
cmd.stdout(stdout_log)

View file

@ -1,14 +0,0 @@
pub(crate) mod dump;
pub(crate) mod rebuild;
mod run_export;
use anyhow::Result;
use crate::args::{StoreCommand, StoreNamespace};
use crate::command_context::CommandContext;
pub(crate) async fn dispatch(ns: StoreNamespace, base_ctx: &CommandContext) -> Result<()> {
match ns.command {
StoreCommand::Dump(args) => dump::dump_command(&args, base_ctx).await,
}
}

View file

@ -1 +0,0 @@
pub(super) use fabro_workflow::run_dump::RunDump as StoreRunExport;

View file

@ -12,9 +12,16 @@ use fabro_server::serve::resolve_bind_request_from_settings;
use fabro_types::settings::{ServerAuthMethod, SettingsLayer};
pub(crate) fn storage_dir(settings: &SettingsLayer) -> Result<PathBuf> {
storage_dir_with_lookup(settings, &|name| std::env::var(name).ok())
}
pub(crate) fn storage_dir_with_lookup(
settings: &SettingsLayer,
lookup: &dyn Fn(&str) -> Option<String>,
) -> Result<PathBuf> {
let storage_root = fabro_config::resolve_storage_root(settings);
let resolved_root = storage_root
.resolve(|name| std::env::var(name).ok())
.resolve(lookup)
.map_err(|err| anyhow::anyhow!("failed to resolve {}: {err}", storage_root.as_source()))?;
Ok(PathBuf::from(resolved_root.value))
}

View file

@ -225,8 +225,8 @@ async fn main_inner() -> (String, Result<()>) {
Commands::Artifact(ns) => {
commands::artifact::dispatch(ns, &base_ctx).await?;
}
Commands::Store(ns) => {
commands::store::dispatch(ns, &base_ctx).await?;
Commands::Dump(args) => {
commands::dump::run(&args, &base_ctx).await?;
}
Commands::RunsCmd(cmd) => {
commands::runs::dispatch(cmd, &base_ctx).await?;
@ -448,7 +448,7 @@ async fn prepare_server_bootstrap(
mod tests {
use args::{
AuthCommand, AuthNamespace, Commands, InstallGitHubStrategyArg, ModelsCommand,
ProviderCommand, ProviderNamespace, StoreCommand, StoreNamespace,
ProviderCommand, ProviderNamespace,
};
use tokio::runtime::Runtime;
@ -883,13 +883,11 @@ level = "warn"
}
#[test]
fn parse_store_dump_command() {
let cli = Cli::try_parse_from(["fabro", "store", "dump", "ABC123", "-o", "./out"])
.expect("should parse");
fn parse_dump_command() {
let cli =
Cli::try_parse_from(["fabro", "dump", "ABC123", "-o", "./out"]).expect("should parse");
match *cli.command.unwrap() {
Commands::Store(StoreNamespace {
command: StoreCommand::Dump(args),
}) => {
Commands::Dump(args) => {
assert_eq!(args.run, "ABC123");
assert_eq!(args.output, std::path::PathBuf::from("./out"));
}

View file

@ -500,21 +500,20 @@ mod tests {
#[test]
fn resolve_local_tcp_credential_does_not_fallback_to_home_dev_token() {
let temp_home = tempfile::tempdir().unwrap();
std::fs::write(
temp_home.path().join("dev-token"),
"fabro_dev_abababababababababababababababababababababababababababababababab",
)
.unwrap();
let original_home = std::env::var_os("FABRO_HOME");
std::env::set_var("FABRO_HOME", temp_home.path());
let _guard = scopeguard::guard(original_home, |original_home| match original_home {
Some(value) => std::env::set_var("FABRO_HOME", value),
None => std::env::remove_var("FABRO_HOME"),
});
let token = "fabro_dev_abababababababababababababababababababababababababababababababab";
std::fs::write(temp_home.path().join("dev-token"), token).unwrap();
let target = ServerTarget::http_url("http://127.0.0.1:32276").unwrap();
let store = AuthStore::new(temp_home.path().join("auth.json"));
assert_eq!(
load_cli_dev_token_from_sources(None, &Home::new(temp_home.path())).as_deref(),
Some(token)
);
assert!(resolve_local_tcp_credential(&target).unwrap().is_none());
assert!(
resolve_local_tcp_credential_with_store(&target, None, &store, Utc::now())
.unwrap()
.is_none()
);
}
#[test]

View file

@ -249,13 +249,13 @@ root = "{{ env.FABRO_STORAGE_ROOT }}"
"#,
);
let temp = tempfile::tempdir().unwrap();
let original = std::env::var_os("FABRO_STORAGE_ROOT");
std::env::set_var("FABRO_STORAGE_ROOT", temp.path());
let _guard = scopeguard::guard(original, |original| match original {
Some(value) => std::env::set_var("FABRO_STORAGE_ROOT", value),
None => std::env::remove_var("FABRO_STORAGE_ROOT"),
});
assert_eq!(storage_dir(&settings).unwrap(), temp.path());
assert_eq!(
local_server::storage_dir_with_lookup(&settings, &|name| {
(name == "FABRO_STORAGE_ROOT").then(|| temp.path().display().to_string())
})
.unwrap(),
temp.path()
);
}
}

View file

@ -16,14 +16,14 @@ use crate::support::{LightweightCli, unique_run_id};
fn help() {
let context = test_context!();
let mut cmd = context.command();
cmd.args(["store", "dump", "--help"]);
cmd.args(["dump", "--help"]);
fabro_snapshot!(context.filters(), cmd, @"
success: true
exit_code: 0
----- stdout -----
Export a run's durable state to a directory
Usage: fabro store dump [OPTIONS] --output <OUTPUT> <RUN>
Usage: fabro dump [OPTIONS] --output <OUTPUT> <RUN>
Arguments:
<RUN> Run ID prefix or workflow name
@ -42,7 +42,7 @@ fn help() {
}
#[test]
fn store_dump_accepts_server_target_from_separate_home() {
fn dump_accepts_server_target_from_separate_home() {
let context = test_context!();
let run = setup_completed_dry_run(&context);
let cli = LightweightCli::new();
@ -51,7 +51,6 @@ fn store_dump_accepts_server_target_from_separate_home() {
let mut cmd = cli.command();
cmd.args([
"store",
"dump",
"--server",
&server,
@ -63,10 +62,10 @@ fn store_dump_accepts_server_target_from_separate_home() {
cmd.env("FABRO_DEV_TOKEN", dev_token);
}
let output = cmd.output().expect("store dump should execute");
let output = cmd.output().expect("dump should execute");
assert!(
output.status.success(),
"store dump via remote server target failed\nstdout:\n{}\nstderr:\n{}",
"dump via remote server target failed\nstdout:\n{}\nstderr:\n{}",
String::from_utf8_lossy(&output.stdout),
String::from_utf8_lossy(&output.stderr)
);
@ -74,7 +73,7 @@ fn store_dump_accepts_server_target_from_separate_home() {
}
#[test]
fn store_dump_exports_large_command_output_backed_by_blob_refs() {
fn dump_exports_large_command_output_backed_by_blob_refs() {
let context = test_context!();
let workflow = context.temp_dir.join("large-output.fabro");
fs::write(
@ -130,17 +129,11 @@ fn store_dump_exports_large_command_output_backed_by_blob_refs() {
let output_dir = context.temp_dir.join("export");
let mut dump_cmd = context.command();
dump_cmd.args([
"store",
"dump",
"--output",
output_dir.to_str().unwrap(),
&run_id,
]);
let dump_output = dump_cmd.output().expect("store dump should execute");
dump_cmd.args(["dump", "--output", output_dir.to_str().unwrap(), &run_id]);
let dump_output = dump_cmd.output().expect("dump should execute");
assert!(
dump_output.status.success(),
"store dump failed\nstdout:\n{}\nstderr:\n{}",
"dump failed\nstdout:\n{}\nstderr:\n{}",
String::from_utf8_lossy(&dump_output.stdout),
String::from_utf8_lossy(&dump_output.stderr)
);
@ -153,7 +146,7 @@ fn store_dump_exports_large_command_output_backed_by_blob_refs() {
}
#[test]
fn store_dump_exports_blob_refs_and_artifacts_together() {
fn dump_exports_blob_refs_and_artifacts_together() {
let context = test_context!();
let workspace_dir = context.temp_dir.join("mixed-export");
fs::create_dir_all(&workspace_dir).unwrap();
@ -233,17 +226,11 @@ include = ["assets/**"]
let output_dir = context.temp_dir.join("export-mixed");
let mut dump_cmd = context.command();
dump_cmd.args([
"store",
"dump",
"--output",
output_dir.to_str().unwrap(),
&run_id,
]);
let dump_output = dump_cmd.output().expect("store dump should execute");
dump_cmd.args(["dump", "--output", output_dir.to_str().unwrap(), &run_id]);
let dump_output = dump_cmd.output().expect("dump should execute");
assert!(
dump_output.status.success(),
"store dump failed\nstdout:\n{}\nstderr:\n{}",
"dump failed\nstdout:\n{}\nstderr:\n{}",
String::from_utf8_lossy(&dump_output.stdout),
String::from_utf8_lossy(&dump_output.stderr)
);
@ -260,14 +247,13 @@ include = ["assets/**"]
}
#[test]
fn store_dump_exports_completed_run_snapshot() {
fn dump_exports_completed_run_snapshot() {
let context = test_context!();
let run = setup_completed_dry_run(&context);
let output_dir = context.temp_dir.join("export");
let mut cmd = context.command();
cmd.args([
"store",
"dump",
"--output",
output_dir.to_str().unwrap(),
@ -298,7 +284,7 @@ fn store_dump_exports_completed_run_snapshot() {
}
#[test]
fn store_dump_rejects_non_empty_output_dir() {
fn dump_rejects_non_empty_output_dir() {
let context = test_context!();
let run = setup_completed_dry_run(&context);
let output_dir = context.temp_dir.join("nonempty");
@ -307,7 +293,6 @@ fn store_dump_rejects_non_empty_output_dir() {
let mut cmd = context.command();
cmd.args([
"store",
"dump",
"--output",
output_dir.to_str().unwrap(),

View file

@ -25,7 +25,7 @@ fn help() {
validate Validate a workflow
graph Render a workflow graph as SVG
artifact Inspect and copy run artifacts (screenshots, reports, traces)
store Export store-backed run state for debugging
dump Export a run's durable state to a directory
rm Remove one or more workflow runs
inspect Show detailed information about a workflow run
archive Mark terminal runs as archived (reviewed, no further action needed). Archived runs are hidden from default listings

View file

@ -9,6 +9,7 @@ mod diff;
mod discord;
mod docs;
mod doctor;
mod dump;
mod exec;
mod fabro;
mod fork;
@ -53,8 +54,6 @@ mod server_start;
mod server_status;
mod server_stop;
mod start;
mod store;
mod store_dump;
pub(crate) mod support;
mod system;
mod system_df;

View file

@ -9,12 +9,17 @@ use crate::support::{fatal_error_line, unique_run_id};
const TEST_DEV_TOKEN: &str =
"fabro_dev_abababababababababababababababababababababababababababababababab";
const TEST_SESSION_SECRET: &str =
"0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef";
fn provision_local_server_auth(context: &fabro_test::TestContext, storage_dir: &std::path::Path) {
context.ensure_home_server_auth_methods();
let server_env_path = Storage::new(storage_dir).runtime_directory().env_path();
envfile::merge_env_file(&server_env_path, [("FABRO_DEV_TOKEN", TEST_DEV_TOKEN)])
.expect("merging FABRO_DEV_TOKEN into server.env");
envfile::merge_env_file(&server_env_path, [
("FABRO_DEV_TOKEN", TEST_DEV_TOKEN),
("SESSION_SECRET", TEST_SESSION_SECRET),
])
.expect("merging server auth into server.env");
dev_token::write_dev_token(
&context.home_dir.join(".fabro").join("dev-token"),
TEST_DEV_TOKEN,

View file

@ -13,16 +13,18 @@ use std::time::{Duration, Instant};
use fabro_store::EventEnvelope;
use fabro_test::{assert_reqwest_status, expect_reqwest_json, fabro_snapshot, test_context};
use fabro_types::{EventBody, FailureReason, RunEvent};
use fabro_types::{EventBody, FailureReason, RunEvent, StageId};
use httpmock::MockServer;
use super::support::{
local_dev_token, output_stderr, run_events, run_state, server_endpoint, server_target,
wait_for_event_names, wait_for_status, write_gated_workflow,
find_run_dir, local_dev_token, output_stderr, run_events, run_state, server_endpoint,
server_target, wait_for_event_names, wait_for_status, write_gated_workflow,
};
use crate::support::{fabro_json_snapshot, unique_run_id};
const SHARED_DAEMON_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(30);
const LEAKED_WORKER_PARENT_TOKEN: &str = "leak-worker-parent-token";
const LEAKED_NEW_RELIC_LICENSE: &str = "leak-new-relic-license";
fn auth_context() -> fabro_test::TestContext {
let context = test_context!();
@ -126,6 +128,20 @@ fn worker_command(context: &fabro_test::TestContext) -> assert_cmd::Command {
cmd
}
fn assert_no_worker_env_leak(scope: &str, content: &str) {
for needle in [
"MY_API_TOKEN=",
"NEW_RELIC_LICENSE_KEY=",
LEAKED_WORKER_PARENT_TOKEN,
LEAKED_NEW_RELIC_LICENSE,
] {
assert!(
!content.contains(needle),
"{scope} leaked {needle:?}:\n{content}"
);
}
}
async fn wait_for_server_question(
client: &fabro_http::HttpClient,
base_url: &str,
@ -373,6 +389,120 @@ digraph DetachedStoreOnly {
assert_worker_succeeded(&run_dir, &output);
}
#[test]
fn server_dispatched_worker_does_not_inherit_parent_secret_env() {
let mut context = test_context!();
let server_root = tempfile::tempdir_in("/tmp").unwrap();
let storage_dir = server_root.path().join("storage");
let socket_path = server_root.path().join("fabro.sock");
let config_path = server_root.path().join("settings.toml");
context.manage_storage_dir(&storage_dir);
std::fs::write(
&config_path,
format!(
r#"_version = 1
[server.storage]
root = "{}"
[server.auth]
methods = ["dev-token"]
"#,
storage_dir.display()
),
)
.expect("writing leak-probe server settings");
let start_output = context
.command()
.env("MY_API_TOKEN", LEAKED_WORKER_PARENT_TOKEN)
.env("NEW_RELIC_LICENSE_KEY", LEAKED_NEW_RELIC_LICENSE)
.args(["server", "start"])
.arg("--storage-dir")
.arg(&storage_dir)
.arg("--bind")
.arg(&socket_path)
.arg("--config")
.arg(&config_path)
.output()
.expect("server start should execute");
assert!(
start_output.status.success(),
"server start failed:\nstdout:\n{}\nstderr:\n{}",
String::from_utf8_lossy(&start_output.stdout),
String::from_utf8_lossy(&start_output.stderr)
);
let workflow_path = context.temp_dir.join("worker-leak-probe.fabro");
std::fs::write(
&workflow_path,
r#"digraph WorkerLeakProbe {
graph [goal="Verify worker subprocess env isolation", default_max_retries=0]
start [shape=Mdiamond, label="Start"]
exit [shape=Msquare, label="Exit"]
probe [shape=parallelogram, label="Probe", script="echo probe-ran; for key in $(printf 'MY%s NEW%s' '_API_TOKEN' '_RELIC_LICENSE_KEY'); do value=$(printenv \"$key\" || true); if [ -n \"$value\" ]; then echo \"$key=$value\"; fi; done"]
start -> probe -> exit
}
"#,
)
.expect("writing leak-probe workflow");
let run_id = unique_run_id();
let dev_token = local_dev_token(&storage_dir).expect("managed server should have a dev token");
let run_output = context
.run_cmd()
.env("FABRO_DEV_TOKEN", dev_token)
.args([
"--server",
socket_path.to_str().expect("socket path should be UTF-8"),
"--run-id",
run_id.as_str(),
"--detach",
"--auto-approve",
"--no-retro",
"--sandbox",
"local",
workflow_path
.to_str()
.expect("workflow path should be UTF-8"),
])
.output()
.expect("detached leak-probe run should execute");
assert!(
run_output.status.success(),
"detached run failed:\nstdout:\n{}\nstderr:\n{}",
String::from_utf8_lossy(&run_output.stdout),
String::from_utf8_lossy(&run_output.stderr)
);
let run_dir = find_run_dir(&storage_dir, &run_id).expect("leak-probe run dir should exist");
wait_for_status(&run_dir, &["succeeded"]);
let state = run_state(&run_dir);
let _probe = state
.node(&StageId::new("probe", 1))
.expect("probe node state should exist");
let stdout = state
.checkpoint
.as_ref()
.and_then(|checkpoint| checkpoint.context_values.get("command.output"))
.and_then(serde_json::Value::as_str)
.expect("probe command output should exist");
assert!(
stdout.contains("probe-ran"),
"probe stage should have executed, got stdout:\n{stdout}"
);
assert_no_worker_env_leak("probe stdout", stdout);
assert_no_worker_env_leak(
"run state",
&serde_json::to_string(&state).expect("run state should serialize"),
);
let server_log =
std::fs::read_to_string(storage_dir.join("logs/server.log")).unwrap_or_default();
assert_no_worker_env_leak("server log", &server_log);
}
#[test]
fn runner_resume_rejects_completed_run_without_mutating_it() {
let context = auth_context();

View file

@ -14,13 +14,15 @@ use std::time::{Duration, Instant};
use fabro_config::{Storage, envfile};
use fabro_test::{
apply_test_isolation, fabro_snapshot, isolated_storage_dir, server_log_files, test_context,
wait_for_log_line, wait_for_path,
TestContext, apply_test_isolation, fabro_snapshot, isolated_storage_dir, server_log_files,
test_context, wait_for_log_line, wait_for_path,
};
use fabro_util::dev_token;
const TEST_DEV_TOKEN: &str =
"fabro_dev_abababababababababababababababababababababababababababababababab";
const TEST_SESSION_SECRET: &str =
"0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef";
fn write_dev_token_server_settings(config_path: &std::path::Path, rest: &str) {
std::fs::write(
@ -32,12 +34,115 @@ fn write_dev_token_server_settings(config_path: &std::path::Path, rest: &str) {
fn provision_dev_token_auth(home_dir: &std::path::Path, storage_dir: &std::path::Path) {
let server_env_path = Storage::new(storage_dir).runtime_directory().env_path();
envfile::merge_env_file(&server_env_path, [("FABRO_DEV_TOKEN", TEST_DEV_TOKEN)])
.expect("merging FABRO_DEV_TOKEN into server.env");
envfile::merge_env_file(&server_env_path, [
("FABRO_DEV_TOKEN", TEST_DEV_TOKEN),
("SESSION_SECRET", TEST_SESSION_SECRET),
])
.expect("merging server auth into server.env");
dev_token::write_dev_token(&home_dir.join(".fabro").join("dev-token"), TEST_DEV_TOKEN)
.expect("writing home dev-token");
}
#[derive(Clone, Copy, Debug)]
enum ServerStartMode {
Foreground,
Daemon,
}
impl ServerStartMode {
const ALL: [Self; 2] = [Self::Foreground, Self::Daemon];
fn name(self) -> &'static str {
match self {
Self::Foreground => "foreground",
Self::Daemon => "daemon",
}
}
fn add_args(self, cmd: &mut assert_cmd::Command) {
if matches!(self, Self::Foreground) {
cmd.arg("--foreground");
}
}
}
struct StartupFailureCase {
name: &'static str,
settings: &'static str,
server_env: &'static [(&'static str, &'static str)],
expected_error: &'static str,
}
fn run_startup_failure(context: &TestContext, mode: ServerStartMode, case: &StartupFailureCase) {
let storage_root = isolated_storage_dir();
let storage_dir = storage_root
.path()
.join(format!("{}-{}", case.name, mode.name()));
let socket_path = storage_root
.path()
.join(format!("{}-{}.sock", case.name, mode.name()));
let config_dir = tempfile::tempdir_in("/tmp").expect("creating startup failure config dir");
let config_path = config_dir.path().join("settings.toml");
std::fs::write(&config_path, case.settings).expect("writing startup failure settings");
if !case.server_env.is_empty() {
envfile::merge_env_file(
&Storage::new(&storage_dir).runtime_directory().env_path(),
case.server_env.iter().copied(),
)
.expect("writing startup failure server.env");
}
let mut cmd = context.command();
cmd.args(["server", "start"]);
mode.add_args(&mut cmd);
cmd.arg("--storage-dir")
.arg(&storage_dir)
.arg("--bind")
.arg(&socket_path)
.arg("--config")
.arg(&config_path);
let output = cmd
.output()
.expect("server start failure command should run");
assert!(
!output.status.success(),
"server start should reject {} in {} mode\nstdout:\n{}\nstderr:\n{}",
case.name,
mode.name(),
String::from_utf8_lossy(&output.stdout),
String::from_utf8_lossy(&output.stderr)
);
assert!(
output.stdout.is_empty(),
"server start rejection should not write stdout for {} in {} mode:\n{}",
case.name,
mode.name(),
String::from_utf8_lossy(&output.stdout)
);
assert_eq!(
String::from_utf8_lossy(&output.stderr),
format!("error: {}\n", case.expected_error),
"unexpected stderr for {} in {} mode",
case.name,
mode.name()
);
let log_path = storage_dir.join("logs/server.log");
match mode {
ServerStartMode::Foreground => assert!(
log_path.exists(),
"foreground validation intentionally runs after log bootstrap for {}",
case.name
),
ServerStartMode::Daemon => assert!(
!log_path.exists(),
"daemon validation should fail before creating server.log for {}",
case.name
),
}
}
#[test]
fn help() {
let context = test_context!();
@ -96,6 +201,120 @@ fn help() {
");
}
#[test]
fn start_rejects_invalid_startup_configuration_in_foreground_and_daemon() {
const DEV_TOKEN_SETTINGS: &str = r#"_version = 1
[server.auth]
methods = ["dev-token"]
"#;
const GITHUB_SETTINGS: &str = r#"_version = 1
[server.web]
enabled = true
[server.auth]
methods = ["github"]
[server.auth.github]
allowed_usernames = ["octocat"]
[server.integrations.github]
client_id = "Iv1.testclient"
"#;
const GITHUB_WITHOUT_CLIENT_ID_SETTINGS: &str = r#"_version = 1
[server.web]
enabled = true
[server.auth]
methods = ["github"]
[server.auth.github]
allowed_usernames = ["octocat"]
"#;
const GITHUB_WEB_DISABLED_SETTINGS: &str = r#"_version = 1
[server.web]
enabled = false
[server.auth]
methods = ["github"]
[server.auth.github]
allowed_usernames = ["octocat"]
[server.integrations.github]
client_id = "Iv1.testclient"
"#;
const EMPTY_AUTH_METHODS_SETTINGS: &str = r"_version = 1
[server.auth]
methods = []
";
let context = test_context!();
let cases = [
StartupFailureCase {
name: "missing-session-secret",
settings: DEV_TOKEN_SETTINGS,
server_env: &[("FABRO_DEV_TOKEN", TEST_DEV_TOKEN)],
expected_error: "Fabro server refuses to start: auth is configured but SESSION_SECRET is not set.",
},
StartupFailureCase {
name: "missing-dev-token",
settings: DEV_TOKEN_SETTINGS,
server_env: &[("SESSION_SECRET", TEST_SESSION_SECRET)],
expected_error: "Fabro server refuses to start: dev-token auth is enabled but FABRO_DEV_TOKEN is not set.",
},
StartupFailureCase {
name: "missing-github-client-secret",
settings: GITHUB_SETTINGS,
server_env: &[("SESSION_SECRET", TEST_SESSION_SECRET)],
expected_error: "Fabro server refuses to start: github auth is enabled but GITHUB_APP_CLIENT_SECRET is not set.",
},
StartupFailureCase {
name: "empty-auth-methods",
settings: EMPTY_AUTH_METHODS_SETTINGS,
server_env: &[],
expected_error: "failed to resolve server settings:\n server.auth.methods: invalid value - must not be empty",
},
StartupFailureCase {
name: "github-web-disabled",
settings: GITHUB_WEB_DISABLED_SETTINGS,
server_env: &[
("SESSION_SECRET", TEST_SESSION_SECRET),
("GITHUB_APP_CLIENT_SECRET", "github-client-secret"),
],
expected_error: "Fabro server refuses to start: github auth is enabled but server.web.enabled is false.",
},
StartupFailureCase {
name: "github-missing-client-id",
settings: GITHUB_WITHOUT_CLIENT_ID_SETTINGS,
server_env: &[
("SESSION_SECRET", TEST_SESSION_SECRET),
("GITHUB_APP_CLIENT_SECRET", "github-client-secret"),
],
expected_error: "Fabro server refuses to start: github auth is enabled but server.integrations.github.client_id is not configured.",
},
StartupFailureCase {
name: "invalid-dev-token",
settings: DEV_TOKEN_SETTINGS,
server_env: &[
("SESSION_SECRET", TEST_SESSION_SECRET),
("FABRO_DEV_TOKEN", "not-a-valid-dev-token"),
],
expected_error: "Fabro server refuses to start: FABRO_DEV_TOKEN has invalid format.",
},
];
for case in &cases {
for mode in ServerStartMode::ALL {
run_startup_failure(&context, mode, case);
}
}
}
#[test]
fn start_already_running_exits_with_error() {
let context = test_context!();

View file

@ -1,29 +0,0 @@
use fabro_test::{fabro_snapshot, test_context};
#[test]
fn help() {
let context = test_context!();
let mut cmd = context.command();
cmd.args(["store", "--help"]);
fabro_snapshot!(context.filters(), cmd, @"
success: true
exit_code: 0
----- stdout -----
Export store-backed run state for debugging
Usage: fabro store [OPTIONS] <COMMAND>
Commands:
dump Export a run's durable state to a directory
help Print this message or the help of the given subcommand(s)
Options:
--json Output as JSON [env: FABRO_JSON=]
--debug Enable DEBUG-level logging (default is INFO) [env: FABRO_DEBUG=]
--no-upgrade-check Disable automatic upgrade check [env: FABRO_NO_UPGRADE_CHECK=true]
--quiet Suppress non-essential output [env: FABRO_QUIET=]
--verbose Enable verbose output [env: FABRO_VERBOSE=]
-h, --help Print help
----- stderr -----
");
}

View file

@ -13,10 +13,16 @@ fn start_status_stop_lifecycle() {
let server_env_path = fabro_config::Storage::new(&storage_dir)
.runtime_directory()
.env_path();
fabro_config::envfile::merge_env_file(&server_env_path, [(
"FABRO_DEV_TOKEN",
"fabro_dev_abababababababababababababababababababababababababababababababab",
)])
fabro_config::envfile::merge_env_file(&server_env_path, [
(
"FABRO_DEV_TOKEN",
"fabro_dev_abababababababababababababababababababababababababababababababab",
),
(
"SESSION_SECRET",
"0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef",
),
])
.unwrap();
fabro_util::dev_token::write_dev_token(
&context.home_dir.join(".fabro").join("dev-token"),

View file

@ -22,7 +22,8 @@ use fabro_server::auth::GithubEndpoints;
use fabro_server::ip_allowlist::IpAllowlistConfig;
use fabro_server::jwt_auth::resolve_auth_mode_with_lookup;
use fabro_server::server::{
RouterOptions, build_router_with_options, create_app_state_with_env_lookup,
RouterOptions, build_router_with_options,
create_app_state_with_env_lookup_and_server_secret_env,
};
use fabro_test::{GitHubAppState, TestContext, apply_test_isolation};
use fabro_types::RunAuthMethod;
@ -79,12 +80,21 @@ impl RealAuthHarness {
_ => None,
})
.expect("auth mode should resolve");
let state = create_app_state_with_env_lookup(settings, 5, move |name| match name {
"SESSION_SECRET" => Some(TEST_SESSION_SECRET.to_string()),
"GITHUB_APP_CLIENT_SECRET" => Some(github_client_secret.clone()),
"FABRO_DEV_TOKEN" => dev_token.clone(),
_ => None,
});
let mut secrets = std::collections::HashMap::from([
(
"SESSION_SECRET".to_string(),
TEST_SESSION_SECRET.to_string(),
),
(
"GITHUB_APP_CLIENT_SECRET".to_string(),
github_client_secret.clone(),
),
]);
if let Some(token) = dev_token.clone() {
secrets.insert("FABRO_DEV_TOKEN".to_string(), token);
}
let state =
create_app_state_with_env_lookup_and_server_secret_env(settings, 5, |_| None, &secrets);
let github_base = github_base_url(&twin.base_url);
let router = build_router_with_options(
state,

View file

@ -6,8 +6,8 @@
use fabro_test::test_context;
use super::{
completed_nodes, find_run_dir, fixture, read_conclusion, run_id_for, sandbox_tests,
store_dump_export, timeout_for,
completed_nodes, dump_export, find_run_dir, fixture, read_conclusion, run_id_for,
sandbox_tests, timeout_for,
};
sandbox_tests!(command_agent_mixed, keys = ["ANTHROPIC_API_KEY"]);
@ -48,7 +48,7 @@ fn scenario_command_agent_mixed(sandbox: &str) {
"verify should be completed"
);
let export_dir = store_dump_export(&context, &run_id_for(&run_dir));
let export_dir = dump_export(&context, &run_id_for(&run_dir));
let stdout = std::fs::read_to_string(export_dir.join("stages/verify@1/stdout.log"))
.expect("verify stdout.log should exist");
assert!(

View file

@ -6,8 +6,8 @@
use fabro_test::test_context;
use super::{
completed_nodes, find_run_dir, fixture, read_conclusion, run_id_for, sandbox_tests,
store_dump_export, timeout_for,
completed_nodes, dump_export, find_run_dir, fixture, read_conclusion, run_id_for,
sandbox_tests, timeout_for,
};
sandbox_tests!(command_pipeline);
@ -47,7 +47,7 @@ fn scenario_command_pipeline(sandbox: &str) {
"step2 should be completed"
);
let export_dir = store_dump_export(&context, &run_id_for(&run_dir));
let export_dir = dump_export(&context, &run_id_for(&run_dir));
let stdout1 = std::fs::read_to_string(export_dir.join("stages/step1@1/stdout.log"))
.expect("step1 stdout.log should exist");
assert!(

View file

@ -6,8 +6,8 @@
use fabro_test::test_context;
use super::{
completed_nodes, find_run_dir, fixture, has_event, read_conclusion, read_run_spec, run_id_for,
sandbox_tests, store_dump_export, timeout_for,
completed_nodes, dump_export, find_run_dir, fixture, has_event, read_conclusion, read_run_spec,
run_id_for, sandbox_tests, timeout_for,
};
sandbox_tests!(full_stack, keys = ["ANTHROPIC_API_KEY"]);
@ -73,7 +73,7 @@ fn scenario_full_stack(sandbox: &str) {
}
// Verify node stdout should contain PASS
let export_dir = store_dump_export(&context, &run_id_for(&run_dir));
let export_dir = dump_export(&context, &run_id_for(&run_dir));
let stdout = std::fs::read_to_string(export_dir.join("stages/verify@1/stdout.log"))
.expect("verify stdout.log should exist");
assert!(

View file

@ -59,17 +59,16 @@ pub(super) fn has_event(run_dir: &Path, event_name: &str) -> bool {
.any(|event| event.event.event_name() == event_name)
}
pub(super) fn store_dump_export(context: &TestContext, run_id: &str) -> PathBuf {
let output_dir = context.temp_dir.join(format!("store-dump-{run_id}"));
pub(super) fn dump_export(context: &TestContext, run_id: &str) -> PathBuf {
let output_dir = context.temp_dir.join(format!("dump-{run_id}"));
context
.command()
.args([
"store",
"dump",
"--output",
output_dir
.to_str()
.expect("store dump output path should be valid UTF-8"),
.expect("dump output path should be valid UTF-8"),
run_id,
])
.assert()

View file

@ -358,7 +358,7 @@ mod tests {
let entries = merge_env_file(&path, [
("SESSION_SECRET", "secret"),
("FABRO_JWT_PUBLIC_KEY", "jwt"),
("FABRO_DEV_TOKEN", "token"),
])
.unwrap();
@ -368,8 +368,8 @@ mod tests {
Some("secret")
);
assert_eq!(
entries.get("FABRO_JWT_PUBLIC_KEY").map(String::as_str),
Some("jwt")
entries.get("FABRO_DEV_TOKEN").map(String::as_str),
Some("token")
);
}

View file

@ -6,17 +6,8 @@
use std::path::{Path, PathBuf};
use anyhow::{Context, Result};
use base64::Engine as _;
use base64::engine::general_purpose::STANDARD as BASE64_STANDARD;
use fabro_config::{Storage, envfile};
use fabro_vault::{SecretType as VaultSecretType, Vault};
use ring::rand::SystemRandom;
use ring::signature::{Ed25519KeyPair, KeyPair as _};
const ED25519_SPKI_PREFIX: [u8; 12] = [
0x30, 0x2A, 0x30, 0x05, 0x06, 0x03, 0x2B, 0x65, 0x70, 0x03, 0x21, 0x00,
];
const ED25519_PUBLIC_KEY_LEN: usize = 32;
pub struct PendingSettingsWrite<'a> {
pub path: &'a Path,
@ -95,47 +86,6 @@ impl std::error::Error for PersistInstallOutputsError {
}
}
fn pem_encode(label: &str, bytes: &[u8]) -> String {
let body = BASE64_STANDARD.encode(bytes);
let mut pem = String::new();
pem.push_str("-----BEGIN ");
pem.push_str(label);
pem.push_str("-----\n");
for chunk in body.as_bytes().chunks(64) {
pem.push_str(std::str::from_utf8(chunk).expect("base64 output should be valid UTF-8"));
pem.push('\n');
}
pem.push_str("-----END ");
pem.push_str(label);
pem.push_str("-----\n");
pem
}
fn ed25519_public_key_spki(public_key: &[u8]) -> Result<Vec<u8>> {
anyhow::ensure!(
public_key.len() == ED25519_PUBLIC_KEY_LEN,
"generated Ed25519 public key had unexpected length"
);
let mut spki = Vec::with_capacity(ED25519_SPKI_PREFIX.len() + public_key.len());
spki.extend_from_slice(&ED25519_SPKI_PREFIX);
spki.extend_from_slice(public_key);
Ok(spki)
}
pub fn generate_jwt_keypair() -> Result<(String, String)> {
let pkcs8 = Ed25519KeyPair::generate_pkcs8(&SystemRandom::new())
.map_err(|_| anyhow::anyhow!("failed to generate Ed25519 keypair"))?;
let keypair = Ed25519KeyPair::from_pkcs8(pkcs8.as_ref())
.map_err(|_| anyhow::anyhow!("failed to parse generated Ed25519 keypair"))?;
let public_der = ed25519_public_key_spki(keypair.public_key().as_ref())?;
Ok((
pem_encode("PRIVATE KEY", pkcs8.as_ref()),
pem_encode("PUBLIC KEY", &public_der),
))
}
pub fn default_web_url() -> String {
"http://127.0.0.1:32276".to_string()
}

View file

@ -540,30 +540,6 @@ fn check_crypto(state: &AppState) -> CheckResult {
}
}
if let Some(raw) = state.server_secret("FABRO_JWT_PUBLIC_KEY") {
if let Err(err) = decode_pem_value("FABRO_JWT_PUBLIC_KEY", &raw).and_then(|pem| {
jsonwebtoken::DecodingKey::from_ed_pem(pem.as_bytes())
.map(|_| ())
.map_err(|e| format!("invalid JWT public key: {e}"))
}) {
errors.push(err);
} else {
details.push(CheckDetail::new("FABRO_JWT_PUBLIC_KEY valid".to_string()));
}
}
if let Some(raw) = state.server_secret("FABRO_JWT_PRIVATE_KEY") {
if let Err(err) = decode_pem_value("FABRO_JWT_PRIVATE_KEY", &raw).and_then(|pem| {
jsonwebtoken::EncodingKey::from_ed_pem(pem.as_bytes())
.map(|_| ())
.map_err(|e| format!("invalid JWT private key: {e}"))
}) {
errors.push(err);
} else {
details.push(CheckDetail::new("FABRO_JWT_PRIVATE_KEY valid".to_string()));
}
}
if errors.is_empty() {
CheckResult {
name: "Crypto".to_string(),

View file

@ -18,9 +18,8 @@ use fabro_config::bind::{Bind, BindRequest};
use fabro_config::envfile::EnvFileUpdate;
use fabro_install::{
InstallListenConfig, OBJECT_STORE_ACCESS_KEY_ID_ENV, OBJECT_STORE_SECRET_ACCESS_KEY_ENV,
PendingSettingsWrite, VaultSecretWrite, generate_jwt_keypair, merge_server_settings,
persist_install_outputs_direct, write_github_app_settings, write_object_store_settings,
write_token_settings,
PendingSettingsWrite, VaultSecretWrite, merge_server_settings, persist_install_outputs_direct,
write_github_app_settings, write_object_store_settings, write_token_settings,
};
use fabro_model::Provider;
use fabro_store::ArtifactStore;
@ -43,7 +42,7 @@ use zeroize::Zeroizing;
use crate::error::ApiError;
use crate::serve::{self, DEFAULT_TCP_PORT};
use crate::server_secrets::ServerSecrets;
use crate::server_secrets::{ServerSecrets, process_env_snapshot};
use crate::{security_headers, static_files};
#[derive(Clone)]
@ -113,6 +112,11 @@ impl InstallAppState {
reason = "test-only: set FABRO_TEST_IN_MEMORY_STORE to a constant so install tests \
don't hang on real S3; parallel tests race on the same value"
)]
#[expect(
clippy::disallowed_methods,
reason = "test-only: forces the in-memory object store for install tests so they \
don't contact real S3"
)]
pub fn for_test_with_paths(token: &str, storage_dir: &Path, config_path: &Path) -> Self {
// Install-flow tests verify persistence and redaction, not S3
// reachability. Force the in-memory object store shortcut so
@ -973,7 +977,8 @@ async fn validate_install_object_store_selection(
let server_env_path = Storage::new(state.storage_dir.as_ref())
.runtime_directory()
.env_path();
let server_secrets = ServerSecrets::load(server_env_path).map_err(|err| err.to_string())?;
let server_secrets = ServerSecrets::load(server_env_path, process_env_snapshot())
.map_err(|err| err.to_string())?;
let build_options = serve::ObjectStoreBuildOptions {
client_options,
retry_config: RetryConfig {
@ -1373,23 +1378,7 @@ async fn post_install_finish(
};
let session_secret = session_secret::generate_session_secret();
let (jwt_private_pem, jwt_public_pem) = match generate_jwt_keypair() {
Ok(value) => value,
Err(err) => {
return install_error_response(StatusCode::INTERNAL_SERVER_ERROR, err.to_string());
}
};
server_env_writes.extend([
make_env_write(
"FABRO_JWT_PRIVATE_KEY",
BASE64_STANDARD.encode(jwt_private_pem.as_bytes()),
),
make_env_write(
"FABRO_JWT_PUBLIC_KEY",
BASE64_STANDARD.encode(jwt_public_pem.as_bytes()),
),
make_env_write("SESSION_SECRET", session_secret),
]);
server_env_writes.push(make_env_write("SESSION_SECRET", session_secret));
if let Some(token) = dev_token.as_ref() {
server_env_writes.push(make_env_write("FABRO_DEV_TOKEN", token.clone()));
}
@ -1968,6 +1957,7 @@ async fn wait_for_shutdown(mut shutdown_rx: watch::Receiver<bool>) {
#[cfg(test)]
mod tests {
use std::collections::HashMap;
use std::io;
use std::sync::atomic::AtomicBool;
use std::sync::{Arc, Mutex};
@ -2108,7 +2098,7 @@ AWS_SESSION_TOKEN=ambient-session\n\
AWS_WEB_IDENTITY_TOKEN_FILE=/tmp/fabro-web-identity-token\n",
)
.unwrap();
let server_secrets = ServerSecrets::with_env_lookup(env_path.clone(), |_| None).unwrap();
let server_secrets = ServerSecrets::load(env_path.clone(), HashMap::new()).unwrap();
let manual_credentials =
InstallAwsCredentialPair::new("submitted-access", "submitted-secret");

View file

@ -31,7 +31,11 @@ pub mod security_headers;
pub mod serve;
pub mod server;
mod server_secrets;
mod spawn_env;
mod startup;
pub mod static_files;
pub mod web_auth;
pub use error::{ApiError, Error, Result};
pub use server_secrets::process_env_snapshot;
pub use startup::validate_startup;

View file

@ -32,12 +32,12 @@ use tracing::{error, info, warn};
use crate::canonical_origin::resolve_canonical_origin;
use crate::github_webhooks::{TailscaleFunnelManager, WEBHOOK_ROUTE, WEBHOOK_SECRET_ENV};
use crate::ip_allowlist::{GitHubMetaResolver, IpAllowlistConfig, resolve_ip_allowlist_config};
use crate::jwt_auth::resolve_auth_mode_with_lookup;
use crate::server::{
AppState, AppStateConfig, RouterOptions, build_app_state, build_router_with_options,
reconcile_incomplete_runs_on_startup, shutdown_active_workers, spawn_scheduler,
};
use crate::server_secrets::ServerSecrets;
use crate::server_secrets::{ServerSecrets, process_env_snapshot};
use crate::startup::resolve_startup;
const TEST_IN_MEMORY_STORE_ENV: &str = "FABRO_TEST_IN_MEMORY_STORE";
const AWS_SESSION_TOKEN_ENV: &str = "AWS_SESSION_TOKEN";
@ -475,6 +475,15 @@ fn resolve_server_settings(file: &SettingsLayer) -> anyhow::Result<ServerNamespa
.map_err(anyhow::Error::from)
}
pub fn resolve_runtime_server_settings_for_start(
args: &ServeArgs,
data_dir: &Path,
) -> anyhow::Result<ServerNamespace> {
let disk_settings = load_settings_config(args.config.as_deref())?;
let effective_settings = apply_runtime_settings(&disk_settings, args, data_dir);
resolve_server_settings(&effective_settings)
}
pub fn resolve_bind_request_from_settings(
settings: &SettingsLayer,
explicit_bind: Option<&str>,
@ -533,7 +542,7 @@ fn resolve_interp_path(value: &InterpString) -> anyhow::Result<PathBuf> {
fn load_server_secrets_for_settings(settings: &ServerNamespace) -> anyhow::Result<ServerSecrets> {
let storage_root = resolve_interp_path(&settings.storage.root)?;
let server_env_path = Storage::new(&storage_root).runtime_directory().env_path();
ServerSecrets::load(server_env_path).map_err(anyhow::Error::from)
ServerSecrets::load(server_env_path, process_env_snapshot()).map_err(anyhow::Error::from)
}
pub(crate) fn build_artifact_object_store_with_server_secrets(
@ -615,24 +624,21 @@ where
let storage = Storage::new(&data_dir);
let vault_path = storage.secrets_path();
let server_env_path = storage.runtime_directory().env_path();
let server_secrets = ServerSecrets::load(server_env_path.clone())?;
let webhook_secret_present = server_secrets.get(WEBHOOK_SECRET_ENV).is_some();
// Shared config for live reloading
let effective_settings = apply_runtime_settings(&disk_settings, &args, &data_dir);
let resolved_server_settings = resolve_server_settings(&effective_settings)?;
let (auth_mode, server_secrets) = resolve_startup(
&server_env_path,
process_env_snapshot(),
&resolved_server_settings,
)?;
let webhook_secret_present = server_secrets.get(WEBHOOK_SECRET_ENV).is_some();
let bind_request =
resolve_bind_request_from_settings(&effective_settings, args.bind.as_deref())?;
let shared_settings = Arc::new(RwLock::new(effective_settings));
std::fs::create_dir_all(&data_dir)
.with_context(|| format!("creating data directory {}", data_dir.display()))?;
let (auth_mode, max_concurrent_runs) = {
let auth_mode = resolve_auth_mode_with_lookup(&resolved_server_settings, |name| {
server_secrets.get(name)
})?;
let max_concurrent_runs = resolved_server_settings.scheduler.max_concurrent_runs;
(auth_mode, max_concurrent_runs)
};
let max_concurrent_runs = resolved_server_settings.scheduler.max_concurrent_runs;
let web_enabled = resolved_server_settings.web.enabled;
let github_meta_resolver = GitHubMetaResolver::from_cache_dir(&storage.cache_dir())?;
@ -665,7 +671,7 @@ where
store,
artifact_store,
vault_path,
server_env_path,
server_secrets,
env_lookup,
http_client: None,
})?;

View file

@ -125,6 +125,7 @@ use crate::run_selector::{ResolveRunError, resolve_run_by_selector};
use crate::server_secrets::{
LlmClientResult, ProviderCredentials, ServerSecrets, auth_issue_message,
};
use crate::spawn_env::{apply_render_graph_env, apply_worker_env};
use crate::{demo, diagnostics, run_manifest, security_headers, static_files, web_auth};
pub(crate) type EnvLookup = Arc<dyn Fn(&str) -> Option<String> + Send + Sync>;
@ -572,7 +573,7 @@ pub struct AppState {
pub(crate) files_in_flight: FilesInFlight,
pub(crate) vault: Arc<AsyncRwLock<Vault>>,
pub(crate) server_secrets: ServerSecrets,
pub(super) server_secrets: ServerSecrets,
pub(crate) provider_credentials: ProviderCredentials,
pub(crate) settings: Arc<RwLock<SettingsLayer>>,
pub(crate) server_settings: RwLock<Arc<ServerSettings>>,
@ -591,7 +592,7 @@ pub(crate) struct AppStateConfig {
pub(crate) store: Arc<Database>,
pub(crate) artifact_store: ArtifactStore,
pub(crate) vault_path: PathBuf,
pub(crate) server_env_path: PathBuf,
pub(crate) server_secrets: ServerSecrets,
pub(crate) env_lookup: EnvLookup,
pub(crate) http_client: Option<fabro_http::HttpClient>,
}
@ -2407,6 +2408,21 @@ pub fn create_app_state_with_env_lookup(
settings: SettingsLayer,
max_concurrent_runs: usize,
env_lookup: impl Fn(&str) -> Option<String> + Send + Sync + 'static,
) -> Arc<AppState> {
create_app_state_with_env_lookup_and_server_secret_env(
settings,
max_concurrent_runs,
env_lookup,
&HashMap::new(),
)
}
#[doc(hidden)]
pub fn create_app_state_with_env_lookup_and_server_secret_env(
settings: SettingsLayer,
max_concurrent_runs: usize,
env_lookup: impl Fn(&str) -> Option<String> + Send + Sync + 'static,
server_secret_env: &HashMap<String, String>,
) -> Arc<AppState> {
let (store, artifact_store) = test_store_bundle();
let env_lookup: EnvLookup = Arc::new(env_lookup);
@ -2415,6 +2431,8 @@ pub fn create_app_state_with_env_lookup(
let mut config = default_test_app_state_config(settings, max_concurrent_runs, env_lookup);
config.store = store;
config.artifact_store = artifact_store;
let server_env_path = config.vault_path.with_file_name("server.env");
config.server_secrets = load_test_server_secrets(server_env_path, server_secret_env.clone());
build_app_state(config).expect("test app state should build")
}
@ -2450,7 +2468,7 @@ pub(crate) fn create_test_app_state_with_session_key(
store,
artifact_store,
vault_path,
server_env_path,
server_secrets: load_test_server_secrets(server_env_path, HashMap::new()),
env_lookup,
http_client: Some(fabro_http::test_http_client().expect("test HTTP client should build")),
})
@ -2485,7 +2503,7 @@ fn default_test_app_state_config(
store,
artifact_store,
vault_path,
server_env_path,
server_secrets: load_test_server_secrets(server_env_path, HashMap::new()),
env_lookup,
http_client: Some(fabro_http::test_http_client().expect("test HTTP client should build")),
}
@ -2542,6 +2560,10 @@ fn default_env_lookup() -> EnvLookup {
Arc::new(|name| std::env::var(name).ok())
}
fn load_test_server_secrets(path: PathBuf, env: HashMap<String, String>) -> ServerSecrets {
ServerSecrets::load(path, env).expect("test server secrets should load")
}
pub(crate) fn build_app_state(config: AppStateConfig) -> anyhow::Result<Arc<AppState>> {
let AppStateConfig {
settings,
@ -2550,16 +2572,12 @@ pub(crate) fn build_app_state(config: AppStateConfig) -> anyhow::Result<Arc<AppS
store,
artifact_store,
vault_path,
server_env_path,
server_secrets,
env_lookup,
http_client,
} = config;
let vault = Arc::new(AsyncRwLock::new(Vault::load(vault_path)?));
let server_secrets = ServerSecrets::with_env_lookup(server_env_path, {
let env_lookup = Arc::clone(&env_lookup);
move |name| env_lookup(name)
})?;
let provider_credentials = ProviderCredentials::with_env_lookup(Arc::clone(&vault), {
let env_lookup = Arc::clone(&env_lookup);
move |name| env_lookup(name)
@ -3718,8 +3736,7 @@ fn worker_command(
.stdout(Stdio::null())
.stderr(Stdio::piped());
cmd.env_remove("FABRO_JSON");
cmd.env_remove("FABRO_DEV_TOKEN");
apply_worker_env(&mut cmd);
if state
.server_settings()
.server
@ -7111,9 +7128,9 @@ async fn render_dot_subprocess(
.map_err(|err| RenderSubprocessError::SpawnFailed(err.to_string()))?;
let exe = render_graph_subprocess_exe(exe_override)?;
let mut cmd = Command::new(exe);
apply_render_graph_env(&mut cmd);
cmd.arg("__render-graph")
.env("FABRO_TELEMETRY", "off")
.env_remove("FABRO_JSON")
.stdin(Stdio::piped())
.stdout(Stdio::piped())
.stderr(Stdio::piped());
@ -7340,9 +7357,12 @@ mod tests {
)]
fn webhook_test_app(auth_mode: AuthMode) -> Router {
let secret = TEST_WEBHOOK_SECRET.to_string();
let state = create_app_state_with_env_lookup(SettingsLayer::default(), 5, move |name| {
(name == WEBHOOK_SECRET_ENV).then(|| secret.clone())
});
let state = create_app_state_with_env_lookup_and_server_secret_env(
SettingsLayer::default(),
5,
|_| None,
&HashMap::from([(WEBHOOK_SECRET_ENV.to_string(), secret)]),
);
build_router_with_options(
state,
&auth_mode,
@ -7782,12 +7802,11 @@ root = "/srv/new"
)
.unwrap();
let secrets =
ServerSecrets::with_env_lookup(dir.path().join("server.env"), |name| match name {
"SESSION_SECRET" => Some("env-value".to_string()),
_ => None,
})
.unwrap();
let secrets = ServerSecrets::load(
dir.path().join("server.env"),
HashMap::from([("SESSION_SECRET".to_string(), "env-value".to_string())]),
)
.unwrap();
assert_eq!(secrets.get("SESSION_SECRET").as_deref(), Some("env-value"));
assert_eq!(
@ -7811,7 +7830,7 @@ root = "/srv/new"
.unwrap();
assert_eq!(
command_env_value(&github_cmd, "FABRO_DEV_TOKEN"),
EnvOverride::Removed
EnvOverride::Unchanged
);
let dev_token = tempfile::tempdir().unwrap();
@ -7867,10 +7886,15 @@ allowed_usernames = ["octocat"]
.write(&runtime_directory)
.unwrap();
create_app_state_with_env_lookup(settings, 5, move |name| match name {
"FABRO_DEV_TOKEN" => dev_token.clone(),
_ => None,
})
let server_secret_env = dev_token
.map(|token| HashMap::from([("FABRO_DEV_TOKEN".to_string(), token)]))
.unwrap_or_default();
create_app_state_with_env_lookup_and_server_secret_env(
settings,
5,
|_| None,
&server_secret_env,
)
}
#[cfg(unix)]

View file

@ -1,5 +1,5 @@
use std::collections::HashMap;
use std::path::PathBuf;
use std::path::Path;
use std::sync::Arc;
use fabro_auth::{CredentialResolver, CredentialUsage, ResolveError, ResolvedCredential};
@ -11,6 +11,10 @@ use tokio::sync::RwLock as AsyncRwLock;
type EnvLookup = Arc<dyn Fn(&str) -> Option<String> + Send + Sync>;
pub fn process_env_snapshot() -> HashMap<String, String> {
std::env::vars().collect()
}
#[derive(Debug, thiserror::Error)]
pub(crate) enum Error {
#[error(transparent)]
@ -18,36 +22,33 @@ pub(crate) enum Error {
}
pub(crate) struct ServerSecrets {
path: PathBuf,
env_entries: HashMap<String, String>,
file_entries: HashMap<String, String>,
env_lookup: EnvLookup,
}
impl ServerSecrets {
pub(crate) fn load(path: PathBuf) -> Result<Self, Error> {
Self::with_env_lookup(path, |name| std::env::var(name).ok())
}
pub(crate) fn with_env_lookup<F>(path: PathBuf, env_lookup: F) -> Result<Self, Error>
where
F: Fn(&str) -> Option<String> + Send + Sync + 'static,
{
pub(crate) fn load(
path: impl AsRef<Path>,
env_entries: HashMap<String, String>,
) -> Result<Self, Error> {
Ok(Self {
file_entries: envfile::read_env_file(&path)?,
path,
env_lookup: Arc::new(env_lookup),
env_entries,
file_entries: envfile::read_env_file(path.as_ref())?,
})
}
pub(crate) fn get(&self, name: &str) -> Option<String> {
(self.env_lookup)(name).or_else(|| self.file_entries.get(name).cloned())
self.env_entries
.get(name)
.cloned()
.or_else(|| self.file_entries.get(name).cloned())
}
}
impl std::fmt::Debug for ServerSecrets {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.debug_struct("ServerSecrets")
.field("path", &self.path)
.field("env_entries", &self.env_entries.keys().collect::<Vec<_>>())
.field(
"file_entries",
&self.file_entries.keys().collect::<Vec<_>>(),
@ -149,13 +150,15 @@ impl std::fmt::Debug for ProviderCredentials {
#[cfg(test)]
mod tests {
use std::collections::HashMap;
use std::sync::Arc;
use fabro_auth::{AuthCredential, AuthDetails};
use fabro_config::envfile;
use fabro_vault::{SecretType, Vault};
use tokio::sync::RwLock as AsyncRwLock;
use super::ProviderCredentials;
use super::{ProviderCredentials, ServerSecrets};
use crate::server_secrets::Provider;
#[tokio::test]
@ -198,4 +201,33 @@ mod tests {
Provider::Anthropic
]);
}
#[test]
fn server_secrets_snapshot_prefers_env_over_file() {
let dir = tempfile::tempdir().unwrap();
let env_path = dir.path().join("server.env");
envfile::write_env_file(
&env_path,
&HashMap::from([
("SESSION_SECRET".to_string(), "file-value".to_string()),
(
"GITHUB_APP_CLIENT_SECRET".to_string(),
"file-client".to_string(),
),
]),
)
.unwrap();
let secrets = ServerSecrets::load(
env_path,
HashMap::from([("SESSION_SECRET".to_string(), "env-value".to_string())]),
)
.unwrap();
assert_eq!(secrets.get("SESSION_SECRET").as_deref(), Some("env-value"));
assert_eq!(
secrets.get("GITHUB_APP_CLIENT_SECRET").as_deref(),
Some("file-client")
);
}
}

View file

@ -0,0 +1,125 @@
use std::ffi::OsString;
use tokio::process::Command;
const WORKER_ENV_ALLOWLIST: &[&str] = &[
"PATH",
"HOME",
"TMPDIR",
"USER",
"RUST_LOG",
"RUST_BACKTRACE",
"FABRO_HOME",
"FABRO_STORAGE_ROOT",
];
const RENDER_GRAPH_ENV_ALLOWLIST: &[&str] = &["PATH", "HOME", "TMPDIR"];
pub(crate) fn apply_worker_env(cmd: &mut Command) {
apply_allowlist(cmd, WORKER_ENV_ALLOWLIST, &|name| std::env::var_os(name));
}
pub(crate) fn apply_render_graph_env(cmd: &mut Command) {
apply_allowlist(cmd, RENDER_GRAPH_ENV_ALLOWLIST, &|name| {
std::env::var_os(name)
});
}
fn apply_allowlist(cmd: &mut Command, keys: &[&str], lookup: &dyn Fn(&str) -> Option<OsString>) {
cmd.env_clear();
for key in keys {
if let Some(value) = lookup(key) {
cmd.env(key, value);
}
}
}
#[cfg(all(test, unix))]
mod tests {
use std::collections::HashMap;
use std::ffi::OsString;
use std::path::Path;
use super::{RENDER_GRAPH_ENV_ALLOWLIST, WORKER_ENV_ALLOWLIST, apply_allowlist};
fn env_command() -> tokio::process::Command {
assert!(Path::new("/usr/bin/env").exists());
tokio::process::Command::new("/usr/bin/env")
}
async fn env_output(mut cmd: tokio::process::Command) -> HashMap<String, String> {
let output = cmd.output().await.expect("running env subprocess");
assert!(output.status.success());
String::from_utf8(output.stdout)
.expect("parsing env subprocess output as UTF-8")
.lines()
.filter_map(|line| {
let (key, value) = line.split_once('=')?;
Some((key.to_string(), value.to_string()))
})
.collect()
}
#[tokio::test]
async fn worker_allowlist_is_fail_closed() {
let env = HashMap::from([
("PATH".to_string(), "/bin".to_string()),
("HOME".to_string(), "/tmp/home".to_string()),
("TMPDIR".to_string(), "/tmp".to_string()),
("USER".to_string(), "alice".to_string()),
("RUST_LOG".to_string(), "debug".to_string()),
("FABRO_HOME".to_string(), "/tmp/fabro-home".to_string()),
(
"FABRO_STORAGE_ROOT".to_string(),
"/tmp/fabro-storage".to_string(),
),
("SESSION_SECRET".to_string(), "leak".to_string()),
("FABRO_DEV_TOKEN".to_string(), "garbage".to_string()),
("MY_API_KEY".to_string(), "blocked".to_string()),
]);
let mut cmd = env_command();
apply_allowlist(&mut cmd, WORKER_ENV_ALLOWLIST, &|name| {
env.get(name).map(OsString::from)
});
cmd.env(
"FABRO_DEV_TOKEN",
"fabro_dev_abababababababababababababababababababababababababababababababab",
);
let actual = env_output(cmd).await;
assert_eq!(actual.get("PATH").map(String::as_str), Some("/bin"));
assert_eq!(actual.get("HOME").map(String::as_str), Some("/tmp/home"));
assert_eq!(
actual.get("FABRO_DEV_TOKEN").map(String::as_str),
Some("fabro_dev_abababababababababababababababababababababababababababababababab")
);
assert!(!actual.contains_key("SESSION_SECRET"));
assert!(!actual.contains_key("MY_API_KEY"));
}
#[tokio::test]
async fn render_graph_allowlist_is_fail_closed() {
let env = HashMap::from([
("PATH".to_string(), "/bin".to_string()),
("HOME".to_string(), "/tmp/home".to_string()),
("TMPDIR".to_string(), "/tmp".to_string()),
("FABRO_TELEMETRY".to_string(), "on".to_string()),
("SESSION_SECRET".to_string(), "leak".to_string()),
]);
let mut cmd = env_command();
apply_allowlist(&mut cmd, RENDER_GRAPH_ENV_ALLOWLIST, &|name| {
env.get(name).map(OsString::from)
});
cmd.env("FABRO_TELEMETRY", "off");
let actual = env_output(cmd).await;
assert_eq!(actual.get("PATH").map(String::as_str), Some("/bin"));
assert_eq!(
actual.get("FABRO_TELEMETRY").map(String::as_str),
Some("off")
);
assert!(!actual.contains_key("SESSION_SECRET"));
}
}

View file

@ -0,0 +1,87 @@
use std::collections::HashMap;
use std::path::Path;
use fabro_types::settings::ServerNamespace;
use crate::jwt_auth::{AuthMode, resolve_auth_mode_with_lookup};
use crate::server_secrets::ServerSecrets;
pub(crate) fn resolve_startup(
env_path: &Path,
env_entries: HashMap<String, String>,
settings: &ServerNamespace,
) -> anyhow::Result<(AuthMode, ServerSecrets)> {
let server_secrets = ServerSecrets::load(env_path, env_entries)?;
let auth_mode = resolve_auth_mode_with_lookup(settings, |name| server_secrets.get(name))?;
Ok((auth_mode, server_secrets))
}
pub fn validate_startup(
env_path: &Path,
env_entries: HashMap<String, String>,
settings: &ServerNamespace,
) -> anyhow::Result<()> {
resolve_startup(env_path, env_entries, settings).map(|_| ())
}
#[cfg(test)]
mod tests {
use std::collections::HashMap;
use fabro_config::parse_settings_layer;
use fabro_types::settings::ServerNamespace;
use super::validate_startup;
fn resolved_settings(auth_methods: &[&str]) -> ServerNamespace {
let settings = parse_settings_layer(&format!(
r"
_version = 1
[server.auth]
methods = [{}]
",
auth_methods
.iter()
.map(|method| format!("\"{method}\""))
.collect::<Vec<_>>()
.join(", ")
))
.unwrap();
fabro_config::resolve_server_from_file(&settings).unwrap()
}
#[test]
fn validate_startup_accepts_configured_secrets() {
let dir = tempfile::tempdir().unwrap();
let env = HashMap::from([
(
"SESSION_SECRET".to_string(),
"0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef".to_string(),
),
(
"FABRO_DEV_TOKEN".to_string(),
"fabro_dev_abababababababababababababababababababababababababababababababab"
.to_string(),
),
]);
let settings = resolved_settings(&["dev-token"]);
assert!(validate_startup(dir.path().join("server.env").as_path(), env, &settings).is_ok());
}
#[test]
fn validate_startup_rejects_missing_secrets() {
let dir = tempfile::tempdir().unwrap();
let settings = resolved_settings(&["dev-token"]);
assert!(
validate_startup(
dir.path().join("server.env").as_path(),
HashMap::new(),
&settings,
)
.is_err()
);
}
}

View file

@ -23,8 +23,12 @@ fn security_doc_does_not_require_jwt_keys_for_the_current_web_flow() {
"security doc should still mention the session secret"
);
assert!(
!security.contains("`FABRO_JWT_PRIVATE_KEY`, `FABRO_JWT_PUBLIC_KEY`, and `SESSION_SECRET`"),
"security doc should not describe JWT keys as required for the current web flow"
!security.contains("FABRO_JWT_PRIVATE_KEY"),
"security doc should not mention removed JWT key settings"
);
assert!(
!security.contains("FABRO_JWT_PUBLIC_KEY"),
"security doc should not mention removed JWT key settings"
);
}

View file

@ -764,8 +764,6 @@ async fn token_install_finish_persists_settings_env_and_vault() {
.env_path(),
)
.unwrap();
assert!(server_env.contains("FABRO_JWT_PRIVATE_KEY="));
assert!(server_env.contains("FABRO_JWT_PUBLIC_KEY="));
assert!(server_env.contains("SESSION_SECRET="));
assert!(server_env.contains("FABRO_DEV_TOKEN="));
assert!(!server_env.contains("AWS_ACCESS_KEY_ID="));

View file

@ -12,7 +12,7 @@ use axum::body::Body;
use axum::http::{Method, Request, StatusCode};
use fabro_server::install::{InstallAppState, build_install_router};
use fabro_server::jwt_auth::AuthMode;
use fabro_server::server::{build_router, create_app_state_with_env_lookup};
use fabro_server::server::{build_router, create_app_state_with_env_lookup_and_server_secret_env};
use serde_yaml::Value;
use tower::ServiceExt;
@ -146,9 +146,12 @@ fn github_webhook_spec_and_sdk_describe_a_json_body() {
async fn github_webhook_spec_route_is_routable_when_webhook_secret_is_present() {
let secret = "test-webhook-secret".to_string();
let app = build_router(
create_app_state_with_env_lookup(test_settings(), 5, move |name| {
(name == "GITHUB_APP_WEBHOOK_SECRET").then(|| secret.clone())
}),
create_app_state_with_env_lookup_and_server_secret_env(
test_settings(),
5,
|_| None,
&std::collections::HashMap::from([("GITHUB_APP_WEBHOOK_SECRET".to_string(), secret)]),
),
AuthMode::Disabled,
);

View file

@ -37,7 +37,7 @@ pub fn spawn_detached(args: &[&str], env: &[(&str, &str)], env_remove: &[&str])
#[expect(
clippy::disallowed_types,
clippy::disallowed_methods,
reason = "Detaching must flush stdio synchronously before the double-fork."
reason = "Detaching must flush stdio synchronously before the double-fork; post-fork pre-exec env mutation is the one allowed exception to the workspace env-mutation ban."
)]
fn spawn_detached_unix(args: &[&str], env: &[(&str, &str)], env_remove: &[&str]) {
// Flush stdout/stderr before forking so the child process doesn't inherit

View file

@ -631,8 +631,11 @@ fn write_settings_file(path: &Path, storage_dir: &Path, rest: &str) {
fn write_test_server_dev_token(storage_dir: &Path) {
let server_env_path = Storage::new(storage_dir).runtime_directory().env_path();
envfile::merge_env_file(&server_env_path, [("FABRO_DEV_TOKEN", TEST_DEV_TOKEN)])
.unwrap_or_else(|err| panic!("failed to write {}: {err}", server_env_path.display()));
envfile::merge_env_file(&server_env_path, [
("FABRO_DEV_TOKEN", TEST_DEV_TOKEN),
("SESSION_SECRET", TEST_SESSION_SECRET),
])
.unwrap_or_else(|err| panic!("failed to write {}: {err}", server_env_path.display()));
}
fn write_test_home_dev_token(settings_path: &Path) {

View file

@ -11,20 +11,21 @@ pub struct Config {
impl Config {
pub fn from_env() -> Result<Self> {
let bind_addr = std::env::var("TWIN_OPENAI_BIND_ADDR")
.ok()
Self::from_lookup(&|name| std::env::var(name).ok())
}
pub fn from_lookup(lookup: &dyn Fn(&str) -> Option<String>) -> Result<Self> {
let bind_addr = lookup("TWIN_OPENAI_BIND_ADDR")
.map(|value| value.parse().context("invalid TWIN_OPENAI_BIND_ADDR"))
.transpose()?
.unwrap_or_else(|| SocketAddr::new(IpAddr::V4(Ipv4Addr::LOCALHOST), 3000));
let require_auth = std::env::var("TWIN_OPENAI_REQUIRE_AUTH")
.ok()
let require_auth = lookup("TWIN_OPENAI_REQUIRE_AUTH")
.map(|value| parse_bool_env(&value, "TWIN_OPENAI_REQUIRE_AUTH"))
.transpose()?
.unwrap_or(true);
let enable_admin = std::env::var("TWIN_OPENAI_ENABLE_ADMIN")
.ok()
let enable_admin = lookup("TWIN_OPENAI_ENABLE_ADMIN")
.map(|value| parse_bool_env(&value, "TWIN_OPENAI_ENABLE_ADMIN"))
.transpose()?
.unwrap_or(true);

View file

@ -2,30 +2,14 @@ use twin_openai::config::Config;
#[test]
fn config_loads_from_environment() {
let prior_bind = std::env::var("TWIN_OPENAI_BIND_ADDR").ok();
let prior_auth = std::env::var("TWIN_OPENAI_REQUIRE_AUTH").ok();
let prior_admin = std::env::var("TWIN_OPENAI_ENABLE_ADMIN").ok();
std::env::set_var("TWIN_OPENAI_BIND_ADDR", "127.0.0.1:4100");
std::env::set_var("TWIN_OPENAI_REQUIRE_AUTH", "false");
std::env::set_var("TWIN_OPENAI_ENABLE_ADMIN", "false");
let config = Config::from_env().expect("config should load");
let config = Config::from_lookup(&|name| match name {
"TWIN_OPENAI_BIND_ADDR" => Some("127.0.0.1:4100".to_string()),
"TWIN_OPENAI_REQUIRE_AUTH" | "TWIN_OPENAI_ENABLE_ADMIN" => Some("false".to_string()),
_ => None,
})
.expect("config should load");
assert_eq!(config.bind_addr.to_string(), "127.0.0.1:4100");
assert!(!config.require_auth);
assert!(!config.enable_admin);
match prior_bind {
Some(value) => std::env::set_var("TWIN_OPENAI_BIND_ADDR", value),
None => std::env::remove_var("TWIN_OPENAI_BIND_ADDR"),
}
match prior_auth {
Some(value) => std::env::set_var("TWIN_OPENAI_REQUIRE_AUTH", value),
None => std::env::remove_var("TWIN_OPENAI_REQUIRE_AUTH"),
}
match prior_admin {
Some(value) => std::env::set_var("TWIN_OPENAI_ENABLE_ADMIN", value),
None => std::env::remove_var("TWIN_OPENAI_ENABLE_ADMIN"),
}
}