mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-02 21:49:42 +02:00
Follow-up to #421 (remote-case file reads over ssh), addressing the review. Symlink escape on a host without `readlink -f` (blocker). The probe's portable fallback canonicalized only the directory chain and returned the final component unresolved, so on macOS < 12.3 `ws/notes.txt -> ~/.ssh/id_rsa` came back as `.../ws/notes.txt` (with the target's size), passed every containment and blocklist check that runs on `realPath`, and `cat` followed the link. The fallback now walks the directory chain with `cd -P`/`pwd -P` and follows the LAST component with plain `readlink` for a bounded number of hops, and anything it cannot fully resolve (a loop, a readlink failure, the hop cap) is reported with an `x` marker that parses as null, i.e. 404. It never returns the unresolved string. Measured on a real /bin/sh with `readlink -f` shadowed: the pre-fix script reports `/ws/notes.txt`, the fixed one `/secret/id_rsa`; both branches (native and fallback) now agree. `PUT /api/sessions/:id/file-content` never had the remote guard the PR described. It sits ahead of `validateSessionFilePath`, which resolves against the LOCAL filesystem, because with a same-named directory on the Codeman host (an sshfs mount of the remote tree, the documented stop-gap) the write landed on the local twin while the viewer believed it edited the remote file. ssh fan-out is bounded. `src/remote-ssh-limiter.ts` is a document-conversion-limiter-shaped semaphore (default 4, env `CODEMAN_MAX_REMOTE_FILE_SSH`) around every probe and buffered read; the attachment-history list resolves its whole history in ONE batched probe (`probeRemoteAttachmentHistory`, threaded into `registerExternalAttachment({remoteProbes})` so the guards run unchanged) instead of one handshake per entry; and probes chunk at 40 paths because the whole script is one argv string. Terminal output in a remote session is written on the remote host, so a prompt-injected agent printing hundreds of `codeman://attach` links forked one ssh per link, each holding a 20 s timeout, and a 100-entry history re-listed on every attachment:detected tripped OpenSSH's default MaxStartups. Streams are deliberately not counted (one per browser request, held for a whole playback, and gated behind a counted probe anyway). Smaller items from the same review: probe records are NUL-terminated and index-keyed after a leading NUL (a newline in a filename can no longer shift the alignment, and the banner is fenced off without last-N-lines guessing); size comes from `stat -c %s || stat -f %z`; the three IO functions refuse under VITEST instead of opening a connection; an unreachable host now reads as unknown (missing: false) for detected AND external history entries, where external used to fold its 502 into missing; a client that aborted during the guard probe has its body's ssh child reaped (`reply.raw.destroyed` is checked before the close listener is attached); `describeExecError` never returns Node's `Command failed: <ssh line>` message, which carried the identity path and the probe script into a 502 body; and the docs note that `isSensitivePath`'s three home-anchored entries resolve against the Codeman host's home, not the remote one. Tests: the probe script runs on a real /bin/sh with a `readlink` shim that rejects `-f` (the escape, a relative chain through a symlinked directory, a loop, a newline filename, banner chatter that itself looks like a record), the limiter's cap and FIFO order, and route tests for the PUT guard (local twin untouched, no connection), the single batched history probe, the unreachable-host alignment and the aborted-client reap. All four route tests fail against the pre-fix file-routes.ts. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
90 lines
3.4 KiB
TypeScript
90 lines
3.4 KiB
TypeScript
/**
|
|
* @fileoverview Global concurrency limiter for the short-lived `ssh` children that
|
|
* remote-case file access spawns (`src/remote-files.ts`: the realpath+stat probe and
|
|
* the buffered text read).
|
|
*
|
|
* Two paths can fan those out without a human behind each one:
|
|
*
|
|
* - `GET /api/sessions/:id/attachments` resolves every history entry (up to
|
|
* `ATTACHMENT_HISTORY_LIMIT`, 100), and the attachments drawer re-runs it on every
|
|
* `attachment:detected` event while it is open, which is exactly when an agent is
|
|
* writing files. The route now batches the probes, but a burst of drawers is still
|
|
* a burst.
|
|
* - A `codeman://attach?path=` magic link in terminal output registers the path
|
|
* fire-and-forget, once per distinct link per PTY chunk. In a remote session that
|
|
* output is written by a process on the remote host, so a prompt-injected agent can
|
|
* print hundreds of links and have the server fork one `ssh` per link, each holding
|
|
* a 20s probe timeout.
|
|
*
|
|
* Without a cap that is the fork-bomb shape `document-conversion-limiter.ts` exists to
|
|
* prevent, and it also trips OpenSSH's default `MaxStartups 10:30:100`, which starts
|
|
* dropping connections at ten unauthenticated handshakes. This is that limiter for
|
|
* ssh: a small fixed pool, FIFO queueing, and a slot handed straight to the next
|
|
* waiter on release so the active count can never exceed the cap under interleaved
|
|
* async resumption.
|
|
*
|
|
* Streams (`remoteCreateReadStream`) are deliberately NOT counted: one is opened per
|
|
* browser request and held for the life of a media playback, so four open videos
|
|
* would otherwise block every preview and the history list. They are already gated
|
|
* behind a counted probe (the guard re-probe runs first), so their spawn RATE is
|
|
* bounded here even though their concurrency is bounded by the browser.
|
|
*
|
|
* NOT re-entrant: never acquire from inside a task already holding a slot.
|
|
*/
|
|
|
|
/**
|
|
* Max remote probes/reads allowed to run concurrently across the whole process.
|
|
* Override with CODEMAN_MAX_REMOTE_FILE_SSH (clamped to >= 1). Four keeps a burst
|
|
* well under OpenSSH's ten-handshake default.
|
|
*/
|
|
const MAX_CONCURRENT_REMOTE_SSH = (() => {
|
|
const raw = Number(process.env.CODEMAN_MAX_REMOTE_FILE_SSH);
|
|
return Number.isFinite(raw) && raw >= 1 ? Math.floor(raw) : 4;
|
|
})();
|
|
|
|
let active = 0;
|
|
const waiters: Array<() => void> = [];
|
|
|
|
/** Test/diagnostic hook: remote calls currently holding a slot. */
|
|
export function getActiveRemoteSshCount(): number {
|
|
return active;
|
|
}
|
|
|
|
/** Test/diagnostic hook: remote calls queued behind the cap. */
|
|
export function getQueuedRemoteSshCount(): number {
|
|
return waiters.length;
|
|
}
|
|
|
|
/** The configured cap, so a test can assert against the real number. */
|
|
export function getRemoteSshLimit(): number {
|
|
return MAX_CONCURRENT_REMOTE_SSH;
|
|
}
|
|
|
|
function acquire(): Promise<void> {
|
|
if (active < MAX_CONCURRENT_REMOTE_SSH) {
|
|
active++;
|
|
return Promise.resolve();
|
|
}
|
|
return new Promise<void>((resolve) => waiters.push(resolve));
|
|
}
|
|
|
|
function release(): void {
|
|
const next = waiters.shift();
|
|
if (next) {
|
|
// Hand the slot straight to the next waiter; `active` stays at the cap.
|
|
next();
|
|
} else {
|
|
active--;
|
|
}
|
|
}
|
|
|
|
/** Run `task` once an ssh slot is free, releasing the slot afterward. */
|
|
export async function runWithRemoteSshLimit<T>(task: () => Promise<T>): Promise<T> {
|
|
await acquire();
|
|
try {
|
|
return await task();
|
|
} finally {
|
|
release();
|
|
}
|
|
}
|