Skip to content

fix: Control Center iframe auth + false setup banner - #78

Merged
man4ish merged 4 commits into
mainfrom
fix/control-center-iframe-auth-and-setup-banner
Sep 4, 2026
Merged

fix: Control Center iframe auth + false setup banner#78
man4ish merged 4 commits into
mainfrom
fix/control-center-iframe-auth-and-setup-banner

Conversation

@man4ish

@man4ish man4ish commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator

Context

Tonight's investigation into webstudio.omnibioai.org's embedded Control Center panel found two independent, unrelated root causes. This PR fixes both, as two separate commits.

FIX 1 — nginx missing Authorization forwarding (5dfdd31)

/_svc/control's gated location correctly validated the request via auth_request (using $control_authorization, a cookie-fallback for iframe navigations, added Jul 29 in 1329fed5) — but never forwarded that resolved header on the actual proxied request to control-center:7070. It only forwarded it to the internal /auth/verify subrequest used for the gate check. control-center's own require_permission() then independently 401'd the header-less request with "Missing or malformed Authorization header". /_svc/toolserver already had the correct pattern (proxy_set_header Authorization $control_authorization; on its own proxy_pass, not just the gate) — /_svc/control was just missing the equivalent line.

Not related to tonight's PR #77/#78 — neither touched GET / (what the iframe loads), which stayed gated throughout both. The real chain: Jul 29 added the cookie fallback but only wired it into the gate subrequest (harmless then, since GET / had no backend auth yet) → Sept 1's 8705cbf (an unrelated control-center route audit) gated GET / for the first time, which is what first exposed this dormant gap.

Before → after (real access token from a real /auth/login call, not synthetic):

/_svc/control/ (with cookie):        401 {"detail":"Missing or malformed Authorization header"}  →  200, real HTML
/_svc/control/ (no cookie):          401 (unchanged — nginx's own @cc_unauthorized gate)
/_svc/control/docker/containers:                                                                    200 with cookie
/_svc/control/{knowledge-base,storage,cron/jobs,coverage/status}:                                    200 with cookie
/_svc/control/{health,summary,report,report/data} (public allowlist, byte-for-byte untouched):        200/401/401/200 — identical to before

Deployment note surfaced along the way: a plain nginx -s reload did not pick up the edit. nginx-router bind-mounts the config as a single file, pinned by inode at container-create time — an editor's atomic rename-on-save orphans that mount from any later host-side edit, permanently, regardless of how many times nginx reloads afterward. Required docker compose up -d --force-recreate nginx-router (this one container only) to actually apply the change. Confirmed via stat -c '%i' inode comparison before concluding this, not guessed.

FIX 2 — wire the existing web-mode config shim, remove the false "Setup required" banner (e6059d6)

App.jsx's header banner fires whenever config.settings.data_dir is unset — an Electron-only concept (the local Docker stack's data directory, read via window.api.loadConfig()). On web (no window.api), that whole branch was skipped, so config never left its hardcoded default — the banner showed unconditionally for every web visitor, nothing actually misconfigured.

src/ui/lib/web/webApi.js already had a loadConfig() built for exactly this, explicitly documented as unwired. Two changes were needed (wiring the call in alone would not have changed anything visible — its return shape is identical to the existing hardcoded default):

  1. Call webApi.loadConfig() when !isElectron() (existing helper, same one ServiceViewer.jsx already uses) — no first-run redirect for this path, unlike Electron's.
  2. Gate the banner on config.mode !== "beta" too — "beta" is this codebase's existing "no local Docker" signal (Launch.jsx/Services.jsx's isBeta), correctly covering both the web deployment and choosing "Beta" inside Electron itself.

Electron's own branch is untouched (confirmed by diff). Grepped src/ui/ for every config.settings reader — only these two call sites exist.

Verified in the actual deployed production bundle (rebuilt + redeployed web-ui), not just source:

o?.mode!=="beta"&&!o?.settings?.data_dir&&(...⚠ Setup required — configure data directory...)

config.mode defaults to "beta" on web both before and after this change, so the banner no longer renders.

