Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -255,28 +255,45 @@ public EditResult edit(
Base64.getEncoder()
.encodeToString(payload.getBytes(java.nio.charset.StandardCharsets.UTF_8));

String cmd =
"python3 -c \"import sys, os, base64, json\\n"
// Edit script is assembled with real newlines, then base64-encoded and piped via stdin
// to `python3 -`; the payload is passed through heredoc stdin.
//
// Do NOT revert to `python3 -c "...\n..."` inline form: under `bash -lc`, \n inside
// double quotes is a literal backslash+n (not a newline), so the entire script collapses
// into one line and Python fails with:
// SyntaxError: unexpected character after line continuation character
// This makes edit_file 100% non-functional in sandbox environments (the model falls back
// to write_file, which refuses to overwrite existing files, so no file can be modified).
// Using printf + heredoc avoids all quoting/escaping issues.
String script =
"import sys, os, base64, json\n"
+ "payload ="
+ " json.loads(base64.b64decode(sys.stdin.read().strip()).decode('utf-8'))\\n"
+ "path, old, new = payload['path'], payload['old'], payload['new']\\n"
+ "replace_all = payload.get('replace_all', False)\\n"
+ "if not os.path.isfile(path):\\n"
+ " print(json.dumps({'error': 'file_not_found'}))\\n"
+ " sys.exit(0)\\n"
+ "with open(path, 'rb') as f: text = f.read().decode('utf-8')\\n"
+ "count = text.count(old)\\n"
+ "if count == 0:\\n"
+ " print(json.dumps({'error': 'string_not_found'}))\\n"
+ " sys.exit(0)\\n"
+ "if count > 1 and not replace_all:\\n"
+ " print(json.dumps({'error': 'multiple_occurrences', 'count': count}))\\n"
+ " sys.exit(0)\\n"
+ " json.loads(base64.b64decode(sys.stdin.read().strip()).decode('utf-8'))\n"
+ "path, old, new = payload['path'], payload['old'], payload['new']\n"
+ "replace_all = payload.get('replace_all', False)\n"
+ "if not os.path.isfile(path):\n"
+ " print(json.dumps({'error': 'file_not_found'}))\n"
+ " sys.exit(0)\n"
+ "with open(path, 'rb') as f: text = f.read().decode('utf-8')\n"
+ "count = text.count(old)\n"
+ "if count == 0:\n"
+ " print(json.dumps({'error': 'string_not_found'}))\n"
+ " sys.exit(0)\n"
+ "if count > 1 and not replace_all:\n"
+ " print(json.dumps({'error': 'multiple_occurrences', 'count': count}))\n"
+ " sys.exit(0)\n"
+ "result = text.replace(old, new) if replace_all else text.replace(old, new,"
+ " 1)\\n"
+ "with open(path, 'wb') as f: f.write(result.encode('utf-8'))\\n"
+ "print(json.dumps({'count': count}))\\n"
+ "\" 2>&1 <<'__EDIT_EOF__'\n"
+ " 1)\n"
+ "with open(path, 'wb') as f: f.write(result.encode('utf-8'))\n"
+ "print(json.dumps({'count': count}))\n";
String scriptB64 =
Base64.getEncoder()
.encodeToString(script.getBytes(java.nio.charset.StandardCharsets.UTF_8));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Info] echo is only guaranteed to pass a single long argument through verbatim by bash's builtin; some echo implementations/versions wrap very long output, and a wrapped base64 stream decodes to a truncated script (manifesting as a Python SyntaxError, i.e. the same symptom this PR is fixing). This repo already hit the mirror-image problem on the read path — see SandboxBackedFilesystem#L209 ("MIME decoder tolerates wrapped base64 output from GNU base64"). printf '%s\n' avoids the ambiguity, and it would be worth extending edit_usesBase64PipedScript with a large-payload case asserting the emitted command is a single line.


String cmd =
"printf '%s\n' "
+ scriptB64
+ " | base64 -d | python3 - 2>&1 <<'__EDIT_EOF__'\n"
+ payloadB64
+ "\n__EDIT_EOF__\n";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,54 @@ void glob_executeFailure_shouldFailInsteadOfErrorAsPaths() {
assertFalse(result.isSuccess(), "glob should fail when the command never ran");
assertTrue(result.error().contains("status=504"), "error should carry the cause");
}

@Test
void edit_usesBase64PipedScript() {
FakeSandboxFilesystem fs = new FakeSandboxFilesystem();

// edit() constructs the command and calls execute().
// FakeSandboxFilesystem.execute() returns empty output, so edit() will
// return an error result, but we only care about verifying the command shape.
var result = fs.edit(RT, "/workspace/test.txt", "old", "new", false);

// Verify command uses printf + base64 pipe + heredoc stdin (not echo/argv)
assertTrue(
fs.lastCommand.contains("printf '%s\n'"),
"edit should use printf for script base64, got: " + fs.lastCommand);
assertTrue(
fs.lastCommand.contains("base64 -d | python3 -"),
"edit should use base64 piped to python3, got: " + fs.lastCommand);
assertTrue(
fs.lastCommand.contains("<<'__EDIT_EOF__'"),
"edit should pass payload via heredoc stdin, got: " + fs.lastCommand);
assertFalse(
fs.lastCommand.contains("python3 -c"),
"edit should NOT use python3 -c inline form");
assertFalse(
fs.lastCommand.contains("echo "),
"edit should NOT use echo (may wrap long base64), got: " + fs.lastCommand);
}

@Test
void edit_largePayloadUsesHeredocNotArgv() {
FakeSandboxFilesystem fs = new FakeSandboxFilesystem();

// Simulate a large edit (multi-KB payload) to verify it goes through
// heredoc stdin, not argv[1] (which would hit ARG_MAX limits).
String largeString = "x".repeat(100_000);
fs.edit(RT, "/workspace/large.txt", "old", largeString, false);

// Payload must NOT appear on the command line (argv)
// The command line should only contain the script base64 and heredoc markers
assertTrue(
fs.lastCommand.contains("<<'__EDIT_EOF__'"),
"large payload should use heredoc stdin, got command length: "
+ fs.lastCommand.length());
// Verify the payload is after the heredoc marker, not before it (i.e., not in argv)
int heredocPos = fs.lastCommand.indexOf("<<'__EDIT_EOF__'");
int editEofPos = fs.lastCommand.indexOf("__EDIT_EOF__", heredocPos + 1);
assertTrue(editEofPos > heredocPos, "payload should be between heredoc markers");
}
}

// ================================================================
Expand Down
Loading