mirror of
https://github.com/morluto/rea.git
synced 2026-11-11 17:28:08 +00:00
fix(environment): readable bundle inputs, an honest replay check and admitted export reads
- The Dockerfile copies manifest.json and evidence.json owned by the node user, so a private host umask cannot leave the runtime user unable to read them. - The generated verifier keeps the first record's own unknown status instead of borrowing another record's, and an error reply never passes the check, even when the recorded status is unknown. - Export checks, before reading an input, that reading and natively parsing the whole file fits live heap headroom, with the same rule the bounded JSON reader applies to native parsing, and refuses it as a resource constraint otherwise. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
ac1fbd282e
commit
178668fc74
6 files changed
+114
-14
No files matched your search
@@ -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
|
||||
|
||||
@@ -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. */
|
||||
|
||||
@@ -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");
|
||||
});
|
||||
});
|
||||
@@ -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"',
|
||||
|
||||
@@ -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<string[]> =>
|
||||
(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);
|
||||
|
||||
@@ -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<Result<EnvironmentExportFile, AnalysisError>> => {
|
||||
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) ?? ""))
|
||||
|
||||
Reference in new issue
Block a user