From 57dd73f3c4d5f04290b2d227c0ecd7d3f2bf12ef Mon Sep 17 00:00:00 2001 From: cyberspace-cs Date: Sat, 19 Sep 2026 19:06:09 +0800 Subject: [PATCH 1/2] fix(ipc): decode explicit null results and stop silent null degradation The IPC protocol is JSON-RPC-style: a successful reply carries "result" and a failed reply carries "error". Two defects made JSON null a silent failure channel instead of a legal value. 1. Decoder (bsk-protocol): ResponseFrame::deserialize and the Frame visitor modelled the result field as Option, which serde collapses "result": null into the same None as a missing field. The daemon emits exactly that shape through its serde_json::to_value(..).unwrap_or(Value::Null) fallback, so a result serialisation failure surfaced on the CLI as a misleading "ambiguous response" / "expected result or error" decode error instead of a null result. Both deserializers now parse the raw value with #[serde(default, deserialize_with)] so an explicit null result decodes to Ok(Value::Null) while a missing field stays an error. 2. Daemon (bsk-cli): every serde_json::to_value(..).unwrap_or(Value::Null) site silently degraded a result serialisation failure (e.g. a payload nesting deeper than serde_json's recursion limit) to JSON null, hiding the real failure and colliding with legitimate null results. These now route through ok_value()/serialise_err() helpers that return a structured protocol_error instead, and the upload/download param staging paths return the same error rather than forwarding null params to the extension. Adds unit tests on both sides: explicit-null decode for Frame and ResponseFrame, serialise round-trip symmetry, missing-field regression, and the daemon-side serialisation-failure path. --- CHANGELOG.md | 10 ++ crates/bsk-cli/src/daemon/ipc.rs | 166 +++++++++++++++++++++++++------ crates/bsk-protocol/src/frame.rs | 86 +++++++++++++++- 3 files changed, 232 insertions(+), 30 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3d928347..b9c13516 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,16 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/). Starting from 0.2.0, CLI / Extension / DSH Plugin share the same version number. +## [Unreleased] + +### Fixed + +- IPC protocol: an explicit `null` result (for example the daemon's + `serde_json::to_value(..).unwrap_or(Value::Null)` fallback) is now decoded as a + success body instead of being rejected as an ambiguous frame; the daemon no + longer silently degrades result serialisation failures to `null` and returns a + structured `protocol_error` instead + ## [0.3.0] - 2026-09-16 ### Added diff --git a/crates/bsk-cli/src/daemon/ipc.rs b/crates/bsk-cli/src/daemon/ipc.rs index 69daf58c..7847f3c0 100644 --- a/crates/bsk-cli/src/daemon/ipc.rs +++ b/crates/bsk-cli/src/daemon/ipc.rs @@ -156,7 +156,7 @@ pub fn default_ping_handler() -> RpcHandler { match method { Method::SystemPing => { let result = PingResult { pong: true }; - ResponseBody::Ok(serde_json::to_value(result).unwrap_or(Value::Null)) + ok_value(result) } other => ResponseBody::Err(RpcError { code: ErrorCode::UnknownMethod, @@ -178,11 +178,11 @@ pub fn system_handler(status: DaemonStatus) -> RpcHandler { match method { Method::SystemPing => { let result = PingResult { pong: true }; - ResponseBody::Ok(serde_json::to_value(result).unwrap_or(Value::Null)) + ok_value(result) } Method::SystemStatus => { let result = status.snapshot(); - ResponseBody::Ok(serde_json::to_value(result).unwrap_or(Value::Null)) + ok_value(result) } other => ResponseBody::Err(RpcError { code: ErrorCode::UnknownMethod, @@ -230,7 +230,7 @@ pub fn full_handler(status: DaemonStatus, state: Arc) -> RpcHandler let body = match method { Method::SystemPing => { let result = PingResult { pong: true }; - ResponseBody::Ok(serde_json::to_value(result).unwrap_or(Value::Null)) + ok_value(result) } Method::SystemStatus => match handle_status(&status, &state, params).await { Ok(v) => ResponseBody::Ok(v), @@ -437,7 +437,16 @@ async fn handle_tool_dispatch( for (file, path) in upload.files.iter_mut().zip(paths) { file.staged_path = Some(path.to_string_lossy().into_owned()); } - params = serde_json::to_value(upload).unwrap_or(Value::Null); + params = match serde_json::to_value(&upload) { + Ok(v) => v, + Err(err) => { + return ResponseBody::Err(RpcError { + code: ErrorCode::ProtocolError, + message: format!("failed to serialise upload params: {err}"), + data: None, + }); + } + }; } else if method == Method::ToolDownload { let mut download: DownloadParams = match serde_json::from_value(params) { Ok(v) => v, @@ -450,7 +459,16 @@ async fn handle_tool_dispatch( download.browser_relative_dir = Some(staging.browser_relative_dir); download.max_byte_size = Some(super::file_transfer::MAX_TRANSFER_BYTES); download_transfer_id = Some(staging.transfer_id); - params = serde_json::to_value(download).unwrap_or(Value::Null); + params = match serde_json::to_value(&download) { + Ok(v) => v, + Err(err) => { + return ResponseBody::Err(RpcError { + code: ErrorCode::ProtocolError, + message: format!("failed to serialise download params: {err}"), + data: None, + }); + } + }; } if let Some(audit_id) = audit_id && let Some(object) = params.as_object_mut() @@ -505,7 +523,7 @@ async fn handle_tool_dispatch( Ok(size) => { result.byte_size = size; result.transfer_id = Some(id); - ResponseBody::Ok(serde_json::to_value(result).unwrap_or(Value::Null)) + ok_value(result) } Err(err) => { state @@ -544,7 +562,7 @@ fn handle_transfer_begin(state: &Arc, params: Value) -> ResponseBod }); } match state.transfers.begin_upload(p) { - Ok(v) => ResponseBody::Ok(serde_json::to_value(v).unwrap_or(Value::Null)), + Ok(v) => ok_value(v), Err(err) => ResponseBody::Err(err), } } @@ -555,7 +573,7 @@ fn handle_transfer_chunk(state: &Arc, params: Value) -> ResponseBod Err(err) => return ResponseBody::Err(invalid_params(err.to_string())), }; match state.transfers.write_chunk(p) { - Ok(v) => ResponseBody::Ok(serde_json::to_value(v).unwrap_or(Value::Null)), + Ok(v) => ok_value(v), Err(err) => ResponseBody::Err(err), } } @@ -566,7 +584,7 @@ fn handle_transfer_finish(state: &Arc, params: Value) -> ResponseBo Err(err) => return ResponseBody::Err(invalid_params(err.to_string())), }; match state.transfers.finish_upload(p) { - Ok(v) => ResponseBody::Ok(serde_json::to_value(v).unwrap_or(Value::Null)), + Ok(v) => ok_value(v), Err(err) => ResponseBody::Err(err), } } @@ -577,7 +595,7 @@ fn handle_transfer_read(state: &Arc, params: Value) -> ResponseBody Err(err) => return ResponseBody::Err(invalid_params(err.to_string())), }; match state.transfers.read_chunk(p) { - Ok(v) => ResponseBody::Ok(serde_json::to_value(v).unwrap_or(Value::Null)), + Ok(v) => ok_value(v), Err(err) => ResponseBody::Err(err), } } @@ -587,7 +605,7 @@ fn handle_transfer_release(state: &Arc, params: Value) -> ResponseB Ok(v) => v, Err(err) => return ResponseBody::Err(invalid_params(err.to_string())), }; - ResponseBody::Ok(serde_json::to_value(state.transfers.release(p)).unwrap_or(Value::Null)) + ok_value(state.transfers.release(p)) } fn invalid_params(message: impl Into) -> RpcError { @@ -598,6 +616,36 @@ fn invalid_params(message: impl Into) -> RpcError { } } +/// Serialise a handler result into a success body. +/// +/// A serialisation failure (e.g. exceeding serde_json's recursion limit +/// on a deeply nested payload) is surfaced as a `protocol_error` instead +/// of silently degrading to JSON `null`. A `null` result would otherwise +/// be indistinguishable from a method legitimately returning null and +/// would hide the real failure from the CLI client; it also trips the +/// wire decoder's "expected result or error" guard on peers that predate +/// the explicit-null fix in `bsk-protocol/src/frame.rs`. +fn ok_value(value: impl Serialize) -> ResponseBody { + match serde_json::to_value(&value) { + Ok(v) => ResponseBody::Ok(v), + Err(err) => ResponseBody::Err(RpcError { + code: ErrorCode::ProtocolError, + message: format!("failed to serialise handler result: {err}"), + data: None, + }), + } +} + +/// Map a result-serialisation failure onto a structured `protocol_error` +/// for handlers that return `Result`. +fn serialise_err(context: &str, err: serde_json::Error) -> RpcError { + RpcError { + code: ErrorCode::ProtocolError, + message: format!("failed to serialise {context}: {err}"), + data: None, + } +} + /// Upper bound on `tool.wait_ms` (5 minutes). Larger values are /// rejected as `invalid_params` so a buggy agent cannot wedge a /// daemon-side sleep beyond a reasonable window. The cap matches the @@ -635,9 +683,7 @@ async fn handle_wait_ms( }); } if params.duration_ms == 0 { - return ResponseBody::Ok( - serde_json::to_value(WaitMsResult { waited_ms: 0 }).unwrap_or(Value::Null), - ); + return ok_value(WaitMsResult { waited_ms: 0 }); } let guard = match registry.register(rpc_id) { Ok(guard) => guard, @@ -652,10 +698,7 @@ async fn handle_wait_ms( let token = guard.token().clone(); let result = tokio::select! { _ = tokio::time::sleep(Duration::from_millis(params.duration_ms)) => { - ResponseBody::Ok( - serde_json::to_value(WaitMsResult { waited_ms: params.duration_ms }) - .unwrap_or(Value::Null), - ) + ok_value(WaitMsResult { waited_ms: params.duration_ms }) } _ = token.cancelled() => { ResponseBody::Err(RpcError { @@ -723,7 +766,7 @@ fn handle_cancel(state: &Arc, params: Value) -> ResponseBody { ); } } - ResponseBody::Ok(serde_json::to_value(CancelResult { cancelled }).unwrap_or(Value::Null)) + ok_value(CancelResult { cancelled }) } /// Test-only re-export of the cancel handler: the system-only handler @@ -743,7 +786,7 @@ fn handle_cancel_with_registry_only(registry: &Arc, params: Value } }; let cancelled = registry.cancel(¶ms.rpc_id); - ResponseBody::Ok(serde_json::to_value(CancelResult { cancelled }).unwrap_or(Value::Null)) + ok_value(CancelResult { cancelled }) } fn tool_dispatch_timeout(params: &Value) -> Result { @@ -872,7 +915,8 @@ async fn handle_status( ) -> Result { let params: StatusParams = parse_params_or_default(params)?; maybe_wait_for_browser(state, params.wait_for_browser_ms).await; - Ok(serde_json::to_value(status.snapshot_with(state)).unwrap_or(Value::Null)) + serde_json::to_value(status.snapshot_with(state)) + .map_err(|err| serialise_err("status snapshot", err)) } fn parse_params_or_default(params: Value) -> Result @@ -976,7 +1020,7 @@ async fn handle_session_start( browser_instance_id: session.browser_id.0.clone(), agent_window_id: session.agent_window_id, }; - Ok(serde_json::to_value(result).unwrap_or(Value::Null)) + serde_json::to_value(result).map_err(|err| serialise_err("session start result", err)) } Err(err) => Err(map_start_error(err)), } @@ -1084,7 +1128,7 @@ async fn handle_session_stop( returned_tab_ids: stop.returned_tab_ids, return_failures: stop.return_failures, }; - Ok(serde_json::to_value(result).unwrap_or(Value::Null)) + serde_json::to_value(result).map_err(|err| serialise_err("session stop result", err)) } Err(StopSessionError::ReturnFailures(stop)) => { let message = format!( @@ -1101,7 +1145,7 @@ async fn handle_session_stop( returned_tab_ids: stop.returned_tab_ids, return_failures: stop.return_failures, }; - Ok(serde_json::to_value(result).unwrap_or(Value::Null)) + serde_json::to_value(result).map_err(|err| serialise_err("session stop result", err)) } Err(err) => Err(map_stop_error(err)), } @@ -1225,7 +1269,7 @@ async fn handle_session_stop_all( returned_tab_ids, return_failures, }; - Ok(serde_json::to_value(result).unwrap_or(Value::Null)) + serde_json::to_value(result).map_err(|err| serialise_err("session stop_all result", err)) } fn handle_session_list(state: &Arc) -> ResponseBody { @@ -1235,7 +1279,7 @@ fn handle_session_list(state: &Arc) -> ResponseBody { .into_iter() .map(|s| s.status_entry()) .collect(); - ResponseBody::Ok(serde_json::to_value(SessionListResult { sessions }).unwrap_or(Value::Null)) + ok_value(SessionListResult { sessions }) } async fn handle_browser_list(state: &Arc, params: Value) -> Result { @@ -1247,7 +1291,8 @@ async fn handle_browser_list(state: &Arc, params: Value) -> Result< // ascending, with `instance_id` as a deterministic tiebreaker // (review I1). let browsers = snapshot_status_entries(&state.browsers, &state.sessions); - Ok(serde_json::to_value(BrowserListResult { browsers }).unwrap_or(Value::Null)) + serde_json::to_value(BrowserListResult { browsers }) + .map_err(|err| serialise_err("browser list result", err)) } // ----- Test-helper IpcServer wrapper around the transport layer ----- @@ -2031,4 +2076,69 @@ mod tests { let _ = tx.send(()); let _ = server.await; } + + // A Serialize impl that always fails, standing in for a deeply + // nested payload that trips serde_json's recursion limit. + struct FailingSerialize; + impl serde::Serialize for FailingSerialize { + fn serialize(&self, _serializer: S) -> Result + where + S: serde::Serializer, + { + Err(serde::ser::Error::custom("recursion limit exceeded")) + } + } + + #[test] + fn ok_value_serialises_regular_result() { + match ok_value(PingResult { pong: true }) { + ResponseBody::Ok(v) => assert_eq!(v, serde_json::json!({ "pong": true })), + other => panic!("expected ok, got {other:?}"), + } + } + + #[test] + fn ok_value_reports_protocol_error_on_failed_serialise() { + // A serialisation failure must surface as a structured + // protocol_error instead of silently degrading to JSON null. + match ok_value(FailingSerialize) { + ResponseBody::Err(err) => { + assert_eq!(err.code, ErrorCode::ProtocolError); + assert!( + err.message.contains("failed to serialise"), + "message: {}", + err.message + ); + } + other => panic!("expected protocol_error, got {other:?}"), + } + } + + #[test] + fn serialise_err_builds_protocol_error_with_context() { + let err = serde_json::from_str::("[").unwrap_err(); + let rpc = serialise_err("status snapshot", err); + assert_eq!(rpc.code, ErrorCode::ProtocolError); + assert!( + rpc.message.contains("status snapshot"), + "message: {}", + rpc.message + ); + } + + #[test] + fn null_result_response_round_trips_over_frame_decode() { + // The daemon can legitimately emit an explicit null result; + // the CLI-side Frame decoder must treat it as a success body + // rather than an ambiguous frame (frame.rs regression guard). + let wire = r#"{"id":"rpc-null","result":null}"#; + let frame: Frame = serde_json::from_str(wire).expect("explicit null result decodes"); + match frame { + Frame::Response(resp) => { + assert_eq!(resp.id, "rpc-null"); + assert_eq!(resp.body, ResponseBody::Ok(serde_json::Value::Null)); + } + other => panic!("expected response frame, got {other:?}"), + } + } } diff --git a/crates/bsk-protocol/src/frame.rs b/crates/bsk-protocol/src/frame.rs index b80eec9b..abb0de8f 100644 --- a/crates/bsk-protocol/src/frame.rs +++ b/crates/bsk-protocol/src/frame.rs @@ -94,6 +94,14 @@ impl<'de> Deserialize<'de> for ResponseFrame { #[derive(Deserialize)] struct Flat { id: RpcId, + // `#[serde(default, deserialize_with)]` keeps a present-but-null + // `result` distinct from an absent field: serde's plain + // `Option` collapses JSON `"result": null` (a legal + // JSON-RPC null result — see the daemon's + // `serde_json::to_value(..).unwrap_or(Value::Null)` fallback in + // `bsk-cli/src/daemon/ipc.rs`) into `None`, which would then + // misclassify an explicit null result as an ambiguous frame. + #[serde(default, deserialize_with = "de_result_field")] result: Option, error: Option, } @@ -114,6 +122,22 @@ impl<'de> Deserialize<'de> for ResponseFrame { } } +/// Deserialise the `result` field of a response frame. +/// +/// A plain `Option` collapses JSON `null` into `None` +/// (serde's `Option` treats `null` as "none"), which would make an +/// explicit `{"result": null}` reply indistinguishable from a missing +/// field and reject it as ambiguous. The daemon can legitimately emit an +/// explicit null result (its `serde_json::to_value(..).unwrap_or(Value::Null)` +/// fallback), so we parse the raw `Value` here and let the caller's match +/// decide — a present-but-null result becomes `Some(Value::Null)`. +fn de_result_field<'de, D>(deserializer: D) -> Result, D::Error> +where + D: Deserializer<'de>, +{ + serde_json::Value::deserialize(deserializer).map(Some) +} + impl Serialize for Frame { fn serialize(&self, serializer: S) -> Result where @@ -152,7 +176,11 @@ impl<'de> Visitor<'de> for FrameVisitor { let mut id = None::; let mut method = None::; let mut params = None::; - let mut result = None::; + // Outer `Option` = field presence, inner = the value. `"result": + // null` parses to `Some(None)` so an explicit null result is + // distinguishable from a missing field (see `ResponseFrame`'s + // `Flat` helper for the rationale). + let mut result = None::>; let mut error = None::; let mut event = None::; let mut payload = None::; @@ -222,10 +250,14 @@ impl<'de> Visitor<'de> for FrameVisitor { let id = id.ok_or_else(|| de::Error::missing_field("id"))?; match (result, error) { - (Some(v), None) => Ok(Frame::Response(ResponseFrame { + (Some(Some(v)), None) => Ok(Frame::Response(ResponseFrame { id, body: ResponseBody::Ok(v), })), + (Some(None), None) => Ok(Frame::Response(ResponseFrame { + id, + body: ResponseBody::Ok(serde_json::Value::Null), + })), (None, Some(e)) => Ok(Frame::Response(ResponseFrame { id, body: ResponseBody::Err(e), @@ -280,4 +312,54 @@ mod tests { Some("sess-1"), ); } + + #[test] + fn explicit_null_result_decodes_as_ok_null() { + // The daemon may legitimately emit `"result": null` (its + // `serde_json::to_value(..).unwrap_or(Value::Null)` fallback in + // `bsk-cli/src/daemon/ipc.rs`). A plain `Option` would + // collapse the explicit null into "field absent" and reject the + // frame as ambiguous; the inner `Option` keeps the distinction. + let wire = serde_json::json!({ "id": "rpc-1", "result": null }); + let frame: Frame = serde_json::from_value(wire).unwrap(); + match frame { + Frame::Response(resp) => { + assert_eq!(resp.id, "rpc-1"); + assert_eq!(resp.body, ResponseBody::Ok(serde_json::Value::Null)); + } + other => panic!("expected response frame, got {other:?}"), + } + } + + #[test] + fn explicit_null_result_response_frame_decodes_as_ok_null() { + // The dedicated `ResponseFrame` deserializer must agree with the + // generic `Frame` visitor on the same wire shape. + let wire = serde_json::json!({ "id": "rpc-2", "result": null }); + let resp: ResponseFrame = serde_json::from_value(wire).unwrap(); + assert_eq!(resp.id, "rpc-2"); + assert_eq!(resp.body, ResponseBody::Ok(serde_json::Value::Null)); + } + + #[test] + fn null_result_round_trips_through_serialise() { + // Serialising `Ok(Value::Null)` emits `"result": null`; decoding + // that wire shape must produce the same body (symmetric protocol). + let frame = Frame::Response(ResponseFrame { + id: "rpc-3".into(), + body: ResponseBody::Ok(serde_json::Value::Null), + }); + let v = serde_json::to_value(&frame).unwrap(); + let back: Frame = serde_json::from_value(v).unwrap(); + assert_eq!(back, frame); + } + + #[test] + fn missing_result_and_error_still_rejected() { + // A response with neither `result` nor `error` remains ambiguous — + // only an *explicit* null result is a legal success payload. + let wire = serde_json::json!({ "id": "rpc-4" }); + assert!(serde_json::from_value::(wire.clone()).is_err()); + assert!(serde_json::from_value::(wire).is_err()); + } } From d0cd44dabafe0e57c1fe42962868ab0960cb669c Mon Sep 17 00:00:00 2001 From: cyberspace-cs Date: Sat, 19 Sep 2026 20:10:20 +0800 Subject: [PATCH 2/2] test(remote): add coverage for remote gateway server Adds 6 tests to crates/bsk-cli/src/daemon/remote/server.rs, the remote gateway module introduced in 0.3.0 which previously had no test coverage: - response_sets_json_no_store_headers_and_body: response() helper emits JSON with no-store cache-control and close connection semantics. - denied_returns_401_with_error_body: authorization failure is 401 with a structured error body. - retry_response_sets_retry_after: rate-limit responses carry retry-after. - remote_gateway_rejects_unauthenticated_requests: end-to-end over a real bound listener - missing Origin, query strings, unknown paths, non-browser methods, missing/malformed Bearer credentials and non-JSON bodies all fail closed; the authorize endpoint rate limits per peer (429 + retry-after: 60). - remote_gateway_pairs_device_over_http: pairing exchange persists a durable device grant and returns a grant token. - remote_gateway_upgrades_authorized_device: an authorized device with a matching bsk-auth. WebSocket subprotocol and extension Origin upgrades to 101. The integration tests bind a real listener and drive the gateway with reqwest and tokio-tungstenite, using the existing isolated() lifecycle helper so parallel test runs do not pollute each other's BSK_HOME. cargo test -p bsk --lib daemon::remote: 14 passed, 0 failed. --- crates/bsk-cli/src/daemon/remote/server.rs | 259 +++++++++++++++++++++ 1 file changed, 259 insertions(+) diff --git a/crates/bsk-cli/src/daemon/remote/server.rs b/crates/bsk-cli/src/daemon/remote/server.rs index d920f53f..b739584b 100644 --- a/crates/bsk-cli/src/daemon/remote/server.rs +++ b/crates/bsk-cli/src/daemon/remote/server.rs @@ -423,3 +423,262 @@ async fn handle( }); Ok(upgrade_response.map(|_| Full::new(Bytes::new()))) } + +#[cfg(test)] +mod tests { + use super::*; + use crate::daemon::remote::ServerConfig; + use crate::daemon::start::DaemonConfig; + use http_body_util::BodyExt; + + const EXTENSION_ORIGIN: &str = "chrome-extension://aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; + + fn server_config(authorize_rate_limit: u32) -> ServerConfig { + ServerConfig { + listen: "127.0.0.1".parse().unwrap(), + public_url: "wss://127.0.0.1/extension".into(), + tls_cert: None, + tls_key: None, + pairing_ttl: Duration::from_secs(300), + device_ttl: Duration::from_secs(90 * 86400), + renew_after: Duration::from_secs(30 * 86400), + max_connections: 64, + authorize_rate_limit, + } + } + + fn daemon(server: ServerConfig) -> Arc { + let mut config = DaemonConfig::new(0); + config.server = Some(server); + Arc::new(DaemonState::new(config)) + } + + async fn body_json(response: Response) -> serde_json::Value { + let bytes = response.into_body().collect().await.unwrap().to_bytes(); + serde_json::from_slice(&bytes).unwrap() + } + + #[tokio::test] + async fn response_sets_json_no_store_headers_and_body() { + let response = response(StatusCode::OK, serde_json::json!({"ok": true})); + assert_eq!(response.status(), StatusCode::OK); + assert_eq!( + response.headers().get("content-type").unwrap(), + "application/json" + ); + assert_eq!(response.headers().get("cache-control").unwrap(), "no-store"); + assert_eq!(response.headers().get("connection").unwrap(), "close"); + assert_eq!(body_json(response).await, serde_json::json!({"ok": true})); + } + + #[tokio::test] + async fn denied_returns_401_with_error_body() { + let response = denied(); + assert_eq!(response.status(), StatusCode::UNAUTHORIZED); + assert_eq!( + response.headers().get("content-type").unwrap(), + "application/json" + ); + assert_eq!( + body_json(response).await, + serde_json::json!({"error": "invalid_authorization"}) + ); + } + + #[tokio::test] + async fn retry_response_sets_retry_after() { + let response = retry_response(StatusCode::TOO_MANY_REQUESTS, "rate_limited", 60); + assert_eq!(response.status(), StatusCode::TOO_MANY_REQUESTS); + assert_eq!(response.headers().get("retry-after").unwrap(), "60"); + assert_eq!( + body_json(response).await, + serde_json::json!({"error": "rate_limited"}) + ); + } + + #[test] + fn remote_gateway_rejects_unauthenticated_requests() { + crate::daemon::test_support::isolated( + "daemon::remote::server::tests::remote_gateway_rejects_unauthenticated_requests", + || { + let rt = tokio::runtime::Builder::new_multi_thread() + .enable_all() + .build() + .unwrap(); + rt.block_on(async { + let state = daemon(server_config(2)); + let handle = bind(state, "127.0.0.1:0".parse().unwrap()).await.unwrap(); + let base = format!("http://{}", handle.local_addr); + let client = reqwest::Client::new(); + + // Missing Origin on the browser path fails closed. + let res = client + .get(format!("{base}/extension")) + .send() + .await + .unwrap(); + assert_eq!(res.status().as_u16(), 401); + + // Query strings are rejected outright. + let res = client + .get(format!("{base}/extension?device=1")) + .send() + .await + .unwrap(); + assert_eq!(res.status().as_u16(), 401); + + // Unknown paths and non-browser methods are 404. + let res = client.get(format!("{base}/other")).send().await.unwrap(); + assert_eq!(res.status().as_u16(), 404); + let res = client + .post(format!("{base}/extension")) + .send() + .await + .unwrap(); + assert_eq!(res.status().as_u16(), 404); + + // Missing and malformed credentials are denied. + let res = client + .post(format!("{base}/extension/authorize")) + .body("{}") + .send() + .await + .unwrap(); + assert_eq!(res.status().as_u16(), 401); + let res = client + .post(format!("{base}/extension/authorize")) + .header("authorization", "Bearer bad token with spaces") + .body("{}") + .send() + .await + .unwrap(); + assert_eq!(res.status().as_u16(), 401); + + // Non-JSON bodies are rejected with 400. + let res = client + .post(format!("{base}/extension/authorize")) + .header("authorization", format!("Bearer {}", "a".repeat(43))) + .body("this is not json") + .send() + .await + .unwrap(); + assert_eq!(res.status().as_u16(), 400); + + // The authorize endpoint is rate limited per peer. + let res = client + .post(format!("{base}/extension/authorize")) + .header("authorization", format!("Bearer {}", "a".repeat(43))) + .body("{}") + .send() + .await + .unwrap(); + assert_eq!(res.status().as_u16(), 429); + assert_eq!(res.headers().get("retry-after").unwrap(), "60"); + + handle.shutdown.notify_waiters(); + let _ = handle.task.await; + }); + }, + ); + } + + #[test] + fn remote_gateway_pairs_device_over_http() { + crate::daemon::test_support::isolated( + "daemon::remote::server::tests::remote_gateway_pairs_device_over_http", + || { + let rt = tokio::runtime::Builder::new_multi_thread() + .enable_all() + .build() + .unwrap(); + rt.block_on(async { + let state = daemon(server_config(60)); + let handle = bind(state, "127.0.0.1:0".parse().unwrap()).await.unwrap(); + let base = format!("http://{}", handle.local_addr); + let client = reqwest::Client::new(); + + let store = AuthorizationStore::at_home(&paths::bsk_home().unwrap()); + let link = store.pair().unwrap().rsplit_once('#').unwrap().1.to_owned(); + let next = "b".repeat(43); + let res = client + .post(format!("{base}/extension/authorize")) + .header("authorization", format!("Bearer {link}")) + .header("content-type", "application/json") + .body( + serde_json::json!({ + "action": "pair", + "next_token": next, + "label": "Browser\nlabel" + }) + .to_string(), + ) + .send() + .await + .unwrap(); + assert_eq!(res.status().as_u16(), 200); + let grant: serde_json::Value = + serde_json::from_str(&res.text().await.unwrap()).unwrap(); + assert_eq!(grant["action"], "pair"); + assert!(!grant["device_id"].as_str().unwrap().is_empty()); + assert_eq!(store.devices().unwrap().len(), 1); + + handle.shutdown.notify_waiters(); + let _ = handle.task.await; + }); + }, + ); + } + + #[test] + fn remote_gateway_upgrades_authorized_device() { + crate::daemon::test_support::isolated( + "daemon::remote::server::tests::remote_gateway_upgrades_authorized_device", + || { + let rt = tokio::runtime::Builder::new_multi_thread() + .enable_all() + .build() + .unwrap(); + rt.block_on(async { + let state = daemon(server_config(60)); + let handle = bind(state, "127.0.0.1:0".parse().unwrap()).await.unwrap(); + + let store = AuthorizationStore::at_home(&paths::bsk_home().unwrap()); + let credential = "c".repeat(43); + store + .exchange( + &store.pair().unwrap().rsplit_once('#').unwrap().1.to_owned(), + AuthorizationRequest { + action: "pair".into(), + next_token: credential.clone(), + label: "Browser\nlabel".into(), + }, + ) + .unwrap(); + + let request = Request::builder() + .uri(format!("ws://{}/extension", handle.local_addr)) + .header("origin", EXTENSION_ORIGIN) + .header("sec-websocket-protocol", format!("bsk-auth.{credential}")) + .body(()) + .unwrap(); + let tcp = tokio::net::TcpStream::connect(handle.local_addr) + .await + .unwrap(); + let (ws, response) = + tokio_tungstenite::client_async_with_config(request, tcp, None) + .await + .unwrap(); + assert_eq!(response.status().as_u16(), 101); + assert_eq!( + response.headers().get("sec-websocket-protocol").unwrap(), + &format!("bsk-auth.{credential}") + ); + drop(ws); + + handle.shutdown.notify_waiters(); + let _ = handle.task.await; + }); + }, + ); + } +}