Skip to content

Reject X-Org-Id header when it conflicts with path {org_id}#14390

Merged
chuckbutkus merged 1 commit into
openhands/x-org-id-header-validationfrom
openhands/path-org-vs-header-guard
May 12, 2026
Merged

Reject X-Org-Id header when it conflicts with path {org_id}#14390
chuckbutkus merged 1 commit into
openhands/x-org-id-header-validationfrom
openhands/path-org-vs-header-guard

Conversation

@chuckbutkus
Copy link
Copy Markdown
Collaborator

@chuckbutkus chuckbutkus commented May 12, 2026

Path-org endpoints (/api/organizations/{org_id}/...) pin the org via the URL: require_permission runs against the path org, and the handlers do not consult the effective-org resolver. That makes any X-Org-Id header on such a request redundant at best, contradictory at worst.

Before this change the conflict was silently ignored, which masked client-state bugs — most commonly a stale org selector in the frontend sending the previous org's id while the user navigates to a new org's page. The request would succeed against the path org, but logs and telemetry tagged with the header value would point at a different org. Hard to debug from either side.

This adds a small REJECT_X_ORG_ID_PATH_MISMATCH dependency in server/auth/org_context.py and attaches it to:

enterprise/server/routes/orgs.py (15 path-org routes)
GET /api/organizations/{org_id}
PATCH /api/organizations/{org_id}
DELETE /api/organizations/{org_id}
GET /api/organizations/{org_id}/me
GET /api/organizations/{org_id}/settings
PATCH /api/organizations/{org_id}/settings
GET /api/organizations/{org_id}/members
GET /api/organizations/{org_id}/members/count
GET /api/organizations/{org_id}/members/financial
DELETE /api/organizations/{org_id}/members/{user_id}
PATCH /api/organizations/{org_id}/members/{user_id}
POST /api/organizations/{org_id}/switch
GET /api/organizations/{org_id}/git-claims
POST /api/organizations/{org_id}/git-claims
DELETE /api/organizations/{org_id}/git-claims/{claim_id}

enterprise/server/routes/org_invitations.py
POST /api/organizations/{org_id}/members/invite
(attached at router level — every route under invitation_router
has {org_id} in its prefix)

Behavior:
header absent ............................. pass
header == path ............................ pass
header != path ............................ 400
header malformed (not a UUID) ............. 400
path org malformed ........................ pass (FastAPI 422s first)
dep attached to non-path-org route ........ no-op (defensive)

