Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
120 changes: 120 additions & 0 deletions devlog/_plan/260905_admin_token_local_ux/000_research.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
# 000 — Research: why a plain local user is shown an admin token box

Two open issues describe the same wound from opposite ends.

- **#3483** (juzijia, Windows 10, 2.42.0) — the admin token dialog paints an
empty red error notice the moment it opens, before anything is submitted.
- **#3353** (Tao-Yida) — after upgrading to 2.40.0 a user was locked out of the
dashboard by a bare password box, assumed a config-loss bug, and had to have
an LLM read the source to learn the token was a new security feature.

The user's framing is stronger than either issue, and it is the one this unit
adopts: **a plain local user should never see that dialog at all.** When it does
appear it is a symptom, and the box asks the user to solve a problem they did
not cause and cannot diagnose.

## How the dashboard is supposed to authenticate

A loopback install never needs a typed credential. The server mints a session
and injects it into the served document:

```
GET /opencodex-session
-> src/server/index.ts:2074 issueGuiSession(...)
-> src/server/gui-session.ts:166
-> meta opencodex-session-token / -csrf / -origin / -server-origin
(src/server/gui-static.ts:71-74)
```

`gui/src/api.ts:loadInjectedSession()` reads those tags on boot. Verified live
against the running 2.43.0 proxy on port 10100:

```text
curl -i -H 'Host: 127.0.0.1:10100' http://127.0.0.1:10100/opencodex-session
HTTP/1.1 200 OK
<meta name="opencodex-session-token" content="ocx_session_CO-4g0m5B_...">
```

So on the happy path the prompt is unreachable. The interesting question is what
happens when that mint fails.

## The fallback that should not be a fallback

`gui/src/api.ts:resolveTokenAfter401()` (around line 247) handles a 401 like this:

```ts
const renewed = await Promise.race([reBootstrapSessionToken(plane), watchdog]);
if (renewed.kind === "minted") return renewed.token;
if (renewed.kind === "failed") return null;
const prompted = await requestAdminToken(token => verifyAdminToken(plane, token));
```

`reBootstrapSessionToken` maps **any 4xx** to `"unavailable"`:

```ts
if (!response.ok) return response.status >= 400 && response.status < 500
? { kind: "unavailable" } : { kind: "failed" };
```

And `"unavailable"` is precisely the branch that raises the password box.

Now read the mint conditions (`src/server/gui-session.ts:172-183`):

```ts
if (!isApiAuthRequired(config)) {
if (!isLoopbackHostname(host.hostname) || !isAllowedManagementOrigin(req, config)) return null;
...
}
```

with `isApiAuthRequired(config) === !isLoopbackHostname(config.hostname)`
(`src/server/auth-cors.ts:285`).

That yields the defect in one sentence: **on a loopback install the only ways to
get a 401 from the bootstrap are a Host or Origin mismatch — a misconfiguration
the admin token cannot fix.** Typing a token there is not a recovery path; it is
a dead end wearing a login form.

And when the bind genuinely is non-loopback, the token is real and required —
but the dialog explains none of that, which is exactly #3353.

## Why the notice is already red and empty (#3483)

`gui/src/admin-token-dialog.ts:76-79` builds the error element up front:

```ts
validationError.className = "notice notice-err";
validationError.hidden = true;
```

`hidden` works only because the UA stylesheet says `[hidden] { display: none }`,
and that rule is the weakest one in the cascade. `gui/src/styles.css:1307` then
says:

```css
.notice { ... display: flex; ... }
```

An author rule with an explicit `display` beats the UA `[hidden]` rule, so the
element stays laid out. It has `notice-err` borders and padding
(`styles.css:1355-1359`) and no text, which renders as the empty red box in the
screenshot. The bug is a CSS cascade defect, not a logic error — which is why no
existing test caught it: happy-dom asserts `hidden === true` happily while a real
browser paints the box.

## What this unit changes

