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..8c036334 --- /dev/null +++ b/rules/no-sensitive-log-fields.yml @@ -0,0 +1,84 @@ +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) + # 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) + - 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)