mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-06 07:29:42 +02:00
fix(files): serve remote-case attachments, the path a click takes outside the case
A clicked path that points OUTSIDE the case directory goes through the attachment routes (the frontend's `_isExternalPreviewPath` sends every absolute path not under `workingDir` to `POST /attachments`), and those had the same local-`fs` assumption as file-raw: `realpathSync`/`fs.stat` on a path that only exists on the remote host, so the file never opened — the case the #415 report was actually about. - `registerExternalAttachment()` accepts `remote` and resolves through `remoteProbePaths` (canonical path, size/mtime, kind, plus the workspace root for the confinement check). Everything around it — blocklist, extension allowlist, workspace confinement, registry/dedupe — is now shared by both branches, so the remote path cannot drift from the local one. - The by-id routes (`raw`, `preview`, `thumbnail`), the metadata poll and the attachment history list resolve over ssh too. `raw` streams with the same Range contract as file-raw; `preview` (office) and `thumbnail` answer 400 for a remote record; an unreachable host answers 502, a vanished file 404. - Which host a record is read from follows the SESSION, never the path string: the same absolute path is a different file on each host, and a remote session never falls back to a local file with that name. - Codex generated artifacts keep force-workspace confinement for a remote case: the well-known artifact directories are anchored at THIS host's home, so only a file inside the remote workspace is trusted. Still local-only by design: writes, office conversion, thumbnails, the file tree/picker and tail-file.
This commit is contained in:
Randalix
+105
-12
@@ -10,10 +10,12 @@ import { randomUUID } from 'node:crypto';
|
||||
import { realpathSync } from 'node:fs';
|
||||
import fs from 'node:fs/promises';
|
||||
import { basename, extname, isAbsolute } from 'node:path';
|
||||
import { isBlockedAttachmentPath, loadAttachmentGuardConfig } from './config/attachment-guard.js';
|
||||
import { isBlockedAttachmentPath, isUnderTree, loadAttachmentGuardConfig } from './config/attachment-guard.js';
|
||||
import { EDITABLE_EXTENSIONS } from './config/file-editing.js';
|
||||
import { validateSessionFilePath } from './web/route-helpers.js';
|
||||
import { remoteProbePaths, RemoteFileAccessError } from './remote-files.js';
|
||||
import type { AttachmentDetectedEvent, AttachmentDetectedType } from './types.js';
|
||||
import type { SessionRemote } from './types/session.js';
|
||||
|
||||
/**
|
||||
* Playable media extensions, single-sourced here because the WORKSPACE preview
|
||||
@@ -215,6 +217,92 @@ export interface RegisterExternalAttachmentOptions {
|
||||
* `codeman attach` CLI (which POSTs directly when a session id is known).
|
||||
*/
|
||||
forceWorkspaceConfinement?: boolean;
|
||||
/**
|
||||
* Remote (SSH) case: the path exists on the REMOTE host, so it is resolved and
|
||||
* stat'ed there (`remoteProbePaths`) instead of with local `realpathSync`/`fs.stat`,
|
||||
* which cannot see it at all (#415). A file outside the case directory is
|
||||
* unreachable exactly like a file inside it.
|
||||
*
|
||||
* `sessionWorkingDir` must then be the REMOTE path too, and the workspace
|
||||
* confinement check (when active) compares against the remotely canonicalized root,
|
||||
* so a symlinked `remotePath` does not refuse every registration.
|
||||
*/
|
||||
remote?: SessionRemote;
|
||||
}
|
||||
|
||||
/**
|
||||
* A path an attachment request resolved to, on whichever host it lives — the local
|
||||
* filesystem or the remote host of a remote-SSH case. The rest of
|
||||
* {@link registerExternalAttachment} (guards, extension allowlist, registry) is then
|
||||
* host-agnostic: it only ever sees canonical absolute paths and numbers.
|
||||
*/
|
||||
interface ResolvedAttachmentFile {
|
||||
resolvedPath: string;
|
||||
size: number;
|
||||
mtimeMs: number;
|
||||
isFile: boolean;
|
||||
extension: string;
|
||||
/** Remote only: the workspace root, with symlinks resolved on the remote host. */
|
||||
workspaceRoot?: string;
|
||||
}
|
||||
|
||||
/** `extension` the way the attachment registry defines it (no dot, lowercased). */
|
||||
function attachmentExtensionOf(path: string): string {
|
||||
return extname(path).toLowerCase().replace(/^\./, '');
|
||||
}
|
||||
|
||||
/** Local resolution: the historical realpath + stat. */
|
||||
async function resolveLocalAttachment(requestedPath: string): Promise<ResolvedAttachmentFile> {
|
||||
let resolvedPath: string;
|
||||
try {
|
||||
resolvedPath = realpathSync(requestedPath);
|
||||
} catch {
|
||||
throw new AttachmentRegistrationError('Attachment file not found', 404);
|
||||
}
|
||||
const stat = await fs.stat(resolvedPath);
|
||||
return {
|
||||
resolvedPath,
|
||||
size: stat.size,
|
||||
mtimeMs: stat.mtimeMs ?? 0,
|
||||
isFile: typeof stat.isFile === 'function' ? stat.isFile() : true,
|
||||
extension: attachmentExtensionOf(resolvedPath),
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Remote resolution for a remote-SSH case: ONE ssh round trip returns the
|
||||
* symlink-resolved path, the size/mtime and the kind, for the file AND (when a
|
||||
* workspace is known) its root, which the confinement check compares against.
|
||||
*/
|
||||
async function resolveRemoteAttachment(
|
||||
requestedPath: string,
|
||||
remote: SessionRemote,
|
||||
sessionWorkingDir?: string
|
||||
): Promise<ResolvedAttachmentFile> {
|
||||
const paths = sessionWorkingDir ? [requestedPath, sessionWorkingDir] : [requestedPath];
|
||||
let probes;
|
||||
try {
|
||||
probes = await remoteProbePaths(remote, paths);
|
||||
} catch (err) {
|
||||
throw new AttachmentRegistrationError(
|
||||
err instanceof RemoteFileAccessError ? err.message : 'remote host unreachable',
|
||||
502
|
||||
);
|
||||
}
|
||||
|
||||
const [probe, rootProbe] = probes;
|
||||
if (!probe) {
|
||||
throw new AttachmentRegistrationError('Attachment file not found', 404);
|
||||
}
|
||||
|
||||
return {
|
||||
resolvedPath: probe.realPath,
|
||||
size: probe.size,
|
||||
mtimeMs: probe.mtimeMs,
|
||||
isFile: probe.kind === 'file',
|
||||
extension: attachmentExtensionOf(probe.realPath),
|
||||
workspaceRoot: rootProbe?.realPath,
|
||||
};
|
||||
}
|
||||
|
||||
export async function registerExternalAttachment(
|
||||
@@ -226,12 +314,9 @@ export async function registerExternalAttachment(
|
||||
throw new AttachmentRegistrationError('Attachment path must be an absolute local path');
|
||||
}
|
||||
|
||||
let resolvedPath: string;
|
||||
try {
|
||||
resolvedPath = realpathSync(requestedPath);
|
||||
} catch {
|
||||
throw new AttachmentRegistrationError('Attachment file not found', 404);
|
||||
}
|
||||
const resolved = await (options.remote
|
||||
? resolveRemoteAttachment(requestedPath, options.remote, options.sessionWorkingDir)
|
||||
: resolveLocalAttachment(requestedPath));
|
||||
|
||||
// COD-53: enforce the active attachment-guard policy on the symlink-resolved
|
||||
// path before doing anything else.
|
||||
@@ -243,7 +328,10 @@ export async function registerExternalAttachment(
|
||||
// the caller forces it for this registration (the magic-link scanner — see
|
||||
// forceWorkspaceConfinement). Strictly more restrictive than the blocklist.
|
||||
const workingDir = options.sessionWorkingDir;
|
||||
if (!workingDir || !validateSessionFilePath(workingDir, resolvedPath)) {
|
||||
const confined = options.remote
|
||||
? !!workingDir && isUnderTree(resolved.resolvedPath, resolved.workspaceRoot ?? workingDir)
|
||||
: !!workingDir && !!validateSessionFilePath(workingDir, resolved.resolvedPath);
|
||||
if (!confined) {
|
||||
throw new AttachmentRegistrationError('Access to this file is blocked', 403);
|
||||
}
|
||||
}
|
||||
@@ -253,20 +341,25 @@ export async function registerExternalAttachment(
|
||||
// operator-configured extra trees. Symlinks are already resolved above.
|
||||
// Cross-workspace attachment of non-blocked files stays allowed, so
|
||||
// codeman-publish and the ~/.codeman review loop keep working.
|
||||
if (isBlockedAttachmentPath(resolvedPath, guard.blockedTrees)) {
|
||||
//
|
||||
// The list is a pattern list over ABSOLUTE paths, so it is host-agnostic and holds
|
||||
// for a remote path exactly as it does for a local one.
|
||||
if (isBlockedAttachmentPath(resolved.resolvedPath, guard.blockedTrees)) {
|
||||
throw new AttachmentRegistrationError('Access to this file is blocked', 403);
|
||||
}
|
||||
|
||||
const extension = extname(resolvedPath).toLowerCase().replace(/^\./, '');
|
||||
const resolvedPath = resolved.resolvedPath;
|
||||
const extension = resolved.extension;
|
||||
if (!isSupportedAttachmentExtension(extension)) {
|
||||
throw new AttachmentRegistrationError('Unsupported attachment type');
|
||||
}
|
||||
|
||||
const stat = await fs.stat(resolvedPath);
|
||||
if (typeof stat.isFile === 'function' && !stat.isFile()) {
|
||||
if (!resolved.isFile) {
|
||||
throw new AttachmentRegistrationError('Attachment path is not a file');
|
||||
}
|
||||
|
||||
const stat = { size: resolved.size, mtimeMs: resolved.mtimeMs };
|
||||
|
||||
const existing = attachmentRegistry.findByFilePath(sessionId, resolvedPath);
|
||||
if (existing) {
|
||||
existing.size = stat.size;
|
||||
|
||||
Reference in New Issue
Block a user