From 19f58964d5e8b877be4e6b4d133032dfada2e659 Mon Sep 17 00:00:00 2001 From: Adrien Langou Date: Mon, 9 Nov 2026 15:00:16 +0100 Subject: [PATCH 1/2] fix(network): suppress SigV4 signing diagnostics Signed-off-by: Adrien Langou --- .../openshell-supervisor-network/src/sigv4.rs | 102 +++++++++++++++++- docs/how-it-works/providers/aws.mdx | 1 + 2 files changed, 99 insertions(+), 4 deletions(-) diff --git a/crates/openshell-supervisor-network/src/sigv4.rs b/crates/openshell-supervisor-network/src/sigv4.rs index 79e4c32e18..88d4104104 100644 --- a/crates/openshell-supervisor-network/src/sigv4.rs +++ b/crates/openshell-supervisor-network/src/sigv4.rs @@ -2,14 +2,26 @@ // SPDX-License-Identifier: Apache-2.0 use aws_credential_types::Credentials; +use aws_sigv4::SigningOutput; use aws_sigv4::http_request::{ - PayloadChecksumKind, SignableBody, SignableRequest, SigningSettings, sign, + PayloadChecksumKind, SignableBody, SignableRequest, SigningInstructions, SigningSettings, sign, }; use aws_sigv4::sign::v4; use aws_smithy_runtime_api::client::identity::Identity; use miette::{Result, miette}; use std::time::SystemTime; +/// Keep AWS signer diagnostics out of supervisor log sinks: the canonical +/// request can contain the raw session token added during signing. +fn sign_without_tracing<'a>( + request: SignableRequest<'a>, + params: &'a aws_sigv4::http_request::SigningParams<'a>, +) -> std::result::Result, aws_sigv4::http_request::SigningError> +{ + let no_tracing = tracing::Dispatch::new(tracing::subscriber::NoSubscriber::default()); + tracing::dispatcher::with_default(&no_tracing, || sign(request, params)) +} + /// AWS regions contain a hyphen followed by a digit (e.g., `us-east-1`). /// Service names like `s3` or `bedrock-runtime` do not. fn looks_like_region(s: &str) -> bool { @@ -233,7 +245,7 @@ fn build_identity(access_key: &str, secret_key: &str, session_token: Option<&str fn rebuild_request( parts: &RequestParts<'_>, - instructions: &aws_sigv4::http_request::SigningInstructions, + instructions: &SigningInstructions, body: &[u8], ) -> Vec { let mut output = Vec::with_capacity(256 + body.len()); @@ -297,7 +309,7 @@ pub fn apply_sigv4_to_request( ) .map_err(|e| miette!("SigV4 signable request: {e}"))?; - let (instructions, _signature) = sign(signable_request, &signing_params) + let (instructions, _signature) = sign_without_tracing(signable_request, &signing_params) .map_err(|e| miette!("SigV4 signing failed: {e}"))? .into_parts(); @@ -364,7 +376,7 @@ pub fn apply_sigv4_headers_only_with_body( ) .map_err(|e| miette!("SigV4 signable request: {e}"))?; - let (instructions, _signature) = sign(signable_request, &signing_params) + let (instructions, _signature) = sign_without_tracing(signable_request, &signing_params) .map_err(|e| miette!("SigV4 signing failed: {e}"))? .into_parts(); @@ -374,6 +386,88 @@ pub fn apply_sigv4_headers_only_with_body( #[cfg(test)] mod tests { use super::*; + use std::sync::{Arc, Mutex}; + use tracing::Subscriber; + use tracing::field::{Field, Visit}; + use tracing_subscriber::Layer; + use tracing_subscriber::layer::SubscriberExt as _; + + #[derive(Clone, Default)] + struct EventCapture(Arc>>); + + struct FieldCapture<'a>(&'a mut String); + + impl Visit for FieldCapture<'_> { + fn record_debug(&mut self, field: &Field, value: &dyn std::fmt::Debug) { + use std::fmt::Write as _; + write!(self.0, "{}={value:?};", field.name()) + .expect("writing tracing fields to a String cannot fail"); + } + } + + impl Layer for EventCapture { + fn on_event( + &self, + event: &tracing::Event<'_>, + _ctx: tracing_subscriber::layer::Context<'_, S>, + ) { + let mut fields = String::new(); + event.record(&mut FieldCapture(&mut fields)); + self.0 + .lock() + .expect("event capture mutex must not be poisoned") + .push(format!("{}:{fields}", event.metadata().target())); + } + } + + #[test] + fn signing_does_not_emit_the_canonical_request_to_tracing() { + let events = EventCapture::default(); + let captured = Arc::clone(&events.0); + let subscriber = tracing_subscriber::registry().with(events); + let session_token = "SYNTHETIC_SIGV4_SESSION_TOKEN"; + let raw = b"GET /bucket/key HTTP/1.1\r\nHost: s3.us-east-1.amazonaws.com\r\n\r\n"; + + tracing::subscriber::with_default(subscriber, || { + tracing::trace!(target: "test_capture", "before signing"); + apply_sigv4_to_request( + raw, + "s3.us-east-1.amazonaws.com", + "us-east-1", + "s3", + "ASIATEST", + "synthetic-secret", + Some(session_token), + ) + .unwrap(); + for body in [ + SignableBody::UnsignedPayload, + SignableBody::StreamingUnsignedPayloadTrailer, + ] { + apply_sigv4_headers_only_with_body( + raw, + "s3.us-east-1.amazonaws.com", + "us-east-1", + "s3", + "ASIATEST", + "synthetic-secret", + Some(session_token), + body, + ) + .unwrap(); + } + tracing::trace!(target: "test_capture", "after signing"); + }); + + let events = captured + .lock() + .expect("event capture mutex must not be poisoned") + .join("\n"); + assert!(events.contains("before signing")); + assert!(events.contains("after signing")); + assert!(!events.contains("aws_sigv4")); + assert!(!events.contains(session_token)); + } #[test] fn extract_region_from_hostname() { diff --git a/docs/how-it-works/providers/aws.mdx b/docs/how-it-works/providers/aws.mdx index 953652e3c4..86cf7dde7a 100644 --- a/docs/how-it-works/providers/aws.mdx +++ b/docs/how-it-works/providers/aws.mdx @@ -166,6 +166,7 @@ endpoints: - OpenShell rejects a signed sandbox policy before activation unless the endpoint has a resolvable AWS credential source. An endpoint-bearing profile must declare `AWS_ACCESS_KEY_ID` and `AWS_SECRET_ACCESS_KEY` and cover the signed host, port, and path. An endpointless profile must declare those keys and be selected with `credential_binding.provider` on the signed endpoint. - The `sigv4:body` mode buffers at most 10 MiB. Requests with larger bodies are rejected. Use `sigv4:no_body` or `sigv4` (auto-detect) for large payloads. - The active provider must contain current `AWS_ACCESS_KEY_ID` and `AWS_SECRET_ACCESS_KEY` values. If either becomes unavailable after policy activation, the request fails closed. +- AWS signer diagnostics are omitted from supervisor logs, including at `TRACE` level, because they can contain session tokens. ## Use from a Sandbox From 565794bacdab69b82cb836242773a07d7dd559af Mon Sep 17 00:00:00 2001 From: Adrien Langou Date: Mon, 9 Nov 2026 15:44:36 +0100 Subject: [PATCH 2/2] refactor(network): simplify SigV4 tracing suppression Signed-off-by: Adrien Langou --- .../openshell-supervisor-network/src/sigv4.rs | 50 +++---------------- 1 file changed, 8 insertions(+), 42 deletions(-) diff --git a/crates/openshell-supervisor-network/src/sigv4.rs b/crates/openshell-supervisor-network/src/sigv4.rs index 88d4104104..fb44920bb0 100644 --- a/crates/openshell-supervisor-network/src/sigv4.rs +++ b/crates/openshell-supervisor-network/src/sigv4.rs @@ -18,8 +18,7 @@ fn sign_without_tracing<'a>( params: &'a aws_sigv4::http_request::SigningParams<'a>, ) -> std::result::Result, aws_sigv4::http_request::SigningError> { - let no_tracing = tracing::Dispatch::new(tracing::subscriber::NoSubscriber::default()); - tracing::dispatcher::with_default(&no_tracing, || sign(request, params)) + tracing::dispatcher::with_default(&tracing::Dispatch::none(), || sign(request, params)) } /// AWS regions contain a hyphen followed by a digit (e.g., `us-east-1`). @@ -386,45 +385,15 @@ pub fn apply_sigv4_headers_only_with_body( #[cfg(test)] mod tests { use super::*; - use std::sync::{Arc, Mutex}; - use tracing::Subscriber; - use tracing::field::{Field, Visit}; - use tracing_subscriber::Layer; - use tracing_subscriber::layer::SubscriberExt as _; - - #[derive(Clone, Default)] - struct EventCapture(Arc>>); - - struct FieldCapture<'a>(&'a mut String); - - impl Visit for FieldCapture<'_> { - fn record_debug(&mut self, field: &Field, value: &dyn std::fmt::Debug) { - use std::fmt::Write as _; - write!(self.0, "{}={value:?};", field.name()) - .expect("writing tracing fields to a String cannot fail"); - } - } - - impl Layer for EventCapture { - fn on_event( - &self, - event: &tracing::Event<'_>, - _ctx: tracing_subscriber::layer::Context<'_, S>, - ) { - let mut fields = String::new(); - event.record(&mut FieldCapture(&mut fields)); - self.0 - .lock() - .expect("event capture mutex must not be poisoned") - .push(format!("{}:{fields}", event.metadata().target())); - } - } #[test] fn signing_does_not_emit_the_canonical_request_to_tracing() { - let events = EventCapture::default(); - let captured = Arc::clone(&events.0); - let subscriber = tracing_subscriber::registry().with(events); + let log = tempfile::NamedTempFile::new().unwrap(); + let subscriber = tracing_subscriber::fmt() + .with_max_level(tracing::Level::TRACE) + .with_ansi(false) + .with_writer(log.reopen().unwrap()) + .finish(); let session_token = "SYNTHETIC_SIGV4_SESSION_TOKEN"; let raw = b"GET /bucket/key HTTP/1.1\r\nHost: s3.us-east-1.amazonaws.com\r\n\r\n"; @@ -459,10 +428,7 @@ mod tests { tracing::trace!(target: "test_capture", "after signing"); }); - let events = captured - .lock() - .expect("event capture mutex must not be poisoned") - .join("\n"); + let events = std::fs::read_to_string(log.path()).unwrap(); assert!(events.contains("before signing")); assert!(events.contains("after signing")); assert!(!events.contains("aws_sigv4"));