Intentionally NOT attached:

  • accept_router in org_invitations.py — accept-invitation routes derive the target org from the (signed) invitation token, not from the URL. Honoring X-Org-Id here would let a header override which invitation gets accepted, which is exactly the wrong direction.
  • /api/organizations (list / create), /llm, /app — no {org_id} in the path, so the dep would be a no-op anyway.
  • /api/service/* — service-to-service auth via X-Service-API-Key, no user identity, X-Org-Id is not part of that contract.

Behavior change to call out for reviewers:
Path-org routes today silently accept a malformed X-Org-Id header (because they never look at it). After this PR they 400. That's the desired behavior, but it could surface client bugs that were previously inert; worth keeping an eye on /api/organizations/* 4xx rates after deploy.

Tests:
enterprise/tests/unit/server/auth/test_reject_x_org_id_path_mismatch.py

  • no header: pass
  • matching header (incl. uppercase UUID): pass
  • empty-string header: documented as pass-or-400 (Starlette behavior)
  • conflicting header: 400 with both ids in detail
  • malformed header: 400 with 'not a valid UUID'
  • dep on non-path-org route: no-op, even with malformed header

Not run in this sandbox: full pytest requires Python 3.12; the env here is 3.13 where the openhands.app_server logger fails to import (aifc removed). Static checks (py_compile, ruff check, ruff format) all pass.

  • A human has tested these changes.

Why

Summary

Issue Number

How to Test

Video/Screenshots

Type

  • Bug fix
  • Feature
  • Refactor
  • Breaking change
  • Docs / chore

Notes


To run this PR locally, use the following command:

GUI with Docker:

docker run -it --rm   -p 3000:3000   -v /var/run/docker.sock:/var/run/docker.sock   --add-host host.docker.internal:host-gateway   --name openhands-app-9017332   docker.openhands.dev/openhands/openhands:9017332

Path-org endpoints (/api/organizations/{org_id}/...) pin the org via
the URL: require_permission runs against the path org, and the
handlers do not consult the effective-org resolver. That makes any
X-Org-Id header on such a request redundant at best, contradictory
at worst.

Before this change the conflict was silently ignored, which masked
client-state bugs — most commonly a stale org selector in the
frontend sending the previous org's id while the user navigates to
a new org's page. The request would succeed against the path org,
but logs and telemetry tagged with the header value would point at
a different org. Hard to debug from either side.

This adds a small REJECT_X_ORG_ID_PATH_MISMATCH dependency in
server/auth/org_context.py and attaches it to:

  enterprise/server/routes/orgs.py (15 path-org routes)
    GET    /api/organizations/{org_id}
    PATCH  /api/organizations/{org_id}
    DELETE /api/organizations/{org_id}
    GET    /api/organizations/{org_id}/me
    GET    /api/organizations/{org_id}/settings
    PATCH  /api/organizations/{org_id}/settings
    GET    /api/organizations/{org_id}/members
    GET    /api/organizations/{org_id}/members/count
    GET    /api/organizations/{org_id}/members/financial
    DELETE /api/organizations/{org_id}/members/{user_id}
    PATCH  /api/organizations/{org_id}/members/{user_id}
    POST   /api/organizations/{org_id}/switch
    GET    /api/organizations/{org_id}/git-claims
    POST   /api/organizations/{org_id}/git-claims
    DELETE /api/organizations/{org_id}/git-claims/{claim_id}

  enterprise/server/routes/org_invitations.py
    POST /api/organizations/{org_id}/members/invite
      (attached at router level — every route under invitation_router
       has {org_id} in its prefix)

Behavior:
  header absent ............................. pass
  header == path ............................ pass
  header != path ............................ 400
  header malformed (not a UUID) ............. 400
  path org malformed ........................ pass (FastAPI 422s first)
  dep attached to non-path-org route ........ no-op (defensive)

Intentionally NOT attached:
- accept_router in org_invitations.py — accept-invitation routes
  derive the target org from the (signed) invitation token, not from
  the URL. Honoring X-Org-Id here would let a header override which
  invitation gets accepted, which is exactly the wrong direction.
- /api/organizations (list / create), /llm, /app — no {org_id} in
  the path, so the dep would be a no-op anyway.
- /api/service/* — service-to-service auth via X-Service-API-Key,
  no user identity, X-Org-Id is not part of that contract.

Behavior change to call out for reviewers:
  Path-org routes today silently accept a malformed X-Org-Id header
  (because they never look at it). After this PR they 400. That's
  the desired behavior, but it could surface client bugs that were
  previously inert; worth keeping an eye on /api/organizations/*
  4xx rates after deploy.

Tests:
  enterprise/tests/unit/server/auth/test_reject_x_org_id_path_mismatch.py
  - no header: pass
  - matching header (incl. uppercase UUID): pass
  - empty-string header: documented as pass-or-400 (Starlette behavior)
  - conflicting header: 400 with both ids in detail
  - malformed header: 400 with 'not a valid UUID'
  - dep on non-path-org route: no-op, even with malformed header

Not run in this sandbox: full pytest requires Python 3.12; the env
here is 3.13 where the openhands.app_server logger fails to import
(aifc removed). Static checks (py_compile, ruff check, ruff format)
all pass.

Co-authored-by: openhands <openhands@all-hands.dev>
@github-actions
Copy link
Copy Markdown
Contributor

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  enterprise/server/auth
  org_context.py 78, 109-139
  enterprise/server/routes
  org_invitations.py
  orgs.py
Project Total  

This report was generated by python-coverage-comment-action

@chuckbutkus chuckbutkus marked this pull request as ready for review May 12, 2026 05:40
Copy link
Copy Markdown
Collaborator

@all-hands-bot all-hands-bot left a comment

Choose a reason for hiding this comment

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

🟢 Good taste - Elegant, well-tested solution that turns silent bugs into actionable errors.

[RISK ASSESSMENT]

  • [Overall PR] ⚠️ Risk Assessment: 🟡 MEDIUM

Behavior change: path-org routes will now 400 on malformed/conflicting X-Org-Id headers instead of silently ignoring them. The change is correct (header was never used by these routes), but could surface client bugs that were previously harmless. Comprehensive tests and clear error messages reduce risk. Monitor /api/organizations/* 4xx rates post-deploy as recommended.

VERDICT:
Worth merging: Clean implementation, excellent tests, well-documented trade-offs.

KEY INSIGHT:
Failing fast with clear errors (400 + both UUIDs in message) beats silent success with inconsistent telemetry.

@chuckbutkus chuckbutkus merged commit accb37a into openhands/x-org-id-header-validation May 12, 2026
21 checks passed
@chuckbutkus chuckbutkus deleted the openhands/path-org-vs-header-guard branch May 12, 2026 05:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants