mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-20 17:19:33 +02:00
fix(html-view): address review — scope, source view, tests, docs
- Refuse to mint for an HTML file in a hidden directory or directly in a broad root (/, home, tmpdir, cases and user-space roots): the capability serves the directory tree recursively to the page's own scripts. - Keep HTML's source view and editor: a Page pill (per-device htmlRendered, like MD) toggles to the file-content text path, and a refused mint (remote case, broad root, hidden dir) falls through to it automatically. - Tests: html-view auth exemption edges, revocation on logout / admin logout / user deletion, out-of-workspace mint (blocked tree, confinement), hidden and broad roots. - Docs: Working-With-Files, architecture-invariants, security-architecture, the file-routes comment and CLAUDE.md. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
1 parent
56978f2cc9
commit
91bdaee724
11 files changed
+275
-27
No files matched your search
@@ -8,10 +8,10 @@
|
||||
|
||||
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
|
||||
import { mkdtempSync, mkdirSync, rmSync, symlinkSync, writeFileSync } from 'node:fs';
|
||||
import { tmpdir } from 'node:os';
|
||||
import { homedir, tmpdir } from 'node:os';
|
||||
import { join } from 'node:path';
|
||||
import { createRouteTestHarness, type RouteTestHarness } from './_route-test-utils.js';
|
||||
import { registerHtmlViewRoutes } from '../../src/web/routes/html-view-routes.js';
|
||||
import { isBroadHtmlViewRoot, registerHtmlViewRoutes } from '../../src/web/routes/html-view-routes.js';
|
||||
import { capabilityFromHtmlViewPath, htmlViewCapabilities } from '../../src/html-view-capabilities.js';
|
||||
|
||||
describe('html-view routes', () => {
|
||||
@@ -106,4 +106,70 @@ describe('html-view routes', () => {
|
||||
it('reuses one capability per directory', async () => {
|
||||
expect(await openUrl()).toBe(await openUrl());
|
||||
});
|
||||
describe('outside the workspace', () => {
|
||||
let outsideDir: string;
|
||||
const saved = {
|
||||
confine: process.env.CODEMAN_ATTACHMENT_CONFINE,
|
||||
blocked: process.env.CODEMAN_ATTACHMENT_BLOCKED_PATHS,
|
||||
};
|
||||
|
||||
beforeEach(() => {
|
||||
outsideDir = join(root, 'reports');
|
||||
mkdirSync(outsideDir);
|
||||
writeFileSync(join(outsideDir, 'report.html'), '<p>report</p>');
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
for (const [key, value] of [
|
||||
['CODEMAN_ATTACHMENT_CONFINE', saved.confine],
|
||||
['CODEMAN_ATTACHMENT_BLOCKED_PATHS', saved.blocked],
|
||||
] as const) {
|
||||
if (value === undefined) delete process.env[key];
|
||||
else process.env[key] = value;
|
||||
}
|
||||
});
|
||||
|
||||
it('mints for a page under the attachment guard', async () => {
|
||||
expect((await open(join(outsideDir, 'report.html'))).statusCode).toBe(200);
|
||||
});
|
||||
|
||||
it('refuses a page in a blocked tree', async () => {
|
||||
process.env.CODEMAN_ATTACHMENT_BLOCKED_PATHS = outsideDir;
|
||||
expect((await open(join(outsideDir, 'report.html'))).statusCode).toBe(403);
|
||||
});
|
||||
|
||||
it('refuses every out-of-workspace page when attachments are confined to the workspace', async () => {
|
||||
process.env.CODEMAN_ATTACHMENT_CONFINE = '1';
|
||||
expect((await open(join(outsideDir, 'report.html'))).statusCode).toBe(403);
|
||||
// The workspace itself is still fine.
|
||||
expect((await open(join(workspace, 'index.html'))).statusCode).toBe(200);
|
||||
});
|
||||
});
|
||||
|
||||
it('refuses an HTML file inside a hidden directory, in or out of the workspace', async () => {
|
||||
mkdirSync(join(workspace, '.cache'));
|
||||
writeFileSync(join(workspace, '.cache', 'r.html'), '<p>r</p>');
|
||||
writeFileSync(join(workspace, '.cache', 'token.json'), '{}');
|
||||
expect((await open(join(workspace, '.cache', 'r.html'))).statusCode).toBe(403);
|
||||
mkdirSync(join(root, '.hidden'));
|
||||
writeFileSync(join(root, '.hidden', 'r.html'), '<p>r</p>');
|
||||
expect((await open(join(root, '.hidden', 'r.html'))).statusCode).toBe(403);
|
||||
});
|
||||
|
||||
it('treats the filesystem root, home and the temp dir as too broad to serve', () => {
|
||||
expect(isBroadHtmlViewRoot('/')).toBe(true);
|
||||
expect(isBroadHtmlViewRoot(homedir())).toBe(true);
|
||||
expect(isBroadHtmlViewRoot(tmpdir())).toBe(true);
|
||||
expect(isBroadHtmlViewRoot(workspace)).toBe(false);
|
||||
});
|
||||
|
||||
it('refuses a page sitting directly in the temp dir', async () => {
|
||||
const page = join(tmpdir(), `html-view-broad-${process.pid}.html`);
|
||||
writeFileSync(page, '<p>broad</p>');
|
||||
try {
|
||||
expect((await open(page)).statusCode).toBe(403);
|
||||
} finally {
|
||||
rmSync(page, { force: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -14,6 +14,7 @@ import Fastify, { type FastifyInstance } from 'fastify';
|
||||
import fastifyCookie from '@fastify/cookie';
|
||||
import { registerAuthMiddleware, registerHostGuard, registerSecurityHeaders } from '../src/web/middleware/auth.js';
|
||||
import { webviewCapabilities } from '../src/webview-capabilities.js';
|
||||
import { htmlViewCapabilities } from '../src/html-view-capabilities.js';
|
||||
import type { HostPolicy } from '../src/web/network-auth-policy.js';
|
||||
|
||||
const POLICY: HostPolicy = { allowedHosts: [], allowLan: true };
|
||||
@@ -21,6 +22,7 @@ const PASSWORD = 'test-password';
|
||||
|
||||
let app: FastifyInstance;
|
||||
let capability: string;
|
||||
let htmlCapability: string;
|
||||
let savedPassword: string | undefined;
|
||||
|
||||
beforeEach(async () => {
|
||||
@@ -29,6 +31,7 @@ beforeEach(async () => {
|
||||
process.env.CODEMAN_PASSWORD = PASSWORD;
|
||||
|
||||
capability = webviewCapabilities.mint('webview-under-test', undefined);
|
||||
htmlCapability = htmlViewCapabilities.mint('/srv/report-under-test', undefined);
|
||||
|
||||
app = Fastify({ logger: false });
|
||||
await app.register(fastifyCookie);
|
||||
@@ -48,6 +51,8 @@ beforeEach(async () => {
|
||||
app.all('/q/:token', async () => ({ qr: true }));
|
||||
app.get('/', async () => 'app shell');
|
||||
app.get('/webviewfoo/bar', async () => 'lookalike');
|
||||
app.all('/html-view/:cap/*', async () => ({ page: true }));
|
||||
app.all('/html-viewx/*', async () => 'lookalike');
|
||||
// Stand-in for @fastify/static mounted at '/', which is what actually serves
|
||||
// /static/app.js in production. It matches EVERY path, so the fence must treat a
|
||||
// root catch-all as "no real route" or the Referer form could never apply at all.
|
||||
@@ -58,6 +63,7 @@ beforeEach(async () => {
|
||||
afterEach(async () => {
|
||||
await app.close();
|
||||
webviewCapabilities.revokeWebview('webview-under-test');
|
||||
htmlViewCapabilities.revokeOwner(undefined);
|
||||
if (savedPassword === undefined) delete process.env.CODEMAN_PASSWORD;
|
||||
else process.env.CODEMAN_PASSWORD = savedPassword;
|
||||
});
|
||||
@@ -319,3 +325,42 @@ describe('a lost web-tab frame', () => {
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
// The rendered-HTML route (/html-view/<cap>/...) has its own, narrower exemption:
|
||||
// read-only, capability in the path only, no Referer form.
|
||||
describe('the html-view exemption', () => {
|
||||
it('lets an unauthenticated GET and HEAD through with a live capability', async () => {
|
||||
for (const method of ['GET', 'HEAD'] as const) {
|
||||
const res = await app.inject({ method, url: `/html-view/${htmlCapability}/index.html` });
|
||||
expect(res.statusCode, method).toBe(200);
|
||||
}
|
||||
});
|
||||
|
||||
it('never exempts a write on the capability path', async () => {
|
||||
for (const method of ['POST', 'PUT'] as const) {
|
||||
const res = await app.inject({ method, url: `/html-view/${htmlCapability}/index.html`, payload: {} });
|
||||
expect(res.statusCode, method).toBe(401);
|
||||
}
|
||||
});
|
||||
|
||||
it('rejects an unknown capability', async () => {
|
||||
const res = await app.inject({ method: 'GET', url: '/html-view/AAAAAAAAAAAAAAAAAAAAAAAA/index.html' });
|
||||
expect(res.statusCode).toBe(401);
|
||||
});
|
||||
|
||||
it('rejects a revoked capability immediately', async () => {
|
||||
htmlViewCapabilities.revokeOwner(undefined);
|
||||
const res = await app.inject({ method: 'GET', url: `/html-view/${htmlCapability}/index.html` });
|
||||
expect(res.statusCode).toBe(401);
|
||||
});
|
||||
|
||||
it('does not match a lookalike prefix', async () => {
|
||||
const res = await app.inject({ method: 'GET', url: `/html-viewx/${htmlCapability}/index.html` });
|
||||
expect(res.statusCode).toBe(401);
|
||||
});
|
||||
|
||||
it('is not a web-tab capability, and a web-tab capability is not an html-view one', async () => {
|
||||
expect((await app.inject({ method: 'GET', url: `/webview/${htmlCapability}/x` })).statusCode).toBe(401);
|
||||
expect((await app.inject({ method: 'GET', url: `/html-view/${capability}/x.html` })).statusCode).toBe(401);
|
||||
});
|
||||
});
|
||||
@@ -6,10 +6,12 @@
|
||||
* 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.
|
||||
* The rendered-HTML capabilities (`htmlViewCapabilities`) are pinned at the same sites.
|
||||
*/
|
||||
|
||||
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
|
||||
import { WebviewCapabilityStore, webviewCapabilities } from '../src/webview-capabilities.js';
|
||||
import { HtmlViewCapabilityStore, htmlViewCapabilities } from '../src/html-view-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';
|
||||
@@ -40,6 +42,28 @@ describe('WebviewCapabilityStore.revokeOwner', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('HtmlViewCapabilityStore.revokeOwner', () => {
|
||||
it('revokes exactly the identity asked for, the single-user `undefined` identity included', () => {
|
||||
const store = new HtmlViewCapabilityStore();
|
||||
const solo = store.mint('/srv/solo', undefined);
|
||||
const alice = store.mint('/srv/alice', 'alice');
|
||||
const bob = store.mint('/srv/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();
|
||||
|
||||
// Re-opening mints a NEW token rather than resurrecting the revoked one.
|
||||
expect(store.mint('/srv/alice', 'alice')).not.toBe(alice);
|
||||
store.dispose();
|
||||
});
|
||||
});
|
||||
|
||||
describe('POST /api/logout', () => {
|
||||
let harness: RouteTestHarness;
|
||||
|
||||
@@ -53,11 +77,14 @@ describe('POST /api/logout', () => {
|
||||
|
||||
it('single-user: every outstanding capability dies with the login', async () => {
|
||||
const cap = webviewCapabilities.mint('wv-logout-solo', undefined);
|
||||
const page = htmlViewCapabilities.mint('/srv/logout-solo', undefined);
|
||||
expect(webviewCapabilities.resolve(cap)).toBeDefined();
|
||||
expect(htmlViewCapabilities.resolve(page)).toBeDefined();
|
||||
|
||||
const res = await harness.app.inject({ method: 'POST', url: '/api/logout' });
|
||||
expect(res.statusCode).toBe(200);
|
||||
expect(webviewCapabilities.resolve(cap)).toBeUndefined();
|
||||
expect(htmlViewCapabilities.resolve(page)).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -80,12 +107,17 @@ describe('POST /api/logout in multi-user mode', () => {
|
||||
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 myPage = htmlViewCapabilities.mint('/srv/peon-own', 'peon');
|
||||
const theirPage = htmlViewCapabilities.mint('/srv/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();
|
||||
expect(htmlViewCapabilities.resolve(myPage)).toBeUndefined();
|
||||
expect(htmlViewCapabilities.resolve(theirPage)).toBeDefined();
|
||||
webviewCapabilities.revokeWebview('wv-boss-own');
|
||||
htmlViewCapabilities.revokeOwner('boss');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -114,19 +146,26 @@ describe('admin routes (multi-user)', () => {
|
||||
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 peonPage = htmlViewCapabilities.mint('/srv/peon-forced', 'peon');
|
||||
const bossPage = htmlViewCapabilities.mint('/srv/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();
|
||||
expect(htmlViewCapabilities.resolve(peonPage)).toBeUndefined();
|
||||
expect(htmlViewCapabilities.resolve(bossPage)).toBeDefined();
|
||||
webviewCapabilities.revokeWebview('wv-boss-forced');
|
||||
htmlViewCapabilities.revokeOwner('boss');
|
||||
});
|
||||
|
||||
it('deleting a user revokes whatever that user had open', async () => {
|
||||
const peon = webviewCapabilities.mint('wv-peon-deleted', 'peon');
|
||||
const peonPage = htmlViewCapabilities.mint('/srv/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();
|
||||
expect(htmlViewCapabilities.resolve(peonPage)).toBeUndefined();
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user