Merge branch 'main' into fix/download-file-path-traversal

# Conflicts:
#	omnigent/tools/builtins/download_file.py
#	tests/tools/builtins/test_file_tools.py

omnigent/tools/builtins/download_file.py

@@ -7,6 +7,7 @@ from pathlib import Path
 from typing import Any
 
 from omnigent.tools.base import Tool, ToolContext
+from omnigent.tools.builtins.upload_file import safe_resolve
 
 
 class DownloadFileTool(Tool):
@@ -101,10 +102,13 @@ class DownloadFileTool(Tool):
                 }
             )
 
-        dest = _resolve_destination(
-            record.filename,
-            ctx.workspace,
-        )
+        try:
+            dest = _resolve_destination(
+                record.filename,
+                ctx.workspace,
+            )
+        except ValueError as exc:
+            return json.dumps({"error": str(exc)})
         dest.parent.mkdir(parents=True, exist_ok=True)
         dest.write_bytes(data)
 
@@ -125,9 +129,17 @@ def _resolve_destination(
     """
     Resolve the save path for a downloaded file.
 
+    The stored ``filename`` is untrusted metadata (it originates from
+    whoever uploaded the file and is persisted verbatim). It is reduced
+    to its bare basename and confined to the workspace via
+    :func:`safe_resolve`, so a malicious name such as ``../../escape``
+    or an absolute path cannot cause a write outside the workspace.
+
     :param filename: The file's original filename from the store.
     :param workspace: The agent's workspace directory, or ``None``.
-    :returns: Absolute path to save the file.
+    :returns: Absolute path to save the file, confined to ``workspace``.
+    :raises ValueError: If the resolved path escapes the workspace.
     """
     base = workspace or Path.cwd()
-    return base / filename
+    name = Path(filename).name or "downloaded.bin"
+    return safe_resolve(name, base)

tests/tools/builtins/test_file_tools.py

@@ -306,6 +306,110 @@ def test_download_file_saves_to_workspace(
     assert saved.name == "hello.txt"
 
 
+@pytest.mark.parametrize(
+    "malicious_filename",
+    [
+        "../escape.txt",
+        "../../escape.txt",
+        "foo/../../bar.txt",
+        "/etc/passwd",
+        "/tmp/abs-escape.txt",
+    ],
+)
+def test_download_file_confines_untrusted_filename_to_workspace(
+    monkeypatch: pytest.MonkeyPatch,
+    tool_ctx: ToolContext,
+    malicious_filename: str,
+) -> None:
+    """
+    A malicious stored filename cannot write outside the workspace.
+
+    The stored filename is untrusted metadata (persisted verbatim from
+    whoever uploaded the file). Traversal sequences and absolute paths
+    must be reduced to a basename and confined to the workspace, never
+    escaping it.
+
+    :param monkeypatch: Pytest monkeypatch fixture.
+    :param tool_ctx: Tool execution context.
+    :param malicious_filename: A traversal/absolute filename to reject.
+    """
+    content = b"payload"
+    monkeypatch.setattr(
+        "omnigent.runtime.get_file_store",
+        lambda: _FakeFileStore(
+            [
+                _FakeFile(
+                    "file_evil",
+                    malicious_filename,
+                    len(content),
+                    "text/plain",
+                    1000,
+                    session_id="conv_alice",
+                ),
+            ]
+        ),
+    )
+    monkeypatch.setattr(
+        "omnigent.runtime.get_artifact_store",
+        lambda: _FakeArtifactStore({"file_evil": content}),
+    )
+
+    tool = DownloadFileTool()
+    result = json.loads(tool.invoke('{"file_id": "file_evil"}', tool_ctx))
+
+    # The write must land strictly inside the workspace.
+    saved = Path(result["path"])
+    workspace = tool_ctx.workspace
+    assert workspace is not None
+    assert saved.resolve().is_relative_to(workspace.resolve())
+    # Nothing was written outside the workspace.
+    assert not Path("/etc/passwd").is_symlink()
+    assert saved.exists()
+    assert saved.read_bytes() == content
+
+
+def test_download_file_basenames_store_filename(
+    monkeypatch: pytest.MonkeyPatch,
+    tool_ctx: ToolContext,
+) -> None:
+    """
+    A filename with leading directory components is saved by basename.
+
+    :param monkeypatch: Pytest monkeypatch fixture.
+    :param tool_ctx: Tool execution context.
+    """
+    content = b"report-bytes"
+    monkeypatch.setattr(
+        "omnigent.runtime.get_file_store",
+        lambda: _FakeFileStore(
+            [
+                _FakeFile(
+                    "file_nested",
+                    "reports/2026/q2.csv",
+                    len(content),
+                    "text/csv",
+                    1000,
+                    session_id="conv_alice",
+                ),
+            ]
+        ),
+    )
+    monkeypatch.setattr(
+        "omnigent.runtime.get_artifact_store",
+        lambda: _FakeArtifactStore({"file_nested": content}),
+    )
+
+    tool = DownloadFileTool()
+    result = json.loads(tool.invoke('{"file_id": "file_nested"}', tool_ctx))
+
+    saved = Path(result["path"])
+    workspace = tool_ctx.workspace
+    assert workspace is not None
+    assert saved.name == "q2.csv"
+    assert saved.parent.resolve() == workspace.resolve()
+    assert saved.read_bytes() == content
+
+
 def test_download_file_not_found(
     monkeypatch: pytest.MonkeyPatch,
     tool_ctx: ToolContext,