Reuse `IpAllowEntry::parse_literal` instead of duplicating `IpNet`
parsing in the resolver, and drop the unreachable defensive branch
in `expand_ip_allow_entries` that called `unwrap_or_default` on a
value that is always `Some` once an entry needs GitHub hooks.
Adds a middleware test covering X-Forwarded-For routing with a
non-zero trusted proxy count, which previously relied on
`extract_client_ip` unit tests alone.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Validate the effective GitHub webhook overlay for Unix listeners,
reuse cached GitHub /meta hook ranges when refresh fails, and
propagate webhook allowlist resolution errors during startup instead of
silently skipping the listener.
Introduces a configurable IP allowlist applied to the main API router
and the GitHub webhook listener. Supports CIDR literals plus a
`github_meta_hooks` keyword that resolves live against GitHub's meta
API for the webhooks override. Adds trusted-proxy handling for
X-Forwarded-For, validation that rejects Unix socket listeners without
a trusted proxy count, and deep-merge logic for the new
server.ip_allowlist and per-integration override layers.
Build a conservative CSP from an inventory of what the embedded SPA
actually loads today: same-origin scripts/styles, Google Fonts CSS and
font files, data: + blob: for images, blob: for workers, and WASM
(viz-js needs wasm-unsafe-eval for Graphviz rendering).
Inline `<script>` hashes are extracted at server startup from the
embedded index.html, so the theme-bootstrap script doesn't drift from
the policy when the template changes. Tests cover:
- known-body hash stability
- whitespace preservation (browsers hash raw bytes between tags)
- external scripts are skipped (they're covered by script-src 'self')
- the embedded SPA template actually yields at least one hash
- the final policy includes the expected directives
Ships as Content-Security-Policy-Report-Only for the initial rollout.
Browsers report violations to DevTools without blocking anything, so
real-world usage surfaces any false positives before we flip to
enforcing. When reports are clean, swap the header name to
Content-Security-Policy in security_headers::apply_csp.
CSP notes:
- 'unsafe-inline' on style-src is a pragmatic concession for React
and Tailwind runtime-injected inline styles. Script-src remains
strict (hash-based).
- No 'strict-dynamic' — the entry chunks are same-origin and covered
by 'self'. Can be added later if dynamic script injection
violations appear.
- No report endpoint wired up yet. DevTools console is sufficient
for the tuning phase; add report-to + collector later.
fabro-server previously sent no security headers beyond content-type
and cache-control. Add a tower middleware that fills in a conservative
default set on every response, preserving any header the handler
already set so routes can still override.
Always applied:
- X-Content-Type-Options: nosniff
- X-Frame-Options: DENY
- Referrer-Policy: strict-origin-when-cross-origin
- Cross-Origin-Opener-Policy: same-origin
- Cross-Origin-Resource-Policy: same-origin
- Permissions-Policy: (deny sensor/payment/xr APIs)
- X-Download-Options: noopen
- X-Permitted-Cross-Domain-Policies: none
- X-XSS-Protection: 0 (current OWASP guidance — the legacy filter
has known bypasses; CSP is the proper replacement)
- Cache-Control: no-store (default; asset routes keep their own)
- Pragma: no-cache
- Vary: Accept-Encoding
Applied only when the request reached an HTTPS edge (direct TLS or
X-Forwarded-Proto: https from a reverse proxy):
- Strict-Transport-Security: max-age=63072000; includeSubDomains
CSP is deliberately not included — it needs a dedicated audit of the
SPA's script/style/font/connect sources and isn't a drop-in header.
Filed as a separate follow-up.
Tests cover each applied header, non-override behavior against the
static-file cache-control, HSTS gating on X-Forwarded-Proto (including
the chained "https, http" leftmost-wins case), and an integration test
against a live router confirming both API and SPA responses carry the
headers.
The static-file fallback previously served index.html (25KB of UI
shell) for any unknown non-/api/v1/ GET — including `curl /healthz`,
scripted fetches, and typos under /api/. Two problems:
1. Unregistered paths like /api/v2/foo or /api/healthz bypassed the
router (which only matched /api/v1/) and fell through to the SPA
fallback, silently returning HTML for API typos.
2. Non-browser clients got the UI shell back for any misspelled path,
making deploy healthchecks, load balancer probes, and API clients
unable to distinguish "route missing" from "server healthy".
Broaden the dispatch guard to route /api/* through the axum Router so
unknown API paths return a clean 404 from the router itself. Gate the
SPA's index.html fallback on `Accept: text/html` so only browser
navigations (which deep-link to client-side routes like /runs/abc123)
get the UI shell; curl/fetch/scripts get 404.
Asset serving is unchanged — favicon.ico, /assets/*, etc. still serve
normally regardless of Accept header; the gate only applies to the
fallback after an asset lookup misses.
Tests: unit coverage for accepts_html + integration tests for the new
404 shape on /setup without Accept and on /api/v2/nonexistent even
with Accept: text/html.
Integration tests spawn the fabro binary as a subprocess and call
env_clear() for isolation, which strips LLVM_PROFILE_FILE. Under
cargo-llvm-cov this dropped subprocess coverage into orphaned
default.profraw files in tempdirs instead of the merged profile.
Add a preserve_coverage_env! macro in fabro-test and call it after
each env_clear() in apply_test_isolation, LightweightCli, and the
exec.rs sites. No-op when the env var is unset (normal test runs).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Non-release builds now append the profile to `fabro --version`
(`x.y (sha date debug)`), `fabro version`, and `fabro system info`,
so users can tell a local build apart from a shipped release. The
API's `SystemInfoResponse` gains a `profile` field so the client
can render the server's build profile too.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
When stderr is a TTY and text output is used, print a yellow `warning:`
line on stderr if the server reports a version that differs from the
client. JSON output and non-interactive contexts stay silent.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Flush the async store logger in workflow test helpers before returning so
callers that reopen the run store immediately do not observe partial state.
This removes the race behind the Linux git checkpoint CI failure.
Path<(String, String)> percent-decodes segments, so an authenticated
user could send owner=foo%2F..%2Fuser (decoded to foo/../user). After
reqwest URL normalization this rewrote the GitHub API endpoint and
reissued the server's privileged token against an unintended path.
Reject anything outside [A-Za-z0-9._-] with length caps, plus the
literals "." and "..".
API keys in query strings leak to access logs, proxies, and request
traces. Move to the header form Google documents as equivalent for both
generateContent and streamGenerateContent endpoints.
Only the server reads this cookie (via cookie_and_demo_middleware), so
HttpOnly is safe unconditionally. Secure is gated on https:// web.url to
match the existing session cookie pattern — preserves localhost HTTP dev.
CLI integration tests spawned the real fabro binary while letting the
parent process's env pass through. The pr_view "no credentials" snapshot
failed in CI because the Nightly workflow's minted GITHUB_TOKEN was
inherited by the child and turned the expected "credentials required"
error into a real GitHub API call (404 / 401). On developer laptops the
same leak occurs whenever gh auth login is active.
Introduce apply_test_isolation(cmd, home) in fabro-test: env_clear() +
re-populate PATH, HOME, NO_COLOR, and the FABRO_* test overrides. Route
TestContext::command(), the internal server bootstrap, and the four
ad-hoc spawners in tests/it/cmd/{attach,render_graph,runner,server_start}
through the same helper so the isolation is systemic instead of
per-callsite. Tests that deliberately need a credential (OPENAI_API_KEY,
GITHUB_APP_PRIVATE_KEY, etc.) continue to set it explicitly on the
returned Command; those survive the clear.
Add a regression test that sets sentinel GITHUB_TOKEN and
ANTHROPIC_API_KEY in the parent, spawns /usr/bin/env through the helper,
and asserts the child sees neither credential while still seeing PATH
and the harness's FABRO_NO_UPGRADE_CHECK override.
Verified: the full workspace (4022 tests) passes with GITHUB_TOKEN and
ANTHROPIC_API_KEY set in the parent, which previously broke the
pr_view_reads_pull_request_from_store_without_pull_request_json
snapshot. cargo fmt and nightly clippy are clean.
daytona-sdk transitively pulls native-tls via reqwest (its own
reqwest v0.12, separate from our rustls-configured workspace
reqwest v0.13). native-tls requires libssl headers at build time,
which musl-gcc cannot satisfy from the host's glibc libssl-dev.
Add a target-specific openssl dep with the vendored feature so
openssl-sys compiles openssl from source for musl builds. glibc
builds are unaffected — they continue to link against the system
libssl that CI runners already have.
Verified end-to-end: aarch64-unknown-linux-musl binary built locally
runs on Alpine 3.20 (pure musl userspace).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The spec declared openapi 3.1.0 but used nullable: true (3.0 idiom)
in 78 places, which Mintlify's parser rejected, breaking doc deploys.
Convert to proper 3.1 patterns (type arrays and oneOf with type: null),
switch the server conformance test from openapiv3 (3.0-only) to a
YAML-level walk so it accepts 3.1 input, and regenerate the typescript
client — it now correctly emits `| null` on nullable fields.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Extend the release matrix to two statically-linked musl variants so
Alpine and other musl-based Linux hosts can install without glibc.
Homebrew and the Docker image remain glibc-only.
- release.yml: add x86_64-unknown-linux-musl (ubuntu-24.04) and
aarch64-unknown-linux-musl (ubuntu-24.04-arm) matrix rows with
musl-tools, CC_*_musl, CARGO_TARGET_*_LINKER, and LIBZ_SYS_STATIC
- Cargo.toml: enable git2 vendored-libgit2 so libgit2 compiles from
source for every target (needed because musl cannot link against
Ubuntu's glibc-built libgit2-dev)
- install.sh: check `ldd --version` for "musl" and rewrite the target
from -gnu to -musl so Alpine users get the right tarball
- upgrade.rs: add detect_linux_libc() / parse_ldd_libc() helper and
route detect_target() Linux arms through it, with unit tests
covering glibc, musl, empty, and unknown output
- tests/it: extend target regex in the dry-run snapshot filter
Ubuntu 24.04 is required for the musl runner: 22.04 ships musl 1.2.2
which SIGSEGVs statically-linked x86_64 test binaries at startup.
Confirmed against graphviz-sys CI before landing here.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Clippy flagged hard_link_or_copy's match as single_match_else;
rewrite as an early-return if. rustfmt reformatted the long
chained path join in brew_command and the multi-arg
hard_link_or_copy call.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add process-level upgrade tests that invoke the real fabro binary from a
fake Homebrew Cellar path so current_exe() detection is exercised end to
end. Update the CLI reference and changelog to document the Homebrew-managed
upgrade path and the flags that remain self-managed-only.
Detect Homebrew-managed installs from the canonicalized executable path
(Cellar/fabro[-nightly]/...) and branch both the background nag and
`fabro upgrade` accordingly.
- Background check fetches the tap's versions.json (raw.githubusercontent)
for the matching channel instead of GitHub's latest release, so the nag
tracks what `brew update` can satisfy.
- Cache payload gains an install_source tag; entries from a different
source (or legacy entries without the field) are ignored, so users who
switch between fabro/fabro-nightly/tarball don't see stale versions.
- `fabro upgrade` on a brew install refuses to overwrite the
Homebrew-managed binary and prints `brew upgrade fabro[-nightly]`.
--dry-run prints the command and exits 0; --version/--prerelease/--force
are rejected with a clear Homebrew-managed error.
- Manifest/exe-resolve failures on brew installs skip the notice rather
than falling back to GitHub latest (which would reintroduce the
tap-lag false positives this change is meant to remove).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Move the shared session lock file out of the deletable session root so
cleanup cannot unlink the lock another test process is relying on.
This hardens the nextest shared-server harness against dev-token startup
races and adds a regression test for the lock path.
A recent CI flake surfaced as bare "error: No such file or directory
(os error 2)" with no chain, because the failing operation lived behind
a raw `?` on a `std::fs::` / `File::create` / `Command::spawn` call. The
error had no verb, no path, no hint at which step in server startup
broke. Retry loops were explicitly rejected -- the goal is to diagnose
the next occurrence, not mask it.
Wraps 50+ such sites across fabro-cli, fabro-server, fabro-workflow,
fabro-util, fabro-vault, fabro-telemetry, fabro-interview, fabro-llm,
and fabro-devcontainer with `.with_context(|| format!("<verb> {path}"))`
so anyhow's error chain carries both the operation and the path when
an io error escapes.
Where the enclosing function returns `io::Result` (fabro-util run_log,
fabro-interview recording, fabro-llm attachment loader), the error is
re-wrapped via `io::Error::new` to keep the signature stable. Where a
crate uses its own thiserror enum, either a new `io_context` helper
was added (fabro-vault) or the path was folded into the existing
`Error::Io(String)` message (fabro-workflow).
No retry loops. No behavior changes. Skipped sites documented:
`.ok()`-swallowed, `match ErrorKind::NotFound`, `let _ = ...`, typed
error variants that already carry the path, and test modules.
Verified: cargo build --workspace, cargo +nightly clippy --workspace
--all-targets -- -D warnings, cargo nextest run --workspace (3991/3991
pass).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two fabro-telemetry sender tests assert that upload/upload_blocking
return an error with "SEGMENT_WRITE_KEY not set" -- a claim that only
holds when SEGMENT_WRITE_KEY is absent at compile time. The release
workflow sets the secret at build time, so these tests now fail under
`cargo nextest run --workspace` in release CI (newly exercised after
switching the release workflow from `cargo test` to nextest).
Guard each assertion with an early return when SEGMENT_WRITE_KEY is
compiled in so the test passes in release CI while still verifying the
no-key path for every other build.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Surfaces the docs website shortcut alongside the other onboarding
commands so new users can find the web docs without hunting through
`fabro help`.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Replace the three-line "help along the way" trio with a Discord
callout in the style of qlty's landing: one line pointing at per-
command --help, one inviting users to the Fabro Discord.
- New final line uses qlty's dim/cyan split: "For a full list of
commands, run `fabro help`."
- Drop `sandbox cp` and `sandbox preview` from the curated list;
`sandbox ssh` stays. Both are still reachable via `fabro help`.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Moves `server start` and `secret set` into Set up so the first section
covers everything a user does once before they have a working install.
Drops `secret list` from the landing — `fabro --help` still surfaces it.
Merges the run-inspection commands (`logs`, `sandbox ssh`, `sandbox
preview`, `sandbox cp`) under a single "Inspect runs" heading, since
they all answer "what happened in this run?".
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Running `fabro` with no subcommand now prints a short, colorized guide
highlighting the most important commands (install, doctor, repo init,
validate, preflight, run, logs, server start, secret set/list, sandbox
ssh/preview/cp) instead of clap's full --help dump.
`fabro --help` and `fabro help` still render clap's comprehensive
reference unchanged. The CLI makes the root subcommand optional and
intercepts the None case in main_inner before telemetry or logging
init; the empty command name also suppresses the "CLI Executed"
tracking event for this pseudo-command.
Includes an inline-snapshot IT test covering the full landing body and
updates four pre-existing usage-line snapshots (<COMMAND> → [COMMAND])
that reflect the now-optional subcommand.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
In release builds with SEGMENT_WRITE_KEY baked in (i.e. CI), the
post-command telemetry flush calls spawn_fabro_subcommand, which in turn
does create_dir_all(~/.fabro/tmp) and writes a JSONL event file. That
silently undoes the directory removal that `fabro uninstall --yes` just
performed — leaving a stray ~/.fabro/tmp/ behind and breaking the
uninstall integration tests on CI release runs.
Fixes:
- run_uninstall calls fabro_telemetry::shutdown() before removal so the
buffered "CLI Executed" track in main() can't be queued or flushed,
and the background thread can't spawn the sender subprocess.
- TestContext::command() exports FABRO_TELEMETRY=off so test subprocesses
never initialise telemetry at all, as a belt-and-suspenders guard.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- server_start::help: correct --watch-web description indent from 14 to 10 spaces
in the stripping regex so the filter matches in debug builds where the flag
is present in --help output.
- fabro-telemetry: split telemetry_level default test into debug/release
variants. The function's default depends on cfg!(debug_assertions), so the
prior single test panicked under cargo test --release.
Verified: cargo nextest run --workspace (debug) → 3990 passed; cargo test
--workspace --release → all 69 test binaries pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Update VERSION filter regex to handle prerelease suffixes (e.g., 0.204.0-beta.1)
- Add VERSION filter to JSON snapshots in fabro_json_snapshot macro
- Fix attach test to use [VERSION] placeholder instead of hardcoded version
- Update upgrade help snapshot to include new --prerelease flag
- Change fake version in upgrade test from v0.176.3 to v999.0.0 to avoid collision
- Strip --watch-web from server start help (debug-only flag, varies by build)
- Gate test_panic module with #[cfg(debug_assertions)] (debug-only command)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Widens the candidate set to include prereleases, picks max semver across
stable + prereleases. Falls back to /releases/latest if no parseable
non-draft tag is returned. Conflicts with --version. Background
auto-upgrade notice remains stable-only.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The fake /bin/sh script in the render_error protocol test printed and
exited without reading stdin, which raced the parent's write_all on
Linux — EPIPE would surface as ChildCrashed (500) instead of the
RenderFailed path (400) the test asserts. macOS pipe buffering masked
the race. Adding `cat >/dev/null` mirrors the sibling
protocol_violation test and makes the child consume the DOT input
before printing the RENDER_ERROR line.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- rust.yml: move clippy to nightly-2026-04-14 (was stable); also pin
fmt to the same nightly date for consistency. Both jobs now use the
dated nightly and the run-step uses `cargo +nightly-2026-04-14 ...`.
- AGENTS.md: update developer commands to match CI.
- Duration constructors: replace `Duration::from_secs(N * 60)` /
`Duration::from_millis(N * 1000)` with `from_mins` / `from_secs` /
`from_hours` across the workspace to satisfy clippy's new
`duration_suboptimal_units` lint. std::time::Duration only — custom
`settings::duration::Duration` sites kept on `from_secs`.
- map/unwrap_or cleanup: `.map(f).unwrap_or(v)` → `.map_or(v, f)`,
`.map(f).unwrap_or(false)` on Result → `.is_ok_and(f)`, per
`clippy::map_unwrap_or`.
- Misc lints: collapse nested `if` into match guard in
handler/llm/api.rs and run_state.rs; replace `columns.len() > 0`
with `!columns.is_empty()`; switch a pair of `sort_by` calls to
`sort_by_key`.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- serve.rs: annotate debug-only `bun run dev` spawn with
#[expect(clippy::disallowed_methods, ...)] and add the missing
watch_web field to three ServeArgs test fixtures.
- install.rs: replace absolute `fabro_server::serve::DEFAULT_TCP_PORT`
path with `serve::DEFAULT_TCP_PORT` (use is already imported) to
satisfy clippy::absolute_paths.
- pagination test: request an explicit page[limit]=100 for the
"fits in one page" case instead of relying on the server default,
so the test stays robust as the built-in model catalog grows.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Stops any running server then starts a fresh one, passing through all
the same flags as `server start` (--watch-web, --foreground, etc.).
Works even if no server is currently running.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Spawns `bun run dev` in apps/fabro-web as a child of the server process,
so a single command starts both the API server and the web asset watcher.
The flag is gated behind #[cfg(debug_assertions)] and does not exist in
release builds.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
--web-url defaulted to http://localhost:3000 but merge_server_settings
hardcoded http://127.0.0.1:32276, causing GitHub OAuth redirect_uri
mismatch. Now both derive from the same --web-url flag (default:
http://127.0.0.1:32276 via DEFAULT_TCP_PORT).
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Print the HTTP URL (cyan) and enabled auth methods after server start,
so users can see at a glance how to access the server and what login
methods are available.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Update 5 more dry_run_examples snapshots to use [GRAPH_PATH] filter
- Skip LLM preflight check when graph has no LLM nodes (fixes
preflight_allows_pull_request_enabled_without_github_credentials)
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add [GRAPH_PATH] filter to run_output_filters so dry_run_simple
snapshot is path-independent
- Double fabro-cli slow-timeout (3s → 6s) to prevent ps test timeouts
- Preserve cloud sandboxes; bump snapshot to fabro-v7 with 8 CPU / 16GB
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The snapshot was overfit to local env — it hardcoded Anthropic models
passing because the server inherited ANTHROPIC_API_KEY from the test
runner. On CI with no API keys, all models are skipped and the snapshot
diverged.
Replace the snapshot with targeted assertions: exit code 0 and "Skipped"
appears in stderr. This works regardless of which credentials are
available in the test environment.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Config discovery walks from the workflow file's parent directory, so
tests using fixtures at their repo path (test/simple.fabro) would find
the repo's .fabro/project.toml. This caused settings like preserve=true
to leak into tests and break sandbox cleanup event assertions.
Add TestContext::install_fixture() which copies fixtures into the test's
temp dir. Update all CLI run/attach/start tests to use it. Remove the
now-unused example_fixture() function.
Restore preserve=true in .fabro/project.toml — tests are now isolated.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Remove EnvGuard and env-mutating test from fabro-auth (shared mutable state)
- Revert project.toml preserve=true that broke sandbox cleanup event tests
- Fix clippy: use is_some_and, scoped imports for StageStatus and render
- Update snapshot tests for new Run: ULID line and model_test output
- Fix preflight test assertion (name said "allows", asserted failure)
- Update cancel_queued_run test: cancelled runs now appear on board
- Add graph direction query param support to get_graph endpoint
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
StageFailedProps lacked duration_ms, so extract_stage_durations_from_events
only found durations from stage.completed events. Failed stages showed "0s"
in the sidebar despite the command.completed event having the real duration.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>