test(audit): pin the suppressed_count zero guard and list_attachments

crates/bugwarden/tests/audit_wiremock.rs

@@ -864,6 +864,174 @@ async fn suppressed_count_totals_both_populations_with_the_ids_elided() {
     );
 }
 
+#[tokio::test]
+async fn bug_comments_with_nothing_filtered_stays_served_and_counts_zero() {
+    // Issue #87: the id-less counter is fed on EVERY call of these tools,
+    // with `0` on the common path where the private filter dropped
+    // nothing. `note_suppressed_count` returns early on zero for exactly
+    // that reason — without it this record would claim `served_filtered`
+    // over an empty suppression, and the one field an operator filters on
+    // to find the calls the guard acted upon would say the same thing
+    // about every call.
+    let mock = MockServer::start().await;
+    mount_fixture(&mock).await;
+    let audited = audited_client_for(HIDE_SECRET_POLICY, &mock, "test-key").await;
+
+    let result = call(&audited.client, "bug_comments", json!({ "id": 7 })).await;
+    assert_ne!(result.is_error, Some(true), "bug 7's comments are served");
+
+    let events = read_events(&audited.audit_path);
+    let tc = last_tool_call(&events);
+    assert_eq!(tc.request.tool, "bug_comments");
+    let guard = tc.guard.as_ref().expect("guard info recorded");
+    assert_eq!(
+        guard.verdict,
+        Verdict::Served,
+        "the fixture's only comment is public and names no bug, so nothing \
+         was withheld: {guard:?}"
+    );
+    assert_eq!(guard.suppressed_count, 0);
+    assert!(guard.suppressed_ids.is_empty());
+    assert_eq!(
+        guard.rule.as_deref(),
+        Some("default"),
+        "and the rule that decided the call survives — a filtered merge \
+         outranks a clean serve and would clear it"
+    );
+}
+
+/// One row of bug 7's attachment metadata, as Bugzilla serves it with the
+/// content excluded.
+fn attachment(id: u64, is_private: bool, file_name: &str) -> Value {
+    json!({
+        "id": id,
+        "bug_id": 7,
+        "is_private": is_private,
+        "file_name": file_name,
+        "summary": "attached",
+        "content_type": "text/plain",
+        "size": 3,
+    })
+}
+
+/// Serve `rows` as bug 7's attachment metadata — the `list_attachments`
+/// counter site. The private rows are what the I5 gate drops, and what it
+/// drops carries no bug id of its own: this tool names no ids at all, so
+/// the call feeds the id-less tally and nothing else.
+async fn mount_attachments(mock: &MockServer, rows: Vec<Value>) {
+    Mock::given(method("GET"))
+        .and(path("/rest/bug"))
+        .and(query_param("id", "7"))
+        .respond_with(ResponseTemplate::new(200).set_body_json(json!({ "bugs": [world_bug(7)] })))
+        .mount(mock)
+        .await;
+    Mock::given(method("GET"))
+        .and(path("/rest/bug/7/attachment"))
+        .respond_with(ResponseTemplate::new(200).set_body_json(json!({ "bugs": { "7": rows } })))
+        .mount(mock)
+        .await;
+}
+
+#[tokio::test]
+async fn list_attachments_counts_the_private_metadata_it_dropped() {
+    // Issue #87: the third id-less site. Two private attachments the I5
+    // gate withheld are counted and never named, so this record carries a
+    // non-zero count over an EMPTY id list — the shape only this tool
+    // produces, and the reason nothing else was pinning its counter.
+    let mock = MockServer::start().await;
+    mount_attachments(
+        &mock,
+        vec![
+            attachment(51, false, "log.txt"),
+            attachment(52, true, "canary-private-3f7b-one.txt"),
+            attachment(53, true, "canary-private-3f7b-two.txt"),
+        ],
+    )
+    .await;
+    let audited = audited_client_for(HIDE_SECRET_POLICY, &mock, "test-key").await;
+
+    let result = call(&audited.client, "list_attachments", json!({ "bug_id": 7 })).await;
+    assert_ne!(result.is_error, Some(true), "the public metadata is served");
+    let envelope = serde_json::to_string(&result).unwrap();
+    assert!(
+        !envelope.contains("canary-private-3f7b"),
+        "private attachment metadata must not reach the client (I5): {envelope}"
+    );
+    // Nor into the operator's own stream: the record counts the drop, it
+    // does not describe what was dropped.
+    let raw = std::fs::read_to_string(&audited.audit_path).expect("audit file readable");
+    assert!(
+        !raw.contains("canary-private-3f7b"),
+        "the withheld metadata must not reach the audit file either"
+    );
+
+    let events = read_events(&audited.audit_path);
+    let tc = last_tool_call(&events);
+    assert_eq!(tc.request.tool, "list_attachments");
+    let guard = tc.guard.as_ref().expect("guard info recorded");
+    assert_eq!(guard.verdict, Verdict::ServedFiltered);
+    assert_eq!(
+        guard.suppressed_count, 2,
+        "both dropped attachments are counted: {guard:?}"
+    );
+    assert!(
+        guard.suppressed_ids.is_empty(),
+        "and neither is named — list_attachments names no ids at all: {:?}",
+        guard.suppressed_ids
+    );
+}
+
+#[tokio::test]
+async fn list_attachments_with_nothing_private_stays_served_and_counts_zero() {
+    // Issue #87 at the CALL SITE, which the zero guard alone does not
+    // cover: an unconditional note added beside the counter — a
+    // `note_redacted`, a rule-less `note_verdict` — leaves the `n == 0`
+    // early return intact and still marks every clean listing
+    // `served_filtered`.
+    //
+    // Two PUBLIC attachments rather than an empty list: the filter keeps
+    // both, so the recorded zero means "nothing was withheld" and not
+    // "there was nothing to withhold", and a counter fed the list length
+    // instead of the drop fails here as well.
+    let mock = MockServer::start().await;
+    mount_attachments(
+        &mock,
+        vec![
+            attachment(51, false, "log.txt"),
+            attachment(52, false, "trace.txt"),
+        ],
+    )
+    .await;
+    let audited = audited_client_for(HIDE_SECRET_POLICY, &mock, "test-key").await;
+
+    let result = call(&audited.client, "list_attachments", json!({ "bug_id": 7 })).await;
+    assert_ne!(result.is_error, Some(true), "both attachments are served");
+    let envelope = serde_json::to_string(&result).unwrap();
+    for file_name in ["log.txt", "trace.txt"] {
+        assert!(
+            envelope.contains(file_name),
+            "the gate kept {file_name}, so nothing was withheld: {envelope}"
+        );
+    }
+
+    let events = read_events(&audited.audit_path);
+    let tc = last_tool_call(&events);
+    assert_eq!(tc.request.tool, "list_attachments");
+    let guard = tc.guard.as_ref().expect("guard info recorded");
+    assert_eq!(
+        guard.verdict,
+        Verdict::Served,
+        "a listing that withheld nothing is a clean serve: {guard:?}"
+    );
+    assert_eq!(guard.suppressed_count, 0);
+    assert!(guard.suppressed_ids.is_empty());
+    assert!(
+        guard.redacted_fields.is_empty(),
+        "and nothing was redacted from it either: {:?}",
+        guard.redacted_fields
+    );
+}
+
 /// Serve a quicksearch corpus (issue #29 tests): search requests are paged
 /// by limit/offset the way Bugzilla pages them, and the I14 link-disclosure
 /// classify fetch — which addresses bugs by id, not by query — gets an

