Skip to content

Commit a764e19

Browse files
mchwangclaude
andauthored
Size the plan import request limit from the parser bound (#146) (#158)
* Size the plan import request limit from the parser bound (#146) POST /api/plan/import refused every body over 16 KiB, though the parser accepts a 1 MiB source and the v1 schema permits plans far larger than 16 KiB. The import body limit is now 6 x 1 MiB + 16 KiB: any JSON encoding writes each UTF-8 source byte as at most 6 bytes, so every source the parser accepts fits. Other POSTs keep 16 KiB. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Pin the other POST limits and the recorded parser refusal (#146 review) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent ae9fc3d commit a764e19

5 files changed

Lines changed: 92 additions & 6 deletions

File tree

‎core/parse-v1.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { isAlias, isMap, isScalar, isSeq, parseDocument } from 'yaml';
22

3-
const MAX_BYTES = 1024 * 1024;
3+
/** The largest plan source import parses, in UTF-8 bytes. The import request limit (web/server.ts) is derived from it. */
4+
export const MAX_SOURCE_BYTES = 1024 * 1024;
45
const number = /^-?(?:0|[1-9][0-9]*)(?:\.[0-9]+)?(?:[eE][+-]?[0-9]+)?$/u;
56
function reject(message: string): never { throw new Error(message); }
67
function numeric(text: string): number {
@@ -21,9 +22,9 @@ function numeric(text: string): number {
2122
}
2223
/** Frozen v1 syntax contract. No runtime I/O or alias/object construction. */
2324
export function parseV1(input: string | Uint8Array, format: 'json' | 'yaml'): unknown {
24-
if (typeof input === 'string' && input.length > MAX_BYTES) reject('Input size exceeds 1 MiB.');
25+
if (typeof input === 'string' && input.length > MAX_SOURCE_BYTES) reject('Input size exceeds 1 MiB.');
2526
const bytes = typeof input === 'string' ? new TextEncoder().encode(input) : input;
26-
if (bytes.byteLength > MAX_BYTES) reject('Input size exceeds 1 MiB.');
27+
if (bytes.byteLength > MAX_SOURCE_BYTES) reject('Input size exceeds 1 MiB.');
2728
const source = new TextDecoder('utf-8', { fatal: true, ignoreBOM: true }).decode(bytes);
2829
if (typeof input === 'string' && input !== source) reject('Invalid UTF-8 source string.');
2930
if (format === 'json') return parseJson(source);

‎docs/implementation/runner-lifecycle.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -358,7 +358,7 @@ F1 owns the production planning endpoints that G4 needs. They wrap E3's coordina
358358

359359
| Endpoint | Store or coordinator call | Guard |
360360
|---|---|---|
361-
| `POST /api/plan/import` | `Store.importRevision` | expected revision |
361+
| `POST /api/plan/import` | `Store.importRevision` | expected revision; body of at most 6 MiB + 16 KiB, so every source the 1 MiB parser accepts fits (see `docs/plan-format.md`, #146) |
362362
| `POST /api/plan/suggestions` | E3 coordinator start (`beginSuggestions`) | expected revision and snapshot |
363363
| `GET /api/plan/suggestions/:id` | `Store.getSuggestions`, with the request's `mode` | reads only the request row; refuses a draft's ID ("Unknown suggestion request.") |
364364
| `POST /api/plan/suggestions/:id/cancel` | `pending`: `handle.cancel(reason)` on the `SuggestionHandle` that `SuggestionCoordinator.start()` returned. E3's coordinator has no cancel-by-ID method, so F keeps a map from request ID to handle, adds each handle when it starts the request, and removes it only after the handle's outcome settles. If the request is `pending` but F holds no handle, F calls `Store.cancelSuggestions`. After a restart this no longer arises while planning is on (a non-demo review with a `github` block): startup fails every request an earlier process left `pending` (#124). Without planning, such a request stays `pending` until it is cancelled here. It is safe to do so because a crashed process's planning container has no route back to the Store: its output went to that process's worker, which ended with it. A live planning provider runs only through D (#117), in planning's own worker, in containers labelled with this database's planning owner (not the runner's or Ask's). Like Ask's, those containers are removed by D's `recoverLeftovers` for that owner before the first planning request of the next process, not at startup; until then a stray container can keep running read-only (its deadline is enforced from the host, which is gone) until Claude exits or that recovery removes it, and its output is discarded. If a planning provider ever runs outside D, or could report into the Store from another process, this endpoint must refuse such a request instead, until it has been reconciled. `ready`: `Store.cancelSuggestions` (dismiss), because no invocation is running. | request is `pending` or `ready`, as `Store.cancelSuggestions` already allows |

‎docs/plan-format.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,8 @@ questions: []
9595
9696
Import accepts UTF-8 JSON or one YAML 1.2 document representing JSON-compatible data. Reject invalid UTF-8, input above 1 MiB, and nesting deeper than 50 containers before producing a plan. Parse into an intermediate syntax tree with duplicate-key detection; do not convert an unchecked YAML graph into application objects.
9797
98+
`POST /api/plan/import` sends the source as one JSON string in a request body of at most 6 MiB + 16 KiB (6,307,840 bytes, `MAX_IMPORT_REQUEST_BYTES` in `web/server.ts`). Any JSON encoding writes each UTF-8 byte of the source as at most 6 bytes: YAML accepts a raw control character in a quoted scalar, and JSON writes it as `\u0001`. So every source within the 1 MiB parser limit fits, with 16 KiB left for the rest of the request. A larger body is refused with HTTP 413 before it is parsed. A body within the limit whose source is above 1 MiB gets the parser's refusal. Every other `POST` keeps its 16 KiB limit (#146).
99+
98100
For both formats, object keys must be strings and unique after decoding at every level; reject duplicates rather than choosing the first or last value. JSON follows RFC 8259 syntax. YAML allows mappings, sequences, strings (including quoted and block strings), the exact plain literals `true`, `false`, and `null`, and numbers spelled using the JSON number grammar. Reject empty implicit values, non-finite values (`.nan`/`.inf`), non-JSON number spellings such as hex/octal or numeric separators, complex/non-string keys, every anchor and alias, merge keys (`<<`), explicit tags (including custom tags), extra documents, and parser warnings/errors before schema validation. Ordinary YAML 1.2 plain strings remain strings; do not infer dates, functions, or application-specific types. Integers used for issue, revision, or indexes must also be exactly representable safe integers in the implementation.
99101

100102
Only after these checks convert to JSON-compatible values, select the registered schema, and validate. Never enable alias expansion or custom object construction. Required fixtures: equivalent JSON/YAML produce identical structured plans; duplicate decoded keys in either format, anchors/aliases, merge keys, tags, non-finite/non-JSON numbers, empty values, complex keys, multiple documents, excessive depth, and oversized input all fail before schema validation. An ordinary quoted string containing `<<` or `&` as part of its text is data, not YAML syntax.

‎test/runner-planning-feedback.test.ts‎

Lines changed: 72 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,8 @@ import { createHash, randomUUID } from 'node:crypto';
77
import { choiceKeys } from '../core/approvals.ts';
88
import { afterEach, describe, expect, it, vi } from 'vitest';
99
import { createDemo } from '../scripts/demo.ts';
10-
import { startServer, type PlanningDeps } from '../web/server.ts';
10+
import { MAX_IMPORT_REQUEST_BYTES, MAX_REQUEST_BYTES, startServer, type PlanningDeps } from '../web/server.ts';
11+
import { MAX_SOURCE_BYTES } from '../core/parse-v1.ts';
1112
import { Store } from '../runner/store.ts';
1213
import { ReviewService } from '../runner/review.ts';
1314
import { fixtureGit } from './fixtures/git.ts';
@@ -144,6 +145,76 @@ describe('planning API for lane G', () => {
144145
expect((await review(app)).plan.revision).toBe(view.plan.revision + 1);
145146
expect((await api(app, 'POST', '/api/plan/import', { ...request, actionId: randomUUID() })).status).toBe(409);
146147
});
148+
describe('import request size (#146)', () => {
149+
/** The plan with items added up to `count` items. Each new item adds files whose change text is `change`. */
150+
const grown = (plan: Record<string, any>, count: number, change: string, files = 40) => ({ ...plan, items: [...plan.items,
151+
...Array.from({ length: count - plan.items.length }, (_, i) => {
152+
const id = `P${plan.items.length + i + 1}`;
153+
return { id, title: `Item ${id}`, intent: 'Fill the plan.', depends_on: [], acceptance: [{ type: 'check', text: 'Covered.' }],
154+
files: Array.from({ length: files }, (_, k) => ({ path: `import-limit/${id}/f${k}.txt`, kind: 'add', renamed_from: null, change })) };
155+
})] });
156+
const post = async (app: App, body: string) => {
157+
const response = await fetch(`${new URL(app.url).origin}/api/plan/import`, { method: 'POST', body,
158+
headers: { 'x-codeboost-token': app.token, 'content-type': 'application/json' } });
159+
return { status: response.status, body: await response.json() as Record<string, any> };
160+
};
161+
const envelope = (source: string, revision: number) => JSON.stringify({ source, format: 'json', expectedRevision: revision, actionId: randomUUID() });
162+
163+
it('imports a schema-valid plan whose request is larger than the 16 KiB other requests get', async () => {
164+
const { app } = await serve();
165+
const view = await review(app);
166+
const body = envelope(JSON.stringify(grown(view.plan, 10, 'Write the generated file.')), view.plan.revision);
167+
expect(Buffer.byteLength(body)).toBeGreaterThan(MAX_REQUEST_BYTES);
168+
expect(await post(app, body)).toEqual({ status: 200, body: { result: { revision: view.plan.revision + 1 } } });
169+
expect((await review(app)).plan.items).toHaveLength(10);
170+
});
171+
it('imports a 1 MiB YAML source that escapes to almost six times its size, the most JSON can expand it', async () => {
172+
const { app } = await serve();
173+
const view = await review(app);
174+
// YAML takes a raw control character in a quoted scalar; JSON writes each one as \u0001.
175+
const yaml = (plan: unknown) => `%YAML 1.2\n---\n${JSON.stringify(plan).replaceAll('\\u0001', '\u0001')}\n`;
176+
let files = 1;
177+
while (Buffer.byteLength(yaml(grown(view.plan, 30, '\u0001'.repeat(1200), files + 1))) < MAX_SOURCE_BYTES) files++;
178+
const unpadded = yaml(grown(view.plan, 30, '\u0001'.repeat(1200), files));
179+
const source = `${unpadded}#${'x'.repeat(MAX_SOURCE_BYTES - Buffer.byteLength(unpadded) - 1)}`;
180+
expect(Buffer.byteLength(source)).toBe(MAX_SOURCE_BYTES);
181+
const body = JSON.stringify({ source, format: 'yaml', expectedRevision: view.plan.revision, actionId: randomUUID() });
182+
expect(Buffer.byteLength(body)).toBeGreaterThan(5.5 * MAX_SOURCE_BYTES);
183+
expect(Buffer.byteLength(body)).toBeLessThanOrEqual(MAX_IMPORT_REQUEST_BYTES);
184+
expect(await post(app, body)).toEqual({ status: 200, body: { result: { revision: view.plan.revision + 1 } } });
185+
expect((await review(app)).plan.items[29].files[0].change).toBe('\u0001'.repeat(1200));
186+
});
187+
it('passes a request at the limit to the parser, which refuses a source over 1 MiB', async () => {
188+
const { app } = await serve();
189+
const view = await review(app), empty = Buffer.byteLength(envelope('', view.plan.revision));
190+
const body = envelope('x'.repeat(MAX_IMPORT_REQUEST_BYTES - empty), view.plan.revision);
191+
expect(Buffer.byteLength(body)).toBe(MAX_IMPORT_REQUEST_BYTES);
192+
const response = await post(app, body);
193+
expect(response).toEqual({ status: 409, body: { error: expect.stringMatching(/exceeds 1 MiB/) } });
194+
// The refusal is recorded like any other import refusal: the same request replays it.
195+
expect(await post(app, body)).toEqual(response);
196+
expect((await review(app)).plan.revision).toBe(view.plan.revision);
197+
});
198+
it('refuses a request one byte over the limit before reading it as JSON, and imports nothing', async () => {
199+
const { app } = await serve();
200+
const view = await review(app), empty = Buffer.byteLength(envelope('', view.plan.revision));
201+
const body = envelope('x'.repeat(MAX_IMPORT_REQUEST_BYTES - empty + 1), view.plan.revision);
202+
expect(Buffer.byteLength(body)).toBe(MAX_IMPORT_REQUEST_BYTES + 1);
203+
expect(await post(app, body)).toEqual({ status: 413, body: { error: 'Request too large.' } });
204+
expect((await review(app)).plan.revision).toBe(view.plan.revision);
205+
});
206+
it('keeps the 16 KiB limit for every other request, the other planning requests included', async () => {
207+
const { app } = await serve();
208+
const body = JSON.stringify({ pad: 'x'.repeat(MAX_REQUEST_BYTES - 9) });
209+
expect(Buffer.byteLength(body)).toBe(MAX_REQUEST_BYTES + 1);
210+
for (const path of ['/api/action', '/api/settings', '/api/issues', '/api/runner', '/api/plan/suggestions', '/api/plan/drafts',
211+
`/api/plan/suggestions/${randomUUID()}/apply`, `/api/plan/drafts/${randomUUID()}/cancel`]) {
212+
const response = await fetch(`${new URL(app.url).origin}${path}`, { method: 'POST', body,
213+
headers: { 'x-codeboost-token': app.token, 'content-type': 'application/json' } });
214+
expect([path, response.status]).toEqual([path, 413]);
215+
}
216+
});
217+
});
147218
it('starts, reads, and applies a suggestion exactly once', async () => {
148219
const { deps, calls } = planning();
149220
const { app } = await serve({ planning: deps });

‎web/server.ts‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import { IssueBoard } from './issues.ts';
2020
import { SuggestionCoordinator, type PlanningMode, type SuggestionHandle, type SuggestionInput, type SuggestionStore } from '../core/planning-suggestions.ts';
2121
import type { AuthorProvider } from '../core/planning-author.ts';
2222
import { PLANNING_BUDGET_MS } from '../runner/planning-provider.ts';
23+
import { MAX_SOURCE_BYTES } from '../core/parse-v1.ts';
2324
import type { PreMergeCoordinator } from '../runner/pre-merge.ts';
2425
export type PlanningDescription = Pick<SuggestionInput, 'issue' | 'approvedLessons'> & {
2526
repo: { name: string; baseRef: string };
@@ -44,6 +45,16 @@ export const PLANNING_SHUTDOWN_GRACE_MS = 20_000;
4445
/** Production planning is built after the Store opens, from the review it serves (see web/cli.ts). */
4546
export type PlanningSetup = (service: ReviewService) => PlanningDeps;
4647
const publicRoot = new URL('./public/', import.meta.url);
48+
/** The largest body of a POST request other than plan import, in bytes. */
49+
export const MAX_REQUEST_BYTES = 16 * 1024;
50+
/**
51+
* The largest body of `POST /api/plan/import`, in bytes (#146). The body carries the plan source as one JSON string.
52+
* Any JSON encoding writes each UTF-8 byte of that source as at most 6 bytes (`\u0001` for a control character, which
53+
* YAML accepts in a scalar). So the limit admits every source within the parser's limit, inside any envelope that
54+
* fits MAX_REQUEST_BYTES with an empty source. A body within the limit whose source is above the parser's limit gets
55+
* the parser's refusal, not a transport one.
56+
*/
57+
export const MAX_IMPORT_REQUEST_BYTES = 6 * MAX_SOURCE_BYTES + MAX_REQUEST_BYTES;
4758
/** The longest shutdown waits for admitted requests to finish before aborting them; below the 15 s request timeout. */
4859
export const MAX_SHUTDOWN_DRAIN_MS = 14_500;
4960
/**
@@ -589,8 +600,9 @@ export async function startServer(config: ReviewConfig, port = 4318, questionAge
589600
const planningPath = path === '/api/plan/import' || PLANNING_REQUEST.test(path);
590601
if (req.method !== 'POST' || !(['/api/action','/api/settings','/api/issues','/api/runner'].includes(path) || planningPath) || req.headers['content-type'] !== 'application/json') { json(405, { error: 'Unsupported request.' }); return; }
591602
const chunks: Buffer[] = []; let size = 0;
603+
const limit = path === '/api/plan/import' ? MAX_IMPORT_REQUEST_BYTES : MAX_REQUEST_BYTES;
592604
activeRequest.readingBody=true;
593-
try { for await (const chunk of req) { size += chunk.length; if (size > 16384) { json(413, { error: 'Request too large.' }); return; } chunks.push(chunk); } }
605+
try { for await (const chunk of req) { size += chunk.length; if (size > limit) { json(413, { error: 'Request too large.' }); return; } chunks.push(chunk); } }
594606
finally { activeRequest.readingBody=false; }
595607
const body = new TextDecoder('utf-8', { fatal: true }).decode(Buffer.concat(chunks));
596608
const input=JSON.parse(body);

0 commit comments

Comments
 (0)