mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-09-30 12:39:42 +02:00
fix(input): an oversized paste no longer poisons the durable input queue (#484)
A single input over MAX_INPUT_LENGTH (64 KiB) was queued for reliable
delivery, refused by both transports (the WebSocket silently, POST with a
400), and never dropped: the client treated the 400 as transient, so the
frame was re-sent every 2 s forever, blocked every later input for that
session, and came back from localStorage on every reload.
- Client: a paste over the frame limit is split into in-limit frames
(never cutting a surrogate pair) delivered in seq order; over 1 MiB, or
an oversized mux write, it is refused with a toast and never queued.
- Client: the POST drain drops a frame answered 400/413; a WS error ACK
drops it too; frames over the limit persisted by an older build are
pruned on load.
- Server: the WebSocket answers an oversized sequenced frame with
{t:'ia',seq,err:'too_large',max} instead of silence (an older client
reads that as a plain ACK and drops it); the POST schema uses
MAX_INPUT_LENGTH instead of a second 100000 limit.
Verified end to end on an isolated instance: a 110 KB paste reached the
PTY byte-identical over both the WebSocket and the POST path, a poisoned
120 KB persisted frame was pruned on load, and a 2 MB paste showed the
refusal toast with nothing queued.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -66,6 +66,31 @@ each `(clientId, seq)` at most once, so a resend can't type the prompt twice.
|
||||
(the 200 is the client's ACK). `curl`/legacy callers omit the fields and always
|
||||
apply.
|
||||
|
||||
## Oversized input (issue #484)
|
||||
|
||||
Delivery has a third outcome besides "applied" and "retry": **refused for good**.
|
||||
Both transports refuse a frame longer than `MAX_INPUT_LENGTH` (64 KiB,
|
||||
`src/config/terminal-limits.ts`; the POST schema uses the same constant). Before
|
||||
#484 the client treated that like a transient failure, so an oversized paste sat
|
||||
at the head of the queue, was re-sent every 2 s forever, blocked every later
|
||||
input for the session, and came back from localStorage on each reload.
|
||||
|
||||
- `_sendInputAsync()` splits a paste over the frame limit into in-limit frames
|
||||
(`CodemanInputLimit.split`, constants.js, never cutting a surrogate pair). They
|
||||
go out in seq order, so the PTY sees one contiguous stream. A paste over
|
||||
`PASTE_MAX_CHARS` (1 MiB), or an oversized `useMux` write (line-oriented, never
|
||||
split), is refused with a toast and never queued.
|
||||
- The WebSocket answers an oversized sequenced frame with
|
||||
`{t:'ia', seq, err:'too_large', max}`; the client drops it with a toast. A
|
||||
client that predates `err` reads it as a plain ACK and drops it too.
|
||||
- The POST drain drops a frame answered `400`/`413` (`401`/`403` stay transient:
|
||||
an expired login delivers once the user signs in again).
|
||||
- `_loadReliableState()` prunes persisted frames over the limit, so a queue
|
||||
poisoned by an older build heals on the first load after upgrading.
|
||||
- ⚠️ The frontend limit (`INPUT_FRAME_MAX_CHARS`) and the composer's
|
||||
`COMPOSER_INPUT_FRAME_LIMIT` must equal `MAX_INPUT_LENGTH`; pinned by
|
||||
`test/input-size-limit.test.ts`.
|
||||
|
||||
## Known limitation
|
||||
|
||||
Dedup state is in-memory on the server. A **server restart** between a write and
|
||||
@@ -79,3 +104,5 @@ across the narrow restart window.
|
||||
semantics (monotonic, per-client, gap-tolerant, eviction-safe).
|
||||
- `test/routes/session-routes.test.ts` — POST `/input` applies a tagged
|
||||
`(clientId, seq)` once on redelivery; untagged input always applies.
|
||||
- `test/input-size-limit.test.ts`: one input limit on both sides, frame
|
||||
splitting, and dropping (never retrying) a frame refused for good (#484).
|
||||
|
||||
+47
-7
@@ -3427,7 +3427,26 @@ class CodemanApp {
|
||||
*/
|
||||
_sendInputAsync(sessionId, input, opts) {
|
||||
if (!sessionId || !input) return;
|
||||
this._reliableSend(sessionId, input, opts?.useMux === true);
|
||||
const useMux = opts?.useMux === true;
|
||||
// Both transports refuse a frame over the server's limit (issue #484), and a
|
||||
// refused frame used to sit at the head of the durable queue for good. So an
|
||||
// oversized paste goes out as several in-limit frames, delivered in seq order
|
||||
// as one contiguous stream. A mux write is line-oriented (it strips newlines
|
||||
// and sends Enter on its own), so it is never split: refuse it instead.
|
||||
const limit = window.CodemanInputLimit;
|
||||
if (limit && input.length > limit.FRAME_MAX_CHARS) {
|
||||
if (useMux || input.length > limit.PASTE_MAX_CHARS) {
|
||||
const max = useMux ? limit.FRAME_MAX_CHARS : limit.PASTE_MAX_CHARS;
|
||||
this.showToast?.(
|
||||
`Input too large (${Math.ceil(input.length / 1024)} KB, limit ${Math.floor(max / 1024)} KB); not sent`,
|
||||
'error'
|
||||
);
|
||||
return;
|
||||
}
|
||||
for (const frame of limit.split(input)) this._reliableSend(sessionId, frame, false);
|
||||
return;
|
||||
}
|
||||
this._reliableSend(sessionId, input, useMux);
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -3547,6 +3566,12 @@ class CodemanApp {
|
||||
// Session no longer exists — the input can never land. Drop it
|
||||
// rather than retry forever (not a "lost" prompt: the target is gone).
|
||||
this._ackDelivery(sessionId, rec.seq);
|
||||
} else if (resp && (resp.status === 400 || resp.status === 413)) {
|
||||
// The frame itself was refused, so a retry gets the same answer. Kept
|
||||
// queued, it was re-POSTed every 2 s forever and blocked every later
|
||||
// input for this session behind it (issue #484). 401/403 stay
|
||||
// transient: an expired login delivers fine once the user signs in.
|
||||
this._dropRejectedInput(sessionId, rec);
|
||||
} else {
|
||||
break; // offline / 5xx — leave queued; sweep + reconnect retry later
|
||||
}
|
||||
@@ -3557,6 +3582,12 @@ class CodemanApp {
|
||||
})();
|
||||
}
|
||||
|
||||
/** Drop a frame the server refused for good, and say so once. */
|
||||
_dropRejectedInput(sessionId, rec) {
|
||||
this._ackDelivery(sessionId, rec.seq);
|
||||
this.showToast?.(`Input refused by the server (${Math.ceil(rec.data.length / 1024)} KB); not sent`, 'error');
|
||||
}
|
||||
|
||||
/** Drop an ACKed record (by exact seq) and persist. */
|
||||
_ackDelivery(sessionId, seq) {
|
||||
const list = this._pendingDeliveries.get(sessionId);
|
||||
@@ -3601,6 +3632,13 @@ class CodemanApp {
|
||||
_onWsInputAck(seq, msg) {
|
||||
const sessionId = this._wsSessionId;
|
||||
if (!sessionId || !Number.isInteger(seq)) return;
|
||||
if (msg && msg.err) {
|
||||
// Refused for good (e.g. over the size limit): retrying cannot help.
|
||||
const rec = (this._pendingDeliveries.get(sessionId) || []).find((r) => r.seq === seq);
|
||||
if (rec) this._dropRejectedInput(sessionId, rec);
|
||||
else this._ackDelivery(sessionId, seq);
|
||||
return;
|
||||
}
|
||||
if (msg && msg.dup) {
|
||||
const list = this._pendingDeliveries.get(sessionId);
|
||||
const rec = list && list.find((r) => r.seq === seq);
|
||||
@@ -3714,11 +3752,13 @@ class CodemanApp {
|
||||
if (saved && saved.pending) {
|
||||
for (const [s, recs] of Object.entries(saved.pending)) {
|
||||
if (Array.isArray(recs) && recs.length) {
|
||||
// Reset sentAt so they re-deliver promptly on this fresh load.
|
||||
this._pendingDeliveries.set(
|
||||
s,
|
||||
recs
|
||||
const frameMax = window.CodemanInputLimit?.FRAME_MAX_CHARS ?? Infinity;
|
||||
const kept = recs
|
||||
.filter((r) => r && typeof r.data === 'string' && Number.isInteger(r.seq))
|
||||
// A frame over the server's limit can never be ACKed; one persisted
|
||||
// by an older build would otherwise come back on every load (#484).
|
||||
.filter((r) => r.data.length <= frameMax)
|
||||
// Reset sentAt so they re-deliver promptly on this fresh load.
|
||||
.map((r) => ({
|
||||
seq: r.seq,
|
||||
data: r.data,
|
||||
@@ -3726,8 +3766,8 @@ class CodemanApp {
|
||||
ts: r.ts || Date.now(),
|
||||
tries: 0,
|
||||
sentAt: 0,
|
||||
}))
|
||||
);
|
||||
}));
|
||||
if (kept.length) this._pendingDeliveries.set(s, kept);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -762,6 +762,49 @@ function resolveTerminalFontWeights(settings) {
|
||||
// without a terminal, a clipboard, or a browser.
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
/**
|
||||
* Largest single input frame the server accepts, in UTF-16 code units.
|
||||
* ⚠️ Must equal MAX_INPUT_LENGTH in src/config/terminal-limits.ts (pinned by
|
||||
* test/input-size-limit.test.ts). Both transports reject a longer frame, and
|
||||
* before issue #484 the durable input queue retried such a frame forever.
|
||||
*/
|
||||
const INPUT_FRAME_MAX_CHARS = 64 * 1024;
|
||||
|
||||
/**
|
||||
* Largest paste the client will deliver at all. Anything up to this is split
|
||||
* into INPUT_FRAME_MAX_CHARS frames that go out in seq order, so the PTY sees
|
||||
* one contiguous byte stream (bracketed-paste markers included). Past it the
|
||||
* input is refused with a toast rather than queued: every frame is persisted
|
||||
* and retried until ACKed, so a multi-megabyte paste would pin the queue.
|
||||
*/
|
||||
const INPUT_PASTE_MAX_CHARS = 1024 * 1024;
|
||||
|
||||
/**
|
||||
* Split input into frames no longer than `max` code units, never cutting a
|
||||
* surrogate pair in half (a lone surrogate reaches the PTY as U+FFFD).
|
||||
*
|
||||
* @param {string} data
|
||||
* @param {number} [max]
|
||||
* @returns {string[]}
|
||||
*/
|
||||
function splitInputFrames(data, max = INPUT_FRAME_MAX_CHARS) {
|
||||
if (typeof data !== 'string' || data.length === 0) return [];
|
||||
if (!(max >= 2)) max = 2;
|
||||
if (data.length <= max) return [data];
|
||||
const frames = [];
|
||||
let start = 0;
|
||||
while (start < data.length) {
|
||||
let end = Math.min(start + max, data.length);
|
||||
if (end < data.length) {
|
||||
const code = data.charCodeAt(end - 1);
|
||||
if (code >= 0xd800 && code <= 0xdbff) end--; // keep the pair together
|
||||
}
|
||||
frames.push(data.slice(start, end));
|
||||
start = end;
|
||||
}
|
||||
return frames;
|
||||
}
|
||||
|
||||
/**
|
||||
* Upper bound on an AUTO-copied selection.
|
||||
*
|
||||
@@ -940,6 +983,11 @@ if (typeof window !== 'undefined') {
|
||||
compare: compareSessionActivity,
|
||||
sort: sortSessionsByActivity,
|
||||
};
|
||||
window.CodemanInputLimit = {
|
||||
FRAME_MAX_CHARS: INPUT_FRAME_MAX_CHARS,
|
||||
PASTE_MAX_CHARS: INPUT_PASTE_MAX_CHARS,
|
||||
split: splitInputFrames,
|
||||
};
|
||||
window.CodemanAutoCopy = {
|
||||
decide: decideAutoCopy,
|
||||
MAX_CHARS: AUTO_COPY_MAX_CHARS,
|
||||
|
||||
@@ -175,7 +175,16 @@ export function registerWsRoutes(app: FastifyInstance, ctx: SessionPort, getHost
|
||||
try {
|
||||
const msg = JSON.parse(String(raw));
|
||||
if (msg.t === 'i' && typeof msg.d === 'string') {
|
||||
if (msg.d.length > MAX_INPUT_LENGTH) return;
|
||||
if (msg.d.length > MAX_INPUT_LENGTH) {
|
||||
// Refused for good, so say so: a silent return left the frame
|
||||
// unACKed and the client redelivered it every few seconds forever
|
||||
// (issue #484). A client that predates `err` reads this as a plain
|
||||
// ACK and drops the frame, which is also the right outcome.
|
||||
if (Number.isInteger(msg.seq) && socket.readyState === 1) {
|
||||
socket.send(`{"t":"ia","seq":${msg.seq as number},"err":"too_large","max":${MAX_INPUT_LENGTH}}`);
|
||||
}
|
||||
return;
|
||||
}
|
||||
// Reliable delivery: when the frame carries a clientId + seq, apply it
|
||||
// exactly once (skip a duplicate redelivery) but ACK it regardless so
|
||||
// the client can drop it from its durable queue. Frames without seq
|
||||
|
||||
+5
-1
@@ -20,6 +20,7 @@ import {
|
||||
import { MAX_EDITABLE_BYTES } from '../config/file-editing.js';
|
||||
import { MIN_MATCH_LENGTH, MAX_MATCH_LENGTH } from '../config/agent-wait.js';
|
||||
import { MAX_WAKE_MACS } from '../config/remote-wake-limits.js';
|
||||
import { MAX_INPUT_LENGTH } from '../config/terminal-limits.js';
|
||||
import { enabledCliIds, enabledClis } from '../config/cli-registry/registry.js';
|
||||
import type { SessionMode } from '../types.js';
|
||||
|
||||
@@ -1500,7 +1501,10 @@ export const SettingsUpdateSchema = z
|
||||
* Schema for POST /api/sessions/:id/input with length limit
|
||||
*/
|
||||
export const SessionInputWithLimitSchema = z.object({
|
||||
input: z.string().max(100000), // 100KB max input
|
||||
// One limit for both transports (issue #484): the route's own length check and
|
||||
// ws-routes.ts read the same constant, so a schema cap above it only hid which
|
||||
// check refused the input.
|
||||
input: z.string().max(MAX_INPUT_LENGTH),
|
||||
useMux: z.boolean().optional(),
|
||||
// Reliable-delivery dedup (optional; absent for curl/legacy clients). The web
|
||||
// client tags each input with a stable clientId + a monotonic per-session seq
|
||||
|
||||
@@ -0,0 +1,108 @@
|
||||
/**
|
||||
* @fileoverview Oversized input must never poison the durable input queue (#484).
|
||||
*
|
||||
* The failure: a paste over MAX_INPUT_LENGTH was queued for reliable delivery,
|
||||
* refused by both transports (the WebSocket silently, the POST with a 400), and
|
||||
* never dropped by the client, which treated a 400 like a transient failure. It
|
||||
* was re-sent every 2 s forever, blocked every later input for that session
|
||||
* behind it, and came back from localStorage on every reload.
|
||||
*
|
||||
* The fix, pinned here: the client splits a large paste into in-limit frames
|
||||
* (one limit, shared with the server), drops a frame the server refused for
|
||||
* good, and prunes oversized frames persisted by an older build.
|
||||
*/
|
||||
import { readFileSync } from 'node:fs';
|
||||
import { resolve } from 'node:path';
|
||||
import vm from 'node:vm';
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import { MAX_INPUT_LENGTH } from '../src/config/terminal-limits.js';
|
||||
import { SessionInputWithLimitSchema } from '../src/web/schemas.js';
|
||||
|
||||
const pub = (f: string) => readFileSync(resolve(import.meta.dirname, '../src/web/public', f), 'utf8');
|
||||
const appSource = pub('app.js');
|
||||
|
||||
type InputLimit = { FRAME_MAX_CHARS: number; PASTE_MAX_CHARS: number; split: (d: string, max?: number) => string[] };
|
||||
|
||||
function loadLimit(): InputLimit {
|
||||
const context = vm.createContext({ window: {}, globalThis: {} });
|
||||
vm.runInContext(pub('constants.js'), context, { filename: 'constants.js' });
|
||||
return (context.window as { CodemanInputLimit: InputLimit }).CodemanInputLimit;
|
||||
}
|
||||
|
||||
describe('one input limit on both sides', () => {
|
||||
it('the frontend frame limit equals the server MAX_INPUT_LENGTH', () => {
|
||||
expect(loadLimit().FRAME_MAX_CHARS).toBe(MAX_INPUT_LENGTH);
|
||||
});
|
||||
|
||||
it('the composer budget is derived from the same number', () => {
|
||||
expect(pub('keyboard-accessory.js')).toMatch(
|
||||
new RegExp(`COMPOSER_INPUT_FRAME_LIMIT = (64 \\* 1024|${MAX_INPUT_LENGTH});`)
|
||||
);
|
||||
expect(MAX_INPUT_LENGTH).toBe(64 * 1024);
|
||||
});
|
||||
|
||||
it('the POST schema caps input at MAX_INPUT_LENGTH, not a second number', () => {
|
||||
expect(SessionInputWithLimitSchema.safeParse({ input: 'x'.repeat(MAX_INPUT_LENGTH) }).success).toBe(true);
|
||||
expect(SessionInputWithLimitSchema.safeParse({ input: 'x'.repeat(MAX_INPUT_LENGTH + 1) }).success).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('splitInputFrames', () => {
|
||||
const { split } = loadLimit();
|
||||
|
||||
it('returns a short input as one frame and nothing for empty input', () => {
|
||||
expect(split('abc')).toEqual(['abc']);
|
||||
expect(split('')).toEqual([]);
|
||||
});
|
||||
|
||||
it('splits a large paste into in-limit frames that rejoin byte-identically', () => {
|
||||
const paste = '\x1b[200~' + 'log line\n'.repeat(15000) + '\x1b[201~';
|
||||
const frames = split(paste);
|
||||
expect(frames.length).toBeGreaterThan(1);
|
||||
for (const f of frames) expect(f.length).toBeLessThanOrEqual(MAX_INPUT_LENGTH);
|
||||
expect(frames.join('')).toBe(paste);
|
||||
});
|
||||
|
||||
it('never cuts a surrogate pair in half', () => {
|
||||
const paste = 'a' + '😀'.repeat(10); // pairs start at odd offsets
|
||||
const frames = split(paste, 4);
|
||||
expect(frames.join('')).toBe(paste);
|
||||
for (const f of frames) {
|
||||
const last = f.charCodeAt(f.length - 1);
|
||||
expect(last >= 0xd800 && last <= 0xdbff).toBe(false);
|
||||
const first = f.charCodeAt(0);
|
||||
expect(first >= 0xdc00 && first <= 0xdfff).toBe(false);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('the client never queues or keeps an undeliverable frame', () => {
|
||||
const sendAsync = appSource.slice(
|
||||
appSource.indexOf('_sendInputAsync(sessionId, input, opts) {'),
|
||||
appSource.indexOf('_sendInputEphemeral(sessionId, input) {')
|
||||
);
|
||||
|
||||
it('splits an oversized paste, and refuses an oversized mux write or giant paste with a toast', () => {
|
||||
expect(sendAsync).toContain('limit.split(input)');
|
||||
expect(sendAsync).toMatch(/useMux \|\| input\.length > limit\.PASTE_MAX_CHARS/);
|
||||
expect(sendAsync).toContain('showToast');
|
||||
});
|
||||
|
||||
it('drops a POST the server refused as invalid instead of retrying it forever', () => {
|
||||
const drain = appSource.slice(appSource.indexOf('_drainSession(sessionId) {'));
|
||||
const body = drain.slice(0, drain.indexOf('_dropRejectedInput(sessionId, rec) {'));
|
||||
expect(body).toMatch(
|
||||
/resp\.status === 400 \|\| resp\.status === 413\)\) \{\s*[\s\S]{0,400}this\._dropRejectedInput\(sessionId, rec\);/
|
||||
);
|
||||
});
|
||||
|
||||
it('drops a frame the WebSocket refused with an error ACK', () => {
|
||||
const handler = appSource.slice(appSource.indexOf('_onWsInputAck(seq, msg) {'));
|
||||
expect(handler.slice(0, 600)).toMatch(/if \(msg && msg\.err\)/);
|
||||
});
|
||||
|
||||
it('prunes oversized frames persisted by an older build on load', () => {
|
||||
const load = appSource.slice(appSource.indexOf('_loadReliableState() {'));
|
||||
expect(load.slice(0, 3000)).toMatch(/r\.data\.length <= frameMax/);
|
||||
});
|
||||
});
|
||||
@@ -282,6 +282,22 @@ describe('ws-routes', () => {
|
||||
}
|
||||
});
|
||||
|
||||
it('refuses an oversized sequenced frame with an error ACK, so the client can drop it', async () => {
|
||||
// Issue #484: a silent return left the frame unACKed, and the client's
|
||||
// durable queue re-sent it every few seconds forever.
|
||||
const ws = await connectWs('/ws/sessions/ws-test-session/terminal');
|
||||
try {
|
||||
const session = ctx._session;
|
||||
const hugeInput = 'y'.repeat(MAX_INPUT_LENGTH + 1);
|
||||
ws.send(JSON.stringify({ t: 'i', d: hugeInput, cid: 'c1', seq: 3 }));
|
||||
|
||||
expect(await nextMessage(ws)).toEqual({ t: 'ia', seq: 3, err: 'too_large', max: MAX_INPUT_LENGTH });
|
||||
expect(session.writeBuffer).not.toContain(hugeInput);
|
||||
} finally {
|
||||
ws.close();
|
||||
}
|
||||
});
|
||||
|
||||
it('ignores malformed JSON messages', async () => {
|
||||
const ws = await connectWs('/ws/sessions/ws-test-session/terminal');
|
||||
try {
|
||||
|
||||
Reference in New Issue
Block a user