From 53d8fb07684199e3aa6895e2f6a8b85a3c3b5fb3 Mon Sep 17 00:00:00 2001 From: Suhaib Mujahid Date: Sun, 23 Aug 2026 21:53:21 -0400 Subject: [PATCH] Add Bugzilla proxy and scoped token flow --- .gitignore | 1 + docs/hackbot/README.md | 25 +- docs/hackbot/bugzilla-proxy.md | 410 ++++++++++++++ docs/hackbot/security.md | 11 + services/bugzilla-proxy/Dockerfile | 35 ++ services/bugzilla-proxy/README.md | 90 ++++ .../bugzilla-proxy/bugzilla_proxy/__init__.py | 0 services/bugzilla-proxy/bugzilla_proxy/app.py | 314 +++++++++++ .../bugzilla-proxy/bugzilla_proxy/config.py | 49 ++ .../bugzilla-proxy/bugzilla_proxy/main.py | 22 + .../bugzilla-proxy/bugzilla_proxy/scope.py | 311 +++++++++++ .../bugzilla-proxy/bugzilla_proxy/tokens.py | 259 +++++++++ .../bugzilla-proxy/bugzilla_proxy/upstream.py | 67 +++ .../bugzilla-proxy/docker-compose.dev.yml | 20 + services/bugzilla-proxy/pyproject.toml | 37 ++ services/bugzilla-proxy/tests/conftest.py | 174 ++++++ services/bugzilla-proxy/tests/test_app.py | 269 ++++++++++ .../tests/test_bugsy_contract.py | 84 +++ services/bugzilla-proxy/tests/test_scope.py | 203 +++++++ services/bugzilla-proxy/tests/test_tokens.py | 310 +++++++++++ services/hackbot-api/app/agents.py | 10 + services/hackbot-api/app/bz_token.py | 252 +++++++++ services/hackbot-api/app/config.py | 15 + services/hackbot-api/app/jobs.py | 58 +- services/hackbot-api/app/routers/runs.py | 17 +- services/hackbot-api/pyproject.toml | 2 + services/hackbot-api/tests/test_bz_token.py | 503 ++++++++++++++++++ .../hackbot-api/tests/test_create_run_api.py | 2 +- uv.lock | 78 ++- 29 files changed, 3591 insertions(+), 37 deletions(-) create mode 100644 docs/hackbot/bugzilla-proxy.md create mode 100644 services/bugzilla-proxy/Dockerfile create mode 100644 services/bugzilla-proxy/README.md create mode 100644 services/bugzilla-proxy/bugzilla_proxy/__init__.py create mode 100644 services/bugzilla-proxy/bugzilla_proxy/app.py create mode 100644 services/bugzilla-proxy/bugzilla_proxy/config.py create mode 100644 services/bugzilla-proxy/bugzilla_proxy/main.py create mode 100644 services/bugzilla-proxy/bugzilla_proxy/scope.py create mode 100644 services/bugzilla-proxy/bugzilla_proxy/tokens.py create mode 100644 services/bugzilla-proxy/bugzilla_proxy/upstream.py create mode 100644 services/bugzilla-proxy/docker-compose.dev.yml create mode 100644 services/bugzilla-proxy/pyproject.toml create mode 100644 services/bugzilla-proxy/tests/conftest.py create mode 100644 services/bugzilla-proxy/tests/test_app.py create mode 100644 services/bugzilla-proxy/tests/test_bugsy_contract.py create mode 100644 services/bugzilla-proxy/tests/test_scope.py create mode 100644 services/bugzilla-proxy/tests/test_tokens.py create mode 100644 services/hackbot-api/app/bz_token.py create mode 100644 services/hackbot-api/tests/test_bz_token.py diff --git a/.gitignore b/.gitignore index f3e58ba936..7fbeb9d173 100644 --- a/.gitignore +++ b/.gitignore @@ -21,6 +21,7 @@ sheets/ .pytest_cache/ *.pyc .coverage +.venv/ # Distribution / packaging .Python diff --git a/docs/hackbot/README.md b/docs/hackbot/README.md index a6835f4cbf..f5f2757f58 100644 --- a/docs/hackbot/README.md +++ b/docs/hackbot/README.md @@ -48,18 +48,19 @@ what it proposed. ## Where to read next -| If you want to… | Read | -| ---------------------------------------------------------- | ---------------------------------- | -| Understand the components and why they're split that way | [architecture.md](architecture.md) | -| Write or modify an agent | [agents.md](agents.md) | -| Know what the runtime hands your agent | [runtime.md](runtime.md) | -| Find a tool your agent can call, or add one | [tools.md](tools.md) | -| Understand how agents change the world (record-then-apply) | [actions.md](actions.md) | -| Work on the control-plane service | [api.md](api.md) | -| Know how runs get started | [triggers.md](triggers.md) | -| Reason about credentials and trust boundaries | [security.md](security.md) | -| Deploy, configure, or run things locally | [deployment.md](deployment.md) | -| Look at traces of a run | [tracing.md](tracing.md) | +| If you want to… | Read | +| ---------------------------------------------------------- | -------------------------------------- | +| Understand the components and why they're split that way | [architecture.md](architecture.md) | +| Write or modify an agent | [agents.md](agents.md) | +| Know what the runtime hands your agent | [runtime.md](runtime.md) | +| Find a tool your agent can call, or add one | [tools.md](tools.md) | +| Understand how agents change the world (record-then-apply) | [actions.md](actions.md) | +| Work on the control-plane service | [api.md](api.md) | +| Know how runs get started | [triggers.md](triggers.md) | +| Reason about credentials and trust boundaries | [security.md](security.md) | +| Follow the proposed private-Bugzilla access design | [bugzilla-proxy.md](bugzilla-proxy.md) | +| Deploy, configure, or run things locally | [deployment.md](deployment.md) | +| Look at traces of a run | [tracing.md](tracing.md) | ## Code map diff --git a/docs/hackbot/bugzilla-proxy.md b/docs/hackbot/bugzilla-proxy.md new file mode 100644 index 0000000000..00d88170c6 --- /dev/null +++ b/docs/hackbot/bugzilla-proxy.md @@ -0,0 +1,410 @@ +# Bugzilla proxy + +> **Status: the service is built, nothing uses it yet.** Phases 1 to 4 are proposed. +> [services/bugzilla-proxy/](../../services/bugzilla-proxy/) exists and is tested, and +> hackbot-api can mint tokens. **No agent is wired to it**: no `AgentSpec` sets a +> `bugzilla_scope`, so no token is minted and every broker still uses its own Bugzilla +> credential. Onboarding the first agent comes after the service is deployed and tested. +> No private bug is reachable through any of this: that is phase 3, and it depends on the +> containment work in phase 2. See [Delivery phases](#delivery-phases). + +## The problem + +Today each agent that needs Bugzilla ships a `broker` sidecar holding a real BMO API key +([security.md](security.md)). That key is a single static credential: every run using a +given agent gets exactly the same access, and the only reason it is safe is that the +account behind it can read nothing confidential. + +Several things we want to build need private bugs. A triage or fix agent pointed at a +security bug cannot read it. A security-bug de-duplication agent cannot search the corpus +it would need to search. Widening the broker's key is not an option: it would give every +run of that agent standing access to everything the account can see, with no per-run limit, +no audit trail beyond BMO's own, and no way to revoke one run. + +So: a per-run capability instead of a static key. + +## Two components, two names + +| Name | Where | Holds | Speaks | +| ---------------- | ------------------ | --------------------- | ------------- | +| `broker` | sidecar in the run | the run's scope token | MCP | +| `bugzilla-proxy` | shared service | the BMO credential | Bugzilla REST | + +The broker keeps its existing job (hosting the MCP tool server, keeping credentials out of +the agent container) but its credential becomes a short-lived, run-scoped token rather than +a BMO key. The proxy is new: it holds the credential, evaluates what a token is allowed to +see, filters content, and logs every access. + +``` +hackbot-api ──mint scoped JWT (signed as its own SA)──┐ + │ │ + └─ POST /agents/{a}/runs ▼ + │ bugzilla-proxy (Cloud Run, IAM-restricted) + ▼ holds the BMO credential, verifies the JWT, + Cloud Run Job task evaluates scope, filters content, audit-logs + ┌──────────────────────┐ │ + │ agent (no creds) │ ▼ + │ ↕ loopback MCP │ BMO /rest + │ broker (holds JWT) ──┼──────────────────┘ + └──────────────────────┘ IAM + scope JWT: two independent gates +``` + +The credential lives in one hardened deployment rather than in every agent task, which +gives one place to revoke, one audit log, and rate limiting that can see enumeration +across runs. The cost is a network hop and a service to operate. + +## The token + +hackbot-api mints at trigger time and signs through IAM's `signJwt`, as **its own service +account**. The signing key is Google-managed: nothing in either service generates, holds, +exchanges or rotates key material. Google publishes each account's public certificates at a +URL derived from its email, so the proxy's entire trust configuration is one string, the +expected issuer's email, and fetching certs from that account's URL is itself the issuer +binding. + +This also needs no new IAM. `signJwt` comes from `roles/iam.serviceAccountTokenCreator`, +which hackbot-api already holds **on itself** for the GCS signed-policy path +([security.md](security.md)). + +The one constraint it adds: **IAM will not sign a JWT more than 12 hours out**. The default +8 hour job timeout plus its grace window fits with room to spare, but `mint` checks the sum +and fails at trigger time with a message naming the settings involved, rather than letting +IAM reject it with something that reads like a permissions problem. + +The proxy verifies and holds no per-run state: the token is the entire policy. + +```json +{ + "iss": "hackbot-api@.iam.gserviceaccount.com", + "aud": "hackbot-bugzilla-proxy", + "sub": "run:", + "jti": "...", + "exp": "", + "agent": "security-dedup", + "requested_by": "someone@mozilla.com", + "bz": { + "read_only": true, + "confidential": true, + "grants": [ + { + "tier": "metadata", + "anchor": { + "groups": ["core-security"], + "product": ["Core", "Firefox"], + "created_after": "2019-01-01" + }, + "fields": [ + "id", + "summary", + "product", + "component", + "status", + "resolution", + "dupe_of", + "keywords", + "creation_time" + ], + "endpoints": ["bug"] + }, + { + "tier": "full", + "anchor": { "static_bugs": [1899123] }, + "endpoints": ["bug", "bug/*/comment"] + } + ], + "promotions_max": 10, + "attachments": false, + "filter_content": "llm" + } +} +``` + +The scope template lives server-side on `AgentSpec`, beside `auto_apply_actions`, with +per-run substitution of the target bug. **A caller never sends a scope.** That keeps scope +server-determined even as scopes get expressive. + +The token travels in `X-Bugzilla-API-Key`, where bugsy already puts `api_key`, so it must +stay small enough for a header: broad anchors are fine, long `static_bugs` lists are not. + +### Why this is not a Google credential + +Signing as a service account raises a fair question: the token carries hackbot-api's own +signature, so could a run present it to Google and act as hackbot-api? Google accepts a +self-signed service account JWT in two flows, and this token satisfies neither: + +| Flow | Google requires | This token has | +| ----------------------------------------- | --------------------------------------------- | -------------------------------------------- | +| Exchange at `oauth2.googleapis.com/token` | `aud` = that URL, a `scope` claim, `exp` ≤ 1h | `hackbot-bugzilla-proxy`, none, ~8h | +| Direct call to `SERVICE.googleapis.com` | `aud` = that API, `sub` == `iss` | `hackbot-bugzilla-proxy`, `sub` = `run:` | + +Four independent mismatches, any one of which is disqualifying. But note what this rests +on: **the claims, not the mechanism.** The signature really is hackbot-api's service +account, so an audience pointing at a Google endpoint would turn a scoped Bugzilla read +token into full impersonation of this service, handed to the least-trusted container in the +system. + +Which is why **the audience is a constant in code, not a setting** +(`bz_token.TOKEN_AUDIENCE`, and `tokens.TOKEN_AUDIENCE` on the other side). Nothing +legitimate varies it, since environments are separated by the signing account, so there is +no deploy-time value to get wrong. Validating a configurable audience would have left the +mistake possible and merely caught it late. + +The certificate URL (`tokens.CERTS_URL_TEMPLATE`) is a constant for a sharper version of +the same reason. It decides which signatures the proxy accepts, so pointing it at a host +someone else controls is not a degradation, it is a complete authentication bypass: whoever +answers gets to mint runs. Google's endpoint is fixed, environments differ only in which +account is substituted into it, and local runs verify against a PEM without fetching at +all. + +Since both are literals duplicated across two packages that cannot import each other, the +proxy's tests parse hackbot-api's module and assert the audiences match, so drift fails in +unit tests rather than at the first real request. + +This is the one place the KMS alternative was safer by construction: a key we own is not a +credential Google would ever accept, whatever we put in the payload. + +### Delivery + +`jobs.py` gains a second `ContainerOverride` targeting the `broker` container, carrying +only the token. That relaxes today's rule that per-execution overrides touch only the +`agent` container, so it needs a guard: the API allowlists exactly which variable names it +may set on `broker`, and the value is minted server-side, never derived from caller input. + +The obvious alternative, having the broker fetch its own token using its Google identity +the way [anthropic_wif.py](../../libs/hackbot-runtime/hackbot_runtime/anthropic_wif.py) +does, does not work as stated: **every container in a Cloud Run task shares one service +account**, so the agent container could mint the same token. It becomes viable only with a +one-shot mint endpoint consumed during ordered broker startup, which is where to go if 8 +hour token lifetimes become uncomfortable. + +### Client side + +`Bugsy(api_key=..., bugzilla_url=...)` with no username makes no network call at +construction and exposes `.session`, so the broker needs no bugsy patching: + +```python +client = bugsy.Bugsy(api_key=scope_token, bugzilla_url=settings.bugzilla_proxy_url) +client.session.auth = GoogleIdTokenAuth(audience=settings.bugzilla_proxy_url) +``` + +`agent_tools/bugzilla.py` already renders proxy codes 101 (endpoint not exposed) and 102 +(access denied) as structured `ToolError`s, so agents already know how to handle a bug +falling outside scope. No tool or context change is needed. + +## The scope model + +Access to private-capable agents is limited to a small allowlist of people who already +have access to all private bugs. That removes the confused-deputy problem: a token can +never hand its holder something they lack. + +The scope therefore is **not** an authorization boundary against the requester. It is still +worth keeping tight, for four other reasons: + +- **Data minimization.** Every bug the scope admits can reach the model context, + `summary.json`, GCS and traces. The requester's clearance does not shrink that footprint. +- **Blast radius.** The token sits in a container beside model-directed code for hours. +- **Provably public agents.** A token that structurally cannot express private scope is + easier to reason about than a policy check. +- **Audit precision.** + +### Structural anchors and narrowing filters + +Rules divide by who controls the underlying field: + +| Class | Fields | Role | +| ----------------- | ------------------------------------------------------------------------------ | ------------------ | +| Structural anchor | `static_bugs`, `product`, `component`, `creation_time`, `status`, `resolution` | may grant | +| Narrowing filter | `keywords`, `whiteboard`, `blocks` | may only intersect | + +Every private-scope grant must carry at least one structural anchor, and narrowing filters +may only intersect it, never union into it. Every configured rule must hold: a grant is an +AND across its rules, never an OR, so adding one can only ever shrink what it admits. + +The point is that narrowing fields are editable by anyone with `editbugs`. With the +requester allowlist that is no longer an escalation path, but it is still an uncontrolled +way for a third party to enlarge a confidential run's footprint. Anchoring on +`product = Core AND groups ⊆ {core-security}` cannot be widened by editing a whiteboard. + +`groups` sits in the anchor on the wire but is not one of these rules. It is a **ceiling**: +a bug in any group the grant does not name is denied, and a public bug matching the other +rules is served whatever it contains. An empty `groups` therefore means public bugs only, +which is what makes private access opt-in rather than opt-out. A bug whose `groups` the +proxy cannot see is denied outright, so the field is added to every upstream request and +stripped again on the way out. + +### Field tiers + +De-duplication needs summary-level fields across a large corpus and full detail for a +handful of finalists, so a grant carries a tier: + +- **metadata**: id, summary, product, component, status, resolution, dupe_of, keywords, + creation_time. Comment and attachment endpoints denied. +- **full**: everything the endpoints allow. + +Three benefits fall out at once. The model context holds many summaries and few full texts. +`filter_content: llm` runs per comment, and the metadata tier serves no comments, so +corpus-wide access costs nothing in model calls. And candidate generation needs no per-bug +comment fetch. + +A run may promote a corpus bug to full tier up to `promotions_max` times, each logged. This +grants no new authority, since the bug was already in the corpus; it makes broad reads +deliberate. With multiple proxy instances the counter is per-instance, so this is a soft +limit and an audit signal, not a hard cap. + +### Search is the primary path + +Under a corpus scope the agent works through `search_bugs`, not `get_bugs`. Post-filtering +results is correct but not sufficient: the proxy must **inject the scope predicate into the +upstream query**, or it pages through BMO to discard most of what it fetches. Post-filtering +remains the authority; query rewriting is what makes it usable. + +Search is also where existence is disclosed, so the audit log records the query and the +returned ids, not only per-bug fetches. + +## Who may request private scope + +Two allowlists, both in hackbot-api: which agents may ever receive private scope, and which +requesters may trigger them. + +This makes requester identity the entire security boundary, and today it does not hold up. +The UI derives the email server-side from the session +([app/api/runs/route.ts](../../services/hackbot-ui/app/api/runs/route.ts)), but hackbot-api +accepts `X-On-Behalf-Of` as a plain header from any caller holding `X-API-Key` +([routers/runs.py](../../services/hackbot-api/app/routers/runs.py)), and that key is shared +between the UI, the pulse listener and scripts. Anyone holding it can assert any identity. + +So before private access ships: split `X-API-Key` per caller class, let only a UI-class key +assert `X-On-Behalf-Of`, and give automated callers keys that cannot reach private-capable +agents. + +The requester allowlist also needs periodic reconciliation against real BMO group +membership, with a named owner, so it does not go stale. + +## Containment + +Granting the read is the smaller half. Once a run touches a private bug, everything +downstream carries it: `summary.json` and artifacts in GCS, the UI (today any +`@mozilla.com` session sees any run), traces, Cloud Logging, notification emails, and above +all the recorded actions. `slack.post_message`, a public `phabricator.submit_patch` and a +try push are each a direct path from a security bug to a public artifact. + +A `confidential` flag is set on the run **at mint time**, since the API has already +resolved the scope before the job starts, and propagates to: + +| Surface | Behaviour when confidential | +| ----------------------------------------------- | --------------------------- | +| UI | run and artifacts gated | +| Tracing | disabled, or scrubbed | +| `bugzilla.add_comment` | `is_private` forced true | +| `slack.post_message` | refused by the applier | +| `phabricator.submit_patch` to a public revision | refused | +| try pushes | blocked | + +The proxy refuses to serve private data to a token not marked confidential, so the two +cannot drift apart. The agent is also told in its prompt, though that is mitigation, not +enforcement. + +## Content filtering + +Comments and attachments from untrusted authors can carry prompt injection, so the token +names how to handle them: `off` relays unchanged, `remove` blanks everything from authors +outside a set of trusted groups, and `llm` classifies each piece and blanks only what is +flagged. + +`remove` is the cheap default, but it is the wrong one for security bugs, where the +reporter is very often an external researcher: it would blank exactly the content the agent +needs. Private-scoped tokens realistically need `llm`, which adds an Anthropic dependency to +the proxy. Injection risk stays real in both directions: untrusted content flows in, and +confidential content must not flow out. + +## Deployment + +- Cloud Run service, own least-privilege service account, `--no-allow-unauthenticated`. +- `run.invoker` granted only to the agent Job service accounts. +- **Ingress `all`, not `internal`.** Cloud Run _jobs_ egress to the internet by default, so + `internal` would block them unless every agent job also gets Direct VPC egress with + all-traffic routing. IAM auth is what restricts callers; Cloud Run's front end rejects + unauthenticated requests before any of our code runs. VPC-internal is a later hardening + step, and this is the detail most likely to cost a day if assumed to work out of the box. +- BMO credential in Secret Manager, `secretAccessor` to the proxy service account only. +- No signing key to provision: hackbot-api signs as its own service account, and the + proxy is configured with that account's email as `token_issuer`. The only IAM needed + is `roles/iam.serviceAccountTokenCreator` on itself, which hackbot-api already has. +- The proxy needs egress to `www.googleapis.com` to fetch the issuer's certificates. + +## Delivery phases + +**0. Proxy in the path, public scope only. Built.** What landed: + +| Piece | Where | +| --------------------------------------------------- | ---------------------------------------------------------------------------------- | +| Scope evaluation: anchors, tiers, the group ceiling | [bugzilla_proxy/scope.py](../../services/bugzilla-proxy/bugzilla_proxy/scope.py) | +| Token verification and the claim shapes it refuses | [bugzilla_proxy/tokens.py](../../services/bugzilla-proxy/bugzilla_proxy/tokens.py) | +| Endpoint allowlist, post-filtering, audit log | [bugzilla_proxy/app.py](../../services/bugzilla-proxy/bugzilla_proxy/app.py) | +| Minting, scope templates, the `$bug_id` placeholder | [hackbot-api/app/bz_token.py](../../services/hackbot-api/app/bz_token.py) | +| The `broker` container override and its allowlist | [hackbot-api/app/jobs.py](../../services/hackbot-api/app/jobs.py) | + +Not yet done in this phase, in order: + +1. Deploy the Cloud Run service and its IAM, and set `token_issuer` to hackbot-api's + service account email. +2. Test it against real BMO traffic with a hand-minted token. +3. **Then** onboard the first agent: point its broker's bugsy client at the proxy (see + [Client side](#client-side)) and set `bugzilla_scope` on its `AgentSpec`. + +Nothing outside this table has changed, deliberately. Nothing in +[agents/](../../agents/) is touched, and no shared client helper exists yet: what a broker +needs depends on whether the deployed service ends up behind IAM invoker auth, which step 1 +settles. Writing that helper before then would be guessing at its shape. + +Because onboarding is opt-in per agent and minting is skipped entirely when no signing key +is configured, the rollback at every step is to stop minting rather than to revert code. + +Deliberately deferred out of phase 0: content filtering (a token asking for `remove` or +`llm` is refused rather than served unfiltered), upstream query rewriting (results are +filtered after they arrive, so a capped search can return fewer rows than asked for), and +tier promotion. + +**1. Trustworthy requester identity.** Split `X-API-Key` per caller class. Small, and +phases 3 and 4 rest on it. + +**2. Confidential runs.** The flag and every enforcement point in the table above. The +largest phase, and independent of the proxy, so it can run in parallel with 0 and 1. Done +when a synthetic confidential run demonstrably fails on each path. + +**3. Private access, explicit ids.** The `groups` rule with deny by default, the requester +allowlist, the privileged BMO credential, one agent on one pilot group. Done when a token +without a private grant gets a clean 102 the agent handles, and one with a grant succeeds +on a run marked confidential. + +**4. Corpus scope.** Structural anchors, field tiers, query rewriting, promotion budget. +This unblocks de-duplication. Done when a bug outside the anchor provably never appears in +search results, and metadata-tier bugs provably cannot yield comments. + +`test-repair` reaches Bugzilla through an injected `BUGZILLA_MCP_URL` rather than a broker, +so it needs the same treatment separately. + +## Long-lead items + +These gate later phases and should start during phase 0: + +- The privileged BMO account: which group memberships, who owns it, MozillaSecurity and BMO + team approval. Longest lead time, gates phase 3. +- Who is on the requester allowlist, and who maintains it. +- Confirmation that Anthropic data handling covers security bug content. +- GCP provisioning: the proxy's own service account and Cloud Run service. +- Booking the security review. + +## Open questions + +- Is `groups` reliably present and complete on BMO search results? Anchor evaluation depends + on it, and if a search can return a bug without its groups the proxy must fail closed and + re-fetch. +- What corpus does de-duplication actually need: all of `core-security` across products, or + per-product? This sets the footprint. +- Does de-duplication need comment 0 for every candidate? That decides whether two tiers + suffice or a third is needed. +- Delegated credentials (the proxy using the requester's own BMO credential) are less + necessary given the allowlist, but retain two advantages: access revokes automatically + when someone leaves a group, and BMO-side audit attributes reads to a person. Worth a + sentence to the BMO team rather than a redesign. diff --git a/docs/hackbot/security.md b/docs/hackbot/security.md index 5d72028d27..5902af3e7a 100644 --- a/docs/hackbot/security.md +++ b/docs/hackbot/security.md @@ -36,6 +36,17 @@ Today `bug-fix`, `build-repair`, `frontend-triage` and `autowebcompat-repro` run `test-plan-generator` needs no credentialed reads at all. The invariant holds in every case: **the key is never in the agent container.** +### Bugzilla reads are moving behind a proxy + +A broker holding a Bugzilla key can read whatever that account can, for every run of that +agent. [bugzilla-proxy](bugzilla-proxy.md) replaces the key with a per-run capability token: +the credential moves to one shared service, and the broker holds only a signed statement of +what this run may read. The service and the minting exist; **no agent uses them yet**, and +every broker still holds its own key until the service is deployed and tested. + +This is what makes private bugs reachable later, under a scope narrow enough to review. The +containment that has to land first is in [bugzilla-proxy.md](bugzilla-proxy.md). + ## Workload Identity Federation Anthropic and W&B both accept a Google-signed OIDC identity token exchanged for a diff --git a/services/bugzilla-proxy/Dockerfile b/services/bugzilla-proxy/Dockerfile new file mode 100644 index 0000000000..3864f75da4 --- /dev/null +++ b/services/bugzilla-proxy/Dockerfile @@ -0,0 +1,35 @@ +FROM python:3.14-slim AS builder + +COPY --from=ghcr.io/astral-sh/uv:latest /uv /uvx /bin/ + +ENV UV_PROJECT_ENVIRONMENT=/opt/venv + +WORKDIR /app + +# Install external deps without building workspace members. +RUN --mount=type=cache,target=/root/.cache/uv \ + --mount=type=bind,source=pyproject.toml,target=pyproject.toml \ + --mount=type=bind,source=uv.lock,target=uv.lock \ + --mount=type=bind,source=VERSION,target=VERSION \ + uv sync --frozen --no-dev --no-install-workspace --package bugzilla-proxy + +RUN --mount=type=cache,target=/root/.cache/uv \ + --mount=type=bind,target=/app,rw \ + uv sync --locked --no-dev --no-editable --package bugzilla-proxy + +FROM python:3.14-slim AS base + +COPY --from=builder /opt/venv /opt/venv +WORKDIR /app + +ENV PYTHONUNBUFFERED=1 +ENV PYTHONDONTWRITEBYTECODE=1 +ENV PORT=8080 +ENV PATH="/opt/venv/bin:$PATH" + +RUN useradd --create-home --shell /bin/bash app +USER app + +EXPOSE 8080 + +CMD ["uvicorn", "bugzilla_proxy.main:app", "--host", "0.0.0.0", "--port", "8080"] diff --git a/services/bugzilla-proxy/README.md b/services/bugzilla-proxy/README.md new file mode 100644 index 0000000000..aeeef42014 --- /dev/null +++ b/services/bugzilla-proxy/README.md @@ -0,0 +1,90 @@ +# bugzilla-proxy + +An authorization proxy for the Bugzilla REST API. It holds the upstream +credential; callers present a per-run capability token saying what they may +read. + +Design and rollout plan: [docs/hackbot/bugzilla-proxy.md](../../docs/hackbot/bugzilla-proxy.md). + +## What it does + +- Verifies an RS256 capability token minted by hackbot-api, against the public + certificates Google publishes for hackbot-api's service account (or a static + PEM locally). No key material is provisioned or exchanged between the two + services: the shared configuration is one email address. +- Exposes exactly four read endpoints. Everything else, and every write, is a + Bugzilla-shaped 101 "endpoint not exposed". +- Decides access per bug from the bug's own fields, fetched with the proxy's + credential. A bug in a security group is denied unless the token names that + group, and a bug whose groups it cannot see is denied outright. +- Projects each bug down to the fields the token's tier exposes. +- Logs every decision with the run, agent and requester behind it. + +## What it does not do yet + +Phase 0 of the plan. Not implemented: content filtering (`filter_content` other +than `off` is refused rather than silently ignored), upstream query rewriting +(search results are filtered after they arrive, so a capped search can return +fewer rows than the caller asked for), and promotion between tiers. + +## Endpoints + +| Path | Serves | +| ------------------------------- | ------------------------------------- | +| `GET /rest/bug` | Search, and bulk fetch by `id` | +| `GET /rest/bug/{id}/comment` | A bug's comments | +| `GET /rest/bug/{id}/attachment` | A bug's attachments | +| `GET /rest/bug/attachment/{id}` | One attachment, authorized by its bug | +| `GET /healthz` | Liveness, no token required | + +## Configuration + +All settings take a `BUGZILLA_PROXY_` prefix. + +| Variable | Purpose | +| ---------------------------- | ----------------------------------------------------------------- | +| `UPSTREAM_URL` | Bugzilla REST base URL | +| `UPSTREAM_API_KEY` | The credential, from Secret Manager | +| `TOKEN_ISSUER` | hackbot-api's service account email, whose certs verify the token | +| `JWT_PUBLIC_KEY` | A PEM instead, for local runs. Exactly one of these two | +| `DECISION_CACHE_TTL_SECONDS` | How long bug metadata is reused (default 300) | +| `MAX_SEARCH_LIMIT` | Ceiling on rows fetched per search (default 500) | + +Deployed, `TOKEN_ISSUER` is the whole trust configuration. The certificate URL is +derived from it, so only that account's keys are ever candidates for verifying a +token. + +Two values are deliberately **not** configurable, and live as constants in +`bugzilla_proxy/tokens.py` instead: + +- `TOKEN_AUDIENCE`, which must match what hackbot-api mints, and which would + make the token a Google credential if pointed at a Google endpoint. +- `CERTS_URL_TEMPLATE`, which decides whose signatures we accept. Pointing it + elsewhere would let whoever answers it authenticate as any run. + +Neither varies legitimately, so neither is something a deploy can get wrong. The +design doc covers the reasoning under "Why this is not a Google credential". + +## Running locally + +```bash +uv sync --extra dev --package bugzilla-proxy +openssl genrsa -out /tmp/bzproxy.pem 2048 +openssl rsa -in /tmp/bzproxy.pem -pubout -out /tmp/bzproxy.pub + +BUGZILLA_PROXY_UPSTREAM_API_KEY=... \ +BUGZILLA_PROXY_JWT_PUBLIC_KEY="$(cat /tmp/bzproxy.pub)" \ + uv run --package bugzilla-proxy python -m bugzilla_proxy.main +``` + +Point hackbot-api at the same keypair with `BZ_TOKEN_PRIVATE_KEY`, and a broker +at the proxy with `BUGZILLA_PROXY_URL` plus `BUGZILLA_PROXY_AUDIENCE=""` (the +local proxy is not behind IAM, so there is no identity token to present). + +## Tests + +```bash +uv run --package bugzilla-proxy pytest +``` + +`tests/test_scope.py` is the one to read first: it is where the refusals live. diff --git a/services/bugzilla-proxy/bugzilla_proxy/__init__.py b/services/bugzilla-proxy/bugzilla_proxy/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/services/bugzilla-proxy/bugzilla_proxy/app.py b/services/bugzilla-proxy/bugzilla_proxy/app.py new file mode 100644 index 0000000000..b680e8878d --- /dev/null +++ b/services/bugzilla-proxy/bugzilla_proxy/app.py @@ -0,0 +1,314 @@ +"""The HTTP surface: a default-deny proxy in front of Bugzilla's REST API. + +The per-run token arrives in ``X-Bugzilla-API-Key``, where bugsy already puts an +API key, so an agent's client needs no special casing. + +Two rules shape the code. **Default deny**: only four paths exist, and anything +else, or any method but GET, is a Bugzilla-shaped 101. **Authorize on upstream +truth**: a bug's own fields decide access, fetched with the proxy's credential, +so the caller's ``include_fields`` can narrow the response but not the decision. +""" + +from __future__ import annotations + +import logging +import re +from contextlib import asynccontextmanager +from typing import Any + +from cachetools import TTLCache +from fastapi import FastAPI, Request +from fastapi.responses import JSONResponse + +from bugzilla_proxy.config import Settings +from bugzilla_proxy.scope import AUTH_FIELDS, Grant, Scope +from bugzilla_proxy.tokens import TokenError, TokenVerifier +from bugzilla_proxy.upstream import Upstream, UpstreamError + +log = logging.getLogger(__name__) + +# 101 and 102 are what agent_tools.bugzilla already renders as +# `endpoint_not_exposed` and `access_denied`. +CODE_NOT_EXPOSED = 101 +CODE_ACCESS_DENIED = 102 +CODE_UPSTREAM = 103 + +# Credentials a caller might try to smuggle past our header swap. +_AUTH_PARAMS = frozenset( + { + "api_key", + "Bugzilla_api_key", + "Bugzilla_login", + "Bugzilla_password", + "Bugzilla_token", + "login", + "password", + "token", + } +) + +# Group names `include_fields` accepts. They mean "widen", so they can never +# narrow a projection. +_FIELD_GROUPS = frozenset({"_default", "_all", "_extra", "_custom"}) + +_BUG_COMMENTS = re.compile(r"^bug/(\d+)/comment$") +_BUG_ATTACHMENTS = re.compile(r"^bug/(\d+)/attachment$") +_ATTACHMENT = re.compile(r"^bug/attachment/(\d+)$") + +_ENDPOINT_BUG = "bug" +_ENDPOINT_COMMENTS = "bug/*/comment" +_ENDPOINT_ATTACHMENTS = "bug/*/attachment" +_ENDPOINT_ATTACHMENT = "bug/attachment/*" + + +def _error(code: int, message: str, status_code: int = 404) -> JSONResponse: + """Render a Bugzilla-shaped error. + + bugsy turns any message mentioning an API key into a `LoginException` + rather than the `BugsyException` the tools expect, so upstream complaints + about credentials are replaced instead of relayed. + """ + if "API key" in message: + message = "Bugzilla rejected the proxy's credentials" + return JSONResponse( + {"error": True, "code": code, "message": message}, + status_code=status_code, + ) + + +def _requested_fields(params: dict[str, str]) -> frozenset[str] | None: + """The caller's ``include_fields``, or None for no narrowing. + + A group name like ``_default`` asks to widen, which is the tier's decision + rather than the caller's, so it counts as no narrowing at all. + """ + raw = params.get("include_fields") + if not raw: + return None + names = {name.strip() for name in str(raw).split(",") if name.strip()} + if not names or names & _FIELD_GROUPS: + return None + return frozenset(names) + + +def _clean_params(request: Request) -> dict[str, Any]: + """The caller's query params, minus anything that looks like a credential.""" + return { + key: value + for key, value in request.query_params.items() + if key not in _AUTH_PARAMS + } + + +def create_app(settings: Settings, upstream: Upstream | None = None) -> FastAPI: + @asynccontextmanager + async def lifespan(app_: FastAPI): + yield + await app_.state.upstream.aclose() + + app = FastAPI( + title="bugzilla-proxy", docs_url=None, redoc_url=None, lifespan=lifespan + ) + + app.state.settings = settings + app.state.verifier = TokenVerifier(settings) + app.state.upstream = upstream or Upstream(settings) + # Keyed by bug id, not by token: this is upstream truth, identical for + # every caller. The TTL bounds how long a newly-private bug keeps being + # served. + app.state.bug_cache = TTLCache( + maxsize=settings.decision_cache_max_entries, + ttl=settings.decision_cache_ttl_seconds, + ) + + @app.get("/healthz") + async def healthz() -> dict[str, str]: + return {"status": "ok"} + + async def _auth_fields_for(app_: FastAPI, bug_id: int) -> dict[str, Any] | None: + """Fetch just enough of a bug to decide access, memoised.""" + cache = app_.state.bug_cache + if bug_id in cache: + return cache[bug_id] + payload = await app_.state.upstream.get( + "bug", + {"id": str(bug_id), "include_fields": ",".join(sorted(AUTH_FIELDS))}, + ) + bugs = payload.get("bugs") or [] + bug = bugs[0] if bugs else None + cache[bug_id] = bug + return bug + + def _audit(scope: Scope, event: str, **fields: Any) -> None: + log.info( + "bzproxy %s run=%s agent=%s requested_by=%s %s", + event, + scope.run_id, + scope.agent, + scope.requested_by or "-", + " ".join(f"{k}={v}" for k, v in fields.items()), + ) + + def _authenticate(request: Request) -> Scope: + token = request.headers.get("X-Bugzilla-API-Key", "") + return request.app.state.verifier.verify(token) + + async def _search(request: Request, scope: Scope) -> JSONResponse: + if not any(g.allows_endpoint(_ENDPOINT_BUG) for g in scope.grants): + return _error(CODE_NOT_EXPOSED, "This proxy does not expose bug search.") + + params = _clean_params(request) + requested = _requested_fields(params) + + upstream_fields = scope.upstream_fields(requested) + if upstream_fields: + params["include_fields"] = ",".join(sorted(upstream_fields)) + else: + params.pop("include_fields", None) + + limit = params.get("limit") + try: + capped = min(int(limit), settings.max_search_limit) if limit else None + except (TypeError, ValueError): + return _error(CODE_UPSTREAM, "The 'limit' parameter must be a number.", 400) + params["limit"] = str(capped or settings.max_search_limit) + + payload = await request.app.state.upstream.get("bug", params) + + visible: list[dict[str, Any]] = [] + for bug in payload.get("bugs") or []: + grant = scope.grant_for_endpoint(bug, _ENDPOINT_BUG) + if grant is not None: + visible.append(grant.project(bug, requested)) + + returned = len(payload.get("bugs") or []) + _audit( + scope, + "search", + upstream=returned, + visible=len(visible), + ids=",".join(str(b.get("id")) for b in visible) or "-", + ) + return JSONResponse({"bugs": visible}) + + async def _authorized_grant( + request: Request, scope: Scope, bug_id: int, endpoint: str + ) -> tuple[Grant | None, JSONResponse | None]: + """The grant covering ``endpoint`` on ``bug_id``, or a denial. + + Out-of-scope and nonexistent give the same answer on purpose: + distinguishing them confirms the existence of bugs the run may not see. + """ + bug = await _auth_fields_for(request.app, bug_id) + if bug is None: + _audit(scope, "deny", bug=bug_id, endpoint=endpoint, reason="not_found") + return None, _error( + CODE_ACCESS_DENIED, f"Bug {bug_id} is not available to this run." + ) + grant = scope.grant_for_endpoint(bug, endpoint) + if grant is None: + _audit(scope, "deny", bug=bug_id, endpoint=endpoint, reason="out_of_scope") + return None, _error( + CODE_ACCESS_DENIED, f"Bug {bug_id} is not available to this run." + ) + _audit(scope, "allow", bug=bug_id, endpoint=endpoint, tier=grant.tier) + return grant, None + + async def _comments(request: Request, scope: Scope, bug_id: int) -> JSONResponse: + _grant, denial = await _authorized_grant( + request, scope, bug_id, _ENDPOINT_COMMENTS + ) + if denial is not None: + return denial + payload = await request.app.state.upstream.get( + f"bug/{bug_id}/comment", _clean_params(request) + ) + return JSONResponse(payload) + + async def _attachments(request: Request, scope: Scope, bug_id: int) -> JSONResponse: + if not scope.attachments: + return _error( + CODE_NOT_EXPOSED, "This run may not read Bugzilla attachments." + ) + _grant, denial = await _authorized_grant( + request, scope, bug_id, _ENDPOINT_ATTACHMENTS + ) + if denial is not None: + return denial + payload = await request.app.state.upstream.get( + f"bug/{bug_id}/attachment", _clean_params(request) + ) + return JSONResponse(payload) + + async def _attachment( + request: Request, scope: Scope, attachment_id: int + ) -> JSONResponse: + """One attachment by its own id. + + Its bug is only discoverable from the attachment, so this fetches first + and authorizes second. Nothing is returned until the bug clears. + """ + if not scope.attachments: + return _error( + CODE_NOT_EXPOSED, "This run may not read Bugzilla attachments." + ) + payload = await request.app.state.upstream.get( + f"bug/attachment/{attachment_id}", _clean_params(request) + ) + attachments = payload.get("attachments") or {} + record = attachments.get(str(attachment_id)) or attachments.get(attachment_id) + bug_id = (record or {}).get("bug_id") + if bug_id is None: + return _error( + CODE_ACCESS_DENIED, + f"Attachment {attachment_id} is not available to this run.", + ) + _grant, denial = await _authorized_grant( + request, scope, int(bug_id), _ENDPOINT_ATTACHMENT + ) + if denial is not None: + return denial + return JSONResponse(payload) + + @app.get("/rest/{path:path}") + async def rest(path: str, request: Request) -> JSONResponse: + try: + scope = _authenticate(request) + except TokenError as exc: + log.warning("Rejected request to %s: %s", path, exc) + return _error(CODE_ACCESS_DENIED, "This run is not authorized.", 401) + + route = path.strip("/") + try: + if route == "bug": + return await _search(request, scope) + match = _BUG_COMMENTS.match(route) + if match: + return await _comments(request, scope, int(match.group(1))) + match = _BUG_ATTACHMENTS.match(route) + if match: + return await _attachments(request, scope, int(match.group(1))) + match = _ATTACHMENT.match(route) + if match: + return await _attachment(request, scope, int(match.group(1))) + except UpstreamError as exc: + log.warning("Upstream failure serving %s: %s", route, exc) + return _error(CODE_UPSTREAM, str(exc), 502) + + _audit(scope, "deny", endpoint=route, reason="not_exposed") + return _error(CODE_NOT_EXPOSED, f"This proxy does not expose '/{route}'.") + + @app.api_route( + "/rest/{path:path}", + methods=["POST", "PUT", "PATCH", "DELETE"], + include_in_schema=False, + ) + async def rest_write(path: str) -> JSONResponse: + """Writes never reach upstream. + + Agents change the world through recorded actions, applied by + hackbot-api after a run is known good. + """ + return _error(CODE_NOT_EXPOSED, "This proxy is read-only.", status_code=405) + + return app diff --git a/services/bugzilla-proxy/bugzilla_proxy/config.py b/services/bugzilla-proxy/bugzilla_proxy/config.py new file mode 100644 index 0000000000..307fa68320 --- /dev/null +++ b/services/bugzilla-proxy/bugzilla_proxy/config.py @@ -0,0 +1,49 @@ +from pydantic_settings import BaseSettings + + +class Settings(BaseSettings): + """Deploy-time configuration. Nothing here varies per run.""" + + # Upstream Bugzilla. The credential lives only here, in this one service. + upstream_url: str = "https://bugzilla.mozilla.org/rest" + upstream_api_key: str = "" + upstream_timeout_seconds: float = 30.0 + + # hackbot-api's service account email, and the whole trust configuration: + # tokens are signed by that account's Google-managed key, and we verify + # against the certs Google publishes for it, so there is no key material to + # provision or rotate. The cert URL is derived from this, so only that + # account's keys are ever candidates. + # + # The audience and cert URL are constants in bugzilla_proxy.tokens, not + # settings, so neither can drift from what hackbot-api mints nor be pointed + # somewhere it should not be. + token_issuer: str = "" + jwt_public_key_ttl_seconds: int = 60 * 60 + # A static PEM for local runs, where there is no service account. Mutually + # exclusive with `token_issuer`. + jwt_public_key: str = "" + + # Per-(token, bug) authorization decisions. Short enough that a bug moving + # into a security group stops being served promptly, long enough that a + # run walking a dependency tree does not re-fetch every bug per tool call. + decision_cache_ttl_seconds: int = 300 + decision_cache_max_entries: int = 10_000 + + # A ceiling on what one search can pull from upstream. Results are filtered + # after they arrive, so an unbounded limit would mean fetching far more + # than the caller can ever see. + max_search_limit: int = 500 + + port: int = 8080 + environment: str = "development" + + model_config = { + "env_file": ".env", + "env_file_encoding": "utf-8", + "env_prefix": "BUGZILLA_PROXY_", + "extra": "ignore", + } + + +settings = Settings() diff --git a/services/bugzilla-proxy/bugzilla_proxy/main.py b/services/bugzilla-proxy/bugzilla_proxy/main.py new file mode 100644 index 0000000000..7fae13eb89 --- /dev/null +++ b/services/bugzilla-proxy/bugzilla_proxy/main.py @@ -0,0 +1,22 @@ +"""Entry point. Fails at startup rather than per request if misconfigured.""" + +import logging + +import uvicorn + +from bugzilla_proxy.app import create_app +from bugzilla_proxy.config import settings + +app = create_app(settings) + + +def main() -> None: + logging.basicConfig( + level=logging.INFO, + format="%(asctime)s - %(name)s - %(levelname)s - %(message)s", + ) + uvicorn.run(app, host="0.0.0.0", port=settings.port, log_config=None) + + +if __name__ == "__main__": + main() diff --git a/services/bugzilla-proxy/bugzilla_proxy/scope.py b/services/bugzilla-proxy/bugzilla_proxy/scope.py new file mode 100644 index 0000000000..bbe0a32c9b --- /dev/null +++ b/services/bugzilla-proxy/bugzilla_proxy/scope.py @@ -0,0 +1,311 @@ +"""What a run's token permits it to read. Pure logic, no I/O. + +A bug is denied unless a grant positively admits it, and a bug whose ``groups`` +we cannot see is denied outright rather than assumed public. +""" + +from __future__ import annotations + +from collections.abc import Iterable, Mapping +from dataclasses import dataclass, field +from datetime import datetime, timezone +from typing import Any + +# Least to most permissive; a bug matching several grants is served at the +# highest tier that admits it. +TIER_METADATA = "metadata" +TIER_FULL = "full" +TIERS = (TIER_METADATA, TIER_FULL) +_TIER_RANK = {tier: rank for rank, tier in enumerate(TIERS)} + +# The metadata tier's default: enough to generate de-duplication candidates, +# not enough to read the bug. +DEFAULT_METADATA_FIELDS = frozenset( + { + "id", + "summary", + "product", + "component", + "status", + "resolution", + "dupe_of", + "keywords", + "creation_time", + "last_change_time", + "type", + "severity", + "priority", + "is_open", + } +) + +# Needed to decide access. Added to every upstream request whatever the caller +# asked for, and stripped again on the way out unless the tier exposes them. +AUTH_FIELDS = frozenset( + { + "id", + "groups", + "product", + "component", + "status", + "resolution", + "keywords", + "whiteboard", + "blocks", + "creation_time", + } +) + +# Rules BMO's own workflow controls. Any grant reaching private bugs needs one: +# the others are editable by anyone with `editbugs`, who could then enlarge a +# run's footprint. +STRUCTURAL_RULES = frozenset( + {"static_bugs", "product", "component", "status", "resolution", "created_after"} +) + + +def _as_int_set(values: Iterable[Any] | None) -> frozenset[int]: + return frozenset(int(v) for v in values or ()) + + +def _as_lower_set(values: Iterable[Any] | None) -> frozenset[str]: + return frozenset(str(v).lower() for v in values or ()) + + +def _parse_time(raw: str) -> datetime: + """Parse a BMO timestamp, which is ISO 8601 with a ``Z`` suffix.""" + parsed = datetime.fromisoformat(raw.replace("Z", "+00:00")) + if parsed.tzinfo is None: + parsed = parsed.replace(tzinfo=timezone.utc) + return parsed + + +@dataclass(frozen=True) +class Anchor: + """The filters deciding which bugs fall inside one grant. + + Every configured rule must match (AND, not OR), and an unconfigured rule is + ignored, so an empty anchor matches everything and is only valid on a grant + that cannot reach private data. + """ + + static_bugs: frozenset[int] = frozenset() + product: frozenset[str] = frozenset() + component: frozenset[str] = frozenset() + status: frozenset[str] = frozenset() + resolution: frozenset[str] = frozenset() + created_after: str | None = None + keywords: frozenset[str] = frozenset() + whiteboard: tuple[str, ...] = () + blocks: frozenset[int] = frozenset() + + @classmethod + def from_claim(cls, raw: Mapping[str, Any]) -> Anchor: + return cls( + static_bugs=_as_int_set(raw.get("static_bugs")), + product=frozenset(str(p) for p in raw.get("product") or ()), + component=frozenset(str(c) for c in raw.get("component") or ()), + status=frozenset(str(s).upper() for s in raw.get("status") or ()), + resolution=frozenset(str(r).upper() for r in raw.get("resolution") or ()), + created_after=raw.get("created_after"), + keywords=_as_lower_set(raw.get("keywords")), + whiteboard=tuple(str(w) for w in raw.get("whiteboard") or ()), + blocks=_as_int_set(raw.get("blocks")), + ) + + def configured_rules(self) -> frozenset[str]: + """Names of the rules this anchor actually constrains.""" + present = { + "static_bugs": self.static_bugs, + "product": self.product, + "component": self.component, + "status": self.status, + "resolution": self.resolution, + "created_after": self.created_after, + "keywords": self.keywords, + "whiteboard": self.whiteboard, + "blocks": self.blocks, + } + return frozenset(name for name, value in present.items() if value) + + def has_structural_rule(self) -> bool: + return bool(self.configured_rules() & STRUCTURAL_RULES) + + def matches(self, bug: Mapping[str, Any]) -> bool: + """True if every configured rule holds for ``bug``.""" + if self.static_bugs and int(bug.get("id", -1)) not in self.static_bugs: + return False + if self.product and bug.get("product") not in self.product: + return False + if self.component and bug.get("component") not in self.component: + return False + if self.status and str(bug.get("status", "")).upper() not in self.status: + return False + if ( + self.resolution + and str(bug.get("resolution", "")).upper() not in self.resolution + ): + return False + if self.created_after: + created = bug.get("creation_time") + if not created: + return False + try: + if _parse_time(str(created)) < _parse_time(self.created_after): + return False + except ValueError: + return False + if self.keywords and not (_as_lower_set(bug.get("keywords")) & self.keywords): + return False + if self.whiteboard: + board = str(bug.get("whiteboard") or "") + if not any(tag in board for tag in self.whiteboard): + return False + if self.blocks and not (_as_int_set(bug.get("blocks")) & self.blocks): + return False + return True + + +@dataclass(frozen=True) +class Grant: + """One tier of access over one set of bugs.""" + + tier: str + anchor: Anchor + # A ceiling, not a filter: a bug in any group outside this is denied, while + # a public bug matching the anchor is served regardless. Empty means public + # only, which is what makes private access opt-in. + groups: frozenset[str] = frozenset() + # Empty falls back to the tier default. + fields: frozenset[str] = frozenset() + endpoints: tuple[str, ...] = () + + @classmethod + def from_claim(cls, raw: Mapping[str, Any]) -> Grant: + anchor_claim = dict(raw.get("anchor") or {}) + # Rides inside `anchor` on the wire, but it is a ceiling not a filter, + # so it lives on the grant here. + groups = frozenset(str(g) for g in anchor_claim.pop("groups", None) or ()) + return cls( + tier=str(raw.get("tier", "")), + anchor=Anchor.from_claim(anchor_claim), + groups=groups, + fields=frozenset(str(f) for f in raw.get("fields") or ()), + endpoints=tuple(str(e) for e in raw.get("endpoints") or ()), + ) + + @property + def is_private(self) -> bool: + return bool(self.groups) + + def permits(self, bug: Mapping[str, Any]) -> bool: + """True if this grant admits ``bug``. + + A missing ``groups`` field is a denial, not an assumption of public: + we always request it, so its absence means something went wrong, and + guessing is how a security bug leaks. + """ + if "groups" not in bug: + return False + if not frozenset(bug.get("groups") or ()) <= self.groups: + return False + return self.anchor.matches(bug) + + def allows_endpoint(self, path: str) -> bool: + """True if ``path`` matches a pattern, ``*`` being exactly one segment.""" + segments = [s for s in path.strip("/").split("/") if s] + for pattern in self.endpoints: + expected = [s for s in pattern.strip("/").split("/") if s] + if len(expected) != len(segments): + continue + if all(e == "*" or e == got for e, got in zip(expected, segments)): + return True + return False + + def visible_fields(self) -> frozenset[str] | None: + """The fields this grant exposes, or None for whatever upstream sent.""" + if self.fields: + return self.fields + if self.tier == TIER_METADATA: + return DEFAULT_METADATA_FIELDS + return None + + def project( + self, bug: Mapping[str, Any], requested: frozenset[str] | None = None + ) -> dict[str, Any]: + """Drop what the tier does not expose, then narrow to ``requested``. + + ``requested`` is the caller's ``include_fields``; it can only narrow. + This is also where the added :data:`AUTH_FIELDS` come back off. + """ + visible = self.visible_fields() + keys = set(bug) if visible is None else (set(bug) & visible) + if requested: + keys &= requested + return {key: bug[key] for key in keys} + + +@dataclass(frozen=True) +class Scope: + """A verified token, in the form the request path needs.""" + + run_id: str + agent: str + jti: str + requested_by: str | None = None + read_only: bool = True + confidential: bool = False + attachments: bool = False + filter_content: str = "off" + promotions_max: int = 0 + grants: tuple[Grant, ...] = field(default_factory=tuple) + + @property + def is_private(self) -> bool: + return any(grant.is_private for grant in self.grants) + + def grant_for(self, bug: Mapping[str, Any]) -> Grant | None: + """The highest-tier grant admitting ``bug``, or None.""" + best: Grant | None = None + for grant in self.grants: + if not grant.permits(bug): + continue + if best is None or _TIER_RANK[grant.tier] > _TIER_RANK[best.tier]: + best = grant + return best + + def grant_for_endpoint(self, bug: Mapping[str, Any], path: str) -> Grant | None: + """The highest-tier grant admitting ``bug`` and exposing ``path``. + + Separate from :meth:`grant_for` because tiers differ in endpoints: a + bug can be readable at the metadata tier while its comments are not. + """ + best: Grant | None = None + for grant in self.grants: + if not grant.permits(bug) or not grant.allows_endpoint(path): + continue + if best is None or _TIER_RANK[grant.tier] > _TIER_RANK[best.tier]: + best = grant + return best + + def upstream_fields(self, requested: frozenset[str] | None) -> frozenset[str]: + """The fields to ask upstream for, given what the caller asked for. + + Always a superset of :data:`AUTH_FIELDS`, so the decision never depends + on the caller's ``include_fields``. + """ + if requested: + return frozenset(requested) | AUTH_FIELDS + widest: frozenset[str] | None = frozenset() + for grant in self.grants: + visible = grant.visible_fields() + if visible is None: + widest = None + break + widest = (widest or frozenset()) | visible + if widest is None: + # A grant serves whole bugs. Name `_default` explicitly rather + # than omitting `include_fields`, so the auth fields stay present + # even if BMO's default set changes. + return frozenset({"_default"}) | AUTH_FIELDS + return widest | AUTH_FIELDS diff --git a/services/bugzilla-proxy/bugzilla_proxy/tokens.py b/services/bugzilla-proxy/bugzilla_proxy/tokens.py new file mode 100644 index 0000000000..66538f1834 --- /dev/null +++ b/services/bugzilla-proxy/bugzilla_proxy/tokens.py @@ -0,0 +1,259 @@ +"""Verifying the per-run capability token and turning it into a Scope. + +hackbot-api signs each token with its own service account's Google-managed key. +We verify against the certificates Google publishes for that account, so no key +material is generated, exchanged or rotated on either side. Locally a PEM stands +in, since there is no service account. + +Everything a request may do comes from the token, so this module fails closed: +an unknown endpoint pattern, an unimplemented content filter or a private grant +without a structural anchor is an error, not a warning. +""" + +from __future__ import annotations + +import logging +import time +from collections.abc import Mapping +from typing import Any + +import httpx +import jwt +from cryptography.hazmat.primitives import serialization as _serialization +from cryptography.x509 import load_pem_x509_certificate + +from bugzilla_proxy.config import Settings +from bugzilla_proxy.scope import TIERS, Grant, Scope + +log = logging.getLogger(__name__) + +# A grant may name a subset of these but may not invent a pattern, so a typo +# cannot quietly widen access and a bare wildcard cannot be smuggled in. +KNOWN_ENDPOINTS = frozenset( + { + "bug", + "bug/*/comment", + "bug/*/attachment", + "bug/attachment/*", + } +) + +# `remove` and `llm` land with private access. Until then a token asking for +# one is rejected rather than served unfiltered. +IMPLEMENTED_FILTER_MODES = frozenset({"off"}) + +# Must match `bz_token.TOKEN_AUDIENCE` in hackbot-api, which mints against the +# same literal. A constant rather than a setting on both sides: see +# docs/hackbot/bugzilla-proxy.md, "Why this is not a Google credential". +TOKEN_AUDIENCE = "hackbot-bugzilla-proxy" + +# The root of trust for every token, and a constant for a sharper reason than +# the audience: point it elsewhere and whoever answers decides which signatures +# we accept, which is an authentication bypass rather than a degradation. +CERTS_URL_TEMPLATE = ( + "https://www.googleapis.com/robot/v1/metadata/x509/{service_account}" +) + + +class TokenError(Exception): + """A token that cannot be trusted, or asks for something unsupported.""" + + +class PublicKeySource: + """The issuer's public keys by key id, TTL-cached. + + A fetch rather than configuration because Google rotates these on its own + schedule. The URL is issuer-specific, so fetching from it *is* the issuer + binding: another account's token has no key here to verify against. + """ + + def __init__(self, settings: Settings) -> None: + if bool(settings.jwt_public_key) == bool(settings.token_issuer): + raise ValueError( + "configure exactly one of token_issuer (the signing service " + "account's email, for a deployment) or jwt_public_key (a PEM, " + "for local runs)" + ) + self._settings = settings + self._cached: dict[str, str] | None = None + self._expires_at = 0.0 + + @property + def is_local(self) -> bool: + return bool(self._settings.jwt_public_key) + + def get(self, kid: str | None) -> str: + """The PEM for ``kid``, refetching once if unknown. + + An unknown id usually means Google rotated since our last fetch, so one + immediate refetch beats failing every request for the rest of the TTL. + """ + if self.is_local: + return self._settings.jwt_public_key + + keys = self._current() + if kid is None: + raise TokenError("token carries no key id") + if kid not in keys: + keys = self._refresh() + if kid not in keys: + raise TokenError(f"unknown signing key {kid!r}") + return keys[kid] + + def _current(self) -> dict[str, str]: + if self._cached is not None and time.monotonic() < self._expires_at: + return self._cached + return self._refresh() + + def _refresh(self) -> dict[str, str]: + url = CERTS_URL_TEMPLATE.format(service_account=self._settings.token_issuer) + try: + response = httpx.get(url, timeout=10.0) + response.raise_for_status() + certs = response.json() + except (httpx.HTTPError, ValueError) as exc: + # A stale cache beats failing every run's reads over a blip. + if self._cached is not None: + log.warning("Could not refresh signing certs, using cached: %s", exc) + return self._cached + raise TokenError("cannot fetch the issuer's signing certificates") from exc + + # Google serves `{key_id: x509 PEM}`. Convert once so the hot path is + # a dict lookup. + self._cached = { + kid: load_pem_x509_certificate(pem.encode()) + .public_key() + .public_bytes( + encoding=_serialization.Encoding.PEM, + format=_serialization.PublicFormat.SubjectPublicKeyInfo, + ) + .decode() + for kid, pem in certs.items() + } + self._expires_at = time.monotonic() + self._settings.jwt_public_key_ttl_seconds + return self._cached + + +def _require_mapping(value: Any, what: str) -> Mapping[str, Any]: + if not isinstance(value, Mapping): + raise TokenError(f"{what} must be an object") + return value + + +def _validate_grant(grant: Grant, *, confidential: bool) -> None: + if grant.tier not in TIERS: + raise TokenError(f"unknown tier {grant.tier!r}") + if not grant.endpoints: + raise TokenError("a grant must name at least one endpoint") + unknown = set(grant.endpoints) - KNOWN_ENDPOINTS + if unknown: + raise TokenError(f"unknown endpoint pattern(s): {sorted(unknown)}") + if not grant.is_private: + return + if not grant.anchor.has_structural_rule(): + raise TokenError( + "a grant with security groups needs at least one structural rule " + "(static_bugs, product, component, status, resolution, " + "created_after); keyword, whiteboard and blocks rules may only " + "narrow one, since anyone with editbugs can change them" + ) + if not confidential: + raise TokenError( + "a grant with security groups requires the run to be marked " + "confidential, so the containment rules apply downstream" + ) + + +def parse_scope(claims: Mapping[str, Any]) -> Scope: + """Turn verified JWT claims into a :class:`Scope`, or reject them.""" + bz = _require_mapping(claims.get("bz"), "the 'bz' claim") + + subject = str(claims.get("sub") or "") + if not subject.startswith("run:"): + raise TokenError("'sub' must be of the form 'run:'") + run_id = subject.removeprefix("run:") + if not run_id: + raise TokenError("'sub' carries no run id") + + jti = str(claims.get("jti") or "") + if not jti: + raise TokenError("'jti' is required, it keys the decision cache") + + if not bool(bz.get("read_only", True)): + raise TokenError("this proxy serves reads only; 'read_only' must be true") + + filter_content = str(bz.get("filter_content", "off")) + if filter_content not in IMPLEMENTED_FILTER_MODES: + raise TokenError( + f"content filtering mode {filter_content!r} is not implemented yet" + ) + + confidential = bool(bz.get("confidential", False)) + + raw_grants = bz.get("grants") or [] + if not isinstance(raw_grants, list) or not raw_grants: + raise TokenError("'grants' must be a non-empty list") + + grants = [] + for raw in raw_grants: + grant = Grant.from_claim(_require_mapping(raw, "each grant")) + _validate_grant(grant, confidential=confidential) + grants.append(grant) + + scope = Scope( + run_id=run_id, + agent=str(claims.get("agent") or ""), + jti=jti, + requested_by=claims.get("requested_by") or None, + read_only=True, + confidential=confidential, + attachments=bool(bz.get("attachments", False)), + filter_content=filter_content, + promotions_max=int(bz.get("promotions_max", 0) or 0), + grants=tuple(grants), + ) + + # Already covered per-grant above; restated so the invariant is findable. + if scope.is_private and not scope.confidential: + raise TokenError("a private scope requires a confidential run") + + return scope + + +class TokenVerifier: + """Verifies a token's signature and claims, then parses it into a Scope.""" + + def __init__(self, settings: Settings) -> None: + self._settings = settings + self._keys = PublicKeySource(settings) + + def verify(self, token: str) -> Scope: + if not token: + raise TokenError("no token presented") + + try: + kid = jwt.get_unverified_header(token).get("kid") + except jwt.PyJWTError as exc: + log.warning("Rejected token with an unreadable header: %s", exc) + raise TokenError("token is not valid") from exc + + key = self._keys.get(kid) + + # Locally the single configured key is the whole trust anchor, so + # there is nothing sensible to check `iss` against. + expected_issuer = None if self._keys.is_local else self._settings.token_issuer + + try: + claims = jwt.decode( + token, + key=key, + algorithms=["RS256"], + audience=TOKEN_AUDIENCE, + issuer=expected_issuer, + options={"require": ["exp", "iat", "iss", "aud", "sub"]}, + ) + except jwt.PyJWTError as exc: + # Terse to the caller; detail goes to the log. + log.warning("Rejected token: %s", exc) + raise TokenError("token is not valid") from exc + return parse_scope(claims) diff --git a/services/bugzilla-proxy/bugzilla_proxy/upstream.py b/services/bugzilla-proxy/bugzilla_proxy/upstream.py new file mode 100644 index 0000000000..d4928b0436 --- /dev/null +++ b/services/bugzilla-proxy/bugzilla_proxy/upstream.py @@ -0,0 +1,67 @@ +"""The one place that talks to Bugzilla with a real credential.""" + +from __future__ import annotations + +import logging +from typing import Any + +import httpx + +from bugzilla_proxy.config import Settings + +log = logging.getLogger(__name__) + + +class UpstreamError(Exception): + """Bugzilla could not be reached, or answered with something unusable.""" + + def __init__(self, message: str, status_code: int = 502) -> None: + super().__init__(message) + self.status_code = status_code + + +class Upstream: + """A thin async client holding the upstream credential. + + Exposes one GET and nothing else, so no code path here can write to BMO + even by mistake. + """ + + def __init__(self, settings: Settings) -> None: + self._client = httpx.AsyncClient( + base_url=settings.upstream_url.rstrip("/"), + headers={ + "X-Bugzilla-API-Key": settings.upstream_api_key, + "User-Agent": "bugzilla-proxy", + }, + timeout=settings.upstream_timeout_seconds, + ) + + async def get(self, path: str, params: dict[str, Any]) -> dict[str, Any]: + try: + response = await self._client.get(f"/{path.lstrip('/')}", params=params) + except httpx.HTTPError as exc: + log.warning("Upstream request to %s failed: %s", path, exc) + raise UpstreamError("Bugzilla is unreachable") from exc + + if response.status_code >= 500: + raise UpstreamError("Bugzilla returned a server error") + + try: + payload = response.json() + except ValueError as exc: + raise UpstreamError("Bugzilla returned a non-JSON response") from exc + + if not isinstance(payload, dict): + raise UpstreamError("Bugzilla returned an unexpected response shape") + + if response.status_code >= 400 or payload.get("error"): + # Pass Bugzilla's own complaint through rather than inventing one, + # but never its code: ours mean something different. + message = str(payload.get("message") or "Bugzilla rejected the request") + raise UpstreamError(message, status_code=response.status_code) + + return payload + + async def aclose(self) -> None: + await self._client.aclose() diff --git a/services/bugzilla-proxy/docker-compose.dev.yml b/services/bugzilla-proxy/docker-compose.dev.yml new file mode 100644 index 0000000000..5186baf737 --- /dev/null +++ b/services/bugzilla-proxy/docker-compose.dev.yml @@ -0,0 +1,20 @@ +services: + bugzilla-proxy: + build: + context: ../.. + dockerfile: services/bugzilla-proxy/Dockerfile + # Inject config into the container (relative to this compose file = repo root .env). + env_file: ../../.env + environment: + # The public half of the keypair hackbot-api signs with locally. Generate + # one with the commands in this service's README and put both halves in + # the root .env, as BUGZILLA_PROXY_JWT_PUBLIC_KEY and BZ_TOKEN_PRIVATE_KEY. + BUGZILLA_PROXY_JWT_PUBLIC_KEY: ${BUGZILLA_PROXY_JWT_PUBLIC_KEY:?generate a dev keypair first} + BUGZILLA_PROXY_UPSTREAM_API_KEY: ${BUGZILLA_API_KEY:?error} + BUGZILLA_PROXY_UPSTREAM_URL: ${BUGZILLA_API_URL:-https://bugzilla.mozilla.org/rest} + # Live-edit the source without rebuilding (cwd takes precedence on sys.path). + volumes: + - ./bugzilla_proxy:/app/bugzilla_proxy + ports: + - "8766:8080" + command: python -m bugzilla_proxy.main diff --git a/services/bugzilla-proxy/pyproject.toml b/services/bugzilla-proxy/pyproject.toml new file mode 100644 index 0000000000..691c050d1e --- /dev/null +++ b/services/bugzilla-proxy/pyproject.toml @@ -0,0 +1,37 @@ +[project] +name = "bugzilla-proxy" +version = "0.1.0" +description = "Authorization proxy for the Bugzilla REST API, scoped by per-run capability tokens" +requires-python = ">=3.12" +dependencies = [ + "fastapi>=0.109.0", + "uvicorn[standard]>=0.27.0", + "pydantic>=2.6.0", + "pydantic-settings>=2.1.0", + "httpx>=0.26.0", + "pyjwt[crypto]>=2.8.0", + "cachetools>=5.3.0", +] + +[project.optional-dependencies] +dev = [ + "pytest>=8.0.0", + "pytest-asyncio>=0.23.0", + "cryptography>=42.0.0", + # Not a runtime dependency: the tests use it to pin how an agent's client + # actually reads this service's error bodies. + "bugsy>=0.12", + # bugsy imports six but does not declare it. + "six>=1.16", +] + +[build-system] +requires = ["hatchling"] +build-backend = "hatchling.build" + +[tool.hatch.build.targets.wheel] +packages = ["bugzilla_proxy"] + +[tool.pytest.ini_options] +asyncio_mode = "auto" +testpaths = ["tests"] diff --git a/services/bugzilla-proxy/tests/conftest.py b/services/bugzilla-proxy/tests/conftest.py new file mode 100644 index 0000000000..d58dec820d --- /dev/null +++ b/services/bugzilla-proxy/tests/conftest.py @@ -0,0 +1,174 @@ +import datetime + +import jwt +import pytest +from bugzilla_proxy.config import Settings +from bugzilla_proxy.tokens import TOKEN_AUDIENCE +from cryptography import x509 +from cryptography.hazmat.primitives import hashes, serialization +from cryptography.hazmat.primitives.asymmetric import rsa +from cryptography.x509.oid import NameOID + +ISSUER = "hackbot-api@example-project.iam.gserviceaccount.com" +# Must match bugzilla_proxy.tokens.TOKEN_AUDIENCE, which is a constant, not +# a setting, so tests mint against the same literal the verifier requires. +AUDIENCE = TOKEN_AUDIENCE +KEY_ID = "abc123" + + +def _private_pem(key) -> str: + return key.private_bytes( + encoding=serialization.Encoding.PEM, + format=serialization.PrivateFormat.PKCS8, + encryption_algorithm=serialization.NoEncryption(), + ).decode() + + +def _public_pem(key) -> str: + return ( + key.public_key() + .public_bytes( + encoding=serialization.Encoding.PEM, + format=serialization.PublicFormat.SubjectPublicKeyInfo, + ) + .decode() + ) + + +def _self_signed_cert(key, subject: str) -> str: + """An x509 cert wrapping ``key``, in the shape Google publishes.""" + name = x509.Name([x509.NameAttribute(NameOID.COMMON_NAME, subject)]) + now = datetime.datetime(2024, 1, 1, tzinfo=datetime.timezone.utc) + cert = ( + x509.CertificateBuilder() + .subject_name(name) + .issuer_name(name) + .public_key(key.public_key()) + .serial_number(x509.random_serial_number()) + .not_valid_before(now) + .not_valid_after(now + datetime.timedelta(days=3650)) + .sign(key, hashes.SHA256()) + ) + return cert.public_bytes(serialization.Encoding.PEM).decode() + + +@pytest.fixture(scope="session") +def keypair() -> tuple[str, str]: + """A throwaway RSA keypair standing in for the service account's.""" + key = rsa.generate_private_key(public_exponent=65537, key_size=2048) + return _private_pem(key), _public_pem(key) + + +@pytest.fixture(scope="session") +def signing_cert(keypair) -> str: + """The public half as an x509 cert, which is how Google serves it.""" + private_pem, _public = keypair + key = serialization.load_pem_private_key(private_pem.encode(), password=None) + return _self_signed_cert(key, ISSUER) + + +@pytest.fixture +def settings(keypair) -> Settings: + """Local mode: a static PEM, no service account to verify against.""" + _private, public_pem = keypair + return Settings( + upstream_url="https://bugzilla.example.com/rest", + upstream_api_key="upstream-key", + jwt_public_key=public_pem, + token_issuer="", + ) + + +@pytest.fixture +def sa_settings() -> Settings: + """Deployed mode: verify against the issuer's published certificates.""" + return Settings( + upstream_url="https://bugzilla.example.com/rest", + upstream_api_key="upstream-key", + jwt_public_key="", + token_issuer=ISSUER, + ) + + +@pytest.fixture +def served_certs(monkeypatch, signing_cert): + """Stand in for Google's per-account certificate endpoint. + + Returns a dict the test can mutate (to simulate rotation) plus a call log + and a way to make the endpoint fail. + """ + state = {"certs": {KEY_ID: signing_cert}, "calls": [], "fail": False} + + class FakeResponse: + def __init__(self, payload): + self._payload = payload + + def raise_for_status(self): + return None + + def json(self): + return self._payload + + def fake_get(url, timeout=None): + state["calls"].append(url) + if state["fail"]: + import httpx + + raise httpx.ConnectError("boom") + return FakeResponse(state["certs"]) + + import bugzilla_proxy.tokens as tokens_module + + monkeypatch.setattr(tokens_module.httpx, "get", fake_get) + return state + + +@pytest.fixture +def mint(keypair): + """Sign a token the way hackbot-api's local path does (no key id).""" + private_pem, _public = keypair + + def _mint(bz: dict, *, headers: dict | None = None, **overrides) -> str: + claims = { + "iss": ISSUER, + "aud": AUDIENCE, + "sub": "run:11111111-2222-3333-4444-555555555555", + "jti": "token-1", + "iat": 1_700_000_000, + "exp": 4_000_000_000, + "agent": "frontend-triage", + "requested_by": "someone@mozilla.com", + "bz": bz, + } + claims.update(overrides) + return jwt.encode(claims, private_pem, algorithm="RS256", headers=headers) + + return _mint + + +@pytest.fixture +def mint_sa(mint): + """Sign the way IAM does: same key, carrying a key id in the header.""" + + def _mint(bz: dict, *, kid: str = KEY_ID, **overrides) -> str: + return mint(bz, headers={"kid": kid}, **overrides) + + return _mint + + +@pytest.fixture +def public_scope() -> dict: + """The shape phase 0 issues: public bugs, whole-bug reads, no attachments.""" + return { + "read_only": True, + "confidential": False, + "attachments": False, + "filter_content": "off", + "grants": [ + { + "tier": "full", + "anchor": {}, + "endpoints": ["bug", "bug/*/comment"], + } + ], + } diff --git a/services/bugzilla-proxy/tests/test_app.py b/services/bugzilla-proxy/tests/test_app.py new file mode 100644 index 0000000000..eefc4ddb51 --- /dev/null +++ b/services/bugzilla-proxy/tests/test_app.py @@ -0,0 +1,269 @@ +"""End-to-end behaviour of the proxy against a stubbed Bugzilla.""" + +import pytest +from bugzilla_proxy.app import create_app +from bugzilla_proxy.upstream import UpstreamError +from fastapi.testclient import TestClient + + +class FakeUpstream: + """Stands in for BMO, and records what the proxy asked it for.""" + + def __init__(self, responses: dict | None = None) -> None: + self.responses = responses or {} + self.calls: list[tuple[str, dict]] = [] + self.error: UpstreamError | None = None + + async def get(self, path: str, params: dict) -> dict: + self.calls.append((path, params)) + if self.error is not None: + raise self.error + return self.responses.get(path, {"bugs": []}) + + async def aclose(self) -> None: + return None + + def last_params(self, path: str) -> dict: + for called_path, params in reversed(self.calls): + if called_path == path: + return params + raise AssertionError(f"{path} was never requested") + + +def bug(**overrides) -> dict: + payload = { + "id": 100, + "groups": [], + "product": "Core", + "component": "DOM", + "status": "NEW", + "resolution": "", + "keywords": [], + "whiteboard": "", + "blocks": [], + "creation_time": "2024-03-01T10:00:00Z", + "summary": "A public bug", + } + payload.update(overrides) + return payload + + +@pytest.fixture +def upstream() -> FakeUpstream: + return FakeUpstream() + + +@pytest.fixture +def client(settings, upstream) -> TestClient: + return TestClient(create_app(settings, upstream=upstream)) + + +def auth(token: str) -> dict: + return {"X-Bugzilla-API-Key": token} + + +class TestAuthentication: + def test_an_unsigned_request_is_refused(self, client): + response = client.get("/rest/bug") + assert response.status_code == 401 + assert response.json()["code"] == 102 + + def test_garbage_in_the_key_header_is_refused(self, client): + response = client.get("/rest/bug", headers=auth("not-a-token")) + assert response.status_code == 401 + + def test_healthz_needs_no_token(self, client): + assert client.get("/healthz").json() == {"status": "ok"} + + +class TestDefaultDeny: + def test_an_unexposed_endpoint_is_a_101(self, client, mint, public_scope): + response = client.get( + "/rest/user/someone@mozilla.com", headers=auth(mint(public_scope)) + ) + assert response.status_code == 404 + assert response.json()["code"] == 101 + + def test_writes_are_refused(self, client, mint, public_scope): + response = client.post("/rest/bug", headers=auth(mint(public_scope))) + assert response.status_code == 405 + assert response.json()["code"] == 101 + + def test_nothing_reaches_upstream_when_denied( + self, client, mint, public_scope, upstream + ): + client.get("/rest/user/x", headers=auth(mint(public_scope))) + assert upstream.calls == [] + + +class TestSearch: + def test_public_bugs_pass_through(self, client, mint, public_scope, upstream): + upstream.responses["bug"] = {"bugs": [bug(), bug(id=101)]} + response = client.get("/rest/bug", headers=auth(mint(public_scope))) + assert [b["id"] for b in response.json()["bugs"]] == [100, 101] + + def test_private_bugs_are_dropped_from_results( + self, client, mint, public_scope, upstream + ): + upstream.responses["bug"] = { + "bugs": [bug(), bug(id=101, groups=["core-security"])] + } + response = client.get("/rest/bug", headers=auth(mint(public_scope))) + assert [b["id"] for b in response.json()["bugs"]] == [100] + + def test_a_bug_outside_the_anchor_is_dropped(self, client, mint, upstream): + scope = { + "grants": [ + { + "tier": "full", + "anchor": {"product": ["Core"]}, + "endpoints": ["bug"], + } + ] + } + upstream.responses["bug"] = {"bugs": [bug(), bug(id=101, product="Firefox")]} + response = client.get("/rest/bug", headers=auth(mint(scope))) + assert [b["id"] for b in response.json()["bugs"]] == [100] + + def test_auth_fields_are_added_to_the_upstream_query( + self, client, mint, public_scope, upstream + ): + client.get( + "/rest/bug?include_fields=id,summary", headers=auth(mint(public_scope)) + ) + fields = set(upstream.last_params("bug")["include_fields"].split(",")) + assert {"groups", "product", "id", "summary"} <= fields + + def test_the_caller_only_gets_the_fields_it_asked_for( + self, client, mint, public_scope, upstream + ): + upstream.responses["bug"] = {"bugs": [bug()]} + response = client.get( + "/rest/bug?include_fields=id,summary", headers=auth(mint(public_scope)) + ) + assert response.json()["bugs"] == [{"id": 100, "summary": "A public bug"}] + + def test_a_metadata_tier_hides_fields_it_needed_for_the_decision( + self, client, mint, upstream + ): + scope = { + "grants": [ + { + "tier": "metadata", + "anchor": {"product": ["Core"]}, + "endpoints": ["bug"], + } + ] + } + upstream.responses["bug"] = {"bugs": [bug(whiteboard="[secret-plan]")]} + response = client.get("/rest/bug", headers=auth(mint(scope))) + served = response.json()["bugs"][0] + assert "whiteboard" not in served + assert "groups" not in served + assert served["summary"] == "A public bug" + + def test_credentials_in_the_query_string_are_stripped( + self, client, mint, public_scope, upstream + ): + client.get( + "/rest/bug?api_key=smuggled&Bugzilla_token=also", + headers=auth(mint(public_scope)), + ) + params = upstream.last_params("bug") + assert "api_key" not in params + assert "Bugzilla_token" not in params + + def test_the_search_limit_is_capped( + self, client, mint, public_scope, upstream, settings + ): + client.get("/rest/bug?limit=100000", headers=auth(mint(public_scope))) + assert upstream.last_params("bug")["limit"] == str(settings.max_search_limit) + + def test_search_is_refused_when_no_grant_exposes_it(self, client, mint): + scope = { + "grants": [{"tier": "full", "anchor": {}, "endpoints": ["bug/*/comment"]}] + } + response = client.get("/rest/bug", headers=auth(mint(scope))) + assert response.json()["code"] == 101 + + +class TestComments: + def test_comments_are_served_for_an_in_scope_bug( + self, client, mint, public_scope, upstream + ): + upstream.responses["bug"] = {"bugs": [bug()]} + upstream.responses["bug/100/comment"] = {"bugs": {"100": {"comments": []}}} + response = client.get("/rest/bug/100/comment", headers=auth(mint(public_scope))) + assert response.status_code == 200 + assert "bugs" in response.json() + + def test_comments_on_a_private_bug_are_refused( + self, client, mint, public_scope, upstream + ): + upstream.responses["bug"] = {"bugs": [bug(groups=["core-security"])]} + response = client.get("/rest/bug/100/comment", headers=auth(mint(public_scope))) + assert response.json()["code"] == 102 + assert "bug/100/comment" not in [path for path, _ in upstream.calls] + + def test_a_metadata_tier_cannot_reach_comments(self, client, mint, upstream): + scope = {"grants": [{"tier": "metadata", "anchor": {}, "endpoints": ["bug"]}]} + upstream.responses["bug"] = {"bugs": [bug()]} + response = client.get("/rest/bug/100/comment", headers=auth(mint(scope))) + assert response.json()["code"] == 102 + + def test_a_missing_bug_looks_the_same_as_a_forbidden_one( + self, client, mint, public_scope, upstream + ): + upstream.responses["bug"] = {"bugs": []} + response = client.get("/rest/bug/100/comment", headers=auth(mint(public_scope))) + assert response.json()["code"] == 102 + assert "not available" in response.json()["message"] + + +class TestAttachments: + def test_attachments_are_refused_unless_the_token_allows_them( + self, client, mint, public_scope, upstream + ): + upstream.responses["bug"] = {"bugs": [bug()]} + response = client.get( + "/rest/bug/100/attachment", headers=auth(mint(public_scope)) + ) + assert response.json()["code"] == 101 + + def test_an_attachment_is_authorized_by_the_bug_behind_it( + self, client, mint, upstream + ): + scope = { + "attachments": True, + "grants": [ + { + "tier": "full", + "anchor": {}, + "endpoints": ["bug", "bug/attachment/*"], + } + ], + } + upstream.responses["bug"] = {"bugs": [bug(groups=["core-security"])]} + upstream.responses["bug/attachment/55"] = { + "attachments": {"55": {"id": 55, "bug_id": 100, "data": "..."}} + } + response = client.get("/rest/bug/attachment/55", headers=auth(mint(scope))) + assert response.json()["code"] == 102 + + +class TestUpstreamFailures: + def test_an_upstream_error_becomes_a_502( + self, client, mint, public_scope, upstream + ): + upstream.error = UpstreamError("Bugzilla is unreachable") + response = client.get("/rest/bug", headers=auth(mint(public_scope))) + assert response.status_code == 502 + assert response.json()["code"] == 103 + + def test_a_credential_complaint_is_not_relayed_as_a_login_error( + self, client, mint, public_scope, upstream + ): + """Bugsy turns any message mentioning an API key into a LoginException.""" + upstream.error = UpstreamError("The API key you supplied is invalid") + response = client.get("/rest/bug", headers=auth(mint(public_scope))) + assert "API key" not in response.json()["message"] diff --git a/services/bugzilla-proxy/tests/test_bugsy_contract.py b/services/bugzilla-proxy/tests/test_bugsy_contract.py new file mode 100644 index 0000000000..76142cfcb8 --- /dev/null +++ b/services/bugzilla-proxy/tests/test_bugsy_contract.py @@ -0,0 +1,84 @@ +"""The contract between this proxy's errors and the agent's client. + +`agent_tools.bugzilla` keys its handling off `BugsyException.code`: 101 is +"endpoint not exposed", 102 is "skip this bug". Those codes only survive if our +bodies parse the way bugsy expects and none trip its login-problem branch, which +is easy to break from this side without noticing. +""" + +import bugsy +import pytest +from bugzilla_proxy.app import create_app +from fastapi.testclient import TestClient + + +class ResponseLike: + """The parts of a `requests.Response` that bugsy's error path touches.""" + + def __init__(self, status_code: int, payload: dict) -> None: + self.status_code = status_code + self._payload = payload + self.text = str(payload) + + def json(self) -> dict: + return self._payload + + +def handle(response: ResponseLike): + client = bugsy.Bugsy(api_key="tok", bugzilla_url="https://proxy.example/rest") + return client._handle_errors(response) + + +def proxy_error(client: TestClient, path: str, token: str) -> ResponseLike: + """Make the proxy produce a real error body, then hand it to bugsy.""" + response = client.get(path, headers={"X-Bugzilla-API-Key": token}) + return ResponseLike(response.status_code, response.json()) + + +class TestErrorCodesSurvive: + def test_an_unexposed_endpoint_reaches_the_agent_as_101( + self, settings, mint, public_scope + ): + client = TestClient(create_app(settings, upstream=_NoUpstream())) + raw = proxy_error(client, "/rest/user/someone", mint(public_scope)) + with pytest.raises(bugsy.BugsyException) as excinfo: + handle(raw) + assert excinfo.value.code == 101 + + def test_a_denied_bug_reaches_the_agent_as_102(self, settings, mint, public_scope): + upstream = _NoUpstream( + {"bug": {"bugs": [{"id": 1, "groups": ["core-security"]}]}} + ) + client = TestClient(create_app(settings, upstream=upstream)) + raw = proxy_error(client, "/rest/bug/1/comment", mint(public_scope)) + with pytest.raises(bugsy.BugsyException) as excinfo: + handle(raw) + assert excinfo.value.code == 102 + + def test_no_error_is_mistaken_for_a_login_failure( + self, settings, mint, public_scope + ): + """No error body may mention an API key or a username and password. + + bugsy turns those into a LoginException, which the tools do not handle. + """ + client = TestClient(create_app(settings, upstream=_NoUpstream())) + for path in ("/rest/user/someone", "/rest/bug/1/comment"): + raw = proxy_error(client, path, mint(public_scope)) + with pytest.raises(bugsy.BugsyException) as excinfo: + handle(raw) + assert not isinstance(excinfo.value, bugsy.LoginException) + + def test_a_successful_body_passes_through_untouched(self): + assert handle(ResponseLike(200, {"bugs": [{"id": 7}]})) == {"bugs": [{"id": 7}]} + + +class _NoUpstream: + def __init__(self, responses: dict | None = None) -> None: + self.responses = responses or {} + + async def get(self, path: str, params: dict) -> dict: + return self.responses.get(path, {"bugs": []}) + + async def aclose(self) -> None: + return None diff --git a/services/bugzilla-proxy/tests/test_scope.py b/services/bugzilla-proxy/tests/test_scope.py new file mode 100644 index 0000000000..6acf93c1a4 --- /dev/null +++ b/services/bugzilla-proxy/tests/test_scope.py @@ -0,0 +1,203 @@ +"""Scope evaluation, especially the ways it must refuse.""" + +from bugzilla_proxy.scope import ( + AUTH_FIELDS, + DEFAULT_METADATA_FIELDS, + Anchor, + Grant, + Scope, +) + + +def public_bug(**overrides) -> dict: + bug = { + "id": 100, + "groups": [], + "product": "Core", + "component": "DOM", + "status": "NEW", + "resolution": "", + "keywords": ["regression"], + "whiteboard": "[fidefe-triage]", + "blocks": [900], + "creation_time": "2024-03-01T10:00:00Z", + "summary": "A public bug", + } + bug.update(overrides) + return bug + + +def grant(**overrides) -> Grant: + defaults = { + "tier": "full", + "anchor": Anchor(), + "endpoints": ("bug",), + } + defaults.update(overrides) + return Grant(**defaults) + + +class TestGroupCeiling: + def test_public_bug_is_admitted_by_a_public_grant(self): + assert grant().permits(public_bug()) + + def test_private_bug_is_refused_by_a_public_grant(self): + assert not grant().permits(public_bug(groups=["core-security"])) + + def test_private_bug_is_admitted_when_its_group_is_granted(self): + g = grant(groups=frozenset({"core-security"})) + assert g.permits(public_bug(groups=["core-security"])) + + def test_a_single_ungranted_group_is_enough_to_refuse(self): + g = grant(groups=frozenset({"core-security"})) + assert not g.permits(public_bug(groups=["core-security", "partner-nda"])) + + def test_group_grant_still_admits_public_bugs(self): + g = grant(groups=frozenset({"core-security"})) + assert g.permits(public_bug(groups=[])) + + def test_a_bug_without_a_groups_field_is_refused(self): + """Missing means unprovable, and unprovable must not mean public.""" + bug = public_bug() + del bug["groups"] + assert not grant(groups=frozenset({"core-security"})).permits(bug) + + +class TestAnchorRules: + def test_every_configured_rule_must_hold(self): + g = grant(anchor=Anchor(product=frozenset({"Core"}), status=frozenset({"NEW"}))) + assert g.permits(public_bug()) + assert not g.permits(public_bug(status="RESOLVED")) + + def test_static_bugs_restricts_to_listed_ids(self): + g = grant(anchor=Anchor(static_bugs=frozenset({100}))) + assert g.permits(public_bug(id=100)) + assert not g.permits(public_bug(id=101)) + + def test_keywords_match_on_intersection(self): + g = grant(anchor=Anchor(keywords=frozenset({"regression"}))) + assert g.permits(public_bug()) + assert not g.permits(public_bug(keywords=["perf"])) + + def test_whiteboard_matches_on_substring(self): + g = grant(anchor=Anchor(whiteboard=("[fidefe-triage]",))) + assert g.permits(public_bug()) + assert not g.permits(public_bug(whiteboard="[other]")) + + def test_created_after_excludes_older_bugs(self): + g = grant(anchor=Anchor(created_after="2024-01-01T00:00:00Z")) + assert g.permits(public_bug()) + assert not g.permits(public_bug(creation_time="2019-06-01T10:00:00Z")) + + def test_created_after_refuses_a_bug_with_no_creation_time(self): + g = grant(anchor=Anchor(created_after="2024-01-01T00:00:00Z")) + bug = public_bug() + del bug["creation_time"] + assert not g.permits(bug) + + def test_an_empty_anchor_matches_anything_public(self): + assert grant().permits(public_bug(product="Firefox", status="RESOLVED")) + + def test_structural_rules_are_distinguished_from_narrowing_ones(self): + assert Anchor(product=frozenset({"Core"})).has_structural_rule() + assert not Anchor(keywords=frozenset({"sec-high"})).has_structural_rule() + assert not Anchor(whiteboard=("[x]",)).has_structural_rule() + assert not Anchor(blocks=frozenset({12})).has_structural_rule() + + +class TestEndpointPatterns: + def test_exact_and_wildcard_segments(self): + g = grant(endpoints=("bug", "bug/*/comment")) + assert g.allows_endpoint("bug") + assert g.allows_endpoint("bug/123/comment") + assert not g.allows_endpoint("bug/123/attachment") + + def test_a_wildcard_covers_exactly_one_segment(self): + g = grant(endpoints=("bug/*/comment",)) + assert not g.allows_endpoint("bug/comment") + assert not g.allows_endpoint("bug/1/2/comment") + + +class TestProjection: + def test_metadata_tier_drops_everything_outside_its_field_set(self): + g = grant(tier="metadata") + projected = g.project(public_bug()) + assert set(projected) <= DEFAULT_METADATA_FIELDS + assert "whiteboard" not in projected + assert "groups" not in projected + assert projected["summary"] == "A public bug" + + def test_full_tier_keeps_what_upstream_sent(self): + projected = grant().project(public_bug()) + assert projected["whiteboard"] == "[fidefe-triage]" + + def test_an_explicit_field_list_wins_over_the_tier_default(self): + g = grant(tier="metadata", fields=frozenset({"id", "summary"})) + assert set(g.project(public_bug())) == {"id", "summary"} + + def test_the_caller_can_narrow_but_not_widen(self): + g = grant(tier="metadata") + projected = g.project(public_bug(), requested=frozenset({"id", "whiteboard"})) + assert projected == {"id": 100} + + +class TestScopeSelection: + def test_the_highest_matching_tier_wins(self): + scope = Scope( + run_id="r", + agent="a", + jti="j", + grants=( + grant(tier="metadata", endpoints=("bug",)), + grant( + tier="full", + anchor=Anchor(static_bugs=frozenset({100})), + endpoints=("bug",), + ), + ), + ) + assert scope.grant_for(public_bug(id=100)).tier == "full" + assert scope.grant_for(public_bug(id=101)).tier == "metadata" + + def test_endpoint_selection_ignores_grants_that_do_not_expose_it(self): + scope = Scope( + run_id="r", + agent="a", + jti="j", + grants=( + grant(tier="metadata", endpoints=("bug",)), + grant( + tier="full", + anchor=Anchor(static_bugs=frozenset({100})), + endpoints=("bug", "bug/*/comment"), + ), + ), + ) + bug = public_bug(id=101) + assert scope.grant_for(bug) is not None + assert scope.grant_for_endpoint(bug, "bug/*/comment") is None + + def test_a_scope_is_private_when_any_grant_is(self): + scope = Scope( + run_id="r", + agent="a", + jti="j", + grants=(grant(), grant(groups=frozenset({"core-security"}))), + ) + assert scope.is_private + + +class TestUpstreamFields: + def test_auth_fields_are_always_requested(self): + scope = Scope(run_id="r", agent="a", jti="j", grants=(grant(tier="metadata"),)) + assert AUTH_FIELDS <= scope.upstream_fields(frozenset({"id", "summary"})) + + def test_a_full_grant_asks_for_the_default_set_plus_auth_fields(self): + scope = Scope(run_id="r", agent="a", jti="j", grants=(grant(),)) + fields = scope.upstream_fields(None) + assert "_default" in fields + assert AUTH_FIELDS <= fields + + def test_metadata_only_scopes_ask_for_no_more_than_they_can_show(self): + scope = Scope(run_id="r", agent="a", jti="j", grants=(grant(tier="metadata"),)) + assert scope.upstream_fields(None) == DEFAULT_METADATA_FIELDS | AUTH_FIELDS diff --git a/services/bugzilla-proxy/tests/test_tokens.py b/services/bugzilla-proxy/tests/test_tokens.py new file mode 100644 index 0000000000..0885d15c17 --- /dev/null +++ b/services/bugzilla-proxy/tests/test_tokens.py @@ -0,0 +1,310 @@ +"""Token verification and the claim shapes it refuses.""" + +import jwt +import pytest +from bugzilla_proxy.tokens import ( + CERTS_URL_TEMPLATE, + TOKEN_AUDIENCE, + TokenError, + TokenVerifier, + parse_scope, +) + + +def claims(bz: dict, **overrides) -> dict: + base = { + "sub": "run:abc", + "jti": "t1", + "agent": "frontend-triage", + "requested_by": "someone@mozilla.com", + "bz": bz, + } + base.update(overrides) + return base + + +PUBLIC_GRANT = {"tier": "full", "anchor": {}, "endpoints": ["bug"]} + + +class TestParseScope: + def test_a_public_scope_parses(self): + scope = parse_scope(claims({"grants": [PUBLIC_GRANT]})) + assert scope.run_id == "abc" + assert scope.agent == "frontend-triage" + assert not scope.is_private + assert not scope.confidential + + def test_sub_must_name_a_run(self): + with pytest.raises(TokenError, match="run:"): + parse_scope(claims({"grants": [PUBLIC_GRANT]}, sub="someone")) + + def test_jti_is_required(self): + with pytest.raises(TokenError, match="jti"): + parse_scope(claims({"grants": [PUBLIC_GRANT]}, jti="")) + + def test_grants_must_be_present(self): + with pytest.raises(TokenError, match="grants"): + parse_scope(claims({"grants": []})) + + def test_writes_are_refused(self): + with pytest.raises(TokenError, match="reads only"): + parse_scope(claims({"read_only": False, "grants": [PUBLIC_GRANT]})) + + def test_an_unimplemented_filter_mode_is_refused_rather_than_ignored(self): + """Serving unfiltered content because we cannot filter is the bad outcome.""" + with pytest.raises(TokenError, match="not implemented"): + parse_scope(claims({"filter_content": "llm", "grants": [PUBLIC_GRANT]})) + + def test_unknown_endpoint_patterns_are_refused(self): + grant = {"tier": "full", "anchor": {}, "endpoints": ["*"]} + with pytest.raises(TokenError, match="unknown endpoint"): + parse_scope(claims({"grants": [grant]})) + + def test_a_grant_needs_at_least_one_endpoint(self): + grant = {"tier": "full", "anchor": {}, "endpoints": []} + with pytest.raises(TokenError, match="at least one endpoint"): + parse_scope(claims({"grants": [grant]})) + + def test_an_unknown_tier_is_refused(self): + grant = {"tier": "everything", "anchor": {}, "endpoints": ["bug"]} + with pytest.raises(TokenError, match="unknown tier"): + parse_scope(claims({"grants": [grant]})) + + +class TestPrivateGrantGuards: + def private_grant(self, anchor: dict) -> dict: + return {"tier": "full", "anchor": anchor, "endpoints": ["bug"]} + + def test_a_private_grant_needs_a_structural_rule(self): + grant = self.private_grant( + {"groups": ["core-security"], "whiteboard": ["[sec-triage]"]} + ) + with pytest.raises(TokenError, match="structural rule"): + parse_scope(claims({"confidential": True, "grants": [grant]})) + + def test_a_narrowing_rule_alone_is_not_enough(self): + grant = self.private_grant( + {"groups": ["core-security"], "keywords": ["sec-high"]} + ) + with pytest.raises(TokenError, match="structural rule"): + parse_scope(claims({"confidential": True, "grants": [grant]})) + + def test_a_structural_rule_satisfies_the_guard(self): + grant = self.private_grant( + { + "groups": ["core-security"], + "product": ["Core"], + "keywords": ["sec-high"], + } + ) + scope = parse_scope(claims({"confidential": True, "grants": [grant]})) + assert scope.is_private + + def test_static_bugs_counts_as_structural(self): + grant = self.private_grant( + {"groups": ["core-security"], "static_bugs": [1899123]} + ) + scope = parse_scope(claims({"confidential": True, "grants": [grant]})) + assert scope.is_private + + def test_a_private_grant_requires_a_confidential_run(self): + grant = self.private_grant( + {"groups": ["core-security"], "static_bugs": [1899123]} + ) + with pytest.raises(TokenError, match="confidential"): + parse_scope(claims({"confidential": False, "grants": [grant]})) + + +class TestVerifier: + def test_a_well_formed_token_verifies(self, settings, mint, public_scope): + scope = TokenVerifier(settings).verify(mint(public_scope)) + assert scope.agent == "frontend-triage" + assert scope.requested_by == "someone@mozilla.com" + + def test_no_token_is_refused(self, settings): + with pytest.raises(TokenError, match="no token"): + TokenVerifier(settings).verify("") + + def test_a_token_signed_by_someone_else_is_refused( + self, settings, public_scope, keypair + ): + from cryptography.hazmat.primitives import serialization + from cryptography.hazmat.primitives.asymmetric import rsa + + other = rsa.generate_private_key(public_exponent=65537, key_size=2048) + pem = other.private_bytes( + encoding=serialization.Encoding.PEM, + format=serialization.PrivateFormat.PKCS8, + encryption_algorithm=serialization.NoEncryption(), + ).decode() + forged = jwt.encode( + { + "iss": settings.token_issuer, + "aud": TOKEN_AUDIENCE, + "sub": "run:abc", + "jti": "t1", + "iat": 1_700_000_000, + "exp": 4_000_000_000, + "bz": public_scope, + }, + pem, + algorithm="RS256", + ) + with pytest.raises(TokenError, match="not valid"): + TokenVerifier(settings).verify(forged) + + def test_an_expired_token_is_refused(self, settings, mint, public_scope): + expired = mint(public_scope, exp=1_700_000_001, iat=1_700_000_000) + with pytest.raises(TokenError, match="not valid"): + TokenVerifier(settings).verify(expired) + + def test_a_token_for_another_audience_is_refused( + self, settings, mint, public_scope + ): + wrong = mint(public_scope, aud="some-other-service") + with pytest.raises(TokenError, match="not valid"): + TokenVerifier(settings).verify(wrong) + + def test_exactly_one_key_source_must_be_configured(self, settings): + settings.token_issuer = "hackbot-api@p.iam.gserviceaccount.com" + with pytest.raises(ValueError, match="exactly one"): + TokenVerifier(settings) + + def test_no_key_source_at_all_is_refused(self, settings): + settings.jwt_public_key = "" + settings.token_issuer = "" + with pytest.raises(ValueError, match="exactly one"): + TokenVerifier(settings) + + +class TestServiceAccountVerification: + """The deployed path, where no key material is configured on either side. + + Covers what replaces it: fetching the right account's certs, picking the + key by id, and binding the issuer. + """ + + def test_a_token_signed_by_the_issuer_verifies( + self, sa_settings, mint_sa, public_scope, served_certs + ): + scope = TokenVerifier(sa_settings).verify(mint_sa(public_scope)) + assert scope.agent == "frontend-triage" + + def test_the_certs_are_fetched_from_the_issuers_own_url( + self, sa_settings, mint_sa, public_scope, served_certs + ): + """An issuer-specific URL is itself the binding to that issuer.""" + TokenVerifier(sa_settings).verify(mint_sa(public_scope)) + assert served_certs["calls"] == [ + "https://www.googleapis.com/robot/v1/metadata/x509/" + "hackbot-api@example-project.iam.gserviceaccount.com" + ] + + def test_certs_are_cached_across_requests( + self, sa_settings, mint_sa, public_scope, served_certs + ): + verifier = TokenVerifier(sa_settings) + verifier.verify(mint_sa(public_scope)) + verifier.verify(mint_sa(public_scope)) + assert len(served_certs["calls"]) == 1 + + def test_a_token_from_another_issuer_is_refused( + self, sa_settings, mint_sa, public_scope, served_certs + ): + wrong = mint_sa(public_scope, iss="someone-else@evil.iam.gserviceaccount.com") + with pytest.raises(TokenError, match="not valid"): + TokenVerifier(sa_settings).verify(wrong) + + def test_a_token_with_no_key_id_is_refused( + self, sa_settings, mint, public_scope, served_certs + ): + with pytest.raises(TokenError, match="no key id"): + TokenVerifier(sa_settings).verify(mint(public_scope)) + + def test_an_unknown_key_id_triggers_one_refetch( + self, sa_settings, mint_sa, public_scope, served_certs, signing_cert + ): + """Google rotates keys, so a miss is worth one refetch before failing.""" + verifier = TokenVerifier(sa_settings) + verifier.verify(mint_sa(public_scope)) + served_certs["certs"] = {"rotated": signing_cert} + + scope = verifier.verify(mint_sa(public_scope, kid="rotated")) + assert scope.agent == "frontend-triage" + assert len(served_certs["calls"]) == 2 + + def test_a_key_id_that_is_still_unknown_after_refetch_is_refused( + self, sa_settings, mint_sa, public_scope, served_certs + ): + with pytest.raises(TokenError, match="unknown signing key"): + TokenVerifier(sa_settings).verify(mint_sa(public_scope, kid="nope")) + + def test_a_stale_cache_survives_a_fetch_failure( + self, sa_settings, mint_sa, public_scope, served_certs + ): + """Better slightly stale keys than every run losing Bugzilla at once.""" + verifier = TokenVerifier(sa_settings) + verifier.verify(mint_sa(public_scope)) + verifier._keys._expires_at = 0.0 + served_certs["fail"] = True + + assert verifier.verify(mint_sa(public_scope)).agent == "frontend-triage" + + def test_a_fetch_failure_with_no_cache_is_refused( + self, sa_settings, mint_sa, public_scope, served_certs + ): + served_certs["fail"] = True + with pytest.raises(TokenError, match="cannot fetch"): + TokenVerifier(sa_settings).verify(mint_sa(public_scope)) + + +class TestCrossServiceContract: + """The audience is one literal duplicated in two packages. + + They cannot import each other, so drift would pass every unit test here + and reject every token in production. + """ + + def _hackbot_api_constant(self, name: str) -> str | None: + import ast + import pathlib + + source = ( + pathlib.Path(__file__).resolve().parents[2] + / "hackbot-api" + / "app" + / "bz_token.py" + ) + if not source.exists(): + pytest.skip("hackbot-api is not checked out beside this service") + tree = ast.parse(source.read_text()) + for node in tree.body: + if not isinstance(node, ast.Assign): + continue + for target in node.targets: + if isinstance(target, ast.Name) and target.id == name: + return ast.literal_eval(node.value) + return None + + def test_the_audience_matches_what_hackbot_api_mints(self): + assert self._hackbot_api_constant("TOKEN_AUDIENCE") == TOKEN_AUDIENCE + + def test_the_audience_is_not_a_google_endpoint(self): + """Google would accept such a token as a credential for the signer.""" + host = TOKEN_AUDIENCE.split("://")[-1].split("/")[0].lower() + assert host != "googleapis.com" + assert not host.endswith(".googleapis.com") + + def test_the_certs_url_is_googles_over_https(self): + """The URL is pinned, not merely defaulted. + + Any other host, or plain HTTP, hands the choice of trusted keys to + whoever answers the request. + """ + assert CERTS_URL_TEMPLATE.startswith("https://www.googleapis.com/") + assert "{service_account}" in CERTS_URL_TEMPLATE + + def test_the_certs_url_is_scoped_to_one_account(self): + """A shared endpoint would let any Google account's key verify.""" + rendered = CERTS_URL_TEMPLATE.format(service_account="someone@example.com") + assert rendered.endswith("/someone@example.com") diff --git a/services/hackbot-api/app/agents.py b/services/hackbot-api/app/agents.py index 0876868949..cd0e8b21c6 100644 --- a/services/hackbot-api/app/agents.py +++ b/services/hackbot-api/app/agents.py @@ -4,6 +4,7 @@ from pydantic import BaseModel +from app.bz_token import BugzillaScope from app.schemas import ( AutowebcompatDiagnosisInputs, AutowebcompatReproInputs, @@ -28,6 +29,15 @@ class AgentSpec: # succeeds. Off by default: actions are still recorded and can always be # applied manually from the UI; only opted-in agents auto-apply. auto_apply_actions: bool = False + # The Bugzilla access this agent's runs get, minted per run as a capability + # token for `bugzilla-proxy` and handed to the broker container. None means + # the broker falls back to its own Bugzilla credential. + # + # No agent sets this yet, on purpose: the proxy is not deployed. Setting it + # is the last step of onboarding an agent, after the service is up and its + # broker knows how to present a token. See docs/hackbot/bugzilla-proxy.md; + # `bz_token.PUBLIC_READ_SCOPE` is the template the first one will use. + bugzilla_scope: BugzillaScope | None = None # Whether auto-apply additionally needs the run's own say-so: `findings.auto_apply` # must be True, or the actions are recorded and held. Set this for agents that # judge their results one run at a time, since only the agent knows how sure it diff --git a/services/hackbot-api/app/bz_token.py b/services/hackbot-api/app/bz_token.py new file mode 100644 index 0000000000..276555e998 --- /dev/null +++ b/services/hackbot-api/app/bz_token.py @@ -0,0 +1,252 @@ +"""Minting the per-run Bugzilla capability token. + +Lets a run read Bugzilla through `bugzilla-proxy` without holding a BMO +credential. Built entirely from the agent's registered scope template: **a +caller never supplies a scope**. + +Signing goes through IAM's `signJwt`, as this service's own identity, using a +Google-managed key. Nothing here holds or provisions key material, and the proxy +verifies against the certificates Google publishes for the same account, so the +two share one string (that account's email) and no keys are exchanged. A PEM +stands in for local runs. +""" + +import base64 +import json +import logging +from dataclasses import dataclass +from datetime import datetime, timedelta, timezone +from functools import lru_cache +from typing import Any +from uuid import uuid4 + +from app.config import settings + +log = logging.getLogger(__name__) + +_JWT_HEADER = {"alg": "RS256", "typ": "JWT"} + +# IAM refuses to sign a JWT more than 12 hours out. Comfortably past the 8 hour +# default job timeout, but a hard ceiling on one token's reach. +MAX_TOKEN_LIFETIME_SECONDS = 12 * 60 * 60 + + +# The `bugzilla-proxy` service verifies against the same literal. +TOKEN_AUDIENCE = "hackbot-bugzilla-proxy" +LOCAL_ISSUER = "hackbot-api" + + +@dataclass(frozen=True) +class BugzillaScope: + """What one agent's runs may read, before per-run substitution. + + On the registry entry rather than in the inputs, which is what keeps the + scope server-determined. `grants` holds raw claims in the wire format the + proxy parses; `"$bug_id"` in a `static_bugs` list becomes this run's bug. + """ + + grants: tuple[dict[str, Any], ...] + attachments: bool = False + filter_content: str = "off" + confidential: bool = False + promotions_max: int = 0 + + def resolve(self, inputs: Any) -> dict[str, Any]: + """Render the `bz` claim for one run.""" + bug_id = getattr(inputs, "bug_id", None) + return { + "read_only": True, + "confidential": self.confidential, + "attachments": self.attachments, + "filter_content": self.filter_content, + "promotions_max": self.promotions_max, + "grants": [_substitute(grant, bug_id) for grant in self.grants], + } + + +def _substitute(grant: dict[str, Any], bug_id: int | None) -> dict[str, Any]: + """Replace `$bug_id` placeholders with this run's bug. + + Raises when the run has none: an empty allowlist would deny everything and + a dropped rule would allow everything, so neither is a safe default. + """ + resolved = json.loads(json.dumps(grant)) + anchor = resolved.get("anchor") + if not isinstance(anchor, dict): + return resolved + static = anchor.get("static_bugs") + if not isinstance(static, list) or "$bug_id" not in static: + return resolved + if bug_id is None: + raise ValueError( + "scope template references $bug_id but the run's inputs have none" + ) + anchor["static_bugs"] = [ + int(bug_id) if entry == "$bug_id" else entry for entry in static + ] + return resolved + + +@lru_cache(maxsize=1) +def _iam_client(): + """The IAM Credentials client, built once and reused. + + A function rather than a module-level import, so the dependency is only + touched on the signing path and tests can stub it. + """ + from google.cloud import iam_credentials_v1 + + return iam_credentials_v1.IAMCredentialsClient() + + +def _sign_with_service_account(claims: dict[str, Any]) -> str: + """Have IAM sign these claims as this service's own identity. + + Needs `iam.serviceAccounts.signJwt` on the account being signed as, granted + by `roles/iam.serviceAccountTokenCreator`, which this service already holds + on itself for the GCS signed-policy path. + """ + response = _iam_client().sign_jwt( + request={ + "name": f"projects/-/serviceAccounts/{settings.bz_token_service_account}", + "payload": json.dumps(claims, separators=(",", ":")), + } + ) + return response.signed_jwt + + +def _sign_locally(claims: dict[str, Any]) -> str: + """Sign with a PEM from the environment, for local runs and tests.""" + from cryptography.hazmat.primitives import hashes + from cryptography.hazmat.primitives.asymmetric import padding + from cryptography.hazmat.primitives.serialization import load_pem_private_key + + signing_input = ".".join( + ( + _b64(json.dumps(_JWT_HEADER, separators=(",", ":")).encode()), + _b64(json.dumps(claims, separators=(",", ":")).encode()), + ) + ).encode() + key = load_pem_private_key(settings.bz_token_private_key.encode(), password=None) + signature = key.sign(signing_input, padding.PKCS1v15(), hashes.SHA256()) + return f"{signing_input.decode()}.{_b64(signature)}" + + +def _b64(raw: bytes) -> str: + return base64.urlsafe_b64encode(raw).rstrip(b"=").decode() + + +def is_configured() -> bool: + """True if this deployment can mint tokens at all. + + False is legitimate during rollout: the broker then falls back to its own + Bugzilla credential, which is the way back if the proxy misbehaves. + """ + return bool(settings.bz_token_service_account or settings.bz_token_private_key) + + +def mint( + *, + run_id: str, + agent: str, + scope: BugzillaScope, + inputs: Any, + requested_by: str | None, +) -> str: + """Build and sign this run's token.""" + if not is_configured(): + raise RuntimeError("no signing key configured") + + lifetime = settings.job_execution_timeout_seconds + settings.bz_token_grace_seconds + if lifetime > MAX_TOKEN_LIFETIME_SECONDS: + # Caught here because IAM's rejection would not mention the job timeout. + raise RuntimeError( + f"a token would need to live {lifetime}s " + f"(job_execution_timeout_seconds + bz_token_grace_seconds), but IAM " + f"will not sign one past {MAX_TOKEN_LIFETIME_SECONDS}s" + ) + + now = datetime.now(timezone.utc) + # Must outlive the job: a run that loses Bugzilla access partway through + # fails for a reason that looks nothing like the cause. + expires = now + timedelta(seconds=lifetime) + + claims = { + # The proxy checks this, and it names whose certificates verify it. + "iss": settings.bz_token_service_account or LOCAL_ISSUER, + "aud": TOKEN_AUDIENCE, + "sub": f"run:{run_id}", + "jti": uuid4().hex, + "iat": int(now.timestamp()), + "exp": int(expires.timestamp()), + "agent": agent, + "requested_by": requested_by, + "bz": scope.resolve(inputs), + } + + if settings.bz_token_service_account: + return _sign_with_service_account(claims) + return _sign_locally(claims) + + +def broker_env_for( + *, + run_id: str, + agent: str, + scope: BugzillaScope | None, + inputs: Any, + requested_by: str | None, +) -> dict[str, str]: + """The broker container's per-run environment, or empty. + + The only thing the API sets on `broker`; `jobs.trigger_execution` refuses + anything outside its allowlist. + """ + if scope is None or not is_configured(): + return {} + token = mint( + run_id=run_id, + agent=agent, + scope=scope, + inputs=inputs, + requested_by=requested_by, + ) + return {"BUGZILLA_SCOPE_TOKEN": token} + + +# Convenience for registry entries: the shape phase 0 issues, where the proxy's +# upstream credential can see no more than the agents' own key could. +PUBLIC_READ_SCOPE = BugzillaScope( + grants=( + { + "tier": "full", + "anchor": {}, + "endpoints": ["bug", "bug/*/comment"], + }, + ), +) + +PUBLIC_READ_WITH_ATTACHMENTS = BugzillaScope( + grants=( + { + "tier": "full", + "anchor": {}, + "endpoints": [ + "bug", + "bug/*/comment", + "bug/*/attachment", + "bug/attachment/*", + ], + }, + ), + attachments=True, +) + +__all__ = [ + "BugzillaScope", + "PUBLIC_READ_SCOPE", + "PUBLIC_READ_WITH_ATTACHMENTS", + "broker_env_for", + "is_configured", + "mint", +] diff --git a/services/hackbot-api/app/config.py b/services/hackbot-api/app/config.py index e812f42ad1..adc8e7899d 100644 --- a/services/hackbot-api/app/config.py +++ b/services/hackbot-api/app/config.py @@ -42,6 +42,21 @@ class Settings(BaseSettings): # API auth external_api_key: str = "" + # Per-run Bugzilla capability tokens (see app/bz_token.py). IAM signs as + # `bz_token_service_account`, this service's own identity, and the proxy + # verifies against the certificates Google publishes for it, so no key + # material is exchanged. Set the proxy's `token_issuer` to the same value. + # `bz_token_private_key` is a PEM standing in for local runs; with neither + # set, no token is minted and brokers keep their own Bugzilla credential. + # + # The audience and local issuer are constants in bz_token.py, not settings: + # an audience pointing at Google would mint an impersonation credential. + bz_token_service_account: str = "" + bz_token_private_key: str = "" + # How far past the job timeout the token stays valid. A run that outlives + # its token fails in a way that looks nothing like the cause. + bz_token_grace_seconds: int = 15 * 60 + phabricator: PhabricatorSettings # Inbound webhook receiver config, embedded as a nested model and populated diff --git a/services/hackbot-api/app/jobs.py b/services/hackbot-api/app/jobs.py index c2809a0934..2b3d5b640a 100644 --- a/services/hackbot-api/app/jobs.py +++ b/services/hackbot-api/app/jobs.py @@ -35,21 +35,47 @@ def _job_resource_name(job_name: str) -> str: _AGENT_CONTAINER_NAME = "agent" +_BROKER_CONTAINER_NAME = "broker" +# The only variables settable on the credentialed `broker` container. What +# keeps a run's inputs out of it is that everything else targets `agent`, and +# this allowlist is what keeps that true as new overrides get added. +_BROKER_ENV_ALLOWLIST = frozenset({"BUGZILLA_SCOPE_TOKEN"}) + + +def _container_override( + name: str, env: dict[str, str] +) -> run_v2.RunJobRequest.Overrides.ContainerOverride: + return run_v2.RunJobRequest.Overrides.ContainerOverride( + name=name, + env=[run_v2.EnvVar(name=k, value=v) for k, v in env.items()], + ) + + +def _trigger_sync( + job_name: str, + env_overrides: dict[str, str], + broker_env: dict[str, str] | None = None, +) -> str: + # Each agent's Job declares two containers: `agent` (no tokens) and + # `broker` (holds tokens, configured at deploy time). Per-execution + # overrides target `agent` so the broker's Secret Manager-backed env is + # untouched. `broker_env` is the exception, carrying the run's capability + # token, and is allowlisted by name above. + broker_env = broker_env or {} + disallowed = set(broker_env) - _BROKER_ENV_ALLOWLIST + if disallowed: + raise ValueError( + f"refusing to set {sorted(disallowed)} on the broker container; " + f"only {sorted(_BROKER_ENV_ALLOWLIST)} may be overridden there" + ) + + containers = [_container_override(_AGENT_CONTAINER_NAME, env_overrides)] + if broker_env: + containers.append(_container_override(_BROKER_CONTAINER_NAME, broker_env)) -def _trigger_sync(job_name: str, env_overrides: dict[str, str]) -> str: - # Each agent's Job manifest declares two containers: `agent` (no - # tokens) and `broker` (holds tokens, fully configured at deploy - # time). Per-execution env overrides target only the agent - # container by name so the broker's env (Secret Manager-backed) is - # untouched. overrides = run_v2.RunJobRequest.Overrides( - container_overrides=[ - run_v2.RunJobRequest.Overrides.ContainerOverride( - name=_AGENT_CONTAINER_NAME, - env=[run_v2.EnvVar(name=k, value=v) for k, v in env_overrides.items()], - ) - ], + container_overrides=containers, timeout={"seconds": settings.job_execution_timeout_seconds}, task_count=1, ) @@ -61,8 +87,12 @@ def _trigger_sync(job_name: str, env_overrides: dict[str, str]) -> str: return operation.metadata.name -async def trigger_execution(job_name: str, env_overrides: dict[str, str]) -> str: - return await asyncio.to_thread(_trigger_sync, job_name, env_overrides) +async def trigger_execution( + job_name: str, + env_overrides: dict[str, str], + broker_env: dict[str, str] | None = None, +) -> str: + return await asyncio.to_thread(_trigger_sync, job_name, env_overrides, broker_env) def _execution_status_sync(execution_name: str) -> ExecutionStatus: diff --git a/services/hackbot-api/app/routers/runs.py b/services/hackbot-api/app/routers/runs.py index 661b2d859f..ace5f9aeec 100644 --- a/services/hackbot-api/app/routers/runs.py +++ b/services/hackbot-api/app/routers/runs.py @@ -9,7 +9,7 @@ from sqlalchemy import select from sqlalchemy.ext.asyncio import AsyncSession -from app import gcs, jobs, pubsub +from app import bz_token, gcs, jobs, pubsub from app.actions_applier import apply_all_pending from app.agents import AGENT_REGISTRY, AgentSpec, model_to_env from app.auth import require_api_key @@ -108,8 +108,21 @@ async def create_run( **(agent.build_env or model_to_env)(inputs), } + # The broker's per-run environment, carrying this run's Bugzilla capability + # token when the agent has a registered scope. Built from the registry, not + # from `payload`, so the caller has no say in what the run may read. + broker_env = bz_token.broker_env_for( + run_id=str(run_id), + agent=agent.name, + scope=agent.bugzilla_scope, + inputs=inputs, + requested_by=on_behalf_of, + ) + try: - execution_name = await jobs.trigger_execution(agent.job_name, env_overrides) + execution_name = await jobs.trigger_execution( + agent.job_name, env_overrides, broker_env + ) except Exception as exc: log.exception("Failed to trigger Cloud Run Job for run %s", run_id) run.status = RunStatus.failed.value diff --git a/services/hackbot-api/pyproject.toml b/services/hackbot-api/pyproject.toml index fe79eeee31..0468dbc687 100644 --- a/services/hackbot-api/pyproject.toml +++ b/services/hackbot-api/pyproject.toml @@ -15,7 +15,9 @@ dependencies = [ "google-cloud-storage>=2.16.0", "google-cloud-run>=0.10.0", "google-cloud-pubsub>=2.21.0", + "google-cloud-iam>=2.15.0", "google-auth>=2.29.0", + "cryptography>=42.0.0", "sentry-sdk>=2.51.0", "cachetools>=5.3.0", "httpx>=0.26.0", diff --git a/services/hackbot-api/tests/test_bz_token.py b/services/hackbot-api/tests/test_bz_token.py new file mode 100644 index 0000000000..e306f7ab6a --- /dev/null +++ b/services/hackbot-api/tests/test_bz_token.py @@ -0,0 +1,503 @@ +"""Minting per-run Bugzilla capability tokens. + +The token is the whole authorization story for a run's Bugzilla access, so +these tests care most about what must *not* be possible: a caller influencing +the scope, a token outliving its run, or a token reaching a container that +should not have one. +""" + +import base64 +import json + +import pytest +from app import bz_token, jobs +from app.bz_token import PUBLIC_READ_SCOPE, BugzillaScope +from app.config import settings +from cryptography.hazmat.primitives import serialization +from cryptography.hazmat.primitives.asymmetric import rsa + + +@pytest.fixture +def signing_key(monkeypatch) -> str: + key = rsa.generate_private_key(public_exponent=65537, key_size=2048) + pem = key.private_bytes( + encoding=serialization.Encoding.PEM, + format=serialization.PrivateFormat.PKCS8, + encryption_algorithm=serialization.NoEncryption(), + ).decode() + monkeypatch.setattr(settings, "bz_token_private_key", pem, raising=False) + monkeypatch.setattr(settings, "bz_token_service_account", "", raising=False) + return pem + + +class Inputs: + def __init__(self, bug_id: int | None = 1899123) -> None: + self.bug_id = bug_id + + +def decode(token: str) -> dict: + payload = token.split(".")[1] + padded = payload + "=" * (-len(payload) % 4) + return json.loads(base64.urlsafe_b64decode(padded)) + + +class TestMint: + def test_claims_identify_the_run_and_the_requester(self, signing_key): + token = bz_token.mint( + run_id="abc", + agent="frontend-triage", + scope=PUBLIC_READ_SCOPE, + inputs=Inputs(), + requested_by="someone@mozilla.com", + ) + claims = decode(token) + assert claims["sub"] == "run:abc" + assert claims["aud"] == bz_token.TOKEN_AUDIENCE + assert claims["iss"] == bz_token.LOCAL_ISSUER + assert claims["agent"] == "frontend-triage" + assert claims["requested_by"] == "someone@mozilla.com" + + def test_the_token_outlives_the_job(self, signing_key): + claims = decode( + bz_token.mint( + run_id="abc", + agent="a", + scope=PUBLIC_READ_SCOPE, + inputs=Inputs(), + requested_by=None, + ) + ) + lifetime = claims["exp"] - claims["iat"] + assert lifetime == ( + settings.job_execution_timeout_seconds + settings.bz_token_grace_seconds + ) + + def test_every_token_gets_its_own_id(self, signing_key): + def jti() -> str: + return decode( + bz_token.mint( + run_id="abc", + agent="a", + scope=PUBLIC_READ_SCOPE, + inputs=Inputs(), + requested_by=None, + ) + )["jti"] + + assert jti() != jti() + + def test_the_default_scope_is_read_only_and_public(self, signing_key): + bz = decode( + bz_token.mint( + run_id="abc", + agent="a", + scope=PUBLIC_READ_SCOPE, + inputs=Inputs(), + requested_by=None, + ) + )["bz"] + assert bz["read_only"] is True + assert bz["confidential"] is False + assert bz["attachments"] is False + assert all(not g["anchor"].get("groups") for g in bz["grants"]) + + def test_the_signature_verifies_against_the_public_key(self, signing_key): + """RS256 over `header.payload`, which is what bugzilla-proxy checks.""" + from cryptography.hazmat.primitives import hashes + from cryptography.hazmat.primitives.asymmetric import padding + + token = bz_token.mint( + run_id="abc", + agent="a", + scope=PUBLIC_READ_SCOPE, + inputs=Inputs(), + requested_by=None, + ) + header_b64, payload_b64, signature_b64 = token.split(".") + signing_input = f"{header_b64}.{payload_b64}".encode() + signature = base64.urlsafe_b64decode( + signature_b64 + "=" * (-len(signature_b64) % 4) + ) + public_key = serialization.load_pem_private_key( + signing_key.encode(), password=None + ).public_key() + + # Raises InvalidSignature if the token was tampered with. + public_key.verify(signature, signing_input, padding.PKCS1v15(), hashes.SHA256()) + + header = json.loads( + base64.urlsafe_b64decode(header_b64 + "=" * (-len(header_b64) % 4)) + ) + assert header == {"alg": "RS256", "typ": "JWT"} + + def test_a_tampered_payload_fails_verification(self, signing_key): + from cryptography.exceptions import InvalidSignature + from cryptography.hazmat.primitives import hashes + from cryptography.hazmat.primitives.asymmetric import padding + + token = bz_token.mint( + run_id="abc", + agent="a", + scope=PUBLIC_READ_SCOPE, + inputs=Inputs(), + requested_by=None, + ) + header_b64, payload_b64, signature_b64 = token.split(".") + claims = decode(token) + claims["bz"]["grants"][0]["anchor"]["groups"] = ["core-security"] + forged_payload = ( + base64.urlsafe_b64encode(json.dumps(claims).encode()).rstrip(b"=").decode() + ) + signature = base64.urlsafe_b64decode( + signature_b64 + "=" * (-len(signature_b64) % 4) + ) + public_key = serialization.load_pem_private_key( + signing_key.encode(), password=None + ).public_key() + + with pytest.raises(InvalidSignature): + public_key.verify( + signature, + f"{header_b64}.{forged_payload}".encode(), + padding.PKCS1v15(), + hashes.SHA256(), + ) + + def test_minting_without_a_key_is_an_error(self, monkeypatch): + monkeypatch.setattr(settings, "bz_token_private_key", "", raising=False) + monkeypatch.setattr(settings, "bz_token_service_account", "", raising=False) + with pytest.raises(RuntimeError, match="no signing key"): + bz_token.mint( + run_id="abc", + agent="a", + scope=PUBLIC_READ_SCOPE, + inputs=Inputs(), + requested_by=None, + ) + + +class TestScopeTemplates: + def test_the_run_bug_is_substituted_into_the_anchor(self, signing_key): + scope = BugzillaScope( + grants=( + { + "tier": "full", + "anchor": {"static_bugs": ["$bug_id"], "groups": ["core-security"]}, + "endpoints": ["bug"], + }, + ), + confidential=True, + ) + bz = decode( + bz_token.mint( + run_id="abc", + agent="a", + scope=scope, + inputs=Inputs(bug_id=1899123), + requested_by=None, + ) + )["bz"] + assert bz["grants"][0]["anchor"]["static_bugs"] == [1899123] + + def test_a_template_needing_a_bug_fails_loudly_without_one(self, signing_key): + """Silently emptying the list would deny everything; dropping it would allow everything.""" + scope = BugzillaScope( + grants=( + { + "tier": "full", + "anchor": {"static_bugs": ["$bug_id"]}, + "endpoints": ["bug"], + }, + ) + ) + with pytest.raises(ValueError, match=r"\$bug_id"): + bz_token.mint( + run_id="abc", + agent="a", + scope=scope, + inputs=Inputs(bug_id=None), + requested_by=None, + ) + + def test_the_template_is_not_mutated_between_runs(self, signing_key): + scope = BugzillaScope( + grants=( + { + "tier": "full", + "anchor": {"static_bugs": ["$bug_id"]}, + "endpoints": ["bug"], + }, + ) + ) + for bug_id in (1, 2): + bz = decode( + bz_token.mint( + run_id="abc", + agent="a", + scope=scope, + inputs=Inputs(bug_id=bug_id), + requested_by=None, + ) + )["bz"] + assert bz["grants"][0]["anchor"]["static_bugs"] == [bug_id] + assert scope.grants[0]["anchor"]["static_bugs"] == ["$bug_id"] + + +class TestBrokerEnv: + def test_an_agent_with_no_scope_gets_no_token(self, signing_key): + assert ( + bz_token.broker_env_for( + run_id="abc", + agent="a", + scope=None, + inputs=Inputs(), + requested_by=None, + ) + == {} + ) + + def test_an_unconfigured_deployment_falls_back_silently(self, monkeypatch): + monkeypatch.setattr(settings, "bz_token_private_key", "", raising=False) + monkeypatch.setattr(settings, "bz_token_service_account", "", raising=False) + assert ( + bz_token.broker_env_for( + run_id="abc", + agent="a", + scope=PUBLIC_READ_SCOPE, + inputs=Inputs(), + requested_by=None, + ) + == {} + ) + + def test_a_configured_deployment_emits_the_token(self, signing_key): + env = bz_token.broker_env_for( + run_id="abc", + agent="a", + scope=PUBLIC_READ_SCOPE, + inputs=Inputs(), + requested_by=None, + ) + assert set(env) == {"BUGZILLA_SCOPE_TOKEN"} + + +class TestBrokerOverrideAllowlist: + """The broker container is credentialed, so what reaches it is fenced.""" + + def test_the_token_is_allowed(self): + assert "BUGZILLA_SCOPE_TOKEN" in jobs._BROKER_ENV_ALLOWLIST + + def test_anything_else_is_refused(self, monkeypatch): + monkeypatch.setattr( + jobs, "_job_resource_name", lambda name: f"projects/p/jobs/{name}" + ) + with pytest.raises(ValueError, match="refusing to set"): + jobs._trigger_sync( + "hackbot-agent-x", + {"BUG_ID": "1"}, + {"BUGZILLA_API_KEY": "smuggled"}, + ) + + def test_no_broker_override_is_added_when_there_is_no_token(self, monkeypatch): + captured = {} + + class FakeClient: + def run_job(self, request): + captured["request"] = request + + class Operation: + metadata = type("M", (), {"name": "executions/1"})() + + return Operation() + + monkeypatch.setattr(jobs, "_jobs_client", lambda: FakeClient()) + monkeypatch.setattr( + jobs, "_job_resource_name", lambda name: f"projects/p/jobs/{name}" + ) + jobs._trigger_sync("hackbot-agent-x", {"BUG_ID": "1"}, {}) + names = [c.name for c in captured["request"].overrides.container_overrides] + assert names == ["agent"] + + def test_the_broker_override_targets_the_broker_container(self, monkeypatch): + captured = {} + + class FakeClient: + def run_job(self, request): + captured["request"] = request + + class Operation: + metadata = type("M", (), {"name": "executions/1"})() + + return Operation() + + monkeypatch.setattr(jobs, "_jobs_client", lambda: FakeClient()) + monkeypatch.setattr( + jobs, "_job_resource_name", lambda name: f"projects/p/jobs/{name}" + ) + jobs._trigger_sync( + "hackbot-agent-x", {"BUG_ID": "1"}, {"BUGZILLA_SCOPE_TOKEN": "tok"} + ) + overrides = { + c.name: {e.name: e.value for e in c.env} + for c in captured["request"].overrides.container_overrides + } + assert overrides["agent"] == {"BUG_ID": "1"} + assert overrides["broker"] == {"BUGZILLA_SCOPE_TOKEN": "tok"} + + +SERVICE_ACCOUNT = "hackbot-api@example-project.iam.gserviceaccount.com" + + +@pytest.fixture +def signing_account(monkeypatch): + """Deployed mode: sign as the service's own identity, no key material.""" + monkeypatch.setattr( + settings, "bz_token_service_account", SERVICE_ACCOUNT, raising=False + ) + monkeypatch.setattr(settings, "bz_token_private_key", "", raising=False) + return SERVICE_ACCOUNT + + +@pytest.fixture +def fake_iam(monkeypatch): + """Stand in for IAM Credentials, recording what we asked it to sign.""" + captured = {} + + class FakeResponse: + signed_jwt = "signed.by.iam" + + class FakeClient: + def sign_jwt(self, request): + captured["request"] = request + return FakeResponse() + + monkeypatch.setattr(bz_token, "_iam_client", lambda: FakeClient()) + return captured + + +class TestServiceAccountSigning: + """Signing as the service's own identity, so no keys are ever exchanged.""" + + def test_signing_is_delegated_to_iam(self, signing_account, fake_iam): + token = bz_token.mint( + run_id="abc", + agent="a", + scope=PUBLIC_READ_SCOPE, + inputs=Inputs(), + requested_by=None, + ) + assert token == "signed.by.iam" + assert ( + fake_iam["request"]["name"] + == f"projects/-/serviceAccounts/{SERVICE_ACCOUNT}" + ) + + def test_the_signed_payload_carries_the_scope(self, signing_account, fake_iam): + bz_token.mint( + run_id="abc", + agent="frontend-triage", + scope=PUBLIC_READ_SCOPE, + inputs=Inputs(), + requested_by="someone@mozilla.com", + ) + claims = json.loads(fake_iam["request"]["payload"]) + assert claims["sub"] == "run:abc" + assert claims["agent"] == "frontend-triage" + assert claims["bz"]["read_only"] is True + + def test_the_issuer_is_the_signing_account(self, signing_account, fake_iam): + """It is what tells the proxy whose published certificates to trust.""" + bz_token.mint( + run_id="abc", + agent="a", + scope=PUBLIC_READ_SCOPE, + inputs=Inputs(), + requested_by=None, + ) + claims = json.loads(fake_iam["request"]["payload"]) + assert claims["iss"] == SERVICE_ACCOUNT + + def test_a_configured_account_alone_is_enough_to_mint(self, signing_account): + """No private key set, yet minting is available.""" + assert bz_token.is_configured() + + +class TestLifetimeCeiling: + def test_a_lifetime_past_the_iam_ceiling_is_refused( + self, signing_account, fake_iam, monkeypatch + ): + """IAM will not sign past 12 hours; say so in terms of the job timeout.""" + monkeypatch.setattr( + settings, "job_execution_timeout_seconds", 13 * 60 * 60, raising=False + ) + with pytest.raises(RuntimeError, match="job_execution_timeout_seconds"): + bz_token.mint( + run_id="abc", + agent="a", + scope=PUBLIC_READ_SCOPE, + inputs=Inputs(), + requested_by=None, + ) + + def test_the_default_job_timeout_fits(self, signing_account, fake_iam): + lifetime = ( + settings.job_execution_timeout_seconds + settings.bz_token_grace_seconds + ) + assert lifetime <= bz_token.MAX_TOKEN_LIFETIME_SECONDS + + +class TestNotAGoogleCredential: + """Signed by hackbot-api's own service account key. + + Only the claims keep one from also being a Google credential for that + account, so these pin the claims that do the work. + """ + + def test_the_subject_is_the_run_not_the_signing_account( + self, signing_account, fake_iam + ): + """Direct JWT auth to a Google API requires sub == iss.""" + bz_token.mint( + run_id="abc", + agent="a", + scope=PUBLIC_READ_SCOPE, + inputs=Inputs(), + requested_by=None, + ) + claims = json.loads(fake_iam["request"]["payload"]) + assert claims["sub"] == "run:abc" + assert claims["sub"] != claims["iss"] + + def test_there_is_no_scope_claim(self, signing_account, fake_iam): + """The OAuth JWT-bearer exchange requires one to grant anything.""" + bz_token.mint( + run_id="abc", + agent="a", + scope=PUBLIC_READ_SCOPE, + inputs=Inputs(), + requested_by=None, + ) + assert "scope" not in json.loads(fake_iam["request"]["payload"]) + + def test_the_audience_is_not_a_google_endpoint(self, signing_account, fake_iam): + bz_token.mint( + run_id="abc", + agent="a", + scope=PUBLIC_READ_SCOPE, + inputs=Inputs(), + requested_by=None, + ) + aud = json.loads(fake_iam["request"]["payload"])["aud"] + assert aud == bz_token.TOKEN_AUDIENCE + assert not aud.split("://")[-1].split("/")[0].lower().endswith("googleapis.com") + + def test_the_audience_cannot_be_reconfigured(self): + """A constant, so no deploy can point it at Google. + + Were it a setting, `https://oauth2.googleapis.com/token` would turn a + Bugzilla read token into an impersonation credential for this service. + """ + assert bz_token.TOKEN_AUDIENCE == "hackbot-bugzilla-proxy" + assert not hasattr(settings, "bz_token_audience") + host = bz_token.TOKEN_AUDIENCE.split("://")[-1].split("/")[0].lower() + assert not host.endswith("googleapis.com") diff --git a/services/hackbot-api/tests/test_create_run_api.py b/services/hackbot-api/tests/test_create_run_api.py index d0084b73e8..9943ef9386 100644 --- a/services/hackbot-api/tests/test_create_run_api.py +++ b/services/hackbot-api/tests/test_create_run_api.py @@ -16,7 +16,7 @@ def _stub_gcp(monkeypatch): async def fake_policy(run_id): return {"url": "https://upload.example/", "fields": {"key": "v"}} - async def fake_trigger(job_name, env): + async def fake_trigger(job_name, env, broker_env=None): return "projects/p/locations/l/jobs/j/executions/e" monkeypatch.setattr(gcs, "run_prefix", lambda run_id: f"results/{run_id}/") diff --git a/uv.lock b/uv.lock index 152dbb598c..5524be0cd7 100644 --- a/uv.lock +++ b/uv.lock @@ -22,6 +22,7 @@ members = [ "bugbug", "bugbug-http-service", "bugbug-mcp", + "bugzilla-proxy", "hackbot-agent-autowebcompat-diagnosis", "hackbot-agent-autowebcompat-repro", "hackbot-agent-bug-fix", @@ -841,6 +842,46 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/50/e6/a99aa6ab1ae7842011ca6d50d159a5e5a500717d13fc2ae42f39131741e3/bugsy-0.12.0-py2.py3-none-any.whl", hash = "sha256:b5df82ff0e708cca73a517bfa80494a43b1b97267d5778a5cba62f7a9d04e9da", size = 27223, upload-time = "2020-07-13T18:47:23.232Z" }, ] +[[package]] +name = "bugzilla-proxy" +version = "0.1.0" +source = { editable = "services/bugzilla-proxy" } +dependencies = [ + { name = "cachetools" }, + { name = "fastapi" }, + { name = "httpx" }, + { name = "pydantic" }, + { name = "pydantic-settings" }, + { name = "pyjwt", extra = ["crypto"] }, + { name = "uvicorn", extra = ["standard"] }, +] + +[package.optional-dependencies] +dev = [ + { name = "bugsy" }, + { name = "cryptography" }, + { name = "pytest" }, + { name = "pytest-asyncio" }, + { name = "six" }, +] + +[package.metadata] +requires-dist = [ + { name = "bugsy", marker = "extra == 'dev'", specifier = ">=0.12" }, + { name = "cachetools", specifier = ">=5.3.0" }, + { name = "cryptography", marker = "extra == 'dev'", specifier = ">=42.0.0" }, + { name = "fastapi", specifier = ">=0.109.0" }, + { name = "httpx", specifier = ">=0.26.0" }, + { name = "pydantic", specifier = ">=2.6.0" }, + { name = "pydantic-settings", specifier = ">=2.1.0" }, + { name = "pyjwt", extras = ["crypto"], specifier = ">=2.8.0" }, + { name = "pytest", marker = "extra == 'dev'", specifier = ">=8.0.0" }, + { name = "pytest-asyncio", marker = "extra == 'dev'", specifier = ">=0.23.0" }, + { name = "six", marker = "extra == 'dev'", specifier = ">=1.16" }, + { name = "uvicorn", extras = ["standard"], specifier = ">=0.27.0" }, +] +provides-extras = ["dev"] + [[package]] name = "cachetools" version = "7.1.7" @@ -2093,6 +2134,23 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/5d/4f/37960d5255988b218c40c9b5a2b731013684294a50735d1dd1ab4894e531/google_cloud_core-2.6.1-py3-none-any.whl", hash = "sha256:2682a8a4474a32f56292fb4bca7fa7e4fb0b4af958f6abfe4bca8d195747fd45", size = 29393, upload-time = "2026-08-06T06:23:11.405Z" }, ] +[[package]] +name = "google-cloud-iam" +version = "2.24.1" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "google-api-core", extra = ["grpc"] }, + { name = "google-auth" }, + { name = "grpc-google-iam-v1" }, + { name = "grpcio" }, + { name = "proto-plus" }, + { name = "protobuf" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/16/d8/ebea2eb4423ee467a50ed7b91ab0a1a0e0749f869151598891456a117a8f/google_cloud_iam-2.24.1.tar.gz", hash = "sha256:2d7edfe48fd24a181f55473a77584c0c947cf52915fc7af835d9ae0408fccae0", size = 561397, upload-time = "2026-08-06T06:24:26.45Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/a4/98/11d466009ffdf7abc987d9db8d6a59bbbb2c08543fd9ee4d98514bafbdc4/google_cloud_iam-2.24.1-py3-none-any.whl", hash = "sha256:7122aca0f5e2aa5bac889ff8136629054e9eef761d14dc5111ac1d109f68052a", size = 515093, upload-time = "2026-08-06T06:23:18.659Z" }, +] + [[package]] name = "google-cloud-pubsub" version = "2.39.1" @@ -2708,8 +2766,10 @@ dependencies = [ { name = "asyncpg" }, { name = "cachetools" }, { name = "cloud-sql-python-connector", extra = ["asyncpg"] }, + { name = "cryptography" }, { name = "fastapi" }, { name = "google-auth" }, + { name = "google-cloud-iam" }, { name = "google-cloud-pubsub" }, { name = "google-cloud-run" }, { name = "google-cloud-storage" }, @@ -2736,8 +2796,10 @@ requires-dist = [ { name = "asyncpg", specifier = ">=0.29.0" }, { name = "cachetools", specifier = ">=5.3.0" }, { name = "cloud-sql-python-connector", extras = ["asyncpg"], specifier = ">=1.5.0" }, + { name = "cryptography", specifier = ">=42.0.0" }, { name = "fastapi", specifier = ">=0.109.0" }, { name = "google-auth", specifier = ">=2.29.0" }, + { name = "google-cloud-iam", specifier = ">=2.15.0" }, { name = "google-cloud-pubsub", specifier = ">=2.21.0" }, { name = "google-cloud-run", specifier = ">=0.10.0" }, { name = "google-cloud-storage", specifier = ">=2.16.0" }, @@ -4656,9 +4718,9 @@ resolution-markers = [ "python_full_version < '3.13' and platform_machine == 'x86_64' and sys_platform == 'darwin'", ] dependencies = [ - { name = "llvmlite", version = "0.36.0", source = { registry = "https://pypi.org/simple" } }, - { name = "numpy" }, - { name = "setuptools" }, + { name = "llvmlite", version = "0.36.0", source = { registry = "https://pypi.org/simple" }, marker = "platform_machine == 'x86_64' and sys_platform == 'darwin'" }, + { name = "numpy", marker = "platform_machine == 'x86_64' and sys_platform == 'darwin'" }, + { name = "setuptools", marker = "platform_machine == 'x86_64' and sys_platform == 'darwin'" }, ] sdist = { url = "https://files.pythonhosted.org/packages/e3/7d/3d61160836e49f40913741c464f119551c15ed371c1d91ea50308495b93b/numba-0.53.1.tar.gz", hash = "sha256:9cd4e5216acdc66c4e9dab2dfd22ddb5bef151185c070d4a3cd8e78638aff5b0", size = 2213956, upload-time = "2021-03-26T09:15:50.402Z" } @@ -4678,8 +4740,8 @@ resolution-markers = [ "(python_full_version < '3.13' and platform_machine != 'x86_64' and sys_platform == 'darwin') or (python_full_version < '3.13' and sys_platform != 'darwin' and sys_platform != 'emscripten' and sys_platform != 'win32')", ] dependencies = [ - { name = "llvmlite", version = "0.48.0", source = { registry = "https://pypi.org/simple" } }, - { name = "numpy" }, + { name = "llvmlite", version = "0.48.0", source = { registry = "https://pypi.org/simple" }, marker = "platform_machine != 'x86_64' or sys_platform != 'darwin'" }, + { name = "numpy", marker = "platform_machine != 'x86_64' or sys_platform != 'darwin'" }, ] sdist = { url = "https://files.pythonhosted.org/packages/ae/a0/570e3dc53e5602b49108f62a13e529f1eec8bfc7ef37d49c825924dcf546/numba-0.66.0.tar.gz", hash = "sha256:b900e63a0e26c05ea9a6d5a3a5a0a177cb64c5011887bf43edb8c3ed2c38d363", size = 2806181, upload-time = "2026-07-01T23:12:46.36Z" } wheels = [ @@ -5109,7 +5171,7 @@ name = "pexpect" version = "4.9.0" source = { registry = "https://pypi.org/simple" } dependencies = [ - { name = "ptyprocess" }, + { name = "ptyprocess", marker = "sys_platform != 'emscripten' and sys_platform != 'win32'" }, ] sdist = { url = "https://files.pythonhosted.org/packages/42/92/cc564bf6381ff43ce1f4d06852fc19a2f11d180f23dc32d9588bee2f149d/pexpect-4.9.0.tar.gz", hash = "sha256:ee7d41123f3c9911050ea2c2dac107568dc43b2d3b0c7557a33212c398ead30f", size = 166450, upload-time = "2023-11-25T09:07:26.339Z" } wheels = [ @@ -6697,8 +6759,8 @@ name = "secretstorage" version = "3.5.0" source = { registry = "https://pypi.org/simple" } dependencies = [ - { name = "cryptography" }, - { name = "jeepney" }, + { name = "cryptography", marker = "(platform_machine != 'x86_64' and sys_platform == 'darwin') or (sys_platform != 'darwin' and sys_platform != 'emscripten' and sys_platform != 'win32')" }, + { name = "jeepney", marker = "(platform_machine != 'x86_64' and sys_platform == 'darwin') or (sys_platform != 'darwin' and sys_platform != 'emscripten' and sys_platform != 'win32')" }, ] sdist = { url = "https://files.pythonhosted.org/packages/1c/03/e834bcd866f2f8a49a85eaff47340affa3bfa391ee9912a952a1faa68c7b/secretstorage-3.5.0.tar.gz", hash = "sha256:f04b8e4689cbce351744d5537bf6b1329c6fc68f91fa666f60a380edddcd11be", size = 19884, upload-time = "2025-11-23T19:02:53.191Z" } wheels = [