1. Never prompt a standalone/loopback dashboard. Tell the user what is actually
wrong instead. (`010`)
2. When the prompt is legitimate, make it self-explanatory and link to a real
setup guide. Fix the empty notice while in there. (`020`)
3. Triage the Windows baseline and #3320 and land what is provable. (`030`)
4. Deliver as a stacked PR chain, admin-merged to `dev`. (`040`)

## Constraints carried from the request

- No repository-wide local suite on this workstation. Focused `bun test <file>`
plus `bun run typecheck`; heavy probes go to SSH hosts.
- A Windows baseline is already in flight on `desktop-c795oh4` under
`/c/ocxwin` (lock `/c/ocxwin/.suite.lock`, shards `base-1..4`). It is read-only
evidence for this unit and must not be disturbed.
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
# 010 — Never prompt a standalone dashboard for an admin token

Work-phase `wp1`. Depends on 000. Criterion `c-1`.

## Problem

`gui/src/api.ts:resolveTokenAfter401()` treats "the server would not mint me a
session" as "ask the human for a token". On a loopback install those are not the
same thing. `issueGuiSession` mints unconditionally for a loopback host with an
allowed origin (`src/server/gui-session.ts:172-183`), so a 401 there means the
request did not look loopback to the server — a Host/Origin/bind problem. No
token the user can type changes that verdict, because the token is not what was
refused.

## The signal

The server already states its topology on every served document:

```ts
// src/server/gui-static.ts:95
function runtimeRoleMeta(runtimeRole: string): string {
return \`<meta name="opencodex-runtime-role" content="\${escapeHtmlAttribute(runtimeRole)}">\`;
}
```

and the GUI already reads it (`gui/src/api-targets.ts:10-14`). That comment block
is explicit that a missing tag means "standalone / older server / Vite dev", i.e.
the safe default. This unit reuses that exact reader rather than inventing a
second topology signal.

The rule: **the admin-token prompt is for a deployment that actually requires a
typed credential.** That is the non-loopback bind, which is the `hub` role. Any
other role — `standalone`, `client`, or an absent tag — must not prompt.
Comment on lines +31 to +33

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Replace the rejected role-only authentication design

The shipped gui/src/api-targets.ts:51-60 consumes opencodex-management-auth-required; a non-loopback standalone must prompt, while a loopback hub can mint a session. 040_delivery_record.md:58-64 also identifies this role-only rule as the rejected first implementation. Presenting it here as the phase's rule, code, and test plan makes the new record internally contradictory and gives future maintainers the wrong authentication invariant; rewrite this section around bind metadata or clearly mark the design as superseded.

Useful? React with 👍 / 👎.


## Change

In `gui/src/api-targets.ts`, add a sibling to `isConnectedRuntime()`:

```ts
/**
* May this dashboard ask the user to type an admin token?
*
* Only a hub does. A standalone loopback install mints its own session
* (src/server/gui-session.ts), so a refusal there is a Host/Origin
* misconfiguration that no typed token can repair — prompting for one asks the
* user to answer a question they did not cause and cannot diagnose (#3353).
* A missing tag reads as standalone, matching runtimeRoleFromDocument's
* existing safe default.
*/
export function adminTokenPromptAllowed(): boolean {
return runtimeRoleFromDocument() === "hub";
}
```

In `gui/src/api.ts:resolveTokenAfter401()`, gate the prompt and record why it was
skipped:

```ts
if (renewed.kind === "failed") return null;
if (!adminTokenPromptAllowed()) {
state.promptCancelled = true; // do not re-ask on every subsequent 401
reportSessionUnavailable(plane); // surface an actionable notice instead
return null;
}
const prompted = await requestAdminToken(...);
```

`promptCancelled = true` matters: without it every failing request re-enters the
resolution path. The existing `storeSession` already resets that flag when a
session is later minted (`gui/src/api.ts:81`), so recovery is automatic once the
misconfiguration is fixed.

## What the user sees instead

A dismissible notice, not a form. Copy names the real cause and the real fix:

