refactor(setup): remove browser-based GitHub app bootstrap

Make fabro install the only supported GitHub App setup path. This removes
HTTP endpoints and browser routes that mutated local server config, rewrites
/setup as an operator instructions page, and aligns the installer manifest
with the live GitHub OAuth callback and setup URLs.
This commit is contained in:
Bryan Helmkamp 2026-04-13 18:25:29 -04:00
parent 3692f0a6fa
commit f4bae6e9bc
13 changed files with 98 additions and 745 deletions

View file

@ -48,14 +48,6 @@ export async function apiJsonOrNull<T>(
return response.json() as Promise<T>;
}
export async function getSetupStatus(): Promise<{ configured: boolean }> {
const response = await fetch("/api/v1/setup/status", { credentials: "include" });
if (!response.ok) {
throw new Response(null, { status: response.status, statusText: response.statusText });
}
return response.json();
}
export async function getAuthConfig(): Promise<{ methods: string[] }> {
const response = await fetch("/api/v1/auth/config", { credentials: "include" });
if (!response.ok) {

View file

@ -25,4 +25,11 @@ describe("browser router", () => {
expect(paths).toContain("/login");
expect(paths).not.toContain("/auth/login");
});
test("exposes /setup but not the removed /setup/complete route", () => {
const paths = collectPaths(routes);
expect(paths).toContain("/setup");
expect(paths).not.toContain("/setup/complete");
});
});

View file

