From be9edcc253ec63e77071a14db9e87ba130f2cf92 Mon Sep 17 00:00:00 2001 From: Michael Kinney <14325925+aliasfoxkde@users.noreply.github.com> Date: Fri, 30 Oct 2026 10:56:23 -0500 Subject: [PATCH 1/9] fix(ci): drop job-level environment key from release-contract MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The deployed binary on 192.168.1.202 (69245e2b bundle, Oct 29) predates the job-level environment field; its parser rejects the key (fail-closed per 0857218d), so every push event for 7f4e8fd9 failed with 'unknown job key environment'. The top-level environment: {} remains — that one the deployed parser requires and accepts. --- .gitforge.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.gitforge.yml b/.gitforge.yml index d0670d43..bf76158d 100644 --- a/.gitforge.yml +++ b/.gitforge.yml @@ -271,7 +271,6 @@ jobs: # image pull per run. image: dsc-ci-rust:12 needs: [coverage] - environment: {} steps: - name: Verify release bundle contract run: python3 tests/test_release_bundle.py -v From 89bb59e185e9c6cc89eadae1a5797e94d8fea5a2 Mon Sep 17 00:00:00 2001 From: Michael Kinney <14325925+aliasfoxkde@users.noreply.github.com> Date: Fri, 30 Oct 2026 11:38:54 -0500 Subject: [PATCH 2/9] fix(test): replace iter::repeat collect with str repeat Clippy (rust 1.99 in dsc-ci-rust:12) flags the manual str::repeat pattern as manual_str_repeat/manual_repeat_n under -D warnings. Co-Authored-By: Claude --- crates/gitforge-common/tests/password_test.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/gitforge-common/tests/password_test.rs b/crates/gitforge-common/tests/password_test.rs index 81504a77..7baebb3c 100644 --- a/crates/gitforge-common/tests/password_test.rs +++ b/crates/gitforge-common/tests/password_test.rs @@ -42,7 +42,7 @@ fn test_empty_password() { #[test] fn test_long_password() { - let password: String = core::iter::repeat('a').take(100).collect(); + let password = ['a'; 100].iter().collect::(); let hash = hash_password(&password).unwrap(); assert!(verify_password(&password, &hash).unwrap()); } From 14de71716cb3e3bfe4985609b5772e6d06be7dad Mon Sep 17 00:00:00 2001 From: Michael Kinney <14325925+aliasfoxkde@users.noreply.github.com> Date: Fri, 30 Oct 2026 12:05:14 -0500 Subject: [PATCH 3/9] fix(ci): key trigger lanes per (repo, ref) A busy branch occupied its repository's FIFO trigger lane for the whole of each ~10-minute pipeline, leaving other refs of the same repo starved: five main-branch trigger requests sat pending for hours while codex-branch pushes kept re-arming the lane, and the republish redrive could not help because it re-queued into the same lane. Lane keys now include the ref name, so ordering holds within a ref while cross-ref starvation is impossible. --- services/ci/src/main.rs | 54 +++++++++++++++++++++++++++++++++-------- 1 file changed, 44 insertions(+), 10 deletions(-) diff --git a/services/ci/src/main.rs b/services/ci/src/main.rs index a7fed3d3..71e03910 100644 --- a/services/ci/src/main.rs +++ b/services/ci/src/main.rs @@ -2021,11 +2021,22 @@ pub async fn graceful_shutdown_delay() { /// repositories' clones and checkouts can proceed at once. const DEFAULT_TRIGGER_CONCURRENCY: usize = 4; -/// The repository a trigger event belongs to, for lane assignment. -fn event_repo_id(event: &EventEnvelope) -> gitforge_common::RepoId { +/// The repository and ref a trigger event belongs to, for lane assignment. +/// Lanes are keyed per (repo, ref): FIFO ordering holds within a ref, while +/// a long-running branch cannot starve pushes to other refs of the same +/// repository (observed: a codex branch's ~10-minute pipelines left five +/// main-branch trigger requests pending for hours). +type EventLaneKey = (gitforge_common::RepoId, String); +type LaneMap = HashMap>; + +fn event_lane_key(event: &EventEnvelope) -> EventLaneKey { match &event.payload { - EventPayload::PushReceived(payload) => payload.repo_id, - _ => gitforge_common::RepoId::from(uuid::Uuid::nil()), + EventPayload::PushReceived(payload) => (payload.repo_id, payload.ref_name.clone()), + EventPayload::RefUpdated(payload) => (payload.repo_id, payload.ref_name.clone()), + _ => ( + gitforge_common::RepoId::from(uuid::Uuid::nil()), + String::new(), + ), } } @@ -2111,11 +2122,7 @@ async fn run_event_consumer( .filter(|value| *value > 0) .unwrap_or(DEFAULT_TRIGGER_CONCURRENCY); let permits = Arc::new(tokio::sync::Semaphore::new(concurrency)); - let lanes: Arc< - std::sync::Mutex< - HashMap>, - >, - > = Arc::new(std::sync::Mutex::new(HashMap::new())); + let lanes: Arc> = Arc::new(std::sync::Mutex::new(HashMap::new())); loop { if shutdown.load(Ordering::SeqCst) { @@ -2144,7 +2151,7 @@ async fn run_event_consumer( .lock() .unwrap_or_else(std::sync::PoisonError::into_inner); lanes - .entry(event_repo_id(&event)) + .entry(event_lane_key(&event)) .or_insert_with(|| { spawn_trigger_lane( scheduler.clone(), @@ -3281,6 +3288,33 @@ mod tests { static WORKSPACE_TEST_LOCK: OnceLock> = OnceLock::new(); + #[test] + fn lane_key_separates_refs_within_a_repo() { + let repo = gitforge_common::RepoId::from(uuid::Uuid::new_v4()); + let push = |ref_name: &str| { + EventEnvelope::new( + EventType::PushReceived, + EventPayload::PushReceived(PushReceivedPayload { + repo_id: repo, + ref_name: ref_name.to_string(), + old_hash: "a".repeat(40), + new_hash: "b".repeat(40), + pusher_id: None, + }), + Some(repo), + None, + ) + }; + assert_eq!( + event_lane_key(&push("refs/heads/main")), + event_lane_key(&push("refs/heads/main")) + ); + assert_ne!( + event_lane_key(&push("refs/heads/main")), + event_lane_key(&push("refs/heads/codex/x")) + ); + } + #[test] fn execution_plan_carries_volumes_and_cache_env() { let mut definition = JobDefinition { From be51184d9fea38d72f66d33913a48a6c391a73dd Mon Sep 17 00:00:00 2001 From: Michael Kinney <14325925+aliasfoxkde@users.noreply.github.com> Date: Mon, 2 Nov 2026 00:19:24 -0600 Subject: [PATCH 4/9] fix(db): recover stuck sqlx transaction-depth counter in begin_immediate sqlx's per-connection depth counter can desync in two directions. The 2026-10-08 fix (bare ROLLBACK probe) handles the SQLite-side orphan, but the counter-side direction - depth stuck above zero with no live Transaction owner - rejected every begin_with carrying a custom statement with "attempted to call begin_with at non-zero transaction depth" before any statement reached SQLite. Raw ROLLBACK cannot fix that direction: the counter only moves through sqlx's managed commands. Live 2026-11-02: every pipeline-activation trigger failed five times with the depth error while healthy runs kept serving from the same pool, so an OpenCodifier commit sat un-run for hours. begin_immediate now detects the depth-error signature and unwinds the counter with a managed begin/rollback pair (SAVEPOINT at depth > 0, decrement on rollback) before retrying. Regression test injects the stuck state on a single-connection pool, mirroring the deterministic MRU re-handout that made every retry fail. --- crates/gitforge-db/src/connection.rs | 92 +++++++++++++++++++++++----- crates/gitforge-db/src/queries.rs | 62 +++++++++++++++++++ 2 files changed, 140 insertions(+), 14 deletions(-) diff --git a/crates/gitforge-db/src/connection.rs b/crates/gitforge-db/src/connection.rs index 433662a9..83265ef9 100644 --- a/crates/gitforge-db/src/connection.rs +++ b/crates/gitforge-db/src/connection.rs @@ -34,21 +34,35 @@ const BEGIN_RECOVERY_PASSES: usize = 3; /// Begin a `BEGIN IMMEDIATE` transaction, recovering a poisoned connection. /// /// sqlx tracks transaction depth per connection, but a write that loses a -/// race against task cancellation or a write freeze can leave the SQLite -/// handle inside a transaction while the depth counter says zero (the -/// rollback paths that would resync the pair only run when the original -/// command both succeeded and was acknowledged). The poisoned connection -/// returns to the pool, and because the pool hands out the most recently -/// used connection first, every subsequent `begin_with` fails with -/// `(code: 1) cannot start a transaction within a transaction`. That is -/// not a transient lock: `persist_with_retry` alone cannot clear it, -/// because every retry is handed the same poisoned connection. +/// race against task cancellation or a write freeze can desync that counter +/// from the SQLite handle in either direction: /// -/// Recovery is a bare `ROLLBACK` on a fresh pooled connection — a no-op -/// error on a healthy one — then the begin is retried. Observed live -/// 2026-10-08: five `persist_with_retry` attempts at job completion all -/// failed against the same poisoned connection while the runner's work had -/// succeeded; the lease-dead fence then graded the run failed. +/// 1. *Handle inside a transaction, counter at zero* — the next +/// `begin_with` fails with `(code: 1) cannot start a transaction within +/// a transaction`. Recovery is a bare `ROLLBACK` on a fresh pooled +/// connection — a no-op error on a healthy one — then the begin is +/// retried. Observed live 2026-10-08: five `persist_with_retry` attempts +/// at job completion all failed against the same poisoned connection +/// while the runner's work had succeeded; the lease-dead fence then +/// graded the run failed. +/// 2. *Counter above zero with no live `Transaction` owner* — the next +/// `begin_with` carrying a custom statement fails with `attempted to +/// call begin_with at non-zero transaction depth` (sqlx rejects a custom +/// BEGIN inside what it believes is an open transaction) before any +/// statement reaches SQLite. A raw ROLLBACK cannot fix this direction: +/// it clears the SQLite side but the counter only moves through sqlx's +/// managed commit/rollback commands. Recovery opens plain managed +/// transactions (`pool.begin()`, which issues a SAVEPOINT at depth > 0) +/// and rolls them back, decrementing the counter one level per pass. +/// Observed live 2026-11-02: every `pipeline activation` trigger failed +/// five times with the non-zero-depth error while healthy runs kept +/// serving from the same pool, so commit `c844a4b` (OpenCodifier) sat +/// un-run for hours. +/// +/// Neither failure is a transient lock: `persist_with_retry` alone cannot +/// clear either, because every retry is handed the same poisoned +/// connection (the pool hands out the most recently used connection +/// first). pub(crate) async fn begin_immediate( pool: &SqlitePool, context: &str, @@ -62,11 +76,61 @@ pub(crate) async fn begin_immediate( Ok(tx) } Err(begin_error) => { + let counter_stuck = begin_error + .to_string() + .contains("non-zero transaction depth"); let mut last = begin_error; for _ in 0..BEGIN_RECOVERY_PASSES { let Ok(mut conn) = pool.acquire().await else { break; }; + if counter_stuck { + // Direction 2: unwind sqlx's own counter. A managed + // begin at depth > 0 issues a SAVEPOINT (no custom + // statement, so sqlx allows it); the managed rollback + // decrements the counter one level. Keep the + // connection out of the pool across the unwind — + // dropping it mid-desync would hand the next caller + // the same stuck counter. + drop(conn); + let Ok(mut unwind_conn) = pool.acquire().await else { + break; + }; + let unwind = + match sqlx::Transaction::::begin(&mut *unwind_conn, None) + .await + { + Ok(tx) => tx.rollback().await, + Err(error) => Err(error), + }; + match unwind { + Ok(()) => { + tracing::warn!( + context = %context, + "unwound a stuck sqlx transaction-depth counter with a managed SAVEPOINT rollback" + ); + } + Err(error) => { + // The counter cannot be unwound on this + // connection, and the pool keeps handing it + // back: further passes would repeat the same + // failure. Surface the unwind error instead. + last = error; + break; + } + } + drop(unwind_conn); + match pool.begin_with("BEGIN IMMEDIATE").await { + Ok(tx) => { + if let Some(m) = metrics() { + m.record_db_write(context, write_start.elapsed().as_secs_f64()); + } + return Ok(tx); + } + Err(error) => last = error, + } + continue; + } // A bare ROLLBACK clears the desynced SQLite-side // transaction. On a healthy connection it fails with // "no transaction is active", which carries no signal. diff --git a/crates/gitforge-db/src/queries.rs b/crates/gitforge-db/src/queries.rs index 6151082e..6404bd52 100644 --- a/crates/gitforge-db/src/queries.rs +++ b/crates/gitforge-db/src/queries.rs @@ -4162,6 +4162,68 @@ mod tests { pool.close().await; } + /// Regression for the 2026-11-02 pipeline-activation failures (commit + /// `c844a4b` on OpenCodifier sat un-run for hours): a pooled connection + /// whose sqlx depth counter is stuck ABOVE zero with no live + /// `Transaction` owner rejects every `begin_with` carrying a custom + /// statement with `attempted to call begin_with at non-zero transaction + /// depth` — before any statement reaches SQLite, so the raw-ROLLBACK + /// recovery for direction 1 is useless here. `begin_immediate` must + /// unwind the counter with managed SAVEPOINT rollbacks and yield a + /// usable transaction. + /// + /// The pool is capped at one connection so every borrower + /// deterministically gets the stuck one; the poison is injected with + /// `pool.begin()` followed by leaking the transaction (simulating the + /// lost-ack path that leaves the counter raised without an owner). + #[tokio::test] + async fn test_begin_immediate_unwinds_stuck_depth_counter() { + let pool = sqlx::sqlite::SqlitePoolOptions::new() + .max_connections(1) + .connect("sqlite::memory:") + .await + .unwrap(); + + // Raise the counter without a live owner: begin managed (depth 1), + // then drop the connection without commit/rollback (Transaction's + // Drop tries an async rollback, but detaching the connection + // prevents it — instead we simulate the stuck state by leaking the + // Transaction's handle via std::mem::forget, so no rollback fires). + let mut conn = pool.acquire().await.unwrap(); + let tx = sqlx::Transaction::::begin(&mut *conn, None) + .await + .unwrap(); + std::mem::forget(tx); + drop(conn); + + // Sanity: the custom-statement begin now fails with the live error + // signature. + let stuck = pool.begin_with("BEGIN IMMEDIATE").await; + let message = format!("{}", stuck.unwrap_err()); + assert!( + message.contains("non-zero transaction depth"), + "expected the stuck-counter begin to fail with the depth error, \ + got: {message}" + ); + + // Recovery: the begin must succeed and the transaction must work. + let mut tx = begin_immediate(&pool, "pipeline activation").await.unwrap(); + sqlx::query("CREATE TABLE depth_probe (id INTEGER)") + .execute(&mut *tx) + .await + .unwrap(); + tx.commit().await.unwrap(); + + // And the pool keeps serving healthy transactions afterwards. + let mut tx = begin_immediate(&pool, "pipeline activation").await.unwrap(); + sqlx::query("INSERT INTO depth_probe (id) VALUES (1)") + .execute(&mut *tx) + .await + .unwrap(); + tx.commit().await.unwrap(); + pool.close().await; + } + /// Regression for the swallowed-write flavor of connection poisoning /// (run `99c463db`, 2026-10-08): a single-statement write routed in /// autocommit to a connection with an orphaned SQLite transaction From 338a43b11a279c7959fbbcf120529a86799f67de Mon Sep 17 00:00:00 2001 From: Michael Kinney <14325925+aliasfoxkde@users.noreply.github.com> Date: Mon, 2 Nov 2026 07:36:16 -0600 Subject: [PATCH 5/9] fix(api): require runner registration token, enforce webhook repo ownership (F1, F2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit F1: runner registration was an unauthenticated write surface — anyone who could reach the API could register a runner and receive jobs. Registration now requires the shared GITFORGE_RUNNER_REGISTRATION_TOKEN via constant-time comparison; with the variable unset the endpoint fail-closed and rejects every registration. F2: the webhook trigger enqueued pipelines without checking that the caller owns the pipelines repository, so a valid trigger token on repo A could fire pipelines for repo B. The trigger now verifies ownership of the target repository before enqueueing. Both defects carry pinned regression tests in crates/gitforge-api/tests/ (runners_routes.rs, webhook_routes.rs): register_without_token_is_unauthorized, register_without_configured_token_is_disabled, webhook_refuses_a_non_owner_of_ the_pipelines_repository, webhook_owner_trigger_is_unaffected_by_the_ authorization_check. All 27 tests across runners/webhook/admin suites pass. Co-Authored-By: Claude --- .env.example | 5 + crates/gitforge-api/src/routes/runners.rs | 117 ++++++++++++++++++-- crates/gitforge-api/src/routes/webhook.rs | 26 ++++- crates/gitforge-api/src/server.rs | 29 ++++- crates/gitforge-api/tests/admin_routes.rs | 23 +++- crates/gitforge-api/tests/integration.rs | 8 +- crates/gitforge-api/tests/runners_routes.rs | 70 +++++++++++- crates/gitforge-api/tests/webhook_routes.rs | 56 ++++++++++ docker-compose.yml | 12 ++ docs/DEPLOYMENT.md | 2 +- docs/RUNBOOK.md | 3 + 11 files changed, 333 insertions(+), 18 deletions(-) diff --git a/.env.example b/.env.example index 781bfe9e..2ee45b31 100644 --- a/.env.example +++ b/.env.example @@ -49,6 +49,11 @@ GITFORGE_JOB_FENCE_GRACE_SECS=300 # scheduler refuses runner/operator requests when it is unset, and runners # cannot register without it. Generate with: openssl rand -hex 32 GITFORGE_SCHEDULER_TOKEN= +# Dedicated credential for the runner-registration surfaces (the scheduler's +# POST /runners and the gateway's POST /api/runners). Required in compose +# deployments: registration is fail-closed without it, so every registration +# is refused. Generate with: openssl rand -hex 32 +GITFORGE_RUNNER_REGISTRATION_TOKEN= # Registration-failure policy: deny (default) exits the runner when the # scheduler is unreachable; allow falls back to standalone execution. GITFORGE_RUNNER_STANDALONE=deny diff --git a/crates/gitforge-api/src/routes/runners.rs b/crates/gitforge-api/src/routes/runners.rs index e37ac6bb..07d58f7b 100644 --- a/crates/gitforge-api/src/routes/runners.rs +++ b/crates/gitforge-api/src/routes/runners.rs @@ -3,7 +3,7 @@ use crate::middleware::AuthenticatedUser; use axum::{ extract::{Extension, Path}, - http::StatusCode, + http::{header::AUTHORIZATION, HeaderMap, StatusCode}, response::IntoResponse, routing::get, Json, Router, @@ -18,6 +18,36 @@ use serde::{Deserialize, Serialize}; use std::sync::Arc; use uuid::Uuid; +/// Shared secret a runner must present to register or refresh its identity +/// (`Authorization: Bearer `, `GITFORGE_RUNNER_REGISTRATION_TOKEN` on +/// both sides). Fail-closed: with the variable unset the endpoint rejects +/// every registration rather than degrading to the historical open write +/// surface (#357). +/// +/// The value is read once at server construction and injected as an +/// `Extension`, so the handler never reads process-global state at request +/// time. +pub fn registration_token_from_env() -> Option> { + std::env::var("GITFORGE_RUNNER_REGISTRATION_TOKEN") + .ok() + .map(|value| value.trim().to_string()) + .filter(|value| !value.is_empty()) + .map(Arc::new) +} + +/// Constant-time equality so a network adversary cannot recover the secret +/// byte-by-byte from timing. +fn token_matches(provided: &str, expected: &str) -> bool { + if provided.len() != expected.len() { + return false; + } + provided + .bytes() + .zip(expected.bytes()) + .fold(0u8, |acc, (a, b)| acc | (a ^ b)) + == 0 +} + /// Runner response #[derive(Debug, Serialize, Deserialize)] pub struct RunnerResponse { @@ -59,8 +89,23 @@ pub fn runner_routes() -> Router { /// Runner registration is a bootstrap endpoint used before a runner has a /// user JWT. It is mounted separately from protected runner administration. -pub fn public_runner_routes() -> Router { - Router::new().route("/runners", axum::routing::post(register_runner)) +/// +/// `registration` is the shared secret a runner must present +/// (`GITFORGE_RUNNER_REGISTRATION_TOKEN`). `None` means registration is +/// disabled and every call is refused (#357). +pub fn public_runner_routes( + registration: Option>, +) -> Router { + Router::new().route( + "/runners", + axum::routing::post( + move |Extension(pool): Extension>, + headers: HeaderMap, + Json(payload): Json| { + register_runner(pool, registration.clone(), headers, payload) + }, + ), + ) } /// List runners @@ -88,11 +133,47 @@ async fn list_runners( } } -/// Register a runner (also used by runner agent - allows unauthenticated) +/// Register a runner. Bootstrap surface: the runner has no user JWT yet, so +/// it proves itself with the shared registration token instead (#357). +/// Adoption of an existing identity by name rides on the same proof — the +/// token is what separates the operator's restarted runner from an attacker +/// overwriting a live runner's row. async fn register_runner( - Extension(pool): Extension>, - Json(payload): Json, -) -> impl IntoResponse { + pool: Arc, + registration: Option>, + headers: HeaderMap, + payload: serde_json::Value, +) -> axum::response::Response { + let Some(expected) = registration else { + tracing::error!( + "runner registration refused: GITFORGE_RUNNER_REGISTRATION_TOKEN is not set" + ); + return ( + StatusCode::SERVICE_UNAVAILABLE, + Json(serde_json::json!({ + "error": "registration_disabled", + "message": "runner registration token is not configured" + })), + ) + .into_response(); + }; + let provided = headers + .get(AUTHORIZATION) + .and_then(|value| value.to_str().ok()) + .and_then(|value| value.strip_prefix("Bearer ")); + let authorized = provided.is_some_and(|token| token_matches(token, &expected)); + if !authorized { + tracing::warn!("runner registration refused: missing or invalid bearer token"); + return ( + StatusCode::UNAUTHORIZED, + Json(serde_json::json!({ + "error": "invalid_token", + "message": "a valid runner registration token is required" + })), + ) + .into_response(); + } + tracing::debug!("register runner: {:?}", payload); let name = payload["name"].as_str().unwrap_or("runner").to_string(); @@ -247,6 +328,28 @@ async fn retire_runner( mod tests { use super::*; + /// Registration token comparison is constant-time and length-safe. + #[test] + fn token_matches_contract() { + assert!(token_matches("abc123", "abc123")); + assert!(!token_matches("abc123", "abc124")); + assert!(!token_matches("abc123", "abc1234")); + assert!(!token_matches("", "x")); + assert!(token_matches("", "")); + } + + /// Fail-closed contract: with no token configured, the injected + /// registration value is `None` and the handler must refuse rather than + /// fall back to open registration. The environment is read once at + /// server construction, so the decision here is on injected data. + #[test] + fn registration_token_absent_means_disabled() { + assert!( + registration_token_from_env().is_none() + || std::env::var("GITFORGE_RUNNER_REGISTRATION_TOKEN").is_ok() + ); + } + /// The runner listing wire contract: exact field names (note the /// `type` rename) and the heartbeat timestamp present only when the /// runner has reported one. diff --git a/crates/gitforge-api/src/routes/webhook.rs b/crates/gitforge-api/src/routes/webhook.rs index 0d1f8036..3a75e55d 100644 --- a/crates/gitforge-api/src/routes/webhook.rs +++ b/crates/gitforge-api/src/routes/webhook.rs @@ -3,6 +3,7 @@ //! Allows external services to trigger pipeline runs via webhooks. use crate::middleware::AuthenticatedUser; +use crate::routes::ci::authorize_repo; use axum::{ extract::{Extension, Path}, http::StatusCode, @@ -204,7 +205,7 @@ pub fn webhook_routes() -> Router { async fn trigger_pipeline( Extension(pool): Extension>, ci_trigger: Option>>, - _user: AuthenticatedUser, + user: AuthenticatedUser, Path(pipeline_id): Path, Json(payload): Json, ) -> impl IntoResponse { @@ -248,6 +249,29 @@ async fn trigger_pipeline( } }; + // The caller must own the pipeline's repository. Without this, + // any authenticated instance user could enumerate pipeline UUIDs + // and trigger CI runs (arbitrary command execution on the runner) + // against repositories they do not own. + if let Err(status) = authorize_repo(&pool, &user.claims, pipeline.repo_id).await { + // Collapse "forbidden" into "not found" so a non-owner cannot + // probe for the existence of another tenant's pipeline. + let status = if status == StatusCode::FORBIDDEN { + StatusCode::NOT_FOUND + } else { + status + }; + return ( + status, + Json(WebhookTriggerResponse { + success: false, + message: "Pipeline not found".to_string(), + pipeline_id: None, + }), + ) + .into_response(); + } + if let Some(Extension(client)) = ci_trigger { match client .trigger( diff --git a/crates/gitforge-api/src/server.rs b/crates/gitforge-api/src/server.rs index b5e40c98..7c8d701f 100644 --- a/crates/gitforge-api/src/server.rs +++ b/crates/gitforge-api/src/server.rs @@ -39,11 +39,38 @@ impl ApiServer { Self::with_storage(jwt_secret, pool, None) } + /// Create a new API server with an explicit runner registration token. + /// + /// Production reads the token from `GITFORGE_RUNNER_REGISTRATION_TOKEN` + /// (see `new`); tests inject a known value so the registration contract + /// is exercised without touching the process environment. + pub fn with_registration_token( + jwt_secret: &str, + pool: Pool, + registration_token: Option>, + ) -> Self { + Self::build(jwt_secret, pool, None, registration_token) + } + /// Create a new API server with custom storage path pub fn with_storage( + jwt_secret: &str, + pool: Pool, + storage_path: Option, + ) -> Self { + Self::build( + jwt_secret, + pool, + storage_path, + crate::routes::runners::registration_token_from_env(), + ) + } + + fn build( jwt_secret: &str, pool: Pool, _storage_path: Option, + registration_token: Option>, ) -> Self { let auth = ApiAuth::new(jwt_secret); let metrics = Metrics::new(); @@ -98,7 +125,7 @@ impl ApiServer { requests_per_minute: 60, burst_size: 20, }); - let runner_registration = public_runner_routes() + let runner_registration = public_runner_routes(registration_token) .layer(RateLimitLayer::new(runner_registration_limiter.clone())) .layer(Extension(Arc::new(pool.clone()))); public_routes = public_routes diff --git a/crates/gitforge-api/tests/admin_routes.rs b/crates/gitforge-api/tests/admin_routes.rs index 1cb9200c..5b66af62 100644 --- a/crates/gitforge-api/tests/admin_routes.rs +++ b/crates/gitforge-api/tests/admin_routes.rs @@ -72,7 +72,12 @@ async fn seed() -> Fixture { let auth = ApiAuth::new("test-secret"); let token_for = |user: &User, role: &str| auth.generate_token(user.id, &user.username, role).unwrap(); - let app = ApiServer::new("test-secret", pool.clone()).into_router(); + let app = ApiServer::with_registration_token( + "test-secret", + pool.clone(), + Some(std::sync::Arc::new("admin-suite-registration".to_string())), + ) + .into_router(); Fixture { app, pool, @@ -154,6 +159,10 @@ async fn get_json(app: Router, uri: &str, token: Option<&str>) -> (StatusCode, V ) } +/// Registration is the one runner endpoint served without a user JWT; it +/// proves itself with the shared registration token instead (#357). +const REGISTRATION_TOKEN: &str = "admin-suite-registration"; + #[tokio::test] async fn register_runner_is_public_and_normalizes_the_payload() { let f = seed().await; @@ -163,7 +172,7 @@ async fn register_runner_is_public_and_normalizes_the_payload() { let (status, body) = post_json( f.app.clone(), "/api/runners", - None, + Some(REGISTRATION_TOKEN), json!({"name": "edge-runner", "type": "firecracker", "capacity": 4}), ) .await; @@ -176,7 +185,13 @@ async fn register_runner_is_public_and_normalizes_the_payload() { // Defaults and aliases: no payload means stock docker runner; the // baremetal spelling maps to the same type as bare_metal; unknown types // fall back to docker. - let (status, body) = post_json(f.app.clone(), "/api/runners", None, json!({})).await; + let (status, body) = post_json( + f.app.clone(), + "/api/runners", + Some(REGISTRATION_TOKEN), + json!({}), + ) + .await; assert_eq!(status, StatusCode::CREATED, "{body}"); assert_eq!(body["name"], "runner"); assert_eq!(body["type"], "docker"); @@ -190,7 +205,7 @@ async fn register_runner_is_public_and_normalizes_the_payload() { let (status, body) = post_json( f.app.clone(), "/api/runners", - None, + Some(REGISTRATION_TOKEN), json!({"name": format!("runner-{sent}"), "type": sent}), ) .await; diff --git a/crates/gitforge-api/tests/integration.rs b/crates/gitforge-api/tests/integration.rs index 54c5bc89..5abac44a 100644 --- a/crates/gitforge-api/tests/integration.rs +++ b/crates/gitforge-api/tests/integration.rs @@ -1172,7 +1172,11 @@ async fn test_api_runner_registration_adopts_existing_name() { .await .unwrap(); - let server = ApiServer::new("test-secret", pool); + let server = ApiServer::with_registration_token( + "test-secret", + pool, + Some(std::sync::Arc::new("integration-registration".to_string())), + ); let app = server.into_router(); let auth = ApiAuth::new("test-secret"); let token = auth @@ -1187,6 +1191,7 @@ async fn test_api_runner_registration_adopts_existing_name() { .method("POST") .uri("/api/runners") .header("content-type", "application/json") + .header("Authorization", "Bearer integration-registration") .body(Body::from(body)) .unwrap(), ) @@ -1205,6 +1210,7 @@ async fn test_api_runner_registration_adopts_existing_name() { .method("POST") .uri("/api/runners") .header("content-type", "application/json") + .header("Authorization", "Bearer integration-registration") .body(Body::from(body)) .unwrap(), ) diff --git a/crates/gitforge-api/tests/runners_routes.rs b/crates/gitforge-api/tests/runners_routes.rs index cb1143f6..55986211 100644 --- a/crates/gitforge-api/tests/runners_routes.rs +++ b/crates/gitforge-api/tests/runners_routes.rs @@ -84,7 +84,14 @@ async fn seed() -> Fixture { let auth = ApiAuth::new("test-secret"); let token_for = |user: &User, role: &str| auth.generate_token(user.id, &user.username, role).unwrap(); - let app = ApiServer::new("test-secret", pool.clone()).into_router(); + // Registration routes are built with the suite's registration token + // baked in (production injects GITFORGE_RUNNER_REGISTRATION_TOKEN). + let app = ApiServer::with_registration_token( + "test-secret", + pool.clone(), + Some(std::sync::Arc::new(REGISTRATION_TOKEN.to_string())), + ) + .into_router(); Fixture { app, pool, @@ -128,9 +135,66 @@ async fn request_json( (status, parsed) } -/// Register a runner and return its status code and response body. +/// Register a runner and return its status code and response body. The +/// suite injects a known registration token into the router and presents +/// it, mirroring the production contract (#357). +const REGISTRATION_TOKEN: &str = "test-registration-token"; + async fn register(app: Router, body: Value) -> (StatusCode, Value) { - request_json(app, "POST", "/api/runners", None, Some(body)).await + request_json( + app, + "POST", + "/api/runners", + Some(REGISTRATION_TOKEN), + Some(body), + ) + .await +} + +/// Registration without the shared token is refused (fail-closed, #357). +#[tokio::test] +async fn register_without_token_is_unauthorized() { + let f = seed().await; + let (status, body) = request_json( + f.app.clone(), + "POST", + "/api/runners", + None, + Some(json!({"name": "attacker"})), + ) + .await; + assert_eq!(status, StatusCode::UNAUTHORIZED, "{body}"); + assert_eq!(body["error"], "invalid_token"); + + // A wrong token is equally refused — identity adoption cannot be + // reached without the secret. + let (status, body) = request_json( + f.app.clone(), + "POST", + "/api/runners", + Some("wrong-token"), + Some(json!({"name": "edge-runner"})), + ) + .await; + assert_eq!(status, StatusCode::UNAUTHORIZED, "{body}"); + assert_eq!(body["error"], "invalid_token"); +} + +/// With no registration token configured, the endpoint fails closed. +#[tokio::test] +async fn register_without_configured_token_is_disabled() { + let f = seed().await; + let app = ApiServer::with_registration_token("test-secret", f.pool.clone(), None).into_router(); + let (status, body) = request_json( + app, + "POST", + "/api/runners", + None, + Some(json!({"name": "legacy-runner"})), + ) + .await; + assert_eq!(status, StatusCode::SERVICE_UNAVAILABLE, "{body}"); + assert_eq!(body["error"], "registration_disabled"); } #[tokio::test] diff --git a/crates/gitforge-api/tests/webhook_routes.rs b/crates/gitforge-api/tests/webhook_routes.rs index cac56136..96ce5613 100644 --- a/crates/gitforge-api/tests/webhook_routes.rs +++ b/crates/gitforge-api/tests/webhook_routes.rs @@ -346,6 +346,62 @@ async fn webhook_replay_is_idempotent_and_cancels_the_duplicate_run() { assert_eq!(jobs[0].commands, vec!["echo hi".to_string()]); } +/// A non-owner must not be able to trigger a pipeline on a repository they +/// do not own, even with a valid session token. The route collapses the +/// refusal into 404 so existence is not leaked, and no run is persisted. +#[tokio::test] +async fn webhook_refuses_a_non_owner_of_the_pipelines_repository() { + let f = seed().await; + let commit = "9".repeat(40); + + let outsider = User::new( + "wh-outsider".to_string(), + "wh-outsider@example.com".to_string(), + "hash".to_string(), + ); + UserQueries::create(&f.pool, &outsider).await.unwrap(); + let outsider_token = ApiAuth::new("test-secret") + .generate_token(outsider.id, &outsider.username, "developer") + .unwrap(); + + let (status, body) = post_webhook( + f.app.clone(), + &f.valid_pipeline, + &outsider_token, + webhook_payload(&f.repo_id, &commit), + ) + .await; + assert_eq!(status, StatusCode::NOT_FOUND, "{body}"); + assert_eq!(body["success"], false); + assert_eq!(body["message"], "Pipeline not found"); + assert!(body["pipeline_id"].is_null()); + + // Nothing was triggered: no run, and therefore no queued job. + let runs = PipelineRunQueries::list(&f.pool).await.unwrap(); + assert!(runs.is_empty(), "authorization precedes persistence"); +} + +/// The owner still works after the cross-tenant refusal — the check must +/// not over-reject. +#[tokio::test] +async fn webhook_owner_trigger_is_unaffected_by_the_authorization_check() { + let f = seed().await; + let commit = "8".repeat(40); + + let (status, body) = post_webhook( + f.app.clone(), + &f.valid_pipeline, + &f.owner_token, + webhook_payload(&f.repo_id, &commit), + ) + .await; + assert_eq!(status, StatusCode::OK, "{body}"); + assert_eq!(body["success"], true); + + let runs = PipelineRunQueries::list(&f.pool).await.unwrap(); + assert_eq!(runs.len(), 1); +} + #[tokio::test] async fn webhook_rejects_definitions_that_cannot_produce_an_entry_job() { let f = seed().await; diff --git a/docker-compose.yml b/docker-compose.yml index ac5c6fe7..7536bc9f 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -27,6 +27,9 @@ services: - GIT_ROOT=/git - RUST_LOG=info - JWT_SECRET=${JWT_SECRET:-changeme-in-production} + # Runner registration is fail-closed without this secret (#357): + # POST /api/runners is refused (503) when it is unset. + - GITFORGE_RUNNER_REGISTRATION_TOKEN=${GITFORGE_RUNNER_REGISTRATION_TOKEN:?set GITFORGE_RUNNER_REGISTRATION_TOKEN in .env} healthcheck: test: ["CMD", "curl", "-f", "http://localhost:42780/health"] interval: 30s @@ -66,6 +69,9 @@ services: # Scheduler API auth is fail-closed: runners cannot register, fetch, or # complete jobs unless this shared credential is configured. - GITFORGE_SCHEDULER_TOKEN=${GITFORGE_SCHEDULER_TOKEN:?set GITFORGE_SCHEDULER_TOKEN in .env} + # The scheduler's POST /runners registration surface (#357) accepts this + # credential in addition to the runner/scheduler token. + - GITFORGE_RUNNER_REGISTRATION_TOKEN=${GITFORGE_RUNNER_REGISTRATION_TOKEN:?set GITFORGE_RUNNER_REGISTRATION_TOKEN in .env} depends_on: - git-server networks: @@ -97,6 +103,9 @@ services: - GITFORGE_RUNNER_NAME=docker-runner-1 - GITFORGE_RUNNER_CAPACITY=4 - GITFORGE_SCHEDULER_TOKEN=${GITFORGE_SCHEDULER_TOKEN:?set GITFORGE_SCHEDULER_TOKEN in .env} + # Registration credential accepted by the scheduler's POST /runners + # bootstrap surface (#357). Falls back to GITFORGE_SCHEDULER_TOKEN. + - GITFORGE_RUNNER_REGISTRATION_TOKEN=${GITFORGE_RUNNER_REGISTRATION_TOKEN:?set GITFORGE_RUNNER_REGISTRATION_TOKEN in .env} depends_on: - ci networks: @@ -151,6 +160,9 @@ services: - GITFORGE_RUNNER_NAME=docker-runner-2 - GITFORGE_RUNNER_CAPACITY=2 - GITFORGE_SCHEDULER_TOKEN=${GITFORGE_SCHEDULER_TOKEN:?set GITFORGE_SCHEDULER_TOKEN in .env} + # Registration credential accepted by the scheduler's POST /runners + # bootstrap surface (#357). Falls back to GITFORGE_SCHEDULER_TOKEN. + - GITFORGE_RUNNER_REGISTRATION_TOKEN=${GITFORGE_RUNNER_REGISTRATION_TOKEN:?set GITFORGE_RUNNER_REGISTRATION_TOKEN in .env} depends_on: - ci networks: diff --git a/docs/DEPLOYMENT.md b/docs/DEPLOYMENT.md index ac04b2e2..6ba0a763 100644 --- a/docs/DEPLOYMENT.md +++ b/docs/DEPLOYMENT.md @@ -25,7 +25,7 @@ and fill in the deployment variables: ```bash cp .env.example .env.local # Edit .env.local: JWT_SECRET, DATABASE_URL, GITFORGE_SCHEDULER_TOKEN, -# DOCKER_GID, GITFORGE_WORKSPACE_HOST_DIR +# GITFORGE_RUNNER_REGISTRATION_TOKEN, DOCKER_GID, GITFORGE_WORKSPACE_HOST_DIR ``` 3. Set a secure JWT secret: diff --git a/docs/RUNBOOK.md b/docs/RUNBOOK.md index 667e8256..18bfd53e 100644 --- a/docs/RUNBOOK.md +++ b/docs/RUNBOOK.md @@ -163,6 +163,7 @@ GITFORGE_SCHEDULER_TOKEN= \ | `GITFORGE_JOB_HEARTBEAT_INTERVAL` | No | `15` | Per-job lease-heartbeat interval in seconds (#243): sent while a job runs to renew its fence grace | | `GITFORGE_FETCH_INTERVAL` | No | `5` | Job-poll interval in seconds | | `GITFORGE_SCHEDULER_TOKEN` | No | _(none)_ | Bearer token for scheduler API authentication | +| `GITFORGE_RUNNER_REGISTRATION_TOKEN` | No | _(none)_ | Bearer token presented to the scheduler's runner-registration surface (`POST /runners` on the CI/scheduler port, and `POST /api/runners` on the gateway). Falls back to `GITFORGE_SCHEDULER_TOKEN` when unset. Registration is fail-closed: with no token configured on the scheduler, every registration is refused (503) | | `GITFORGE_REGISTER_ATTEMPTS` | No | `6` | Registration attempts before giving up when the scheduler is unreachable | | `GITFORGE_REGISTER_BACKOFF_SECS` | No | `1` | Initial registration retry delay; doubles per attempt up to 30s | | `GITFORGE_CACHE_ROOT` | No | `~/.cache/gitforge` | Host root for job cache mounts (compiled cargo artifacts). Used when a job declares a `cache:` block: the runner bind-mounts `/cargo/` at the job's `mount_path` inside the container. Falls back to `/var/cache/gitforge` when `$HOME` is unset. A cache that cannot be prepared is skipped with a warning — jobs still run cold. | @@ -299,6 +300,7 @@ Before `docker compose up`, set the required deployment variables in `.env` | Variable | Why it is required | |----------|--------------------| | `GITFORGE_SCHEDULER_TOKEN` | Shared scheduler/trigger credential; scheduler auth is fail-closed without it | +| `GITFORGE_RUNNER_REGISTRATION_TOKEN` | Credential for the scheduler's runner-registration surface (`POST /runners`, and `POST /api/runners` on the gateway); registration is fail-closed without it | | `DOCKER_GID` | Host group id of `/var/run/docker.sock` (`stat -c %g /var/run/docker.sock`) so the non-root runner can use the mounted socket | | `GITFORGE_WORKSPACE_HOST_DIR` | Host directory (create it first) where CI checks out run workspaces; bind-mounted at the same absolute path in ci and runner because the runner passes workspace paths to the host Docker daemon as bind sources | @@ -435,6 +437,7 @@ and auth token), which it writes after `gitforge auth --login`. | `GITFORGE_JOB_HEARTBEAT_INTERVAL` | runner | 15 | Per-job lease-heartbeat interval in seconds (#243) | | `GITFORGE_FETCH_INTERVAL` | runner | 5 | Job-poll interval in seconds | | `GITFORGE_SCHEDULER_TOKEN` | runner | _(none)_ | Bearer token for scheduler API. Required in compose: the scheduler rejects runner/operator requests when unset | +| `GITFORGE_RUNNER_REGISTRATION_TOKEN` | api, ci, runner | _(none)_ | Bearer token for runner registration at the scheduler (`POST /runners`) and the gateway (`POST /api/runners`). Required in compose: registration is refused when unset. The runner falls back to `GITFORGE_SCHEDULER_TOKEN` when it is unset locally | | `GITFORGE_RUNNER_STANDALONE` | runner | `deny` | `deny` exits the runner when scheduler registration fails; `allow` falls back to standalone execution | | `GITFORGE_REGISTER_ATTEMPTS` | runner | `6` | Registration attempts against an unreachable scheduler before giving up | | `GITFORGE_REGISTER_BACKOFF_SECS` | runner | `1` | Initial registration backoff; doubles per failed attempt up to a 30s cap. Auth rejections (401/403) are never retried | From 8088c2826c984fd9cc058847e6747acf156c5f51 Mon Sep 17 00:00:00 2001 From: Michael Kinney <14325925+aliasfoxkde@users.noreply.github.com> Date: Mon, 2 Nov 2026 13:50:25 -0600 Subject: [PATCH 6/9] fix(scheduler): dedicated runner registration token scope (#357) Registration (POST /runners on scheduler and gateway) now accepts the shared runner token (back-compat) or a dedicated GITFORGE_RUNNER_REGISTRATION_TOKEN, but the registration token no longer authorizes job lifecycle routes. Runner config grows registration_token; tests cover scope separation on both surfaces. Co-Authored-By: Claude --- crates/gitforge-api/src/routes/ci.rs | 2 +- crates/gitforge-runner/src/agent.rs | 70 +++++++++- crates/gitforge-scheduler/src/server.rs | 173 +++++++++++++++++++++++- 3 files changed, 240 insertions(+), 5 deletions(-) diff --git a/crates/gitforge-api/src/routes/ci.rs b/crates/gitforge-api/src/routes/ci.rs index a6d10666..0a1f9396 100644 --- a/crates/gitforge-api/src/routes/ci.rs +++ b/crates/gitforge-api/src/routes/ci.rs @@ -105,7 +105,7 @@ fn can_manage_jobs(claims: &Claims) -> bool { } /// Require ownership of a repository, with admin/maintainer override. -async fn authorize_repo( +pub(crate) async fn authorize_repo( pool: &Pool, claims: &Claims, repo_id: gitforge_common::RepoId, diff --git a/crates/gitforge-runner/src/agent.rs b/crates/gitforge-runner/src/agent.rs index da37057c..1e615f25 100644 --- a/crates/gitforge-runner/src/agent.rs +++ b/crates/gitforge-runner/src/agent.rs @@ -43,6 +43,9 @@ pub struct RunnerConfig { pub fetch_interval_secs: u64, /// Bearer token used for scheduler service authentication. pub scheduler_token: Option, + /// Bearer token for runner registration (#357). Falls back to + /// `scheduler_token` when unset. + pub registration_token: Option, /// Registration attempts against an unreachable scheduler before giving /// up. Covers the compose race where a runner starts before the /// scheduler's listener is up. @@ -91,6 +94,10 @@ impl fmt::Debug for RunnerConfig { "scheduler_token", &self.scheduler_token.as_ref().map(|_| ""), ) + .field( + "registration_token", + &self.registration_token.as_ref().map(|_| ""), + ) .field("register_attempts", &self.register_attempts) .field("register_backoff_secs", &self.register_backoff_secs) .field("allow_standalone", &self.allow_standalone) @@ -111,6 +118,7 @@ impl Default for RunnerConfig { job_heartbeat_interval_secs: 15, fetch_interval_secs: 5, scheduler_token: None, + registration_token: None, register_attempts: 6, register_backoff_secs: 1, allow_standalone: false, @@ -151,6 +159,7 @@ impl RunnerConfig { let mut job_heartbeat_interval_secs: Option = None; let mut fetch_interval_secs: Option = None; let mut scheduler_token: Option> = None; + let mut registration_token: Option> = None; let mut register_attempts: Option = None; let mut register_backoff_secs: Option = None; let mut allow_standalone: Option = None; @@ -239,6 +248,14 @@ impl RunnerConfig { Some(v.to_string()) }); } + "GITFORGE_RUNNER_REGISTRATION_TOKEN" => { + let v = value.trim(); + registration_token = Some(if v.is_empty() { + None + } else { + Some(v.to_string()) + }); + } "GITFORGE_REGISTER_ATTEMPTS" => { let v = value.trim(); if !v.is_empty() { @@ -306,6 +323,7 @@ impl RunnerConfig { job_heartbeat_interval_secs: job_heartbeat_interval_secs.unwrap_or(15), fetch_interval_secs: fetch_interval_secs.unwrap_or(5), scheduler_token: scheduler_token.unwrap_or(None), + registration_token: registration_token.unwrap_or(None), register_attempts: register_attempts.unwrap_or(6), register_backoff_secs: register_backoff_secs.unwrap_or(1), allow_standalone: allow_standalone.unwrap_or(false), @@ -326,6 +344,8 @@ impl RunnerConfig { /// /// **Optional credentials** (no default — runner runs unauthenticated if unset): /// - `GITFORGE_SCHEDULER_TOKEN` — bearer token for scheduler API + /// - `GITFORGE_RUNNER_REGISTRATION_TOKEN` — bearer token for runner + /// registration (#357); falls back to `GITFORGE_SCHEDULER_TOKEN` /// /// **Optional policy:** /// - `GITFORGE_RUNNER_STANDALONE` — `deny` (default) makes registration @@ -388,6 +408,33 @@ mod config_tests { assert_eq!(cfg.runner_type, "docker"); } + /// Registration token parsing (#357): explicit value wins, empty value + /// means unset (falls back to the scheduler token), absent means unset. + #[test] + fn test_parse_registration_token() { + let cfg = RunnerConfig::parse_from_iter(env([ + ("GITFORGE_SCHEDULER_URL", Some("http://ci:42781")), + ("GITFORGE_SCHEDULER_TOKEN", Some("job-token")), + ("GITFORGE_RUNNER_REGISTRATION_TOKEN", Some("reg-token")), + ])) + .unwrap(); + assert_eq!(cfg.registration_token.as_deref(), Some("reg-token")); + + let cfg = RunnerConfig::parse_from_iter(env([ + ("GITFORGE_SCHEDULER_URL", Some("http://ci:42781")), + ("GITFORGE_RUNNER_REGISTRATION_TOKEN", Some("")), + ])) + .unwrap(); + assert!(cfg.registration_token.is_none()); + + let cfg = RunnerConfig::parse_from_iter(env([( + "GITFORGE_SCHEDULER_URL", + Some("http://ci:42781"), + )])) + .unwrap(); + assert!(cfg.registration_token.is_none()); + } + // ── Optional fields absent → defaults ─────────────────────────────────── #[test] @@ -882,7 +929,15 @@ impl RunnerAgent { for attempt in 1..=attempts { let mut register_request = self.client.post(®ister_url).json(&request); - if let Some(token) = &self.config.scheduler_token { + // Registration authenticates with the dedicated registration + // token; the scheduler job token is the fallback so a deployment + // with one shared secret keeps working. + let registration_token = self + .config + .registration_token + .as_deref() + .or(self.config.scheduler_token.as_deref()); + if let Some(token) = registration_token { register_request = register_request.bearer_auth(token); } match register_request.send().await { @@ -2462,6 +2517,7 @@ mod tests { // Registration failure is fatal by default: a runner must not appear // healthy when it cannot reach the scheduler that assigns it work. let config = RunnerConfig { + registration_token: None, scheduler_url: "http://localhost:99999".to_string(), // Invalid URL register_attempts: 1, // no retries: keeps this test instantaneous ..Default::default() @@ -2484,6 +2540,7 @@ mod tests { // Legacy standalone fallback remains available behind an explicit // policy opt-in. let config = RunnerConfig { + registration_token: None, scheduler_url: "http://localhost:99999".to_string(), register_attempts: 1, // no retries: keeps this test instantaneous allow_standalone: true, @@ -2564,6 +2621,7 @@ mod tests { // and adopt the runner id once the scheduler accepts. let (url, connections) = spawn_status_server(&[503, 503, 201]).await; let config = RunnerConfig { + registration_token: None, scheduler_url: url, register_attempts: 5, register_backoff_secs: 0, @@ -2589,6 +2647,7 @@ mod tests { // is made and the error names the authentication failure. let (url, connections) = spawn_status_server(&[401]).await; let config = RunnerConfig { + registration_token: None, scheduler_url: url, register_attempts: 5, register_backoff_secs: 0, @@ -2622,6 +2681,7 @@ mod tests { drop(reserved); let config = RunnerConfig { + registration_token: None, scheduler_url: format!("http://127.0.0.1:{port}"), register_attempts: 3, register_backoff_secs: 0, @@ -2705,6 +2765,7 @@ mod tests { #[test] fn test_runner_config_custom_values() { let config = RunnerConfig { + registration_token: None, scheduler_url: "http://custom:8081".to_string(), name: "custom-runner".to_string(), runner_type: "kubernetes".to_string(), @@ -2950,6 +3011,7 @@ mod tests { return; } let config = RunnerConfig { + registration_token: None, scheduler_url: "http://localhost:99999".to_string(), allow_standalone: true, ..Default::default() @@ -2969,6 +3031,7 @@ mod tests { #[test] fn test_runner_config_all_fields() { let config = RunnerConfig { + registration_token: None, scheduler_url: "http://example.com:8081".to_string(), name: "test-runner".to_string(), runner_type: "firecracker".to_string(), @@ -3085,6 +3148,7 @@ mod tests { return; } let config = RunnerConfig { + registration_token: None, scheduler_url: "http://localhost:99999".to_string(), allow_standalone: true, ..Default::default() @@ -3110,6 +3174,7 @@ mod tests { #[test] fn test_runner_config_with_zero_capacity() { let config = RunnerConfig { + registration_token: None, scheduler_url: "http://localhost:42781".to_string(), name: "zero-cap".to_string(), runner_type: "docker".to_string(), @@ -3203,6 +3268,7 @@ mod tests { #[test] fn test_runner_config_with_special_url() { let config = RunnerConfig { + registration_token: None, scheduler_url: "http://user:pass@host:9090/path".to_string(), name: "special-url-runner".to_string(), runner_type: "docker".to_string(), @@ -3239,6 +3305,7 @@ mod tests { return; } let config = RunnerConfig { + registration_token: None, scheduler_url: "http://custom-scheduler:8081".to_string(), name: "custom-runner".to_string(), runner_type: "kubernetes".to_string(), @@ -3298,6 +3365,7 @@ mod tests { return; } let config = RunnerConfig { + registration_token: None, scheduler_url: "http://localhost:99999".to_string(), allow_standalone: true, ..Default::default() diff --git a/crates/gitforge-scheduler/src/server.rs b/crates/gitforge-scheduler/src/server.rs index e9e832ea..ac7708e0 100644 --- a/crates/gitforge-scheduler/src/server.rs +++ b/crates/gitforge-scheduler/src/server.rs @@ -27,6 +27,11 @@ pub struct SchedulerServerState { pub scheduler: Arc, runner_auth_token: Option>, operator_auth_token: Option>, + /// Alternate credential accepted only on `POST /runners` + /// (`GITFORGE_RUNNER_REGISTRATION_TOKEN`, #357). A deployment can rotate + /// the runner job token without re-provisioning every runner, and the + /// registration secret never authorizes job fetch/complete/heartbeat. + registration_auth_token: Option>, artifact_storage: Option>, } @@ -114,6 +119,12 @@ pub fn create_state_with_artifact_storage( .filter(|token| !token.is_empty()) .map(Arc::from) .or_else(|| shared.clone()); + // The dedicated registration token is read here (not in the CI service) + // so the credential decision stays inside the crate that enforces it. + let registration = std::env::var("GITFORGE_RUNNER_REGISTRATION_TOKEN") + .ok() + .filter(|token| !token.is_empty()) + .map(Arc::from); let operator = std::env::var("GITFORGE_SCHEDULER_OPERATOR_TOKEN") .ok() .filter(|token| !token.is_empty()) @@ -123,6 +134,7 @@ pub fn create_state_with_artifact_storage( scheduler: Arc::new(scheduler), runner_auth_token: runner, operator_auth_token: operator, + registration_auth_token: registration, artifact_storage, } } @@ -152,8 +164,9 @@ pub fn scheduler_routes_with_tokens( runner_auth_token: Option>, operator_auth_token: Option>, ) -> Router { + let registration_auth_token = state.registration_auth_token.clone(); + let lifecycle_runner_token = runner_auth_token.clone(); let runner_routes = Router::new() - .route("/runners", post(register_runner)) .route("/runners/{id}/heartbeat", post(runner_heartbeat)) .route("/jobs/pending", get(get_pending_jobs)) .route("/queue/status", get(get_queue_status)) @@ -166,7 +179,20 @@ pub fn scheduler_routes_with_tokens( .route("/jobs/{id}/cancelled", get(job_cancelled)) .route("/jobs/{id}/complete", post(complete_job)) .layer(middleware::from_fn(move |request, next: Next| { - require_scheduler_auth(request, next, runner_auth_token.clone(), "runner") + require_scheduler_auth(request, next, lifecycle_runner_token.clone(), "runner") + })); + // Registration is its own credential scope: it accepts the runner token + // (back-compat) or the dedicated registration token (#357), but the + // registration token must not reach job lifecycle routes. + let registration_routes = Router::new() + .route("/runners", post(register_runner)) + .layer(middleware::from_fn(move |request, next: Next| { + require_registration_auth( + request, + next, + runner_auth_token.clone(), + registration_auth_token.clone(), + ) })); let operator_routes = Router::new() .route("/jobs/{id}/cancel", post(cancel_job)) @@ -175,7 +201,10 @@ pub fn scheduler_routes_with_tokens( .layer(middleware::from_fn(move |request, next: Next| { require_scheduler_auth(request, next, operator_auth_token.clone(), "operator") })); - runner_routes.merge(operator_routes).with_state(state) + runner_routes + .merge(registration_routes) + .merge(operator_routes) + .with_state(state) } async fn require_scheduler_auth( @@ -207,6 +236,46 @@ async fn require_scheduler_auth( } } +/// Registration bootstrap credential check (#357): accepts the runner job +/// token (single-secret deployments) or the dedicated registration token. +/// When neither is configured the endpoint fails closed with 503, mirroring +/// the gateway's registration-disabled contract. +async fn require_registration_auth( + request: Request, + next: Next, + runner_token: Option>, + registration_token: Option>, +) -> Response { + let accepted: Vec> = [runner_token, registration_token] + .into_iter() + .flatten() + .collect(); + if accepted.is_empty() { + tracing::error!( + "runner registration refused: no registration credential is configured" + ); + return ( + StatusCode::SERVICE_UNAVAILABLE, + Json(serde_json::json!({"error": "registration_disabled"})), + ) + .into_response(); + } + let supplied = request + .headers() + .get(axum::http::header::AUTHORIZATION) + .and_then(|value| value.to_str().ok()) + .and_then(|value| value.strip_prefix("Bearer ")); + if supplied.is_some_and(|token| accepted.iter().any(|entry| &**entry == token)) { + next.run(request).await + } else { + ( + StatusCode::UNAUTHORIZED, + Json(serde_json::json!({"error": "scheduler_auth_required"})), + ) + .into_response() + } +} + /// Return a durable pipeline run for LAN control-plane adapters. async fn get_pipeline_run( State(state): State, @@ -1882,6 +1951,7 @@ mod tests { scheduler: Arc::new(crate::Scheduler::new()), runner_auth_token: Some(Arc::from("runner-secret")), operator_auth_token: Some(Arc::from("operator-secret")), + registration_auth_token: None, artifact_storage: None, } } @@ -1927,6 +1997,102 @@ mod tests { assert_eq!(response.status(), StatusCode::UNAUTHORIZED); } + /// Registration credential contract (#357): the dedicated registration + /// token registers, a wrong token is refused, the registration token + /// never reaches job lifecycle routes, and with no credential configured + /// the endpoint fails closed with 503. + #[tokio::test] + async fn test_registration_token_scoping() { + let state = SchedulerServerState { + scheduler: Arc::new(crate::Scheduler::new()), + runner_auth_token: Some(Arc::from("runner-secret")), + operator_auth_token: Some(Arc::from("operator-secret")), + registration_auth_token: Some(Arc::from("registration-secret")), + artifact_storage: None, + }; + let app: Router = scheduler_routes_with_tokens( + state, + Some(Arc::from("runner-secret")), + Some(Arc::from("operator-secret")), + ); + + let payload = |name: &str| { + axum::body::Body::from( + serde_json::json!({"name": name, "type": "docker", "capacity": 1}).to_string(), + ) + }; + let register = |app: Router, token: Option<&str>, name: &str| { + let mut builder = Request::builder() + .method("POST") + .uri("/runners") + .header("content-type", "application/json"); + if let Some(token) = token { + builder = builder.header("authorization", format!("Bearer {token}")); + } + app.oneshot(builder.body(payload(name)).unwrap()) + }; + + let response = register(app.clone(), Some("registration-secret"), "reg-token-runner") + .await + .unwrap(); + assert_eq!(response.status(), StatusCode::CREATED); + + let response = register(app.clone(), Some("runner-secret"), "runner-token-reg") + .await + .unwrap(); + assert_eq!(response.status(), StatusCode::CREATED); + + let response = register(app.clone(), Some("wrong-secret"), "attacker") + .await + .unwrap(); + assert_eq!(response.status(), StatusCode::UNAUTHORIZED); + + let response = register(app.clone(), None, "anonymous") + .await + .unwrap(); + assert_eq!(response.status(), StatusCode::UNAUTHORIZED); + + // The registration token must not authorize the job lifecycle: it is + // a bootstrap credential, not a runner credential. + let response = app + .oneshot( + Request::builder() + .uri("/jobs/pending") + .header("authorization", "Bearer registration-secret") + .body(axum::body::Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + assert_eq!(response.status(), StatusCode::UNAUTHORIZED); + } + + #[tokio::test] + async fn test_registration_without_credentials_fails_closed() { + let state = SchedulerServerState { + scheduler: Arc::new(crate::Scheduler::new()), + runner_auth_token: None, + operator_auth_token: None, + registration_auth_token: None, + artifact_storage: None, + }; + let app: Router = scheduler_routes_with_tokens(state, None, None); + let response = app + .oneshot( + Request::builder() + .method("POST") + .uri("/runners") + .header("content-type", "application/json") + .body(axum::body::Body::from( + serde_json::json!({"name": "orphan", "type": "docker"}).to_string(), + )) + .unwrap(), + ) + .await + .unwrap(); + assert_eq!(response.status(), StatusCode::SERVICE_UNAVAILABLE); + } + #[tokio::test] async fn test_operator_token_can_cancel_job() { let state = authenticated_test_state(); @@ -1988,6 +2154,7 @@ mod tests { scheduler: Arc::new(crate::Scheduler::with_db(pool)), runner_auth_token: Some(Arc::from("runner-secret")), operator_auth_token: Some(Arc::from("operator-secret")), + registration_auth_token: None, artifact_storage: None, }; let app: Router = scheduler_routes_with_tokens( From e58597d02a53a49b8cbc4d17d9e6e85bc36978a7 Mon Sep 17 00:00:00 2001 From: Michael Kinney <14325925+aliasfoxkde@users.noreply.github.com> Date: Mon, 2 Nov 2026 14:13:14 -0600 Subject: [PATCH 7/9] style(scheduler): rustfmt drift in registration middleware Co-Authored-By: Claude --- crates/gitforge-scheduler/src/server.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/crates/gitforge-scheduler/src/server.rs b/crates/gitforge-scheduler/src/server.rs index ac7708e0..3b9be420 100644 --- a/crates/gitforge-scheduler/src/server.rs +++ b/crates/gitforge-scheduler/src/server.rs @@ -251,9 +251,7 @@ async fn require_registration_auth( .flatten() .collect(); if accepted.is_empty() { - tracing::error!( - "runner registration refused: no registration credential is configured" - ); + tracing::error!("runner registration refused: no registration credential is configured"); return ( StatusCode::SERVICE_UNAVAILABLE, Json(serde_json::json!({"error": "registration_disabled"})), @@ -2047,9 +2045,7 @@ mod tests { .unwrap(); assert_eq!(response.status(), StatusCode::UNAUTHORIZED); - let response = register(app.clone(), None, "anonymous") - .await - .unwrap(); + let response = register(app.clone(), None, "anonymous").await.unwrap(); assert_eq!(response.status(), StatusCode::UNAUTHORIZED); // The registration token must not authorize the job lifecycle: it is From cf648aaa39e4db204ef0812911384ab2b8c8e3ba Mon Sep 17 00:00:00 2001 From: Michael Kinney <14325925+aliasfoxkde@users.noreply.github.com> Date: Mon, 2 Nov 2026 15:40:00 -0600 Subject: [PATCH 8/9] fix(cli): pipeline commands fail loudly instead of exit 0 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Trigger, watch, create, list, show, and delete errors were printed with a ❌ prefix and swallowed; watch timeouts also exited 0. Convert every pipeline error path to anyhow::bail so failures exit non-zero, and fail a trigger whose run was never scheduled. Co-Authored-By: Claude --- crates/gitforge-cli/src/main.rs | 206 ++++++++++++++++++++------------ 1 file changed, 127 insertions(+), 79 deletions(-) diff --git a/crates/gitforge-cli/src/main.rs b/crates/gitforge-cli/src/main.rs index b4d960e2..ce8341ea 100644 --- a/crates/gitforge-cli/src/main.rs +++ b/crates/gitforge-cli/src/main.rs @@ -863,7 +863,7 @@ pub async fn run_cli(cli: Cli) -> Result<()> { println!(" Run `gitforge pipeline create ` to create a pipeline."); } Err(e) => { - println!("❌ {e}"); + anyhow::bail!("failed to list pipelines: {e}"); } } } else if let Some(id) = show { @@ -875,7 +875,7 @@ pub async fn run_cli(cli: Cli) -> Result<()> { println!(" Enabled: {}", pipeline.enabled); } Err(e) => { - println!("❌ {e}"); + anyhow::bail!("failed to fetch pipeline {id}: {e}"); } } } else if let Some(id) = run { @@ -887,18 +887,26 @@ pub async fn run_cli(cli: Cli) -> Result<()> { println!(" Run: {run_id}"); println!(" Watch it: gitforge pipeline --watch {run_id}"); } - None => println!( - " Accepted, but the orchestrator did not schedule a run \ - (pipeline may not be registered with the scheduler)" - ), + None => { + anyhow::bail!( + "pipeline {id} accepted the trigger, but the orchestrator \ + did not schedule a run (pipeline may not be registered \ + with the scheduler)" + ); + } } } - Err(e) => println!("❌ {e}"), + Err(e) => anyhow::bail!("failed to trigger pipeline {id}: {e}"), } } else if let Some(run_id) = watch { match watch_pipeline_run(&api_client, run_id).await { - Ok(status) => println!("👁️ Run {run_id} finished: {status}"), - Err(e) => println!("❌ {e}"), + Ok(status) => { + println!("👁️ Run {run_id} finished: {status}"); + if status != "succeeded" { + anyhow::bail!("run {run_id} finished with status: {status}"); + } + } + Err(e) => anyhow::bail!("failed to watch run {run_id}: {e}"), } } else if let Some(repo) = create { let path = create_file.as_deref().unwrap_or(".gitforge.yml"); @@ -909,9 +917,9 @@ pub async fn run_cli(cli: Cli) -> Result<()> { println!(" ID: {}", pipeline.id); println!(" Run it: gitforge pipeline --run {}", pipeline.id); } - Err(e) => println!("❌ {e}"), + Err(e) => anyhow::bail!("failed to create pipeline: {e}"), }, - Err(e) => println!("❌ Failed to read {path}: {e}"), + Err(e) => anyhow::bail!("failed to read {path}: {e}"), } } else if let Some(id) = delete { match api_client.delete_pipeline(id).await { @@ -922,7 +930,7 @@ pub async fn run_cli(cli: Cli) -> Result<()> { .unwrap_or("pipeline removed"); println!("🗑️ Pipeline {id}: {message}"); } - Err(e) => println!("❌ {e}"), + Err(e) => anyhow::bail!("failed to delete pipeline {id}: {e}"), } } } @@ -1465,6 +1473,10 @@ mod tests { } } + fn test_command(command: Commands) -> Commands { + command + } + #[tokio::test] async fn test_auth_status_not_authenticated() { let cli = test_cli(Commands::Auth { @@ -1580,19 +1592,30 @@ mod tests { assert!(run_cli(cli).await.is_ok()); } + /// An address nothing can listen on: the API call must surface as an + /// error now that command arms propagate failures instead of printing. + fn unreachable() -> Option { + Some("http://127.0.0.1:1".to_string()) + } + #[tokio::test] - async fn test_pipeline_list() { - let cli = test_cli(Commands::Pipeline { - list: true, - show: None, - run: None, - ref_: None, - watch: None, - create: None, - file: None, - delete: None, - }); - assert!(run_cli(cli).await.is_ok()); + async fn test_pipeline_list_unreachable_server_is_err() { + let cli = Cli { + verbose: false, + server: unreachable(), + token: None, + command: test_command(Commands::Pipeline { + list: true, + show: None, + run: None, + ref_: None, + watch: None, + create: None, + file: None, + delete: None, + }), + }; + assert!(run_cli(cli).await.is_err()); } #[tokio::test] @@ -1754,77 +1777,102 @@ mod tests { #[tokio::test] async fn test_pipeline_show() { - let cli = test_cli(Commands::Pipeline { - list: false, - show: Some("pipeline-123".to_string()), - run: None, - ref_: None, - watch: None, - create: None, - file: None, - delete: None, - }); - assert!(run_cli(cli).await.is_ok()); + let cli = Cli { + verbose: false, + server: unreachable(), + token: None, + command: test_command(Commands::Pipeline { + list: false, + show: Some("pipeline-123".to_string()), + run: None, + ref_: None, + watch: None, + create: None, + file: None, + delete: None, + }), + }; + assert!(run_cli(cli).await.is_err()); } #[tokio::test] async fn test_pipeline_run() { - let cli = test_cli(Commands::Pipeline { - list: false, - show: None, - run: Some("pipeline-123".to_string()), - ref_: None, - watch: None, - create: None, - file: None, - delete: None, - }); - assert!(run_cli(cli).await.is_ok()); + let cli = Cli { + verbose: false, + server: unreachable(), + token: None, + command: test_command(Commands::Pipeline { + list: false, + show: None, + run: Some("pipeline-123".to_string()), + ref_: None, + watch: None, + create: None, + file: None, + delete: None, + }), + }; + assert!(run_cli(cli).await.is_err()); } #[tokio::test] async fn test_pipeline_watch() { - let cli = test_cli(Commands::Pipeline { - list: false, - show: None, - run: None, - ref_: None, - watch: Some("pipeline-123".to_string()), - create: None, - file: None, - delete: None, - }); - assert!(run_cli(cli).await.is_ok()); + let cli = Cli { + verbose: false, + server: unreachable(), + token: None, + command: test_command(Commands::Pipeline { + list: false, + show: None, + run: None, + ref_: None, + watch: Some("pipeline-123".to_string()), + create: None, + file: None, + delete: None, + }), + }; + assert!(run_cli(cli).await.is_err()); } #[tokio::test] async fn test_pipeline_create() { - let cli = test_cli(Commands::Pipeline { - list: false, - show: None, - run: None, - ref_: None, - watch: None, - create: Some("new-pipeline".to_string()), - file: None, - delete: None, - }); - assert!(run_cli(cli).await.is_ok()); + let cli = Cli { + verbose: false, + server: unreachable(), + token: None, + command: test_command(Commands::Pipeline { + list: false, + show: None, + run: None, + ref_: None, + watch: None, + create: Some("new-pipeline".to_string()), + file: None, + delete: None, + }), + }; + assert!(run_cli(cli).await.is_err()); } #[tokio::test] async fn test_pipeline_delete() { - let cli = test_cli(Commands::Pipeline { - list: false, - show: None, - run: None, - ref_: None, - watch: None, - create: None, - file: None, - delete: Some("pipeline-123".to_string()), - }); - assert!(run_cli(cli).await.is_ok()); + let cli = Cli { + verbose: false, + server: unreachable(), + token: None, + command: test_command(Commands::Pipeline { + list: false, + show: None, + run: None, + ref_: None, + watch: None, + create: None, + file: None, + delete: Some("pipeline-123".to_string()), + }), + }; + assert!(run_cli(cli).await.is_err()); } #[tokio::test] From ce06089eb6a414ff00e48a8585c8627d1d809463 Mon Sep 17 00:00:00 2001 From: Michael Kinney <14325925+aliasfoxkde@users.noreply.github.com> Date: Mon, 2 Nov 2026 15:52:47 -0600 Subject: [PATCH 9/9] fix(ci): route fork PRs to ephemeral hosted runners (#318) Persistent self-hosted runners must never execute fork pull_request code. Same-repo PRs and push/tag/schedule keep the gitforge-github runner; fork PRs run the same checks on disposable ubuntu-latest. Also fix qualityctl: pass GITFORGE_BUILD_SOCKET to the daemon it spawns, and drop test parallelism to 2 to survive memory pressure. Co-Authored-By: Claude --- .github/workflows/ai-review.yml | 2 +- .github/workflows/always.yml | 2 +- .github/workflows/codeql.yml | 2 +- .github/workflows/gitforge-ci.yml | 14 ++++--- .github/workflows/integration.yml | 4 +- .github/workflows/rust-ci.yml | 14 +++---- .github/workflows/security.yml | 10 ++--- README.md | 2 +- docs/CI_RUNNER_TRUST_POLICY.md | 70 +++++++++++++++++++++++++++++++ scripts/qualityctl | 4 +- 10 files changed, 99 insertions(+), 25 deletions(-) create mode 100644 docs/CI_RUNNER_TRUST_POLICY.md diff --git a/.github/workflows/ai-review.yml b/.github/workflows/ai-review.yml index 2361f47e..f7767917 100644 --- a/.github/workflows/ai-review.yml +++ b/.github/workflows/ai-review.yml @@ -40,7 +40,7 @@ env: jobs: ai-review: name: AI Code Review - runs-on: [self-hosted, linux, x64, gitforge-github] + runs-on: ${{ !github.event.pull_request.head.repo.fork && fromJSON('["self-hosted", "linux", "x64", "gitforge-github"]') || 'ubuntu-latest' }} steps: - name: Checkout code uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 diff --git a/.github/workflows/always.yml b/.github/workflows/always.yml index a8127039..022b69b1 100644 --- a/.github/workflows/always.yml +++ b/.github/workflows/always.yml @@ -21,7 +21,7 @@ permissions: jobs: check: - runs-on: [self-hosted, linux, x64, gitforge-github] + runs-on: ubuntu-latest steps: - name: CI check run: echo "CI check passed — repository validated" diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index 6278aed9..0a8c8eb6 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -16,7 +16,7 @@ concurrency: jobs: analyze: name: Analyze - runs-on: [self-hosted, linux, x64, gitforge-github] + runs-on: ${{ !github.event.pull_request.head.repo.fork && fromJSON('["self-hosted", "linux", "x64", "gitforge-github"]') || 'ubuntu-latest' }} # Rust query evaluation can exceed the default job budget on this # multi-crate workspace; keep the guard bounded but allow completion. timeout-minutes: 45 diff --git a/.github/workflows/gitforge-ci.yml b/.github/workflows/gitforge-ci.yml index 058b6f23..173170e9 100644 --- a/.github/workflows/gitforge-ci.yml +++ b/.github/workflows/gitforge-ci.yml @@ -38,8 +38,12 @@ jobs: # Enqueue build job to GitForge queue instead of running directly enqueue-build: name: Enqueue Build to GitForge - if: vars.GITFORGE_ENABLED == 'true' - runs-on: [self-hosted, linux, x64, gitforge-github] + # Fork PRs never reach the private GitForge instance: secrets are + # unavailable and their head commits are not on the mirror. The + # required-check gap this would leave is acceptable — the live + # Safeguards ruleset carries no required_status_checks rule. + if: vars.GITFORGE_ENABLED == 'true' && !github.event.pull_request.head.repo.fork + runs-on: ${{ !github.event.pull_request.head.repo.fork && fromJSON('["self-hosted", "linux", "x64", "gitforge-github"]') || 'ubuntu-latest' }} outputs: run_id: ${{ steps.enqueue.outputs.run_id }} commit: ${{ steps.enqueue.outputs.commit }} @@ -138,7 +142,7 @@ jobs: name: Poll GitForge Build Status needs: enqueue-build if: needs.enqueue-build.outputs.build_status == 'enqueued' - runs-on: [self-hosted, linux, x64, gitforge-github] + runs-on: ${{ !github.event.pull_request.head.repo.fork && fromJSON('["self-hosted", "linux", "x64", "gitforge-github"]') || 'ubuntu-latest' }} outputs: terminal_status: ${{ steps.poll.outputs.terminal_status }} steps: @@ -247,8 +251,8 @@ jobs: report-status: name: Report Build Status needs: [enqueue-build, poll-build] - if: always() && vars.GITFORGE_ENABLED == 'true' - runs-on: [self-hosted, linux, x64, gitforge-github] + if: always() && vars.GITFORGE_ENABLED == 'true' && !github.event.pull_request.head.repo.fork + runs-on: ${{ !github.event.pull_request.head.repo.fork && fromJSON('["self-hosted", "linux", "x64", "gitforge-github"]') || 'ubuntu-latest' }} steps: - name: Report completion run: | diff --git a/.github/workflows/integration.yml b/.github/workflows/integration.yml index 7d4d82c4..a35ee96b 100644 --- a/.github/workflows/integration.yml +++ b/.github/workflows/integration.yml @@ -15,7 +15,7 @@ permissions: jobs: integration: - runs-on: [self-hosted, linux, x64, gitforge-github] + runs-on: ${{ !github.event.pull_request.head.repo.fork && fromJSON('["self-hosted", "linux", "x64", "gitforge-github"]') || 'ubuntu-latest' }} steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -42,7 +42,7 @@ jobs: fi e2e: - runs-on: [self-hosted, linux, x64, gitforge-github] + runs-on: ${{ !github.event.pull_request.head.repo.fork && fromJSON('["self-hosted", "linux", "x64", "gitforge-github"]') || 'ubuntu-latest' }} steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 diff --git a/.github/workflows/rust-ci.yml b/.github/workflows/rust-ci.yml index 889fd153..f8f3786b 100644 --- a/.github/workflows/rust-ci.yml +++ b/.github/workflows/rust-ci.yml @@ -26,7 +26,7 @@ jobs: # ─── Code Formatting ───────────────────────────────────────────────────────── fmt: name: Format - runs-on: [self-hosted, linux, x64, gitforge-github] + runs-on: ${{ !github.event.pull_request.head.repo.fork && fromJSON('["self-hosted", "linux", "x64", "gitforge-github"]') || 'ubuntu-latest' }} steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - name: Install Rust @@ -40,7 +40,7 @@ jobs: # ─── Linting ──────────────────────────────────────────────────────────────── clippy: name: Clippy - runs-on: [self-hosted, linux, x64, gitforge-github] + runs-on: ${{ !github.event.pull_request.head.repo.fork && fromJSON('["self-hosted", "linux", "x64", "gitforge-github"]') || 'ubuntu-latest' }} steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - name: Install Rust @@ -86,7 +86,7 @@ jobs: # GITFORGE_SANDBOX_ACQUIRE_SECS raises the acquire window alongside it. test: name: Test - runs-on: [self-hosted, linux, x64, gitforge-github] + runs-on: ${{ !github.event.pull_request.head.repo.fork && fromJSON('["self-hosted", "linux", "x64", "gitforge-github"]') || 'ubuntu-latest' }} env: GITFORGE_DOCKER_TIMEOUT_SECS: "600" GITFORGE_SANDBOX_ACQUIRE_SECS: "300" @@ -104,7 +104,7 @@ jobs: # race detector requires a separate supported toolchain/target strategy. test-serialized: name: Test (Serialized) - runs-on: [self-hosted, linux, x64, gitforge-github] + runs-on: ${{ !github.event.pull_request.head.repo.fork && fromJSON('["self-hosted", "linux", "x64", "gitforge-github"]') || 'ubuntu-latest' }} env: GITFORGE_DOCKER_TIMEOUT_SECS: "600" GITFORGE_SANDBOX_ACQUIRE_SECS: "300" @@ -129,7 +129,7 @@ jobs: # 120 s-timeout signature on 2026-09-24 while the Test jobs stayed green). coverage: name: Coverage - runs-on: [self-hosted, linux, x64, gitforge-github] + runs-on: ${{ !github.event.pull_request.head.repo.fork && fromJSON('["self-hosted", "linux", "x64", "gitforge-github"]') || 'ubuntu-latest' }} env: CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} GITFORGE_DOCKER_TIMEOUT_SECS: "600" @@ -192,7 +192,7 @@ jobs: # ─── Security Audit ───────────────────────────────────────────────────────── security: name: Security Audit - runs-on: [self-hosted, linux, x64, gitforge-github] + runs-on: ${{ !github.event.pull_request.head.repo.fork && fromJSON('["self-hosted", "linux", "x64", "gitforge-github"]') || 'ubuntu-latest' }} steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - name: Install Rust @@ -226,7 +226,7 @@ jobs: # ─── Build ────────────────────────────────────────────────────────────────── build: name: Build - runs-on: [self-hosted, linux, x64, gitforge-github] + runs-on: ${{ !github.event.pull_request.head.repo.fork && fromJSON('["self-hosted", "linux", "x64", "gitforge-github"]') || 'ubuntu-latest' }} steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - name: Install Rust diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml index 45d0599e..9e40ac1f 100644 --- a/.github/workflows/security.yml +++ b/.github/workflows/security.yml @@ -32,7 +32,7 @@ jobs: # ─── Security Audit (Rust) ───────────────────────────────────────────────── cargo-audit: name: Cargo Audit - runs-on: [self-hosted, linux, x64, gitforge-github] + runs-on: ${{ !github.event.pull_request.head.repo.fork && fromJSON('["self-hosted", "linux", "x64", "gitforge-github"]') || 'ubuntu-latest' }} steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -49,7 +49,7 @@ jobs: # ─── Secrets scan (all repos — always runs) ───────────────────────────────── secrets-scan: - runs-on: [self-hosted, linux, x64, gitforge-github] + runs-on: ${{ !github.event.pull_request.head.repo.fork && fromJSON('["self-hosted", "linux", "x64", "gitforge-github"]') || 'ubuntu-latest' }} steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: @@ -75,7 +75,7 @@ jobs: # dependency-graph capability is enabled. Cargo Audit remains mandatory; # opt this supplementary gate in only after GitHub reports it supported. if: vars.DEPENDENCY_REVIEW_ENABLED == 'true' - runs-on: [self-hosted, linux, x64, gitforge-github] + runs-on: ${{ !github.event.pull_request.head.repo.fork && fromJSON('["self-hosted", "linux", "x64", "gitforge-github"]') || 'ubuntu-latest' }} permissions: contents: read steps: @@ -89,7 +89,7 @@ jobs: # an unreviewable baseline. New high/critical findings fail the workflow. aegis-scan: name: Aegis Changed-Line Scan - runs-on: [self-hosted, linux, x64, gitforge-github] + runs-on: ${{ !github.event.pull_request.head.repo.fork && fromJSON('["self-hosted", "linux", "x64", "gitforge-github"]') || 'ubuntu-latest' }} steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: @@ -192,7 +192,7 @@ jobs: # ─── Rust-specific Security Checks ───────────────────────────────────────── rust-security: name: Rust Security - runs-on: [self-hosted, linux, x64, gitforge-github] + runs-on: ${{ !github.event.pull_request.head.repo.fork && fromJSON('["self-hosted", "linux", "x64", "gitforge-github"]') || 'ubuntu-latest' }} steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 diff --git a/README.md b/README.md index a1a0e647..14c421e5 100644 --- a/README.md +++ b/README.md @@ -80,7 +80,7 @@ docker-compose up -d | **User Guides** | [Deployment](docs/DEPLOYMENT.md) · [Runbook](docs/RUNBOOK.md) · [API Reference](docs/API.md) | | **Architecture** | [Architecture Overview](docs/ARCHITECTURE.md) · [Hooks](docs/HOOKS.md) · [Testing Strategy](docs/TESTING_STRATEGY.md) | | **Development** | [Contributing](docs/CONTRIBUTING.md) · [Branch Strategy](docs/BRANCH_STRATEGY.md) | -| **Project** | [Changelog](docs/CHANGELOG_RECENT.md) · [Security](docs/SECURITY.md) · [Release readiness](docs/RELEASE_READINESS.md) | +| **Project** | [Changelog](docs/CHANGELOG_RECENT.md) · [Security](docs/SECURITY.md) · [Release readiness](docs/RELEASE_READINESS.md) · [CI runner trust policy](docs/CI_RUNNER_TRUST_POLICY.md) | ## Project Structure diff --git a/docs/CI_RUNNER_TRUST_POLICY.md b/docs/CI_RUNNER_TRUST_POLICY.md new file mode 100644 index 00000000..6b790c14 --- /dev/null +++ b/docs/CI_RUNNER_TRUST_POLICY.md @@ -0,0 +1,70 @@ +# CI Runner Trust & Required-Check Policy + +Resolves #318 (evidence 2026-10-15; implemented 2026-11-02). + +## Execution boundary for untrusted PR code + +This repository is **public with fork-allow enabled**. GitHub's guidance is +unambiguous: persistent self-hosted runners must never execute code from +untrusted `pull_request` events, because PR checkout runs with no secrets but +full access to the runner host's filesystem, Docker socket, and any ambient +credentials. + +### Policy + +`pull_request` jobs route by origin: + +- **Same-repo PRs and all `push`/`tag`/`schedule` events** → persistent + `[self-hosted, linux, x64, gitforge-github]` runner (`fedora-gitforge-01`). +- **Fork PRs** → ephemeral GitHub-hosted `ubuntu-latest` runners. Same checks, + same steps, disposable host. + +Implementation: every job that can fire on a `pull_request` uses + +```yaml +runs-on: ${{ !github.event.pull_request.head.repo.fork && fromJSON('["self-hosted", "linux", "x64", "gitforge-github"]') || 'ubuntu-latest' }} +``` + +Fork-reachability audit (2026-11-02): rust-ci (7 jobs), security (5), codeql +(1), integration (2), gitforge-ci (3), ai-review (1) — all guarded. +`always.yml` runs its trivial echo on `ubuntu-latest` unconditionally. +`auto-merge.yml` is fork-gated by its `user.login == repository.owner.login` +condition. `release.yml` and `wiki.yml` have no `pull_request` trigger. + +### Secrets on fork PRs + +Fork PRs receive no `secrets.*` (except `GITHUB_TOKEN`, read-scoped). This is +load-bearing: `gitforge-ci.yml` additionally gates its `enqueue-build` and +`report-status` jobs on `!github.event.pull_request.head.repo.fork`, because +fork PR head commits do not exist on the 202 mirror and the trigger token +would 401 anyway. A fork PR therefore skips the GitForge pipeline leg rather +than failing against it. + +### Residual exposure + +`pull_request` forks still get a *fresh* GitHub-hosted VM, so no persistent +state (Fedora services, NAS mounts, runner workspaces, Docker images) is +reachable from fork code. There is no `pull_request_target` usage in this +repo; adding one requires the same review bar as a credential-bearing change. + +## Required checks + +The live `Safeguards` ruleset (as of 2026-11-02) contains **no +`required_status_checks` rule**: enforcement is via `code_coverage` +(min 70%, max drop 10%) and `code_scanning` (CodeQL, all alerts). Branch +protection is not separately configured. Consequently the historical +concern — path-filtered workflows leaving 20 required contexts pending on a +docs-only PR — does not apply to the current ruleset. + +If required contexts are re-added, follow the issue's rule: gate workflows +must trigger unconditionally (event filters on `types`, never `paths`) with +per-job `paths` scoping inside, so every required context is emitted for +every applicable PR. + +## Promotion of changes to this policy + +Workflow or ruleset changes follow the standard review path: PR into `main`, +validated by the checks this repo runs (including the GitForge pipeline on +the 202 instance), merged under the Safeguards ruleset. The ruleset itself +changes via the API (`PUT /repos/{owner}/{repo}/rulesets/{id}`) and must be +verified afterwards by reading it back. diff --git a/scripts/qualityctl b/scripts/qualityctl index 01471f77..bf21f34d 100755 --- a/scripts/qualityctl +++ b/scripts/qualityctl @@ -52,7 +52,7 @@ cleanup() { trap cleanup EXIT INT TERM if [[ ! -S "$SOCKET" ]]; then - "$BUILD_DAEMON" >"$log_dir/daemon.log" 2>&1 & + GITFORGE_BUILD_SOCKET="$SOCKET" "$BUILD_DAEMON" >"$log_dir/daemon.log" 2>&1 & daemon_pid=$! for _ in $(seq 1 100); do [[ -S "$SOCKET" ]] && break @@ -129,7 +129,7 @@ PY failed=0 record_stage format fmt --all -- --check || failed=1 record_stage clippy clippy --workspace --all-targets -- -D warnings || failed=1 -record_stage test test --workspace --locked --no-fail-fast || failed=1 +record_stage test test --workspace --locked --no-fail-fast -- --test-threads=2 || failed=1 if [[ "$FAST" != true ]]; then record_stage coverage llvm-cov --workspace --all-features --locked --no-fail-fast \ --cobertura --output-path target/quality/cobertura.xml --fail-under-lines 79.9 || failed=1