> **The dashboard could not start a session.** OpenCodex is running, but this
> page's address is not one it recognises as local. Open the dashboard at the
> address the proxy prints on startup (usually `http://127.0.0.1:<port>`), or see
> the dashboard access guide.

`reportSessionUnavailable` is a thin, testable seam: it dispatches a
`CustomEvent` the shell renders. It must not be a `alert()` and must not block.

## Verification

`bun test gui/tests/api-auth-deadline.test.ts` plus a new case:

- role `standalone` (and absent tag): after 401 + `unavailable` rebootstrap the
injected `adminTokenPrompt` spy is **not** called, the request resolves, and a
second failing request does not call it either.
- role `hub`: the spy **is** called (the legitimate path stays intact).

The role must be settable per test — the tests build their own `happy-dom`
document, so the case writes the meta tag before `installApiAuthFetch()`.

## Out of scope

No server change. `issueGuiSession`, `requireManagementAuth`, and the CORS
resolvers keep their current semantics; this phase only stops the GUI from
asking a question the server never wanted asked.
63 changes: 63 additions & 0 deletions devlog/_plan/260905_admin_token_local_ux/020_dialog_repair.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
# 020 — Repair the dialog itself

Work-phase `wp2`. Criteria `c-2`, `c-3`, `c-4`. Landed in #3491 and #3493.

Two defects in one surface, deliberately split across two PRs because they have
nothing to do with each other beyond sharing a file.

## #3483 — the empty red notice

`gui/src/admin-token-dialog.ts` builds the error element up front and hides it
with the `hidden` attribute. That works only because the UA stylesheet says
`[hidden] { display: none }`, and `gui/src/styles.css` overrode it:

```css
.notice { ... display: flex; ... }
```

Author origin beats user-agent origin. **Specificity never enters the
comparison** — which is why this looked like a validation false-positive and why
a `.notice[hidden]` fix would have been the wrong shape (it would still lose to
the three-class `.startup-runtime-notice` rule at `0,3,0`).

The repository had already solved this exact problem once, in
`gui/src/styles-combos-workspace.css`, where a bare `display: flex` left both
tab panels painted at once. That comment block is the precedent this fix
follows: move the display onto `:not([hidden])`.

Applied to `.notice`, `.notice-warn` (used without `.notice` in several places),
and `.notice.notice-warn.startup-runtime-notice`.

### Testing this required two tests, not one

happy-dom applies no author stylesheet and performs no layout, so
`expect(alert.hidden).toBe(true)` **passes today, unfixed**, while a real browser
paints the box. A DOM assertion cannot see this class of bug.

So the DOM test asserts the notice is hidden AND empty on open (keeping the two
halves of "no error" from drifting), and a second test reads `styles.css` and
fails any `.notice` rule that sets `display` without the guard. The second was
driven red by reverting the CSS before being accepted.

## #3353 — the box that explained nothing

The dialog's only text was a title naming an environment variable. A user who
had never set one had no way in.

Ground truth, verified in source rather than assumed:

- the proxy writes the token to `getConfigDir()/admin-api-token` on first start
(`src/lib/admin-secrets.ts`), `0600`, matching `/^ocx_admin_[A-Za-z0-9_-]{43}$/`
- `OPENCODEX_ADMIN_AUTH_TOKEN` replaces it entirely and is not regex-checked
- **no CLI prints it.** `ocx doctor` deliberately reports presence without ever
returning a value

That last point is worth stating in the docs explicitly rather than omitting:
hunting for `ocx token` is the obvious next move, and silence about it wastes
the user's time.

The dialog now carries a help paragraph plus a link to a new
"Finding the admin token" anchor, styled like the existing in-app docs links
(`target="_blank"`, `rel="noreferrer"`, accent colour) per
`gui/src/pages/dashboard-dialogs.tsx`. Copy landed in all nine locales;
`gui/tests/i18n-locales.test.ts` enforces key parity.
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
# 030 — Windows baseline: what the failures actually were

Work-phase `wp3`. Criterion `c-5`. Evidence, not a landed fix.