@ -9,7 +9,6 @@ import {
import Root, { ErrorBoundary as RootErrorBoundary } from "./root";
import * as RedirectHome from "./routes/redirect-home";
import * as Setup from "./routes/setup";
import * as SetupComplete from "./routes/setup-complete";
import * as AuthLogin from "./routes/auth-login";
import * as Start from "./routes/start";
import * as Workflows from "./routes/workflows";
@ -81,7 +80,6 @@ export const routes: RouteObject[] = [
children: [
indexRoute(RedirectHome),
route("setup", Setup),
route("setup/complete", SetupComplete),
route("login", AuthLogin),
{
loader: appShellLoader,

View file

@ -1,12 +1,7 @@
import { redirect } from "react-router";
import { getAuthMe, getSetupStatus } from "../api";
import { getAuthMe } from "../api";
export async function loader() {
const setup = await getSetupStatus();
if (!setup.configured) {
return redirect("/setup");
}
try {
await getAuthMe();
} catch (error) {

View file

@ -1,105 +0,0 @@
import { useEffect, useMemo, useRef, useState } from "react";
import { AuthLayout } from "../components/auth-layout";
type SetupState = "registering" | "done" | "error";
export default function SetupComplete() {
const code = useMemo(() => new URLSearchParams(window.location.search).get("code"), []);
const [state, setState] = useState<SetupState>(code ? "registering" : "done");
const [error, setError] = useState<string | null>(null);
const [restartRequired, setRestartRequired] = useState(false);
const registeredRef = useRef(false);
useEffect(() => {
if (registeredRef.current) return;
registeredRef.current = true;
async function register() {
if (!code) {
setState("done");
return;
}
const response = await fetch("/api/v1/setup/register", {
method: "POST",
credentials: "include",
headers: { "Content-Type": "application/json" },
body: JSON.stringify({ code }),
});
if (!response.ok) {
const payload = await response.json().catch(() => ({}));
setError(payload.error ?? "Setup registration failed.");
setState("error");
return;
}
const payload = await response.json().catch(() => ({}));
setRestartRequired(payload.restart_required === true);
setState("done");
window.history.replaceState({}, "", "/setup/complete");
}
void register();
}, [code]);
return (
<AuthLayout>
{state === "registering" && (
<>
<h1 className="text-center text-lg font-semibold text-fg">
Finishing setup
</h1>
<p className="mt-2 text-center text-sm text-fg-3">
Registering your GitHub App and writing local configuration.
</p>
</>
)}
{state === "done" && (
<>
<h1 className="text-center text-lg font-semibold text-fg">
Setup complete
</h1>
<p className="mt-2 text-center text-sm text-fg-3">
{restartRequired
? "Your GitHub App is configured. Restart the Fabro server before attempting login."
: "Your GitHub App has been registered and configured."}
</p>
{restartRequired ? (
<a
href="/setup"
className="mt-6 flex w-full items-center justify-center rounded-lg border border-line-strong px-4 py-2.5 text-sm font-medium text-fg-2 transition-colors hover:bg-overlay-strong"
>
Back to setup
</a>
) : (
<a
href="/login"
className="mt-6 flex w-full items-center justify-center rounded-lg bg-teal-500 px-4 py-2.5 text-sm font-medium text-white transition-colors hover:bg-teal-300"
>
Continue to sign in
</a>
)}
</>
)}
{state === "error" && (
<>
<h1 className="text-center text-lg font-semibold text-fg">
Setup failed
</h1>
<p className="mt-2 text-center text-sm text-fg-3">
{error}
</p>
<a
href="/setup"
className="mt-6 flex w-full items-center justify-center rounded-lg border border-line-strong px-4 py-2.5 text-sm font-medium text-fg-2 transition-colors hover:bg-overlay-strong"
>
Try again
</a>
</>
)}
</AuthLayout>
);
}

View file

@ -1,58 +1,45 @@
import { useMemo } from "react";
import { AuthLayout } from "../components/auth-layout";
export default function Setup() {
const manifest = useMemo(() => {
const baseUrl = window.location.origin;
const suffix = Math.random().toString(16).slice(2, 8);
return JSON.stringify({
name: `Fabro-${suffix}`,
url: "https://fabro.sh",
redirect_url: `${baseUrl}/setup/complete`,
callback_urls: [`${baseUrl}/auth/callback/github`],
setup_url: `${baseUrl}/setup/complete`,
public: false,
default_permissions: {
contents: "write",
metadata: "read",
pull_requests: "write",
checks: "write",
issues: "write",
emails: "read",
},
default_events: [],
});
}, []);
export default function Setup() {
return (
<AuthLayout>
<AuthLayout footer="GitHub App setup is managed from the terminal, not the browser.">
<h1 className="text-center text-lg font-semibold text-fg">
Set up Fabro
</h1>
<p className="mt-2 text-center text-sm text-fg-3">
Register a GitHub App to enable OAuth login and repository access.
Run the installer on the same host that runs the Fabro server to
register a GitHub App and write local configuration.
</p>
<form
method="POST"
action="https://github.com/settings/apps/new"
className="mt-6"
>
<input type="hidden" name="manifest" value={manifest} />
<button
type="submit"
className="flex w-full items-center justify-center gap-2 rounded-lg bg-teal-500 px-4 py-2.5 text-sm font-medium text-white transition-colors hover:bg-teal-300"
>
<GitHubMark />
Register GitHub App
</button>
</form>
<div className="mt-6 space-y-4">
<div className="rounded-lg border border-line-strong bg-overlay px-4 py-3">
<p className="text-xs font-medium uppercase tracking-wide text-fg-muted">
1. Open a terminal on the server host
</p>
<pre className="mt-2 overflow-x-auto text-sm text-fg-2">
<code>fabro install</code>
</pre>
</div>
<div className="rounded-lg border border-line-strong bg-overlay px-4 py-3">
<p className="text-xs font-medium uppercase tracking-wide text-fg-muted">
2. Choose GitHub App setup
</p>
<p className="mt-2 text-sm text-fg-3">
The CLI opens GitHub, exchanges the manifest code, and writes the
required settings and secrets locally.
</p>
</div>
<div className="rounded-lg border border-line-strong bg-overlay px-4 py-3">
<p className="text-xs font-medium uppercase tracking-wide text-fg-muted">
3. Restart the server, then return to sign in
</p>
<a
href="/login"
className="mt-3 flex w-full items-center justify-center rounded-lg bg-teal-500 px-4 py-2.5 text-sm font-medium text-white transition-colors hover:bg-teal-300"
>
Continue to sign in
</a>
</div>
</div>
</AuthLayout>
);
}
function GitHubMark() {
return (
<svg width="18" height="18" viewBox="0 0 16 16" fill="currentColor">
<path d="M8 0C3.58 0 0 3.58 0 8c0 3.54 2.29 6.53 5.47 7.59.4.07.55-.17.55-.38 0-.19-.01-.82-.01-1.49-2.01.37-2.53-.49-2.69-.94-.09-.23-.48-.94-.82-1.13-.28-.15-.68-.52-.01-.53.63-.01 1.08.58 1.23.82.72 1.21 1.87.87 2.33.66.07-.52.28-.87.51-1.07-1.78-.2-3.64-.89-3.64-3.95 0-.87.31-1.59.82-2.15-.08-.2-.36-1.02.08-2.12 0 0 .67-.21 2.2.82.64-.18 1.32-.27 2-.27.68 0 1.36.09 2 .27 1.53-1.04 2.2-.82 2.2-.82.44 1.1.16 1.92.08 2.12.51.56.82 1.27.82 2.15 0 3.07-1.87 3.75-3.65 3.95.29.25.54.73.54 1.48 0 1.07-.01 1.93-.01 2.2 0 .21.15.46.55.38A8.013 8.013 0 0016 8c0-4.42-3.58-8-8-8z" />
</svg>
);
}

View file

@ -39,15 +39,18 @@ The rest of this page describes the `app` strategy, which is required for browse
### Prerequisites
- A GitHub account (personal or organization)
- The Fabro web app running (`cd apps/fabro-web && bun run dev`)
- Shell access to the host that runs the Fabro server
- The Fabro web app running (`cd apps/fabro-web && bun run dev`) if you want browser sign-in after setup
### Register the GitHub App
### Register the GitHub App with `fabro install`
1. Navigate to the web app (default `http://localhost:3000`). If no GitHub App is configured, you'll be redirected to the setup page automatically.
Run the installer on the same machine that will run the Fabro server:
2. Choose where to register the app. If you have the `gh` CLI installed, Fabro detects your GitHub username and any organizations you administer and lets you pick. For organization-owned apps, the app is registered under that org's settings. If `gh` is not available, the app is registered under your personal account.
```bash
fabro install
```
3. Click **Register GitHub App**. This takes you to GitHub with a pre-filled [App Manifest](https://docs.github.com/en/apps/sharing-github-apps/registering-a-github-app-from-a-manifest) containing:
When you choose the GitHub App strategy, the CLI opens GitHub with a pre-filled [App Manifest](https://docs.github.com/en/apps/sharing-github-apps/registering-a-github-app-from-a-manifest) containing:
| Permission | Level | Purpose |
|---|---|---|
@ -58,15 +61,20 @@ The rest of this page describes the `app` strategy, which is required for browse
| Issues | Write | Create issues from workflows |
| Emails | Read | Read verified email for OAuth login |
4. Review the permissions on GitHub and click **Create GitHub App**.
The installer:
5. GitHub redirects back to Fabro, which automatically:
- Exchanges the temporary code for permanent app credentials
- Writes `app_id`, `client_id`, and `slug` to `~/.fabro/settings.toml`
- Stores `GITHUB_APP_CLIENT_SECRET`, `GITHUB_APP_WEBHOOK_SECRET`, and `GITHUB_APP_PRIVATE_KEY` in `<data_dir>/server.env`
- Marks the change as restart-bound so the server must be restarted before login
1. Lets you choose where to register the app. If you have the `gh` CLI installed, Fabro detects your GitHub username and any organizations you administer and lets you pick. If `gh` is not available, the app is registered under your personal account.
2. Opens GitHub in your browser so you can review the manifest and click **Create GitHub App**.
3. Receives GitHub's temporary callback on localhost, exchanges it for permanent app credentials, and writes:
- `app_id`, `client_id`, and `slug` to `~/.fabro/settings.toml`
- `GITHUB_APP_CLIENT_SECRET`, `GITHUB_APP_WEBHOOK_SECRET`, and `GITHUB_APP_PRIVATE_KEY` to `<data_dir>/server.env`
4. Prints the resulting app slug so you can install the app on the repositories Fabro should access.
6. **Install the app** on your GitHub account or organization. Go to `https://github.com/settings/apps/<your-app-slug>/installations` and install it on the repositories Fabro should access.
After setup completes, restart the Fabro server before attempting browser login.
### Install the app on GitHub
Go to `https://github.com/settings/apps/<your-app-slug>/installations` and install it on the repositories Fabro should access.
### Verify the configuration
@ -143,8 +151,7 @@ The web app uses the GitHub App's OAuth credentials to authenticate users:
Configure allowed users in `settings.toml`:
```toml title="settings.toml"
[web.auth]
provider = "github"
[server.auth.github]
allowed_usernames = ["alice", "bob"]
```
@ -218,8 +225,8 @@ The app is installed but doesn't have access to this specific repository. Update
### "GitHub App authentication failed"
The `app_id` in `settings.toml` or the `GITHUB_APP_PRIVATE_KEY` environment variable is incorrect. Re-run the setup flow or verify the values match your GitHub App.
The `app_id` in `settings.toml` or the `GITHUB_APP_PRIVATE_KEY` environment variable is incorrect. Re-run `fabro install` on the server host or verify the values match your GitHub App.
### Clone fails for private repositories
If you see `Git clone failed ... If this is a private repository, configure a GitHub App`, the GitHub App credentials are not configured. Run the setup flow through the web UI or verify with `fabro doctor`.
If you see `Git clone failed ... If this is a private repository, configure a GitHub App`, the GitHub App credentials are not configured. Run `fabro install` on the server host or verify with `fabro doctor`.

View file

@ -760,10 +760,10 @@ struct CallbackParams {
fn build_github_app_manifest(app_name: &str, port: u16, web_url: &str) -> serde_json::Value {
serde_json::json!({
"name": app_name,
"url": "https://github.com/apps/arc",
"url": "https://fabro.sh",
"redirect_url": format!("http://127.0.0.1:{port}/callback"),
"callback_urls": [format!("{web_url}/auth/callback")],
"setup_url": format!("{web_url}/setup/callback"),
"callback_urls": [format!("{web_url}/auth/callback/github")],
"setup_url": format!("{web_url}/setup"),
"public": false,
"default_permissions": {
"contents": "write",
@ -1680,13 +1680,14 @@ client_id = "client-id"
let web_url = "https://app.example.com";
let manifest = build_github_app_manifest("Fabro-test", 12345, web_url);
assert_eq!(manifest["url"], serde_json::json!("https://fabro.sh"),);
assert_eq!(
manifest["callback_urls"],
serde_json::json!(["https://app.example.com/auth/callback"]),
serde_json::json!(["https://app.example.com/auth/callback/github"]),
);
assert_eq!(
manifest["setup_url"],
serde_json::json!("https://app.example.com/setup/callback"),
serde_json::json!("https://app.example.com/setup"),
);
}

View file

@ -4,7 +4,7 @@ use std::time::Duration;
use anyhow::Context;
use clap::Args;
use fabro_config::user::{active_settings_path, load_settings_config};
use fabro_config::user::load_settings_config;
use fabro_config::{Storage, resolve_server_from_file};
use fabro_sandbox::SandboxProvider;
use fabro_types::settings::server::GithubIntegrationStrategy;
@ -88,10 +88,6 @@ fn load_settings(path: Option<&Path>) -> anyhow::Result<SettingsLayer> {
Ok(load_settings_config(path)?)
}
fn resolved_config_path(path: Option<&Path>) -> PathBuf {
active_settings_path(path)
}
fn apply_serve_overrides(
base: &SettingsLayer,
args: &ServeArgs,
@ -258,7 +254,6 @@ where
let config_path = args.config.clone();
let disk_settings = load_settings(config_path.as_deref())?;
let disk_server_settings = resolve_server_settings(&disk_settings)?;
let active_config_path = resolved_config_path(config_path.as_deref());
let data_dir = match storage_dir_override {
Some(path) => path,
None => resolve_interp_path(&disk_server_settings.storage.root)?,
@ -329,7 +324,6 @@ where
store,
artifact_store,
&vault_path,
active_config_path,
true,
)?;
let reconciled = reconcile_incomplete_runs_on_startup(&state).await?;

View file

@ -524,7 +524,6 @@ pub struct AppState {
pub(crate) provider_credentials: ProviderCredentials,
pub(crate) settings: Arc<RwLock<SettingsLayer>>,
pub(crate) server_settings: RwLock<Arc<ResolvedServerSettings>>,
pub(crate) config_path: PathBuf,
pub(crate) local_daemon_mode: bool,
shutting_down: AtomicBool,
registry_factory_override: Option<Box<RegistryFactoryOverride>>,
@ -712,11 +711,6 @@ impl AppState {
.expect("server settings lock poisoned") = resolved;
Ok(())
}
pub(crate) fn reload_settings_from_disk(&self) -> anyhow::Result<()> {
let reloaded = fabro_config::load_settings_path(&self.config_path)?;
self.replace_settings(reloaded)
}
}
fn artifact_upload_token_keys() -> ArtifactUploadTokenKeys {
@ -858,6 +852,10 @@ impl Default for RouterOptions {
}
}
fn removed_web_route(path: &str) -> bool {
matches!(path, "/setup/complete")
}
/// Build the axum Router with configurable web surface routing.
pub fn build_router_with_options(
state: Arc<AppState>,
@ -931,6 +929,8 @@ pub fn build_router_with_options(
|| (options.web_enabled && path.starts_with("/auth/"));
if dispatch_path {
dispatch.oneshot(req).await
} else if options.web_enabled && removed_web_route(&path) {
Ok::<_, std::convert::Infallible>(StatusCode::NOT_FOUND.into_response())
} else if options.web_enabled
&& matches!(req.method(), &Method::GET | &Method::HEAD)
{
@ -2155,7 +2155,6 @@ pub fn create_app_state_with_settings_and_registry_factory(
store,
artifact_store,
&test_secret_store_path(),
test_config_path(),
false,
)
.expect("test app state should build")
@ -2201,7 +2200,6 @@ pub(crate) fn create_test_app_state_with_session_key(
store,
artifact_store,
&secrets_path,
test_config_path(),
local_daemon_mode,
)
.expect("test app state should build")
@ -2231,7 +2229,6 @@ pub fn create_app_state_with_store(
store,
artifact_store,
&test_secret_store_path(),
test_config_path(),
false,
)
.expect("test app state should build")
@ -2244,7 +2241,6 @@ pub(crate) fn build_app_state_with_path(
store: Arc<Database>,
artifact_store: ArtifactStore,
vault_path: &std::path::Path,
config_path: PathBuf,
local_daemon_mode: bool,
) -> anyhow::Result<Arc<AppState>> {
let vault = Arc::new(AsyncRwLock::new(Vault::load(vault_path.to_path_buf())?));
@ -2306,7 +2302,6 @@ pub(crate) fn build_app_state_with_path(
provider_credentials,
settings,
server_settings: RwLock::new(resolved_server_settings),
config_path,
local_daemon_mode,
shutting_down: AtomicBool::new(false),
registry_factory_override,
@ -2319,10 +2314,6 @@ fn test_secret_store_path() -> PathBuf {
std::env::temp_dir().join(format!("fabro-test-secrets-{}.json", Ulid::new()))
}
fn test_config_path() -> PathBuf {
std::env::temp_dir().join(format!("fabro-test-settings-{}.toml", Ulid::new()))
}
fn board_column(status: RunStatus) -> Option<&'static str> {
match status {
RunStatus::Running => Some("working"),

View file

@ -42,16 +42,6 @@ impl ServerSecrets {
pub(crate) fn get(&self, name: &str) -> Option<String> {
(self.env_lookup)(name).or_else(|| self.file_entries.get(name).cloned())
}
pub(crate) fn persist_updates<I, K, V>(&mut self, updates: I) -> Result<(), Error>
where
I: IntoIterator<Item = (K, V)>,
K: Into<String>,
V: Into<String>,
{
self.file_entries = envfile::merge_env_file(&self.path, updates)?;
Ok(())
}
}
impl std::fmt::Debug for ServerSecrets {

View file

@ -1,16 +1,12 @@
use std::sync::Arc;
use anyhow::{Context, anyhow};
use axum::extract::{Query, State};
use axum::http::{HeaderMap, HeaderValue, StatusCode, header};
use axum::response::{IntoResponse, Redirect, Response};
use axum::routing::{get, post};
use axum::{Extension, Json, Router};
use base64::Engine as _;
use base64::engine::general_purpose::STANDARD as BASE64_STANDARD;
use cookie::time::Duration;
use cookie::{Cookie, CookieJar, Key, SameSite};
use fabro_config::Storage;
use fabro_types::RunAuthMethod;
use fabro_types::settings::{InterpString, ServerAuthMethod, SettingsLayer};
use fabro_util::dev_token::validate_dev_token_format;
@ -18,10 +14,8 @@ use serde::{Deserialize, Serialize};
use serde_json::json;
use tracing::{debug, error, info, warn};
use crate::error::ApiError;
use crate::jwt_auth::{AuthMode, AuthenticatedSubject, auth_method_name, dev_token_matches};
use crate::jwt_auth::{AuthMode, auth_method_name, dev_token_matches};
use crate::server::AppState;
use crate::server_secrets::ServerSecrets;
pub const SESSION_COOKIE_NAME: &str = "__fabro_session";
const OAUTH_STATE_COOKIE_NAME: &str = "fabro_oauth_state";
@ -51,21 +45,11 @@ struct DevTokenLoginRequest {
token: String,
}
#[derive(Deserialize)]
struct SetupRegisterRequest {
code: String,
}
#[derive(Deserialize)]
struct DemoToggleRequest {
enabled: bool,
}
#[derive(Serialize)]
struct SetupStatusResponse {
configured: bool,
}
#[derive(Serialize)]
struct AuthConfigResponse {
methods: Vec<String>,
@ -111,16 +95,6 @@ struct GitHubEmail {
verified: bool,
}
#[derive(Deserialize)]
struct GitHubManifestConversion {
id: i64,
slug: String,
client_id: String,
client_secret: String,
webhook_secret: Option<String>,
pem: String,
}
pub fn routes() -> Router<Arc<AppState>> {
Router::new()
.route("/login/dev-token", post(login_dev_token))
@ -133,8 +107,6 @@ pub fn api_routes() -> Router<Arc<AppState>> {
Router::new()
.route("/auth/config", get(auth_config))
.route("/auth/me", get(auth_me))
.route("/setup/register", post(setup_register))
.route("/setup/status", get(setup_status))
.route("/demo/toggle", post(toggle_demo))
}
@ -669,20 +641,6 @@ async fn auth_me(State(state): State<Arc<AppState>>, headers: HeaderMap) -> Resp
.into_response()
}
async fn setup_status(
State(state): State<Arc<AppState>>,
Extension(auth_mode): Extension<AuthMode>,
) -> Response {
let configured = state
.server_settings()
.integrations
.github
.client_id
.is_some()
|| auth_method_enabled(&auth_mode, ServerAuthMethod::DevToken);
Json(SetupStatusResponse { configured }).into_response()
}
async fn toggle_demo(Json(payload): Json<DemoToggleRequest>) -> Response {
let mut jar = CookieJar::new();
jar.add(
@ -697,277 +655,6 @@ async fn toggle_demo(Json(payload): Json<DemoToggleRequest>) -> Response {
response
}
async fn setup_register(
subject: AuthenticatedSubject,
State(state): State<Arc<AppState>>,
headers: HeaderMap,
Json(payload): Json<SetupRegisterRequest>,
) -> Response {
if subject.auth_method != RunAuthMethod::DevToken {
return ApiError::forbidden().into_response();
}
let origin = headers
.get(header::ORIGIN)
.or_else(|| headers.get(header::REFERER))
.and_then(|v| v.to_str().ok())
.and_then(|s| fabro_http::Url::parse(s).ok())
.map(|url| format!("{}://{}", url.scheme(), url.authority()));
let http = match fabro_http::http_client() {
Ok(http) => http,
Err(err) => {
error!(error = %err, "Setup register failed: could not build GitHub HTTP client");
return json_response(
StatusCode::SERVICE_UNAVAILABLE,
json!({"error": format!("Failed to build GitHub HTTP client: {err}")}),
);
}
};
let response = match http
.post(format!(
"https://api.github.com/app-manifests/{}/conversions",
payload.code
))
.header(header::ACCEPT, "application/vnd.github+json")
.header(header::USER_AGENT, "fabro-server")
.send()
.await
{
Ok(response) => response,
Err(err) => {
error!(error = %err, "Setup register failed: GitHub manifest conversion request failed");
return json_response(
StatusCode::BAD_GATEWAY,
json!({"error": format!("GitHub manifest conversion failed: {err}")}),
);
}
};
if !response.status().is_success() {
let status = response.status();
let body = response.text().await.unwrap_or_default();
error!(status = %status, body = %body, "Setup register failed: GitHub manifest conversion returned error");
return json_response(
StatusCode::BAD_GATEWAY,
json!({"error": format!("GitHub manifest conversion failed: {status}")}),
);
}
let body = match response.text().await {
Ok(body) => body,
Err(err) => {
error!(error = %err, "Setup register failed: could not read conversion response body");
return json_response(
StatusCode::BAD_GATEWAY,
json!({"error": "Failed to read GitHub manifest conversion response"}),
);
}
};
let data = match serde_json::from_str::<GitHubManifestConversion>(&body) {
Ok(data) => data,
Err(err) => {
error!(error = %err, body = %body, "Setup register failed: could not parse conversion response");
return json_response(
StatusCode::BAD_GATEWAY,
json!({"error": format!("Failed to parse GitHub manifest conversion response: {err}")}),
);
}
};
let settings_path = state.config_path.clone();
// Edit the settings file in place via `toml_edit::DocumentMut`, which
// preserves existing comments, whitespace, and key ordering. The value-
// tree parser (`toml::Value`) would strip all of that on round-trip.
if let Some(parent) = settings_path.parent() {
if let Err(err) = std::fs::create_dir_all(parent) {
error!(error = %err, path = %parent.display(), "Setup register failed: could not create settings parent directory");
return json_response(
StatusCode::INTERNAL_SERVER_ERROR,
json!({"error": format!("Failed to create settings directory: {err}")}),
);
}
}
let existing = std::fs::read_to_string(&settings_path).unwrap_or_default();
let mut doc: toml_edit::DocumentMut = if existing.is_empty() {
toml_edit::DocumentMut::new()
} else {
match existing
.parse::<toml_edit::DocumentMut>()
.context("failed to parse existing settings config")
{
Ok(doc) => doc,
Err(err) => {
error!(error = %err, path = %settings_path.display(), "Setup register failed: could not parse settings config");
return json_response(
StatusCode::INTERNAL_SERVER_ERROR,
json!({"error": format!("Failed to parse settings config: {err}")}),
);
}
}
};
if let Err(err) = merge_settings_keys(&mut doc, &data, origin.as_deref()) {
error!(error = %err, "Setup register failed: could not merge settings");
return json_response(
StatusCode::INTERNAL_SERVER_ERROR,
json!({"error": format!("Failed to update settings config: {err}")}),
);
}
if let Err(err) = std::fs::write(&settings_path, doc.to_string()) {
error!(error = %err, path = %settings_path.display(), "Setup register failed: could not write settings config");
return json_response(
StatusCode::INTERNAL_SERVER_ERROR,
json!({"error": format!("Failed to write settings config: {err}")}),
);
}
let mut secret_updates = vec![
("GITHUB_APP_CLIENT_SECRET", data.client_secret.clone()),
(
"GITHUB_APP_PRIVATE_KEY",
BASE64_STANDARD.encode(data.pem.as_bytes()),
),
];
if let Some(ref webhook_secret) = data.webhook_secret {
secret_updates.push(("GITHUB_APP_WEBHOOK_SECRET", webhook_secret.clone()));
}
let server_env_path = Storage::new(state.server_storage_dir())
.server_state()
.env_path();
let mut server_secrets = match ServerSecrets::load(server_env_path.clone()) {
Ok(server_secrets) => server_secrets,
Err(err) => {
error!(error = %err, path = %server_env_path.display(), "Setup register failed: could not load server env");
return json_response(
StatusCode::INTERNAL_SERVER_ERROR,
json!({"error": format!("Failed to load server env: {err}")}),
);
}
};
if let Err(err) = server_secrets.persist_updates(secret_updates) {
error!(error = %err, path = %server_env_path.display(), "Setup register failed: could not write server env");
return json_response(
StatusCode::INTERNAL_SERVER_ERROR,
json!({"error": format!("Failed to write server env: {err}")}),
);
}
// Re-parse the freshly-written settings file and swap it into the
// in-memory state so the non-secret GitHub App config becomes visible
// immediately. Secret material remains restart-bound through server.env.
match state.reload_settings_from_disk() {
Ok(()) => {}
Err(err) => {
error!(error = %err, path = %settings_path.display(), "Setup register failed: could not reload written settings config");
return json_response(
StatusCode::INTERNAL_SERVER_ERROR,
json!({"error": format!("Failed to reload settings config after write: {err}")}),
);
}
}
info!(slug = %data.slug, app_id = %data.id, "GitHub App registered successfully");
Json(json!({"ok": true, "restart_required": true})).into_response()
}
/// Walk dotted `path` into `doc`, creating missing intermediate tables,
/// and return a mutable reference to the terminal table.
///
/// Uses `toml_edit`'s [`toml_edit::Entry::or_insert`] so existing tables
/// keep their comments, ordering, and any sibling keys untouched.
fn ensure_nested_table<'a>(
doc: &'a mut toml_edit::DocumentMut,
path: &[&str],
) -> anyhow::Result<&'a mut toml_edit::Table> {
let mut current: &mut toml_edit::Table = doc.as_table_mut();
for segment in path {
let next = current
.entry(segment)
.or_insert(toml_edit::Item::Table(toml_edit::Table::new()));
current = next
.as_table_mut()
.ok_or_else(|| anyhow!("settings config [{segment}] is not a table"))?;
}
Ok(current)
}
/// Set a scalar value on `table[key]`, preserving any key-level decor
/// (leading comments, blank lines) that was attached to the existing entry.
///
/// `toml_edit`'s default `Table::insert` replaces the entry wholesale and
/// drops its prefix decoration, which would strip a top-of-file comment
/// attached to a key we're updating. Copying the decor forward keeps the
/// user's formatting intact.
fn set_preserving_decor(table: &mut toml_edit::Table, key: &str, value: toml_edit::Item) {
let preserved_decor = table.key(key).map(|existing| existing.leaf_decor().clone());
table.insert(key, value);
if let (Some(decor), Some(mut updated)) = (preserved_decor, table.key_mut(key)) {
*updated.leaf_decor_mut() = decor;
}
}
fn merge_settings_keys(
doc: &mut toml_edit::DocumentMut,
data: &GitHubManifestConversion,
origin: Option<&str>,
) -> anyhow::Result<()> {
let web_url = origin.map_or_else(|| "http://localhost:3000".to_string(), str::to_string);
// Make sure the freshly-written file is a valid v2 file. `_version` is
// always `1` at the moment, so skip the write entirely if it's already
// there -- otherwise we'd trample any top-of-file comment attached to
// the key.
let root = doc.as_table_mut();
if !root.contains_key("_version") {
root.insert("_version", toml_edit::value(1_i64));
}
let web = ensure_nested_table(doc, &["server", "web"])?;
set_preserving_decor(web, "enabled", toml_edit::value(true));
set_preserving_decor(web, "url", toml_edit::value(web_url));
let auth = ensure_nested_table(doc, &["server", "auth"])?;
let mut methods = auth
.get("methods")
.and_then(|item| item.as_array())
.map(|array| {
array
.iter()
.filter_map(|value| value.as_str().map(str::to_string))
.collect::<Vec<_>>()
})
.unwrap_or_default();
if !methods.iter().any(|method| method == "github") {
methods.push("github".to_string());
}
if methods.is_empty() {
methods.push("github".to_string());
}
set_preserving_decor(
auth,
"methods",
toml_edit::value(
methods
.into_iter()
.map(toml_edit::Value::from)
.collect::<toml_edit::Array>(),
),
);
let github = ensure_nested_table(doc, &["server", "integrations", "github"])?;
set_preserving_decor(github, "app_id", toml_edit::value(data.id.to_string()));
set_preserving_decor(
github,
"client_id",
toml_edit::value(data.client_id.clone()),
);
set_preserving_decor(github, "slug", toml_edit::value(data.slug.clone()));
Ok(())
}
#[cfg(test)]
mod tests {
use std::sync::Arc;
@ -986,10 +673,7 @@ mod tests {
use serde_json::{Value, json};
use tower::ServiceExt;
use super::{
GitHubManifestConversion, SessionCookie, api_routes, merge_settings_keys,
read_private_session, routes,
};
use super::{SessionCookie, api_routes, read_private_session, routes};
use crate::jwt_auth::{AuthMode, ConfiguredAuth};
use crate::server;
@ -1036,19 +720,6 @@ mod tests {
}
}
fn encode_session_cookie(key: &Key, session: &SessionCookie) -> String {
let mut jar = cookie::CookieJar::new();
jar.private_mut(key).add(cookie::Cookie::new(
super::SESSION_COOKIE_NAME,
serde_json::to_string(session).unwrap(),
));
jar.delta()
.next()
.expect("private cookie should exist")
.encoded()
.to_string()
}
fn test_auth_router_with_settings(
settings: SettingsLayer,
auth_mode: AuthMode,
@ -1087,143 +758,6 @@ mod tests {
serde_json::from_slice(&to_bytes(response.into_body(), usize::MAX).await.unwrap()).unwrap()
}
fn sample_conversion() -> GitHubManifestConversion {
GitHubManifestConversion {
id: 123,
slug: "fabro".to_string(),
client_id: "abc".to_string(),
client_secret: "shh".to_string(),
pem: String::new(),
webhook_secret: None,
}
}
fn parse_doc(source: &str) -> toml_edit::DocumentMut {
source
.parse::<toml_edit::DocumentMut>()
.expect("fixture should parse as TOML")
}
#[test]
fn merge_settings_keys_writes_v2_server_integrations_github() {
let mut doc = parse_doc("_version = 1\n");
merge_settings_keys(&mut doc, &sample_conversion(), Some("https://example.test")).unwrap();
let methods = doc["server"]["auth"]["methods"]
.as_array()
.expect("server.auth.methods should exist");
assert_eq!(
methods.iter().next().and_then(|value| value.as_str()),
Some("github")
);
let github = doc["server"]["integrations"]["github"]
.as_table()
.expect("server.integrations.github should exist");
assert_eq!(github["app_id"].as_str(), Some("123"));
assert_eq!(github["slug"].as_str(), Some("fabro"));
assert_eq!(github["client_id"].as_str(), Some("abc"));
let web = doc["server"]["web"]
.as_table()
.expect("server.web should exist");
assert_eq!(web["url"].as_str(), Some("https://example.test"));
assert_eq!(web["enabled"].as_bool(), Some(true));
// Re-parse the emitted document to prove it round-trips into a
// valid v2 `SettingsLayer`.
let emitted = doc.to_string();
let file = fabro_config::parse_settings_layer(&emitted)
.expect("merged output should parse as a v2 SettingsLayer");
let server = file.server.as_ref().expect("[server] should be present");
let integrations = server
.integrations
.as_ref()
.expect("[server.integrations] should be present");
let github = integrations
.github
.as_ref()
.expect("[server.integrations.github] should be present");
assert_eq!(
github
.app_id
.as_ref()
.map(fabro_types::settings::InterpString::as_source),
Some("123".to_string())
);
}
#[test]
fn merge_settings_keys_preserves_comments_and_unrelated_keys() {
let existing = r##"# Top-of-file comment explaining the settings layout.
_version = 1
# Storage root comment — should survive the edit.
[server.storage]
root = "/srv/fabro-data"
# A pre-existing integration that is NOT github.
[server.integrations.slack]
default_channel = "#ops"
[run.model]
provider = "anthropic"
name = "claude-sonnet"
"##;
let mut doc = parse_doc(existing);
merge_settings_keys(
&mut doc,
&sample_conversion(),
Some("https://fabro.example"),
)
.unwrap();
let emitted = doc.to_string();
// Comments must survive the round-trip.
assert!(
emitted.contains("# Top-of-file comment explaining the settings layout."),
"top-of-file comment was stripped:\n{emitted}"
);
assert!(
emitted.contains("# Storage root comment — should survive the edit."),
"inline table comment was stripped:\n{emitted}"
);
assert!(
emitted.contains("# A pre-existing integration that is NOT github."),
"sibling-table comment was stripped:\n{emitted}"
);
// Unrelated keys must still be intact.
assert!(
emitted.contains(r#"root = "/srv/fabro-data""#),
"server.storage.root was lost:\n{emitted}"
);
assert!(
emitted.contains(r##"default_channel = "#ops""##),
"server.integrations.slack.default_channel was lost:\n{emitted}"
);
assert!(
emitted.contains(r#"provider = "anthropic""#),
"run.model.provider was lost:\n{emitted}"
);
// And the new keys must be present.
assert!(
emitted.contains(r#"app_id = "123""#),
"server.integrations.github.app_id missing:\n{emitted}"
);
assert!(
emitted.contains(r#"url = "https://fabro.example""#),
"server.web.url missing:\n{emitted}"
);
// Finally, the whole thing must still parse as a valid v2
// SettingsLayer.
fabro_config::parse_settings_layer(&emitted)
.expect("merged output should still parse as v2 after the edit");
}
#[tokio::test]
async fn login_dev_token_mints_session_with_dev_token_provider() {
let key = Key::derive_from(b"web-auth-test-key-material-0123456789");
@ -1317,61 +851,6 @@ name = "claude-sonnet"
assert_eq!(body, json!({ "methods": ["dev-token"] }));
}
#[tokio::test]
async fn setup_register_requires_authentication() {
let app = test_auth_router_with_settings(SettingsLayer::default(), dev_token_auth_mode());
let response = app
.oneshot(
Request::builder()
.method("POST")
.uri("/api/v1/setup/register")
.body(Body::from("not json"))
.unwrap(),
)
.await
.unwrap();
assert_eq!(response.status(), StatusCode::UNAUTHORIZED);
}
#[tokio::test]
async fn setup_register_forbids_github_sessions() {
let key = Key::derive_from(b"web-auth-test-key-material-0123456789");
let app = test_auth_router_with_settings(
github_settings("https://fabro.example"),
github_auth_mode(),
);
let now = chrono::Utc::now().timestamp();
let session_cookie = encode_session_cookie(&key, &SessionCookie {
v: 1,
login: "octocat".to_string(),
auth_method: RunAuthMethod::Github,
provider_id: Some(1),
name: "Octocat".to_string(),
email: "octocat@example.com".to_string(),
avatar_url: "https://avatars.example/octocat".to_string(),
user_url: "https://github.com/octocat".to_string(),
iat: now,
exp: now + 60,
});
let response = app
.oneshot(
Request::builder()
.method("POST")
.uri("/api/v1/setup/register")
.header(header::CONTENT_TYPE, "application/json")
.header(header::COOKIE, session_cookie)
.body(Body::from(json!({ "code": "fake-code" }).to_string()))
.unwrap(),
)
.await
.unwrap();
assert_eq!(response.status(), StatusCode::FORBIDDEN);
}
#[tokio::test]
async fn login_github_sets_secure_state_cookie_for_https_web_url() {
let app = test_auth_router_with_settings(

View file

@ -97,13 +97,29 @@ async fn web_enabled_serves_web_only_routes() {
let auth_me_response = app.clone().oneshot(auth_me_request).await.unwrap();
assert_eq!(auth_me_response.status(), StatusCode::UNAUTHORIZED);
let setup_request = Request::builder()
.method("GET")
.uri("/setup")
.body(Body::empty())
.unwrap();
let setup_response = app.clone().oneshot(setup_request).await.unwrap();
assert_eq!(setup_response.status(), StatusCode::OK);
let setup_status_request = Request::builder()
.method("GET")
.uri("/api/v1/setup/status")
.body(Body::empty())
.unwrap();
let setup_status_response = app.clone().oneshot(setup_status_request).await.unwrap();
assert_eq!(setup_status_response.status(), StatusCode::OK);
assert_eq!(setup_status_response.status(), StatusCode::NOT_FOUND);
let setup_complete_request = Request::builder()
.method("GET")
.uri("/setup/complete")
.body(Body::empty())
.unwrap();
let setup_complete_response = app.clone().oneshot(setup_complete_request).await.unwrap();
assert_eq!(setup_complete_response.status(), StatusCode::NOT_FOUND);
let demo_toggle_request = Request::builder()
.method("POST")
@ -138,6 +154,7 @@ enabled = false
for (method, path, body) in [
("GET", "/", Body::empty()),
("GET", "/setup", Body::empty()),
("GET", "/runs/abc", Body::empty()),
("GET", "/auth/login/github", Body::empty()),
("GET", "/api/v1/auth/me", Body::empty()),