fabro/lib/crates/fabro-server/src/worker_runtime.rs
fabro-sh-0530[bot] 9ee576690b
refactor: remove in-process IP allowlist and introduce WorkerRuntime (#444)
## Summary

This PR does two things: it removes the in-process inbound source-IP
allowlist entirely, and it lays the foundation for pluggable worker
compute backends by introducing a `WorkerRuntime` abstraction.

## IP allowlist removal

The `[server.ip_allowlist]` setting and its GitHub webhook overlay
(`[server.integrations.github.webhooks.ip_allowlist]`) have been removed
from config parsing, the settings API, and the OpenAPI spec. The
`ip_allowlist.rs` module (~600 lines including the `GitHubMetaResolver`,
middleware, and cache logic) is deleted.

**Migration:** Existing `settings.toml` files containing those keys will
now fail to parse as unknown fields. Source-IP restrictions should be
moved to a reverse proxy, firewall, VPN, Tailscale ACL, or cloud ingress
— as documented in the new security guidance.

`build_router_with_options` loses the `ip_allowlist_config:
Arc<IpAllowlistConfig>` parameter and `RouterOptions` loses
`github_webhook_ip_allowlist`. Call sites in tests and the auth harness
are updated accordingly. TCP serving no longer uses
`make_service_with_connect_info` since `ConnectInfo` was only needed for
IP extraction.

## WorkerRuntime abstraction

A new `worker_runtime.rs` module introduces:

- **`WorkerRuntime` trait** — `start`, `request_stop`, `force_stop`,
`is_alive`
- **`WorkerLaunchSpec`** — all inputs needed to describe a worker
process, replacing the former `worker_command` helper
- **`WorkerRef::Local { pid, process_group_id }`** — replaces the
`worker_pid` / `worker_pgid` pair on `ManagedRun`
- **`StartedWorker`** — carries the ref, optional stderr stream, and a
`wait` future
- **`LocalWorkerRuntime`** — the only implementation for now; wraps the
existing subprocess spawn logic

`AppState` stores an `Arc<dyn WorkerRuntime>` and `AppStateConfig`
accepts an optional override in `#[cfg(test)]` for injection. Stop/kill
paths in `server.rs` and `lifecycle.rs` now call
`worker_runtime.request_stop` / `force_stop` / `is_alive` instead of
issuing signals directly.

### Plan Summary

- **Task 1:** New `worker_runtime.rs` with trait, types, and
`LocalWorkerRuntime` impl
- **Task 2:** Wire `Arc<dyn WorkerRuntime>` into `AppState` /
`AppStateConfig` / `TestAppStateBuilder`
- **Task 3:** Replace `worker_pid` / `worker_pgid` on `ManagedRun` with
`worker_ref: Option<WorkerRef>`; build `WorkerLaunchSpec` in
`execute_run_subprocess`
- **Task 4:** Route all stop/kill calls through the runtime
(`terminate_worker_for_deletion`, `shutdown_active_workers`,
`cancel_run` fallback)
- **Task 5:** Fake `RecordingWorkerRuntime` for unit tests; new
cancel-fallback and shutdown tests


### Fabro Details

<details>
<summary>Ran 8 stages in 107m 33s for $21.66</summary>

| Stage | Duration | Cost | Retries |
|---|---|---|---|
| start | 0s | – | 0 |
| toolchain | 1s | – | 0 |
| preflight_compile | 2m 10s | – | 0 |
| preflight_lint | 2m 21s | – | 0 |
| implement | 29m 0s | $11.86 | 0 |
| simplify_opus | 11m 9s | $5.94 | 0 |
| simplify_gpt | 53m 13s | $3.85 | 0 |
| verify | 9m 23s | – | 0 |
| **Total** | **107m 33s** | **$21.66** | **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>
2026-05-27 23:17:23 -04:00

166 lines
5.3 KiB
Rust

use std::path::PathBuf;
use std::pin::Pin;
use std::process::Stdio;
use anyhow::{Context, Result};
use async_trait::async_trait;
use fabro_static::EnvVars;
use fabro_types::RunId;
use fabro_types::settings::server::LogDestination;
use futures_util::future::BoxFuture;
use tokio::io::AsyncRead;
use tokio::process::Command;
use crate::spawn_env::apply_worker_env;
#[async_trait]
pub(crate) trait WorkerRuntime: Send + Sync {
async fn start(&self, spec: WorkerLaunchSpec) -> Result<StartedWorker>;
async fn request_stop(&self, worker_ref: &WorkerRef);
async fn force_stop(&self, worker_ref: &WorkerRef);
async fn is_alive(&self, worker_ref: &WorkerRef) -> bool;
}
#[derive(Clone, Debug, PartialEq, Eq)]
pub(crate) enum WorkerRef {
/// A worker running as a local subprocess. `pre_exec_setpgid` ensures the
/// child is the leader of its own process group with `pgid == pid`, so a
/// single PID identifies both the process and its group.
Local { pid: u32 },
}
pub(crate) struct WorkerLaunchSpec {
pub(crate) executable: PathBuf,
pub(crate) server_target: String,
pub(crate) storage_dir: PathBuf,
pub(crate) run_dir: PathBuf,
pub(crate) run_id: RunId,
pub(crate) mode: &'static str,
pub(crate) worker_token: String,
pub(crate) log_destination: LogDestination,
pub(crate) fabro_log: Option<String>,
pub(crate) active_config_path: PathBuf,
pub(crate) github_app_private_key: Option<String>,
}
pub(crate) struct StartedWorker {
pub(crate) worker_ref: WorkerRef,
pub(crate) stderr: Pin<Box<dyn AsyncRead + Send + 'static>>,
pub(crate) wait: BoxFuture<'static, Result<WorkerExit>>,
}
#[derive(Debug)]
pub(crate) struct WorkerExit {
pub(crate) success: bool,
pub(crate) detail: String,
}
#[derive(Default)]
pub(crate) struct LocalWorkerRuntime;
impl LocalWorkerRuntime {
pub(crate) fn new() -> Self {
Self
}
pub(crate) fn command_for_spec(spec: &WorkerLaunchSpec) -> Command {
let worker_stdout = match spec.log_destination {
LogDestination::Stdout => Stdio::inherit(),
LogDestination::File => Stdio::null(),
};
let log_destination_env: &'static str = spec.log_destination.into();
let mut cmd = Command::new(&spec.executable);
cmd.arg("__run-worker")
.arg("--server")
.arg(&spec.server_target)
.arg("--storage-dir")
.arg(&spec.storage_dir)
.arg("--run-dir")
.arg(&spec.run_dir)
.arg("--run-id")
.arg(spec.run_id.to_string())
.arg("--mode")
.arg(spec.mode)
.stdin(Stdio::null())
.stdout(worker_stdout)
.stderr(Stdio::piped());
apply_worker_env(&mut cmd);
if let Some(level) = spec.fabro_log.as_deref() {
cmd.env(EnvVars::FABRO_LOG, level);
}
cmd.env(EnvVars::FABRO_LOG_DESTINATION, log_destination_env);
cmd.env(EnvVars::FABRO_CONFIG, &spec.active_config_path);
cmd.env_remove(EnvVars::FABRO_WORKER_TOKEN);
cmd.env(EnvVars::FABRO_WORKER_TOKEN, &spec.worker_token);
if let Some(pem) = spec.github_app_private_key.as_deref() {
cmd.env(EnvVars::GITHUB_APP_PRIVATE_KEY, pem);
}
#[cfg(unix)]
fabro_proc::pre_exec_setpgid(cmd.as_std_mut());
cmd
}
}
#[async_trait]
impl WorkerRuntime for LocalWorkerRuntime {
async fn start(&self, spec: WorkerLaunchSpec) -> Result<StartedWorker> {
let mut child = Self::command_for_spec(&spec)
.spawn()
.context("spawning run worker process")?;
let Some(pid) = child.id() else {
let _ = child.start_kill();
anyhow::bail!("worker process did not report a PID");
};
let Some(stderr) = child.stderr.take() else {
let _ = child.start_kill();
anyhow::bail!("worker child stderr should be piped");
};
let stderr: Pin<Box<dyn AsyncRead + Send + 'static>> = Box::pin(stderr);
let wait: BoxFuture<'static, Result<WorkerExit>> = Box::pin(async move {
let status = child.wait().await.context("worker wait failed")?;
Ok(WorkerExit {
success: status.success(),
detail: status.to_string(),
})
});
Ok(StartedWorker {
worker_ref: WorkerRef::Local { pid },
stderr,
wait,
})
}
async fn request_stop(&self, worker_ref: &WorkerRef) {
let WorkerRef::Local { pid } = worker_ref;
#[cfg(unix)]
fabro_proc::sigterm_process_group(*pid);
#[cfg(not(unix))]
let _ = pid;
}
async fn force_stop(&self, worker_ref: &WorkerRef) {
let WorkerRef::Local { pid } = worker_ref;
#[cfg(unix)]
fabro_proc::sigkill_process_group(*pid);
#[cfg(not(unix))]
let _ = pid;
}
async fn is_alive(&self, worker_ref: &WorkerRef) -> bool {
let WorkerRef::Local { pid } = worker_ref;
#[cfg(unix)]
{
fabro_proc::process_group_alive(*pid)
}
#[cfg(not(unix))]
{
fabro_proc::process_running(*pid)
}
}
}