Skip to content

fix(dashboard): move CORSMiddleware outermost so OPTIONS preflight bypasses auth - #59072

Open
liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:liuhao/cron-bugfix-59052-cors-preflight-auth-order
Open

fix(dashboard): move CORSMiddleware outermost so OPTIONS preflight bypasses auth#59072
liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:liuhao/cron-bugfix-59052-cors-preflight-auth-order

Conversation

@liuhao1024

Copy link
Copy Markdown
Contributor

What does this PR do?

Moves the CORSMiddleware registration in hermes_cli/web_server.py from before the @app.middleware("http") auth decorators to after them. In Starlette's onion model, the last-added middleware is the outermost (runs first). Previously, CORS was innermost — an OPTIONS preflight from a cross-origin SPA hit the auth middlewares first, which returned 401 before CORS could answer with the proper Access-Control-Allow-* headers.

Related Issue

Fixes #59052

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • hermes_cli/web_server.py — Moved app.add_middleware(CORSMiddleware, …) from line 289 (before @app.middleware("http") registrations) to after the last @app.middleware("http") decorator (_token_auth_seam), making CORS the outermost middleware. Added comment explaining the ordering rationale.
  • tests/hermes_cli/test_web_server_cors_preflight.py (new) — 5 regression tests verifying OPTIONS preflight returns CORS headers, allowed/disallowed origin handling, and same-origin requests are unaffected.

How to Test

  1. Run python -m pytest tests/hermes_cli/test_web_server_cors_preflight.py -v — all 5 tests should pass.
  2. Run python -m pytest tests/hermes_cli/ -v — all existing dashboard tests should still pass (auth tests verify 401 for unauthenticated non-OPTIONS requests is unchanged).
  3. Manual verification (optional): start hermes dashboard, then from a different localhost port:
    curl -i -X OPTIONS \
      -H 'Origin: http://127.0.0.1:8092' \
      -H 'Access-Control-Request-Method: GET' \
      -H 'Access-Control-Request-Headers: x-hermes-session-token' \
      http://127.0.0.1:9119/api/skills
    
    Expected: HTTP/1.1 200 OK with Access-Control-Allow-Origin: http://127.0.0.1:8092. Before this fix: HTTP/1.1 401 Unauthorized.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS 26.4.1

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

…passes auth

app.add_middleware(CORSMiddleware, ...) was registered before the
@app.middleware(http) auth decorators, making CORS the innermost
layer in Starlette's onion model.  A cross-origin OPTIONS preflight
hit auth first → 401, and the CORS headers were never sent.

Moving the add_middleware call after all @app.middleware registrations
makes CORS outermost, so preflights are answered before auth runs.

Fixes NousResearch#59052
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/cli CLI entry point, hermes_cli/, setup wizard comp/dashboard Web dashboard / control panel UI (dashboard/, landing) area/auth Authentication, OAuth, credential pools labels Jul 5, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused regression fix. Current main still registers CORSMiddleware at hermes_cli/web_server.py:299 before the HTTP middleware decorators, while _token_auth_seam documents at hermes_cli/web_server.py:599-604 that the last registration is outermost and runs first. The loopback token gate returns 401 for tokenless protected /api/ requests at hermes_cli/web_server.py:590-595, so the reported browser preflight path remains affected.

The proposed reorder directly makes CORS outermost without changing authentication of the actual follow-up request. The added tests cover allowed and disallowed preflights plus authenticated/same-origin GET behavior; current dashboard tests cover the token-gated GET path but do not cover this preflight regression.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/auth Authentication, OAuth, credential pools comp/cli CLI entry point, hermes_cli/, setup wizard comp/dashboard Web dashboard / control panel UI (dashboard/, landing) P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

OPTIONS preflight returns 401 on /api/* — dashboard auth middleware ordered before CORS

3 participants