A full Windows suite was already running on `desktop-c795oh4` when this unit
started (`/c/ocxwin/repo` at `00834d710`, shards `base-1..4`). It was read-only
evidence for this unit and was not disturbed. A second run on Bun `1.4.0`
followed (`v140-1..4`).

## The headline

| run | Bun | fail |
|---|---|---:|
| `base-1..4` | 1.3.14 | 179 |
| `v140-1..4` | 1.4.0 | 25 |

Shard 1 alone went from 100 `(fail)` lines to 2. **The Windows suite was not
telling us about 179 product defects; it was mostly reporting one harness
failure over and over.**

## Why 161 of them were one bug

`tests/preload.ts` calls `acquireTestRunLock` (line 42) BEFORE it arms the guard
with `OCX_TEST_HOME_GUARD=1` (line 64). On Windows the lock path needs the
effective account SID, and under 4-shard load that lookup timed out:

```text
CodexUserIdentityRefusal: Windows effective-account lookup timed out.
at powershellValue (src/codex/user-identity.ts:252)
at resolveWindowsSid (src/codex/user-identity.ts:261)
at resolveDefaultTestRunLockPath (scripts/test-run-lock.ts:227)
at tests/preload.ts:42
```

The worker then ran with the guard permanently off, and every test that asserts
"this helper is only available under the repository test preload" failed as a
cascade: 44 in `codex-reset-credit-operation-ledger`, 68 in
`codex-reset-credit-recovery`, 49 in `lab-fabric-task`.

**That cascade had teeth.** With the guard down, two suites reached live
machine state instead of being refused:

- `windows-elevation-spawn.test.ts:89` expected `launcherPid: null` and got
`18144` — a real PowerShell process was launched.
- `service.test.ts:1283`/`:1307` expected "refusing to mutate the
machine-global Windows Task Scheduler from an armed test process" and instead
got real scheduler-registration outcomes.

So the ordering in `preload.ts` is not only noisy, it is the difference between
a refused test and one that touches the developer's own Task Scheduler. Worth
fixing on its own merits, independently of the Bun version that exposed it.

## What survives on Bun 1.4.0

25 failures in three groups:

- `multi-account auth store` — 22 of the 25, one file. Not yet diagnosed.
- `ocx v2 keep-native-v1` — 2. Dirac classified the `base` occurrence as a test
artifact: the product CLI exited 0 and the V2 disable took effect; only the
spy compares raw argv, and Windows `.cmd` invocation goes through the ComSpec
wrapper (`src/lib/win-exec.ts:79`), which is correct behaviour.
- `cli wiring > interactiveGuardOk ... when cwd is unlinked` — 1.

## Not fixed here, and why

This unit's authority is the admin-token UX. None of the surviving failures are
in that surface, and each needs its own reproduction on Windows before a fix is
more than a guess — the `base` run's evidence is contaminated by the guard
cascade, so a fix written against it would be written against an artifact.

The honest carry-forward is three separate units:

1. Arm `OCX_TEST_HOME_GUARD` before `acquireTestRunLock`, or make the SID lookup
fail closed instead of proceeding unguarded. Highest value: it is a safety
defect, not just a flake.
Comment on lines +73 to +75

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Remove the unfixed safety finding from tracked devlog

This publishes an explicit remediation for a defect that the document says is not fixed, after explaining how the failure leaves the test guard off and permits suites to reach the developer's live Task Scheduler. That is pre-disclosure bypass reasoning and a patch plan for an unfixed fail-open safety issue; keep these details in .tmp/ until the fix ships, then publish the closed outcome.

AGENTS.md reference: AGENTS.md:L103-L110

Useful? React with 👍 / 👎.

2. Diagnose `multi-account auth store` on Windows.
3. Decide whether `keep-native-v1` should assert on parsed argv rather than the
raw ComSpec string.

Issue #3320 (non-ASCII account names misclassifying a valid scheduler task) is
adjacent to (1) — both are Windows identity handling — but it is `needs-info`
and was not reproduced here, so it stays open.
Loading
Loading