Testing

  • npm run test:ui (full suite): 28 files, 198 tests, all pass — including tests/ui/app-shell.test.jsx's own first-run-banner test (Electron, mode: "local", no data_dir → banner still expected and still shown).
  • Both fixes verified live against the running omnibioai-studio stack (see curl output above), not just unit tests.

Not merging — opening for review, per the explicit ask.

🤖 Generated with Claude Code

man4ish and others added 4 commits September 3, 2026 21:54
packages/omnibioai-ui (Table/Card/Tabs component library):
- Add branch-coverage tests for Card's padding/actions-only header, Tabs'
  empty/no-match tab states, and Table's null-comparator and equal-value
  sort branches.
- Raise the branches threshold in vitest.config.ts from 90% to 95%, matching
  statements/lines/functions.
Result: 98.59% stmts / 98.19% branches / 100% funcs / 100% lines.

Root src/ui app (Electron + web UI):
- Add 23 new test files under tests/ui/ covering every page and component
  that was previously thin or untested: Jobs, Settings, Launch, Services,
  RoleManagement, Cloud/LLM/HPC, the App shell, GrafanaViewer, LicenseGate,
  OAuthLinkConfirm, Videos, Logs, Workbench, IdeServices, Wizard,
  ServiceViewer, BugReport, ErrorBoundary, Mode, Sidebar/MobileNav/
  UpdateBanner, and the web-build session/API/roles modules.
- Extend session.test.js and store.test.js with the branches their existing
  tests missed (password login, OAuth link/redirect edge cases, refresh
  failure paths, launchSystem failure, setConfig/setSystemStatus).
- Enforce a 95% threshold (statements/lines/functions/branches) in
  vitest.app.config.js, already wired to `npm run coverage:ui`.
- Remove the leftover scratch vitest.coverage.config.js — its include list
  duplicated vitest.app.config.js's existing test glob — and gitignore the
  generated coverage/ report directory.
Result: went from 47% to 99.01% stmts / 95.02% branches / 99.47% funcs /
99.47% lines.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012pAAjARYEX2VDvfGPJcbj2
Raise UI test coverage to 95%+ across the board (packages/omnibioai-ui and
the root src/ui app).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012pAAjARYEX2VDvfGPJcbj2
The gated /_svc/control location correctly validates the request via
auth_request /internal/auth/verify -- which resolves $control_authorization
from either the real Authorization header or, when absent (an iframe
navigation can't set custom headers), the omnibioai_access_token cookie
(map added in 1329fed, Jul 29, specifically for this iframe case). That
covers the *gate*, but the actual proxied request to control-center:7070
was never told to carry that resolved header -- it just forwarded
whatever the original request had, which for a cookie-only iframe
navigation is nothing. control-center's own require_permission()
dependency then independently 401s with "Missing or malformed
Authorization header", since it re-validates the JWT itself rather than
trusting nginx's gate alone.

/_svc/toolserver (a few lines below) already gets this right --
proxy_set_header Authorization $control_authorization; on its proxy_pass,
not just on the auth_request subrequest. /_svc/control was missing the
equivalent line.

This is not related to tonight's PR #77/#78 (the /report/status, /llms,
/report/data, /report/public-stats gating changes in control-center's
main.py) -- neither touched GET / (what the iframe actually loads),
which has stayed platform.manage_infra-gated throughout both. The real
chain: 1329fed (Jul 29) added the cookie fallback but only wired it into
the auth_request subrequest, not the main proxy_pass -- harmless at the
time, since GET / had no backend-side auth yet. 8705cbf (Sept 1, an
unrelated control-center route audit) gated GET / for the first time,
which is what first exposed this dormant gap as a visible failure.

