mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-09-30 12:39:42 +02:00
fix(webview): recover a proxied dashboard that reloads on its landing page
The runtime shim masks `/webview/<cap>/` off a proxied page's URL so its router boots on the path it expects, and the landing page masks to exactly `/`. A `location.reload()` there (a Vite dev server on a config change or a failed HMR update, the likeliest case in the feature's own motivating scenario) therefore asks for Codeman's root as an iframe navigation. `serveLostWebviewFrame()` returned early for `/`, so on a passwordless install the frame received Codeman's own app shell and rendered it inside the web tab, and with a password it got a 401 in the frame. Either way no `codeman:webview-lost` message was posted, and because the document loaded fine the load handler cleared the failed-frame panel, so the Reload / Open in new tab affordances never appeared. Before masking the frame's URL was the prefixed one, so a reload worked; this was a regression. `/` is the one lost-frame path a registered route also serves, so the route table cannot tell that reload from a real navigation. Credentials can: nothing in Codeman frames its own root, and a sandboxed frame is opaque-origin with no cookie and no Authorization header. `carriesAuthCredentials()` (pure, in webview-proxy.ts) makes that test, and `/` is now admitted by the auth hook only when it fails; a framed `/` that does carry credentials still gets the shell. Without a password no auth hook runs at all, so the index route applies the same test itself (`isLostWebviewRootFrame`) before rendering the shell, and the three places that emitted the recovery page share `sendLostWebviewFramePage()`. Tests: the password form in webview-auth-exemption (recovery page for a credential-free framed `/`, shell with valid Basic auth, 401 with a stale cookie or a top-level navigation), the passwordless form against a real WebServer in webview-lost-root-frame (port 3198), and the credential predicate in webview-proxy. All three fail without the fix. Verified against a live isolated instance as well: a framed `GET /` with no credentials answers the 470-byte recovery page, a top-level `GET /` and a framed one carrying a cookie answer the shell. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
@@ -26,6 +26,7 @@ import { webviewCapabilities } from '../../webview-capabilities.js';
|
||||
import {
|
||||
capabilityFromProxyPath,
|
||||
capabilityFromReferer,
|
||||
carriesAuthCredentials,
|
||||
isLostWebviewFrameNavigation,
|
||||
lostWebviewFramePage,
|
||||
LOST_FRAME_PAGE_CSP,
|
||||
@@ -190,20 +191,55 @@ function hasValidWebviewCapability(req: FastifyRequest, basePath = ''): boolean
|
||||
* login challenge inside the tab nor counts as a failed attempt against the
|
||||
* caller's IP — a dev server that full-reloads on every save would otherwise
|
||||
* rate-limit its own user out of Codeman. Fenced like the Referer exemption: a
|
||||
* path that resolves to a real route (the app shell, /api, /q) is never answered
|
||||
* this way, so a genuine unauthenticated navigation still gets the 401.
|
||||
* path that resolves to a real route (/api, /q, a registered handler) is never
|
||||
* answered this way, so a genuine unauthenticated navigation still gets the 401.
|
||||
*
|
||||
* `/` is the one registered route that IS answered here, and only when the
|
||||
* request carries neither the session cookie nor an Authorization header. The
|
||||
* shim maps `/webview/<cap>/` to exactly `/`, so a dashboard that reloads on its
|
||||
* landing page (a Vite dev server on a config change) asks for Codeman's root
|
||||
* as an iframe navigation; answering that with the app shell put Codeman inside
|
||||
* its own web tab, and with a password it was a 401 in the frame. Nothing in
|
||||
* Codeman frames its own root and the sandboxed frame has no credentials, so the
|
||||
* credential-free form can only be that frame; a framed `/` WITH credentials is
|
||||
* still the shell. Property worth knowing: a non-browser client can set these
|
||||
* headers too, so an unauthenticated caller can tell a registered route (401)
|
||||
* from a non-route (200) and enumerate the route table. Accepted, because the
|
||||
* routes are public in docs/api-reference.md.
|
||||
*
|
||||
* @returns true when the reply was sent.
|
||||
*/
|
||||
function serveLostWebviewFrame(req: FastifyRequest, reply: FastifyReply): boolean {
|
||||
if (!isLostWebviewFrameNavigation(req)) return false;
|
||||
const url = (req.url ?? '').split('?')[0];
|
||||
if (url === '/' || url.startsWith('/api/') || url.startsWith('/ws/') || url.startsWith('/q/')) return false;
|
||||
if (matchesRegisteredRoute(req, url)) return false;
|
||||
if (url.startsWith('/api/') || url.startsWith('/ws/') || url.startsWith('/q/')) return false;
|
||||
if (url === '/') {
|
||||
if (carriesAuthCredentials(req.headers, AUTH_COOKIE_NAME)) return false;
|
||||
} else if (matchesRegisteredRoute(req, url)) {
|
||||
return false;
|
||||
}
|
||||
sendLostWebviewFramePage(reply);
|
||||
return true;
|
||||
}
|
||||
|
||||
/**
|
||||
* The landing-page case of serveLostWebviewFrame, for the index route. Without
|
||||
* CODEMAN_PASSWORD no auth hook runs at all, so a lost frame's reload of `/`
|
||||
* reaches `GET /` directly and the route asks this before rendering the shell.
|
||||
* Under a password the hook has already answered a credential-free lost frame,
|
||||
* so here it only ever sees the credentialed form, which stays the shell.
|
||||
*/
|
||||
export function isLostWebviewRootFrame(req: FastifyRequest): boolean {
|
||||
if (!isLostWebviewFrameNavigation(req)) return false;
|
||||
if ((req.url ?? '').split('?')[0] !== '/') return false;
|
||||
return !carriesAuthCredentials(req.headers, AUTH_COOKIE_NAME);
|
||||
}
|
||||
|
||||
/** Send the static recovery page (lostWebviewFramePage) with its own CSP, uncached. */
|
||||
export function sendLostWebviewFramePage(reply: FastifyReply): FastifyReply {
|
||||
reply.header('content-security-policy', LOST_FRAME_PAGE_CSP);
|
||||
reply.header('cache-control', 'no-store');
|
||||
reply.type('text/html; charset=utf-8').send(lostWebviewFramePage());
|
||||
return true;
|
||||
return reply.type('text/html; charset=utf-8').send(lostWebviewFramePage());
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
+17
-8
@@ -148,7 +148,13 @@ import { getLatestPlanUsage, setLatestCodexPlanUsage } from './plan-usage-latest
|
||||
import { telemetrySignature } from '../usage-telemetry.js';
|
||||
import { readCodexPlanUsage, resolveCodexBinaryPath } from '../utils/codex-cli-resolver.js';
|
||||
import type { ScheduledRun } from './ports/index.js';
|
||||
import { registerAuthMiddleware, registerSecurityHeaders, registerHostGuard } from './middleware/auth.js';
|
||||
import {
|
||||
registerAuthMiddleware,
|
||||
registerSecurityHeaders,
|
||||
registerHostGuard,
|
||||
isLostWebviewRootFrame,
|
||||
sendLostWebviewFramePage,
|
||||
} from './middleware/auth.js';
|
||||
import { isMultiUserMode } from '../config/multiuser.js';
|
||||
import { bootstrapInitialAdmin, hasUsers, resolveClaudeModeForUsername } from '../user-store.js';
|
||||
import { installRouteErrorHandler } from './route-error-handler.js';
|
||||
@@ -181,7 +187,7 @@ import {
|
||||
registerTabLayoutRoutes,
|
||||
tryWebviewRefererFallback,
|
||||
} from './routes/index.js';
|
||||
import { isLostWebviewFrameNavigation, lostWebviewFramePage, LOST_FRAME_PAGE_CSP } from './webview-proxy.js';
|
||||
import { isLostWebviewFrameNavigation } from './webview-proxy.js';
|
||||
import { CronService } from '../cron/cron-service.js';
|
||||
|
||||
const __dirname = dirname(fileURLToPath(import.meta.url));
|
||||
@@ -805,7 +811,14 @@ export class WebServer extends EventEmitter {
|
||||
|
||||
// Security headers + CORS
|
||||
registerSecurityHeaders(this.app, this.https, this.basePath);
|
||||
this.app.get('/', async (_req, reply) => {
|
||||
this.app.get('/', async (req, reply) => {
|
||||
// A web-tab frame that reloaded on its dashboard's landing page. The proxy's
|
||||
// runtime shim maps `/webview/<cap>/` to exactly `/`, so that reload asks for
|
||||
// Codeman's own root as an iframe navigation, and it used to get the app
|
||||
// shell rendered inside the web tab. Only the credential-free form is taken
|
||||
// (nothing in Codeman frames its root; the sandboxed frame has no cookie and
|
||||
// no Authorization); under a password the auth hook has answered it already.
|
||||
if (isLostWebviewRootFrame(req)) return sendLostWebviewFramePage(reply);
|
||||
return reply
|
||||
.header('Cache-Control', 'no-cache')
|
||||
.type('text/html; charset=utf-8')
|
||||
@@ -981,11 +994,7 @@ export class WebServer extends EventEmitter {
|
||||
// that navigated itself off its proxy prefix: the runtime shim masks the
|
||||
// prefix so the page's router sees its own path, and a reload of that page
|
||||
// lands here. The unauthenticated form is answered in the auth middleware.
|
||||
if (!req.url.startsWith('/api') && isLostWebviewFrameNavigation(req)) {
|
||||
reply.header('content-security-policy', LOST_FRAME_PAGE_CSP);
|
||||
reply.header('cache-control', 'no-store');
|
||||
return reply.type('text/html; charset=utf-8').send(lostWebviewFramePage());
|
||||
}
|
||||
if (!req.url.startsWith('/api') && isLostWebviewFrameNavigation(req)) return sendLostWebviewFramePage(reply);
|
||||
if (req.url.startsWith('/api')) {
|
||||
return reply.code(404).send(createErrorResponse(ApiErrorCode.NOT_FOUND, notFound));
|
||||
}
|
||||
|
||||
@@ -734,7 +734,9 @@ export const LOST_FRAME_PAGE_CSP = `default-src 'none'; script-src 'sha256-${LOS
|
||||
* no capability anywhere on it: no prefix in the path, no cookie in an
|
||||
* opaque-origin frame, and a Referer that names the masked page. Such a request
|
||||
* is recognisable by shape alone: a top-level navigation of an `<iframe>`
|
||||
* (`Sec-Fetch-Dest`), asking for HTML, for a path Codeman does not serve.
|
||||
* (`Sec-Fetch-Dest`), asking for HTML, for a path Codeman does not serve. The
|
||||
* one served path that still qualifies is `/` itself, which the callers admit
|
||||
* only when the request carries no credentials (see carriesAuthCredentials).
|
||||
*
|
||||
* The answer is `lostWebviewFramePage()`, a static page whose only content is a
|
||||
* `postMessage` to the parent naming the path; the Codeman tab that owns the
|
||||
@@ -754,6 +756,31 @@ export function isLostWebviewFrameNavigation(req: {
|
||||
return typeof accept === 'string' && accept.includes('text/html');
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether a request carries something Codeman's auth would recognise: the
|
||||
* session cookie, or an `Authorization` header (Basic auth, which a browser
|
||||
* re-sends on every request to the realm once it has been accepted).
|
||||
*
|
||||
* `/` is the one lost-frame path a registered route also serves (the app shell),
|
||||
* so the route table cannot tell a landing-page reload of a proxied dashboard
|
||||
* (the runtime shim maps `/webview/<cap>/` to exactly `/`) from a genuine
|
||||
* navigation. Credentials can: nothing in Codeman frames its own root, and a
|
||||
* sandboxed web-tab frame is opaque-origin and carries neither, so an `<iframe>`
|
||||
* navigation of `/` with NEITHER credential can only be that frame. A framed
|
||||
* `/` that does carry credentials is left to the shell.
|
||||
*/
|
||||
export function carriesAuthCredentials(
|
||||
headers: Record<string, string | string[] | undefined>,
|
||||
sessionCookieName: string
|
||||
): boolean {
|
||||
const authorization = headers.authorization;
|
||||
if (Array.isArray(authorization) ? authorization.length > 0 : (authorization ?? '').trim() !== '') return true;
|
||||
const cookie = headers.cookie;
|
||||
const cookies = Array.isArray(cookie) ? cookie.join('; ') : cookie;
|
||||
if (typeof cookies !== 'string' || cookies === '') return false;
|
||||
return cookies.split(';').some((part) => part.trim().startsWith(`${sessionCookieName}=`));
|
||||
}
|
||||
|
||||
/** The static page that hands a lost frame back to its owning tab. */
|
||||
export function lostWebviewFramePage(): string {
|
||||
return (
|
||||
|
||||
@@ -257,7 +257,7 @@ describe('a lost web-tab frame', () => {
|
||||
});
|
||||
|
||||
it('never for a path Codeman actually serves, and never for a plain navigation', async () => {
|
||||
expect((await app.inject({ method: 'GET', url: '/', headers: lostFrame })).statusCode).toBe(401);
|
||||
expect((await app.inject({ method: 'GET', url: '/webviewfoo/bar', headers: lostFrame })).statusCode).toBe(401);
|
||||
expect((await app.inject({ method: 'GET', url: '/api/sessions/abc', headers: lostFrame })).statusCode).toBe(401);
|
||||
expect((await app.inject({ method: 'GET', url: '/about' })).statusCode).toBe(401);
|
||||
expect(
|
||||
@@ -273,4 +273,49 @@ describe('a lost web-tab frame', () => {
|
||||
// A genuinely unauthenticated request afterwards is still a plain 401, not a 429.
|
||||
expect((await app.inject({ method: 'GET', url: '/static/app.js' })).statusCode).toBe(401);
|
||||
});
|
||||
|
||||
/**
|
||||
* The landing page. The shim maps `/webview/<cap>/` to exactly `/`, so a reload
|
||||
* there asks for Codeman's ROOT as an iframe navigation, and `/` is a registered
|
||||
* route (the app shell), which the route-table fence cannot tell from a real
|
||||
* navigation. It used to answer 401 inside the frame (or the shell itself on a
|
||||
* passwordless install), with no recovery message and the failed-frame panel
|
||||
* cleared because the document loaded fine. Credentials are the separator:
|
||||
* nothing in Codeman frames its own root, and the sandboxed frame carries none.
|
||||
*/
|
||||
describe('a reload on the dashboard landing page', () => {
|
||||
it('gets the recovery page when the iframe navigation of / carries no credentials', async () => {
|
||||
for (const url of ['/', '/?tab=2']) {
|
||||
const res = await app.inject({ method: 'GET', url, headers: lostFrame });
|
||||
expect(res.statusCode, url).toBe(200);
|
||||
expect(res.body, url).toContain('codeman:webview-lost');
|
||||
expect(res.headers['content-security-policy']).toContain("default-src 'none'");
|
||||
}
|
||||
});
|
||||
|
||||
it('is the shell, or the usual 401, once a session cookie or Authorization header is present', async () => {
|
||||
const ok = `Basic ${Buffer.from(`admin:${PASSWORD}`).toString('base64')}`;
|
||||
const shell = await app.inject({ method: 'GET', url: '/', headers: { ...lostFrame, authorization: ok } });
|
||||
expect(shell.statusCode).toBe(200);
|
||||
expect(shell.body).toBe('app shell');
|
||||
const wrong = `Basic ${Buffer.from('admin:nope').toString('base64')}`;
|
||||
expect(
|
||||
(await app.inject({ method: 'GET', url: '/', headers: { ...lostFrame, authorization: wrong } })).statusCode
|
||||
).toBe(401);
|
||||
const cookie = 'codeman_session=stale; other=1';
|
||||
expect((await app.inject({ method: 'GET', url: '/', headers: { ...lostFrame, cookie } })).statusCode).toBe(401);
|
||||
});
|
||||
|
||||
it('is still a 401 for a top-level navigation of /, and for a frame asking for JSON', async () => {
|
||||
expect((await app.inject({ method: 'GET', url: '/' })).statusCode).toBe(401);
|
||||
expect(
|
||||
(await app.inject({ method: 'GET', url: '/', headers: { ...lostFrame, 'sec-fetch-dest': 'document' } }))
|
||||
.statusCode
|
||||
).toBe(401);
|
||||
expect(
|
||||
(await app.inject({ method: 'GET', url: '/', headers: { ...lostFrame, accept: 'application/json' } }))
|
||||
.statusCode
|
||||
).toBe(401);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -0,0 +1,74 @@
|
||||
/**
|
||||
* @fileoverview A web-tab frame reloading on its dashboard's landing page, on an
|
||||
* install with NO password.
|
||||
*
|
||||
* The proxy's runtime shim masks `/webview/<cap>/` off the page's URL, and the
|
||||
* landing page masks to exactly `/`. A `location.reload()` there (a Vite dev
|
||||
* server on a config change) therefore asks for Codeman's own root as an iframe
|
||||
* navigation. Without a password no auth hook runs, so the request used to reach
|
||||
* the index route and render Codeman's app shell INSIDE the web tab, with no
|
||||
* recovery message and the failed-frame panel cleared because the document loaded
|
||||
* fine. `test/webview-auth-exemption.test.ts` pins the password form; this boots a
|
||||
* real WebServer in test mode for the passwordless one, where the index route
|
||||
* itself has to answer.
|
||||
*
|
||||
* Port: 3198
|
||||
*/
|
||||
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
|
||||
import { WebServer } from '../src/web/server.js';
|
||||
|
||||
const PORT = 3198;
|
||||
const lostFrame = { 'sec-fetch-dest': 'iframe', 'sec-fetch-mode': 'navigate', accept: 'text/html,*/*;q=0.8' };
|
||||
|
||||
describe('landing-page reload of a proxied dashboard, passwordless install', () => {
|
||||
let server: WebServer;
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
let app: any;
|
||||
beforeAll(async () => {
|
||||
server = new WebServer(PORT);
|
||||
await server.start();
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
app = (server as any).app;
|
||||
});
|
||||
afterAll(async () => {
|
||||
await server.stop();
|
||||
});
|
||||
|
||||
it('answers a credential-free iframe navigation of / with the recovery page, not the shell', async () => {
|
||||
for (const url of ['/', '/?tab=2']) {
|
||||
const res = await app.inject({ method: 'GET', url, headers: lostFrame });
|
||||
expect(res.statusCode, url).toBe(200);
|
||||
expect(res.headers['content-type'], url).toContain('text/html');
|
||||
expect(res.headers['content-security-policy'], url).toContain("default-src 'none'");
|
||||
expect(res.headers['cache-control'], url).toBe('no-store');
|
||||
expect(res.body, url).toContain('codeman:webview-lost');
|
||||
expect(res.body, url).not.toContain('<base href');
|
||||
}
|
||||
});
|
||||
|
||||
it('still serves the shell to a top-level navigation, and to a framed / that carries credentials', async () => {
|
||||
const top = await app.inject({ method: 'GET', url: '/' });
|
||||
expect(top.statusCode).toBe(200);
|
||||
expect(top.body).toContain('<base href');
|
||||
expect(top.body).not.toContain('codeman:webview-lost');
|
||||
for (const headers of [
|
||||
{ ...lostFrame, cookie: 'codeman_session=abc' },
|
||||
{ ...lostFrame, authorization: 'Basic YWRtaW46eA==' },
|
||||
{ ...lostFrame, 'sec-fetch-dest': 'document' },
|
||||
]) {
|
||||
const res = await app.inject({ method: 'GET', url: '/', headers });
|
||||
expect(res.statusCode).toBe(200);
|
||||
expect(res.body).toContain('<base href');
|
||||
}
|
||||
});
|
||||
|
||||
it('keeps answering any other lost path from the 404 handler', async () => {
|
||||
const res = await app.inject({ method: 'GET', url: '/settings/users', headers: lostFrame });
|
||||
expect(res.statusCode).toBe(200);
|
||||
expect(res.body).toContain('codeman:webview-lost');
|
||||
// The API-shaped 404 is untouched by the recovery path.
|
||||
const api = await app.inject({ method: 'GET', url: '/api/nope', headers: lostFrame });
|
||||
expect(api.statusCode).toBe(404);
|
||||
expect(JSON.parse(api.body).success).toBe(false);
|
||||
});
|
||||
});
|
||||
@@ -28,6 +28,7 @@ import {
|
||||
stripFrameAncestors,
|
||||
upstreamWebSocketUrl,
|
||||
isLostWebviewFrameNavigation,
|
||||
carriesAuthCredentials,
|
||||
lostWebviewFramePage,
|
||||
LOST_FRAME_PAGE_CSP,
|
||||
} from '../src/web/webview-proxy.js';
|
||||
@@ -816,4 +817,16 @@ describe('lost-frame recovery', () => {
|
||||
// The page must never carry a Referer that would leak anything about the tab.
|
||||
expect(page).toContain('name="referrer" content="no-referrer"');
|
||||
});
|
||||
|
||||
it('tells a credential-free request (a sandboxed frame reloading on /) from one that could authenticate', () => {
|
||||
const has = (headers: Record<string, string | string[] | undefined>) =>
|
||||
carriesAuthCredentials(headers, 'codeman_session');
|
||||
expect(has({})).toBe(false);
|
||||
expect(has({ cookie: 'theme=dark; codeman_sessions=lookalike' })).toBe(false);
|
||||
expect(has({ authorization: '' })).toBe(false);
|
||||
expect(has({ cookie: 'codeman_session=abc' })).toBe(true);
|
||||
expect(has({ cookie: 'theme=dark; codeman_session=abc' })).toBe(true);
|
||||
expect(has({ cookie: ['theme=dark', 'codeman_session=abc'] })).toBe(true);
|
||||
expect(has({ authorization: 'Basic YWRtaW46eA==' })).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user