diff --git a/crates/didbot-serve/src/routes.rs b/crates/didbot-serve/src/routes.rs index b2bdd5a8..947f6656 100644 --- a/crates/didbot-serve/src/routes.rs +++ b/crates/didbot-serve/src/routes.rs @@ -543,10 +543,16 @@ impl MakeSpan for TurnSpan { // the turn. Named "request" rather than "http.request" for the same // reason the rest of this log is short: it is read by a person // watching a terminal. + // + // The path, not the URI: `/oauth/authorize` and + // `bot.did.getAuthorization` carry a pushed-request reference in their + // query, and that reference redeems a pending sign-in. Logs are shipped + // and copied like operational data, so nothing that spends a credential + // goes in one. tracing::info_span!( "request", method = %request.method(), - uri = %request.uri(), + uri = %request.uri().path(), version = ?request.version(), turn = %turn, ) diff --git a/crates/didbot-serve/tests/logging.rs b/crates/didbot-serve/tests/logging.rs index 13e77984..e7966fd4 100644 --- a/crates/didbot-serve/tests/logging.rs +++ b/crates/didbot-serve/tests/logging.rs @@ -12,6 +12,11 @@ //! use to impersonate the account they belong to. An identifier — a handle //! or a DID — is not a secret and is logged freely; this file asserts the //! password and the tokens are not, not that nothing about the request is. +//! +//! The query string is held to the same bar. `/oauth/authorize` and +//! `bot.did.getAuthorization` carry a pushed-request reference in theirs, and +//! that reference redeems a pending sign-in, so the request log records the +//! path alone. use std::sync::{Arc, Mutex}; @@ -29,6 +34,9 @@ use didbot_identity::Zone; const ZONE_HOST: &str = "agents.localhost"; const APP_PASSWORD: &str = "quernstone-correct-horse-battery"; +/// Stands in for a live pushed-request reference. Shaped like one, and +/// distinctive enough that finding it anywhere in the log is unambiguous. +const PAR_REFERENCE: &str = "urn:ietf:params:oauth:request_uri:quernstone-thurible-9f3c"; /// Collects everything a `tracing` subscriber writes. Identical to the one /// in `didbot-pds`'s `logging.rs`; not shared, because the two are separate @@ -208,6 +216,7 @@ async fn a_session_lifecycle_logs_nothing_that_would_let_a_reader_impersonate_th // deleteSession, ending the family. let response = app + .clone() .oneshot( Request::builder() .method("POST") @@ -220,6 +229,24 @@ async fn a_session_lifecycle_logs_nothing_that_would_let_a_reader_impersonate_th .expect("router is infallible"); assert_eq!(response.status(), StatusCode::OK); + // The consent page, named the way an agent's client names it: the + // pushed-request reference rides in the query, and `GET /oauth/authorize` + // is the unauthenticated route that spends it. + let response = app + .oneshot( + Request::builder() + .method("GET") + .uri(format!( + "/oauth/authorize?request_uri={PAR_REFERENCE}&client_id=https%3A%2F%2Fagent.pds.example%2Fclient-metadata.json" + )) + .body(Body::empty()) + .expect("request builds"), + ) + .await + .expect("router is infallible"); + // Whatever it answers, the line is written; this test is about the line. + let _ = response.status(); + let rendered = log.contents(); // The identifier is expected to appear: it is a DID, not a secret, and @@ -235,6 +262,13 @@ async fn a_session_lifecycle_logs_nothing_that_would_let_a_reader_impersonate_th "expected the lifecycle to be visible:\n{rendered}" ); + // The route is expected to appear, for the same reason the DID is: the + // absence check below means nothing if the request went unlogged. + assert!( + rendered.contains("/oauth/authorize"), + "expected the consent route in the log:\n{rendered}" + ); + for secret in [ APP_PASSWORD, access_token.as_str(), @@ -242,10 +276,17 @@ async fn a_session_lifecycle_logs_nothing_that_would_let_a_reader_impersonate_th new_access.as_str(), new_refresh.as_str(), agent_token.as_str(), + PAR_REFERENCE, ] { assert!( !rendered.contains(secret), "a secret reached the log: {secret:?}\n{rendered}" ); } + + // Nothing carried a query into the log, reference or not. + assert!( + !rendered.contains('?'), + "a query string reached the log:\n{rendered}" + ); }