Repository navigation
Add Bugzilla proxy and scoped token flow - #6740
Draft
suhaibmujahid wants to merge 2 commits into
Draft
suhaibmujahid wants to merge 2 commits into
suhaibmujahid wants to merge 2 commits into
Conversation
suhaibmujahid
force-pushed
the
worktree-bugzilla-proxy-design
branch
from
October 26, 2026 14:52
53d8fb0 to
3d36c0c
Compare
Ask BMO to leave out private bugs on public-only searches
The proxy filters search results after they arrive, so for a scope that
can see no private bug, every private bug BMO returned used up a row of
the limit and left the page short.
A search under such a scope, which includes every passthrough token,
now carries one more condition, bug_group isempty, numbered after the
caller's highest so it cannot replace one. It is an efficiency, not the
guard: the caller controls the rest of the search, and j_top=OR or a
negated group can neutralize it (verified on bugzilla.mozilla.org), so
every result is still checked against the grants.
Fix the key rotation test's import of a conftest helper
Test modules cannot import from conftest under pytest's default import
mode, so the rotation test failed with ModuleNotFoundError before
exercising anything. Expose the JWK builder as a fixture instead.
Stop query parameters from widening a bugzilla-proxy request
BMO merges query parameters into the call a path maps to, so
bug/X/comment?ids=Y returned bug Y's comments after only X was
authorized, comment_ids pulled comments from any bug, and
bug/attachment/N?ids=B added every attachment on bug B.
The per-bug endpoints now forward an allowlist of parameters. Comment
requests keep supporting ids, which get_bugs relies on, by authorizing
each bug in one upstream lookup and forwarding only those in scope.
Responses are cut down to the authorized bugs and attachments, so an
upstream surprise cannot leak either.
Never inline comments, attachments or history in proxy search
BMO inlines all three into bugs on request, via include_fields or a
group such as _all. Search authorizes against the bug endpoint only, so
a token without comment access, or with attachments off, could read
both through search instead.
Search now drops them from the upstream request and the response.
Their dedicated endpoints remain the one way to read them, which keeps
their authorization, and the filtering still to come, in one place.
Emit the bugzilla-proxy audit log when served by uvicorn
The container starts the app with uvicorn bugzilla_proxy.main:app,
which never calls main(), where logging was configured. uvicorn sets up
only its own loggers, so the root logger stayed at WARNING and every
INFO audit line was dropped while warnings still appeared.
Configure logging at import, as hackbot-api does, and test it by
importing the module the way uvicorn does in a fresh interpreter.
Serve private comments and attachments only to private grants
A comment or attachment marked is_private is visible upstream to
insiders whatever the bug's own groups, so a public bug can carry
private content. Harmless while the proxy's credential sees no more
than the agents' key, it would leak once that credential is privileged.
The proxy now drops is_private content unless the grant serving the
bug is cleared for private data, which already requires a confidential
run. A missing is_private counts as private, and the field is always
requested so a caller's include_fields cannot hide it.
Keep every value of a repeated query parameter in the proxy
query_params.items() keeps only the last value of a repeated name, so a
search for product=Core&product=Firefox reached BMO as Firefox alone.
Forward every value, join repeats of the comma-list parameters the
proxy parses itself, and refuse an ambiguous repeated limit.
Mint the Bugzilla scope token off the event loop
Signing goes through IAM's signJwt, a blocking network call, and ran
directly inside the async create_run handler, stalling every other
request on the instance while it waited. Run it in a thread, as
trigger_execution already does.
Also cover what happens when minting fails: the run is already
committed by then, so it has to be marked failed rather than left
pending.
Verify proxy tokens off the event loop and bound key refetches
PyJWKClient fetches the issuer's keys with a blocking HTTP call, which
ran inside the async handler. Verification now runs in a thread.
An unknown key id forced a refetch from Google on every request, so a
stream of tokens with made-up key ids cost one outbound request each.
Forced refetches are now at most one per 30 seconds; Google publishes a
key before signing with it, so the refetch that does happen still picks
up a real rotation.
A failure to reach Google's key endpoint was reported to the caller as
an untrusted token. It is now a 503, still refusing the request but
telling the caller to retry. A malformed token is also refused before
any key lookup instead of escaping as an unhandled decode error.
Refuse to start a confidential run when tokens cannot be minted
With no signing key configured, broker_env_for returned nothing and the
broker fell back to its own Bugzilla credential. That is the intended
rollback for public scopes, but a confidential scope would then run
without the proxy, and so without the scope and containment it was
declared with. Raise instead, which marks the run failed.
Answer the brokers' version probe in bugzilla-proxy
The build-repair broker requests /rest/version at startup to check its
URL and key. Through the proxy that was a 101, logged as a failed
Bugzilla health check on every start. Serve it to any valid token,
passing on only the version string.
Fix ruff findings in bugzilla-proxy scope code and tests
The repo's pinned ruff flags SIM103 in Anchor.matches and SIM300 in two
scope test assertions, which pre-commit would fail on. No behaviour
change.
Document the bugzilla-proxy hardening
Describe the parameter allowlist, the search and private-content
filtering, the logging fix and the confidential fail-closed rule in the
service README and the design doc's phase 0 section.
Refuse an out-of-range search limit instead of adjusting it
A limit above the ceiling was capped, 0 was replaced by the ceiling, and
a negative limit went to BMO unchanged, so the caller got fewer rows
than asked for, or a different query, without being told. Each is now a
400 naming the accepted range, and pointing at offset for paging. An
absent limit still gets the ceiling, as BMO applies its own default.
Adds error code 104 for requests the proxy refuses as written, and
moves the existing malformed-request errors onto it from 103, which
means an upstream failure.
Refuse unsupported and credential parameters instead of dropping them
The per-bug endpoints silently dropped any parameter outside their
allowlist, and every endpoint silently stripped credentials from the
query string, so a caller got an answer to a different question than it
asked, with no hint why. Both are now a 400 naming the offending
parameters; the per-bug endpoints also list what they support, and the
credential error says where the token belongs.
Rejection happens before any upstream call, and before the bug is
looked up, so it reveals nothing about the bug.
Refuse search fields a token cannot return instead of omitting them
Search silently stripped comments, attachments and history, and a field
the token's tier hides simply went missing from the response. A caller
had no way to tell a refused field from an empty one.
Search now refuses, with a 400 saying what to do instead:
- comments, attachments, history, and description (comment 0's text,
which BMO also inlines), pointing at the endpoint that serves each;
- the _all and _extra groups, which BMO expands to include them;
- a field, or a widening group, that no grant on the token can return,
listing the fields it does expose.
Fields the proxy adds for its own decisions stay invisible to the
caller: excluding groups now hides the field instead of quietly denying
every bug, and is_private, added to comment and attachment requests for
filtering, is only returned when the caller's own field lists ask for
it.
Fail a comment request naming bugs in ids the run may not see
An out-of-scope bug in a comment request's ids was dropped before the
upstream call, so the caller got comments for fewer bugs than it named
and could not tell why. The request now fails with a 102 listing those
bugs, without calling upstream.
This matches BMO, which fails the whole call on an inaccessible id, so
the denial reveals nothing BMO would not. Out-of-scope and nonexistent
bugs are listed alike.
Tell the caller why the proxy rejected its token
Every token failure reached the caller as "This run is not authorized",
so an expired token, one for the wrong audience and a malformed header
all looked the same. The response now carries the reason: expired, not
yet valid, wrong audience, untrusted issuer, missing claim, bad
signature or malformed, plus where to send the token when none came.
The reasons describe the token alone, never anything from Bugzilla, and
none helps forge a token, whose signature still has to verify.
Make proxy denials reveal no more than Bugzilla would
The proxy's credential can see more than a caller, so a denial that
varies with its reason reports what that credential saw.
- An attachment denied because of its bug answered "Bug N is not
available", telling the caller which bug a hidden attachment belongs
to. It now names only the attachment.
- BMO refusing an attachment to the proxy's credential was passed on as
a 502 with BMO's text, unlike every other unavailable attachment. It
now gets the same 102.
- Every denial reads "does not exist or is outside this run's Bugzilla
scope", which is both clearer about what to do and identical for every
reason. The reason goes to the audit log only.
tests/test_disclosure.py checks each endpoint against every way a bug
or attachment can be unavailable, and that no Bugzilla data appears in
a denial. The README and design doc now state the error policy: refuse
rather than adjust, and filter silently only where BMO does.
Bring the proxy docs in line with refusing rather than dropping
The hardening notes still described parameters as dropped and search
fields as stripped, and called a limited search "capped". Describe
what the proxy does now.
Read with an unprivileged Bugzilla key unless a scope needs private access
The proxy held one Bugzilla key and used it for every token. Once that
key belongs to an account in security groups, a public-only token would
be kept away from private bugs by the proxy's own checks alone.
The proxy now holds two keys. BUGZILLA_API_KEY becomes
BUGZILLA_PRIVILEGED_API_KEY, alongside a new
BUGZILLA_UNPRIVILEGED_API_KEY from an account in no groups, and the key
is chosen per token from its scope: unprivileged for any scope without
a private grant, privileged only for one with. For every public-only
scope a read then has to pass both the proxy and BMO, while the proxy's
checks still deny a security bug the unprivileged account can see, for
example because it was cc'd. Which key reads never changes what a
request may look like, so moving an agent between public and private
scopes is a change to its template only.
Each key has its own upstream client and its own decision cache, since
the two see different bugs. The proxy refuses to start if the keys are
the same, and a token needing a key that is not configured gets a 503
saying so. Both clients send BUGZILLA_EDGE_KEY as Mozilla-Edge-Key,
named as hackbot-api already names it, exempting the proxy from BMO's
edge rate limiting; a per-run limit is a follow-up.
Stream single attachments instead of holding them in memory
Serving bug/attachment/{id} parsed BMO's whole response, base64 data
included, to find the attachment's bug before deciding, so a large
attachment was held in memory, about a third bigger than the file.
The proxy now fetches only id, bug_id and is_private, decides on that,
and only then requests the full response, which it streams through
unread. A denied attachment never has its data requested. Since the
body is no longer inspected, the parameter allowlist is what keeps it
to this one attachment: ids and attachment_ids are already refused.
Applies to both modes, so they still answer alike. The response is now
BMO's own, so it may carry BMO's empty bugs map alongside attachments.
This branch has not been deployed
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
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.
Resolves #6694