fix(preview): folder grid review — multi-user scope, bounded probing, smaller items

- Multi-user: a non-admin's folder must resolve inside their own userSpacePath
  or the session's workingDir (folderAllowedForUser, the path picker's rule),
  checked on the resolved folder before it is read and again on each served
  file after symlink resolution; 403 otherwise.
- listFolder probes the local folder with probePathKind({ pastCap: true })
  first (unknown -> 503 via describeUnknownPath, never 404), then async
  fs.realpath; resolveFolderFile uses probePathKind instead of fs.stat.
- The guard runs on the realpath before readdir.
- FOLDER_PATH_LINK_PATTERN ends a folder at ':' (quadratic `://` rescan on
  colon runs; PATH=/a:/b linked as one folder).
- Load counters drop stale preview / folder-grid responses; closing the grid
  invalidates a listing still in flight.
- validFolderFileName refuses a leading dot; the remote listing matches
  extensions in any case.
- Docs: folder grid in the z-index lists; multi-user rule and bounded probe in
  the invariants and security docs.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
RandalixandClaude Opus 5.5 committed 2026-10-11 14:03:20 +02:00
1 parent da7ffec734
commit 3857fba065
12 files changed
+332 -37

No files matched your search

+64 -19
View File
@@ -18,7 +18,6 @@
*/
import { randomUUID } from 'node:crypto';
import { realpathSync } from 'node:fs';
import fs from 'node:fs/promises';
import { homedir } from 'node:os';
import { extname, isAbsolute, join, posix } from 'node:path';
@@ -28,9 +27,11 @@ import {
supportedAttachmentExtensions,
} from './attachment-registry.js';
import { isBlockedAttachmentPath, isUnderTree, loadAttachmentGuardConfig } from './config/attachment-guard.js';
import { isMultiUserMode, userSpacePath } from './config/multiuser.js';
import { remoteListFolder, RemoteFileAccessError, type RemoteFolderEntry } from './remote-files.js';
import type { AttachmentDetectedType } from './types.js';
import type { AttachmentDetectedType, AuthUser } from './types.js';
import type { SessionRemote } from './types/session.js';
import { describeUnknownPath, probePathKind } from './utils/bounded-path-probe.js';
import { validateSessionFilePath } from './web/route-helpers.js';
/** Most entries (files + subfolders) one listing returns. */
@@ -111,11 +112,12 @@ export const folderGrants = new FolderGrants();
/**
* A file name inside a granted folder, or null when it is not one: no path
* separator, not `.`/`..`, no NUL, and a type the preview supports.
* separator, not hidden (which also rules out `.`/`..`; the listing leaves hidden
* entries out, so the routes do too), no NUL, and a type the preview supports.
*/
export function validFolderFileName(name: unknown): string | null {
if (typeof name !== 'string' || !name || name.length > 255) return null;
if (name === '.' || name === '..' || name.includes('/') || name.includes('\\') || name.includes('\0')) return null;
if (name.startsWith('.') || name.includes('/') || name.includes('\\') || name.includes('\0')) return null;
const extension = extname(name).toLowerCase().replace(/^\./, '');
return isSupportedAttachmentExtension(extension) ? name : null;
}
@@ -128,13 +130,43 @@ export function folderFilePath(dir: string, name: string): string {
interface ListFolderOptions {
sessionWorkingDir?: string;
remote?: SessionRemote;
/** The requesting identity; in multi-user mode a regular user is held to {@link folderAllowedForUser}. */
user?: AuthUser;
}
/**
* Multi-user rule for folder grants, the same one the filesystem path picker follows:
* a regular user reaches only their own space (`userSpacePath`) or the session's
* `workingDir`. User spaces live inside `homedir()`, so without it a user could list
* and read another user's space. Always true in single-user mode and for admins; a
* remote session's folders are on its host, where only the `workingDir` applies.
*
* @param path Already resolved (realpath on the session's host).
*/
export function folderAllowedForUser(
path: string,
user: AuthUser | undefined,
scope: { workingDir?: string; remote?: SessionRemote }
): boolean {
if (!isMultiUserMode()) return true;
if (!user) return false;
if (user.role === 'admin') return true;
if (scope.remote) return !!scope.workingDir && isUnderTree(path, scope.workingDir);
return [userSpacePath(user.username), scope.workingDir].some(
(root) => !!root && !!validateSessionFilePath(root, path)
);
}
/**
* Resolve, guard, list and grant a folder for `sessionId`.
*
* A local folder is probed bounded first (`probePathKind`): one on a network mount
* that stopped answering must not freeze the server, and "did not answer" is never
* reported as "not found". The guard runs on the resolved folder before it is read.
*
* @param requestedPath Absolute, or `~/…` (the home of the session's host).
* @throws FolderListingError 400 not absolute / not a folder, 403 blocked, 404 missing, 502 remote unreachable.
* @throws FolderListingError 400 not absolute / not a folder, 403 blocked or outside the
* user's space, 404 missing, 502 remote unreachable, 503 local folder not answering.
*/
export async function listFolder(
sessionId: string,
@@ -146,6 +178,22 @@ export async function listFolder(
throw new FolderListingError('Folder path must be absolute');
}
const guard = await loadAttachmentGuardConfig();
const workingDir = options.sessionWorkingDir;
const assertAllowed = (dir: string): void => {
const confined = !guard.confineToWorkspace
? true
: options.remote
? !!workingDir && isUnderTree(dir, workingDir)
: !!workingDir && !!validateSessionFilePath(workingDir, dir);
if (!confined || isBlockedAttachmentPath(dir, guard.blockedTrees)) {
throw new FolderListingError('Access to this folder is blocked', 403);
}
if (!folderAllowedForUser(dir, options.user, { workingDir, remote: options.remote })) {
throw new FolderListingError('Folder is outside your space', 403);
}
};
let dir: string;
let entries: RemoteFolderEntry[];
let truncated = false;
@@ -160,29 +208,26 @@ export async function listFolder(
if (!listing) throw new FolderListingError('Folder not found', 404);
if (listing === 'not-a-folder') throw new FolderListingError('Not a folder');
({ dir, entries, truncated } = listing);
// One ssh call lists and resolves together, so the guard can only run after it;
// nothing of a refused listing leaves this function.
assertAllowed(dir);
} else {
const expanded = homeRelative ? join(homedir(), requestedPath.slice(2)) : requestedPath;
const kind = await probePathKind(expanded, { pastCap: true });
if (kind === 'unknown') {
throw new FolderListingError(describeUnknownPath('Folder', expanded, { pastCap: true }), 503);
}
if (kind === 'absent') throw new FolderListingError('Folder not found', 404);
if (kind !== 'directory') throw new FolderListingError('Not a folder');
try {
dir = realpathSync(expanded);
dir = await fs.realpath(expanded);
} catch {
throw new FolderListingError('Folder not found', 404);
}
const stat = await fs.stat(dir);
if (!stat.isDirectory()) throw new FolderListingError('Not a folder');
assertAllowed(dir);
({ entries, truncated } = await listLocalFolder(dir));
}
const guard = await loadAttachmentGuardConfig();
const workingDir = options.sessionWorkingDir;
const confined = !guard.confineToWorkspace
? true
: options.remote
? !!workingDir && isUnderTree(dir, workingDir)
: !!workingDir && !!validateSessionFilePath(workingDir, dir);
if (!confined || isBlockedAttachmentPath(dir, guard.blockedTrees)) {
throw new FolderListingError('Access to this folder is blocked', 403);
}
const files: FolderFileEntry[] = [];
const folders: string[] = [];
for (const entry of entries) {
+4 -4
View File
@@ -293,7 +293,7 @@ export interface RemoteFolderListing {
/**
* List one remote folder in a SINGLE ssh round trip: its canonical path, then every
* non-hidden subfolder and every regular file (symlinks followed) whose name ends in
* one of `extensions` (either case), up to `maxEntries`.
* one of `extensions` (any case), up to `maxEntries`.
*
* Same output discipline as {@link buildRemoteProbeCommand}: a lone NUL first (so rc
* chatter before it is discarded), then NUL-terminated records — `E|n` (no such
@@ -303,9 +303,9 @@ export interface RemoteFolderListing {
* hold. A leading `~/` is the remote user's `$HOME`, as in the probe.
*/
export function buildRemoteFolderListCommand(dir: string, extensions: readonly string[], maxEntries: number): string {
const patterns = extensions
.flatMap((ext) => [ext.toLowerCase(), ext.toUpperCase()])
.map((ext) => `*.${ext}`)
// `[pP][nN][gG]`: any case, as the local listing compares lowercased (`shot.Png`).
const patterns = [...new Set(extensions.map((ext) => ext.toLowerCase()))]
.map((ext) => `*.${ext.replace(/[a-z]/g, (c) => `[${c}${c.toUpperCase()}]`)}`)
.join('|');
return [
`d=${shellescape(dir)}`,
+5 -2
View File
@@ -2054,13 +2054,16 @@ function findFilePathLinks(text) {
* trailing slash (`builds/captures/`). Same boundary in front as the relative
* file pattern (group 1), the candidate itself is group 2 and goes through
* {@link folderLinkPath}, which decides whether it reads as a folder at all.
* File links are found first and win any overlap.
* File links are found first and win any overlap. A folder path ends at `:`,
* which is also a boundary in front: `PATH=/a/bin:/b/bin` is two candidates, and
* the `://` lookahead stops at the next colon instead of rescanning the token from
* every one (quadratic on `a:a:a:…//`).
*
* ⚠ Like its siblings, never share an instance (`lastIndex`): use
* {@link findFilePathLinks}.
*/
const FOLDER_PATH_LINK_PATTERN =
/(^|[\s"'`(\[{<>=,:])((?:\/(?:home|Users|tmp|var|private|opt|mnt|srv|media|data|workspace)\/|~\/|(?![^\s"'`<>|;&()[\]{}=,\n\x00-\x1f]*:\/\/)[^\s"'`<>|;&()[\]{}=,\/~\n\x00-\x1f])[^\s"'`<>|;&()[\]{}=,\n\x00-\x1f]*)/g;
/(^|[\s"'`(\[{<>=,:])((?:\/(?:home|Users|tmp|var|private|opt|mnt|srv|media|data|workspace)\/|~\/|(?![^\s"'`<>|;&()[\]{}=,:\n\x00-\x1f]*:\/\/)[^\s"'`<>|;&()[\]{}=,:\/~\n\x00-\x1f])[^\s"'`<>|;&()[\]{}=,:\n\x00-\x1f]*)/g;
/**
* A {@link FOLDER_PATH_LINK_PATTERN} candidate as a folder link, or null.
+21 -1
View File
@@ -4107,6 +4107,12 @@ Object.assign(CodemanApp.prototype, {
if (!overlay || !bodyEl) return;
// Only the newest load may write: paging ←/→ through a folder starts the next
// load while the previous one still awaits its fetch, and its late answer would
// put the previous file's text under the next file's title.
const load = (this._filePreviewLoad = (this._filePreviewLoad || 0) + 1);
const stale = () => load !== this._filePreviewLoad;
// Edit mode: reset any prior editor state whenever a preview (re)loads.
this._resetFilePreviewEdit();
// Stop whatever the previous preview was playing. Overwriting innerHTML
@@ -4138,6 +4144,7 @@ Object.assign(CodemanApp.prototype, {
let externalSize = 0;
if (!attachmentId && !folderFile && this._isExternalPreviewPath(filePath, sessionId)) {
const external = await this._registerExternalPreview(filePath, sessionId);
if (stale()) return;
attachmentId = external.attachmentId || null;
externalError = external.error || '';
externalSize = external.size || 0;
@@ -4211,6 +4218,7 @@ Object.assign(CodemanApp.prototype, {
const res = await fetch(`${base}/raw`, { headers: { Range: `bytes=0-${TEXT_PREVIEW_MAX_BYTES - 1}` } });
if (!res.ok) throw new Error('Failed to load attachment');
const text = await res.text();
if (stale()) return;
const clippedByBytes = res.status === 206 && text.length >= TEXT_PREVIEW_MAX_BYTES;
const lines = text.split('\n');
// Markdown keeps every line the Range read returned: the byte bound is
@@ -4229,6 +4237,7 @@ Object.assign(CodemanApp.prototype, {
footerEl.textContent = `${footerEl.textContent} (${note})`;
}
} catch (err) {
if (stale()) return;
bodyEl.innerHTML = `<div class="binary-message">Error: ${escapeHtml(err.message)}</div>`;
}
}
@@ -4261,11 +4270,14 @@ Object.assign(CodemanApp.prototype, {
try {
const res = await fetch(`/api/sessions/${sessionId}/file-raw?path=${encodeURIComponent(filePath)}`);
if (!res.ok) throw new Error('Failed to load image');
const blobUrl = URL.createObjectURL(new Blob([await res.text()], { type: 'image/svg+xml' }));
const svgText = await res.text();
if (stale()) return;
const blobUrl = URL.createObjectURL(new Blob([svgText], { type: 'image/svg+xml' }));
bodyEl.innerHTML = `<img src="${blobUrl}" alt="${escapeHtml(filePath)}">`;
const img = bodyEl.querySelector('img');
if (img) img.onload = () => URL.revokeObjectURL(blobUrl);
} catch (err) {
if (stale()) return;
bodyEl.innerHTML = `<div class="binary-message">Error: ${escapeHtml(err.message)}</div>`;
}
return;
@@ -4281,6 +4293,7 @@ Object.assign(CodemanApp.prototype, {
if (!res.ok) throw new Error('Failed to load file');
const result = await res.json();
if (stale()) return;
if (!result.success) throw new Error(result.error || 'Failed to load file');
const data = result.data;
@@ -4320,6 +4333,7 @@ Object.assign(CodemanApp.prototype, {
}
}
} catch (err) {
if (stale()) return;
console.error('Failed to preview file:', err);
bodyEl.innerHTML = `<div class="binary-message">Error: ${escapeHtml(err.message)}</div>`;
}
@@ -4341,6 +4355,8 @@ Object.assign(CodemanApp.prototype, {
const body = this.$('folderGridBody');
if (!overlay || !body) return;
this._bindFolderGridKeys();
// Two quick opens (a chip, then its parent): only the newer one may render.
const load = (this._folderGridLoad = (this._folderGridLoad || 0) + 1);
let result = null;
let status = 0;
try {
@@ -4352,9 +4368,11 @@ Object.assign(CodemanApp.prototype, {
status = res.status;
result = await res.json().catch(() => null);
} catch (err) {
if (load !== this._folderGridLoad) return;
this.showToast(`Cannot open folder: ${err.message || err}`, 'error');
return;
}
if (load !== this._folderGridLoad) return;
if (!result?.success || !result.data) {
const reason = result?.error || `HTTP ${status}`;
if (status === 400 && /not a folder/i.test(reason)) {
@@ -4371,6 +4389,8 @@ Object.assign(CodemanApp.prototype, {
},
closeFolderGrid() {
// A listing still on its way must not reopen the grid that was just closed.
this._folderGridLoad = (this._folderGridLoad || 0) + 1;
this.$('folderGridOverlay')?.classList.remove('visible');
const body = this.$('folderGridBody');
// Drop the tiles: lazy <img>s still loading would keep fetching otherwise.
+26 -6
View File
@@ -23,6 +23,7 @@ import type {
} from '../../types.js';
import { ApiErrorCode, createErrorResponse, getErrorMessage } from '../../types.js';
import { compileFileQuery } from '../../utils/file-query.js';
import { describeUnknownPath, probePathKind } from '../../utils/bounded-path-probe.js';
import { fileStreamManager } from '../../file-stream-manager.js';
import {
AUDIO_ATTACHMENT_EXTENSIONS,
@@ -41,6 +42,7 @@ import {
FolderListingError,
folderFilePath,
folderGrants,
folderAllowedForUser,
listFolder,
validFolderFileName,
} from '../../folder-listing.js';
@@ -2387,14 +2389,18 @@ export function registerFileRoutes(app: FastifyInstance, ctx: SessionPort & Even
return;
}
try {
const listing = await listFolder(id, body.path, { sessionWorkingDir: scope.workingDir, remote: scope.remote });
const listing = await listFolder(id, body.path, {
sessionWorkingDir: scope.workingDir,
remote: scope.remote,
user: getAuthUser(req),
});
return { success: true, data: listing };
} catch (err) {
if (err instanceof FolderListingError) {
const code =
err.statusCode === 404
? ApiErrorCode.NOT_FOUND
: err.statusCode === 502
: err.statusCode >= 500
? ApiErrorCode.OPERATION_FAILED
: ApiErrorCode.INVALID_INPUT;
reply.code(err.statusCode).send(createErrorResponse(code, err.message));
@@ -2410,7 +2416,9 @@ export function registerFileRoutes(app: FastifyInstance, ctx: SessionPort & Even
* One file of a granted folder, guard-checked like an attachment: the name is a
* bare file name of a supported type, and the joined path goes through the same
* {@link resolveServableAttachmentPath} (blocklist, workspace confinement, symlink
* re-resolution, remote re-probe) an attachment by id does.
* re-resolution, remote re-probe) an attachment by id does. The multi-user rule
* ({@link folderAllowedForUser}) runs again on the resolved file: a symlink inside
* the granted folder must not lead into another user's space.
*/
async function resolveFolderFile(
req: FastifyRequest,
@@ -2431,9 +2439,17 @@ export function registerFileRoutes(app: FastifyInstance, ctx: SessionPort & Even
}
const filePath = folderFilePath(grant.dir, fileName);
if (!scope.remote) {
try {
if (!(await fs.stat(filePath)).isFile()) throw new Error('not a file');
} catch {
// Bounded, like the folder itself: the grant may point at a mount that has stopped answering since.
const kind = await probePathKind(filePath, { pastCap: true });
if (kind === 'unknown') {
reply
.code(503)
.send(
createErrorResponse(ApiErrorCode.OPERATION_FAILED, describeUnknownPath('File', filePath, { pastCap: true }))
);
return null;
}
if (kind !== 'file') {
reply.code(404).send(createErrorResponse(ApiErrorCode.NOT_FOUND, 'File not found'));
return null;
}
@@ -2453,6 +2469,10 @@ export function registerFileRoutes(app: FastifyInstance, ctx: SessionPort & Even
};
const servable = await resolveServableAttachmentPath(reply, record, scope);
if (!servable) return null;
if (!folderAllowedForUser(servable.path, getAuthUser(req), scope)) {
reply.code(403).send(createErrorResponse(ApiErrorCode.INVALID_INPUT, 'File is outside your space'));
return null;
}
return { scope, servable, name: fileName, extension };
}