diff --git a/docs/agent-environments.md b/docs/agent-environments.md index 768fd57cd1..46dff7a05a 100644 --- a/docs/agent-environments.md +++ b/docs/agent-environments.md @@ -202,7 +202,9 @@ grafana-example/ tasks/smoke.json deploy_env (local sandbox), then env_outcome_verifier ``` -The command reads each file once, then authenticates and cross-checks the +The command reads each file once, after checking that reading and parsing the +whole file fits the process's live heap headroom (a file that does not is +refused as a `resource_constraint` before it is read), then authenticates and cross-checks the pair as `environment-serve` does, so a manifest projected from another capture is refused with the same `invalid_request` before anything is written, and the bytes it copies are the bytes it authenticated. The bundle directory must be @@ -219,7 +221,8 @@ framework maps for an MCP server env; the framework's own deploy supplies `MCP_HOST`, `MCP_PORT` and `ENVIRONMENT_NAME`, so the served card carries the env's name. It installs Debian's `python3` because the framework's local sandbox probes a container's readiness with `python3` inside it, which the slim -Node image lacks. The image runs as the image's non-root `node` user and starts +Node image lacks. It copies the two inputs owned by the image's non-root `node` +user, so a private host umask cannot leave them unreadable, runs as that user and starts `rea environment-serve /env/manifest.json --evidence /env/evidence.json`. The `env.toml` names the environment because the framework otherwise reads the name from Python sources the image does not have. @@ -227,8 +230,10 @@ name from Python sources the image does not have. `verify.py` is the framework's `async def verify(mcp_url)` shape over its Python MCP client: it lists the tools and checks the count equals the manifest's, calls the first `GET` tool without path parameters with no -arguments and checks the replayed `status` equals the recorded status of that -tool's first cited record, and returns one `replay` result whose description +arguments and checks the call did not fail and its replayed `status` equals the +recorded status of that tool's first cited record (an unknown recorded status +stays unknown; it is not borrowed from another record), and returns one +`replay` result whose description reports both checks. A manifest with no such tool yields a verifier that checks the count only and says so. `tasks/smoke.json` deploys the env on the local sandbox and runs that verifier against the env's MCP endpoint; the first run diff --git a/src/application/JsonInputFile.ts b/src/application/JsonInputFile.ts index 25e5fcec02..27b4d511d6 100644 --- a/src/application/JsonInputFile.ts +++ b/src/application/JsonInputFile.ts @@ -33,13 +33,17 @@ const PACKED_TOKEN_PART_CODE_UNITS = 64 * 1024; // This selects an implementation; larger files remain accepted through streaming. const NATIVE_PARSE_READ_BUDGET = 8 * 1024 * 1024; -/** Keep native parsing only when input copies and value expansion fit live headroom. */ -const nativeParseFits = (inputBytes: number): boolean => - inputBytes <= NATIVE_PARSE_READ_BUDGET && +/** Heap a whole-file native JSON.parse may need: the assembler's 64x container expansion plus byte and text copies. */ +export const nativeJsonParseRequiredBytes = (inputBytes: number): number => // The assembler estimate reserves up to 128 bytes per two-byte container. // Allow that 64x expansion plus byte/text copies before native JSON.parse, // whose individual container allocations cannot be intercepted. - inputBytes * 72 + READ_CHUNK_BYTES <= + inputBytes * 72 + READ_CHUNK_BYTES; + +/** Keep native parsing only when input copies and value expansion fit live headroom. */ +const nativeParseFits = (inputBytes: number): boolean => + inputBytes <= NATIVE_PARSE_READ_BUDGET && + nativeJsonParseRequiredBytes(inputBytes) <= getHeapStatistics().total_available_size; /** One selected root member; repeated keys begin again with last-value semantics. */ diff --git a/src/application/environment/environmentBundle.test.ts b/src/application/environment/environmentBundle.test.ts index 5482416164..69d85a1348 100644 --- a/src/application/environment/environmentBundle.test.ts +++ b/src/application/environment/environmentBundle.test.ts @@ -62,7 +62,9 @@ describe("planEnvironmentBundle", () => { expect(lines).toContain( `RUN npm install -g --ignore-scripts --no-fund --no-audit rea-agents@${PACKAGE_METADATA.version}`, ); - expect(lines).toContain("COPY manifest.json evidence.json /env/"); + expect(lines).toContain( + "COPY --chown=node:node manifest.json evidence.json /env/", + ); expect(lines).toContain("USER node"); expect(lines.at(-1)).toBe( 'CMD ["rea","environment-serve","/env/manifest.json","--evidence","/env/evidence.json"]', @@ -102,6 +104,9 @@ describe("planEnvironmentBundle", () => { expect(verifier).toContain("TOOL_COUNT = 19"); expect(verifier).toContain('TOOL = "get_annotations"'); expect(verifier).toContain("RECORDED_STATUS = 200"); + expect(verifier).toContain( + "passed = passed and not result.isError and status == RECORDED_STATUS", + ); expect(verifier).toContain("await session.list_tools()"); expect(verifier).toContain("await session.call_tool(TOOL, {})"); expect(verifier).toContain( @@ -180,3 +185,25 @@ describe("planEnvironmentBundle", () => { } }); }); + +describe("planEnvironmentBundle replay check", () => { + it("keeps the first record's unknown status instead of borrowing another record's", () => { + const source = environmentServeSource(); + const first = source.exchanges.get(25); + if (first === undefined) throw new Error("Fixture lacks record 25"); + const exchanges = new Map(source.exchanges); + exchanges.set(25, { ...first, status: null }); + const planned = planEnvironmentBundle( + { ...source, exchanges }, + { packageSource: registryPackageSource() }, + ); + if (!planned.ok) throw planned.error; + expect(planned.value.check).toMatchObject({ + recordOrdinal: 25, + status: null, + }); + expect( + file(planned.value, "artifacts/check_grafana-example/verify.py"), + ).toContain("RECORDED_STATUS = None"); + }); +}); diff --git a/src/application/environment/environmentBundle.ts b/src/application/environment/environmentBundle.ts index 5947f14ca3..45cf72af95 100644 --- a/src/application/environment/environmentBundle.ts +++ b/src/application/environment/environmentBundle.ts @@ -103,7 +103,8 @@ export const bundleDockerfile = (source: EnvironmentPackageSource): string => `ENV MCP_PORT=${String(BUNDLE_MCP_PORT)}`, READINESS_PROBE_LAYER, installLine(source), - "COPY manifest.json evidence.json /env/", + // Owned by the runtime user, so a private host umask cannot make the inputs unreadable. + "COPY --chown=node:node manifest.json evidence.json /env/", "USER node", 'CMD ["rea","environment-serve","/env/manifest.json","--evidence","/env/evidence.json"]', "", @@ -125,7 +126,9 @@ const replayCheck = ( return { tool: tool.name, recordOrdinal: ordinal, - status: exchange?.status ?? tool.statuses[0] ?? null, + // An exchange's own unknown status is what replay returns; fall back only when the record is absent. + status: + exchange === undefined ? (tool.statuses[0] ?? null) : exchange.status, }; }; @@ -169,7 +172,7 @@ export const bundleVerifier = ( ' description = f"listed {count} tools, expected {TOOL_COUNT}"', " if result is not None:", " status = None if result.isError else replayed_status(result)", - " passed = passed and status == RECORDED_STATUS", + " passed = passed and not result.isError and status == RECORDED_STATUS", ' description += f"; {TOOL} replayed status {status!r}, recorded {RECORDED_STATUS!r}"', " else:", ' description += "; no GET tool without path parameters to replay"', diff --git a/src/application/environment/environmentExport.test.ts b/src/application/environment/environmentExport.test.ts index 24338157ae..eb792603f0 100644 --- a/src/application/environment/environmentExport.test.ts +++ b/src/application/environment/environmentExport.test.ts @@ -1,7 +1,8 @@ import { mkdir, readdir, readFile, writeFile } from "node:fs/promises"; import { join } from "node:path"; +import * as v8 from "node:v8"; -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; import { environmentServeFixture, @@ -16,6 +17,8 @@ import { readEnvironmentExportInputs, } from "./environmentExport.js"; +vi.mock("node:v8", { spy: true }); + const listFiles = async (root: string): Promise => (await readdir(root, { recursive: true, withFileTypes: true })) .filter((entry) => entry.isFile()) @@ -60,6 +63,32 @@ describe("readEnvironmentExportInputs", () => { expect(inputs.value.evidence.value).toEqual(fixture.capture); }); + it("refuses an input whose whole read and parse would not fit live heap headroom, before reading it", async () => { + const root = await createTestTempDirectory("rea-environment-export-heap-"); + const files = await writeEnvironmentServeFiles( + root, + environmentServeFixture(), + ); + vi.mocked(v8.getHeapStatistics).mockReturnValueOnce({ + ...v8.getHeapStatistics(), + total_available_size: 1024, + }); + const refused = await readEnvironmentExportInputs( + files.manifestPath, + files.capturePath, + ); + expect(refused.ok).toBe(false); + if (refused.ok) return; + expect(refused.error).toMatchObject({ + _tag: "AnalysisResourceConstraintError", + resource: "memory", + reportedLimits: { + input_path: files.manifestPath, + available_heap_bytes: 1024, + }, + }); + }); + it("refuses inline JSON, a directory and a file that is not JSON by role", async () => { const root = await createTestTempDirectory("rea-environment-inputs-bad-"); const files = await writeEnvironmentServeFiles(root); diff --git a/src/application/environment/environmentExport.ts b/src/application/environment/environmentExport.ts index c5fcd10151..298e8e931f 100644 --- a/src/application/environment/environmentExport.ts +++ b/src/application/environment/environmentExport.ts @@ -1,6 +1,7 @@ import { constants } from "node:fs"; import { copyFile, mkdir, readdir, stat, writeFile } from "node:fs/promises"; import { dirname, join } from "node:path"; +import { getHeapStatistics } from "node:v8"; import type { AnalysisError } from "../../domain/analysisErrorBase.js"; import { @@ -11,8 +12,12 @@ import { } from "../../domain/analysisErrorCore.js"; import type { JsonValue } from "../../domain/jsonValue.js"; import { err, ok, type Result } from "../../domain/result.js"; +import { nativeJsonParseRequiredBytes } from "../JsonInputFile.js"; import { readRegularFile } from "../RegularFileRead.js"; -import { parseUtf8Json } from "../Utf8JsonInput.js"; +import { + JSON_INPUT_RESOURCE_REMEDIATION, + parseUtf8Json, +} from "../Utf8JsonInput.js"; import { ENVIRONMENT_EXPORT_OPERATION, planEnvironmentBundle, @@ -172,6 +177,30 @@ const NOT_A_FILE_CODES = new Set([ "ENAMETOOLONG", ]); +/** Refuse, as a resource constraint, a file whose whole read and native parse would not fit live heap headroom. */ +const headroomRefusal = ( + path: string, + inputBytes: number, +): AnalysisResourceConstraintError | undefined => { + const heap = getHeapStatistics(); + const requiredBytes = nativeJsonParseRequiredBytes(inputBytes); + return requiredBytes <= heap.total_available_size + ? undefined + : new AnalysisResourceConstraintError( + ENVIRONMENT_EXPORT_OPERATION, + "memory", + "Insufficient heap headroom to read and parse the complete JSON file", + { + input_path: path, + input_file_bytes: inputBytes, + required_heap_bytes: requiredBytes, + available_heap_bytes: heap.total_available_size, + heap_size_limit_bytes: heap.heap_size_limit, + }, + { remediationAction: JSON_INPUT_RESOURCE_REMEDIATION }, + ); +}; + /** Read one input once: the same bytes are parsed for authentication and copied into the bundle. */ const readInput = async ( role: "manifest" | "evidence", @@ -179,6 +208,9 @@ const readInput = async ( ): Promise> => { let bytes: Buffer; try { + // Admit the whole read before materializing it; the bundle needs these exact bytes. + const refused = headroomRefusal(path, (await stat(path)).size); + if (refused !== undefined) return err(refused); bytes = await readRegularFile(path); } catch (cause: unknown) { if (NOT_A_FILE_CODES.has(systemCode(cause) ?? ""))