From aa82cf93a2add4a5071b7bc1c707cd688a433439 Mon Sep 17 00:00:00 2001 From: Martin Pluskal Date: Tue, 28 Jul 2026 09:42:01 +0200 Subject: [PATCH] fix(server): steer exact id sets to bug_info MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit bugs_quicksearch prefixes the status filter (default ALL) to the query, so under any non-empty status a query that is just a list of bug ids is content-matched and returns every bug that merely MENTIONS one of those numbers — a reported call with 8 ids came back with 14 bugs. With an empty status the query goes upstream bare, and Bugzilla routes a bare all-number query to an exact id lookup (bug_id + anyexact) instead. Either way the right tool for an exact id set is bug_info, whose envelope answers every requested id either as a bug or under 'restricted' instead of silently dropping it. Steer clients there in three places: - The bugs_quicksearch tool description and the query/status field docs now state the status-prefixing rule, its content-matching consequence, and the empty-status exception, and point exact id sets to bug_info. quicksearch_syntax, the README row, and the DESIGN.md row tell the same story, since Bugzilla's syntax page advertises a jump-to-bug- number shortcut that only the bare-query path takes. - A runtime advisory: when the query consists entirely of comma/ whitespace-separated bug ids (optional leading '#' per id), the result envelope carries a `note` saying so. The note is a pure function of the client's request — the query and status strings, never search results, guard verdicts, or anything upstream said — so it opens no new oracle (I2/I3), and the `bugs` array is byte-identical with or without it. On the empty-status path the note drops the content-matching claim (an id lookup is what actually happens there), and a query naming more distinct ids than bug_info's 25-per-call cap steers to batched bug_info calls instead of straight into the too_many_ids refusal — the cap is already public in that refusal's own text. The query is still searched, never rerouted. Integration tests pin all of it, including the oracle edges: the note appears for a pure id-list query, not for a content query; the served bugs match byte-for-byte apart from the note; a policy-hidden bug, an all-hidden (empty) result, and I14 link scrubbing each change the served bugs but never the note; and the wording tracks only the request's status and distinct-id count. --- README.md | 2 +- crates/bugwarden/src/server.rs | 143 +++++++++++- crates/bugwarden/tests/tools_wiremock.rs | 268 ++++++++++++++++++++++- docs/DESIGN.md | 2 +- 4 files changed, 407 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 47c1488..0ecda43 100644 --- a/README.md +++ b/README.md @@ -331,7 +331,7 @@ action. | `bug_info` | Details for a set of bug ids. Per id: full details with `read`, redacted summary with `summary`, otherwise a uniform "not accessible" entry | `read` / `summary` | | `bug_history` | Change history of a bug, optionally only entries newer than a timestamp | `history` | | `bug_comments` | Comments on a bug; private comments only per the private-comment gate | `comments` | -| `bugs_quicksearch` | Bugzilla [quicksearch](https://bugzilla.readthedocs.io/en/latest/using/finding.html#quicksearch); results are silently policy-filtered (denied dropped, summary-only redacted) | per result: `read` / `summary` | +| `bugs_quicksearch` | Bugzilla [quicksearch](https://bugzilla.readthedocs.io/en/latest/using/finding.html#quicksearch) — the `status` filter (default `ALL`) is prefixed to the query, and under any non-empty status a number in the query is content-matched, so it also matches bugs that merely mention it; with an empty `status` the query goes to Bugzilla bare, where a query of nothing but numbers is an exact id lookup (use `bug_info` for an exact set of known ids; an all-ids query gets an advisory note saying so). Results are silently policy-filtered (denied dropped, summary-only redacted) | per result: `read` / `summary` | | `summarize_bug` | Returns a summarization prompt built from the bug's public comments | `comments` | | `list_attachments` | Attachment metadata (never attachment content) | `attachments` | | `download_attachment` | Content of one attachment (raster images as image content, everything else as a base64 blob resource), capped by `max_attachment_bytes`; private attachments need the private-content double opt-in and, on download, a *missing* privacy flag counts as private | `attachments` on the owning bug | diff --git a/crates/bugwarden/src/server.rs b/crates/bugwarden/src/server.rs index d2c21b8..e053a43 100644 --- a/crates/bugwarden/src/server.rs +++ b/crates/bugwarden/src/server.rs @@ -132,6 +132,69 @@ fn too_many_ids(ids: &[u64]) -> Option { }) } +/// Advisory attached to a quicksearch envelope when the query is nothing but +/// bug ids (comma/whitespace-separated numbers, each optionally prefixed +/// with '#'). +/// +/// With a non-empty `status` the tool prefixes it to the query, so upstream +/// content-matches the whole expression and a client holding an exact set of +/// ids is better served by `bug_info` — this note says so. With an empty +/// `status` the query goes upstream bare, and Bugzilla routes a bare query +/// of nothing but numbers to an exact id lookup instead; the note still +/// steers to `bug_info` there, with wording that is true on that path. A +/// query naming more distinct ids than [`Guard::MAX_ASSESS_IDS`] gets +/// steering that mentions the per-call cap and batching — the cap is already +/// public in bug_info's refusal text and parameter doc — so the advice does +/// not walk the client straight into a refusal. +/// +/// The note is computed from the CLIENT'S OWN REQUEST ALONE (the query and +/// status strings): never from search results, guard verdicts, or anything +/// upstream said. That keeps it off the oracle surface — its presence and +/// wording tell the client nothing it did not already know (I2/I3) — and +/// the `bugs` array it accompanies is byte-identical to what the same +/// request returns without it. The query is NOT rerouted: a client genuinely +/// searching for a number still gets the search it asked for. +fn id_list_advisory(query: &str, status: &str) -> Option { + let mut ids: BTreeSet<&str> = BTreeSet::new(); + for token in query.split(|c: char| c == ',' || c.is_whitespace()) { + if token.is_empty() { + continue; + } + let digits = token.strip_prefix('#').unwrap_or(token); + if digits.is_empty() || !digits.bytes().all(|b| b.is_ascii_digit()) { + return None; + } + // Distinct ids, not distinct spellings: "07" names bug 7. + let canonical = digits.trim_start_matches('0'); + ids.insert(if canonical.is_empty() { "0" } else { canonical }); + } + if ids.is_empty() { + return None; + } + let semantics = if status.is_empty() { + "This query is only bug ids and, because status is empty, it is \ + passed to Bugzilla bare; Bugzilla treats a bare query of nothing \ + but numbers as an exact id lookup, not a content search." + } else { + "This query is only bug ids, but quicksearch matches bug text, so a \ + number also matches bugs that merely mention it." + }; + let steer = if ids.len() > Guard::MAX_ASSESS_IDS { + format!( + "For an exact set of known ids call bug_info with its bug_ids \ + array, at most {} ids per call — batch a longer list across \ + calls: every requested id comes back either as a bug or under \ + 'restricted'.", + Guard::MAX_ASSESS_IDS + ) + } else { + "For an exact set of known ids call bug_info with its bug_ids array: \ + every requested id comes back either as a bug or under 'restricted'." + .to_string() + }; + Some(format!("{semantics} {steer}")) +} + /// Project a bug object to the requested field set, preserving the /// `_redacted` marker when present. fn project_fields(bug: &Value, fields: &BTreeSet) -> Value { @@ -271,9 +334,17 @@ pub struct BugCommentsParams { #[derive(Debug, serde::Deserialize, schemars::JsonSchema)] pub struct QuicksearchParams { - /// Query in bugzilla quicksearch syntax. + /// Query in bugzilla quicksearch syntax. Under any non-empty status + /// (the default is ALL) this is content matching, not id lookup: a + /// number in the query is matched as text and returns bugs that merely + /// mention that number. With an empty status the query is sent to + /// bugzilla bare, and a bare query of nothing but numbers is an exact + /// id lookup. For an exact set of known bug ids use the bug_info tool + /// instead. pub query: String, - /// Status filter (e.g., ALL, OPEN, CLOSED). + /// Status filter (e.g., ALL, OPEN, CLOSED), prefixed to the query. + /// Empty means bugzilla's default search over open bugs — and a bare + /// query of nothing but numbers then becomes an exact id lookup. #[serde(default = "default_status")] pub status: String, /// Comma-separated list of fields to return for each bug. @@ -765,7 +836,7 @@ impl BugWarden { } #[tool( - description = "Search bugs using bugzilla's quicksearch syntax.\n\nTo reduce the token limit & response time, only returns a subset of fields for each bug. The user can query full details of each bug using the bug_info tool. Returns the top-level bug data envelope containing the matched bugs.", + description = "Search bugs by CONTENT using bugzilla's quicksearch syntax (full-text matching over bug summaries and text). The status filter is prefixed to the query expression, and under any non-empty status (the default is ALL) a number in the query is matched as text like any other word — it returns bugs that merely MENTION that number, not an id lookup. The one exception: an empty status sends the query to bugzilla bare, and bugzilla treats a bare query of nothing but numbers as an exact id lookup. Either way, for an exact set of known bug ids call bug_info with its bug_ids array instead: every requested id comes back either as a bug or under 'restricted', so an inaccessible id is reported rather than silently missing from a search.\n\nTo reduce the token limit & response time, only returns a subset of fields for each bug. The user can query full details of each bug using the bug_info tool. Returns the top-level bug data envelope containing the matched bugs.", annotations(read_only_hint = true, open_world_hint = true) )] async fn bugs_quicksearch( @@ -843,7 +914,14 @@ impl BugWarden { Guard::scrub_bug_links(bug, base_url, &allowed); } - Ok(ok_json(json!({ "bugs": projected }))) + let mut envelope = json!({ "bugs": projected }); + // Steering only, computed from the request the client itself sent + // (query and status) — never from results or verdicts (see + // id_list_advisory). + if let Some(note) = id_list_advisory(&p.query, &p.status) { + envelope["note"] = json!(note); + } + Ok(ok_json(envelope)) } #[tool( @@ -1559,7 +1637,7 @@ impl BugWarden { } #[tool( - description = "Access the documentation of the bugzilla quicksearch syntax. LLM can learn using this tool. Response is in HTML", + description = "Access the documentation of the bugzilla quicksearch syntax. LLM can learn using this tool. Response is in HTML. Note: through this server's bugs_quicksearch the status filter is prefixed to the query, so under any non-empty status (the default is ALL) a number in the query is content-matched as text; the syntax page's jump-to-bug-number shortcut applies only when status is empty and the query is nothing but numbers. Look up known bug ids with the bug_info tool.", annotations(read_only_hint = true, open_world_hint = true) )] async fn quicksearch_syntax(&self) -> Result { @@ -1896,6 +1974,61 @@ mod tests { ); assert!(too_many_ids(&[]).is_none()); } + + #[test] + fn id_list_advisory_fires_on_pure_id_lists_only() { + // Pure id lists: commas and/or whitespace, optional '#' per id. + assert!(id_list_advisory("123456", "ALL").is_some()); + assert!(id_list_advisory("#123456", "ALL").is_some()); + assert!(id_list_advisory("111, 222,333", "ALL").is_some()); + assert!(id_list_advisory("#111 #222\t333,", "ALL").is_some()); + + // Anything else is a content search and gets no note. + assert!(id_list_advisory("", "ALL").is_none()); + assert!(id_list_advisory(" ,\t", "ALL").is_none()); + assert!(id_list_advisory("#", "ALL").is_none()); + assert!(id_list_advisory("kernel crash 123", "ALL").is_none()); + assert!(id_list_advisory("123x", "ALL").is_none()); + assert!(id_list_advisory("product:openSUSE 42", "ALL").is_none()); + } + + #[test] + fn id_list_advisory_wording_tracks_status_and_id_count() { + // Non-empty status: the tool prefixes it, upstream content-matches. + let prefixed = id_list_advisory("101, 102", "ALL").expect("note"); + assert!(prefixed.contains("matches bug text"), "{prefixed}"); + assert!(!prefixed.contains("id lookup"), "{prefixed}"); + + // Empty status: the query goes upstream bare, where an all-number + // query is an exact id lookup — no content-matching claim there. + let bare = id_list_advisory("101, 102", "").expect("note"); + assert!(bare.contains("exact id lookup"), "{bare}"); + assert!(!bare.contains("matches bug text"), "{bare}"); + assert!(bare.contains("bug_info"), "{bare}"); + + // Over bug_info's per-call cap the steering mentions batching + // instead of walking the client into the too_many_ids refusal; the + // cap value is already public in that refusal's own text. + let over: Vec = (1..=Guard::MAX_ASSESS_IDS as u64 + 1) + .map(|i| i.to_string()) + .collect(); + let long = id_list_advisory(&over.join(" "), "ALL").expect("note"); + assert!( + long.contains(&format!("at most {} ids", Guard::MAX_ASSESS_IDS)), + "{long}" + ); + assert!(long.contains("batch"), "{long}"); + + // At the cap — and for repeated spellings of one id — no batching + // talk: the count is of distinct ids, like the refusal it averts. + let at_cap: Vec = (1..=Guard::MAX_ASSESS_IDS as u64) + .map(|i| i.to_string()) + .collect(); + let short = id_list_advisory(&at_cap.join(" "), "ALL").expect("note"); + assert!(!short.contains("batch"), "{short}"); + let repeats = id_list_advisory(&"7 07 #7 ".repeat(30), "ALL").expect("note"); + assert!(!repeats.contains("batch"), "{repeats}"); + } use super::*; use bugwarden_core::policy::Policy; diff --git a/crates/bugwarden/tests/tools_wiremock.rs b/crates/bugwarden/tests/tools_wiremock.rs index 8bd0b17..31ce033 100644 --- a/crates/bugwarden/tests/tools_wiremock.rs +++ b/crates/bugwarden/tests/tools_wiremock.rs @@ -11,7 +11,11 @@ //! - swapping `Capability::Attach` for `Capability::Attachments` at the //! add_attachment gate; //! - deleting the `may_create` call from create_bug; -//! - deleting the upload size-cap call from add_attachment. +//! - deleting the upload size-cap call from add_attachment; +//! - attaching the quicksearch id-list advisory only when the served `bugs` +//! array is non-empty; +//! - suppressing the quicksearch id-list advisory when I14 link scrubbing +//! removed anything. use std::sync::Arc; @@ -379,6 +383,268 @@ async fn add_attachment_size_cap_blocks_before_any_upload() { ); } +// ---------- bugs_quicksearch id-list advisory ---------- + +/// Mount an upstream that answers every quicksearch scan with `rows` and the +/// I14 link-disclosure padding fetch (`id=0`) with an empty envelope. +async fn mount_search(mock: &MockServer, rows: Vec) { + // Mounted first so it wins for the id=0 fetch; search requests carry no + // `id` parameter and fall through to the catch-all below. + Mock::given(method("GET")) + .and(path("/rest/bug")) + .and(query_param("id", "0")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ "bugs": [] }))) + .mount(mock) + .await; + Mock::given(method("GET")) + .and(path("/rest/bug")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ "bugs": rows }))) + .mount(mock) + .await; +} + +/// Parsed JSON of a successful quicksearch call for `query`. +async fn quicksearch_json(client: &RunningService, query: &str) -> Value { + quicksearch_json_args(client, json!({ "query": query })).await +} + +/// Parsed JSON of a successful quicksearch call with full `args`. +async fn quicksearch_json_args(client: &RunningService, args: Value) -> Value { + let result = call(client, "bugs_quicksearch", args).await; + assert!(!is_error(&result), "search failed: {}", text_of(&result)); + serde_json::from_str(&text_of(&result)).expect("quicksearch returns JSON") +} + +#[tokio::test] +async fn quicksearch_id_list_advisory_tracks_the_query_alone() { + // The upstream serves the same rows whatever the query, so any + // difference between the two results below is the server's own doing. + let mock = MockServer::start().await; + mount_search( + &mock, + vec![world_readable_bug(101), world_readable_bug(102)], + ) + .await; + let client = client_for("", &mock).await; + + // (a) A pure id-list query carries the advisory note. + let mut with_note = quicksearch_json(&client, "#101, 102").await; + let note = with_note["note"] + .as_str() + .expect("an id-list query must carry the advisory") + .to_string(); + assert!(note.contains("bug_info"), "the note must steer to bug_info"); + + // (b) A content query — even one containing a number — carries none. + let without = quicksearch_json(&client, "kernel crash 101").await; + assert!( + without.get("note").is_none(), + "a content query must not carry the advisory" + ); + + // (c) Apart from the note the envelopes are identical: same bugs, same + // order — the advisory never changes what is returned. + with_note.as_object_mut().unwrap().remove("note"); + assert_eq!(with_note, without); +} + +#[tokio::test] +async fn quicksearch_advisory_ignores_hidden_bugs() { + // Same policy, same id-list query, two upstreams: in one a returned bug + // is policy-hidden. The hidden bug is silently dropped (I3), but the + // advisory — presence and text — must not move: a note that tracked + // verdicts would be a brand-new oracle. + let policy = concat!( + "[[rule]]\nname = \"hide-secret\"\naction = \"deny\"\n", + "[rule.match]\nproducts = [\"Secret*\"]\n", + ); + + let plain = MockServer::start().await; + mount_search( + &plain, + vec![world_readable_bug(101), world_readable_bug(102)], + ) + .await; + let client = client_for(policy, &plain).await; + let served_both = quicksearch_json(&client, "101, 102").await; + assert_eq!(served_both["bugs"].as_array().unwrap().len(), 2); + let note_both = served_both["note"] + .as_str() + .expect("note present") + .to_string(); + + let hiding = MockServer::start().await; + let mut hidden = world_readable_bug(101); + hidden["product"] = json!("SecretSauce"); + mount_search(&hiding, vec![hidden, world_readable_bug(102)]).await; + let client = client_for(policy, &hiding).await; + let served_one = quicksearch_json(&client, "101, 102").await; + let bugs = served_one["bugs"].as_array().unwrap(); + assert_eq!(bugs.len(), 1, "the hidden bug is silently dropped"); + assert_eq!(bugs[0]["id"], json!(102)); + assert_eq!( + served_one["note"].as_str().expect("note still present"), + note_both, + "a hidden bug must not change the advisory" + ); +} + +#[tokio::test] +async fn quicksearch_advisory_survives_an_all_hidden_result() { + // Every matching bug is policy-hidden: the served `bugs` array is empty + // (I3), and the advisory must still be present, byte-identical to the + // note the same query carries when everything is visible. A note gated + // on served results would tell "no match" apart from "all matches + // hidden" — a fresh oracle. + let policy = concat!( + "[[rule]]\nname = \"hide-secret\"\naction = \"deny\"\n", + "[rule.match]\nproducts = [\"Secret*\"]\n", + ); + + let plain = MockServer::start().await; + mount_search( + &plain, + vec![world_readable_bug(101), world_readable_bug(102)], + ) + .await; + let client = client_for(policy, &plain).await; + let visible = quicksearch_json(&client, "101, 102").await; + assert_eq!(visible["bugs"].as_array().unwrap().len(), 2); + let reference_note = visible["note"].as_str().expect("note").to_string(); + + let hiding = MockServer::start().await; + let mut h1 = world_readable_bug(101); + h1["product"] = json!("SecretSauce"); + let mut h2 = world_readable_bug(102); + h2["product"] = json!("SecretSauce"); + mount_search(&hiding, vec![h1, h2]).await; + let client = client_for(policy, &hiding).await; + let empty = quicksearch_json(&client, "101, 102").await; + assert_eq!( + empty["bugs"].as_array().unwrap().len(), + 0, + "every match is policy-hidden" + ); + assert_eq!( + empty["note"] + .as_str() + .expect("the note must survive an empty result"), + reference_note, + "an all-hidden result must not move the advisory" + ); +} + +#[tokio::test] +async fn quicksearch_advisory_unmoved_by_link_scrubbing() { + // A served bug's depends_on names bug 666. Two upstreams, same policy, + // same request: in one 666 is world-readable, in the other it is + // policy-hidden, so I14 scrubbing empties the link field. The note — + // presence and bytes — must not move with it: scrubbed ids are invisible + // to the client, so a note that tracked scrubbing would be a covert + // verdict channel. + let policy = concat!( + "[[rule]]\nname = \"hide-secret\"\naction = \"deny\"\n", + "[rule.match]\nproducts = [\"Secret*\"]\n", + ); + let args = json!({ + "query": "101, 102", + "include_fields": "id,summary,depends_on", + }); + let mut linked = world_readable_bug(101); + linked["depends_on"] = json!([666]); + + // Control: the linked bug is disclosable, nothing is scrubbed. The + // id=666 mock is mounted before mount_search's catch-all so it wins the + // link-disclosure fetch; search requests carry no `id` parameter. + let plain = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/rest/bug")) + .and(query_param("id", "666")) + .respond_with( + ResponseTemplate::new(200).set_body_json(json!({ "bugs": [world_readable_bug(666)] })), + ) + .mount(&plain) + .await; + mount_search(&plain, vec![linked.clone()]).await; + let client = client_for(policy, &plain).await; + let unscrubbed = quicksearch_json_args(&client, args.clone()).await; + assert_eq!(unscrubbed["bugs"][0]["depends_on"], json!([666])); + let reference_note = unscrubbed["note"].as_str().expect("note").to_string(); + + // Same request, but 666 is policy-hidden: the link is scrubbed (I14) + // and the advisory must not react. + let hiding = MockServer::start().await; + let mut secret = world_readable_bug(666); + secret["product"] = json!("SecretSauce"); + Mock::given(method("GET")) + .and(path("/rest/bug")) + .and(query_param("id", "666")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ "bugs": [secret] }))) + .mount(&hiding) + .await; + mount_search(&hiding, vec![linked]).await; + let client = client_for(policy, &hiding).await; + let scrubbed = quicksearch_json_args(&client, args).await; + assert_eq!( + scrubbed["bugs"][0]["depends_on"], + json!([]), + "the hidden link must actually be scrubbed (I14)" + ); + assert_eq!( + scrubbed["note"] + .as_str() + .expect("the note must survive link scrubbing"), + reference_note, + "link scrubbing must not move the advisory" + ); +} + +#[tokio::test] +async fn quicksearch_advisory_wording_tracks_status_and_id_count() { + // Request-only steering, end to end: with an empty status the query + // goes upstream bare, where an all-number query is an exact id lookup, + // so the note must not claim content matching there; and a list longer + // than bug_info's per-call cap must steer to batching, not straight + // into the too_many_ids refusal. The upstream serves no rows at all, + // so every note below also rides on an empty `bugs` array. + let mock = MockServer::start().await; + mount_search(&mock, vec![]).await; + let client = client_for("", &mock).await; + + // Default (non-empty) status: content-matching wording, no batching. + let dflt = quicksearch_json(&client, "101, 102").await; + let dflt_note = dflt["note"].as_str().expect("id-list note"); + assert!(dflt_note.contains("matches bug text"), "{dflt_note}"); + assert!(!dflt_note.contains("id lookup"), "{dflt_note}"); + + // Explicitly empty status: id-lookup wording, still steering to + // bug_info. + let bare = quicksearch_json_args(&client, json!({ "query": "101, 102", "status": "" })).await; + let bare_note = bare["note"].as_str().expect("note on the bare path"); + assert!(bare_note.contains("exact id lookup"), "{bare_note}"); + assert!(!bare_note.contains("matches bug text"), "{bare_note}"); + assert!(bare_note.contains("bug_info"), "{bare_note}"); + + // 26 distinct ids: the steering mentions the cap and batching. + let long_query = (1..=26) + .map(|i| i.to_string()) + .collect::>() + .join(" "); + let long = quicksearch_json(&client, &long_query).await; + let long_note = long["note"].as_str().expect("note on a long id list"); + assert!(long_note.contains("at most 25 ids"), "{long_note}"); + assert!(long_note.contains("batch"), "{long_note}"); + + // 25 distinct ids: no batching talk. + let cap_query = (1..=25) + .map(|i| i.to_string()) + .collect::>() + .join(" "); + let cap = quicksearch_json(&client, &cap_query).await; + let cap_note = cap["note"].as_str().expect("note at the cap"); + assert!(!cap_note.contains("batch"), "{cap_note}"); +} + #[tokio::test] async fn add_attachment_comment_travels_as_a_plain_string() { // Bug.add_attachment documents `comment` as a plain string — NOT the diff --git a/docs/DESIGN.md b/docs/DESIGN.md index 1c933bf..7835b41 100644 --- a/docs/DESIGN.md +++ b/docs/DESIGN.md @@ -441,7 +441,7 @@ constraints the model must know. | bug_info | bug_ids: Vec | per-id: Read => full, else Summary => redacted, else restricted entry | envelope `{"bugs":[..], "restricted":[{"id":N,"note":denial(N)}]}`; full fetch only for Read-granted ids. Every fetched body is RE-CLASSIFIED before it is served (assemble_bug_info): the verdict came from the classification fetch and the body from a later request, so a bug embargoed in between must not be served on the stale verdict (TOCTOU; same reason download_attachment re-checks). Costs no request — the body is a superset of CLASSIFY_FIELDS — and a body that now earns only summary is served as the summary view, one that earns nothing becomes the uniform restricted entry. A failed body fetch is logged server-side ONLY and leaves those ids restricted: Bugzilla's message names the bug and says whether it exists, so forwarding it would undo I2 | | bug_history | id, new_since?: DateTime | history | | | bug_comments | id, include_private: bool = false, new_since? | comments | filter_comments applied (I5) | -| bugs_quicksearch | query, status: String = "ALL", include_fields: String = "id,product,component,assigned_to,status,resolution,summary,last_change_time", limit: u32 = 50, offset: u32 = 0 | post-filter | fetch include_fields = requested ∪ CLASSIFY_FIELDS; after filter, project kept bugs to requested fields (keep `_redacted` marker); envelope `{"bugs":[..]}` only (I3) | **limit/offset address the bugs the client may SEE, not upstream rows** (Guard::quicksearch_window): filtering an already-paginated page left a hole exactly where a hidden bug sat — a short page the next offset contradicted — and since quicksearch matches summary text that hole was a probe for the hidden title, one word at a time. The guard now scans upstream from row 0 in 200-row chunks, classifies each, and fills the window from the survivors; rows are deduped on the server-reported id (relevance order is not stable between calls) and an id-less row is dropped (I4). Bounds: MAX_SEARCH_WINDOW=1000 addressable, 2000 rows scanned (<=10 sequential requests); hitting either truncates, which looks exactly like the end of results. The objects returned are the ones classified. The scan target is quantised to whole chunks so the stopping point does not track the client's `limit`; without that, `limit` could be binary-searched against the clock to recover each block's exact hidden count. Residual, accepted: filling a window of VISIBLE bugs needs more rows when bugs are hidden, so a stopwatch still learns one bit per scanned block ("not entirely visible"). Removing that would mean scanning the worst case on every search, or letting pages go short again. Search failure returns a bare "Search failed"; the upstream text is logged server-side only (it can name a bug and say whether it exists) | +| bugs_quicksearch | query, status: String = "ALL", include_fields: String = "id,product,component,assigned_to,status,resolution,summary,last_change_time", limit: u32 = 50, offset: u32 = 0 | post-filter | fetch include_fields = requested ∪ CLASSIFY_FIELDS; after filter, project kept bugs to requested fields (keep `_redacted` marker); envelope `{"bugs":[..]}` only (I3), except an advisory `note` when the query is nothing but bug ids (comma/whitespace-separated, optional `#` per id) steering exact id sets to bug_info — the note is a pure function of the CLIENT'S REQUEST (the query and status strings), never of results, verdicts, or anything upstream said (no new oracle), and the `bugs` array is byte-identical with or without it (the query is still searched, never rerouted); its wording tracks the request: a non-empty status is prefixed to the query so upstream content-matches the whole expression, while an empty status sends the query bare and Bugzilla routes a bare all-number query to an exact id lookup (bug_id + anyexact) — on that path the note drops the content-matching claim — and a query naming more distinct ids than MAX_ASSESS_IDS steers to batched bug_info calls (the cap is already public in the too_many_ids refusal text) instead of straight into that refusal | **limit/offset address the bugs the client may SEE, not upstream rows** (Guard::quicksearch_window): filtering an already-paginated page left a hole exactly where a hidden bug sat — a short page the next offset contradicted — and since quicksearch matches summary text that hole was a probe for the hidden title, one word at a time. The guard now scans upstream from row 0 in 200-row chunks, classifies each, and fills the window from the survivors; rows are deduped on the server-reported id (relevance order is not stable between calls) and an id-less row is dropped (I4). Bounds: MAX_SEARCH_WINDOW=1000 addressable, 2000 rows scanned (<=10 sequential requests); hitting either truncates, which looks exactly like the end of results. The objects returned are the ones classified. The scan target is quantised to whole chunks so the stopping point does not track the client's `limit`; without that, `limit` could be binary-searched against the clock to recover each block's exact hidden count. Residual, accepted: filling a window of VISIBLE bugs needs more rows when bugs are hidden, so a stopwatch still learns one bit per scanned block ("not entirely visible"). Removing that would mean scanning the worst case on every search, or letting pages go short again. Search failure returns a bare "Search failed"; the upstream text is logged server-side only (it can name a bug and say whether it exists) | | create_bug | product, component, summary, version, description = "", severity?, priority?, op_sys?, platform?, keywords?: Vec, groups?: Vec | create (write), judged on the bug AS REQUESTED (Guard::may_create) | there is no bug id to assess, so the request itself is classified BEFORE any upstream call (I8): the rules that hide a product by name refuse filing into it, a field the request omits fails closed (I4), and a client-claimed `groups` list is never trusted — Bugzilla unions the product's mandatory groups in server-side, so may_create forces groups to unknown, which means a policy whose applicable rules consult groups/group_restricted refuses ALL creation. **Both refusals are one refusal**: a policy refusal and an upstream failure return the same fixed create_denial text after the same single upstream request — the refused path burns one classify call against bug id 0 (never a valid id, creates nothing; download_attachment's padding precedent) instead of the POST. Two texts, or 0 vs 1 requests, would be a free policy-enumeration oracle: send a guaranteed-invalid `version` plus a probe product and read the policy off which refusal (or which latency) comes back, with nothing created. Residual, accepted: a SUCCESSFUL create still confirms the product is allowed — that is the tool doing its job, and it costs a real, attributable bug; and the padding equalizes request count, not the upstream handler's exact latency (GET classify vs rejected POST). Bugzilla's failure message is logged server-side only (it can say whether a product/component exists) | | add_attachment | bug_id, data (base64), file_name, summary, content_type, comment = "", is_private = false, is_patch = false | attach (write) on bug_id | guard assessment before the upload (I8), uniform denial (I2); then global.max_attachment_bytes caps the DECODED size of `data` (0 = no cap) — the ceiling the operator set on downloads binds uploads through the same server too, measured after base64 expansion is stripped so encoding overhead cannot shrink it. The refusal names neither the payload's size nor the cap value (max_attachment_bytes is not I1-disclosable, exactly as on the download path). `comment` travels as a PLAIN string — Bug.add_attachment documents it so; the `{"comment": {"body": ..}}` shape belongs to Bug.update only | | add_comment | bug_id, comment, is_private: bool = false | comment (write) | |