fix: scrub denied bug ids from bodies, history and comments - #19
Merged
Conversation
A bug the policy hides could still name itself inside a bug the client was allowed to read. bug_info returned Bugzilla's default field set, so blocks, depends_on, dupe_of, see_also and url carried the ids of hidden bugs; bugs_quicksearch let the client CHOOSE those fields and returned them wholesale; bug_history returned change records untouched, and those record exactly which bug was linked or unlinked and when; bug_comments and summarize_bug passed through Bugzilla's own duplicate markers. Uniform denials are pointless when the neighbouring bug names the secret. The write paths already refuse to CREATE such a link without assessing the target — reading one back out must not be the way around that. So the same bar applies: an id may be named inside served content only if it earns Capability::Summary, recorded as invariant I14 and applied on all five paths. Only ids actually SERVED in the same response are exempt. A requested id that was DENIED is not: whitelisting it would mean that naming a hidden bug in the request reveals it through the links of one the client may read, turning the scrub into an existence oracle for any id worth guessing. Candidate ids come from Bugzilla's answer rather than from the client, so unlike assess() they are judged in one bounded batch whatever their number. That request is issued even when there is nothing to assess: skipping it would let the clock separate "this bug has no links" from "every link is hidden". Both stock duplicate-marker templates are matched, in both directions — the one written on the duplicate is preceded by whatever the closer typed, so neither can be found by looking at the start of a comment. see_also and url are matched host-and-path, case-insensitively and either scheme, since Bugzilla stores them as the user typed them. Limits recorded rather than papered over: free text where a person writes a bug number is untouched, localised marker templates are not recognised, and an instance reachable under a second hostname is not matched in see_also. Also corrects the previous commit's rationale for dropping the batch in assess(): whether a batch omits an unserviceable id or fails outright is deployment-dependent (stock /rest/bug?id=.. omits), so what the old code actually leaked was the retry that fired only on failure.
plusky
force-pushed
the
fix/scrub-denied-bug-ids
branch
from
July 27, 2026 20:32
8fc148a to
0bbcd16
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The hole
A bug the policy hides could still name itself inside a bug the client was
allowed to read:
bug_infofetched withinclude_fields: None, i.e. Bugzilla's defaultfield set — so
blocks,depends_on,dupe_ofandsee_alsocarried theids of hidden bugs straight to the client.
bug_historyreturned change records untouched, and those recordprecisely which bug was linked or unlinked, by whom, and when.
bug_commentspassed through Bugzilla's own*** Bug N has been marked as a duplicate of this bug ***.Uniform denials (I2) are pointless when the neighbouring bug names the secret.
The asymmetry was stark:
update_bug_dependenciesandmark_as_duplicatealready assess every link target before writing one, on the grounds that
such a link leaks existence — while every read path handed the same links back
out for free.
The rule
An id may be named inside a served body only if it earns
Capability::Summary— the same bar the write paths use (I8/I11), recordednow as I14.
Candidate ids come from Bugzilla's answer, not from the client, so unlike
assessthey are judged in one batched request whatever their number: aclient cannot choose them in order to probe with them. A failed assessment
scrubs everything (I4).
see_alsois matched against this instance's own base URL — a link to anothertracker is not ours to scrub, and a host that merely starts with ours is not
ours to trust either.
What it does to the output
blocks/depends_on/regressed_by/…dupe_ofnullsee_alsoLimits, deliberate and recorded
Free text where a person writes a bug number is untouched: recognising that
would mean gutting comments. And the duplicate marker is matched as template
text, so an instance with localised or customised templates evades it — a
mitigation for what Bugzilla itself writes, not a guarantee. Both are in
DESIGN.md under I14 rather than left for a reviewer to discover.
Verification
157 tests (+6). They pin: ids collected from every id-bearing field including
see_alsoURLs in both forms; only non-disclosable ids removed while foreigntrackers survive;
dupe_ofblanked; history changes edited down andfully-scrubbed entries dropped; duplicate markers recognised but ordinary
prose mentioning a bug number not mistaken for one; and
see_alsomatchinganchored so
evil.exampleandother.cgi?id=are not treated as local links.