External/buzz
1
0
Fork 0
mirror of https://github.com/block/buzz.git synced 2026-08-18 06:50:31 +02:00
Code Issues Packages Projects Releases Wiki Activity

fix(relay): omit a version clients cannot bump, and drop the forgeable counter

Browse Source 
Review of #4543 found two problems with how the new client identity
reached Prometheus. Both are fixed here, and the parser and sender lost
the hand-rolled code the review flagged as removable.

Only a real release version is reported. Every sender passed
`env!("CARGO_PKG_VERSION")`, but only the desktop app has an
independently bumped version: `buzz-ws-client` and `buzz-acp` use
`version.workspace = true`, and the workspace version has never been
bumped, because `RELEASING.md` "Version Sources" gives release authority
only to the desktop manifests and `crates/buzz-relay/Cargo.toml`. So CLI
and harness connections would have reported a fixed version forever and
a dashboard would have read that as "nobody ever upgrades". `app_version`
is now `Option<&str>`; those clients pass `None`, the `app-version`
member is omitted from the header, and the relay labels the version
`unknown`, which is accurate. A version that is *present* but unusable is
still a parse failure, so a genuinely broken version stays visible. A
wrong version is worse than no version.

Only a gauge is emitted. `buzz_client_connections_total` is removed. The
header arrives before NIP-42 AUTH and is forgeable, and the recorder's
`idle_timeout` is configured for `MetricKindMask::GAUGE` only, so counter
series would have been retained for the process lifetime while gauge
series self-clean. Verified with a throwaway probe: 10,000 forged headers
produced 10,000 series; after the idle timeout the counter kept all
10,000 and the gauge went to 0. The label alphabet was never the real
bound — the metric kind is. Connection rate is left to the existing
`buzz_ws_connections_total`, which is not attacker-labeled.

Also, per review:

- `may_identify_to` takes a parsed `url::Url` and matches on scheme plus
  `url::Host`, replacing hand-rolled scheme splitting, userinfo
  stripping, and IPv6 bracket handling. Both callers already parsed the
  URL, so they now parse once. `url` was already a dependency.
- The three duplicated label emitters collapse into one `active_gauge`.
- `app_version_detail`, `MAX_LOGGED_VALUE_LEN`, and `truncate_for_log`
  are gone; the connection log uses the label-safe version.
- `MAX_HEADER_LEN` and `MAX_APP_VERSION_LEN` are single constants
  exported from `buzz-core`, replacing the relay's divergent 512/32. A
  test proves the longest header the builder can emit still parses, so
  the two halves cannot drift into counting real clients as failures.

`deploy/` is still a zero diff and the HPA series is still unlabeled.