docs/DESIGN.md

@@ -1097,12 +1097,17 @@ Decisions, all deliberate:
   call: a refusal answered from the request alone, the pre-dispatch gate
   (the guard never ran), a search (the verdict is the window's, not one
   bug's), the create gate on either arm (it judges the request as a
-  whole), an id with no matching `Access`, and the attachment withhold
-  together with its constant-cost bug-0 padding assessment. A tool the
-  router never carried (I13) is not this case at all: it records no
-  `guard` object. Re-encoding a default decision AS absence would be a
-  record-schema change and is deferred to #34; schema v1 records
-  `"default"`.
+  whole), an id with no matching `Access`, the attachment withhold
+  together with its constant-cost bug-0 padding assessment, and a SERVE
+  the cell later upgraded to `served_filtered` through a rule-less note —
+  a suppression, a redaction, a dropping scan — since that note outranks
+  the grant and the worst-wins merge clears the rule with it (a
+  `list_attachments` call that dropped private metadata is the plain
+  case: the default granted the bug, and the record still names no
+  rule). A tool the router never carried (I13) is not this case at all:
+  it records no `guard` object. Re-encoding a default decision AS
+  absence would be a record-schema change and is deferred to #34; schema
+  v1 records `"default"`.
 - **What `guard.suppressed_count` totals (issue #68).** The cell keeps two
   tallies: a `BTreeSet` of bug ids the guard withheld (`note_suppressed` —
   I14-scrubbed links, scrubbed duplicate markers, verdict-dropped search
@@ -1673,7 +1678,24 @@ wired, `server.rs` and `main.rs` are the reference.
   DEFENDS the disjointness the sum rests on: harvesting marker ids from
   the pre-filter comment list counts that one comment in both tallies
   and records `6` over three ids, and both tools' tests fail on the
-  number (plus the cell-level sum tests in audit.rs).
+  number (plus the cell-level sum tests in audit.rs); and the counter's
+  zero guard (issue #87) — a `bug_comments` call whose private filter
+  dropped nothing records verdict `served`, `suppressed_count == 0` and
+  the rule that decided it, so deleting `note_suppressed_count`'s
+  `n == 0` early return, which would merge `served_filtered` (rule-less,
+  clearing the rule) on EVERY call of the three tools that feed the
+  counter, fails on the verdict rather than on a count that stays `0`
+  either way; and `list_attachments`, the id-less site whose guard fields
+  no record assertion had reached (the one-record-per-call test calls the
+  tool but reads only its envelope), records a non-zero count over an
+  EMPTY `suppressed_ids` when the I5 gate drops private attachment
+  metadata, and `served` with a zero count over a listing of PUBLIC
+  attachments the gate kept — a real "nothing was withheld" rather than
+  an empty list's "nothing to withhold". Between them: deleting that
+  counter call, counting the whole list rather than the drop, and adding
+  any unconditional note beside it (a `note_redacted`, a rule-less
+  `note_verdict`) — which the zero guard, living inside
+  `note_suppressed_count`, cannot stop — are all detectable.
 - Identity tests (#[cfg(test)] in crates/bugwarden/src/server.rs and
   crates/bugwarden-core/src/client.rs; crates/bugwarden/tests/
   http_transport_wiremock.rs, crates/bugwarden/tests/binary_user_agent.rs