From 4713e347508d4aa80b32ee1c421d21d2b627a4d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Chv=C3=A1tal?= Date: Sat, 17 Oct 2026 17:07:03 +0100 Subject: [PATCH 1/5] fix(server): name anonymous landing in identity preflight failures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A Bugzilla probe refused with login_required (envelope code 410) reached Bugzilla anonymously: the API key did not arrive in a form the deployment honours. Preflight diagnosed it as something else — the whoami arm blamed the endpoint's existence, the declared arm a wrong login or a generic endpoint failure — and a valid_login false, which an anonymous request also produces, named only the account. Both server-custody arms now key on code 410 and name the anonymous landing and the API key placement (the api_key query parameter by default, an Authorization: Bearer header under --use-auth-header), including the transit that can strip the chosen form. The valid_login-false bail names the same possibility alongside the login. DESIGN.md records the diagnosis in the client contract and the Identity preflight section; three wiremock tests pin it, each verified to fail when its piece of the wording is removed. --- crates/bugwarden/src/server.rs | 93 +++++++++++++--- crates/bugwarden/tests/preflight_wiremock.rs | 111 ++++++++++++++++++- docs/DESIGN.md | 32 +++++- 3 files changed, 219 insertions(+), 17 deletions(-) diff --git a/crates/bugwarden/src/server.rs b/crates/bugwarden/src/server.rs index 9c9539b..1fe7bde 100644 --- a/crates/bugwarden/src/server.rs +++ b/crates/bugwarden/src/server.rs @@ -468,6 +468,21 @@ fn upstream_hint(tool: &str, code: i64, reach: UpstreamReach) -> Option<&'static .map(|row| row.text) } +/// Bugzilla's `login_required` code: the refusal an anonymous request +/// meets. Keyed on the envelope code, not the HTTP status — Bugzilla +/// answers this one as HTTP 401. +const LOGIN_REQUIRED_CODE: i64 = 410; + +/// Whether `err` is Bugzilla's `login_required` refusal: the request +/// reached Bugzilla anonymously, whatever the operator configured — +/// the API key did not arrive in a form the deployment honours, or +/// did not arrive at all. +fn login_required(err: &anyhow::Error) -> bool { + err.downcast_ref::() + .and_then(|bz| bz.code()) + == Some(LOGIN_REQUIRED_CODE) +} + /// A write tool's failure text for an upstream refusal: its fixed first /// line — the audit gate's refusal, so an unhinted failure and the gate /// refusal are byte-identical — plus the hint `code` selects, if any. @@ -3522,6 +3537,16 @@ impl BugWarden { /// closed, distinct from a transport failure so the operator does not /// mistake a wrong login for a broken key); a transport/parse error /// bails naming the endpoint, the same as the `whoami` arm. + /// + /// A refusal carrying Bugzilla code 410 (`login_required`) is + /// diagnosed separately in both server-custody arms: the request + /// reached Bugzilla anonymously, so the bail names the API key + /// placement — the `api_key` query parameter by default, an + /// `Authorization: Bearer` header under `--use-auth-header` — and + /// the transit that can strip the chosen form, rather than the + /// endpoint-existence or wrong-login explanations a 410 is neither + /// of. A `valid_login` `false` can mean the same thing, so that + /// bail names the possibility too. /// [`BugWarden::new`] already refuses to construct a declared-identity /// server under per-request custody (A5), so that combination cannot /// reach this method — the match below still names it explicitly @@ -3539,16 +3564,29 @@ impl BugWarden { tracing::info!(login, "identity endpoint verified"); Ok(()) } - Err(err) => Err(err).context( - "GET /rest/whoami failed during startup preflight; the guard policy \ - consults created_by_me, which needs this endpoint to resolve the \ - caller's identity. Stock Bugzilla Core v1 does not define /rest/whoami \ - (it is a fork/BMO extension) — if this deployment does not have it, \ - starting anyway would deny every access classification the policy's \ - identity rules reach (I4). Confirm the endpoint exists, set \ - global.identity_source = \"declared\" instead, or remove the \ - created_by_me criteria from the policy", - ), + Err(err) => { + let context = if login_required(&err) { + "GET /rest/whoami reached Bugzilla anonymously during startup \ + preflight: Bugzilla refused it with login_required (code 410), \ + so the configured API key did not authenticate the request. \ + The key travels as the api_key query parameter by default, or \ + as an Authorization: Bearer header with --use-auth-header; a \ + deployment honouring only the other form, or a proxy stripping \ + the chosen one in transit, produces exactly this refusal while \ + the key itself stays valid. Match the placement to the \ + deployment and the network path, then restart" + } else { + "GET /rest/whoami failed during startup preflight; the guard policy \ + consults created_by_me, which needs this endpoint to resolve the \ + caller's identity. Stock Bugzilla Core v1 does not define /rest/whoami \ + (it is a fork/BMO extension) — if this deployment does not have it, \ + starting anyway would deny every access classification the policy's \ + identity rules reach (I4). Confirm the endpoint exists, set \ + global.identity_source = \"declared\" instead, or remove the \ + created_by_me criteria from the policy" + }; + Err(err).context(context) + } }, KeyCustody::PerRequest => { tracing::warn!( @@ -3595,13 +3633,38 @@ impl BugWarden { "GET /rest/valid_login says the configured API key does not \ authenticate as \"{login}\" (global.identity_login); Bugzilla \ compares logins case-sensitively, so check for a case mismatch as \ - well as a wrong account. Fail closed: starting anyway would deny \ + well as a wrong account. A request that reaches Bugzilla \ + anonymously answers false here too: the key travels as the api_key \ + query parameter by default, or as an Authorization: Bearer header \ + with --use-auth-header, and a deployment honouring only the other \ + form, or a proxy stripping the chosen one in transit, makes a \ + valid key read exactly like a wrong account — check the \ + placement as well. Fail closed: starting anyway would deny \ every access classification the policy's identity rules reach (I4)", ), - Err(err) => Err(err).context(format!( - "GET /rest/valid_login failed during startup preflight while \ - verifying the declared login \"{login}\" (global.identity_login)", - )), + Err(err) => { + let context = if login_required(&err) { + format!( + "GET /rest/valid_login reached Bugzilla anonymously during \ + startup preflight while verifying the declared login \ + \"{login}\" (global.identity_login): Bugzilla refused it \ + with login_required (code 410), so the configured API key \ + did not authenticate the request. The key travels as the \ + api_key query parameter by default, or as an Authorization: \ + Bearer header with --use-auth-header; a deployment \ + honouring only the other form, or a proxy stripping the \ + chosen one in transit, produces exactly this refusal while \ + the key itself stays valid. Match the placement to the \ + deployment and the network path, then restart" + ) + } else { + format!( + "GET /rest/valid_login failed during startup preflight while \ + verifying the declared login \"{login}\" (global.identity_login)" + ) + }; + Err(err).context(context) + } } } } diff --git a/crates/bugwarden/tests/preflight_wiremock.rs b/crates/bugwarden/tests/preflight_wiremock.rs index faed529..936a662 100644 --- a/crates/bugwarden/tests/preflight_wiremock.rs +++ b/crates/bugwarden/tests/preflight_wiremock.rs @@ -15,7 +15,12 @@ //! identity criterion (the laziness contract must hold at startup too); //! - `preflight` bailing under `KeyCustody::PerRequest` instead of //! warning-and-continuing (there is no server-held key to probe with); -//! - an API key leaking into a preflight error's text (I12). +//! - an API key leaking into a preflight error's text (I12); +//! - a `login_required` (code 410) refusal diagnosed with the generic +//! endpoint-existence or endpoint-failure context instead of the +//! anonymous-landing one naming the API key placement; +//! - the `valid_login` `false` bail dropping its anonymous-landing +//! sentence (a stripped placement reads exactly like a wrong login). use std::sync::Arc; @@ -310,3 +315,107 @@ async fn preflight_transport_error_does_not_leak_the_api_key_i12() { "API key leaked into a preflight error: {msg}" ); } + +#[tokio::test] +async fn preflight_whoami_login_required_names_the_anonymous_landing() { + // The key never arrived in a form Bugzilla honours: whoami answers + // the login_required refusal (HTTP 401, envelope code 410). The bail + // must name the anonymous landing and the placement — this deployment + // HAS the endpoint, so the endpoint-existence context a 404 earns + // would send the operator debugging the wrong thing. + let mock = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/rest/whoami")) + .respond_with(ResponseTemplate::new(401).set_body_json(serde_json::json!({ + "error": true, + "code": 410, + "message": "You must log in before using this part of Bugzilla.", + }))) + .mount(&mock) + .await; + let server = server_custody_server(IDENTITY_POLICY, &mock); + + let err = server + .preflight() + .await + .expect_err("a login_required whoami refusal must fail preflight"); + let msg = format!("{err:#}"); + assert!( + msg.contains("anonymously"), + "preflight error must name the anonymous landing: {msg}" + ); + assert!( + msg.contains("--use-auth-header"), + "preflight error must name the placement that decides it: {msg}" + ); + assert!( + !msg.contains("does not define /rest/whoami"), + "a 410 is not an endpoint-existence failure: {msg}" + ); +} + +#[tokio::test] +async fn preflight_valid_login_login_required_names_the_anonymous_landing() { + // The same refusal on the declared arm: valid_login answers + // login_required instead of a result. The bail names the endpoint + // and the anonymous landing, not a generic endpoint failure. + let mock = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/rest/valid_login")) + .respond_with(ResponseTemplate::new(401).set_body_json(serde_json::json!({ + "error": true, + "code": 410, + "message": "You must log in before using this part of Bugzilla.", + }))) + .mount(&mock) + .await; + let server = server_custody_server(DECLARED_IDENTITY_POLICY, &mock); + + let err = server + .preflight() + .await + .expect_err("a login_required valid_login refusal must fail preflight"); + let msg = format!("{err:#}"); + assert!( + msg.contains("GET /rest/valid_login"), + "preflight error must name the failing endpoint: {msg}" + ); + assert!( + msg.contains("anonymously"), + "preflight error must name the anonymous landing: {msg}" + ); + assert!( + msg.contains("--use-auth-header"), + "preflight error must name the placement that decides it: {msg}" + ); +} + +#[tokio::test] +async fn preflight_declared_login_false_names_the_anonymous_possibility() { + // valid_login answering false is also what an anonymous request + // gets, so alongside the login the bail must name the placement + // that decides whether the key arrived at all — otherwise a + // stripped header or query string is indistinguishable from a + // wrong account and sends the operator after the wrong fix. + let mock = MockServer::start().await; + mount_valid_login(&mock, "svc@example.com", false, 1).await; + let server = server_custody_server(DECLARED_IDENTITY_POLICY, &mock); + + let err = server + .preflight() + .await + .expect_err("a login the key does not own must fail preflight"); + let msg = format!("{err:#}"); + assert!( + msg.contains("svc@example.com"), + "preflight error must name the declared login: {msg}" + ); + assert!( + msg.contains("anonymously"), + "preflight error must name the anonymous-landing possibility: {msg}" + ); + assert!( + msg.contains("--use-auth-header"), + "preflight error must name the placement that decides it: {msg}" + ); +} diff --git a/docs/DESIGN.md b/docs/DESIGN.md index 642de8b..3aa3b9b 100644 --- a/docs/DESIGN.md +++ b/docs/DESIGN.md @@ -812,6 +812,21 @@ Endpoint mapping: Auth per request: `use_auth_header` ? header `Authorization: Bearer {key}` : query param `api_key={key}`. Always `Accept: application/json`. + +The two placements are not interchangeable dialects of one form: stock +Bugzilla and BMO honour the query parameter (under both the `api_key` +and `Bugzilla_api_key` names), Red Hat Bugzilla requires the Bearer +header, and stock Bugzilla's own header form (`X-Bugzilla-API-Key`) is +not sent. Whichever form the path between the server and Bugzilla +strips — a proxy dropping headers, a gateway rewriting query strings — +every request lands anonymous, and Bugzilla's only signal is +`login_required` (envelope code 410, answered as HTTP 401): reads +silently narrow to the anonymous view and writes refuse. The client +cannot detect the strip, so the diagnosis lives where an operator +first meets a failure: the identity preflight names the anonymous +landing and the placement whenever a probe meets code 410, and the +`valid_login`-false bail names it as a possibility (see "Identity +preflight"). Errors: a non-2xx status, a body `{"error": true}` under any status, or a 2xx body that is not JSON => a `BugzillaError` inside anyhow. Its Display is `bugzilla error (HTTP {status}): {message}` — the text the untyped error @@ -1323,6 +1338,17 @@ The failure path attaches the sanitized client error (I12: `whoami` / `valid_login` already strip the URL) as the `anyhow` error's source/context — no key material reaches the bailed-out message either. +A probe refusal carrying envelope code 410 (`login_required`) is worded +separately in both server-custody arms: the request reached Bugzilla +anonymously, so the bail names the API key placement — the `api_key` +query parameter by default, an `Authorization: Bearer` header under +`--use-auth-header` — and the transit that can strip the chosen form, +instead of the endpoint-existence (`whoami`) or wrong-login +(`declared`) explanations a 410 is neither of. The `declared` arm's +`false` bail names the same possibility: an anonymous `valid_login` +answers `false`, so a placement the path strips reads exactly like a +wrong account. + Tool descriptions: concise and action-oriented; state the defaults and constraints the model must know. @@ -4297,7 +4323,11 @@ wired, `server.rs` and `main.rs` are the reference. the `declared` arm — a correct declared login passes preflight via one `valid_login` request and zero whoami requests, a login the key does NOT authenticate as fails preflight naming that login, and a transport-level - `valid_login` failure fails preflight naming the endpoint. + `valid_login` failure fails preflight naming the endpoint; a + `login_required` (code 410) refusal on either probe fails preflight + naming the anonymous landing and the API key placement (for `whoami`, + without the endpoint-existence wording), and the `valid_login`-false + bail names the anonymous-landing possibility alongside the login. - Integration tests (crates/bugwarden/tests/http_transport_wiremock.rs, wiremock + rmcp client over REAL streamable HTTP — the only harness in which the per-request key header physically exists; the wiremock upstream From 0f637030b5dd78583850a83cf6e6dec8a03c10c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Chv=C3=A1tal?= Date: Sat, 17 Oct 2026 17:07:03 +0100 Subject: [PATCH 2/5] docs(readme): document API key placement choice and failure signature MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The API key setup now says which deployment honours which placement (the query parameter for stock Bugzilla and BMO, the Bearer header for Red Hat Bugzilla), what a stripped placement looks like — every request anonymous, writes refused with code 410 — and that the identity preflight names the cause at startup. --- README.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/README.md b/README.md index ae02b0d..1137b6d 100644 --- a/README.md +++ b/README.md @@ -390,6 +390,18 @@ instances that reject the `api_key` query parameter and require this affects only server-to-Bugzilla authentication, not the client-facing header. +Choose the placement the deployment in front of you honours: the query +parameter is what stock Bugzilla and BMO accept, the Bearer header is +the Red Hat form, and no other header form is sent. If the network path +between bugwarden and Bugzilla strips the chosen form — a proxy dropping +headers, a gateway rewriting query strings — every request arrives +anonymous: reads quietly narrow to what an anonymous caller may see, +and writes fail with "You must log in before using this part of +Bugzilla" (code 410). The key itself is fine in that state; switch the +placement to the form that survives the path. When the guard policy +consults identity, the startup preflight names this cause before the +server starts serving. + #### Server-held key mode (fleet deployments) With `--api-key-file` the Bugzilla API key belongs to the *server*: every From 3ffd2c8e3b3898ed785c465158bbdd246bdc3343 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Chv=C3=A1tal?= Date: Mon, 19 Oct 2026 16:20:10 +0100 Subject: [PATCH 3/5] docs: correct which Bugzilla deployment honours which API key form Stock Bugzilla 5.0/5.2 has no header form at all; the X-Bugzilla-API-Key header is BMO's. BMO still accepts the query parameter, but documents it as deprecated; its supported forms are that header and OAuth2 bearer tokens. --- README.md | 12 +++++++----- docs/DESIGN.md | 10 +++++++--- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 1137b6d..e19f365 100644 --- a/README.md +++ b/README.md @@ -391,11 +391,13 @@ this affects only server-to-Bugzilla authentication, not the client-facing header. Choose the placement the deployment in front of you honours: the query -parameter is what stock Bugzilla and BMO accept, the Bearer header is -the Red Hat form, and no other header form is sent. If the network path -between bugwarden and Bugzilla strips the chosen form — a proxy dropping -headers, a gateway rewriting query strings — every request arrives -anonymous: reads quietly narrow to what an anonymous caller may see, +parameter is what stock Bugzilla accepts, and what BMO still accepts +but documents as deprecated — BMO's supported forms are its own +`X-Bugzilla-API-Key` header and OAuth2 bearer tokens. The Bearer token +is the Red Hat form, and no other header form is sent. If the network +path between bugwarden and Bugzilla strips the chosen form — a proxy +dropping headers, a gateway rewriting query strings — every request +arrives anonymous: reads quietly narrow to what an anonymous caller may see, and writes fail with "You must log in before using this part of Bugzilla" (code 410). The key itself is fine in that state; switch the placement to the form that survives the path. When the guard policy diff --git a/docs/DESIGN.md b/docs/DESIGN.md index 3aa3b9b..8de8068 100644 --- a/docs/DESIGN.md +++ b/docs/DESIGN.md @@ -814,9 +814,13 @@ Auth per request: `use_auth_header` ? header `Authorization: Bearer {key}` : query param `api_key={key}`. Always `Accept: application/json`. The two placements are not interchangeable dialects of one form: stock -Bugzilla and BMO honour the query parameter (under both the `api_key` -and `Bugzilla_api_key` names), Red Hat Bugzilla requires the Bearer -header, and stock Bugzilla's own header form (`X-Bugzilla-API-Key`) is +Bugzilla honours the query parameter (under both the `api_key` and +`Bugzilla_api_key` names). BMO still accepts it, but documents it as +deprecated — a stopgap for old callers; the forms it supports are its +own `X-Bugzilla-API-Key` header and OAuth2 bearer tokens, so an API +key carried as a Bearer token lands anonymous there too. Red Hat +Bugzilla requires the Bearer header. Stock Bugzilla 5.0/5.2 has no +header form at all; BMO's own header form (`X-Bugzilla-API-Key`) is not sent. Whichever form the path between the server and Bugzilla strips — a proxy dropping headers, a gateway rewriting query strings — every request lands anonymous, and Bugzilla's only signal is From 0021298627e5fae0293a6498411978f9a6062d67 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Chv=C3=A1tal?= Date: Mon, 19 Oct 2026 16:20:36 +0100 Subject: [PATCH 4/5] test(preflight): pin the declared-arm transport error against the anonymous-landing diagnosis Both preflight arms print GET /rest/valid_login in their transport and login_required contexts, so the endpoint assertion alone cannot catch a login_required() that answers true for every error. Assert the transport error never claims an anonymous landing, mirroring the whoami arm's pin. --- crates/bugwarden/tests/preflight_wiremock.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/crates/bugwarden/tests/preflight_wiremock.rs b/crates/bugwarden/tests/preflight_wiremock.rs index 936a662..bbcd2f5 100644 --- a/crates/bugwarden/tests/preflight_wiremock.rs +++ b/crates/bugwarden/tests/preflight_wiremock.rs @@ -276,6 +276,10 @@ async fn preflight_declared_login_transport_error_names_the_endpoint() { msg.contains("GET /rest/valid_login"), "preflight error must name the failing endpoint: {msg}" ); + assert!( + !msg.contains("anonymously"), + "a transport failure must not be diagnosed as an anonymous landing: {msg}" + ); } #[tokio::test] From 60c4add754a4a0206c47db3708a4dd42507a7864 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Chv=C3=A1tal?= Date: Mon, 19 Oct 2026 16:20:48 +0100 Subject: [PATCH 5/5] docs(preflight): separate the paragraphs the rustdoc and DESIGN run together The server.rs paragraph ending 'names the possibility too.' ran straight into the BugWarden::new paragraph, and DESIGN ran the same text straight into 'Errors:'; add the missing blank lines so both render as the two paragraphs they are. --- crates/bugwarden/src/server.rs | 1 + docs/DESIGN.md | 1 + 2 files changed, 2 insertions(+) diff --git a/crates/bugwarden/src/server.rs b/crates/bugwarden/src/server.rs index 1fe7bde..49fd16a 100644 --- a/crates/bugwarden/src/server.rs +++ b/crates/bugwarden/src/server.rs @@ -3547,6 +3547,7 @@ impl BugWarden { /// endpoint-existence or wrong-login explanations a 410 is neither /// of. A `valid_login` `false` can mean the same thing, so that /// bail names the possibility too. + /// /// [`BugWarden::new`] already refuses to construct a declared-identity /// server under per-request custody (A5), so that combination cannot /// reach this method — the match below still names it explicitly diff --git a/docs/DESIGN.md b/docs/DESIGN.md index 8de8068..77ebf45 100644 --- a/docs/DESIGN.md +++ b/docs/DESIGN.md @@ -831,6 +831,7 @@ first meets a failure: the identity preflight names the anonymous landing and the placement whenever a probe meets code 410, and the `valid_login`-false bail names it as a possibility (see "Identity preflight"). + Errors: a non-2xx status, a body `{"error": true}` under any status, or a 2xx body that is not JSON => a `BugzillaError` inside anyhow. Its Display is `bugzilla error (HTTP {status}): {message}` — the text the untyped error