From d465c5679ad80d166d7cd8eb8b892124db2c7cc3 Mon Sep 17 00:00:00 2001 From: Gregory Schier Date: Mon, 14 Sep 2026 22:38:07 -0700 Subject: [PATCH] Make web proxy request logging opt-in --- crates-server/yaak-web/README.md | 13 +++- crates-server/yaak-web/src/guard.rs | 4 +- crates-server/yaak-web/src/lib.rs | 13 ++-- crates-server/yaak-web/src/main.rs | 5 +- crates-server/yaak-web/src/send.rs | 9 ++- crates-server/yaak-web/tests/logging.rs | 98 +++++++++++++++++++++++++ 6 files changed, 128 insertions(+), 14 deletions(-) create mode 100644 crates-server/yaak-web/tests/logging.rs diff --git a/crates-server/yaak-web/README.md b/crates-server/yaak-web/README.md index 161088df..23a33d01 100644 --- a/crates-server/yaak-web/README.md +++ b/crates-server/yaak-web/README.md @@ -94,6 +94,14 @@ arguments; `--help` lists them all. | `--max-concurrent` | 256 | Sends in flight at once. | | `--trust-forwarded-for` | off | Take the client IP from `X-Forwarded-For`. Only behind a load balancer that sets it. | +## Logging + +Default logs contain operational warnings and errors, not request URLs, client IPs, +DNS lookups, or per-request timing. To diagnose a self-hosted instance, opt in with +`RUST_LOG=warn,yaak_http=error,yaak_web=debug`. This includes request URLs and IPs, +so only enable it while debugging. Hosting infrastructure may maintain its own +access logs independently. + ## Serving the app `--serve DIR` puts a file server behind the API routes: `/v1/*` is matched @@ -150,8 +158,9 @@ caught, and so is a `Location:` header that points at one. It also refuses body types that would read files on its own disk (`binary`, multipart file fields), since no browser tab could legitimately mean those. -Refusals are logged with the reason. On a public instance (`web.yaak.app`, or -anything else strangers can reach) this must stay on: the machine's private +Refusal reasons are included in debug logs when enabled. On a public instance +(`web.yaak.app`, or anything else strangers can reach), private-network protection +must stay on: the machine's private network is the host's, not the user's, so a `localhost` or LAN API is not the user's to reach through it — the desktop app is what reaches those. On an instance you run for yourself, that reasoning is inverted, and diff --git a/crates-server/yaak-web/src/guard.rs b/crates-server/yaak-web/src/guard.rs index 284fb7a1..3055ce8d 100644 --- a/crates-server/yaak-web/src/guard.rs +++ b/crates-server/yaak-web/src/guard.rs @@ -11,7 +11,7 @@ //! resolves every hop. use async_trait::async_trait; -use log::warn; +use log::debug; use std::net::{IpAddr, Ipv4Addr, Ipv6Addr}; use std::sync::Arc; use tokio::sync::mpsc; @@ -186,7 +186,7 @@ impl HttpSender for GuardedSender { event_tx: mpsc::Sender, ) -> yaak_http::error::Result { if let Err(reason) = self.policy.check_url(&request.url) { - warn!("Refused {} {}: {reason}", request.method, request.url); + debug!("Refused {} {}: {reason}", request.method, request.url); return Err(yaak_http::error::Error::RequestError(reason)); } self.inner.send(request, event_tx).await diff --git a/crates-server/yaak-web/src/lib.rs b/crates-server/yaak-web/src/lib.rs index 0b3b098a..06f15986 100644 --- a/crates-server/yaak-web/src/lib.rs +++ b/crates-server/yaak-web/src/lib.rs @@ -27,7 +27,7 @@ use axum::routing::{get, post}; pub use config::Config; use guard::DestinationPolicy; use limits::RateLimiter; -use log::{info, warn}; +use log::{debug, info, warn}; use send::{Refusal, SendLimits}; use serde_json::json; use std::net::{IpAddr, SocketAddr}; @@ -40,6 +40,9 @@ use tower_http::cors::{AllowOrigin, CorsLayer}; use tower_http::services::{ServeDir, ServeFile}; use wire::SendRequest; +/// Minimal operational logging; request diagnostics require an explicit `RUST_LOG` setting. +pub const DEFAULT_LOG_FILTER: &str = "warn,yaak_http=error"; + #[derive(Clone)] struct AppState { config: Arc, @@ -181,7 +184,7 @@ async fn send_http( ) -> Response { let ip = client_ip(&state.config, &headers, peer); if let Err(wait) = state.rate_limiter.check(ip) { - warn!("Rate limited {ip}"); + debug!("Rate limited {ip}"); let mut res = error_response( StatusCode::TOO_MANY_REQUESTS, format!("Rate limit reached; try again in {}s", wait.as_secs().max(1)), @@ -191,7 +194,7 @@ async fn send_http( } let Ok(permit) = state.in_flight.clone().try_acquire_owned() else { - warn!("At capacity; refusing {ip}"); + debug!("At capacity; refusing {ip}"); return error_response(StatusCode::SERVICE_UNAVAILABLE, "This server is at capacity"); }; @@ -200,13 +203,13 @@ async fn send_http( Err(Refusal::Unsupported(m)) => return error_response(StatusCode::BAD_REQUEST, m), Err(Refusal::Invalid(m)) => return error_response(StatusCode::BAD_REQUEST, m), Err(Refusal::Destination(m)) => { - warn!("Refused send from {ip}: {m}"); + debug!("Refused send from {ip}: {m}"); return error_response(StatusCode::FORBIDDEN, m); } }; let description = prepared.describe(); - info!("{ip} -> {description}"); + debug!("{ip} -> {description}"); let started = Instant::now(); let (tx, rx) = tokio::sync::mpsc::channel(send::FRAME_CHANNEL_CAPACITY); diff --git a/crates-server/yaak-web/src/main.rs b/crates-server/yaak-web/src/main.rs index be32b082..aae5ac81 100644 --- a/crates-server/yaak-web/src/main.rs +++ b/crates-server/yaak-web/src/main.rs @@ -7,7 +7,10 @@ use yaak_web::{Config, router}; #[tokio::main] async fn main() { - env_logger::Builder::from_env(env_logger::Env::default().default_filter_or("info")).init(); + env_logger::Builder::from_env( + env_logger::Env::default().default_filter_or(yaak_web::DEFAULT_LOG_FILTER), + ) + .init(); let config = Config::parse(); let bind = config.bind; let rate_limit_per_minute = config.rate_limit_per_minute; diff --git a/crates-server/yaak-web/src/send.rs b/crates-server/yaak-web/src/send.rs index e6010c79..841d229e 100644 --- a/crates-server/yaak-web/src/send.rs +++ b/crates-server/yaak-web/src/send.rs @@ -9,7 +9,7 @@ use crate::guard::{DestinationPolicy, GuardedSender}; use crate::wire::{Frame, SendRequest}; use base64::Engine; use bytes::Bytes; -use log::{info, warn}; +use log::{debug, warn}; use std::convert::Infallible; use std::sync::Arc; use std::sync::atomic::{AtomicU64, Ordering}; @@ -344,7 +344,8 @@ async fn write_frame(frames: &FrameSender, frame: &Frame) -> Result<(), ()> { let mut line = match serde_json::to_vec(frame) { Ok(v) => v, Err(e) => { - warn!("Failed to serialize frame: {e}"); + warn!("Failed to serialize response frame"); + debug!("Frame serialization error: {e}"); return Err(()); } }; @@ -352,7 +353,7 @@ async fn write_frame(frames: &FrameSender, frame: &Frame) -> Result<(), ()> { frames.send(Ok(Bytes::from(line))).await.map_err(|_| ()) } -/// Log a finished send at info: destination, outcome, and how long, never the content. +/// Request diagnostics are opt-in, including destinations and completion timing. pub fn log_outcome(description: &str, started: Instant, outcome: &str) { - info!("{description} -> {outcome} in {:?}", started.elapsed()); + debug!("{description} -> {outcome} in {:?}", started.elapsed()); } diff --git a/crates-server/yaak-web/tests/logging.rs b/crates-server/yaak-web/tests/logging.rs new file mode 100644 index 00000000..99b43164 --- /dev/null +++ b/crates-server/yaak-web/tests/logging.rs @@ -0,0 +1,98 @@ +use axum::Router; +use axum::body::{Body, to_bytes}; +use axum::extract::{ConnectInfo, Request}; +use axum::http::StatusCode; +use axum::routing::get; +use clap::Parser; +use std::io::{self, Write}; +use std::net::SocketAddr; +use std::sync::{Arc, Mutex}; +use tower::ServiceExt; +use yaak_web::{Config, DEFAULT_LOG_FILTER, router}; + +#[derive(Clone)] +struct LogCapture(Arc>>); + +impl Write for LogCapture { + fn write(&mut self, bytes: &[u8]) -> io::Result { + self.0.lock().unwrap().extend_from_slice(bytes); + Ok(bytes.len()) + } + + fn flush(&mut self) -> io::Result<()> { + Ok(()) + } +} + +fn send_request(url: &str) -> Request { + let payload = serde_json::json!({ + "request": { "url": url, "method": "GET" }, + "settings": { + "validateCertificates": true, + "followRedirects": true, + "timeoutMs": 2000, + "sendCookies": true, + "storeCookies": true + } + }); + Request::builder() + .method("POST") + .uri("/v1/http/send") + .header("content-type", "application/json") + .extension(ConnectInfo("192.0.2.42:1234".parse::().unwrap())) + .body(Body::from(payload.to_string())) + .unwrap() +} + +#[tokio::test] +async fn default_logging_keeps_operational_warnings_without_request_details() { + let capture = LogCapture(Arc::new(Mutex::new(Vec::new()))); + env_logger::Builder::new() + .parse_filters(DEFAULT_LOG_FILTER) + .target(env_logger::Target::Pipe(Box::new(capture.clone()))) + .init(); + + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let port = listener.local_addr().unwrap().port(); + let upstream = tokio::spawn(async move { + axum::serve( + listener, + Router::new().route("/private-path", get(|| async { "secret body" })), + ) + .await + .unwrap(); + }); + let mut config = Config::parse_from(["yaak-web"]); + config.allow_private_networks = true; + config.rate_limit_per_minute = 1; + let app = router(config.clone()); + assert!(String::from_utf8_lossy(&capture.0.lock().unwrap()).contains("ALLOWED")); + capture.0.lock().unwrap().clear(); + + // Exercise the HTTP engine and DNS lookup as well as the proxy's own logs. + let url = format!("http://localhost:{port}/private-path?token=secret"); + let response = app.clone().oneshot(send_request(&url)).await.unwrap(); + assert_eq!(response.status(), StatusCode::OK); + let body = to_bytes(response.into_body(), 64 * 1024).await.unwrap(); + assert!(String::from_utf8_lossy(&body).contains("\"type\":\"done\"")); + assert_eq!( + app.oneshot(send_request(&url)).await.unwrap().status(), + StatusCode::TOO_MANY_REQUESTS + ); + + config.allow_private_networks = false; + let app = router(config.clone()); + assert_eq!( + app.oneshot(send_request("http://127.0.0.1/private-path")).await.unwrap().status(), + StatusCode::FORBIDDEN + ); + config.max_concurrent = 0; + assert_eq!( + router(config).oneshot(send_request(&url)).await.unwrap().status(), + StatusCode::SERVICE_UNAVAILABLE + ); + + upstream.abort(); + let logs = capture.0.lock().unwrap(); + assert!(logs.is_empty(), "Unexpected request logs: {}", String::from_utf8_lossy(&logs)); +}