Skip to content

fix(security): scope fences read the routed path, reject path-shifting Host headers (#3102) - #3108

Merged
vybe merged 1 commit into
devfrom
feature/3102-scope-path-fences
Sep 30, 2026
Merged

vybe merged 1 commit into
devfrom
feature/3102-scope-path-fences

Conversation

@obasilakis

@obasilakis obasilakis commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • The five scope fences in get_current_user (event loopback, portal_delegate, connector, ephemeral agent, ops) compare request.scope["path"], the path the router dispatched. request.url is rebuilt from the Host header, so a crafted Host could make request.url.path name an allowlisted route while a forbidden one ran.
  • New utils/host_header.HostHeaderGuard: a pure-ASGI middleware, registered outermost in main.py. It rejects a Host containing / ? # \ @, whitespace or control characters (HTTP 400, WebSocket close 1008). Plain names, IPv6 literals, ports and an empty or missing Host pass.
  • Every audit endpoint= label (~90 sites) records scope["path"].

Changes

  • src/backend/dependencies.py: the five fences.
  • src/backend/utils/host_header.py, src/backend/main.py: the guard.
  • src/backend/routers/**, client_portal/router.py, services/fleet_ops_service.py: audit labels (mechanical).
  • tests/unit/test_3102_host_header_fences.py, new:
    • drives each fence with a real Starlette Request and a crafted Host;
    • covers the guard over HTTP and WebSocket;
    • AST-checks that the guard is registered last;
    • AST-scans src/backend and fails on any .url.path read.
  • ~20 unit test fakes switched from url.path to scope["path"]. Those fakes are why no existing test caught this.
  • Docs:
    • docs/memory/architecture/security.md: fence rule and guard.
    • feature-flows/audit-trail.md: snippet updated.
    • Learnings fragment added.
    • CSO diff report added.

Test Plan

  • cd tests && pytest unit/test_3102_host_header_fences.py -v: 23 passed.
  • Regression check: with the old dependencies.py restored, all 5 fence tests fail.
  • Full unit suite: 19,441 passed, and every fake it surfaced is fixed. The remaining local failures also fail on untouched dev: IPv6/SSRF checks under Python 3.11, a file-type check, a missing opentelemetry module, and the tests.unit import path.
  • /cso --diff: no findings. A fresh-context verifier tried duplicate Host headers, percent-encoding, absolute-form request targets, a missing Host, non-latin-1 bytes, middleware order and WebSocket; every check held.

Out of scope

  • Starlette/FastAPI bump.
  • Absolute URLs echoed from a plain-letter Host (a2a.py, paid.py, schedules.py, settings/mcp_url.py), a separate Host-injection class.
  • --forwarded-allow-ips=* in the backend Dockerfile.
  • The private submodule's two audit-label reads. It has no fences, and the guard covers it at runtime.

Fixes #3102

🤖 Generated with Claude Code

…g Host headers (#3102)

Starlette rebuilds request.url from the Host header, so a Host carrying
"/", "?" or "#" made request.url.path name an allowlisted route while the
router dispatched a forbidden one. The event-loopback, portal_delegate,
connector, ephemeral and ops fences in get_current_user now compare
request.scope["path"], and every audit endpoint= label records it too.

Defense in depth: utils/host_header.HostHeaderGuard, a pure-ASGI middleware
registered outermost, rejects a Host containing / ? # \ @, whitespace or
control characters (HTTP 400, WebSocket close 1008).

test_3102_host_header_fences drives each fence with a real Starlette
request and a crafted Host, covers the guard over HTTP and WebSocket, and
fails on any .url.path read under src/backend. Unit fakes that modelled
request.url.path now model scope["path"].

Fixes #3102

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@obasilakis obasilakis added the ui PR touches the frontend UI — triggers Playwright e2e tests label Sep 30, 2026

@vybe vybe left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merge-train: batch validated on train/20260930-1240 (#3118 green on Tier 1 + Tier 2)

@vybe
vybe merged commit 3dfad0e into dev Sep 30, 2026
28 checks passed
vybe pushed a commit that referenced this pull request Sep 30, 2026
…url.path (#3085)

test_3102_host_header_fences (#3108, now on dev) forbids request.url.path in
backend code: request.url is rebuilt from the Host header. The two ent#720
audit calls in routers/users.py predate the guard.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
vybe pushed a commit that referenced this pull request Sep 30, 2026
…e (ent#720) (#3085)

* fix(auth): a sign-in email is proven, unique, and written in one place (Abilityai/trinity-enterprise#720)

Every sign-in path resolves the account by email alone, so whoever holds an
address on a users row holds that identity. These are the residual doors
after #711:

1. Mailbox proof to bind. POST /api/users/me/email/code sends a 6-digit code
   to the NEW address (3 per 10 min). PUT /api/users/me/email requires
   {email, code}. Codes carry a purpose (email_login_codes.purpose):
   `email_bind:<user id>` completes only that account's bind; a bind code
   never signs in and a sign-in code never binds. Both routes are
   interactive-only. The one no-proof bind is the #82 transition on an install
   that cannot deliver mail (provider `console`): an interactive admin, audited
   as email_bind_unverified. Anyone else there gets 409
   email_verification_unavailable. The onboarding step and the Settings card
   gain the code step (auth store `bindOwnEmail`).
2. Unique. idx_users_email_unique ON users(lower(email)) WHERE email IS NOT
   NULL, on both tracks (SQLite `ent720_email_identity`, Alembic
   `0081_ent720_email_identity`). Existing duplicates are resolved first: blank
   becomes NULL, and per address the earliest-created account keeps it while
   the rest become NULL, logged by username only.
3. One writer. create_user, update_user, the password upsert and email sign-in
   creation all write through `_insert_user` / `_update_user_row`. These refuse
   a held address (EmailInUseError → 409 email_in_use) and map a lost race on
   the index to the same refusal. A writer-census test enumerates them.
   get_user_by_email is case-insensitive.
4. A reclaimed username is not a 500. Email sign-in whose `username = email`
   is taken creates a suffixed username. Auth0 sign-in resolves the HOLDER of
   the address first and never hands it back to an account that re-bound away.
5. Redeemers honour suspension. Telegram and WhatsApp redemption, the MCP
   inline redeemer, email_has_agent_access and the per-message channel gate
   (open_access included) refuse a suspended account, through
   db.is_email_account_suspended.

Second factor on the channel redeemers is out of scope: they cannot present
a challenge. It is recorded in FR-4 as a follow-up.

Tests:
- test_ent720_email_binding (28 tests);
- suspended cases in the Telegram, WhatsApp and router suites;
- emailBindProof.spec.js (mounted).

Mutations: bind-without-proof, no unique index, no username suffix, and
suspension ignored each turn tests red. The related backend suites (67 files)
pass on seeds 12345 and 99999 (1571 passed). Frontend: 181 files and 3728
tests pass, and the build is OK.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(auth): tolerate a reduced users shape in the ent#720 migration; eviction-proof tests

The duplicate-email sweep read username/created_at unconditionally, which
crashed the #1160 concurrent-boot fixture's minimal users table. Read those
columns only when present. The unit tests now resolve EmailInUseError and
UserOperations from the module the live db singleton uses (another test may
re-import db.users), and stub alembic.op via monkeypatch.setitem so the
sys.modules lint ratchet stays at zero.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(auth): cap wrong guesses on the email bind code; eviction-proof test helper

PUT /api/users/me/email verified the bind code without counting failures,
so a caller could mint 3 live codes per window and guess without limit.
Reuse the sign-in OTP limiter (OTP_MAX_ATTEMPTS=5 per 10 min) under a
bind-scoped key `otp_attempts:bind:{user_id}:{email}` — never the bare
address, which would let wrong bind guesses lock the owner out of email
sign-in. Past the cap even the right code is refused (429
too_many_attempts); a successful bind clears the counter, as sign-in does.

_users_mod now reads the raising function's own globals, so the tests hold
after another suite evicts and re-imports db.users.

Refs Abilityai/trinity-enterprise#720

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* test(auth): bind-route tests load the real OTP limiter under any order

A sibling suite (test_telegram_login_gate) parks a bare `routers.auth` stub
in sys.modules at collection time. The bind route now reaches the OTP
limiter through that module, so the `api` fixture re-imports the real one
when a stub holds the slot and always backs its counter with an in-memory
Redis.

Refs Abilityai/trinity-enterprise#720

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* docs(feature-flows): the sign-in email bind flow (ent#720)

POST /api/users/me/email/code and PUT /api/users/me/email: code purpose,
console-provider admin bypass, the bind-scoped attempt cap, refusal codes,
and what the duplicate-resolving migration does.

Refs Abilityai/trinity-enterprise#720

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* test(2996): pin the console provider for the codeless session bind

ent#720 requires the mailed code for PUT /me/email, except an admin on a
console-provider install. #2996's session-path test binds without a code,
so it now states that exception explicitly.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* merge-train: declare idx_users_email_unique in db/tables.py (#3085)

schema.py and both migration tracks create the index; tables.py MetaData did
not, so #746 autogenerate would propose dropping it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(auth): bind-code requests never spend the address's sign-in allowance (#3085)

count_recent_code_requests had no purpose filter, so sign-in (routers/auth.py)
and the MCP code request counted bind rows: any signed-in user could request
bind codes for someone else's address three times per 10 minutes and keep the
owner's sign-in codes suppressed while flooding their inbox.

- Sign-in counters count sign-in codes only (purpose IS NULL), the same
  exact-match rule verify_login_code applies.
- The bind route limits its CALLER across every address
  (count_recent_codes_for_purpose('email_bind:<id>')), 3 per 10 minutes, so no
  account can spend another's allowance.

Tests drive the real route: another account's bind requests leave the sign-in
counter at 0; the cap is per caller across addresses; one account cannot spend
another's; sign-in codes do not spend a bind allowance.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* merge-train: audit endpoint reads request.scope['path'], not request.url.path (#3085)

test_3102_host_header_fences (#3108, now on dev) forbids request.url.path in
backend code: request.url is rebuilt from the Host header. The two ent#720
audit calls in routers/users.py predate the guard.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: trinity-ability <309458136+trinity-ability@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ui PR touches the frontend UI — triggers Playwright e2e tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants