mirror of https://github.com/fabro-sh/fabro.git synced 2026-10-10 03:30:59 +00:00

fabro-sh-0530[bot] e8f0aceee8

refactor: rationalize server secret scopes (vault-only for optional int… (#401 )

## Summary

Separates Fabro server secrets into two explicit scopes: **bootstrap**
secrets that come from process env or `server.env`, and **optional
integration** secrets that come exclusively from the vault. This makes
secret resolution simple and predictable, and removes all `process env →
server.env` fallback paths for optional integrations such as GitHub App,
Slack, Daytona, Brave Search, and LLM provider keys.

## What changed

**New `ToolSecrets` struct in `fabro-agent`** — Brave Search API key is
now passed explicitly through `SessionOptions.tool_secrets` rather than
read from process env inside the tool. The standalone CLI reads the key
at the CLI boundary (with an explicit
`#[expect(clippy::disallowed_methods)]` annotation); the server will
read it from the vault. The error message changes from
`"BRAVE_SEARCH_API_KEY environment variable is not set"` to
`"BRAVE_SEARCH_API_KEY is not configured"`.

**`VaultCredentialSource::vault_only` constructor in `fabro-auth`** —
Adds a constructor that passes `|_| None` as the env lookup, ensuring
the server LLM credential source never resolves provider keys from
process env.

**GitHub App secrets move to vault in install flows** — Both the CLI
`fabro install github` path and the browser install finish handler now
write `GITHUB_APP_PRIVATE_KEY`, `GITHUB_APP_CLIENT_SECRET`, and
`GITHUB_APP_WEBHOOK_SECRET` to the vault instead of `server.env`.
Switching strategies removes stale secrets from the other strategy's
storage location. The `vault_set` field type changes from `Vec<(String,
String)>` to `Vec<VaultSecretWrite>` to carry per-secret type metadata
(file vs. token).

**`fabro-vault` gains a `fabro-static` dependency** — Needed so the
vault crate can reference canonical env-var names from the shared
registry without a cycle.

**`GH_TOKEN` fallback removed** — `GITHUB_TOKEN` is now read from the
vault only; the changelog and `server-configuration.mdx` note drops
mention of `GH_TOKEN` as an accepted fallback.

**Version bump** — Workspace crates promoted from `0.244.0-nightly.0` to
`0.244.0`.

**Docs** — Internal strategy doc, public admin docs (Docker, Railway,
server-configuration, security, troubleshooting), and integration docs
(GitHub, Slack, Daytona, Brave Search, LiteLLM, tools reference, models)
all updated to reflect vault-only optional secrets and direct users to
`fabro secret set` rather than process env or `server.env`.

### Plan Summary

- **Task 1** (secret registry) — not yet present in this diff;
classification lives in the places that consume it.
- **Task 3–6** (vault-only lookups for GitHub, Slack, Daytona, LLM) —
implemented via `vault_only` constructor, `tool_secrets` threading, and
install-path changes.
- **Task 7** (Brave Search explicit injection) — `ToolSecrets`,
`register_core_tools` wiring, CLI boundary read.
- **Task 8** (install persistence) — GitHub App secrets written to
vault; token strategy writes `GITHUB_TOKEN` to vault and clears app
vault keys; app strategy clears `GITHUB_TOKEN` vault key.
- **Task 9** (docs) — all public and internal docs updated.


### Fabro Details

<details>
<summary>Ran 0 stages in 155m 26s for $60.85</summary>

| Stage | Duration | Cost | Retries |
|---|---|---|---|
| **Total** | **155m 26s** | **$60.85** | **0** |

</details>

<details>
<summary>Ran <code>ImplementPlan.fabro</code> (11 nodes and 14
edges)</summary>

```dot
digraph ImplementPlan {
    graph [
        goal="Implement and simplify",
        model_stylesheet="
            * { model: claude-opus-4-7; }
        "
    ]
    rankdir=LR

    start [shape=Mdiamond, label="Start"]
    exit  [shape=Msquare, label="Exit"]

    toolchain         [label="Toolchain", shape=parallelogram, script="command -v cargo >/dev/null || { curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y && sudo ln -sf $HOME/.cargo/bin/* /usr/local/bin/; }; cargo --version 2>&1", max_retries=0]
    preflight_compile [label="Preflight Compile", shape=parallelogram, script="cargo check -q --workspace 2>&1", max_retries=0]
    preflight_lint    [label="Preflight Lint", shape=parallelogram, script="cargo +nightly-2026-04-14 clippy -q --workspace --all-targets -- -D warnings 2>&1", max_retries=0]
    fix_lints         [label="Fix Lints", prompt="The preflight lint step failed. Read the build output from context and fix all clippy lint warnings.", max_visits=3]
    implement         [label="Implement", prompt="Read the plan file referenced in the goal and implement every step. Make all the code changes described in the plan. Use red/green TDD.", model="gpt-55", reasoning_effort="xhigh"]
    simplify_opus     [label="Simplify (Opus)", prompt="@prompts/simplify.md"]
    simplify_gpt      [label="Simplify (GPT-55)", prompt="@prompts/simplify.md", model="gpt-55"]
    verify            [label="Verify", shape=parallelogram, script="git fetch origin main 2>&1 && git merge --no-edit --no-stat origin/main 2>&1 && cargo +nightly-2026-04-14 fmt --all 2>&1 && cargo dev docs refresh 2>&1 && cargo +nightly-2026-04-14 fmt --check --all 2>&1 && { command -v rg >/dev/null 2>&1 || { echo 'rg is required for verify'; exit 127; }; } && ! rg -n 'AuthMode::Disabled|RunAuthMethod|RunSubjectProvenance|\bActorRef\b|\bActorKind\b|AuthenticatedSubject|AuthenticatedService|AuthorizeRunScoped|AuthorizeRunBlob|AuthorizeStageArtifact|AuthorizeCommandLog|auth_method\s*==\s*\"disabled\"' lib/crates apps lib/packages docs/public/api-reference/fabro-api.yaml 2>&1 && cargo +nightly-2026-04-14 clippy --workspace --all-targets -- -D warnings 2>&1 && cargo nextest run --workspace --status-level slow --profile ci 2>&1 && cargo dev docs check 2>&1 && bun install --frozen-lockfile 2>&1 && (cd apps/fabro-web && bun run typecheck) 2>&1 && (cd apps/fabro-web && bun run test) 2>&1 && (cd lib/packages/fabro-api-client && bun run typecheck) 2>&1 && cargo dev build -- -p fabro-cli --release 2>&1", goal_gate=true, retry_target="fixup"]
    fixup             [label="Fixup", prompt="The verify step failed. Read the build output from context and fix all format, clippy, Rust test, docs, TypeScript typecheck/test, and build failures.", max_visits=3]

    start -> toolchain
    toolchain -> preflight_compile [condition="outcome=succeeded"]
    toolchain -> exit
    preflight_compile -> preflight_lint [condition="outcome=succeeded"]
    preflight_compile -> exit
    preflight_lint -> implement [condition="outcome=succeeded"]
    preflight_lint -> fix_lints
    fix_lints -> preflight_lint
    implement -> simplify_opus -> simplify_gpt -> verify
    verify -> exit  [condition="outcome=succeeded"]
    verify -> fixup
    fixup -> verify
}

```

</details>

⚒️ Generated with [Fabro](https://fabro.sh)

---------

Co-authored-by: Fabro <noreply@fabro.sh>
Co-authored-by: Bryan Helmkamp <bryan@brynary.com>

2026-05-25 17:26:01 -04:00

5 KiB

Raw Blame History

Server Secrets Strategy

This document defines how Fabro handles server-level secrets.

Core Rules

ServerSecrets is the canonical reader for bootstrap server secrets only.
It reads bootstrap secrets 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.
Optional integration secrets are vault-only in server runtime. Do not add optional server integrations to ServerSecrets or add new runtime env fallback paths.
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. Enforced by clippy via disallowed_methods in clippy.toml; intentional exceptions must be annotated with a scoped #[expect(clippy::disallowed_methods, reason = "...")] at the call site.

Bootstrap Server Secrets

These values may be read via state.server_secret(...) because the server can need them before optional integrations are available:

Secret	Used by
`SESSION_SECRET`	Cookie encryption and JWT signing derivation
`FABRO_DEV_TOKEN`	Dev-token user auth when `server.auth.methods` includes `dev-token`
`AWS_ACCESS_KEY_ID` / `AWS_SECRET_ACCESS_KEY` / `AWS_SESSION_TOKEN`	Static S3 object-store credentials for server storage builders

These optional integration secrets are not server bootstrap secrets. They are read from the vault only:

LLM provider API keys and OAuth credential records
GITHUB_TOKEN
GITHUB_APP_PRIVATE_KEY
GITHUB_APP_CLIENT_SECRET
GITHUB_APP_WEBHOOK_SECRET
FABRO_SLACK_APP_TOKEN
FABRO_SLACK_BOT_TOKEN
DAYTONA_API_KEY
BRAVE_SEARCH_API_KEY

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 from the vault when GitHub auth is enabled
Requiredness is independent from source. GitHub auth can require a vault secret at startup even though it is not a bootstrap ServerSecrets value.
Other optional integration secrets remain lazy/feature-specific rather than universal boot blockers.

Provisioning

Bootstrap secrets come from one of two sources:

Platform env for 12-factor deployments
server.env written by install flows

Optional integration secrets are provisioned into the vault, usually with fabro secret set or fabro install.

There is no startup-time secret generation. A temporary startup migration moves recognized legacy optional secrets from process env or server.env into the vault, removes matching server.env entries after writing a backup, and logs conflicts by key name only. Runtime lookup remains vault-only after that migration step. See migrations-strategy.md for the migration pattern.

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. For worker subprocesses this is FABRO_WORKER_TOKEN, plus any explicitly required internal value such as a vault-derived GITHUB_APP_PRIVATE_KEY; it is not user auth state such as FABRO_DEV_TOKEN or auth.json.
The worker reads FABRO_WORKER_TOKEN from its env at startup (in main() before Tokio initializes) and immediately calls std::env::remove_var to scrub it. The token then flows through function arguments to runner::execute. Every descendant process (hooks, sandbox commands, devcontainer setup, MCP stdio, etc.) therefore inherits a worker env that no longer contains the bearer, so an unscrubbed spawn site cannot leak it.
The daemon child inherits the parent env unchanged except for output-format hygiene (FABRO_JSON removal).

Tests

In-process tests must inject bootstrap server secrets with construction-time stubs (EnvSource, StubEnv) or by writing server.env.
In-process tests for optional integrations must write the vault and must not rely on process env or 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

Classify it in fabro-static as Bootstrap or OptionalVault.
For bootstrap secrets, provision through platform env or install-written server.env, then read through state.server_secret(...).
For optional integration secrets, provision through the vault and read through state.vault_secret(...).
Decide explicitly whether startup should fail when it is absent.
If a worker or render subprocess needs it, re-inject it explicitly rather than broadening inheritance casually.

5 KiB Raw Blame History