Fix: base64-encode UTF-8 code executor output files before saving them as artifacts - #677
Open
AmaadMartin wants to merge 2 commits into
Open
Fix: base64-encode UTF-8 code executor output files before saving them as artifacts#677AmaadMartin wants to merge 2 commits into
AmaadMartin wants to merge 2 commits into
Conversation
added 2 commits
August 5, 2026 03:59
Part.inlineData.data must be base64, but File.content is raw UTF-8 text whenever the file declares contentEncoding UTF8. Read the declared encoding instead of guessing so callers can build a correct artifact Part.
FileArtifactService and GcsArtifactService base64-decode Part.inlineData.data before they write it. The processor passed the raw UTF-8 text of every text output file straight through, so the decode wrote a few bytes of noise and the artifact was unrecoverable. Binary output files were already base64 and stay untouched.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Please ensure you have read the contribution guide before creating a pull request.
Link to Issue or Description of Change
N/A
Problem:
postProcessCodeExecutionResultputsFile.contentstraight intoPart.inlineData.data. That field is defined to be base64, andFileArtifactServiceandGcsArtifactServiceboth decode it withBuffer.from(data, 'base64'). A text output file carries raw UTF-8 (File.contentEncoding === 'utf-8'), so the decode silently drops the non-base64 characters and writes a few bytes of noise.hello from script(17 chars) becomes 11 bytes of binary; the artifact is unrecoverable.Solution: A new helper
getFileContentAsBase64reads the file's declaredcontentEncodingand encodes only UTF-8 content. Base64 content passes through, so binary files are never encoded twice, and a file with no declared encoding is still treated as base64 — that preservesAgentEngineSandboxCodeExecutor's current behavior exactly. I did not reusegetEncodedFileContent, whoseisBase64Encodedheuristic corrupts plain text that happens to be valid base64 (isBase64Encoded('data')istrue). Artifacts written before this fix stay corrupt; there is no migration.Collision check (575 open PRs on the fork scanned): no PR fixes this call site. #639 edits the same
forloop but only moves the artifact-service guard, and #675 renames the file. #410 adds an equivalent helper,toBase64Content, incode_execution_utils.tsfor the skill-script path, but leavescode_execution_request_processor.tsuntouched, so the bug survives it. I based this onmainrather than stacking on #410 because #410 is a 913-line feature PR competing with #517 and #564 for the same feature; stacking would tie this fix to whichever of them lands. Whichever of the two merges second should drop its copy of the helper.Scope notes:
getFileContentAsBase64is deliberately not exported fromcommon.ts/index.ts. It is an internal helper.Artifact service is not initialized.guard. It is pre-existing behavior that Fix: require an artifact service only when a code execution produces output files #639 is actively reworking; a test pinning it here would contradict that PR.npm run ts:checkis red onmain(281 errors, from@google/adkresolving tocore/dist/typeswhile tests also import../../src). CI does not run it. This change adds 2 errors of that same pre-existing kind, both already present 12 times in the file it touches.Testing Plan
Please describe the tests that you ran to verify your changes. This is required for all PRs that are not small documentation or typo fixes.
All pass. Coverage on
core/src/code_executors/code_execution_utils.tsis 100% lines and 100% branches; the changed lines in the processor are each executed by 3 tests.Proving the tests can fail. Two mutations were run:
data: outputFile.content. Two tests failed, one passed:base64-encodes a utf-8 output file—expected 'hello from script' to be 'aGVsbG8gZnJvbSBzY3JpcHQ='round-trips a utf-8 output file through FileArtifactService—expected '\ufffd\ufffde\ufffd\ufffd\ufffd\ufffd…' to be 'hello from script'passes a base64 output file through without encoding it twicestill passed, which is the point: the fix does not change binary files.FileContentEncoding.BASE64. Three tests failed:encodes a utf-8 file—expected '# Notes\n' to be 'IyBOb3Rlcwo='passes a base64 file through unchanged—expected 'aVZCT1J3MEtHZ289' to be 'iVBORw0KGgo='encodes utf-8 text that is itself valid base64—expected 'data' to be 'ZGF0YQ=='Unit Tests:
[x] I have added or updated unit tests for my change.
[x] All unit tests pass locally.
Manual End-to-End (E2E) Tests:
Please provide instructions on how to manually test your changes, including any necessary setup or configuration.
Run an
LlmAgentwith anUnsafeLocalCodeExecutorand aFileArtifactServicerooted at a temp directory. Ask it to write a markdown file. Read the stored payload at<root>/users/<user>/sessions/<session>/artifacts/<name>/versions/0/<name>and confirm it holds the original text. Before this change it holds a few bytes of noise.The third unit test covers the same path automatically: it drives the real processor into a real
FileArtifactServiceunderfs.mkdtemp, with no mocks, and reads the artifact back.Checklist
[x] I have read the CONTRIBUTING.md document.
[x] I have performed a self-review of my own code.
[x] I have commented my code, particularly in hard-to-understand areas.
[x] I have added tests that prove my fix is effective or that my feature works.
[x] New and existing unit tests pass locally with my changes.