fabro/lib/crates/fabro-server/tests/it/openapi_conformance.rs
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

220 lines
7.4 KiB
Rust

//! Conformance tests: spec ↔ router consistency.
#![allow(
clippy::absolute_paths,
clippy::default_trait_access,
clippy::manual_assert,
clippy::manual_let_else,
reason = "These spec/router conformance tests prefer direct assertions over pedantic style lints."
)]
use axum::body::Body;
use axum::http::{Method, Request, StatusCode};
use fabro_server::install::{InstallAppState, build_install_router};
use fabro_server::test_support::TestAppStateBuilder;
use serde_yaml::Value;
use tower::ServiceExt;
use super::helpers::{read_repo_file, test_app_state, test_settings};
fn load_spec() -> Value {
let text = read_repo_file("docs/public/api-reference/fabro-api.yaml");
serde_yaml::from_str(&text).expect("failed to parse spec")
}
fn resolve_path(path: &str) -> String {
path.replace("{id}", "test-id")
.replace("{qid}", "test-qid")
.replace("{stageId}", "test-stage")
.replace("{name}", "test-name")
.replace("{slug}", "test-slug")
}
fn methods_for_path_item(item: &Value) -> Vec<Method> {
const HTTP_METHODS: &[(&str, Method)] = &[
("get", Method::GET),
("post", Method::POST),
("put", Method::PUT),
("delete", Method::DELETE),
("patch", Method::PATCH),
];
let Some(map) = item.as_mapping() else {
return Vec::new();
};
HTTP_METHODS
.iter()
.filter(|(key, _)| map.contains_key(Value::String((*key).to_string())))
.map(|(_, method)| method.clone())
.collect()
}
fn path_item_has_tag(item: &Value, expected: &str) -> bool {
let Some(map) = item.as_mapping() else {
return false;
};
map.values().any(|operation| {
operation
.get("tags")
.and_then(Value::as_sequence)
.is_some_and(|tags| tags.iter().any(|tag| tag.as_str() == Some(expected)))
})
}
fn request_for(method: &Method, uri: &str) -> Request<Body> {
let mut builder = Request::builder().method(method).uri(uri);
let body = if method == Method::POST || method == Method::PUT || method == Method::PATCH {
builder = builder.header("content-type", "application/json");
Body::from("{}")
} else {
Body::empty()
};
builder
.body(body)
.expect("OpenAPI conformance request should build")
}
#[tokio::test]
async fn all_spec_routes_are_routable() {
let spec = load_spec();
let normal_app = fabro_server::test_support::build_test_router(test_app_state());
let install_app = build_install_router(InstallAppState::for_test("test-install-token"));
let paths = spec
.get("paths")
.and_then(Value::as_mapping)
.expect("spec is missing `paths`");
let mut checked = 0;
for (path_key, item) in paths {
let path = path_key.as_str().expect("path key must be a string");
let uri = resolve_path(path);
let app = if path_item_has_tag(item, "Install") {
install_app.clone()
} else {
normal_app.clone()
};
for method in methods_for_path_item(item) {
let response = app
.clone()
.oneshot(request_for(&method, &uri))
.await
.unwrap();
assert_ne!(
response.status(),
StatusCode::METHOD_NOT_ALLOWED,
"Route {method} {path} returned 405 — not registered in the router"
);
checked += 1;
}
}
assert!(checked > 0, "No routes were checked — is the spec empty?");
}
#[test]
fn github_webhook_spec_and_sdk_describe_a_json_body() {
let spec = load_spec();
let webhook_schema = spec["paths"]["/api/v1/webhooks/github"]["post"]["requestBody"]["content"]
["application/json"]["schema"]
.clone();
assert_eq!(
webhook_schema.get("type").and_then(Value::as_str),
Some("object"),
"GitHub webhook request body should be modeled as JSON, not a binary file upload"
);
assert!(
webhook_schema.get("format").is_none(),
"GitHub webhook JSON schema should not declare a binary format"
);
let generated_client =
read_repo_file("lib/packages/fabro-api-client/src/api/integrations-api.ts");
assert!(
!generated_client.contains("@param {File} body"),
"generated TypeScript client should not expose the webhook body as File"
);
assert!(
!generated_client.contains("receiveGithubWebhook: async (body: File"),
"generated TypeScript client should not require File for a JSON webhook payload"
);
}
#[tokio::test]
async fn github_webhook_spec_route_is_routable_when_webhook_secret_is_present() {
let secret = "test-webhook-secret";
let settings = test_settings();
let app = fabro_server::test_support::build_test_router(
TestAppStateBuilder::new()
.runtime_settings(settings.server_settings, settings.manifest_run_defaults)
.max_concurrent_runs(5)
.env_lookup(|_| None)
.vault_entries([("GITHUB_APP_WEBHOOK_SECRET", secret)])
.build(),
);
let response = app
.oneshot(request_for(&Method::POST, "/api/v1/webhooks/github"))
.await
.unwrap();
assert_eq!(
response.status(),
StatusCode::UNAUTHORIZED,
"Webhook spec route should be mounted when GITHUB_APP_WEBHOOK_SECRET is present"
);
}
#[tokio::test]
async fn install_and_normal_routes_stay_isolated() {
let spec = load_spec();
let normal_app = fabro_server::test_support::build_test_router(test_app_state());
let install_app = build_install_router(InstallAppState::for_test("test-install-token"));
let paths = spec
.get("paths")
.and_then(Value::as_mapping)
.expect("spec is missing `paths`");
for (path_key, item) in paths {
let path = path_key.as_str().expect("path key must be a string");
let uri = resolve_path(path);
let install_only = path_item_has_tag(item, "Install");
let api_path = path.starts_with("/api/");
for method in methods_for_path_item(item) {
if install_only {
let response = normal_app
.clone()
.oneshot(request_for(&method, &uri))
.await
.unwrap();
assert_eq!(
response.status(),
StatusCode::NOT_FOUND,
"Install route {method} {path} should be absent from the normal router"
);
} else if api_path {
let response = install_app
.clone()
.oneshot(request_for(&method, &uri))
.await
.unwrap();
assert_eq!(
response.status(),
StatusCode::NOT_FOUND,
"Normal API route {method} {path} should be absent from the install router"
);
}
}
}
}
// Note: the earlier `server_settings_keys_match_openapi_spec` drift check
// was deleted in Stage 6.3b alongside the legacy flat `fabro_types::Settings`
// struct that it instantiated. `/api/v1/settings` now returns dense
// `ServerSettings`, and `/api/v1/runs/:id/settings` returns a dense
// `WorkflowSettings` snapshot. Property-level conformance for those payloads
// lives in the `fabro-api` round-trip tests that pin the Rust types against
// the OpenAPI schema names.