mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-09-30 12:39:42 +02:00
fix(files): give the picker a root when the server runs as root
Link Existing's Browse did nothing: GET /api/filesystem/browse answered 403
"No filesystem browse roots are available".
Two rules were fighting. /root is a default blocked tree in the attachment
guard, and Codeman running as root — containers, plenty of servers — makes
homedir() exactly /root, so the picker's own allowlisted Home root was blocked;
the other candidates live under it or do not exist. The root list came out
empty and there was nothing the user could open.
The blocked trees exist to keep ~/.ssh and friends out of reach, not to seal off
the user's own home. Only trees that would swallow a configured root whole are
dropped now: /root goes when Home is it (or sits inside it), /etc holds no
configured root and is untouched. Secrets stay protected — isSensitivePath
independently matches .ssh/, .env and credentials* at any depth, and it is what
the directory probe asks about.
⚠️ Navigation must reuse the same narrowed list the roots were chosen with.
Handing the raw trees downstream admits a root and then refuses every path
inside it, which reads as a picker that opens and does nothing.
This commit is contained in:
@@ -38,7 +38,7 @@ import {
|
||||
import { generateFirstPageThumbnail } from '../../document-thumbnailer.js';
|
||||
import { getOfficePreviewPdfPath, getPreviewPdfDownloadName } from '../../document-preview-cache.js';
|
||||
import { sanitizeAttachmentHistoryItem } from '../../session-attachment-history.js';
|
||||
import { isBlockedAttachmentPath, loadAttachmentGuardConfig } from '../../config/attachment-guard.js';
|
||||
import { isBlockedAttachmentPath, isUnderTree, loadAttachmentGuardConfig } from '../../config/attachment-guard.js';
|
||||
import { isMultiUserMode, userSpacePath } from '../../config/multiuser.js';
|
||||
import {
|
||||
CASES_DIR,
|
||||
@@ -425,6 +425,40 @@ function getFilesystemPreviewKind(fileName: string): FilesystemPreviewKind | und
|
||||
return undefined;
|
||||
}
|
||||
|
||||
/**
|
||||
* Blocked trees, minus any tree that would swallow a configured picker root
|
||||
* whole.
|
||||
*
|
||||
* `/root` is a default blocked tree, and Codeman running as root (containers,
|
||||
* plenty of servers) makes `homedir()` exactly `/root` — so the picker's own
|
||||
* allowlisted Home root was blocked by the attachment guard, every other
|
||||
* candidate lives under it or does not exist, and the endpoint answered 403
|
||||
* "No filesystem browse roots are available" with no root the user could reach.
|
||||
*
|
||||
* Dropping the tree does NOT expose secrets: `isSensitivePath` independently
|
||||
* matches `.ssh/`, `.env`, `credentials*` and friends at any depth, and it is
|
||||
* what the directory probe below asks about. Trees with no configured root
|
||||
* beneath them (`/etc`) are untouched.
|
||||
*/
|
||||
function pickerBlockedTrees(blockedTrees: readonly string[], roots: readonly string[]): readonly string[] {
|
||||
if (roots.length === 0) return blockedTrees;
|
||||
return blockedTrees.filter((tree) => !roots.some((root) => isUnderTree(root, tree)));
|
||||
}
|
||||
|
||||
/** Resolve candidate roots to realpaths, dropping the ones that do not exist. */
|
||||
function resolveCandidateRootPaths(candidates: ReadonlyArray<{ path: string }>): string[] {
|
||||
const out: string[] = [];
|
||||
for (const candidate of candidates) {
|
||||
if (!isAbsolute(candidate.path)) continue;
|
||||
try {
|
||||
out.push(realpathSync(candidate.path));
|
||||
} catch {
|
||||
// Optional roots (for example /mnt/d on non-WSL hosts) are omitted.
|
||||
}
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
function isBlockedPickerPath(path: string, blockedTrees: readonly string[], directory = false): boolean {
|
||||
if (isBlockedAttachmentPath(path, blockedTrees)) return true;
|
||||
// The shared sensitive-path matcher describes file locations such as
|
||||
@@ -491,13 +525,14 @@ async function resolveFilesystemPickerRoots(
|
||||
}
|
||||
|
||||
const guard = await loadAttachmentGuardConfig();
|
||||
const trees = pickerBlockedTrees(guard.blockedTrees, resolveCandidateRootPaths(candidates));
|
||||
const roots: FilesystemBrowseRoot[] = [];
|
||||
const seen = new Set<string>();
|
||||
for (const candidate of candidates) {
|
||||
if (!isAbsolute(candidate.path)) continue;
|
||||
try {
|
||||
const resolved = realpathSync(candidate.path);
|
||||
if (seen.has(resolved) || isBlockedPickerPath(resolved, guard.blockedTrees, true)) continue;
|
||||
if (seen.has(resolved) || isBlockedPickerPath(resolved, trees, true)) continue;
|
||||
const stat = await fs.stat(resolved);
|
||||
if (!stat.isDirectory()) continue;
|
||||
seen.add(resolved);
|
||||
@@ -556,7 +591,19 @@ async function resolveFilesystemPickerPath(
|
||||
}
|
||||
|
||||
const guard = await loadAttachmentGuardConfig();
|
||||
return { candidatePath, resolvedPath, roots, matchingRoot, blockedTrees: guard.blockedTrees };
|
||||
// Navigation must use the SAME narrowed list the roots were selected with.
|
||||
// Handing the raw trees down here would admit a root and then refuse every
|
||||
// path inside it, which reads as a picker that opens and then does nothing.
|
||||
return {
|
||||
candidatePath,
|
||||
resolvedPath,
|
||||
roots,
|
||||
matchingRoot,
|
||||
blockedTrees: pickerBlockedTrees(
|
||||
guard.blockedTrees,
|
||||
roots.map((root) => root.path)
|
||||
),
|
||||
};
|
||||
}
|
||||
|
||||
function appendDownloadFlag(url: string): string {
|
||||
|
||||
@@ -0,0 +1,53 @@
|
||||
/**
|
||||
* @fileoverview The picker must offer a root when Codeman runs as root.
|
||||
*
|
||||
* `/root` is a DEFAULT blocked tree in the attachment guard, and Codeman running
|
||||
* as root — containers, plenty of servers — makes `homedir()` exactly `/root`.
|
||||
* The picker's own allowlisted Home root was therefore blocked by the guard,
|
||||
* every other candidate lives under it or does not exist, and the endpoint
|
||||
* answered 403 "No filesystem browse roots are available" with nothing the user
|
||||
* could open. The fix drops only the trees that would swallow a configured root
|
||||
* whole; `isSensitivePath` still guards what is inside.
|
||||
*/
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { readFileSync } from 'node:fs';
|
||||
import { isBlockedAttachmentPath, isUnderTree } from '../src/config/attachment-guard.js';
|
||||
|
||||
const TREES = ['/root', '/etc'];
|
||||
|
||||
/** Mirror of pickerBlockedTrees in file-routes.ts. */
|
||||
const narrow = (trees: readonly string[], roots: readonly string[]) =>
|
||||
roots.length === 0 ? trees : trees.filter((t) => !roots.some((r) => isUnderTree(r, t)));
|
||||
|
||||
describe('file picker roots when the server runs as root', () => {
|
||||
it('drops the tree that would swallow the configured Home root', () => {
|
||||
expect(narrow(TREES, ['/root'])).toEqual(['/etc']);
|
||||
});
|
||||
|
||||
it('keeps trees that hold no configured root', () => {
|
||||
expect(narrow(TREES, ['/home/alice'])).toEqual(['/root', '/etc']);
|
||||
expect(narrow(TREES, [])).toEqual(['/root', '/etc']);
|
||||
});
|
||||
|
||||
it('also frees a root nested under the blocked tree', () => {
|
||||
// ~/codeman-cases is /root/codeman-cases when running as root.
|
||||
expect(narrow(TREES, ['/root/codeman-cases'])).toEqual(['/etc']);
|
||||
});
|
||||
|
||||
it('still refuses secrets inside the freed tree', () => {
|
||||
const trees = narrow(TREES, ['/root']);
|
||||
for (const p of ['/root/.ssh/id_rsa', '/root/.aws/credentials', '/root/app/.env']) {
|
||||
expect(isBlockedAttachmentPath(p, trees)).toBe(true);
|
||||
}
|
||||
// …while ordinary files under it become reachable, which is the point.
|
||||
expect(isBlockedAttachmentPath('/root/projects/readme.md', trees)).toBe(false);
|
||||
});
|
||||
|
||||
it('navigation reuses the same narrowed list the roots were chosen with', () => {
|
||||
// Handing the raw trees to navigation would admit a root and then refuse
|
||||
// every path inside it — a picker that opens and then does nothing.
|
||||
const src = readFileSync(new URL('../src/web/routes/file-routes.ts', import.meta.url), 'utf8');
|
||||
expect(src.match(/pickerBlockedTrees\(/g)?.length).toBeGreaterThanOrEqual(3);
|
||||
expect(src).not.toMatch(/blockedTrees:\s*guard\.blockedTrees/);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user