fix(gateway): redact credential query values in access logs

This commit is contained in:
MMEXA
2026-05-17 15:26:56 +00:00
parent a2f91b4108
commit ed570155a1
3 changed files with 169 additions and 9 deletions

View File

@@ -7,7 +7,7 @@ use crate::constants::{
}; };
use crate::control::GatewayControlDecision; use crate::control::GatewayControlDecision;
use crate::control::GatewayPublicRequestContext; use crate::control::GatewayPublicRequestContext;
use crate::middleware::{should_downgrade_access_log, RequestLogEmitted}; use crate::middleware::{sanitize_access_log_path, should_downgrade_access_log, RequestLogEmitted};
use crate::AppState; use crate::AppState;
use aether_runtime::{maybe_hold_axum_response_permit, AdmissionPermit}; use aether_runtime::{maybe_hold_axum_response_permit, AdmissionPermit};
use axum::body::{Body, Bytes}; use axum::body::{Body, Bytes};
@@ -95,11 +95,12 @@ pub(super) fn finalize_gateway_response(
.map(|auth_context| auth_context.api_key_id.as_str()) .map(|auth_context| auth_context.api_key_id.as_str())
.unwrap_or("-"); .unwrap_or("-");
let status_code = response.status().as_u16(); let status_code = response.status().as_u16();
let sanitized_path_and_query = sanitize_access_log_path(path_and_query);
emit_admin_audit( emit_admin_audit(
&mut response, &mut response,
trace_id, trace_id,
method, method,
path_and_query, sanitized_path_and_query.as_str(),
control_decision, control_decision,
); );
if response.status().is_server_error() { if response.status().is_server_error() {
@@ -112,7 +113,7 @@ pub(super) fn finalize_gateway_response(
request_id, request_id,
remote_addr = %remote_addr, remote_addr = %remote_addr,
method = %method, method = %method,
path = %path_and_query, path = %sanitized_path_and_query,
user_id, user_id,
api_key_id, api_key_id,
route_class, route_class,
@@ -122,7 +123,7 @@ pub(super) fn finalize_gateway_response(
elapsed_ms, elapsed_ms,
"gateway request failed" "gateway request failed"
); );
} else if should_downgrade_access_log(method, path_and_query) { } else if should_downgrade_access_log(method, sanitized_path_and_query.as_str()) {
trace!( trace!(
event_name = "http_request_completed", event_name = "http_request_completed",
log_type = "access", log_type = "access",
@@ -132,7 +133,7 @@ pub(super) fn finalize_gateway_response(
request_id, request_id,
remote_addr = %remote_addr, remote_addr = %remote_addr,
method = %method, method = %method,
path = %path_and_query, path = %sanitized_path_and_query,
user_id, user_id,
api_key_id, api_key_id,
route_class, route_class,
@@ -152,7 +153,7 @@ pub(super) fn finalize_gateway_response(
request_id, request_id,
remote_addr = %remote_addr, remote_addr = %remote_addr,
method = %method, method = %method,
path = %path_and_query, path = %sanitized_path_and_query,
user_id, user_id,
api_key_id, api_key_id,
route_class, route_class,
@@ -254,3 +255,106 @@ pub(super) fn finalize_gateway_response_with_context(
request_permit, request_permit,
) )
} }
#[cfg(test)]
mod tests {
use super::finalize_gateway_response;
use crate::control::GatewayControlDecision;
use crate::AppState;
use axum::body::Body;
use axum::http::{Method, Response, StatusCode};
use std::sync::{Arc, Mutex};
use std::time::Instant;
use tracing_subscriber::filter::LevelFilter;
use tracing_subscriber::prelude::*;
#[derive(Clone, Default)]
struct SharedBuffer(Arc<Mutex<Vec<u8>>>);
struct SharedBufferWriter(Arc<Mutex<Vec<u8>>>);
impl SharedBuffer {
fn lines(&self) -> Vec<serde_json::Value> {
String::from_utf8(self.0.lock().expect("buffer should lock").clone())
.expect("buffer should contain valid utf-8")
.lines()
.filter(|line| !line.trim().is_empty())
.map(|line| serde_json::from_str(line).expect("json log line should parse"))
.collect()
}
}
impl std::io::Write for SharedBufferWriter {
fn write(&mut self, buf: &[u8]) -> std::io::Result<usize> {
self.0
.lock()
.expect("buffer should lock")
.extend_from_slice(buf);
Ok(buf.len())
}
fn flush(&mut self) -> std::io::Result<()> {
Ok(())
}
}
impl<'a> tracing_subscriber::fmt::writer::MakeWriter<'a> for SharedBuffer {
type Writer = SharedBufferWriter;
fn make_writer(&'a self) -> Self::Writer {
SharedBufferWriter(Arc::clone(&self.0))
}
}
#[test]
fn finalize_gateway_response_logs_sanitized_path_and_query() {
let state = AppState::new().expect("gateway state should build");
let writer = SharedBuffer::default();
let subscriber = tracing_subscriber::registry().with(
tracing_subscriber::fmt::layer()
.json()
.flatten_event(true)
.with_current_span(false)
.with_span_list(false)
.with_writer(writer.clone())
.with_filter(LevelFilter::INFO),
);
let dispatch = tracing::Dispatch::new(subscriber);
let _guard = tracing::dispatcher::set_default(&dispatch);
let remote_addr = "127.0.0.1:8080"
.parse()
.expect("remote address should parse");
let control_decision = GatewayControlDecision::synthetic(
"/v1beta/models/gemini-3-flash-preview:generateContent",
Some("ai_public".to_string()),
Some("gemini".to_string()),
Some("generate_content".to_string()),
Some("gemini:generate_content".to_string()),
);
let response = Response::builder()
.status(StatusCode::OK)
.body(Body::empty())
.expect("response should build");
let _response = finalize_gateway_response(
&state,
response,
"trace-finalize",
&remote_addr,
&Method::GET,
"/v1beta/models/gemini-3-flash-preview:generateContent?key=secret&alt=sse",
Some(&control_decision),
"execution_runtime_sync",
&Instant::now(),
None,
);
let logs = writer.lines();
assert_eq!(logs.len(), 1);
assert_eq!(
logs[0]["path"],
"/v1beta/models/gemini-3-flash-preview:generateContent?alt=sse"
);
}
}

View File

@@ -13,6 +13,7 @@ use crate::constants::{
}; };
use crate::headers::extract_or_generate_trace_id; use crate::headers::extract_or_generate_trace_id;
use crate::log_ids::short_request_id; use crate::log_ids::short_request_id;
use aether_ai_formats::api::sanitize_request_path_and_query;
#[derive(Debug, Clone, Copy)] #[derive(Debug, Clone, Copy)]
pub(crate) struct RequestLogEmitted; pub(crate) struct RequestLogEmitted;
@@ -52,14 +53,19 @@ pub(crate) fn should_downgrade_access_log(method: &Method, path: &str) -> bool {
|| normalized_path.starts_with("/api/admin/monitoring/trace/") || normalized_path.starts_with("/api/admin/monitoring/trace/")
} }
pub(crate) fn sanitize_access_log_path(path: &str) -> String {
sanitize_request_path_and_query(path, None).unwrap_or_else(|| "/".to_string())
}
pub(crate) async fn access_log_middleware(mut request: Request<Body>, next: Next) -> Response { pub(crate) async fn access_log_middleware(mut request: Request<Body>, next: Next) -> Response {
let started_at = Instant::now(); let started_at = Instant::now();
let method = request.method().clone(); let method = request.method().clone();
let path = request let raw_path = request
.uri() .uri()
.path_and_query() .path_and_query()
.map(|value| value.as_str().to_string()) .map(|value| value.as_str().to_string())
.unwrap_or_else(|| "/".to_string()); .unwrap_or_else(|| "/".to_string());
let path = sanitize_access_log_path(&raw_path);
let trace_id = extract_or_generate_trace_id(request.headers()); let trace_id = extract_or_generate_trace_id(request.headers());
if !request.headers().contains_key(TRACE_ID_HEADER) { if !request.headers().contains_key(TRACE_ID_HEADER) {
request.headers_mut().insert( request.headers_mut().insert(
@@ -158,7 +164,7 @@ pub(crate) async fn access_log_middleware(mut request: Request<Body>, next: Next
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use super::{access_log_middleware, should_downgrade_access_log}; use super::{access_log_middleware, sanitize_access_log_path, should_downgrade_access_log};
use crate::constants::{ use crate::constants::{
CONTROL_REQUEST_ID_HEADER, CONTROL_ROUTE_CLASS_HEADER, EXECUTION_PATH_HEADER, CONTROL_REQUEST_ID_HEADER, CONTROL_ROUTE_CLASS_HEADER, EXECUTION_PATH_HEADER,
TRACE_ID_HEADER, TRACE_ID_HEADER,
@@ -212,6 +218,56 @@ mod tests {
} }
} }
#[test]
fn access_log_path_redacts_credential_query_values() {
assert_eq!(
sanitize_access_log_path(
"/v1beta/models/gemini-3-flash-preview:generateContent?key=secret&alt=sse&pageSize=10&token=hidden"
),
"/v1beta/models/gemini-3-flash-preview:generateContent?alt=sse&pageSize=10"
);
}
#[tokio::test(flavor = "current_thread")]
async fn access_log_emits_sanitized_path() {
let writer = SharedBuffer::default();
let subscriber = tracing_subscriber::registry().with(
tracing_subscriber::fmt::layer()
.json()
.flatten_event(true)
.with_current_span(false)
.with_span_list(false)
.with_writer(writer.clone())
.with_filter(LevelFilter::INFO),
);
let dispatch = tracing::Dispatch::new(subscriber);
let _guard = tracing::dispatcher::set_default(&dispatch);
let app = Router::new()
.route(
"/v1beta/models/gemini-3-flash-preview:generateContent",
get(|| async { Response::new(Body::empty()) }),
)
.layer(axum::middleware::from_fn(access_log_middleware));
let _response = app
.oneshot(
Request::builder()
.uri("/v1beta/models/gemini-3-flash-preview:generateContent?key=secret&alt=sse")
.body(Body::empty())
.expect("request should build"),
)
.await
.expect("request should succeed");
let logs = writer.lines();
assert_eq!(logs.len(), 1);
assert_eq!(
logs[0]["path"],
"/v1beta/models/gemini-3-flash-preview:generateContent?alt=sse"
);
}
#[tokio::test(flavor = "current_thread")] #[tokio::test(flavor = "current_thread")]
async fn access_log_emits_completed_events_by_default() { async fn access_log_emits_completed_events_by_default() {
let writer = SharedBuffer::default(); let writer = SharedBuffer::default();

View File

@@ -3,7 +3,7 @@ mod frontdoor_cors;
mod strip_cf_headers; mod strip_cf_headers;
pub(crate) use access_log::{ pub(crate) use access_log::{
access_log_middleware, should_downgrade_access_log, RequestLogEmitted, access_log_middleware, sanitize_access_log_path, should_downgrade_access_log, RequestLogEmitted,
}; };
pub(crate) use frontdoor_cors::frontdoor_cors_middleware; pub(crate) use frontdoor_cors::frontdoor_cors_middleware;
pub use strip_cf_headers::strip_cf_headers_middleware; pub use strip_cf_headers::strip_cf_headers_middleware;