From c75bb60e03d85418d286eee92b272c064f4d8a6f Mon Sep 17 00:00:00 2001 From: ualtinok <94532+ualtinok@users.noreply.github.com> Date: Tue, 27 Oct 2026 23:35:29 +0100 Subject: [PATCH 1/9] mason: refuse worker gh writes with report-to-parent line Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com> (cherry picked from commit bd95058b5db40cd375158c8fadf5d49a187e2060) --- .../next/gh-worker-write-refusal.md | 1 + crates/aft/src/gh_shim.rs | 48 ++- .../aft/src/gh_shim/security_audit_tests.rs | 289 ++++++++++++++++++ 3 files changed, 334 insertions(+), 4 deletions(-) create mode 100644 .alfonso/release-notes/next/gh-worker-write-refusal.md diff --git a/.alfonso/release-notes/next/gh-worker-write-refusal.md b/.alfonso/release-notes/next/gh-worker-write-refusal.md new file mode 100644 index 0000000000..6479537ad0 --- /dev/null +++ b/.alfonso/release-notes/next/gh-worker-write-refusal.md @@ -0,0 +1 @@ +- A GitHub write from a worker shell (for example, `gh issue comment`) is now refused immediately with `gh_shim_worker_write_refused`, telling the agent that workers cannot write to GitHub and to report what should be posted to its parent instead, rather than suggesting a retry that could never succeed. diff --git a/crates/aft/src/gh_shim.rs b/crates/aft/src/gh_shim.rs index f1863b75e0..41cc14bd2d 100644 --- a/crates/aft/src/gh_shim.rs +++ b/crates/aft/src/gh_shim.rs @@ -536,6 +536,12 @@ const REPO_VALUE_FLAGS: &[&str] = &[ const RESERVED_SELF_REPORT: &[&str] = &["--status", "--shim-version"]; const CO_AUTHOR_LINE_REPORT: &str = "--co-author-line"; const GOVERNANCE_UNAVAILABLE_TEXT: &str = "the governance daemon is unreachable and this repository's actions are identity-governed; retry after the daemon returns"; +/// Refusal text for a write-class command from a worker shell. A worker shell +/// carries a read broker, never a write ticket, so no write can ever be +/// spoken from it: the text names the permanent condition and tells the agent +/// where to take the write instead of suggesting a retry. +const WORKER_WRITE_REFUSED_TEXT: &str = + "workers cannot write to GitHub; report what should be posted to your parent instead"; /// Human-readable text for a local discovery-probe deadline expiry. A deadline /// that expires on a loaded host does not mean the daemon is unreachable, so /// this arm must not say "unreachable". The classification stays @@ -555,6 +561,7 @@ const GH_SHIM_STATE_DIR_ENV: &str = "AFT_GH_SHIM_STATE_DIR"; #[derive(Clone, Copy, Debug, Eq, PartialEq)] pub enum RefusalCode { WorkerNativeGh, + WorkerWriteRefused, Unclassified, AdminTier, ManifestBelowFloor, @@ -577,8 +584,9 @@ pub enum RefusalCode { } impl RefusalCode { - pub const ALL: [Self; 20] = [ + pub const ALL: [Self; 21] = [ Self::WorkerNativeGh, + Self::WorkerWriteRefused, Self::Unclassified, Self::AdminTier, Self::ManifestBelowFloor, @@ -603,6 +611,7 @@ impl RefusalCode { pub const fn as_str(self) -> &'static str { match self { Self::WorkerNativeGh => "gh_shim_worker_native_gh", + Self::WorkerWriteRefused => "gh_shim_worker_write_refused", Self::Unclassified => "gh_shim_unclassified", Self::AdminTier => "gh_shim_admin_tier", Self::ManifestBelowFloor => "gh_shim_manifest_below_floor", @@ -788,6 +797,21 @@ fn run_with( ); } + // A worker shell carries a read broker, never a write ticket, so no + // write-class command can be spoken from it. Refuse here, before the + // manifest lookup, the rung probe for governance reachability, and any + // relay traffic, with an + // instruction instead of a retry hint: retrying cannot help, and the old + // governance-unavailable text sent agents back to retry forever. Reads + // (`is_unbound_safe`) fall through to the governed-read path unchanged. + if relay_client::RelayContext::from_process() + .worker_broker + .is_some() + && !is_unbound_safe(args) + { + return refuse(RefusalCode::WorkerWriteRefused, WORKER_WRITE_REFUSED_TEXT); + } + // Presence-based regressed-manifest arm. Decide from the installed artifact // BEFORE any rung probe so a governed refusal never depends on daemon // reachability: a validation failure after a prior valid manifest makes @@ -7827,9 +7851,13 @@ fn exec_real_gh(real_gh: PathBuf, args: &[OsString]) -> i32 { } } -fn refuse(code: RefusalCode, text: &str) -> i32 { +fn refusal_line(code: RefusalCode, text: &str) -> String { let text = text.replace(['\n', '\r'], " "); - eprintln!("gh-shim: {}: {text}", code.as_str()); + format!("gh-shim: {}: {text}", code.as_str()) +} + +fn refuse(code: RefusalCode, text: &str) -> i32 { + eprintln!("{}", refusal_line(code, text)); match code { RefusalCode::OutcomeUnknown => OUTCOME_UNKNOWN_EXIT_STATUS, _ => REFUSAL_EXIT_STATUS, @@ -10826,7 +10854,7 @@ mod tests { #[test] fn refusal_and_self_report_codes_are_separate_closed_sets() { - assert_eq!(RefusalCode::ALL.len(), 20); + assert_eq!(RefusalCode::ALL.len(), 21); assert_eq!( RefusalCode::WorkerNativeGh.as_str(), "gh_shim_worker_native_gh" @@ -10834,10 +10862,22 @@ mod tests { assert!(RefusalCode::ALL .iter() .all(|code| code.as_str().starts_with("gh_shim_"))); + assert_eq!( + RefusalCode::WorkerWriteRefused.as_str(), + "gh_shim_worker_write_refused" + ); assert_eq!( RefusalCode::GovernanceUnavailable.as_str(), "gh_shim_governance_unavailable" ); + assert_eq!( + WORKER_WRITE_REFUSED_TEXT, + "workers cannot write to GitHub; report what should be posted to your parent instead" + ); + assert_eq!( + refusal_line(RefusalCode::WorkerWriteRefused, WORKER_WRITE_REFUSED_TEXT), + "gh-shim: gh_shim_worker_write_refused: workers cannot write to GitHub; report what should be posted to your parent instead" + ); assert_eq!( RefusalCode::OutcomeUnknown.as_str(), "gh_shim_outcome_unknown" diff --git a/crates/aft/src/gh_shim/security_audit_tests.rs b/crates/aft/src/gh_shim/security_audit_tests.rs index 7b2de4567a..73bde38abd 100644 --- a/crates/aft/src/gh_shim/security_audit_tests.rs +++ b/crates/aft/src/gh_shim/security_audit_tests.rs @@ -1000,3 +1000,292 @@ fn audit_head_child_adapters_do_not_inherit_worker_markers() { .iter_full_env_as_str() .all(|(key, _)| key != crate::gh_shim_ticket::GH_SHIM_WORKER_ENV)); } + +/// A worker shell carries a read broker, never a write ticket. Every +/// write-class command from it must be refused with the report-to-parent line +/// and exit 86 before any identity lookup, delegation, or relay traffic. +#[test] +fn audit_worker_write_is_refused_before_any_broker_or_relay_traffic() { + use std::sync::Arc; + let _lock = crate::test_env::process_env_lock(); + let fixture = Fixture::new(20); + // A live relay is configured so that any contact would be observed; the + // worker refusal must happen before the shim reaches it. + let daemon = relay_client::tests::FakeRelayDaemon::spawn(Arc::new(|body| { + let operation = body["op"].as_str().unwrap_or("?"); + if operation == crate::gh_shim_relay::BINDINGS_READ_OPERATION { + return json!({"op": operation, "status": "ok", "data": {"result": { + "repo_binding_generation": 1, + "bindings": [], + }}}); + } + json!({"op": operation, "status": "ok", "data": { + "repo_binding_generation": 1, + "result": {"status": "completed", "result": {"url": "https://github.com/cortexkit/aft/issues/7#issuecomment-1"}}, + }}) + })); + let connection = fixture.home.path().join("connection.json"); + daemon.write_connection_file(&connection); + let config = fixture.home.path().join("cortexkit/aft.jsonc"); + fs::create_dir_all(config.parent().unwrap()).unwrap(); + fs::write( + &config, + json!({"subc":{"connection_file":connection}}).to_string(), + ) + .unwrap(); + // The worker shell: read-broker env present, no ticket. + let _worker = Env::set( + crate::gh_shim_ticket::GH_SHIM_WORKER_ENV, + Some(OsStr::new("1")), + ); + let socket = fixture.home.path().join("broker.sock"); + let capability = fixture.home.path().join("capability"); + fs::write(&capability, "0123456789abcdef0123456789abcdef").unwrap(); + let _socket_env = Env::set( + crate::gh_shim_worker_broker::SOCKET_ENV, + Some(socket.as_os_str()), + ); + let _capability_env = Env::set( + crate::gh_shim_worker_broker::CAPABILITY_FILE_ENV, + Some(capability.as_os_str()), + ); + let _no_ticket = Env::set(crate::gh_shim_ticket::GH_SHIM_TICKET_ENV, None); + let context = relay_client::RelayContext::from_process(); + assert!( + context.worker_broker.is_some(), + "the test did not build a worker relay context" + ); + assert!(context.ticket.is_none()); + assert!(context.connection_file.is_none()); + let mut delegated = Vec::new(); + let mut lookups = 0; + for raw in [ + vec!["issue", "comment", "7", "--body", "hello", "--repo", "cortexkit/aft"], + vec!["pr", "review", "7", "--approve", "--repo", "cortexkit/aft"], + vec!["issue", "edit", "7", "--add-label", "bug", "--repo", "cortexkit/aft"], + vec!["issue", "close", "7", "--reason", "completed", "--repo", "cortexkit/aft"], + vec!["pr", "close", "7", "--repo", "cortexkit/aft"], + vec!["api", "-X", "POST", "repos/cortexkit/aft/issues/7/comments", "-f", "body=hello"], + ] { + let status = run_with( + &args(&raw), + |forwarded| { + delegated.push(forwarded.to_vec()); + 0 + }, + |_, _, _, _| { + lookups += 1; + Ok(None) + }, + ); + assert_eq!(status, 86, "{raw:?}"); + } + assert!( + delegated.is_empty(), + "worker writes must never delegate: {delegated:?}" + ); + assert_eq!(lookups, 0, "worker writes must not reach identity lookup"); + assert!( + daemon.requests.lock().unwrap().is_empty(), + "worker writes must not reach the relay" + ); +} + +/// The refusal above must not open a broker connection either. A bound +/// listener observes the socket: with the refusal first, no handshake arrives. +#[cfg(unix)] +#[test] +fn audit_worker_write_opens_no_broker_connection() { + use std::os::unix::net::UnixListener; + let _lock = crate::test_env::process_env_lock(); + let fixture = Fixture::new(20); + let _worker = Env::set( + crate::gh_shim_ticket::GH_SHIM_WORKER_ENV, + Some(OsStr::new("1")), + ); + let socket = fixture.home.path().join("broker.sock"); + let capability = fixture.home.path().join("capability"); + fs::write(&capability, "0123456789abcdef0123456789abcdef").unwrap(); + let _socket_env = Env::set( + crate::gh_shim_worker_broker::SOCKET_ENV, + Some(socket.as_os_str()), + ); + let _capability_env = Env::set( + crate::gh_shim_worker_broker::CAPABILITY_FILE_ENV, + Some(capability.as_os_str()), + ); + let _no_ticket = Env::set(crate::gh_shim_ticket::GH_SHIM_TICKET_ENV, None); + let listener = UnixListener::bind(&socket).unwrap(); + let status = run_with( + &args(&["issue", "comment", "7", "--body", "hello", "--repo", "cortexkit/aft"]), + |_| panic!("worker write must not delegate"), + |_, _, _, _| panic!("worker write must not reach identity lookup"), + ); + assert_eq!(status, 86); + listener.set_nonblocking(true).unwrap(); + match listener.accept() { + Err(error) if error.kind() == std::io::ErrorKind::WouldBlock => {} + connected => panic!("worker write opened a broker connection: {connected:?}"), + } +} + +/// A worker read still goes to the read broker: the new write-only refusal +/// must not divert it. A minimal fake broker answers the session context and +/// one read; the test asserts the shim spoke to it and the read succeeded. +#[cfg(unix)] +#[test] +fn audit_worker_read_still_reaches_the_broker() { + use std::io::{Read, Write}; + use std::os::unix::net::UnixListener; + let _lock = crate::test_env::process_env_lock(); + let fixture = Fixture::new(20); + let _worker = Env::set( + crate::gh_shim_ticket::GH_SHIM_WORKER_ENV, + Some(OsStr::new("1")), + ); + let socket = fixture.home.path().join("broker.sock"); + let capability = fixture.home.path().join("capability"); + fs::write(&capability, "0123456789abcdef0123456789abcdef").unwrap(); + let _socket_env = Env::set( + crate::gh_shim_worker_broker::SOCKET_ENV, + Some(socket.as_os_str()), + ); + let _capability_env = Env::set( + crate::gh_shim_worker_broker::CAPABILITY_FILE_ENV, + Some(capability.as_os_str()), + ); + let _no_ticket = Env::set(crate::gh_shim_ticket::GH_SHIM_TICKET_ENV, None); + let seen = std::sync::Arc::new(std::sync::Mutex::new(Vec::new())); + let served = seen.clone(); + let server = std::thread::spawn(move || { + let listener = UnixListener::bind(&socket).unwrap(); + loop { + let Ok((mut stream, _)) = listener.accept() else { + break; + }; + let mut size = [0u8; 4]; + if stream.read_exact(&mut size).is_err() { + break; + } + let mut body = vec![0u8; u32::from_be_bytes(size) as usize]; + if stream.read_exact(&mut body).is_err() { + break; + } + let request: Value = serde_json::from_slice(&body).unwrap(); + let operation = request["operation"]["op"].as_str().unwrap().to_string(); + let data = match operation.as_str() { + "gh_shim.session_context" => { + json!({"worker": true, "head": false, "repository": "cortexkit/aft"}) + } + "gh_shim.read_request" => json!({ + "result": {"status": 200, "body": {"number": 42}, "next_page": null}, + "isError": false, + }), + unexpected => panic!("worker read sent an unexpected broker op: {unexpected}"), + }; + let reply = serde_json::to_vec(&json!({"status": "ok", "data": data})).unwrap(); + stream.write_all(&(reply.len() as u32).to_be_bytes()).unwrap(); + stream.write_all(&reply).unwrap(); + served.lock().unwrap().push(operation.clone()); + if operation == "gh_shim.read_request" { + break; + } + } + }); + let mut delegated = Vec::new(); + let status = run_with( + &args(&["pr", "view", "42", "--json", "number", "--repo", "cortexkit/aft"]), + |forwarded| { + delegated.push(forwarded.to_vec()); + 23 + }, + worker_reads::identity_for_command, + ); + assert_eq!(status, 0); + assert!( + delegated.is_empty(), + "a worker read must be brokered, not delegated: {delegated:?}" + ); + let seen = seen.lock().unwrap().clone(); + assert_eq!( + seen, + vec!["gh_shim.session_context", "gh_shim.session_context", "gh_shim.read_request"], + "a worker read must authenticate its session and read through the broker" + ); + server.join().unwrap(); +} + +/// A head-session write keeps its existing route: the worker-only refusal +/// must not catch it. The fake relay answers the bindings check and +/// completes the write, so the command succeeds without delegation. +#[test] +fn audit_head_write_still_routes_through_the_relay() { + use std::sync::Arc; + let _lock = crate::test_env::process_env_lock(); + let fixture = Fixture::new(20); + let manifest: Manifest = serde_json::from_str(include_str!( + "../../tests/fixtures/gh_shim/v20-manifest.json" + )) + .unwrap(); + let bindings: Vec = manifest + .bindings + .iter() + .map(|(repo, agent)| { + json!({"repository": repo, "agent_id": agent, "app_handle_id": "test"}) + }) + .collect(); + let daemon = relay_client::tests::FakeRelayDaemon::spawn(Arc::new(move |body| { + let operation = body["op"].as_str().unwrap_or("?"); + if operation == crate::gh_shim_relay::BINDINGS_READ_OPERATION { + return json!({"op": operation, "status": "ok", "data": {"result": { + "repo_binding_generation": 1, + "bindings": bindings, + }}}); + } + assert_eq!(operation, crate::gh_shim_relay::BOT_REQUEST_OPERATION); + json!({"op": operation, "status": "ok", "data": { + "repo_binding_generation": 1, + "result": {"status": "completed", "result": {"url": "https://github.com/cortexkit/aft/issues/7#issuecomment-1", "id": 1}}, + }}) + })); + let connection = fixture.home.path().join("connection.json"); + daemon.write_connection_file(&connection); + let config = fixture.home.path().join("cortexkit/aft.jsonc"); + fs::create_dir_all(config.parent().unwrap()).unwrap(); + fs::write( + &config, + json!({"subc":{"connection_file":connection}}).to_string(), + ) + .unwrap(); + let _no_worker = Env::set(crate::gh_shim_ticket::GH_SHIM_WORKER_ENV, None); + let _no_socket = Env::set(crate::gh_shim_worker_broker::SOCKET_ENV, None); + let _no_capability = Env::set(crate::gh_shim_worker_broker::CAPABILITY_FILE_ENV, None); + let _ticket = Env::set( + crate::gh_shim_ticket::GH_SHIM_TICKET_ENV, + Some(OsStr::new("head-ticket")), + ); + let argv = args(&["issue", "comment", "7", "--body", "hello", "--repo", "cortexkit/aft"]); + let mut delegated = Vec::new(); + let status = run_with( + &argv, + |forwarded| { + delegated.push(forwarded.to_vec()); + 23 + }, + worker_reads::identity_for_command, + ); + assert_eq!(status, 0); + assert!( + delegated.is_empty(), + "a head write must route, not delegate: {delegated:?}" + ); + assert!( + daemon + .requests + .lock() + .unwrap() + .iter() + .any(|request| request["op"] == crate::gh_shim_relay::BOT_REQUEST_OPERATION), + "a head write must still reach the relay" + ); +} From bb0546dd2159773482fc89d455d4fa25cef69f5a Mon Sep 17 00:00:00 2001 From: ualtinok <94532+ualtinok@users.noreply.github.com> Date: Tue, 27 Oct 2026 23:42:56 +0100 Subject: [PATCH 2/9] mason: cargo fmt new worker refusal tests Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com> (cherry picked from commit bfc3c8cd7001d84ab0eb298bdf849ca0a165bb4e) --- .../aft/src/gh_shim/security_audit_tests.rs | 85 ++++++++++++++++--- 1 file changed, 73 insertions(+), 12 deletions(-) diff --git a/crates/aft/src/gh_shim/security_audit_tests.rs b/crates/aft/src/gh_shim/security_audit_tests.rs index 73bde38abd..34e32d0d19 100644 --- a/crates/aft/src/gh_shim/security_audit_tests.rs +++ b/crates/aft/src/gh_shim/security_audit_tests.rs @@ -1060,12 +1060,43 @@ fn audit_worker_write_is_refused_before_any_broker_or_relay_traffic() { let mut delegated = Vec::new(); let mut lookups = 0; for raw in [ - vec!["issue", "comment", "7", "--body", "hello", "--repo", "cortexkit/aft"], + vec![ + "issue", + "comment", + "7", + "--body", + "hello", + "--repo", + "cortexkit/aft", + ], vec!["pr", "review", "7", "--approve", "--repo", "cortexkit/aft"], - vec!["issue", "edit", "7", "--add-label", "bug", "--repo", "cortexkit/aft"], - vec!["issue", "close", "7", "--reason", "completed", "--repo", "cortexkit/aft"], + vec![ + "issue", + "edit", + "7", + "--add-label", + "bug", + "--repo", + "cortexkit/aft", + ], + vec![ + "issue", + "close", + "7", + "--reason", + "completed", + "--repo", + "cortexkit/aft", + ], vec!["pr", "close", "7", "--repo", "cortexkit/aft"], - vec!["api", "-X", "POST", "repos/cortexkit/aft/issues/7/comments", "-f", "body=hello"], + vec![ + "api", + "-X", + "POST", + "repos/cortexkit/aft/issues/7/comments", + "-f", + "body=hello", + ], ] { let status = run_with( &args(&raw), @@ -1117,7 +1148,15 @@ fn audit_worker_write_opens_no_broker_connection() { let _no_ticket = Env::set(crate::gh_shim_ticket::GH_SHIM_TICKET_ENV, None); let listener = UnixListener::bind(&socket).unwrap(); let status = run_with( - &args(&["issue", "comment", "7", "--body", "hello", "--repo", "cortexkit/aft"]), + &args(&[ + "issue", + "comment", + "7", + "--body", + "hello", + "--repo", + "cortexkit/aft", + ]), |_| panic!("worker write must not delegate"), |_, _, _, _| panic!("worker write must not reach identity lookup"), ); @@ -1184,7 +1223,9 @@ fn audit_worker_read_still_reaches_the_broker() { unexpected => panic!("worker read sent an unexpected broker op: {unexpected}"), }; let reply = serde_json::to_vec(&json!({"status": "ok", "data": data})).unwrap(); - stream.write_all(&(reply.len() as u32).to_be_bytes()).unwrap(); + stream + .write_all(&(reply.len() as u32).to_be_bytes()) + .unwrap(); stream.write_all(&reply).unwrap(); served.lock().unwrap().push(operation.clone()); if operation == "gh_shim.read_request" { @@ -1194,7 +1235,15 @@ fn audit_worker_read_still_reaches_the_broker() { }); let mut delegated = Vec::new(); let status = run_with( - &args(&["pr", "view", "42", "--json", "number", "--repo", "cortexkit/aft"]), + &args(&[ + "pr", + "view", + "42", + "--json", + "number", + "--repo", + "cortexkit/aft", + ]), |forwarded| { delegated.push(forwarded.to_vec()); 23 @@ -1209,7 +1258,11 @@ fn audit_worker_read_still_reaches_the_broker() { let seen = seen.lock().unwrap().clone(); assert_eq!( seen, - vec!["gh_shim.session_context", "gh_shim.session_context", "gh_shim.read_request"], + vec![ + "gh_shim.session_context", + "gh_shim.session_context", + "gh_shim.read_request" + ], "a worker read must authenticate its session and read through the broker" ); server.join().unwrap(); @@ -1230,9 +1283,9 @@ fn audit_head_write_still_routes_through_the_relay() { let bindings: Vec = manifest .bindings .iter() - .map(|(repo, agent)| { - json!({"repository": repo, "agent_id": agent, "app_handle_id": "test"}) - }) + .map( + |(repo, agent)| json!({"repository": repo, "agent_id": agent, "app_handle_id": "test"}), + ) .collect(); let daemon = relay_client::tests::FakeRelayDaemon::spawn(Arc::new(move |body| { let operation = body["op"].as_str().unwrap_or("?"); @@ -1264,7 +1317,15 @@ fn audit_head_write_still_routes_through_the_relay() { crate::gh_shim_ticket::GH_SHIM_TICKET_ENV, Some(OsStr::new("head-ticket")), ); - let argv = args(&["issue", "comment", "7", "--body", "hello", "--repo", "cortexkit/aft"]); + let argv = args(&[ + "issue", + "comment", + "7", + "--body", + "hello", + "--repo", + "cortexkit/aft", + ]); let mut delegated = Vec::new(); let status = run_with( &argv, From 3a4652fe11bbc474b1d8827b30cfdfed97790def Mon Sep 17 00:00:00 2001 From: ualtinok <94532+ualtinok@users.noreply.github.com> Date: Wed, 28 Oct 2026 00:00:08 +0100 Subject: [PATCH 3/9] mason: worker credential commands keep credential refusal; refusal sink for tests Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com> (cherry picked from commit 5cc3b1815a20bfe70881531873d40667d269ac05) --- crates/aft/src/gh_shim.rs | 57 +++++++++++++-- .../aft/src/gh_shim/security_audit_tests.rs | 70 +++++++++++++++++++ 2 files changed, 122 insertions(+), 5 deletions(-) diff --git a/crates/aft/src/gh_shim.rs b/crates/aft/src/gh_shim.rs index 41cc14bd2d..c9f28f46d5 100644 --- a/crates/aft/src/gh_shim.rs +++ b/crates/aft/src/gh_shim.rs @@ -800,13 +800,17 @@ fn run_with( // A worker shell carries a read broker, never a write ticket, so no // write-class command can be spoken from it. Refuse here, before the // manifest lookup, the rung probe for governance reachability, and any - // relay traffic, with an - // instruction instead of a retry hint: retrying cannot help, and the old - // governance-unavailable text sent agents back to retry forever. Reads - // (`is_unbound_safe`) fall through to the governed-read path unchanged. + // relay traffic, with an instruction instead of a retry hint: retrying + // cannot help, and the old governance-unavailable text sent agents back + // to retry forever. Credential commands (`gh auth token`, token-revealing + // status flags, login/logout and friends) are not writes: they are + // excluded here so they keep their existing operator-credentials refusal + // below. Reads (`is_unbound_safe`) fall through to the governed-read path + // unchanged. if relay_client::RelayContext::from_process() .worker_broker .is_some() + && operator_credential_use(args).is_none() && !is_unbound_safe(args) { return refuse(RefusalCode::WorkerWriteRefused, WORKER_WRITE_REFUSED_TEXT); @@ -7856,8 +7860,51 @@ fn refusal_line(code: RefusalCode, text: &str) -> String { format!("gh-shim: {}: {text}", code.as_str()) } +/// Test seam: when a test installs a refusal sink, `refuse` records the +/// rendered line there instead of writing to stderr, so tests can assert the +/// exact refusal the agent would see without stealing the process's shared +/// file descriptor 2 (which parallel tests also use). The sink is +/// thread-local, so parallel tests never see each other's refusals. +#[cfg(test)] +thread_local! { + static REFUSAL_SINK: std::cell::RefCell< + Option>>>, + > = const { std::cell::RefCell::new(None) }; +} + +/// Holds an installed refusal sink; dropping restores plain stderr output. +#[cfg(test)] +pub(crate) struct RefusalSinkGuard; + +#[cfg(test)] +impl Drop for RefusalSinkGuard { + fn drop(&mut self) { + REFUSAL_SINK.with(|slot| slot.replace(None)); + } +} + +/// Records every `refuse` line until the returned guard drops. +#[cfg(test)] +pub(crate) fn capture_refusals_for_test() -> ( + RefusalSinkGuard, + std::sync::Arc>>, +) { + let lines = std::sync::Arc::new(std::sync::Mutex::new(Vec::new())); + REFUSAL_SINK.with(|slot| slot.replace(Some(lines.clone()))); + (RefusalSinkGuard, lines) +} + +fn emit_refusal_line(line: &str) { + #[cfg(test)] + if let Some(sink) = REFUSAL_SINK.with(|slot| slot.borrow().clone()) { + sink.lock().unwrap().push(line.to_string()); + return; + } + eprintln!("{line}"); +} + fn refuse(code: RefusalCode, text: &str) -> i32 { - eprintln!("{}", refusal_line(code, text)); + emit_refusal_line(&refusal_line(code, text)); match code { RefusalCode::OutcomeUnknown => OUTCOME_UNKNOWN_EXIT_STATUS, _ => REFUSAL_EXIT_STATUS, diff --git a/crates/aft/src/gh_shim/security_audit_tests.rs b/crates/aft/src/gh_shim/security_audit_tests.rs index 34e32d0d19..869e3c8a5c 100644 --- a/crates/aft/src/gh_shim/security_audit_tests.rs +++ b/crates/aft/src/gh_shim/security_audit_tests.rs @@ -1050,6 +1050,7 @@ fn audit_worker_write_is_refused_before_any_broker_or_relay_traffic() { Some(capability.as_os_str()), ); let _no_ticket = Env::set(crate::gh_shim_ticket::GH_SHIM_TICKET_ENV, None); + let (_sink, lines) = capture_refusals_for_test(); let context = relay_client::RelayContext::from_process(); assert!( context.worker_broker.is_some(), @@ -1120,6 +1121,20 @@ fn audit_worker_write_is_refused_before_any_broker_or_relay_traffic() { daemon.requests.lock().unwrap().is_empty(), "worker writes must not reach the relay" ); + // The refusal the agent actually sees: the exact report-to-parent line, + // once per refused command. + let lines = lines.lock().unwrap(); + assert_eq!( + lines.len(), + 6, + "every worker write must refuse loudly: {lines:?}" + ); + for line in lines.iter() { + assert_eq!( + line, + "gh-shim: gh_shim_worker_write_refused: workers cannot write to GitHub; report what should be posted to your parent instead" + ); + } } /// The refusal above must not open a broker connection either. A bound @@ -1146,6 +1161,7 @@ fn audit_worker_write_opens_no_broker_connection() { Some(capability.as_os_str()), ); let _no_ticket = Env::set(crate::gh_shim_ticket::GH_SHIM_TICKET_ENV, None); + let (_sink, lines) = capture_refusals_for_test(); let listener = UnixListener::bind(&socket).unwrap(); let status = run_with( &args(&[ @@ -1161,6 +1177,10 @@ fn audit_worker_write_opens_no_broker_connection() { |_, _, _, _| panic!("worker write must not reach identity lookup"), ); assert_eq!(status, 86); + assert_eq!( + lines.lock().unwrap().as_slice(), + ["gh-shim: gh_shim_worker_write_refused: workers cannot write to GitHub; report what should be posted to your parent instead"], + ); listener.set_nonblocking(true).unwrap(); match listener.accept() { Err(error) if error.kind() == std::io::ErrorKind::WouldBlock => {} @@ -1194,6 +1214,7 @@ fn audit_worker_read_still_reaches_the_broker() { Some(capability.as_os_str()), ); let _no_ticket = Env::set(crate::gh_shim_ticket::GH_SHIM_TICKET_ENV, None); + let (_sink, refusals) = capture_refusals_for_test(); let seen = std::sync::Arc::new(std::sync::Mutex::new(Vec::new())); let served = seen.clone(); let server = std::thread::spawn(move || { @@ -1251,6 +1272,10 @@ fn audit_worker_read_still_reaches_the_broker() { worker_reads::identity_for_command, ); assert_eq!(status, 0); + assert!( + refusals.lock().unwrap().is_empty(), + "a successful worker read must refuse nothing" + ); assert!( delegated.is_empty(), "a worker read must be brokered, not delegated: {delegated:?}" @@ -1327,6 +1352,7 @@ fn audit_head_write_still_routes_through_the_relay() { "cortexkit/aft", ]); let mut delegated = Vec::new(); + let (_sink, refusals) = capture_refusals_for_test(); let status = run_with( &argv, |forwarded| { @@ -1336,6 +1362,10 @@ fn audit_head_write_still_routes_through_the_relay() { worker_reads::identity_for_command, ); assert_eq!(status, 0); + assert!( + refusals.lock().unwrap().is_empty(), + "a routed head write must refuse nothing" + ); assert!( delegated.is_empty(), "a head write must route, not delegate: {delegated:?}" @@ -1350,3 +1380,43 @@ fn audit_head_write_still_routes_through_the_relay() { "a head write must still reach the relay" ); } + +/// Credential commands are not writes: `gh auth token`, a token-revealing +/// status flag, and `gh auth login` from a worker shell must keep the +/// operator-credentials refusal (exit 86), never the worker-write line. +#[test] +fn audit_worker_credential_command_keeps_the_credential_refusal() { + let _lock = crate::test_env::process_env_lock(); + let fixture = Fixture::new(20); + let _worker = Env::set( + crate::gh_shim_ticket::GH_SHIM_WORKER_ENV, + Some(OsStr::new("1")), + ); + let _no_ticket = Env::set(crate::gh_shim_ticket::GH_SHIM_TICKET_ENV, None); + let _no_bypass = Env::set("GH_SHIM_BYPASS", None); + let (_sink, lines) = capture_refusals_for_test(); + for raw in [ + vec!["auth", "token"], + vec!["auth", "status", "--show-token"], + vec!["auth", "login"], + ] { + let status = run_with( + &args(&raw), + |_| panic!("a credential refusal must not delegate: {raw:?}"), + |_, _, _, _| panic!("a credential refusal must not reach identity lookup: {raw:?}"), + ); + assert_eq!(status, 86, "{raw:?}"); + } + let lines = lines.lock().unwrap(); + assert_eq!( + lines.len(), + 3, + "every credential command must refuse loudly: {lines:?}" + ); + for line in lines.iter() { + assert!( + line.starts_with("gh-shim: gh_shim_operator_credentials: "), + "a worker credential command must keep the credential refusal code: {line}" + ); + } +} From 319b1bc409d05aea42dd5ffbf09c9b2b3c3c141c Mon Sep 17 00:00:00 2001 From: ualtinok <94532+ualtinok@users.noreply.github.com> Date: Wed, 28 Oct 2026 00:09:31 +0100 Subject: [PATCH 4/9] mason: credential test tolerates pre-arm identity check Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com> (cherry picked from commit 282847dbf9ab0ade4186d38a096ce632ca911f65) --- crates/aft/src/gh_shim/security_audit_tests.rs | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/crates/aft/src/gh_shim/security_audit_tests.rs b/crates/aft/src/gh_shim/security_audit_tests.rs index 869e3c8a5c..ffd2727d62 100644 --- a/crates/aft/src/gh_shim/security_audit_tests.rs +++ b/crates/aft/src/gh_shim/security_audit_tests.rs @@ -1395,6 +1395,7 @@ fn audit_worker_credential_command_keeps_the_credential_refusal() { let _no_ticket = Env::set(crate::gh_shim_ticket::GH_SHIM_TICKET_ENV, None); let _no_bypass = Env::set("GH_SHIM_BYPASS", None); let (_sink, lines) = capture_refusals_for_test(); + let mut lookups = 0; for raw in [ vec!["auth", "token"], vec!["auth", "status", "--show-token"], @@ -1403,10 +1404,20 @@ fn audit_worker_credential_command_keeps_the_credential_refusal() { let status = run_with( &args(&raw), |_| panic!("a credential refusal must not delegate: {raw:?}"), - |_, _, _, _| panic!("a credential refusal must not reach identity lookup: {raw:?}"), + // The read-identity check runs before the credential arm and + // declines credential shapes without any lookup; it must still + // never report a worker session for them. + |_, _, _, _| { + lookups += 1; + Ok(None) + }, ); assert_eq!(status, 86, "{raw:?}"); } + assert_eq!( + lookups, 3, + "each credential shape consults read identity once" + ); let lines = lines.lock().unwrap(); assert_eq!( lines.len(), From 07d6c0e4aac70623865dcfbca214cc742a6a7b54 Mon Sep 17 00:00:00 2001 From: ualtinok <94532+ualtinok@users.noreply.github.com> Date: Wed, 28 Oct 2026 00:24:29 +0100 Subject: [PATCH 5/9] mason: fix deny-warnings breaks: doc comment, unused fixture Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com> (cherry picked from commit 97e5251a0a4fb29131b3fc4a74c61bc99a964bce) --- crates/aft/src/gh_shim.rs | 11 ++++++----- crates/aft/src/gh_shim/security_audit_tests.rs | 4 +++- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/crates/aft/src/gh_shim.rs b/crates/aft/src/gh_shim.rs index c9f28f46d5..f7274558bf 100644 --- a/crates/aft/src/gh_shim.rs +++ b/crates/aft/src/gh_shim.rs @@ -7860,11 +7860,12 @@ fn refusal_line(code: RefusalCode, text: &str) -> String { format!("gh-shim: {}: {text}", code.as_str()) } -/// Test seam: when a test installs a refusal sink, `refuse` records the -/// rendered line there instead of writing to stderr, so tests can assert the -/// exact refusal the agent would see without stealing the process's shared -/// file descriptor 2 (which parallel tests also use). The sink is -/// thread-local, so parallel tests never see each other's refusals. +// Test seam: when a test installs a refusal sink, `refuse` records the +// rendered line there instead of writing to stderr, so tests can assert the +// exact refusal the agent would see without stealing the process's shared +// file descriptor 2 (which parallel tests also use). The sink is +// thread-local, so parallel tests never see each other's refusals. +// (Plain comments: doc comments do not attach to a `thread_local!` block.) #[cfg(test)] thread_local! { static REFUSAL_SINK: std::cell::RefCell< diff --git a/crates/aft/src/gh_shim/security_audit_tests.rs b/crates/aft/src/gh_shim/security_audit_tests.rs index ffd2727d62..a0fbc29beb 100644 --- a/crates/aft/src/gh_shim/security_audit_tests.rs +++ b/crates/aft/src/gh_shim/security_audit_tests.rs @@ -1387,7 +1387,9 @@ fn audit_head_write_still_routes_through_the_relay() { #[test] fn audit_worker_credential_command_keeps_the_credential_refusal() { let _lock = crate::test_env::process_env_lock(); - let fixture = Fixture::new(20); + // Held for its environment and installed manifest, not its paths: the + // credential arm needs an active manifest, nothing else from the fixture. + let _fixture = Fixture::new(20); let _worker = Env::set( crate::gh_shim_ticket::GH_SHIM_WORKER_ENV, Some(OsStr::new("1")), From 223ecf6cd9a4eabcf1a51b65239b575417858a6f Mon Sep 17 00:00:00 2001 From: ualtinok <94532+ualtinok@users.noreply.github.com> Date: Wed, 28 Oct 2026 00:39:17 +0100 Subject: [PATCH 6/9] Search-quality descriptor for the worker write refusal Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com> --- .../land-gh-worker-write-refusal.json | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 benchmarks/aft-search/slice-descriptors/land-gh-worker-write-refusal.json diff --git a/benchmarks/aft-search/slice-descriptors/land-gh-worker-write-refusal.json b/benchmarks/aft-search/slice-descriptors/land-gh-worker-write-refusal.json new file mode 100644 index 0000000000..45ae31a5e2 --- /dev/null +++ b/benchmarks/aft-search/slice-descriptors/land-gh-worker-write-refusal.json @@ -0,0 +1,12 @@ +{ + "slice_class": "non_ranking", + "targeted_mechanism": "none", + "kind": "harness", + "fixtures": [ + "harness-goldens" + ], + "slices": [ + "gh-worker-write-refusal" + ], + "note": "gh-shim refuses GitHub writes from worker shells with a report-to-parent instruction. No changed path matches RANKING_FENCE_PREFIXES." +} From ca44ffad753d37bd296966f69568f10435854a71 Mon Sep 17 00:00:00 2001 From: ualtinok <94532+ualtinok@users.noreply.github.com> Date: Wed, 28 Oct 2026 01:17:19 +0100 Subject: [PATCH 7/9] mason: head-write refusal assertion names captured lines Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com> --- crates/aft/src/gh_shim/security_audit_tests.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/crates/aft/src/gh_shim/security_audit_tests.rs b/crates/aft/src/gh_shim/security_audit_tests.rs index a0fbc29beb..408245b315 100644 --- a/crates/aft/src/gh_shim/security_audit_tests.rs +++ b/crates/aft/src/gh_shim/security_audit_tests.rs @@ -1361,7 +1361,12 @@ fn audit_head_write_still_routes_through_the_relay() { }, worker_reads::identity_for_command, ); - assert_eq!(status, 0); + assert_eq!( + status, + 0, + "a head write was refused instead of routed: {:?}", + refusals.lock().unwrap() + ); assert!( refusals.lock().unwrap().is_empty(), "a routed head write must refuse nothing" From e7dde156180f18ae9aa08c32cc4a47e656e38314 Mon Sep 17 00:00:00 2001 From: ualtinok <94532+ualtinok@users.noreply.github.com> Date: Wed, 28 Oct 2026 01:30:29 +0100 Subject: [PATCH 8/9] mason: seed cached R3 rung in head-write relay test Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com> --- crates/aft/src/gh_shim/security_audit_tests.rs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/crates/aft/src/gh_shim/security_audit_tests.rs b/crates/aft/src/gh_shim/security_audit_tests.rs index 408245b315..6b17352069 100644 --- a/crates/aft/src/gh_shim/security_audit_tests.rs +++ b/crates/aft/src/gh_shim/security_audit_tests.rs @@ -1353,6 +1353,22 @@ fn audit_head_write_still_routes_through_the_relay() { ]); let mut delegated = Vec::new(); let (_sink, refusals) = capture_refusals_for_test(); + // Seed a fresh cached R3 rung with the same provenance the shim computes, + // so the test never depends on live rung discovery timing: on a loaded + // runner the daemon probe can overrun the discovery budget (the test then + // takes ~2 s and the write refuses as governance-unavailable, as seen in + // Windows CI), while a cached fresh R3 record is reused with no probe at + // all. The write still travels the live relay below, so the test keeps + // proving the worker-only gate lets a head write through to the relay. + let now = unix_seconds(); + let provenance = RungRecordProvenance::for_target( + &TargetRepository::from_invocation(&argv), + &std::env::current_dir().unwrap(), + ); + write_rung_record_silently( + &fixture.paths, + &RungDetermination::r3(now, manifest.manifest_version, &provenance).record, + ); let status = run_with( &argv, |forwarded| { From 9a128a6688795548c0954990c364dc69d9d1904a Mon Sep 17 00:00:00 2001 From: ualtinok <94532+ualtinok@users.noreply.github.com> Date: Wed, 28 Oct 2026 01:56:35 +0100 Subject: [PATCH 9/9] mason: declare exercised tuple for running platform in head-write test Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com> --- .../aft/src/gh_shim/security_audit_tests.rs | 27 ++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/crates/aft/src/gh_shim/security_audit_tests.rs b/crates/aft/src/gh_shim/security_audit_tests.rs index 6b17352069..77466f165f 100644 --- a/crates/aft/src/gh_shim/security_audit_tests.rs +++ b/crates/aft/src/gh_shim/security_audit_tests.rs @@ -1301,10 +1301,35 @@ fn audit_head_write_still_routes_through_the_relay() { use std::sync::Arc; let _lock = crate::test_env::process_env_lock(); let fixture = Fixture::new(20); - let manifest: Manifest = serde_json::from_str(include_str!( + let mut manifest_value: Value = serde_json::from_str(include_str!( "../../tests/fixtures/gh_shim/v20-manifest.json" )) .unwrap(); + // The fixture manifests review only macOS and Linux, but the shim + // classifies with current_platform(): anywhere else the exercised tuple + // would read as undeclared and the write could never route. Extend the + // exercised tuple to the running platform so run_with can reach the relay + // on every platform this test runs on. Nothing else about the fixture + // changes. + if let Some(entries) = manifest_value + .get_mut("tiers") + .and_then(|tiers| tiers.get_mut("governed")) + .and_then(Value::as_array_mut) + { + for entry in entries { + if entry.get("tuple").and_then(Value::as_str) != Some("issue comment") { + continue; + } + if let Some(platforms) = entry.get_mut("platform").and_then(Value::as_array_mut) { + let current = Value::String(current_platform().to_string()); + if !platforms.contains(¤t) { + platforms.push(current); + } + } + } + } + let manifest: Manifest = serde_json::from_value(manifest_value).unwrap(); + tests::write_signed_manifest(&fixture.paths, manifest.clone(), unix_seconds()); let bindings: Vec = manifest .bindings .iter()