mirror of
https://github.com/browser-use/browser-use.git
synced 2026-10-02 04:04:36 +08:00
binary write: validate magic bytes + no ghost file on failed write (cubic review)
- Reject valid base64 that isn't actually an image (e.g. aGVsbG8= -> b'hello'), and reject correct image bytes under the wrong extension (magic mismatch), so upload flows never receive a corrupt file. - Register a new file in self.files only after a successful write, so a rejected binary write leaves no ghost entry in list_files()/state. - Binary files cannot be appended (base64 fragments would corrupt). - Strengthen tests: content-integrity round-trip, non-image base64, wrong-magic, and no-ghost assertions.
This commit is contained in:
@@ -399,11 +399,47 @@ class Base64BinaryFile(BaseFile):
|
||||
flows, not for arbitrary large binaries.
|
||||
"""
|
||||
|
||||
# Leading magic bytes that identify a real file of this type. Keyed by extension so
|
||||
# base64 that decodes but isn't actually an image (e.g. 'aGVsbG8=' -> b'hello') is
|
||||
# rejected instead of written as a corrupt upload.
|
||||
_MAGIC: dict[str, tuple[bytes, ...]] = {
|
||||
'png': (b'\x89PNG\r\n\x1a\n',),
|
||||
'gif': (b'GIF87a', b'GIF89a'),
|
||||
'jpg': (b'\xff\xd8\xff',),
|
||||
'jpeg': (b'\xff\xd8\xff',),
|
||||
'webp': (b'RIFF',), # RIFF container; 'WEBP' tag checked below
|
||||
}
|
||||
|
||||
def _decoded(self) -> bytes:
|
||||
# Strip all whitespace (the write_file action appends a trailing newline) then
|
||||
# decode strictly so non-base64 text is rejected rather than silently corrupted.
|
||||
return base64.b64decode(''.join(self.content.split()), validate=True)
|
||||
|
||||
def _validate(self, content: str) -> None:
|
||||
"""Decode and confirm the bytes actually are an image of this extension. Raises FileSystemError."""
|
||||
try:
|
||||
data = base64.b64decode(''.join(content.split()), validate=True)
|
||||
except Exception as e:
|
||||
raise FileSystemError(
|
||||
f"Error: content for '{self.full_name}' is not valid base64. "
|
||||
f'For images, provide the base64 of a valid {self.extension} file. ({e})'
|
||||
)
|
||||
magic = self._MAGIC.get(self.extension, ())
|
||||
if magic and not any(data.startswith(m) for m in magic):
|
||||
raise FileSystemError(
|
||||
f"Error: content for '{self.full_name}' is valid base64 but not a {self.extension} image "
|
||||
f'(wrong magic bytes). Provide the base64 of a real {self.extension} file.'
|
||||
)
|
||||
if self.extension == 'webp' and not (data[:4] == b'RIFF' and data[8:12] == b'WEBP'):
|
||||
raise FileSystemError(f"Error: content for '{self.full_name}' is not a valid WEBP file.")
|
||||
|
||||
def write_file_content(self, content: str) -> None:
|
||||
self._validate(content)
|
||||
self.update_content(content)
|
||||
|
||||
def append_file_content(self, content: str) -> None:
|
||||
raise FileSystemError(f"Error: cannot append to binary file '{self.full_name}'. Overwrite it instead.")
|
||||
|
||||
def sync_to_disk_sync(self, path: Path) -> None:
|
||||
(path / self.full_name).write_bytes(self._decoded())
|
||||
|
||||
@@ -853,15 +889,16 @@ class FileSystem:
|
||||
if not file_class:
|
||||
raise ValueError(f"Error: Invalid file extension '{extension}' for file '{full_filename}'.")
|
||||
|
||||
# Create or get existing file using full filename as key
|
||||
if full_filename in self.files:
|
||||
file_obj = self.files[full_filename]
|
||||
else:
|
||||
file_obj = file_class(name=name_without_ext)
|
||||
self.files[full_filename] = file_obj # Use full filename as key
|
||||
# Create or get existing file using full filename as key. A NEW file is only
|
||||
# registered after a successful write, so a failed write (e.g. invalid base64
|
||||
# for an image) leaves no ghost entry in self.files / state.
|
||||
is_new = full_filename not in self.files
|
||||
file_obj = self.files[full_filename] if not is_new else file_class(name=name_without_ext)
|
||||
|
||||
# Use file-specific write method
|
||||
await file_obj.write(content, self.data_dir)
|
||||
if is_new:
|
||||
self.files[full_filename] = file_obj
|
||||
sanitize_note = f" (auto-corrected from '{original_filename}')" if was_sanitized else ''
|
||||
return f'Data written to file {full_filename} successfully.{sanitize_note}'
|
||||
except FileSystemError as e:
|
||||
|
||||
@@ -282,9 +282,32 @@ class TestBinaryFileCreation:
|
||||
result = await fs.write_file('bad.png', 'this is definitely not base64 @@@')
|
||||
assert 'Error' in result
|
||||
assert not (fs.get_dir() / 'bad.png').exists() # no 0-byte / corrupt file left for upload
|
||||
# No ghost entry in the in-memory filesystem / state either
|
||||
assert 'bad.png' not in fs.list_files()
|
||||
assert 'bad.png' not in [f for f in fs.get_state().model_dump().get('files', {})]
|
||||
|
||||
async def test_valid_base64_but_not_an_image_is_rejected(self, tmp_path: Path):
|
||||
"""'aGVsbG8=' is valid base64 (-> b'hello') but not a PNG: must be rejected, not written."""
|
||||
fs = FileSystem(str(tmp_path))
|
||||
result = await fs.write_file('fake.png', 'aGVsbG8=')
|
||||
assert 'Error' in result
|
||||
assert not (fs.get_dir() / 'fake.png').exists()
|
||||
assert 'fake.png' not in fs.list_files()
|
||||
|
||||
async def test_wrong_magic_for_extension_is_rejected(self, tmp_path: Path):
|
||||
"""PNG bytes written under a .gif name are rejected (magic mismatch)."""
|
||||
fs = FileSystem(str(tmp_path))
|
||||
result = await fs.write_file('mislabeled.gif', self.PNG_1X1)
|
||||
assert 'Error' in result
|
||||
assert 'mislabeled.gif' not in fs.list_files()
|
||||
|
||||
async def test_state_round_trip_preserves_binary(self, tmp_path: Path):
|
||||
fs = FileSystem(str(tmp_path))
|
||||
await fs.write_file('logo.png', self.PNG_1X1)
|
||||
original_bytes = (fs.get_dir() / 'logo.png').read_bytes()
|
||||
fs2 = FileSystem.from_state(fs.get_state())
|
||||
assert fs2.get_file('logo.png') is not None
|
||||
restored = fs2.get_file('logo.png')
|
||||
assert restored is not None
|
||||
# Content integrity, not just existence: decoded bytes must round-trip and be a real PNG
|
||||
assert restored._decoded() == original_bytes
|
||||
assert restored._decoded()[:8] == b'\x89PNG\r\n\x1a\n'
|
||||
|
||||
Reference in New Issue
Block a user