Fix admin ticket validation format mismatch

This commit is contained in:
2026-09-06 20:32:00 +02:00
parent e6db5428eb
commit 71f588360a
4 changed files with 24 additions and 3 deletions
+2
View File
@@ -4,6 +4,8 @@ This file is working memory. Update it during active work and before handoff; do
## Development state ## Development state
- **8D.6 Open admin bug fix (2026-09-06), target retest pending:** Confirmed frontend/backend ticket mismatch: `openAdmin()` required 32 URL-safe characters (serial format), while admin issuance returns 64 lowercase hex characters. Valid POST responses therefore threw before WebSocket construction with the user's exact generic message. Fixed only the admin validator; split the previously shared serial/admin test fixture, assert exact admin query URL and reject malformed/serial-format tickets without closing serial. Correct-format fixture reproduced pre-fix failure; final production-renderer/CSP + 17 UI groups, cookie/admin endpoint integration, transport 19/tickets 12 and lifecycle 11 pass. These are separate frontend and backend harnesses, not real-browser/on-wire integration. CSP loader hash matches the reported `o6St...` policy; rejected UUID/index.js scripts' origin is unverified, not grounds to relax CSP. Final bounded `pio run` passed in **7.96 s**, **95,580 B RAM / 1,646,489 B flash**, unchanged sizes. Diff check passes. No assets, upload, commit, backend/admission changes or target pass. Short bug record in `docs/phase8d6_implementation.md`; parent independent review and user target retest pending.
- **8D.6 final re-review complete (2026-09-06):** Independent reviewer confirmed session-identity isolation and bounded fit-readiness fixes and their regressions, with no remaining actionable findings in the reviewed changes. Final build/test results below stand; no further source change or build for this documentation update. Browser/target validation and numeric reserves remain pending, not phase sign-off or M2 completion. Stop before 8D.7 until separately requested after the validation decision. - **8D.6 final re-review complete (2026-09-06):** Independent reviewer confirmed session-identity isolation and bounded fit-readiness fixes and their regressions, with no remaining actionable findings in the reviewed changes. Final build/test results below stand; no further source change or build for this documentation update. Browser/target validation and numeric reserves remain pending, not phase sign-off or M2 completion. Stop before 8D.7 until separately requested after the validation decision.
- **8D.6 independent review fixes implemented / host-tested / build-verified (2026-09-06), target pending:** Fixed both P2 findings without unrelated scope. Before session adoption, compare retained username/role/session-stable CSRF; mismatch hides old terminals, closes both routes/fences work and replaces the document at `/`, including live admin and logout paths. Pagehide keeps buffers hidden until same-session validation; unchanged identities retain scrollback. Fit caches bounds only after valid measurement/success, with three bounded readiness retries, teardown cancellation and generation fencing. Expanded UI harness passes **16 groups**; login eight groups, cookie/admin integration, parser 268 and store/serial reruns pass. Final finite `pio run` after all fixes: **21.35 s, 95,580 B RAM / 1,646,489 B flash**; review delta **0 / +1,376 B**, versus 8D.5 **0 / +9,216 B**, versus 8D.0 **+1,048 / +46,516 B**. `git diff --check` passes. See `docs/phase8d6_implementation.md` for exact regression cases and unchanged pending target checklist. Parent reviews fixes only; no target pass, reserve approval, sign-off or M2 completion. Existing worktree edits preserved; no server/protocol/assets/8D.7/upload/commit action. Supersedes initial 8D.6 counts/build below. - **8D.6 independent review fixes implemented / host-tested / build-verified (2026-09-06), target pending:** Fixed both P2 findings without unrelated scope. Before session adoption, compare retained username/role/session-stable CSRF; mismatch hides old terminals, closes both routes/fences work and replaces the document at `/`, including live admin and logout paths. Pagehide keeps buffers hidden until same-session validation; unchanged identities retain scrollback. Fit caches bounds only after valid measurement/success, with three bounded readiness retries, teardown cancellation and generation fencing. Expanded UI harness passes **16 groups**; login eight groups, cookie/admin integration, parser 268 and store/serial reruns pass. Final finite `pio run` after all fixes: **21.35 s, 95,580 B RAM / 1,646,489 B flash**; review delta **0 / +1,376 B**, versus 8D.5 **0 / +9,216 B**, versus 8D.0 **+1,048 / +46,516 B**. `git diff --check` passes. See `docs/phase8d6_implementation.md` for exact regression cases and unchanged pending target checklist. Parent reviews fixes only; no target pass, reserve approval, sign-off or M2 completion. Existing worktree edits preserved; no server/protocol/assets/8D.7/upload/commit action. Supersedes initial 8D.6 counts/build below.
+8
View File
@@ -6,6 +6,14 @@ Implemented, host-tested and build-verified on 2026-09-06. **Target/browser vali
The starting worktree was clean. Production changes are confined to authored `src/web_ui.c`; focused tests extend `tests/web_ui_session/`. No server/protocol/settings/8D.7 policy change, generated asset regeneration, commit, upload or erase. The change fits the work-unit guideline without a preparatory split. The starting worktree was clean. Production changes are confined to authored `src/web_ui.c`; focused tests extend `tests/web_ui_session/`. No server/protocol/settings/8D.7 policy change, generated asset regeneration, commit, upload or erase. The change fits the work-unit guideline without a preparatory split.
## Open Admin Bug Follow-up (2026-09-06)
User reports working serial and mode selection, but Open admin displays `Admin connection failed. Open admin to retry.` Source diagnosis: `web_ui.c:openAdmin()` incorrectly validated a 32-character URL-safe serial ticket; `web_admin_tickets.c:web_admin_tickets_issue()` emits 32 random bytes as **64 lowercase hex characters**. A successful admin-ticket POST was rejected locally before `/ws/admin` construction/admission. No Origin, role, shared-console, fit, subprotocol or private-adapter change is needed for this demonstrated failure. Repeated attempts can leave the two unconsumed tickets occupied until their 30-second expiry.
Changed only the admin validator to the backend's emitted format. The UI harness had incorrectly shared the serial fixture with admin; separate 64-hex admin responses now exercise the exact query URL, plus malformed/serial-format rejection and serial isolation. The corrected fixture failed before the source fix (only the serial socket existed), then all **17 UI groups** passed. Production C renderer verifies the unchanged inline loader against the exact reported `sha256-o6St1XqFiWgZZKDDKYP8Y1ROJxvOnf96z55w4i/dC20=` CSP. The other reported UUID/index.js hashes are not that loader; their source is not established, and CSP was not relaxed.
Final focused reruns: `python3 tests/web_ui_session/run.py`, `python3 tests/web_cookie_auth/run.py --admin` (real store/ticket/endpoint-to-101 admission), `python3 tests/web_admin_transport/run.py --tickets` (19/12 groups), and `python3 tests/web_admin_transport/server_lifecycle.py` (11 groups) all pass. Frontend request behavior and backend admission were tested in separate harnesses, not an end-to-end browser/network session. Final bounded `pio run` passed in **7.96 s**, **95,580 B RAM / 1,646,489 B flash**, unchanged from the previous build. `git diff --check` passes. No assets, uploads, commits or target execution. Parent review and user target retest remain pending; this bug fix is not a new phase or sign-off. On next user-controlled deployment, reload the app and verify explicit Open admin reaches a prompt while serial remains connected; no ticket/cookie/CSRF values are needed in any report.
## Browser Contract ## Browser Contract
- Validated `/api/session` role reveals Serial/Admin selection only for admins. Ordinary users retain serial-only navigation; existing server authorization remains authoritative. - Validated `/api/session` role reveals Serial/Admin selection only for admins. Ordinary users retain serial-only navigation; existing server authorization remains authoritative.
+1 -1
View File
@@ -270,7 +270,7 @@ static const char s_app_js[] =
" adminDetail.textContent = 'Opening admin shell...'; updateControls();\n" " adminDetail.textContent = 'Opening admin shell...'; updateControls();\n"
" try {\n" " try {\n"
" const {payload} = await api('/api/admin/ws-ticket', work, {method: 'POST', signal: controller.signal, current});\n" " const {payload} = await api('/api/admin/ws-ticket', work, {method: 'POST', signal: controller.signal, current});\n"
" if (!payload || !/^[A-Za-z0-9_-]{32}$/.test(payload.ticket)) throw new Error('Invalid ticket');\n" " if (!payload || !/^[0-9a-f]{64}$/.test(payload.ticket)) throw new Error('Invalid ticket');\n"
" if (!live(work) || !current()) return;\n" " if (!live(work) || !current()) return;\n"
" const url = new URL('/ws/admin', window.location.origin); url.protocol = 'wss:';\n" " const url = new URL('/ws/admin', window.location.origin); url.protocol = 'wss:';\n"
" url.searchParams.set('ticket', payload.ticket); payload.ticket = '';\n" " url.searchParams.set('ticket', payload.ticket); payload.ticket = '';\n"
+13 -2
View File
@@ -55,7 +55,8 @@ function browser({onlyLoader = false, withLoader = false, role = 'user'} = {}) {
if (next !== undefined) return typeof next === 'function' ? next(options) : next; if (next !== undefined) return typeof next === 'function' ? next(options) : next;
if (url === '/api/session') return session({role}); if (url === '/api/session') return session({role});
if (url === '/api/status') return json({}); if (url === '/api/status') return json({});
if (url === '/api/ws-ticket' || url === '/api/admin/ws-ticket') return ticket(); if (url === '/api/ws-ticket') return ticket();
if (url === '/api/admin/ws-ticket') return json({ticket: '0123456789abcdef'.repeat(4), expires_in: 30});
throw new Error('network unavailable'); throw new Error('network unavailable');
}}); }});
if (withLoader || onlyLoader) vm.runInContext(loader, context); if (withLoader || onlyLoader) vm.runInContext(loader, context);
@@ -207,7 +208,7 @@ async function test(name, fn) { await fn(); ++passed; console.log('PASS JS:', na
const u = await connected(); assert.equal(u.nodes['terminal-selector'].hidden, true); const u = await connected(); assert.equal(u.nodes['terminal-selector'].hidden, true);
u.click('select-admin'); u.click('admin-toggle'); await tick(); assert.equal(u.terminals.length, 1); assert.equal(u.sockets.length, 1); u.click('select-admin'); u.click('admin-toggle'); await tick(); assert.equal(u.terminals.length, 1); assert.equal(u.sockets.length, 1);
const b = await adminBrowser(), [serial, admin] = b.sockets, [st, at] = b.terminals; const b = await adminBrowser(), [serial, admin] = b.sockets, [st, at] = b.terminals;
assert.match(admin.url, /\/ws\/admin\?ticket=/); assert.equal(admin.url, 'wss://sak.local/ws/admin?ticket=' + '0123456789abcdef'.repeat(4));
assert.equal(b.calls.find(c => c.url === '/api/admin/ws-ticket').headers['X-CSRF-Token'], token); assert.equal(b.calls.find(c => c.url === '/api/admin/ws-ticket').headers['X-CSRF-Token'], token);
st.input('WRONG'); at.input('x'.repeat(1025)); assert.equal(serial.sent.length, 0); st.input('WRONG'); at.input('x'.repeat(1025)); assert.equal(serial.sent.length, 0);
assert.deepEqual(admin.sent.map(x => x.length), [512, 512, 1]); assert.deepEqual(admin.sent.map(x => x.length), [512, 512, 1]);
@@ -224,6 +225,16 @@ async function test(name, fn) { await fn(); ++passed; console.log('PASS JS:', na
assert.ok(!serial.closed); assert.equal(b.nodes['admin-toggle'].textContent, 'Open admin'); assert.ok(!serial.closed); assert.equal(b.nodes['admin-toggle'].textContent, 'Open admin');
b.click('select-serial'); st.input('ok'); assert.deepEqual([...serial.sent.at(-1)], [111, 107]); b.click('select-serial'); st.input('ok'); assert.deepEqual([...serial.sent.at(-1)], [111, 107]);
}); });
await test('admin rejects serial-format and malformed tickets before upgrade without disturbing serial', async () => {
for (const value of ['t'.repeat(32), 'a'.repeat(63), 'a'.repeat(65), 'g'.repeat(64), null]) {
const b = browser({role: 'admin'}); b.start(); await tick();
const serial = b.sockets[0]; serial.emit('open');
b.queues['/api/admin/ws-ticket'].push(json({ticket: value}));
b.click('select-admin'); b.click('admin-toggle'); await tick();
assert.equal(b.sockets.length, 1); assert.ok(!serial.closed);
assert.equal(b.nodes['admin-detail'].textContent, 'Admin connection failed. Open admin to retry.');
}
});
await test('bounded hidden output continues draining, separate scrollback and input overflow closes only admin', async () => { await test('bounded hidden output continues draining, separate scrollback and input overflow closes only admin', async () => {
const b = await adminBrowser(), [serial, admin] = b.sockets, [st, at] = b.terminals; const b = await adminBrowser(), [serial, admin] = b.sockets, [st, at] = b.terminals;
assert.equal(st.options.scrollback, 5000); assert.equal(at.options.scrollback, 5000); assert.equal(st.options.scrollback, 5000); assert.equal(at.options.scrollback, 5000);