Verified against the live nginx-router container with a real access
token from a real /auth/login call (not a synthetic JWT):
  - /_svc/control/ (the iframe's actual URL) with the cookie: 401 -> 200,
    real HTML instead of {"detail":"Missing or malformed Authorization
    header"}.
  - /_svc/control/docker/containers, /knowledge-base, /storage,
    /cron/jobs, /coverage/status (all gated, non-allowlist): 200 with the
    cookie.
  - No cookie at all: still 401 via nginx's own @cc_unauthorized --
    the gate itself is unaffected.
  - The public allowlist (health/summary/report/report/data) is
    byte-for-byte untouched and behaves identically to before (health:
    200, summary/report: 401 from control-center's own independent
    gating, report/data: 200 per tonight's PR #78) -- they never needed
    forwarding since they were never nginx-gated to begin with.

Applied via: dated backup (docker/nginx-router.conf.bak-2026-09-03-
pre-control-auth-forward, gitignored per existing convention, not
committed), syntax-tested in a throwaway container before touching the
real file, then against the live container's own nginx -t. A plain
`nginx -s reload` did NOT pick up the change -- discovered live that this
docker-compose service bind-mounts the single config *file* by inode,
and an editor's atomic rename-on-save orphans that bind mount from any
subsequent host-side edit regardless of how many times nginx reloads.
Required `docker compose up -d --force-recreate nginx-router` (this one
container only, ~2s of its own downtime, nothing else in the stack
touched) to re-resolve the mount against the current file.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VevfQdYwX27LkcGqfRAxQL
App.jsx's header shows "Setup required — configure data directory"
whenever config.settings.data_dir is unset. data_dir is an Electron-only
concept (the local Docker stack's data directory, read via
window.api.loadConfig() -- electron/preload.js). On the web/cloud
deployment (webstudio.omnibioai.org), window.api is never injected into
a plain browser tab, so that whole branch was skipped and `config` stayed
stuck at its hardcoded initial default forever -- data_dir structurally
can never be set there, so the banner showed unconditionally for every
web visitor, regardless of anything actually being misconfigured.

src/ui/lib/web/webApi.js already had a purpose-built loadConfig() for
exactly this case -- returns { mode: "beta", ..., settings: {} } with a
comment explaining there's nothing to load, since there's no local
Docker stack to configure when connecting to an already-running backend
-- but its own top-of-file comment says it was never wired into any
existing shared page, "out of scope for this isolation-only pass".

Two changes, both required -- wiring in the call alone would NOT have
fixed the visible banner, since webApi.loadConfig()'s return value
(settings: {}) is identical in shape to the hardcoded default already in
useState, so it changes nothing observable by itself:

1. When window.api is unavailable, detect web mode via the existing
   isElectron() helper (lib/session.js -- same helper ServiceViewer.jsx
   already uses for its own Electron/web branching) and call
   webApi.loadConfig(), setting config from its result. No first-run
   setStep(8) redirect for this path, unlike the Electron branch --
   data_dir isn't a first-run condition to redirect out of when it
   structurally doesn't apply.
2. Gate the banner itself on config.mode !== "beta", not just
   !data_dir -- "beta" is this codebase's own existing signal for "no
   local Docker stack" (already used identically by Launch.jsx's and
   Services.jsx's isBeta checks), and correctly applies whether beta
   mode is reached via the web deployment or via choosing "Beta" inside
   the Electron app itself -- both cases have no data_dir to configure,
   for the same reason.

Electron's own branch (window.api.loadConfig() present) is completely
untouched -- confirmed by diff. Existing local/hpc/cloud/hybrid-mode
Electron behavior, including tests/ui/app-shell.test.jsx's own
first-run-banner test (mode: "local", no data_dir -> banner still
expected), is unaffected: that test and the full suite (28 files, 198
tests) pass unchanged. Grepped the whole src/ui/ tree for other
config.settings readers -- only these two call sites exist anywhere.

Verified: rebuilt and redeployed the web-ui container
(`docker compose up -d --build web-ui`), confirmed in the actual served
production bundle (not just source) that the compiled condition is
`o?.mode!=="beta"&&!o?.settings?.data_dir&&(...Setup required...)` --
config.mode defaults to "beta" on web both before and after this change
and nothing there ever moves it off "beta", so the banner no longer
renders.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VevfQdYwX27LkcGqfRAxQL
@man4ish
man4ish merged commit c8786f4 into main Sep 4, 2026
6 of 7 checks passed
@man4ish
man4ish deleted the fix/control-center-iframe-auth-and-setup-banner branch September 4, 2026 05:11
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.

1 participant