From f37d89b5844a50d79726788989b1a5b27aa157e5 Mon Sep 17 00:00:00 2001 From: Davanum Srinivas Date: Sun, 27 Sep 2026 17:49:05 +0000 Subject: [PATCH 1/3] fix(supervisor-network): keep workload bytes read with a mediated CONNECT header (#3745) The proxy read the synthesized CONNECT header of a mediated open with a full-width read into an 8192-byte buffer through a BufReader of the same size, so the workload's request could arrive in the same read. The CONNECT path never looked past the header, and the request was lost until the workload timed out. Take the header out of the BufReader with fill_buf and consume only through the terminator, so the bytes behind it stay buffered for the relay. Signed-off-by: Davanum Srinivas --- .../openshell-supervisor-network/src/proxy.rs | 160 +++++++++++++++++- 1 file changed, 155 insertions(+), 5 deletions(-) diff --git a/crates/openshell-supervisor-network/src/proxy.rs b/crates/openshell-supervisor-network/src/proxy.rs index 5f224b474e..6ac6628cc4 100644 --- a/crates/openshell-supervisor-network/src/proxy.rs +++ b/crates/openshell-supervisor-network/src/proxy.rs @@ -2455,13 +2455,24 @@ async fn handle_mediated_connection( return Ok(()); } - let n = client.read(&mut buf[used..]).await.into_diagnostic()?; - if n == 0 { + // A mediated open's first workload bytes follow the synthesized CONNECT header. + // Take only the header out of the reader so the bytes behind it stay buffered for + // the relay; overlap three bytes so a terminator split across fills is found. + let available = client.fill_buf().await.into_diagnostic()?; + if available.is_empty() { return Ok(()); } - used += n; - - if buf[..used].windows(4).any(|win| win == b"\r\n\r\n") { + let n = available.len().min(buf.len() - used); + buf[used..used + n].copy_from_slice(&available[..n]); + let start = used.saturating_sub(3); + let end = buf[start..used + n] + .windows(4) + .position(|window| window == b"\r\n\r\n") + .map(|position| start + position + 4); + let consumed = end.map_or(n, |end| end - used); + client.consume(consumed); + used += consumed; + if end.is_some() { break; } } @@ -7660,6 +7671,145 @@ process: { run_as_user: sandbox, run_as_group: sandbox } assert!(response.contains("\"format\":\"yaml\""), "{response}"); } + #[tokio::test] + async fn mediated_connect_keeps_workload_bytes_read_with_the_synthesized_header() { + let Some(upstream_ip) = non_loopback_test_ipv4() else { + eprintln!("skipping: no routable non-loopback IPv4 test address"); + return; + }; + let upstream = TcpListener::bind((upstream_ip, 0)).await.unwrap(); + let destination = upstream.local_addr().unwrap(); + let (request_tx, request_rx) = tokio::sync::oneshot::channel(); + tokio::spawn(async move { + let (mut stream, _) = upstream.accept().await.unwrap(); + // Report whatever arrives before the request terminator or EOF, so a + // swallowed request shows up as an empty read, not as a hang. + let mut request = Vec::new(); + loop { + let mut byte = [0_u8; 1]; + match stream.read(&mut byte).await { + Ok(1) => request.push(byte[0]), + _ => break, + } + if request.ends_with(b"\r\n\r\n") { + break; + } + } + request_tx.send(request).unwrap(); + stream + .write_all(b"HTTP/1.1 204 No Content\r\nConnection: close\r\n\r\n") + .await + .unwrap(); + }); + let engine = Arc::new( + OpaEngine::from_strings( + include_str!("../data/sandbox-policy.rego"), + &format!( + r#" +network_policies: + allowed: + name: allowed + endpoints: + - host: "{upstream_ip}" + port: {port} + tls: skip + binaries: + - path: /usr/bin/curl +"#, + port = destination.port() + ), + ) + .unwrap(), + ); + let cache = Arc::new(BinaryIdentityCache::new()); + let (stream, mut workload) = tokio::io::duplex(4096); + let (decision, completion) = tokio::sync::oneshot::channel(); + let pending = PendingTcpOpen { + stream: Box::new(stream), + binary_identity: Ok(ContractBinaryIdentity { + executable: ContractExecutableIdentity { + path: PathBuf::from("/usr/bin/curl"), + digest: Some("44".repeat(32).parse().unwrap()), + }, + ancestors: Vec::new(), + cmdline_paths: Vec::new(), + }), + destination, + socket: openshell_isolation_interface::contract::NetworkSocketMetadata { + socket_cookie: 7, + nonblocking: false, + process_generation: 1, + }, + policy_generation: engine.current_generation(), + timing: MediationTiming::default(), + decision, + }; + let (stream, supplied_identity, socket_addrs, transparent) = + preauthorize_transparent_open(pending, None, &engine, &cache, None, None, false, None) + .await + .expect("open is admitted"); + assert_eq!(completion.await.unwrap(), TcpOpenDecision::RelayReady); + // The workload writes before the proxy reads anything, so its request sits + // right behind the synthesized CONNECT header in the proxy's first read. + workload + .write_all( + format!("GET /ping HTTP/1.1\r\nHost: {destination}\r\nConnection: close\r\n\r\n") + .as_bytes(), + ) + .await + .unwrap(); + workload.shutdown().await.unwrap(); + let handler = tokio::spawn(handle_mediated_connection( + tokio::io::BufReader::new(stream), + supplied_identity, + socket_addrs, + transparent, + None, + engine, + cache, + Arc::new(AtomicU32::new(0)), + None, + None, + AgentProposals::new(true), + Arc::new(None), + Arc::new(None), + Arc::new(None), + None, + None, + None, + None, + None, + None, + )); + let request = tokio::time::timeout(std::time::Duration::from_secs(5), request_rx) + .await + .expect("upstream never received the workload's request") + .unwrap(); + assert!( + request.starts_with(b"GET /ping HTTP/1.1\r\n"), + "upstream received {:?} instead of the workload's request", + String::from_utf8_lossy(&request) + ); + let mut response = Vec::new(); + tokio::time::timeout( + std::time::Duration::from_secs(5), + workload.read_to_end(&mut response), + ) + .await + .expect("response timed out") + .unwrap(); + assert!( + response.starts_with(b"HTTP/1.1 204 "), + "{}", + String::from_utf8_lossy(&response) + ); + tokio::time::timeout(std::time::Duration::from_secs(5), handler) + .await + .expect("handler did not finish") + .unwrap() + .unwrap(); + } + #[test] fn policy_local_route_requires_one_exact_host_and_an_origin_form_target() { assert!(valid_policy_local_request( From d1a19c70ee3ed730e6e38ab66b76666a57b5e984 Mon Sep 17 00:00:00 2001 From: Drew Newberry Date: Sun, 27 Sep 2026 18:56:08 +0000 Subject: [PATCH 2/3] fix(homebrew): overwrite generated gateway config during migration (#3752) Closes #3746 Use Homebrew atomic_write for exact legacy config migrations and keep the first-install write distinct in the formula test. Signed-off-by: Drew Newberry --- architecture/build.md | 2 ++ python/openshell/release_formula_test.py | 3 ++- tasks/scripts/release.py | 2 +- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/architecture/build.md b/architecture/build.md index 639dde8ba8..a164f9aaa7 100644 --- a/architecture/build.md +++ b/architecture/build.md @@ -226,6 +226,8 @@ The Homebrew service keeps gateway TLS under the Homebrew state directory but mirrors Docker sandbox client TLS into `$HOME/.local/state/openshell/homebrew/tls` at service start, because Docker Desktop bind mounts must use paths visible to the macOS user's shared home directory. +On upgrade, the formula atomically replaces only exact package-generated +schema-v1 gateway configs, leaving user-edited configs untouched. Local image work should use `mise` tasks rather than direct Docker commands so the same staging and tagging assumptions are used locally and in CI. diff --git a/python/openshell/release_formula_test.py b/python/openshell/release_formula_test.py index 4db21b0951..e1bc1c9c54 100644 --- a/python/openshell/release_formula_test.py +++ b/python/openshell/release_formula_test.py @@ -100,7 +100,8 @@ def test_generate_homebrew_formula_uses_channel_urls_and_exact_version( assert 'bind_address = "[::1]:17670"' in legacy_ipv6_config.group("contents") assert "gateway_config.read == legacy_empty_gateway_config_contents ||" in formula assert "gateway_config.read == legacy_ipv6_gateway_config_contents" in formula - assert "gateway_config.write gateway_config_contents" in formula + assert formula.count("gateway_config.write gateway_config_contents") == 1 + assert "gateway_config.atomic_write gateway_config_contents" in formula assert '# compute_driver = "vm"' not in formula assert ( "openshell gateway add https://localhost:17670 --local --name openshell" diff --git a/tasks/scripts/release.py b/tasks/scripts/release.py index 78695824c2..4d76f13048 100644 --- a/tasks/scripts/release.py +++ b/tasks/scripts/release.py @@ -448,7 +448,7 @@ def post_install # Keep any user-edited config untouched. if gateway_config.read == legacy_empty_gateway_config_contents || gateway_config.read == legacy_ipv6_gateway_config_contents - gateway_config.write gateway_config_contents + gateway_config.atomic_write gateway_config_contents end end From c9da461a588f04b4ae1cc9f46aff98e30ca1bf44 Mon Sep 17 00:00:00 2001 From: Drew Newberry Date: Sun, 27 Sep 2026 19:20:30 +0000 Subject: [PATCH 3/3] fix(ci): keep snap canary sandbox name within limit (#3751) Signed-off-by: Drew Newberry --- .github/workflows/release-canary.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/release-canary.yml b/.github/workflows/release-canary.yml index f6e729fb40..69fd3b3e55 100644 --- a/.github/workflows/release-canary.yml +++ b/.github/workflows/release-canary.yml @@ -242,7 +242,7 @@ jobs: - name: Create and exercise a sandbox run: | set -euo pipefail - sandbox="rc-system-docker-${GITHUB_RUN_ID}" + sandbox="rc-${GITHUB_RUN_ID}" openshell sandbox create --name "$sandbox" --detach openshell sandbox exec --name "$sandbox" --no-tty -- true openshell sandbox delete "$sandbox"