From 804eb4d0129ab3e93421387e2c44040810e106bc Mon Sep 17 00:00:00 2001 From: Bnjoroge Date: Thu, 20 Aug 2026 21:29:25 -0400 Subject: [PATCH 1/2] fix(log): scrub capability tokens from INFO/WARN logs The conformance recorder keeps full fidelity, but the control plane's own logs must not carry bearer material: registration tokens, blob-store tokens, artifact bodies, and distributed-task payloads were interpolated into INFO/WARN records that land in journald and, with the observability layer, OTLP. Each call site now logs operation, kind, size, block, and result fields instead of the capability itself. Adds rules/no-sensitive-log-fields.yml, an ast-grep rule that fails the scan for any tracing field named token, authorization, cookie, headers, body, payload, or signed_url (shorthand and assigned forms), exempting the conformance recorder. Entire-Checkpoint: 01M0GYZSHJ4PMAKPH7JJBVGTHB --- .../src/artifact_twirp.rs | 7 ++- .../preloop-runner-server/src/blob_store.rs | 24 ++++++---- .../src/distributed_task.rs | 12 ++++- .../src/results_twirp.rs | 25 +++++++--- crates/preloop-runner-server/src/store.rs | 2 +- rules/no-sensitive-log-fields.yml | 48 +++++++++++++++++++ 6 files changed, 100 insertions(+), 18 deletions(-) create mode 100644 rules/no-sensitive-log-fields.yml diff --git a/crates/preloop-runner-server/src/artifact_twirp.rs b/crates/preloop-runner-server/src/artifact_twirp.rs index c2ebfb49..e8ba7fa6 100644 --- a/crates/preloop-runner-server/src/artifact_twirp.rs +++ b/crates/preloop-runner-server/src/artifact_twirp.rs @@ -91,7 +91,12 @@ pub(crate) async fn twirp_artifact_v2_create( } } let upload_url = format!("{}/twirp-blob/artifact/{token}", runner_base_url()); - info!(token, name = request.name, "artifact v2 create"); + info!( + name = request.name, + workflow_run_backend_id = request.workflow_run_backend_id, + workflow_job_run_backend_id = request.workflow_job_run_backend_id, + "artifact v2 create" + ); Ok(Json(json!({ "ok": true, "signed_upload_url": upload_url }))) } diff --git a/crates/preloop-runner-server/src/blob_store.rs b/crates/preloop-runner-server/src/blob_store.rs index 41fa0cd2..0d308f40 100644 --- a/crates/preloop-runner-server/src/blob_store.rs +++ b/crates/preloop-runner-server/src/blob_store.rs @@ -60,14 +60,13 @@ pub(crate) async fn blob_put( let safe_id = blockid_to_filename(&block_id); let blocks_dir = blob_root.join("blocks"); if let Err(e) = tokio::fs::create_dir_all(&blocks_dir).await { - warn!(kind, token, "failed to create blocks dir: {e}"); + warn!(kind, "failed to create blocks dir: {e}"); return StatusCode::INTERNAL_SERVER_ERROR; } match tokio::fs::write(blocks_dir.join(&safe_id), &body).await { Ok(()) => { debug!( kind, - token, block = safe_id, bytes = body.len(), "blob block staged" @@ -75,7 +74,7 @@ pub(crate) async fn blob_put( StatusCode::CREATED } Err(e) => { - warn!(kind, token, "failed to write block {safe_id}: {e}"); + warn!(kind, block = %safe_id, "failed to write block: {e}"); StatusCode::INTERNAL_SERVER_ERROR } } @@ -92,7 +91,7 @@ pub(crate) async fn blob_put( match tokio::fs::read(blocks_dir.join(&safe_id)).await { Ok(bytes) => assembled.extend_from_slice(&bytes), Err(e) => { - warn!(kind, token, "failed to read block {safe_id}: {e}"); + warn!(kind, block = %safe_id, "failed to read block: {e}"); return StatusCode::INTERNAL_SERVER_ERROR; } } @@ -102,7 +101,6 @@ pub(crate) async fn blob_put( let _ = tokio::fs::remove_dir_all(&blocks_dir).await; info!( kind, - token, size = assembled.len(), blocks = block_ids.len(), "blob assembled from blocks" @@ -110,7 +108,11 @@ pub(crate) async fn blob_put( StatusCode::CREATED } Err(e) => { - warn!(kind, token, "failed to write assembled blob: {e}"); + warn!( + kind, + blocks = block_ids.len(), + "failed to write assembled blob: {e}" + ); StatusCode::INTERNAL_SERVER_ERROR } } @@ -118,17 +120,21 @@ pub(crate) async fn blob_put( _ => { // Single-shot upload. if let Err(e) = tokio::fs::create_dir_all(&blob_root).await { - warn!(kind, token, "failed to create blob dir: {e}"); + warn!(kind, "failed to create blob dir: {e}"); return StatusCode::INTERNAL_SERVER_ERROR; } let data_path = blob_root.join("data"); match tokio::fs::write(&data_path, &body).await { Ok(()) => { - info!(kind, token, size = body.len(), "blob single-shot upload"); + info!(kind, size = body.len(), "blob single-shot upload"); StatusCode::CREATED } Err(e) => { - warn!(kind, token, "failed to write single-shot blob: {e}"); + warn!( + kind, + size = body.len(), + "failed to write single-shot blob: {e}" + ); StatusCode::INTERNAL_SERVER_ERROR } } diff --git a/crates/preloop-runner-server/src/distributed_task.rs b/crates/preloop-runner-server/src/distributed_task.rs index 6bfde3f2..55fd1eb6 100644 --- a/crates/preloop-runner-server/src/distributed_task.rs +++ b/crates/preloop-runner-server/src/distributed_task.rs @@ -324,7 +324,17 @@ pub(crate) async fn agent_request_patch( Path((pool_id, request_id)): Path<(i64, i64)>, Json(body): Json, ) -> Json { - info!(?body, "agent_request_patch received"); + let result_hint = body + .get("result") + .and_then(|v| v.as_str()) + .unwrap_or("renew"); + info!( + pool_id, + request_id, + result = %result_hint, + has_result = body.get("result").is_some(), + "agent_request_patch received" + ); // If this is a completion (has result), delegate to complete_job_inner // so summarize_run, promote_ready_jobs, and notify_waiters all fire. // The result field is only present on the final PATCH; renewals have no result. diff --git a/crates/preloop-runner-server/src/results_twirp.rs b/crates/preloop-runner-server/src/results_twirp.rs index dbcfd84c..ca0732f5 100644 --- a/crates/preloop-runner-server/src/results_twirp.rs +++ b/crates/preloop-runner-server/src/results_twirp.rs @@ -683,8 +683,8 @@ pub(crate) async fn twirp_cache_v2_create( inner.cache_v2_pending.insert( token.clone(), CacheV2Pending { - key: storage_key, - version, + key: storage_key.clone(), + version: version.clone(), }, ); let meta = crate::store::build_meta_snapshot(&inner); @@ -710,7 +710,11 @@ pub(crate) async fn twirp_cache_v2_create( )); } let upload_url = format!("{}/twirp-blob/cache/{token}", runner_base_url()); - info!(token, "cache v2 create entry"); + info!( + key = %storage_key, + version = %version, + "cache v2 create entry" + ); Ok(pb_or_json( &headers, PbCreateCacheEntryResponse { @@ -996,8 +1000,14 @@ mod cache_pb_tests { metadata: Some(PbCacheMetadata { repository_id: 42, scope: vec![ - PbCacheScope { scope: "refs/heads/main".to_string(), permission: 1 }, - PbCacheScope { scope: "refs/heads/feature".to_string(), permission: 2 }, + PbCacheScope { + scope: "refs/heads/main".to_string(), + permission: 1, + }, + PbCacheScope { + scope: "refs/heads/feature".to_string(), + permission: 2, + }, ], }), key: "k".to_string(), @@ -1015,7 +1025,10 @@ mod cache_pb_tests { let fixture = include_bytes!("../../../fixtures/wire/cache-multi-scope.pb"); let (_, _, _, fixture_scopes, _) = pb_cache_request(fixture, CacheRequestKind::GetDownloadUrl).unwrap(); - assert_eq!(fixture_scopes, vec!["refs/heads/main", "refs/heads/feature"]); + assert_eq!( + fixture_scopes, + vec!["refs/heads/main", "refs/heads/feature"] + ); } #[test] diff --git a/crates/preloop-runner-server/src/store.rs b/crates/preloop-runner-server/src/store.rs index 56c86110..87b5ff50 100644 --- a/crates/preloop-runner-server/src/store.rs +++ b/crates/preloop-runner-server/src/store.rs @@ -18,8 +18,8 @@ use async_trait::async_trait; use preloop_gha_protocol::SessionId; use rusqlite::{params, Connection, OptionalExtension, Transaction}; use sha2::Digest; -use std::sync::Mutex as StdMutex; use std::sync::atomic::{AtomicU64, Ordering}; +use std::sync::Mutex as StdMutex; const DATABASE_FILE: &str = "preloop.db"; pub(crate) const SNAPSHOT_FORMAT: u8 = 2; diff --git a/rules/no-sensitive-log-fields.yml b/rules/no-sensitive-log-fields.yml new file mode 100644 index 00000000..8dacbdff --- /dev/null +++ b/rules/no-sensitive-log-fields.yml @@ -0,0 +1,48 @@ +id: no-sensitive-log-fields +message: | + INFO/WARN/ERROR tracing fields must not carry capability material: token, + authorization, cookie, headers, body, payload, or signed_url. These leak + bearer material into journald and OTLP. Use operation/kind/size/result + fields instead. The conformance flow recorder (recording.rs) is exempt. +severity: error +language: rust +# recording.rs deliberately captures every header and body for conformance. +ignores: + - "**/recording.rs" +rule: + any: + # info!(token, …) / warn!(authorization = …, …) + - pattern: info!($$$ARGS, token, $$$REST) + - pattern: warn!($$$ARGS, token, $$$REST) + - pattern: error!($$$ARGS, token, $$$REST) + - pattern: info!(token, $$$REST) + - pattern: warn!(token, $$$REST) + - pattern: error!(token, $$$REST) + - pattern: info!($$$ARGS, authorization, $$$REST) + - pattern: warn!($$$ARGS, authorization, $$$REST) + - pattern: error!($$$ARGS, authorization, $$$REST) + - pattern: info!($$$ARGS, cookie, $$$REST) + - pattern: warn!($$$ARGS, cookie, $$$REST) + - pattern: error!($$$ARGS, cookie, $$$REST) + - pattern: info!($$$ARGS, headers, $$$REST) + - pattern: warn!($$$ARGS, headers, $$$REST) + - pattern: error!($$$ARGS, headers, $$$REST) + - pattern: info!($$$ARGS, signed_url, $$$REST) + - pattern: warn!($$$ARGS, signed_url, $$$REST) + - pattern: error!($$$ARGS, signed_url, $$$REST) + # ?body / body = … inside the macro + - pattern: info!(?body, $$$REST) + - pattern: warn!(?body, $$$REST) + - pattern: error!(?body, $$$REST) + - pattern: info!(body, $$$REST) + - pattern: warn!(body, $$$REST) + - pattern: error!(body, $$$REST) + - pattern: info!(%body, $$$REST) + - pattern: warn!(%body, $$$REST) + - pattern: error!(%body, $$$REST) + - pattern: info!(payload, $$$REST) + - pattern: warn!(payload, $$$REST) + - pattern: error!(payload, $$$REST) + - pattern: info!(?payload, $$$REST) + - pattern: warn!(?payload, $$$REST) + - pattern: error!(?payload, $$$REST) From 1422f164c230f0b712d57215ec5c614e7b5b91ef Mon Sep 17 00:00:00 2001 From: Bnjoroge Date: Thu, 20 Aug 2026 21:29:40 -0400 Subject: [PATCH 2/2] fix(log): catch assigned-form sensitive fields in the log rule MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The rule only matched shorthand tracing fields (info!(token, …)); the assigned form (info!(token = value)) bypassed it entirely. Add assigned variants for token, authorization, cookie, headers, and signed_url across info/warn/error, plus body and payload in plain, ?-debug and %-display forms. Verified with a probe that every form fails the scan. Entire-Checkpoint: 01M0GZ080HJ7TKVVNHNMPSNSEK --- rules/no-sensitive-log-fields.yml | 36 +++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/rules/no-sensitive-log-fields.yml b/rules/no-sensitive-log-fields.yml index 8dacbdff..8c036334 100644 --- a/rules/no-sensitive-log-fields.yml +++ b/rules/no-sensitive-log-fields.yml @@ -30,6 +30,42 @@ rule: - pattern: info!($$$ARGS, signed_url, $$$REST) - pattern: warn!($$$ARGS, signed_url, $$$REST) - pattern: error!($$$ARGS, signed_url, $$$REST) + # Assigned form: info!(token = value, …) — the shorthand patterns above + # do not match it, so a sensitive field could bypass the rule. + - pattern: info!($$$ARGS, token = $$$VALUE, $$$REST) + - pattern: warn!($$$ARGS, token = $$$VALUE, $$$REST) + - pattern: error!($$$ARGS, token = $$$VALUE, $$$REST) + - pattern: info!($$$ARGS, authorization = $$$VALUE, $$$REST) + - pattern: warn!($$$ARGS, authorization = $$$VALUE, $$$REST) + - pattern: error!($$$ARGS, authorization = $$$VALUE, $$$REST) + - pattern: info!($$$ARGS, cookie = $$$VALUE, $$$REST) + - pattern: warn!($$$ARGS, cookie = $$$VALUE, $$$REST) + - pattern: error!($$$ARGS, cookie = $$$VALUE, $$$REST) + - pattern: info!($$$ARGS, headers = $$$VALUE, $$$REST) + - pattern: warn!($$$ARGS, headers = $$$VALUE, $$$REST) + - pattern: error!($$$ARGS, headers = $$$VALUE, $$$REST) + - pattern: info!($$$ARGS, signed_url = $$$VALUE, $$$REST) + - pattern: warn!($$$ARGS, signed_url = $$$VALUE, $$$REST) + - pattern: error!($$$ARGS, signed_url = $$$VALUE, $$$REST) + # Assigned body/payload in plain, ?-debug and %-display forms. + - pattern: info!($$$ARGS, body = $$$VALUE, $$$REST) + - pattern: warn!($$$ARGS, body = $$$VALUE, $$$REST) + - pattern: error!($$$ARGS, body = $$$VALUE, $$$REST) + - pattern: info!($$$ARGS, ?body = $$$VALUE, $$$REST) + - pattern: warn!($$$ARGS, ?body = $$$VALUE, $$$REST) + - pattern: error!($$$ARGS, ?body = $$$VALUE, $$$REST) + - pattern: info!($$$ARGS, %body = $$$VALUE, $$$REST) + - pattern: warn!($$$ARGS, %body = $$$VALUE, $$$REST) + - pattern: error!($$$ARGS, %body = $$$VALUE, $$$REST) + - pattern: info!($$$ARGS, payload = $$$VALUE, $$$REST) + - pattern: warn!($$$ARGS, payload = $$$VALUE, $$$REST) + - pattern: error!($$$ARGS, payload = $$$VALUE, $$$REST) + - pattern: info!($$$ARGS, ?payload = $$$VALUE, $$$REST) + - pattern: warn!($$$ARGS, ?payload = $$$VALUE, $$$REST) + - pattern: error!($$$ARGS, ?payload = $$$VALUE, $$$REST) + - pattern: info!($$$ARGS, %payload = $$$VALUE, $$$REST) + - pattern: warn!($$$ARGS, %payload = $$$VALUE, $$$REST) + - pattern: error!($$$ARGS, %payload = $$$VALUE, $$$REST) # ?body / body = … inside the macro - pattern: info!(?body, $$$REST) - pattern: warn!(?body, $$$REST)