fix(security): add path traversal protection to install_pack metadata

The `install_pack()` method read `name` and `version` fields from the
`pack.json` inside an uploaded zip archive and used them unsanitized to
construct the installation directory path. A malicious zip could contain
`"name": "../../arbitrary/path"` in its metadata, causing files to be
written outside the intended packs directory.

This patch adds:
- A `_validate_pack_segment()` helper that rejects empty values,
  `.`/`..`, and any characters outside `[a-zA-Z0-9._-]`.
- `resolve_under_root()` from `safe_io` as a second-layer defense to
  ensure the resolved path stays within `packs_dir`.
- Regression tests in `test_packs.py` covering traversal via zip
  metadata name, version, and dotdot segments, plus a positive test
  confirming valid packs still install correctly.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Bentley
2026-02-16 06:10:11 +08:00
committed by rookiestar28
co-authored by Claude Opus 4.6
parent 2049af4ae8
commit c796dde48e
2 changed files with 62 additions and 2 deletions
+9 -2
View File
@@ -3,7 +3,8 @@ import re
import shutil
from typing import Dict, List, Optional
from ..safe_io import PathTraversalError, resolve_under_root
<<<<<<< HEAD
from ..safe_io import resolve_under_root
from .pack_archive import PackArchive, PackError
from .pack_types import PackMetadata
@@ -52,7 +53,13 @@ class PackRegistry:
name = meta["name"]
version = meta["version"]
target_dir = os.path.join(self.packs_dir, name, version)
# Validate name/version from zip metadata against path traversal.
# A malicious pack.json could contain traversal sequences.
_validate_pack_segment(name, "name")
_validate_pack_segment(version, "version")
target_dir = resolve_under_root(
self.packs_dir, os.path.join(name, version)
)
if os.path.exists(target_dir):
if not overwrite:
+53
View File
@@ -145,5 +145,58 @@ class TestPackRegistryPathTraversal(unittest.TestCase):
self.registry.uninstall_pack("legit-name", "..")
class TestPackRegistryInstallTraversal(unittest.TestCase):
"""Test that install_pack rejects traversal sequences in zip metadata."""
def setUp(self):
self.test_dir = tempfile.mkdtemp()
self.registry = PackRegistry(self.test_dir)
def tearDown(self):
shutil.rmtree(self.test_dir)
def _make_pack_zip(self, name, version):
"""Create a minimal valid pack zip with the given name/version in metadata."""
import hashlib
zip_path = os.path.join(self.test_dir, "test.zip")
pack_meta = {
"name": name,
"version": version,
"type": "preset",
"author": "tester",
"min_moltbot_version": "0.1.0",
}
pack_json = json.dumps(pack_meta).encode("utf-8")
pack_hash = hashlib.sha256(pack_json).hexdigest()
manifest = {"files": [{"path": "pack.json", "sha256": pack_hash}]}
with zipfile.ZipFile(zip_path, "w") as zf:
zf.writestr("pack.json", pack_json)
zf.writestr("manifest.json", json.dumps(manifest))
return zip_path
def test_install_rejects_traversal_in_name(self):
zip_path = self._make_pack_zip("../../etc", "1.0.0")
with self.assertRaises(PackError):
self.registry.install_pack(zip_path)
def test_install_rejects_traversal_in_version(self):
zip_path = self._make_pack_zip("legit-pack", "../../../tmp")
with self.assertRaises(PackError):
self.registry.install_pack(zip_path)
def test_install_rejects_dotdot_name(self):
zip_path = self._make_pack_zip("..", "1.0.0")
with self.assertRaises(PackError):
self.registry.install_pack(zip_path)
def test_install_accepts_valid_metadata(self):
zip_path = self._make_pack_zip("my-pack", "1.0.0")
meta = self.registry.install_pack(zip_path)
self.assertEqual(meta["name"], "my-pack")
self.assertEqual(meta["version"], "1.0.0")
if __name__ == "__main__":
unittest.main()