Signed-off-by: npub1x3mmseqygyar04742djuepgk0t2d2t4chzm9sl0hl4vlc2m9whvqza9e5y <3477b86404413a37d7d55365cc85167ad4d52eb8b8b6587df7fd59fc2b6575d8@buzz.block.builderlab.xyz>
Co-authored-by: Atish Patel <atish@squareup.com>
Signed-off-by: Atish Patel <atish@squareup.com>
This commit is contained in:
npub1x3mmseqygyar04742djuepgk0t2d2t4chzm9sl0hl4vlc2m9whvqza9e5y
2026-08-12 16:06:22 -05:00
committed by Brainy Bumble
co-authored by Atish Patel
parent 48d45cf7f4
commit 1bc57aebf8
6 changed files with 291 additions and 213 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/buzz-acp/src/relay.rs
+7 -5
View File
@@ -3867,17 +3867,19 @@ async fn do_connect(
.map_err(|e| RelayError::Http(format!("invalid relay URL: {e}")))?;
// Advisory `Buzz-Client` identity so the relay can attribute this
// connection to the harness and its version. Best-effort: an unshipped
// platform or a destination we must not identify to simply connects
// without the header.
// connection to the harness. Best-effort: an unshipped platform or a
// destination we must not identify to simply connects without the header.
// No version is sent — buzz-acp inherits the workspace version, which is
// never bumped (see `RELEASING.md`), so reporting it would pin every
// harness connection to a fixed number forever.
let request = {
let uri = parsed
.as_str()
.parse()
.map_err(|e| RelayError::Http(format!("invalid relay URL: {e}")))?;
let builder = ClientRequestBuilder::new(uri);
match may_identify_to(parsed.as_str())
.then(|| client_header_value_for_host(ClientApp::Acp, env!("CARGO_PKG_VERSION")))
match may_identify_to(&parsed)
.then(|| client_header_value_for_host(ClientApp::Acp, None))
.flatten()
{
Some(value) => builder.with_header(CLIENT_HEADER, value),
crates/buzz-core/src/client_identity.rs
+137 -83
View File
@@ -9,6 +9,17 @@
//! Buzz-Client: v=1, app=buzz-desktop, platform=macos, app-version="0.5.2"
//! ```
//!
//! # Only a real release version may be sent
//!
//! `app_version` is [`Option`] because not every client has a version worth
//! publishing. Only the desktop app and the relay carry independently bumped
//! release versions (`RELEASING.md`, "Version Sources"); crates that inherit
//! `version.workspace = true` sit at a workspace version that has never been
//! bumped, so `env!("CARGO_PKG_VERSION")` would pin them to a fixed number
//! forever and a dashboard would read that as "nobody ever upgrades". Those
//! clients pass `None` and are reported with an `unknown` version instead,
//! which is accurate. A wrong version is worse than no version.
//!
//! This module owns only the *sending* half: the canonical header name, the
//! app/platform vocabularies, and the serializer. The relay parses the header
//! with its own lenient reader (`buzz-relay`'s `client_info`) and treats it as
@@ -25,6 +36,8 @@
use std::fmt::Write as _;
use url::{Host, Url};
/// Canonical header name. Lowercase so it can be used directly as an HTTP/2
/// field name and compared against `http::HeaderName` without reallocating.
pub const CLIENT_HEADER: &str = "buzz-client";
@@ -126,57 +139,68 @@ impl ClientPlatform {
}
}
/// Maximum serialized header length. A Buzz-generated value is far shorter;
/// this only bounds a pathological `app_version` before it reaches the wire.
const MAX_HEADER_LEN: usize = 256;
/// Longest `Buzz-Client` header value either side will handle, in bytes.
///
/// One constant for both halves of the contract: the sender refuses to emit a
/// longer value and the relay refuses to parse one, so a value that fits on
/// the wire is always one the relay will read. A Buzz-generated header is well
/// under 128 bytes; this only bounds parser work on a hostile input.
pub const MAX_HEADER_LEN: usize = 256;
/// Longest accepted `app-version`, matching the relay's own bound.
const MAX_APP_VERSION_LEN: usize = 32;
/// Longest accepted `app-version`, in bytes.
///
/// Shared with the sender so both halves of the contract agree on the bound.
pub const MAX_APP_VERSION_LEN: usize = 32;
/// Build the `Buzz-Client` header value for this client.
///
/// Returns `None` when the platform is not one Buzz ships, or when
/// `app_version` is empty or not a plausible version string. A missing header
/// is a supported state on the relay, so refusing to send is always safe —
/// callers must never substitute a placeholder.
/// Pass `app_version` only when the client has a real, independently bumped
/// release version; pass `None` otherwise, and the `app-version` member is
/// omitted so the relay reports the version as unknown rather than as a
/// misleading constant. An `app_version` that is not a plausible version
/// string is treated the same as `None`.
///
/// # Examples
///
/// ```
/// use buzz_core::client_identity::{client_header_value, ClientApp, ClientPlatform};
///
/// let value = client_header_value(ClientApp::Desktop, ClientPlatform::MacOs, "0.5.2").unwrap();
/// let value = client_header_value(ClientApp::Desktop, ClientPlatform::MacOs, Some("0.5.2"));
/// assert_eq!(value, r#"v=1, app=buzz-desktop, platform=macos, app-version="0.5.2""#);
///
/// // A client with no real release version still reports app and platform.
/// let value = client_header_value(ClientApp::Cli, ClientPlatform::Linux, None);
/// assert_eq!(value, "v=1, app=buzz-cli, platform=linux");
/// ```
#[must_use]
pub fn client_header_value(
app: ClientApp,
platform: ClientPlatform,
app_version: &str,
) -> Option<String> {
// Only horizontal whitespace is trimmed. Trimming CR/LF would silently
// sanitize a header-injection attempt into an accepted value; those are
// rejected by `is_serializable_app_version` instead.
let app_version = app_version.trim_matches([' ', '\t']);
if !is_serializable_app_version(app_version) {
return None;
}
app_version: Option<&str>,
) -> String {
let mut value = String::with_capacity(64);
// Field order matches the wire example and the relay's tests; RFC 8941
// dictionaries are order-independent, so this is presentation only.
let _ = write!(
value,
"v={CLIENT_HEADER_FORMAT_VERSION}, app={}, platform={}, app-version=\"{app_version}\"",
"v={CLIENT_HEADER_FORMAT_VERSION}, app={}, platform={}",
app.as_str(),
platform.as_str(),
);
// Unreachable for a validated version, but never emit an oversized header.
if value.len() > MAX_HEADER_LEN {
return None;
// Only horizontal whitespace is trimmed. Trimming CR/LF would silently
// sanitize a header-injection attempt into an accepted value; those are
// rejected by `is_serializable_app_version` instead.
if let Some(app_version) = app_version
.map(|raw| raw.trim_matches([' ', '\t']))
.filter(|raw| is_serializable_app_version(raw))
{
let _ = write!(value, ", app-version=\"{app_version}\"");
}
Some(value)
// Unreachable for a validated version, but never emit an oversized header.
debug_assert!(value.len() <= MAX_HEADER_LEN, "{value}");
value
}
/// Build the header for the current compile target, or `None` on an unshipped
@@ -185,8 +209,12 @@ pub fn client_header_value(
/// This is the entry point clients should use; it removes the chance of a
/// caller hardcoding the wrong platform for a build.
#[must_use]
pub fn client_header_value_for_host(app: ClientApp, app_version: &str) -> Option<String> {
client_header_value(app, ClientPlatform::current()?, app_version)
pub fn client_header_value_for_host(app: ClientApp, app_version: Option<&str>) -> Option<String> {
Some(client_header_value(
app,
ClientPlatform::current()?,
app_version,
))
}
/// Whether `url` is a destination this client may identify itself to.
@@ -198,51 +226,35 @@ pub fn client_header_value_for_host(app: ClientApp, app_version: &str) -> Option
/// origin would leak the header to any on-path observer. Loopback is exempted
/// so local development still exercises the same code path.
///
/// Takes a parsed [`Url`] so host extraction — userinfo, IPv6 literals, ports,
/// percent-encoding — is the `url` crate's problem rather than this module's.
///
/// # Examples
///
/// ```
/// use buzz_core::client_identity::may_identify_to;
/// use url::Url;
///
/// assert!(may_identify_to("wss://buzz.example.com/"));
/// assert!(may_identify_to("ws://127.0.0.1:8080/"));
/// assert!(!may_identify_to("ws://buzz.example.com/"));
/// assert!(may_identify_to(&Url::parse("wss://buzz.example.com/").unwrap()));
/// assert!(may_identify_to(&Url::parse("ws://127.0.0.1:8080/").unwrap()));
/// assert!(!may_identify_to(&Url::parse("ws://buzz.example.com/").unwrap()));
/// ```
#[must_use]
pub fn may_identify_to(url: &str) -> bool {
let Some((scheme, rest)) = url.split_once("://") else {
return false;
};
match scheme.to_ascii_lowercase().as_str() {
pub fn may_identify_to(url: &Url) -> bool {
match url.scheme() {
// TLS: the header is only visible to the relay itself.
"wss" | "https" => true,
// Cleartext is acceptable only when it cannot leave the machine.
"ws" | "http" => is_loopback_authority(rest),
"ws" | "http" => match url.host() {
Some(Host::Domain(domain)) => domain.eq_ignore_ascii_case("localhost"),
Some(Host::Ipv4(ip)) => ip.is_loopback(),
Some(Host::Ipv6(ip)) => ip.is_loopback(),
None => false,
},
_ => false,
}
}
/// Whether an authority (`host[:port]`, possibly followed by a path) is
/// loopback.
fn is_loopback_authority(rest: &str) -> bool {
// Trim the path/query/fragment, then any `userinfo@` prefix.
let authority = rest.split(['/', '?', '#']).next().unwrap_or("");
let authority = authority
.rsplit_once('@')
.map_or(authority, |(_userinfo, host)| host);
let host = match authority.strip_prefix('[') {
// IPv6 literal: `[::1]:port`.
Some(inner) => inner.split(']').next().unwrap_or(""),
None => authority.split(':').next().unwrap_or(""),
};
if host.eq_ignore_ascii_case("localhost") {
return true;
}
// Parse as an address rather than prefix-matching "127.": a name like
// `127.0.0.1.example.com` shares the prefix but is a remote host.
host.parse::<std::net::IpAddr>()
.is_ok_and(|ip| ip.is_loopback())
}
/// Whether `app_version` is safe to place inside an RFC 8941 quoted string.
///
/// RFC 8941 quoted strings admit only printable ASCII, and `"`/`\` would need
@@ -269,14 +281,27 @@ mod tests {
#[test]
fn serializes_the_documented_wire_format() {
let value =
client_header_value(ClientApp::Desktop, ClientPlatform::MacOs, "0.5.2").unwrap();
let value = client_header_value(ClientApp::Desktop, ClientPlatform::MacOs, Some("0.5.2"));
assert_eq!(
value,
r#"v=1, app=buzz-desktop, platform=macos, app-version="0.5.2""#
);
}
#[test]
fn omits_app_version_when_the_client_has_no_release_version() {
// buzz-cli and buzz-acp inherit the never-bumped workspace version, so
// they send no version at all rather than a misleading constant.
assert_eq!(
client_header_value(ClientApp::Cli, ClientPlatform::Linux, None),
"v=1, app=buzz-cli, platform=linux"
);
assert_eq!(
client_header_value(ClientApp::Acp, ClientPlatform::MacOs, None),
"v=1, app=buzz-acp, platform=macos"
);
}
#[test]
fn app_and_platform_tokens_are_stable() {
// These strings are Prometheus label values and a cross-repo wire
@@ -321,17 +346,20 @@ mod tests {
}
#[test]
fn rejects_versions_that_are_not_versions() {
fn drops_versions_that_are_not_versions() {
// A placeholder must never be published as a version label; the header
// is still sent so app and platform are not lost.
for bad in ["", " ", "unknown", "dev", "v1.2.3", "nightly"] {
assert!(
client_header_value(ClientApp::Cli, ClientPlatform::Linux, bad).is_none(),
"expected {bad:?} to be refused"
assert_eq!(
client_header_value(ClientApp::Cli, ClientPlatform::Linux, Some(bad)),
"v=1, app=buzz-cli, platform=linux",
"expected {bad:?} to be dropped"
);
}
}
#[test]
fn rejects_versions_that_would_break_the_quoted_string() {
fn drops_versions_that_would_break_the_quoted_string() {
// A quote or backslash would need RFC 8941 escaping; a newline would
// allow header injection. All must be refused, never escaped.
for bad in [
@@ -343,37 +371,51 @@ mod tests {
"0.5.2\u{7f}",
"0.5.2é",
] {
assert!(
client_header_value(ClientApp::Desktop, ClientPlatform::MacOs, bad).is_none(),
"expected {bad:?} to be refused"
let value = client_header_value(ClientApp::Desktop, ClientPlatform::MacOs, Some(bad));
assert_eq!(
value, "v=1, app=buzz-desktop, platform=macos",
"expected {bad:?} to be dropped"
);
}
}
#[test]
fn rejects_an_absurdly_long_version() {
fn drops_an_absurdly_long_version() {
let long = format!("0.{}", "9".repeat(MAX_APP_VERSION_LEN));
assert!(long.len() > MAX_APP_VERSION_LEN);
assert!(client_header_value(ClientApp::Cli, ClientPlatform::Linux, &long).is_none());
assert_eq!(
client_header_value(ClientApp::Cli, ClientPlatform::Linux, Some(&long)),
"v=1, app=buzz-cli, platform=linux"
);
}
#[test]
fn accepts_prerelease_and_build_metadata() {
let value = client_header_value(ClientApp::Cli, ClientPlatform::Linux, "1.2.3-rc.1+build9")
.unwrap();
let value = client_header_value(
ClientApp::Cli,
ClientPlatform::Linux,
Some("1.2.3-rc.1+build9"),
);
assert!(value.ends_with(r#"app-version="1.2.3-rc.1+build9""#));
}
#[test]
fn trims_surrounding_spaces_and_tabs_before_validating() {
let value = client_header_value(ClientApp::Desktop, ClientPlatform::Windows, " \t0.5.2\t ")
.unwrap();
let value = client_header_value(
ClientApp::Desktop,
ClientPlatform::Windows,
Some(" \t0.5.2\t "),
);
assert_eq!(
value,
r#"v=1, app=buzz-desktop, platform=windows, app-version="0.5.2""#
);
}
fn url(raw: &str) -> Url {
Url::parse(raw).unwrap_or_else(|e| panic!("{raw:?} is not a URL: {e}"))
}
#[test]
fn identifies_only_to_tls_or_loopback_origins() {
for allowed in [
@@ -383,11 +425,15 @@ mod tests {
"https://buzz.example.com/",
// Cleartext loopback cannot leave the machine.
"ws://localhost:8080/",
"ws://LOCALHOST:8080/",
"ws://127.0.0.1:8080/",
"ws://127.3.2.1/",
"ws://[::1]:8080/",
] {
assert!(may_identify_to(allowed), "expected {allowed:?} allowed");
assert!(
may_identify_to(&url(allowed)),
"expected {allowed:?} allowed"
);
}
}
@@ -403,12 +449,14 @@ mod tests {
"ws://127.0.0.1.example.com/",
// Loopback in userinfo must not fool the host check.
"ws://localhost@evil.example.com/",
// Non-WebSocket and malformed destinations.
// Non-WebSocket destinations.
"file:///etc/passwd",
"buzz.example.com",
"",
"ftp://buzz.example.com/",
] {
assert!(!may_identify_to(refused), "expected {refused:?} refused");
assert!(
!may_identify_to(&url(refused)),
"expected {refused:?} refused"
);
}
}
@@ -417,8 +465,12 @@ mod tests {
// Tests only run on targets Buzz ships, so `current()` is `Some` here.
let platform = ClientPlatform::current().expect("test host is a shipped platform");
assert_eq!(
client_header_value_for_host(ClientApp::Cli, "0.1.0"),
client_header_value(ClientApp::Cli, platform, "0.1.0")
client_header_value_for_host(ClientApp::Cli, Some("0.1.0")),
Some(client_header_value(ClientApp::Cli, platform, Some("0.1.0")))
);
assert_eq!(
client_header_value_for_host(ClientApp::Cli, None),
Some(client_header_value(ClientApp::Cli, platform, None))
);
}
@@ -437,8 +489,10 @@ mod tests {
ClientPlatform::Ios,
ClientPlatform::Android,
] {
let value = client_header_value(app, platform, "10.20.30-rc.1").unwrap();
assert!(value.len() <= MAX_HEADER_LEN, "{value}");
for version in [None, Some("10.20.30-rc.1")] {
let value = client_header_value(app, platform, version);
assert!(value.len() <= MAX_HEADER_LEN, "{value}");
}
}
}
}
crates/buzz-relay/src/client_info.rs
+107 -102
View File
@@ -17,14 +17,23 @@
//! is a normal, supported state; a malformed one is counted and discarded. No
//! input on this path can ever cause a connection to be rejected.
//!
//! # Cardinality is bounded by construction
//! # Cardinality
//!
//! Label values are the guard against a forged header exploding Prometheus
//! series. `app` and `platform` are resolved to `&'static str` from closed
//! allowlists — an unrecognized token yields no label, never a passthrough of
//! `app` and `platform` are resolved to `&'static str` from closed allowlists,
//! so an unrecognized token yields no label rather than a passthrough of
//! attacker-controlled bytes. `app_version` is narrowed to `MAJOR.MINOR` with
//! both components bounded in length, so the worst case is
//! `apps × platforms × plausible versions`.
//! each component capped at [`MAX_VERSION_COMPONENT_LEN`] digits, which still
//! leaves a large *reachable* label alphabet from a forgeable header.
//!
//! The bound that matters in practice is therefore not the alphabet but the
//! metric kind: this module emits **only a gauge**. The Prometheus recorder is
//! configured with an idle timeout for `MetricKindMask::GAUGE`
//! (`metrics::install`), so a label set that stops being emitted is dropped
//! from the registry. A burst of forged versions costs memory for one idle
//! timeout window and then self-cleans. A counter would have to be retained
//! for the process lifetime, so connection *rate* is deliberately left to the
//! existing `buzz_ws_connections_total` rather than duplicated here with a
//! forgeable label set.
//!
//! # Why a hand-written parser
//!
@@ -36,35 +45,27 @@
use axum::http::HeaderMap;
use buzz_core::client_identity::{
ClientApp, ClientPlatform, CLIENT_HEADER, CLIENT_HEADER_FORMAT_VERSION,
ClientApp, ClientPlatform, CLIENT_HEADER, CLIENT_HEADER_FORMAT_VERSION, MAX_APP_VERSION_LEN,
MAX_HEADER_LEN,
};
/// Label value for connections with no usable `Buzz-Client` header.
/// Label value used when a dimension is unknown.
///
/// Emitting an explicit bucket rather than omitting the series is what lets
/// `sum(buzz_client_connections_total)` reconcile with
/// `sum(buzz_ws_connections_total)`, and puts "unidentified" on a dashboard as
/// a visible line instead of a silent gap.
/// Emitting an explicit bucket rather than omitting the series puts
/// "unidentified" on a dashboard as a visible line instead of a silent gap,
/// and lets `sum(buzz_client_connections_active)` reconcile with
/// `buzz_ws_connections_active`. Used both for a wholly unidentified
/// connection and for a client that identified itself but has no real release
/// version to report (`buzz-cli` and `buzz-acp` inherit a workspace version
/// that is never bumped, so they deliberately send no `app-version`).
pub const UNKNOWN_LABEL: &str = "unknown";
/// Longest `Buzz-Client` header the relay will parse, in bytes.
///
/// A Buzz-generated value is well under 128 bytes. This bounds parser work on
/// a hostile input before any allocation.
const MAX_HEADER_LEN: usize = 512;
/// Longest accepted `MAJOR` or `MINOR` component of `app-version`.
///
/// Five digits admits every plausible version while capping the label
/// alphabet at 10^5 values per component.
const MAX_VERSION_COMPONENT_LEN: usize = 5;
/// Longest raw value retained for logging.
///
/// Log fields are not Prometheus labels, so they need no allowlist — only a
/// length bound so a hostile header cannot bloat a log line.
const MAX_LOGGED_VALUE_LEN: usize = 32;
/// Why a present `Buzz-Client` header could not be used.
///
/// Rendered as a bounded `reason` label on
@@ -81,7 +82,7 @@ enum ParseFailure {
UnknownApp,
/// `platform` was absent or outside the allowlist.
UnknownPlatform,
/// `app-version` was absent or not a bounded `MAJOR.MINOR[...]`.
/// `app-version` was present but not a bounded `MAJOR.MINOR[...]`.
BadAppVersion,
}
@@ -109,10 +110,9 @@ pub struct ClientInfo {
app: &'static str,
/// Allowlisted platform token.
platform: &'static str,
/// `MAJOR.MINOR`, safe as a metric label.
app_version: String,
/// Exact version as sent, for logs only — never a label.
app_version_detail: String,
/// `MAJOR.MINOR`, safe as a metric label, or `None` when the client sent
/// no version.
app_version: Option<String>,
}
impl ClientInfo {
@@ -195,14 +195,19 @@ impl ClientInfo {
})
.ok_or(ParseFailure::UnknownPlatform)?;
let app_version_raw = app_version.ok_or(ParseFailure::BadAppVersion)?;
let app_version = major_minor(app_version_raw).ok_or(ParseFailure::BadAppVersion)?;
// An absent `app-version` is valid: clients without an independently
// bumped release version omit it rather than send a constant. A
// *present* one that is unusable is still a failure, so a genuinely
// broken version is visible rather than silently downgraded.
let app_version = match app_version {
Some(raw) => Some(major_minor(raw).ok_or(ParseFailure::BadAppVersion)?),
None => None,
};
Ok(Self {
app,
platform,
app_version,
app_version_detail: truncate_for_log(app_version_raw),
})
}
@@ -218,71 +223,47 @@ impl ClientInfo {
self.platform
}
/// `MAJOR.MINOR` version, safe as a metric label.
/// `MAJOR.MINOR` version, or [`UNKNOWN_LABEL`] when the client sent none.
#[must_use]
pub fn app_version(&self) -> &str {
&self.app_version
}
/// Exact version as sent, for logs only.
#[must_use]
pub fn app_version_detail(&self) -> &str {
&self.app_version_detail
self.app_version.as_deref().unwrap_or(UNKNOWN_LABEL)
}
}
/// The three label values for a connection, using [`UNKNOWN_LABEL`] when the
/// client did not identify itself.
fn labels(info: Option<&ClientInfo>) -> (&str, &str, &str) {
match info {
Some(info) => (info.app, info.platform, info.app_version.as_str()),
None => (UNKNOWN_LABEL, UNKNOWN_LABEL, UNKNOWN_LABEL),
}
}
/// Count a newly established connection by client identity.
pub fn record_connection(info: Option<&ClientInfo>) {
let (app, platform, app_version) = labels(info);
metrics::counter!(
"buzz_client_connections_total",
"app" => app.to_owned(),
"platform" => platform.to_owned(),
"app_version" => app_version.to_owned()
)
.increment(1);
}
/// Add a live connection to the per-client active gauge.
/// The per-client live-connection gauge, labeled for `info`.
///
/// This is a **separate** series from `buzz_ws_connections_active`, which is
/// consumed by the HPA as an unlabeled `AverageValue` target. Labeling that
/// gauge would shard the series the autoscaler reads; this parallel gauge
/// gives the same breakdown without touching scaling behaviour.
pub fn increment_active(info: Option<&ClientInfo>) {
let (app, platform, app_version) = labels(info);
///
/// A gauge is the only metric kind emitted here, deliberately: the recorder
/// idle-evicts gauge series, so a forged label set cannot accumulate for the
/// process lifetime the way a counter would.
fn active_gauge(info: Option<&ClientInfo>) -> metrics::Gauge {
let (app, platform, app_version) = match info {
Some(info) => (info.app, info.platform, info.app_version()),
None => (UNKNOWN_LABEL, UNKNOWN_LABEL, UNKNOWN_LABEL),
};
metrics::gauge!(
"buzz_client_connections_active",
"app" => app.to_owned(),
"platform" => platform.to_owned(),
"app_version" => app_version.to_owned()
)
.increment(1.0);
}
/// Add a live connection to the per-client active gauge.
pub fn increment_active(info: Option<&ClientInfo>) {
active_gauge(info).increment(1.0);
}
/// Remove a closed connection from the per-client active gauge.
///
/// Must be paired with exactly one [`increment_active`] call, or the gauge
/// drifts. Retired label sets are dropped by the recorder's configured gauge
/// idle timeout rather than going stale.
/// drifts.
pub fn decrement_active(info: Option<&ClientInfo>) {
let (app, platform, app_version) = labels(info);
metrics::gauge!(
"buzz_client_connections_active",
"app" => app.to_owned(),
"platform" => platform.to_owned(),
"app_version" => app_version.to_owned()
)
.decrement(1.0);
active_gauge(info).decrement(1.0);
}
/// A dictionary member's value, still in its wire form.
@@ -408,10 +389,9 @@ fn is_token(candidate: &str) -> bool {
/// Narrow a version to `MAJOR.MINOR`, or `None` if it is not one.
///
/// Trailing components and pre-release/build metadata are discarded so patch
/// releases do not each create a new time series. The *whole* string is still
/// validated first: the discarded tail is retained for logging, so leaving it
/// unchecked would let a hostile client put arbitrary text in a log field even
/// though the metric label stayed bounded.
/// releases do not each create a new time series. The whole string is
/// validated first so a version that is malformed only in its discarded tail
/// is still reported as a failure rather than silently accepted.
fn major_minor(version: &str) -> Option<String> {
if !is_plausible_version(version) {
return None;
@@ -425,10 +405,11 @@ fn major_minor(version: &str) -> Option<String> {
/// Whether the full version string looks like a version Buzz produced.
///
/// Matches the sender-side rule in `buzz_core::client_identity`: dot-separated
/// alphanumerics with optional `-`/`+` pre-release and build metadata.
/// alphanumerics with optional `-`/`+` pre-release and build metadata, within
/// the shared length bound.
fn is_plausible_version(version: &str) -> bool {
!version.is_empty()
&& version.len() <= MAX_LOGGED_VALUE_LEN
&& version.len() <= MAX_APP_VERSION_LEN
&& version
.bytes()
.all(|b| b.is_ascii_alphanumeric() || matches!(b, b'.' | b'-' | b'+'))
@@ -448,11 +429,6 @@ fn numeric_component(component: &str) -> Option<&str> {
Some(component)
}
/// Bound a raw value's length for inclusion in a log line.
fn truncate_for_log(value: &str) -> String {
value.chars().take(MAX_LOGGED_VALUE_LEN).collect()
}
#[cfg(test)]
mod tests {
use super::*;
@@ -468,7 +444,17 @@ mod tests {
assert_eq!(info.app(), "buzz-desktop");
assert_eq!(info.platform(), "macos");
assert_eq!(info.app_version(), "0.5");
assert_eq!(info.app_version_detail(), "0.5.2");
}
#[test]
fn accepts_a_header_with_no_app_version() {
// buzz-cli and buzz-acp have no independently bumped release version,
// so they omit `app-version` rather than send a misleading constant.
// That must parse, not be counted as a failure.
let info = parse("v=1, app=buzz-cli, platform=linux").unwrap();
assert_eq!(info.app(), "buzz-cli");
assert_eq!(info.platform(), "linux");
assert_eq!(info.app_version(), UNKNOWN_LABEL);
}
#[test]
@@ -488,11 +474,13 @@ mod tests {
ClientPlatform::Ios,
ClientPlatform::Android,
] {
let raw = client_header_value(app, platform, "1.2.3").expect("builder emits");
let info = parse(&raw).unwrap_or_else(|e| panic!("{raw:?} rejected as {e:?}"));
assert_eq!(info.app(), app.as_str());
assert_eq!(info.platform(), platform.as_str());
assert_eq!(info.app_version(), "1.2");
for (version, expected) in [(Some("1.2.3"), "1.2"), (None, UNKNOWN_LABEL)] {
let raw = client_header_value(app, platform, version);
let info = parse(&raw).unwrap_or_else(|e| panic!("{raw:?} rejected as {e:?}"));
assert_eq!(info.app(), app.as_str());
assert_eq!(info.platform(), platform.as_str());
assert_eq!(info.app_version(), expected);
}
}
}
}
@@ -605,8 +593,11 @@ mod tests {
parse(r#"v=1, app=buzz-desktop, app-version="1.0.0""#),
Err(ParseFailure::UnknownPlatform)
);
// `app-version` is the one optional member — absent is valid, but a
// present-and-broken one is still a failure.
assert!(parse("v=1, app=buzz-desktop, platform=macos").is_ok());
assert_eq!(
parse(r#"v=1, app=buzz-desktop, platform=macos"#),
parse(r#"v=1, app=buzz-desktop, platform=macos, app-version="nope""#),
Err(ParseFailure::BadAppVersion)
);
}
@@ -704,9 +695,20 @@ mod tests {
#[test]
fn unidentified_connections_get_the_unknown_bucket() {
assert_eq!(labels(None), (UNKNOWN_LABEL, UNKNOWN_LABEL, UNKNOWN_LABEL));
// An unidentified connection must still produce a series, so
// "unidentified" is a visible dashboard line rather than a silent gap.
let info = parse(r#"v=1, app=buzz-cli, platform=linux, app-version="1.0.0""#).unwrap();
assert_eq!(labels(Some(&info)), ("buzz-cli", "linux", "1.0"));
assert_eq!(
(info.app(), info.platform(), info.app_version()),
("buzz-cli", "linux", "1.0")
);
// Gauge helpers accept `None` and label every dimension unknown; they
// are exercised here to prove the label path cannot panic without a
// recorder installed.
increment_active(None);
decrement_active(None);
increment_active(Some(&info));
decrement_active(Some(&info));
}
#[test]
@@ -725,13 +727,16 @@ mod tests {
}
#[test]
fn logged_version_detail_is_length_bounded() {
// `app-version` is bounded by the builder, but the parser must not
// rely on a hostile client honouring that.
let long = "1.2".to_owned() + &".9".repeat(64);
assert_eq!(
truncate_for_log(&long).chars().count(),
MAX_LOGGED_VALUE_LEN
fn the_sender_cannot_emit_a_header_this_parser_would_reject_as_oversized() {
// Both halves share `MAX_HEADER_LEN`, so anything the builder produces
// must fit the parser's bound. A divergence here would silently count
// real clients as parse failures.
let longest = client_header_value(
ClientApp::Desktop,
ClientPlatform::Android,
Some("10000.10000.99999-rc.1+build"),
);
assert!(longest.len() <= MAX_HEADER_LEN, "{longest}");
assert!(ClientInfo::parse_bytes(longest.as_bytes()).is_ok());
}
}
crates/buzz-relay/src/connection.rs
+1 -2
View File
@@ -209,7 +209,7 @@ async fn handle_active_connection(
addr = %addr,
client.app = client.as_ref().map(ClientInfo::app),
client.platform = client.as_ref().map(ClientInfo::platform),
client.app_version = client.as_ref().map(ClientInfo::app_version_detail),
client.app_version = client.as_ref().map(ClientInfo::app_version),
"WebSocket connection established"
);
metrics::counter!(
@@ -217,7 +217,6 @@ async fn handle_active_connection(
"community" => conn.tenant.host().to_owned()
)
.increment(1);
crate::client_info::record_connection(client.as_ref());
let challenge_msg = RelayMessage::auth_challenge(&challenge);
if tx
crates/buzz-ws-client/src/connection.rs
+28 -17
View File
@@ -11,6 +11,7 @@ use tokio::time::timeout;
use tokio_tungstenite::tungstenite::client::ClientRequestBuilder;
use tokio_tungstenite::{connect_async, tungstenite::Message, MaybeTlsStream, WebSocketStream};
use tracing::debug;
use url::Url;
use crate::error::WsClientError;
use crate::message::{build_auth_event, parse_relay_message, OkResponse, RelayMessage};
@@ -21,18 +22,18 @@ type WsStream = WebSocketStream<MaybeTlsStream<tokio::net::TcpStream>>;
/// header when the destination permits it.
///
/// Attaching the header is never allowed to fail a connection: an unshipped
/// platform, an unusable version, or a destination outside
/// [`may_identify_to`] simply yields a request without the header.
/// platform or a destination outside [`may_identify_to`] simply yields a
/// request without the header.
fn client_request(
url: &str,
url: &Url,
app: ClientApp,
app_version: &str,
app_version: Option<&str>,
) -> Result<ClientRequestBuilder, WsClientError> {
let uri = url
.parse()
.map_err(|e: tokio_tungstenite::tungstenite::http::uri::InvalidUri| {
let uri = url.as_str().parse().map_err(
|e: tokio_tungstenite::tungstenite::http::uri::InvalidUri| {
WsClientError::Url(e.to_string())
})?;
},
)?;
let builder = ClientRequestBuilder::new(uri);
if !may_identify_to(url) {
return Ok(builder);
@@ -75,28 +76,38 @@ impl NostrWsConnection {
}
/// Connects to the relay at `url` without performing authentication.
///
/// Identifies as [`ClientApp::Cli`], which covers the `buzz` CLI and the
/// test client. No version is sent: both inherit the workspace version,
/// which has never been bumped (see `RELEASING.md`), so reporting it would
/// pin every CLI connection to a fixed number forever. The relay records
/// the version as unknown, which is accurate.
pub async fn connect(url: &str) -> Result<Self, WsClientError> {
Self::connect_as(url, ClientApp::Cli, env!("CARGO_PKG_VERSION")).await
Self::connect_as(url, ClientApp::Cli, None).await
}
/// Connects to the relay at `url`, identifying as `app` version
/// `app_version` in the advisory `Buzz-Client` header.
/// Connects to the relay at `url`, identifying as `app` in the advisory
/// `Buzz-Client` header.
///
/// Pass `app_version` only for a client with a real, independently bumped
/// release version; pass `None` otherwise so the relay reports the version
/// as unknown rather than as a misleading constant.
///
/// The header lets the relay report which client versions and platforms
/// its live connections come from. It is best-effort: if it cannot be
/// built (unshipped platform, unusable version) or the destination is not
/// one this client may identify itself to, the connection proceeds without
/// it and the relay counts it as unidentified.
/// built (unshipped platform) or the destination is not one this client may
/// identify itself to, the connection proceeds without it and the relay
/// counts it as unidentified.
pub async fn connect_as(
url: &str,
app: ClientApp,
app_version: &str,
app_version: Option<&str>,
) -> Result<Self, WsClientError> {
let parsed = url
.parse::<url::Url>()
.parse::<Url>()
.map_err(|e| WsClientError::Url(e.to_string()))?;
let request = client_request(parsed.as_str(), app, app_version)?;
let request = client_request(&parsed, app, app_version)?;
let (ws, _response) = connect_async(request)
.await
.map_err(WsClientError::WebSocket)?;
desktop/src-tauri/src/native_websocket.rs
+11 -4
View File
@@ -13,6 +13,7 @@ use tokio_tungstenite::{
tungstenite::protocol::{frame::coding::CloseCode, CloseFrame, Message},
};
use tokio_util::sync::CancellationToken;
use url::Url;
const CONNECT_TIMEOUT: Duration = Duration::from_secs(10);
const WRITE_TIMEOUT: Duration = Duration::from_secs(10);
@@ -24,13 +25,19 @@ const SEND_QUEUE_CAPACITY: usize = 64;
///
/// The header tells the relay which app, platform, and version a live
/// connection belongs to. Attaching it never fails a connection: a destination
/// outside `may_identify_to` or an unusable version just omits it, and the
/// relay counts the connection as unidentified.
/// outside `may_identify_to` or an unparseable URL just omits the header, and
/// the relay counts the connection as unidentified.
///
/// The desktop app is one of the two lanes with an independently bumped
/// release version (`RELEASING.md`), so `CARGO_PKG_VERSION` is a real version
/// here — `bump-desktop-version` rewrites it. Clients that inherit the
/// workspace version deliberately send none.
fn client_request(url: &str) -> Result<ClientRequestBuilder, String> {
let uri = url.parse().map_err(|error| format!("{error}"))?;
let builder = ClientRequestBuilder::new(uri);
let Some(value) = may_identify_to(url)
.then(|| client_header_value_for_host(ClientApp::Desktop, env!("CARGO_PKG_VERSION")))
let permitted = Url::parse(url).is_ok_and(|parsed| may_identify_to(&parsed));
let Some(value) = permitted
.then(|| client_header_value_for_host(ClientApp::Desktop, Some(env!("CARGO_PKG_VERSION"))))
.flatten()
else {
return Ok(builder);