Skip to content

fix(dashboard): read SPA gating from the mounted app, not the module global - #71150

Open
joelbrilliant wants to merge 1 commit into
NousResearch:mainfrom
joelbrilliant:fix/mount-spa-app-scope
Open

fix(dashboard): read SPA gating from the mounted app, not the module global#71150
joelbrilliant wants to merge 1 commit into
NousResearch:mainfrom
joelbrilliant:fix/mount-spa-app-scope

Conversation

@joelbrilliant

Copy link
Copy Markdown
Contributor

The problem

Every route in mount_spa(application) registers on the application parameter, but _serve_index decided the auth scheme from the module-level app:

gated = bool(getattr(app.state, "auth_required", False))   # module global

Production calls mount_spa(app), so the two are the same object and today's behaviour is correct. But the SPA's auth scheme was being read from an object it was not mounted on.

Why it matters

That flag decides whether the long-lived _SESSION_TOKEN is injected into index.html. Gated mode withholds it and the browser authenticates with a cookie session instead; ungated mode injects it — which is exactly the property that makes it safe to put the dashboard behind a reverse proxy.

So a second mount (an embedded host, a test harness, any future sub-app) inherits the global's gating rather than its own. A gated app whose global is ungated would emit the very token gated mode exists to withhold.

Found while writing a test for that no-token invariant: setting auth_required on the app under test had no effect, because the code was reading a different app.

The fix

Read from application — the app the SPA was actually mounted on.

One line. Behaviour-identical in production (single caller, mount_spa(app)), correct for every other mount.

Tests

Two, with the app and the global deliberately disagreeing, which is the only way to tell the seams apart:

  • gated app + ungated global → stays gated, no token in HTML (the security-relevant direction)
  • ungated app + gated global → stays ungated, token present

Both fail before the change and pass after. tests/hermes_cli/test_web_server.py: 508 passed.

@alt-glitch alt-glitch added type/security Security vulnerability or hardening P3 Low — cosmetic, nice to have comp/cli CLI entry point, hermes_cli/, setup wizard comp/dashboard Web dashboard / control panel UI (dashboard/, landing) area/auth Authentication, OAuth, credential pools sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data labels Jul 25, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused fix. Current main confirms the premise: mount_spa(application) registers SPA handlers on application (hermes_cli/web_server.py:15868, hermes_cli/web_server.py:15886), while _serve_index currently reads the gate from module-global app (hermes_cli/web_server.py:15818). That gate controls the token-injecting branch at hermes_cli/web_server.py:15820-15835.

The proposed application.state.auth_required read in 7ee4818f scopes the decision to the mounted app and the two opposite-state cases cover the relevant seam.

Automated hermes-sweeper review.

…global

Every route in mount_spa(application) registers on the "application"
parameter, but _serve_index decided the auth scheme from the module-level
"app". Production calls mount_spa(app), so the two are the same object and
behaviour is unchanged - but the SPA's auth scheme was being read from an
object it was not mounted on.

That matters because this flag decides whether the long-lived _SESSION_TOKEN
is injected into index.html. Gated mode withholds it and the browser uses a
cookie session instead; ungated mode injects it. A second mount (embedded
host, test harness, any future sub-app) inherits the global's gating, so a
gated app whose global is ungated would emit the very token gated mode
exists to withhold.

Tests pin both directions with the app and the global deliberately
disagreeing: a gated app stays gated when the global is not, and an ungated
app stays ungated when the global is. 508 passed in test_web_server.py.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@joelbrilliant
joelbrilliant force-pushed the fix/mount-spa-app-scope branch from 7ee4818 to 4e3a1d8 Compare July 30, 2026 11:34

Copy link
Copy Markdown
Contributor Author

Thanks for confirming the mounted-app seam. I’ve rebased the branch onto current main as 4e3a1d877 and retained the two opposite-state regressions. The focused web server suite passes 106/106 and Ruff is clean. The merge conflict is cleared and fresh CI is now running.

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) P3 Low — cosmetic, nice to have sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/security Security vulnerability or hardening

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants