fix: Control Center iframe auth + false setup banner - #78
Merged
Conversation
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 viaauth_request(using$control_authorization, a cookie-fallback for iframe navigations, added Jul 29 in1329fed5) — but never forwarded that resolved header on the actual proxied request tocontrol-center:7070. It only forwarded it to the internal/auth/verifysubrequest used for the gate check. control-center's ownrequire_permission()then independently 401'd the header-less request with"Missing or malformed Authorization header"./_svc/toolserveralready had the correct pattern (proxy_set_header Authorization $control_authorization;on its ownproxy_pass, not just the gate) —/_svc/controlwas 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, sinceGET /had no backend auth yet) → Sept 1's8705cbf(an unrelated control-center route audit) gatedGET /for the first time, which is what first exposed this dormant gap.Before → after (real access token from a real
/auth/logincall, not synthetic):Deployment note surfaced along the way: a plain
nginx -s reloaddid not pick up the edit.nginx-routerbind-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. Requireddocker compose up -d --force-recreate nginx-router(this one container only) to actually apply the change. Confirmed viastat -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 wheneverconfig.settings.data_diris unset — an Electron-only concept (the local Docker stack's data directory, read viawindow.api.loadConfig()). On web (nowindow.api), that whole branch was skipped, soconfignever left its hardcoded default — the banner showed unconditionally for every web visitor, nothing actually misconfigured.src/ui/lib/web/webApi.jsalready had aloadConfig()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):webApi.loadConfig()when!isElectron()(existing helper, same oneServiceViewer.jsxalready uses) — no first-run redirect for this path, unlike Electron's.config.mode !== "beta"too —"beta"is this codebase's existing "no local Docker" signal (Launch.jsx/Services.jsx'sisBeta), 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 everyconfig.settingsreader — only these two call sites exist.Verified in the actual deployed production bundle (rebuilt + redeployed
web-ui), not just source:config.modedefaults 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 — includingtests/ui/app-shell.test.jsx's own first-run-banner test (Electron,mode: "local", nodata_dir→ banner still expected and still shown).omnibioai-studiostack (see curl output above), not just unit tests.Not merging — opening for review, per the explicit ask.
🤖 Generated with Claude Code