mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-08-04 14:57:42 +00:00
🖼️ feat: Return Sandbox Images From read_file as Viewable Artifacts (#14277)
Some checks are pending
Docker Dev Branch Images Build / build (Dockerfile, lc-dev, node) (push) Waiting to run
Docker Dev Branch Images Build / build (Dockerfile.multi, lc-dev-api, api-build) (push) Waiting to run
GitNexus Index / index (push) Waiting to run
GitNexus Index / post-index (push) Blocked by required conditions
Docker Dev Images Build / build (Dockerfile, librechat-dev, node) (push) Waiting to run
Docker Dev Images Build / build (Dockerfile.multi, librechat-dev-api, api-build) (push) Waiting to run
Sync Locize Translations & Create Translation PR / Sync Translation Keys with Locize (push) Waiting to run
Sync Locize Translations & Create Translation PR / Create Translation PR on Version Published (push) Blocked by required conditions
Sync Helm Chart Tags / Ignore non-main push (push) Waiting to run
Sync Helm Chart Tags / Sync chart tags (push) Waiting to run
Some checks are pending
Docker Dev Branch Images Build / build (Dockerfile, lc-dev, node) (push) Waiting to run
Docker Dev Branch Images Build / build (Dockerfile.multi, lc-dev-api, api-build) (push) Waiting to run
GitNexus Index / index (push) Waiting to run
GitNexus Index / post-index (push) Blocked by required conditions
Docker Dev Images Build / build (Dockerfile, librechat-dev, node) (push) Waiting to run
Docker Dev Images Build / build (Dockerfile.multi, librechat-dev-api, api-build) (push) Waiting to run
Sync Locize Translations & Create Translation PR / Sync Translation Keys with Locize (push) Waiting to run
Sync Locize Translations & Create Translation PR / Create Translation PR on Version Published (push) Blocked by required conditions
Sync Helm Chart Tags / Ignore non-main push (push) Waiting to run
Sync Helm Chart Tags / Sync chart tags (push) Waiting to run
* 🖼️ feat: Return Sandbox Images From `read_file` as Viewable Artifacts The code-execution sandbox `read_file` path refused every image extension because it reads files via `cat` over codeapi's JSON `/exec` transport, which lossily corrupts non-UTF-8 bytes. The skill-file read path already surfaced images as artifacts; this brings the sandbox path to parity so an agent can actually see a chart/screenshot it reads. - `readSandboxImage` (process.js): a Python base64 reader over `/exec` with an in-sandbox size guard so oversize images never cross the wire; base64 is ASCII-safe where `cat` corrupts. - `handleSandboxImageRead` (handlers.ts): byte-integrity check (guards against a truncated `/exec` stdout), MIME resolved purely from the magic-byte sniff (extension only routes; a mislabeled non-image falls back to the bash hint), and graceful degradation on every failure mode. - Shared `buildImageArtifactResult` used by both read paths; the result's `artifact.content` image_url reaches the UI (tool-end callbacks save it as an attachment) and the LLM (SDK folds it into the model-visible message for Anthropic/OpenAI/Google). * ✅ test: Sync read_file code-only description assertions with image wording * 🛡️ fix: Harden sandbox image reads (regular-file guard, completeness check) Addresses Codex review on PR #14277: - readSandboxImage now os.stat's the target and rejects non-regular files (FIFOs, sockets, /dev/* symlinks) via stat.S_ISREG, and bounds the read at limit+1 bytes — a device/FIFO can no longer stream unbounded into memory until the request times out. - handleSandboxImageRead validates completeness (not just the magic header): PNG must end with the IEND trailer and WebP's RIFF size must match the byte length, so a truncated/interrupted image degrades to the bash hint instead of being sent as a corrupt image_url. JPEG/GIF stay header-level (they can carry trailing metadata; a strict end-marker would risk false rejections). * 🩹 fix: Chunk sandbox image reads to fit the runner stdout cap Inlining any real image failed with "is an image file (.png) and cannot be read as text". Root cause: readSandboxImage base64-encodes the file to STDOUT, but the runner caps stdout at SANDBOX_OUTPUT_MAX_SIZE (1024 bytes by default) and SIGKILLs the job on overflow (status OL), truncating the JSON mid-base64. The parse then threw and the handler degraded to the binary hint. The in-sandbox MAX_BINARY_BYTES=5MB guard never fired because the *transport*, not the file size, is the real ceiling: a 5MB image needs ~6.8MB of stdout. Reproduced against a live MicroVM — a 186KB matplotlib PNG died with 'stdout length exceeded' at exactly the 65536-byte cap. Read the file in windows instead: each /exec pulls raw bytes at an offset and base64s only that slice, so every response stays under the cap regardless of how the runner is configured; the chunks are reassembled and verified against the sandbox-reported total. Verified end-to-end on a real MicroVM: 25KB and 186KB PNGs both round-trip byte-exact (sha256 match). Also: - Detect the truncation explicitly (status OL) and name the fixable cause (chunk size / SANDBOX_OUTPUT_MAX_SIZE) instead of "unexpected output". - Parse the LAST stdout line so a shell banner can't break the read, and include a stdout snippet when it genuinely is unparseable. - LIBRECHAT_CODE_IMAGE_CHUNK_BYTES (default 32KB) tunes the window. - Tests drive the real reader against a mocked /exec transport rather than mocking readSandboxImage, which is why the existing suite stayed green through this bug. * 🎯 fix: Cap sandbox inline images at 1MB, separate from skill-file reads The sandbox and skill-file image paths shared MAX_BINARY_BYTES (5MB), but their transports differ: skill files stream from storage, while sandbox bytes come back base64 over /exec stdout under the runner's output cap, so the reader windows the file and cost scales in round-trips (~160 at 5MB vs ~32 at 1MB). Nothing is gained by allowing more — vision providers downsample to ~1.5-2k px regardless, so multi-MB originals buy no fidelity while grinding through round-trips. Give the sandbox path its own MAX_SANDBOX_INLINE_IMAGE_BYTES (1MB), used for both the read cap and the over-limit message (which previously quoted 5MB while the reader enforced something else). Skill-file reads keep 5MB. Verified against a live MicroVM: a 186KB PNG round-trips byte-exact, and a 1.4MB file returns tooLarge in a single round-trip with zero bytes transferred, degrading to the existing bash_tool hint.
This commit is contained in:
parent
035228360d
commit
20cd00c492
7 changed files with 882 additions and 56 deletions
|
|
@ -5,6 +5,7 @@ const {
|
|||
getSessionInfo,
|
||||
checkIfActive,
|
||||
readSandboxFile,
|
||||
readSandboxImage,
|
||||
writeSandboxFile,
|
||||
} = require('~/server/services/Files/Code/process');
|
||||
const {
|
||||
|
|
@ -358,6 +359,12 @@ const skillToolDeps = {
|
|||
* the agents-side `ToolNode` via `tc.codeSessionContext`.
|
||||
*/
|
||||
readSandboxFile,
|
||||
/**
|
||||
* Companion to `readSandboxFile` for the raster-image case: pulls the
|
||||
* bytes base64-encoded (size-guarded in-sandbox) so `read_file` can
|
||||
* return an image the model can see instead of refusing it as binary.
|
||||
*/
|
||||
readSandboxImage,
|
||||
writeSandboxFile,
|
||||
};
|
||||
|
||||
|
|
|
|||
|
|
@ -1045,6 +1045,235 @@ async function readSandboxFile({ file_path, session_id, files, runtime_session_h
|
|||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Reads a small image file out of the code-execution sandbox as base64 so
|
||||
* `read_file` can surface it to vision-capable models. `readSandboxFile`'s
|
||||
* `cat` round-trips stdout through codeapi's JSON transport, which lossily
|
||||
* replaces non-UTF-8 bytes and corrupts image data. Here a tiny Python
|
||||
* reader stats the file, refuses (without transferring) anything over
|
||||
* `maxBytes`, and otherwise base64-encodes the bytes IN the sandbox so the
|
||||
* payload stays ASCII-safe across the JSON `/exec` transport. Session
|
||||
* forwarding mirrors `readSandboxFile` so the read lands in the same
|
||||
* sandbox session that holds the agent's prior-turn artifacts.
|
||||
*
|
||||
* @param {Object} params
|
||||
* @param {string} params.file_path - Path inside the sandbox (e.g. `/mnt/data/chart.png`).
|
||||
* @param {string} [params.session_id] - Sandbox session id from the seeded context.
|
||||
* @param {Array<{id: string, name: string, session_id?: string}>} [params.files] - File refs to mount.
|
||||
* @param {string} [params.runtime_session_hint] - Per-conversation stateful runtime-session hint.
|
||||
* @param {number} [params.maxBytes] - In-sandbox size cap; larger files return `{ tooLarge, bytes }`.
|
||||
* @param {ServerRequest} [params.req] - Current authenticated request, used to mint Code API auth.
|
||||
* @returns {Promise<{base64: string, bytes: number} | {tooLarge: true, bytes: number} | null>}
|
||||
* `null` when codeapi is unavailable; throws on transport / read errors.
|
||||
*/
|
||||
async function readSandboxImage({
|
||||
file_path,
|
||||
session_id,
|
||||
files,
|
||||
runtime_session_hint,
|
||||
maxBytes,
|
||||
req,
|
||||
}) {
|
||||
const baseURL = getCodeBaseURL();
|
||||
if (!baseURL) {
|
||||
return null;
|
||||
}
|
||||
|
||||
const limit = typeof maxBytes === 'number' && maxBytes > 0 ? maxBytes : 5 * megabyte;
|
||||
const chunkBytes = getImageChunkBytes();
|
||||
const maxChunks = Math.ceil(limit / chunkBytes) + 1;
|
||||
|
||||
/** @type {Buffer[]} */
|
||||
const parts = [];
|
||||
let offset = 0;
|
||||
let total = null;
|
||||
|
||||
for (let i = 0; i < maxChunks; i++) {
|
||||
const payload = Buffer.from(
|
||||
JSON.stringify({ file_path, limit, offset, chunk: chunkBytes }),
|
||||
'utf8',
|
||||
).toString('base64');
|
||||
const code = [
|
||||
"python3 - <<'PY'",
|
||||
'import base64, json, os, stat',
|
||||
`payload = ${JSON.stringify(payload)}`,
|
||||
"data = json.loads(base64.b64decode(payload).decode('utf-8'))",
|
||||
"p = data['file_path']",
|
||||
"limit = data['limit']",
|
||||
"offset = data['offset']",
|
||||
"chunk = data['chunk']",
|
||||
'try:',
|
||||
' st = os.stat(p)',
|
||||
'except OSError as e:',
|
||||
' print(json.dumps({"error": str(e)}))',
|
||||
' raise SystemExit(0)',
|
||||
// Reject FIFOs, sockets, and device files (e.g. a symlink to /dev/zero):
|
||||
// os.stat can report a small/zero size while an unbounded read blocks or
|
||||
// streams forever until the request times out.
|
||||
'if not stat.S_ISREG(st.st_mode):',
|
||||
' print(json.dumps({"error": "not a regular file"}))',
|
||||
' raise SystemExit(0)',
|
||||
'if st.st_size > limit:',
|
||||
' print(json.dumps({"too_large": True, "bytes": st.st_size}))',
|
||||
' raise SystemExit(0)',
|
||||
// Read only this window. The whole base64 payload cannot be emitted in
|
||||
// one shot: the runner caps stdout at SANDBOX_OUTPUT_MAX_SIZE (1024
|
||||
// bytes by default) and SIGKILLs the job on overflow, which truncates
|
||||
// the JSON mid-string. Windowing keeps every response under that cap.
|
||||
"with open(p, 'rb') as f:",
|
||||
' f.seek(offset)',
|
||||
' raw = f.read(chunk)',
|
||||
'print(json.dumps({"total": st.st_size, "n": len(raw), "b64": base64.b64encode(raw).decode("ascii")}))',
|
||||
'PY',
|
||||
].join('\n');
|
||||
|
||||
const parsed = await execSandboxImageChunk({
|
||||
baseURL,
|
||||
code,
|
||||
file_path,
|
||||
session_id,
|
||||
runtime_session_hint,
|
||||
files,
|
||||
req,
|
||||
chunkBytes,
|
||||
});
|
||||
|
||||
if (parsed.error) {
|
||||
throw new Error(String(parsed.error));
|
||||
}
|
||||
if (parsed.too_large === true) {
|
||||
return { tooLarge: true, bytes: Number(parsed.bytes) || 0 };
|
||||
}
|
||||
if (typeof parsed.b64 !== 'string' || typeof parsed.n !== 'number') {
|
||||
return null;
|
||||
}
|
||||
|
||||
if (total == null) {
|
||||
total = Number(parsed.total) || 0;
|
||||
if (total > limit) {
|
||||
return { tooLarge: true, bytes: total };
|
||||
}
|
||||
} else if (Number(parsed.total) !== total) {
|
||||
/* The file changed underneath us; a spliced-together buffer would be
|
||||
* a mix of two versions rather than any real image. */
|
||||
throw new Error(`"${file_path}" changed while being read from the sandbox`);
|
||||
}
|
||||
|
||||
parts.push(Buffer.from(parsed.b64, 'base64'));
|
||||
offset += parsed.n;
|
||||
|
||||
if (parsed.n === 0 || offset >= total) {
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
const buffer = Buffer.concat(parts);
|
||||
if (total == null) {
|
||||
return null;
|
||||
}
|
||||
if (buffer.length !== total) {
|
||||
/* Ran out of chunk budget (or short reads); returning a partial image
|
||||
* would render as a corrupt file, so surface it as unreadable-inline. */
|
||||
return { tooLarge: true, bytes: total };
|
||||
}
|
||||
return { base64: buffer.toString('base64'), bytes: buffer.length };
|
||||
}
|
||||
|
||||
/**
|
||||
* Raw bytes pulled per `/exec` round-trip when inlining a sandbox image.
|
||||
* Each chunk is base64-encoded (~1.33x) into the response's stdout, which
|
||||
* the runner truncates + SIGKILLs past `SANDBOX_OUTPUT_MAX_SIZE`. The
|
||||
* default leaves headroom for a runner configured at 64KB; deployments
|
||||
* with a smaller cap must lower this, and a larger cap can raise it to cut
|
||||
* round-trips.
|
||||
* @returns {number}
|
||||
*/
|
||||
function getImageChunkBytes() {
|
||||
const parsed = Number(process.env.LIBRECHAT_CODE_IMAGE_CHUNK_BYTES);
|
||||
return Number.isFinite(parsed) && parsed > 0 ? Math.floor(parsed) : 32 * 1024;
|
||||
}
|
||||
|
||||
/**
|
||||
* Runs one image-chunk read over `/exec` and parses its JSON line.
|
||||
* @returns {Promise<Record<string, unknown>>}
|
||||
*/
|
||||
async function execSandboxImageChunk({
|
||||
baseURL,
|
||||
code,
|
||||
file_path,
|
||||
session_id,
|
||||
runtime_session_hint,
|
||||
files,
|
||||
req,
|
||||
chunkBytes,
|
||||
}) {
|
||||
/** @type {Record<string, unknown>} */
|
||||
const postData = { lang: 'bash', code };
|
||||
if (session_id) {
|
||||
postData.session_id = session_id;
|
||||
}
|
||||
if (runtime_session_hint) {
|
||||
postData.runtime_session_hint = runtime_session_hint;
|
||||
}
|
||||
if (files && files.length > 0) {
|
||||
postData.files = files;
|
||||
}
|
||||
|
||||
try {
|
||||
const authHeaders = await getCodeApiAuthHeaders(req);
|
||||
const response = await axios({
|
||||
method: 'post',
|
||||
url: `${baseURL}/exec`,
|
||||
data: postData,
|
||||
headers: {
|
||||
'Content-Type': 'application/json',
|
||||
'User-Agent': 'LibreChat/1.0',
|
||||
...authHeaders,
|
||||
},
|
||||
httpAgent: codeServerHttpAgent,
|
||||
httpsAgent: codeServerHttpsAgent,
|
||||
timeout: 15000,
|
||||
});
|
||||
const result = response?.data ?? {};
|
||||
/* The runner truncates stdout at SANDBOX_OUTPUT_MAX_SIZE and SIGKILLs the
|
||||
* job (status `OL`). Detect that explicitly: the surviving stdout is a
|
||||
* base64 string cut mid-flight, so parsing it yields a misleading
|
||||
* "unexpected output" instead of naming the real, fixable cause. */
|
||||
if (result.status === 'OL') {
|
||||
throw new Error(
|
||||
`Reading "${file_path}" exceeded the sandbox stdout limit (chunk ${chunkBytes} bytes). ` +
|
||||
'Lower LIBRECHAT_CODE_IMAGE_CHUNK_BYTES or raise SANDBOX_OUTPUT_MAX_SIZE on the runner.',
|
||||
);
|
||||
}
|
||||
if (result.stderr && (result.stdout == null || result.stdout === '')) {
|
||||
throw new Error(String(result.stderr).trim());
|
||||
}
|
||||
if (result.stdout == null || String(result.stdout).trim() === '') {
|
||||
return {};
|
||||
}
|
||||
/* Parse the LAST non-empty line: the reader's JSON is the final thing it
|
||||
* prints, so anything a shell profile or library emitted ahead of it
|
||||
* (banners, warnings) must not break the read. */
|
||||
const lines = String(result.stdout)
|
||||
.split('\n')
|
||||
.map((line) => line.trim())
|
||||
.filter(Boolean);
|
||||
try {
|
||||
return JSON.parse(lines[lines.length - 1]);
|
||||
} catch {
|
||||
throw new Error(
|
||||
`Unexpected output while reading image bytes from the sandbox: ${String(result.stdout).slice(0, 120)}`,
|
||||
);
|
||||
}
|
||||
} catch (error) {
|
||||
logAxiosError({
|
||||
message: `Error reading sandbox image "${file_path}"`,
|
||||
error,
|
||||
});
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Writes a UTF-8 text file into the code-execution sandbox by running a
|
||||
* small Python writer through the sandbox `/exec` endpoint. The payload is
|
||||
|
|
@ -1150,6 +1379,7 @@ module.exports = {
|
|||
getSessionInfo,
|
||||
processCodeOutput,
|
||||
readSandboxFile,
|
||||
readSandboxImage,
|
||||
writeSandboxFile,
|
||||
runPreviewFinalize,
|
||||
};
|
||||
|
|
|
|||
|
|
@ -165,6 +165,7 @@ const {
|
|||
processCodeOutput,
|
||||
getSessionInfo,
|
||||
readSandboxFile,
|
||||
readSandboxImage,
|
||||
writeSandboxFile,
|
||||
primeFiles,
|
||||
} = require('./process');
|
||||
|
|
@ -2069,4 +2070,113 @@ describe('Code Process', () => {
|
|||
expect(result.toolContext).not.toContain('preview');
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* These drive the REAL reader against a mocked `/exec` transport (rather
|
||||
* than mocking `readSandboxImage` itself), because the bug this covers
|
||||
* lived entirely in the transport: base64 leaves the sandbox on stdout,
|
||||
* which the runner truncates + SIGKILLs past `SANDBOX_OUTPUT_MAX_SIZE`.
|
||||
*/
|
||||
describe('readSandboxImage', () => {
|
||||
const crypto = require('crypto');
|
||||
const PNG_HEADER = Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]);
|
||||
/** Reply as the sandbox would: serve `buffer` through the windowed reader. */
|
||||
const serveFile = (buffer) =>
|
||||
mockAxios.mockImplementation(async ({ data }) => {
|
||||
const payload = JSON.parse(
|
||||
Buffer.from(JSON.parse(/payload = ("[^"]+")/.exec(data.code)[1]), 'base64').toString(),
|
||||
);
|
||||
const slice = buffer.subarray(payload.offset, payload.offset + payload.chunk);
|
||||
return {
|
||||
data: {
|
||||
stdout: JSON.stringify({
|
||||
total: buffer.length,
|
||||
n: slice.length,
|
||||
b64: slice.toString('base64'),
|
||||
}),
|
||||
},
|
||||
};
|
||||
});
|
||||
|
||||
beforeEach(() => {
|
||||
process.env.LIBRECHAT_CODE_BASEURL = 'http://code.test/v1';
|
||||
delete process.env.LIBRECHAT_CODE_IMAGE_CHUNK_BYTES;
|
||||
mockAxios.mockReset();
|
||||
});
|
||||
|
||||
it('reassembles an image larger than one chunk, byte-for-byte', async () => {
|
||||
/* 200KB of PNG-headed noise: > 6 chunks at the 32KB default, and the
|
||||
* exact shape that used to blow the stdout cap and SIGKILL the job. */
|
||||
const source = Buffer.concat([PNG_HEADER, crypto.randomBytes(200 * 1024)]);
|
||||
serveFile(source);
|
||||
|
||||
const result = await readSandboxImage({ file_path: '/mnt/data/big.png' });
|
||||
|
||||
expect(mockAxios.mock.calls.length).toBeGreaterThan(1);
|
||||
expect(result.bytes).toBe(source.length);
|
||||
expect(Buffer.from(result.base64, 'base64').equals(source)).toBe(true);
|
||||
});
|
||||
|
||||
it('reads a single-chunk image in one round-trip', async () => {
|
||||
const source = Buffer.concat([PNG_HEADER, crypto.randomBytes(1024)]);
|
||||
serveFile(source);
|
||||
|
||||
const result = await readSandboxImage({ file_path: '/mnt/data/small.png' });
|
||||
|
||||
expect(mockAxios).toHaveBeenCalledTimes(1);
|
||||
expect(Buffer.from(result.base64, 'base64').equals(source)).toBe(true);
|
||||
});
|
||||
|
||||
it('names the real cause when a chunk overflows the runner stdout cap', async () => {
|
||||
/* The runner truncates stdout and SIGKILLs with status `OL`; the old
|
||||
* reader parsed the truncated base64 and reported a misleading
|
||||
* "unexpected output" instead of the fixable limit. */
|
||||
mockAxios.mockResolvedValue({
|
||||
data: { stdout: '{"total":999999,"n":32768,"b64":"iVBORw0KGg', status: 'OL', code: 137 },
|
||||
});
|
||||
|
||||
await expect(readSandboxImage({ file_path: '/mnt/data/big.png' })).rejects.toThrow(
|
||||
/exceeded the sandbox stdout limit/,
|
||||
);
|
||||
});
|
||||
|
||||
it('honors LIBRECHAT_CODE_IMAGE_CHUNK_BYTES', async () => {
|
||||
process.env.LIBRECHAT_CODE_IMAGE_CHUNK_BYTES = '1024';
|
||||
const source = Buffer.concat([PNG_HEADER, crypto.randomBytes(4 * 1024)]);
|
||||
serveFile(source);
|
||||
|
||||
const result = await readSandboxImage({ file_path: '/mnt/data/x.png' });
|
||||
|
||||
expect(mockAxios.mock.calls.length).toBe(5);
|
||||
expect(Buffer.from(result.base64, 'base64').equals(source)).toBe(true);
|
||||
});
|
||||
|
||||
it('parses the reader JSON even when the shell emits a banner first', async () => {
|
||||
const source = Buffer.concat([PNG_HEADER, crypto.randomBytes(64)]);
|
||||
mockAxios.mockResolvedValue({
|
||||
data: {
|
||||
stdout: `motd banner\n${JSON.stringify({
|
||||
total: source.length,
|
||||
n: source.length,
|
||||
b64: source.toString('base64'),
|
||||
})}`,
|
||||
},
|
||||
});
|
||||
|
||||
const result = await readSandboxImage({ file_path: '/mnt/data/x.png' });
|
||||
|
||||
expect(Buffer.from(result.base64, 'base64').equals(source)).toBe(true);
|
||||
});
|
||||
|
||||
it('refuses an oversize file in-sandbox without transferring bytes', async () => {
|
||||
mockAxios.mockResolvedValue({
|
||||
data: { stdout: JSON.stringify({ too_large: true, bytes: 9 * 1024 * 1024 }) },
|
||||
});
|
||||
|
||||
const result = await readSandboxImage({ file_path: '/mnt/data/huge.png' });
|
||||
|
||||
expect(result).toEqual({ tooLarge: true, bytes: 9 * 1024 * 1024 });
|
||||
expect(mockAxios).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue