mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-09-30 12:39:42 +02:00
fix(webview): revoke proxy capabilities on logout and stamp Referrer-Policy
WebviewCapabilityStore.revokeOwner() shipped for two releases with a docstring claiming logout called it and no caller at all. The capability is a bearer credential exempt from cookie auth with a rolling TTL refreshed on every use, so a proxy URL that leaked (browser history, a screenshot, a dashboard with a loose referrer policy) stayed valid for as long as anything kept polling it. - POST /api/logout revokes the caller's capabilities (all of them in single-user mode), the admin forced logout revokes the target user's, and user deletion revokes whatever that user had open. revokeOwner returns the count for the admin audit line. - Proxied responses carry `Referrer-Policy: same-origin` and the upstream's own policy is dropped: every URL inside the frame carries the capability, and a dashboard on no-referrer-when-downgrade or unsafe-url handed it to any third-party host it linked. Verified with Playwright that a sandboxed frame under an upstream `unsafe-url` sends no Referer to a third party while the root-absolute fetch and the CSS-triggered 404 fallback still reach the dashboard. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WKtW48T1UjAaecHAJxKobE
This commit is contained in:
@@ -35,6 +35,7 @@ import {
|
||||
UserStoreError,
|
||||
} from '../../user-store.js';
|
||||
import { getAuthUser, requireAdmin, revokeUserSessions } from '../route-helpers.js';
|
||||
import { webviewCapabilities } from '../../webview-capabilities.js';
|
||||
import { appendAdminAudit } from '../admin-audit.js';
|
||||
import { SseEvent } from '../sse-events.js';
|
||||
import type { AuthPort } from '../ports/auth-port.js';
|
||||
@@ -179,7 +180,10 @@ export function registerAdminRoutes(app: FastifyInstance, ctx: SessionPort & Aut
|
||||
if (!gate(req, reply)) return;
|
||||
const { username } = req.params as { username: string };
|
||||
const revoked = revokeUserSessions(ctx.authSessions, username);
|
||||
audit(req, 'user.logout', username, { revoked });
|
||||
// Web-tab proxy capabilities are a second credential the cookie purge does not
|
||||
// touch; a forced logout that left them alive would not be a logout.
|
||||
const revokedWebviews = webviewCapabilities.revokeOwner(normalizeUsername(username));
|
||||
audit(req, 'user.logout', username, { revoked, revokedWebviews });
|
||||
return { success: true, data: { revoked } };
|
||||
});
|
||||
|
||||
@@ -201,6 +205,7 @@ export function registerAdminRoutes(app: FastifyInstance, ctx: SessionPort & Aut
|
||||
await ctx.cleanupSession(id, true, 'admin_delete_user').catch(() => {});
|
||||
}
|
||||
revokeUserSessions(ctx.authSessions, username);
|
||||
webviewCapabilities.revokeOwner(normalizeUsername(username));
|
||||
if (deleteSpace) await deleteUserSpace(username);
|
||||
audit(req, 'user.delete', username, { deleteSpace, killedSessions: owned.length });
|
||||
ctx.broadcast(SseEvent.AdminUsersChanged, {});
|
||||
|
||||
@@ -31,6 +31,7 @@ import {
|
||||
} from '../../types.js';
|
||||
import { Session, isAltScreenStripMode, isExternalCliMode, isMuxAltScreenOnlyStripMode } from '../../session.js';
|
||||
import { SseEvent } from '../sse-events.js';
|
||||
import { webviewCapabilities } from '../../webview-capabilities.js';
|
||||
import {
|
||||
CreateSessionSchema,
|
||||
SessionNameSchema,
|
||||
@@ -821,6 +822,10 @@ export function registerSessionRoutes(
|
||||
if (sessionToken) {
|
||||
ctx.authSessions?.delete(sessionToken);
|
||||
}
|
||||
// The web-tab proxy authenticates on capabilities, not on this cookie, so a
|
||||
// logout has to retire them too or every dashboard URL opened during this
|
||||
// login keeps relaying without one (WebviewCapabilityStore.revokeOwner).
|
||||
webviewCapabilities.revokeOwner(ownerFor(req));
|
||||
reply.clearCookie(AUTH_COOKIE_NAME, { path: '/' });
|
||||
return {};
|
||||
});
|
||||
|
||||
@@ -93,6 +93,10 @@ const DROP_RESPONSE_HEADERS = new Set([
|
||||
'access-control-allow-headers',
|
||||
'access-control-expose-headers',
|
||||
'access-control-max-age',
|
||||
// The capability rides in every proxied URL, so the upstream's own referrer
|
||||
// policy must not decide whether third parties receive it. Ours is stamped in
|
||||
// buildDownstreamResponseHeaders.
|
||||
'referrer-policy',
|
||||
]);
|
||||
|
||||
/** The same-origin path prefix an iframe loads for a given capability. */
|
||||
@@ -352,6 +356,15 @@ export function buildDownstreamResponseHeaders(
|
||||
headers[lower] = value;
|
||||
}
|
||||
|
||||
// Every URL inside the frame carries the capability, and a dashboard that sets
|
||||
// `no-referrer-when-downgrade` or `unsafe-url` would hand it to any third-party
|
||||
// host it links or embeds. `same-origin` keeps the Referer on requests back to
|
||||
// Codeman (the 404 fallback and `refererPath` rely on it; both compare URL
|
||||
// origins, which an opaque-origin frame still satisfies) and strips it for
|
||||
// everyone else. A `<meta name="referrer">` inside the document can still
|
||||
// override this; that is the dashboard author's own decision about their page.
|
||||
headers['referrer-policy'] = 'same-origin';
|
||||
|
||||
const setCookie = setCookies.map((cookie) => rewriteSetCookie(cookie, capability, secureContext));
|
||||
|
||||
return { headers, setCookie, csp };
|
||||
|
||||
@@ -13,7 +13,8 @@
|
||||
*
|
||||
* - 128 bits of `randomBytes` entropy, base64url, never derived from anything.
|
||||
* - Held in memory only. A restart invalidates every outstanding capability.
|
||||
* - Rolling TTL: refreshed on use, expired after inactivity.
|
||||
* - Rolling TTL: refreshed on use, expired after inactivity, and revoked outright
|
||||
* on logout, admin logout and user deletion (`revokeOwner`).
|
||||
* - Bound to the minting user, so multi-user ownership survives the exemption.
|
||||
* - Grants exactly one thing: relaying bytes to that one saved URL. It reaches no
|
||||
* session, no file, no API surface.
|
||||
@@ -81,15 +82,33 @@ export class WebviewCapabilityStore {
|
||||
}
|
||||
}
|
||||
|
||||
/** Revoke every capability minted by a user (called on logout / user deletion). */
|
||||
revokeOwner(owner: string): void {
|
||||
/**
|
||||
* Revoke every capability bound to an identity. Called from `POST /api/logout`
|
||||
* (the caller's own identity, which in single-user mode is `undefined`, i.e.
|
||||
* every capability there is), from the admin logout route, and from user
|
||||
* deletion.
|
||||
*
|
||||
* ⚠️ This method shipped for two releases with NO caller while its docstring
|
||||
* claimed logout invoked it. The rolling TTL is refreshed on every use, so a
|
||||
* proxy URL that leaked (browser history, a shared screenshot, a dashboard with
|
||||
* a loose referrer policy) stayed valid indefinitely as long as something kept
|
||||
* polling it. Logging out is the user's one deliberate "invalidate what I
|
||||
* opened" gesture, and it has to reach here; `test/webview-capability-revocation.test.ts`
|
||||
* pins each call site.
|
||||
*
|
||||
* @returns how many capabilities were revoked (for the admin audit line).
|
||||
*/
|
||||
revokeOwner(owner: string | undefined): number {
|
||||
let revoked = 0;
|
||||
for (const [webviewId, token] of [...this.byWebview]) {
|
||||
const record = this.capabilities.peek(token);
|
||||
if (record?.owner === owner) {
|
||||
this.capabilities.delete(token);
|
||||
this.byWebview.delete(webviewId);
|
||||
revoked++;
|
||||
}
|
||||
}
|
||||
return revoked;
|
||||
}
|
||||
|
||||
get size(): number {
|
||||
|
||||
@@ -0,0 +1,132 @@
|
||||
/**
|
||||
* Web-tab proxy capabilities must die with the login that minted them.
|
||||
*
|
||||
* `WebviewCapabilityStore.revokeOwner()` shipped for two releases with a docstring
|
||||
* saying logout called it and NO caller. The capability is a bearer credential
|
||||
* exempt from cookie auth, with a rolling TTL refreshed on every use, so a leaked
|
||||
* proxy URL stayed valid indefinitely. These tests pin every call site:
|
||||
* `POST /api/logout` (own identity), the admin forced logout, and user deletion.
|
||||
*/
|
||||
|
||||
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
|
||||
import { WebviewCapabilityStore, webviewCapabilities } from '../src/webview-capabilities.js';
|
||||
import { createRouteTestHarness, type RouteTestHarness } from './routes/_route-test-utils.js';
|
||||
import { registerSessionRoutes } from '../src/web/routes/session-routes.js';
|
||||
import { registerAdminRoutes } from '../src/web/routes/admin-routes.js';
|
||||
import { createUser, invalidateUsersCache } from '../src/user-store.js';
|
||||
|
||||
const PASSWORD = 'correct-horse-battery-staple';
|
||||
|
||||
describe('WebviewCapabilityStore.revokeOwner', () => {
|
||||
it('revokes exactly the identity asked for, the single-user `undefined` identity included', () => {
|
||||
const store = new WebviewCapabilityStore();
|
||||
const solo = store.mint('wv-solo', undefined);
|
||||
const alice = store.mint('wv-alice', 'alice');
|
||||
const bob = store.mint('wv-bob', 'bob');
|
||||
|
||||
expect(store.revokeOwner('alice')).toBe(1);
|
||||
expect(store.resolve(alice)).toBeUndefined();
|
||||
expect(store.resolve(bob)).toBeDefined();
|
||||
expect(store.resolve(solo)).toBeDefined();
|
||||
|
||||
expect(store.revokeOwner(undefined)).toBe(1);
|
||||
expect(store.resolve(solo)).toBeUndefined();
|
||||
expect(store.resolve(bob)).toBeDefined();
|
||||
|
||||
// A later open mints a NEW token rather than resurrecting the revoked one.
|
||||
expect(store.mint('wv-alice', 'alice')).not.toBe(alice);
|
||||
expect(store.revokeOwner('nobody')).toBe(0);
|
||||
store.dispose();
|
||||
});
|
||||
});
|
||||
|
||||
describe('POST /api/logout', () => {
|
||||
let harness: RouteTestHarness;
|
||||
|
||||
beforeAll(async () => {
|
||||
harness = await createRouteTestHarness(registerSessionRoutes);
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
await harness.app.close();
|
||||
});
|
||||
|
||||
it('single-user: every outstanding capability dies with the login', async () => {
|
||||
const cap = webviewCapabilities.mint('wv-logout-solo', undefined);
|
||||
expect(webviewCapabilities.resolve(cap)).toBeDefined();
|
||||
|
||||
const res = await harness.app.inject({ method: 'POST', url: '/api/logout' });
|
||||
expect(res.statusCode).toBe(200);
|
||||
expect(webviewCapabilities.resolve(cap)).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe('POST /api/logout in multi-user mode', () => {
|
||||
let harness: RouteTestHarness;
|
||||
let savedMode: string | undefined;
|
||||
|
||||
beforeAll(async () => {
|
||||
savedMode = process.env.CODEMAN_MULTIUSER;
|
||||
process.env.CODEMAN_MULTIUSER = '1';
|
||||
harness = await createRouteTestHarness(registerSessionRoutes, { authUser: { username: 'peon', role: 'user' } });
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
await harness.app.close();
|
||||
if (savedMode === undefined) delete process.env.CODEMAN_MULTIUSER;
|
||||
else process.env.CODEMAN_MULTIUSER = savedMode;
|
||||
});
|
||||
|
||||
it("revokes only the caller's capabilities, never another user's", async () => {
|
||||
const mine = webviewCapabilities.mint('wv-peon-own', 'peon');
|
||||
const theirs = webviewCapabilities.mint('wv-boss-own', 'boss');
|
||||
|
||||
const res = await harness.app.inject({ method: 'POST', url: '/api/logout' });
|
||||
expect(res.statusCode).toBe(200);
|
||||
expect(webviewCapabilities.resolve(mine)).toBeUndefined();
|
||||
expect(webviewCapabilities.resolve(theirs)).toBeDefined();
|
||||
webviewCapabilities.revokeWebview('wv-boss-own');
|
||||
});
|
||||
});
|
||||
|
||||
describe('admin routes (multi-user)', () => {
|
||||
let harness: RouteTestHarness;
|
||||
let savedMode: string | undefined;
|
||||
|
||||
// The temp HOME from test/setup.ts is per-FILE, so users.json persists across
|
||||
// the tests in this block.
|
||||
beforeAll(async () => {
|
||||
savedMode = process.env.CODEMAN_MULTIUSER;
|
||||
process.env.CODEMAN_MULTIUSER = '1';
|
||||
invalidateUsersCache();
|
||||
await createUser({ username: 'boss', role: 'admin', password: PASSWORD });
|
||||
await createUser({ username: 'peon', role: 'user', password: PASSWORD });
|
||||
harness = await createRouteTestHarness(registerAdminRoutes, { authUser: { username: 'boss', role: 'admin' } });
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
await harness.app.close();
|
||||
if (savedMode === undefined) delete process.env.CODEMAN_MULTIUSER;
|
||||
else process.env.CODEMAN_MULTIUSER = savedMode;
|
||||
invalidateUsersCache();
|
||||
});
|
||||
|
||||
it('a forced logout revokes the target user (normalised) and leaves the admin alone', async () => {
|
||||
const peon = webviewCapabilities.mint('wv-peon-forced', 'peon');
|
||||
const boss = webviewCapabilities.mint('wv-boss-forced', 'boss');
|
||||
|
||||
const res = await harness.app.inject({ method: 'POST', url: '/api/admin/users/PEON/logout' });
|
||||
expect(res.statusCode).toBe(200);
|
||||
expect(webviewCapabilities.resolve(peon)).toBeUndefined();
|
||||
expect(webviewCapabilities.resolve(boss)).toBeDefined();
|
||||
webviewCapabilities.revokeWebview('wv-boss-forced');
|
||||
});
|
||||
|
||||
it('deleting a user revokes whatever that user had open', async () => {
|
||||
const peon = webviewCapabilities.mint('wv-peon-deleted', 'peon');
|
||||
|
||||
const res = await harness.app.inject({ method: 'DELETE', url: '/api/admin/users/peon' });
|
||||
expect(res.statusCode).toBe(200);
|
||||
expect(webviewCapabilities.resolve(peon)).toBeUndefined();
|
||||
});
|
||||
});
|
||||
@@ -653,3 +653,34 @@ describe('misc helpers', () => {
|
||||
expect(proxyPrefixFor(CAP)).toBe(PREFIX);
|
||||
});
|
||||
});
|
||||
|
||||
describe('referrer policy on proxied responses', () => {
|
||||
const CAP = 'c'.repeat(32);
|
||||
const requestUrl = new URL('http://127.0.0.1:4000/');
|
||||
|
||||
it('stamps same-origin and drops the upstream policy, so the capability in the URL never reaches a third party', () => {
|
||||
const { headers } = buildDownstreamResponseHeaders(
|
||||
[
|
||||
['referrer-policy', 'unsafe-url'],
|
||||
['content-type', 'text/html'],
|
||||
],
|
||||
[],
|
||||
CAP,
|
||||
requestUrl,
|
||||
false
|
||||
);
|
||||
expect(headers['referrer-policy']).toBe('same-origin');
|
||||
expect(headers['content-type']).toBe('text/html');
|
||||
});
|
||||
|
||||
it('stamps it even when the upstream sent none (the browser default would still leak on a downgrade-style policy)', () => {
|
||||
const { headers } = buildDownstreamResponseHeaders(
|
||||
[['content-type', 'application/json']],
|
||||
[],
|
||||
CAP,
|
||||
requestUrl,
|
||||
false
|
||||
);
|
||||
expect(headers['referrer-policy']).toBe('same-origin');
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user