Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion desktop/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
"lint": "biome lint .",
"check": "biome check . && pnpm check:file-sizes && pnpm check:px-text && pnpm check:pubkey-truncation",
"format": "biome format --write .",
"test": "node --import ./test-loader.mjs --experimental-strip-types --test \"src/**/*.test.mjs\"",
"test": "node --import ./test-loader.mjs --experimental-strip-types --test \"src/**/*.test.mjs\" && node --import ./test-jsdom-setup.mjs --import ./test-loader.mjs --experimental-strip-types --test-force-exit --test \"src/**/*.jsdom-test.mjs\"",
"preview": "vite preview",
"tauri": "tauri",
"test:e2e": "pnpm build:e2e && playwright test",
Expand Down
2 changes: 2 additions & 0 deletions desktop/scripts/check-pubkey-truncation.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ const overrides = new Set([
"src/features/messages/lib/threadPanel.ts:395",
"src/features/projects/ui/ProjectsView.tsx:166",
"src/features/projects/ui/ProjectsOverviewPanel.tsx:209",
// Error message prefix in a console-internal action error (never rendered as identity).
"src/features/admin-console/AdminConsoleStaffingTab.tsx:108",
]);

await runPubkeyTruncationCheck({
Expand Down
100 changes: 100 additions & 0 deletions desktop/src-tauri/src/commands/admin/client.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
//! Dedicated no-redirect HTTP client for admin API requests.
//!
//! A separate client (not the app-wide `http_client`) ensures that:
//! - 3xx responses are surfaced as errors rather than followed — preventing
//! redirect-hop SSRF where a relay-issued redirect could forward the NIP-98
//! `Authorization` header to an off-origin host.
//! - Timeouts are tuned for synchronous UI feedback rather than media downloads.

use std::sync::OnceLock;

/// Request timeout for admin API calls.
pub(crate) const ADMIN_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(30);

/// The module-level singleton admin HTTP client.
///
/// Built once via `OnceLock` — panics on build failure so there is no
/// silent fallback to a redirect-following client.
pub static ADMIN_CLIENT: OnceLock<reqwest::Client> = OnceLock::new();

/// Initialise the admin client singleton. Must be called from `setup()` before
/// any admin command can be invoked. Subsequent calls are no-ops.
pub fn init_admin_client() {
ADMIN_CLIENT.get_or_init(|| {
reqwest::Client::builder()
.resolve("localhost", std::net::SocketAddr::from(([127, 0, 0, 1], 0)))
.pool_idle_timeout(std::time::Duration::from_secs(10))
.pool_max_idle_per_host(2)
.redirect(reqwest::redirect::Policy::none())
.timeout(ADMIN_TIMEOUT)
.build()
.expect(
"admin HTTP client must build with redirect::Policy::none(); \
a redirect-following fallback would forward the NIP-98 \
Authorization header across origins (redirect-hop SSRF)",
)
});
}

#[cfg(test)]
mod tests {
use super::*;

/// The admin client must be buildable and must refuse to follow redirects.
/// This mirrors the `build_media_fetch_client_succeeds_with_no_redirect_policy`
/// test in `media_download.rs`.
#[test]
fn admin_client_builds_with_no_redirect_policy() {
init_admin_client();
assert!(ADMIN_CLIENT.get().is_some());
}

/// A live test that the client does not follow a 302.
///
/// Mirrors `media_fetch_client_does_not_follow_redirects` in
/// `media_download.rs`. Serves a 302 pointing at the metadata endpoint
/// and asserts exactly one connection was accepted.
#[tokio::test]
async fn admin_client_does_not_follow_redirects() {
use std::io::{Read, Write};
use std::sync::atomic::{AtomicUsize, Ordering};
use std::sync::Arc;

init_admin_client();
let client = ADMIN_CLIENT.get().expect("client initialised");

let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap();
let addr = listener.local_addr().unwrap();
let connections = Arc::new(AtomicUsize::new(0));

let server_connections = Arc::clone(&connections);
let server = std::thread::spawn(move || {
if let Ok((mut stream, _)) = listener.accept() {
server_connections.fetch_add(1, Ordering::SeqCst);
let mut buf = [0u8; 1024];
let _ = stream.read(&mut buf);
let response = "HTTP/1.1 302 Found\r\n\
Location: http://169.254.169.254/latest/meta-data/\r\n\
Content-Length: 0\r\n\
Connection: close\r\n\r\n";
let _ = stream.write_all(response.as_bytes());
let _ = stream.flush();
}
});

let resp = client
.get(format!("http://{addr}/api/admin/v1/reports"))
.timeout(std::time::Duration::from_secs(5))
.send()
.await
.expect("request should complete without following the redirect");

assert_eq!(resp.status().as_u16(), 302);
server.join().unwrap();
assert_eq!(
connections.load(Ordering::SeqCst),
1,
"exactly one request must be issued — redirect must not be followed",
);
}
}
69 changes: 69 additions & 0 deletions desktop/src-tauri/src/commands/admin/error.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
//! Typed error for admin mutation commands.

/// Error from an admin mutation command, carrying whether the relay
/// authoritatively answered so the UI can decide idempotency-retry policy
/// without string-matching the message.
///
/// `relayStatus` is `Some(code)` only when the relay returned an HTTP status —
/// the request reached the relay and it answered. It is `None` for a
/// pre-response transport failure (`send()` error, DNS/connect/timeout) or a
/// pre-send failure (auth build, body serialisation): the relay never
/// committed anything, so a retry must reuse the same idempotency key.
///
/// `bodyComplete` is `true` only when the relay's full response body was read —
/// an authoritative verdict. A non-409 4xx with `bodyComplete: true` is a
/// definitive pre-commit rejection, so the UI may mint a fresh idempotency key.
/// A status that arrives but whose body is lost mid-stream (or rejected over
/// the size cap) carries `bodyComplete: false`: the outcome is unknown, so the
/// caller preserves idempotency and lets the retry dedupe even on a 4xx.
///
/// Serialises `rename_all = "camelCase"`; the JS bridge surfaces it as the
/// rejected `TauriInvokeError.payload`, from which the UI reads `relayStatus`
/// and `bodyComplete`.
#[derive(Debug, serde::Serialize)]
#[serde(rename_all = "camelCase")]
pub struct AdminMutationError {
/// Human-readable message — byte-identical to the string the command
/// produced before typing, so existing message parsing is unaffected.
pub message: String,
/// The relay's HTTP status when a response was received; `None` for a
/// transport/pre-send failure where no relay answer exists.
pub relay_status: Option<u16>,
/// Whether the relay's full response body was read. `true` only for an
/// authoritative verdict; `false` when the body was lost or truncated.
pub body_complete: bool,
}

impl AdminMutationError {
/// The relay answered with an HTTP status and its full body was read — an
/// authoritative verdict.
pub(super) fn authoritative(status: reqwest::StatusCode, message: String) -> Self {
Self {
message,
relay_status: Some(status.as_u16()),
body_complete: true,
}
}

/// The relay answered with an HTTP status but the body was not fully read
/// (redirect, over-cap, or a mid-stream read failure) — outcome unknown.
pub(super) fn partial(status: reqwest::StatusCode, message: String) -> Self {
Self {
message,
relay_status: Some(status.as_u16()),
body_complete: false,
}
}
}

/// Pre-send and transport failures carry no relay status: the relay never saw
/// the request (or never answered), so the outcome is unambiguously "no commit".
impl From<String> for AdminMutationError {
fn from(message: String) -> Self {
Self {
message,
relay_status: None,
body_complete: false,
}
}
}
Loading