Merge pull request #16 from musher-dev/fix/security-integrity-audit

fix: resolve path traversal, cache integrity, and HTTP parsing vulnerabilities

Author: justinmerrell

README.md

@@ -157,8 +157,8 @@ Cache management functions are available at the module level and on the `Client`
 import musher
 
 info = musher.cache_info()  # cache statistics
-musher.cache_remove("myorg/my-bundle:1.0.0")  # remove a specific bundle
-musher.cache_clean()  # remove expired entries
+musher.cache_remove("myorg/my-bundle:1.0.0")  # remove cached metadata for a bundle version
+musher.cache_clean()  # reclaim expired entries and unreferenced blobs
 musher.cache_clear()  # remove all cached data
 path = musher.cache_path()  # cache directory path
 ```

docs/configuration.md

@@ -16,7 +16,9 @@ The SDK uses platform-aware directory resolution with the following precedence:
 | config | `MUSHER_CONFIG_HOME` | `~/.config/musher` | `~/Library/Application Support/musher` | `%LOCALAPPDATA%\musher\config` |
 | data | `MUSHER_DATA_HOME` | `~/.local/share/musher` | `~/Library/Application Support/musher` | `%LOCALAPPDATA%\musher\data` |
 | state | `MUSHER_STATE_HOME` | `~/.local/state/musher` | `~/Library/Application Support/musher` | `%LOCALAPPDATA%\musher\state` |
-| runtime | `MUSHER_RUNTIME_DIR` | `$XDG_RUNTIME_DIR/musher` | `~/Library/Caches/TemporaryItems/musher` | `%LOCALAPPDATA%\musher\runtime` |
+| runtime | `MUSHER_RUNTIME_DIR` | `$XDG_RUNTIME_DIR/musher` | `<tempdir>/musher/run`* | `<tempdir>\musher\run`* |
+
+\* `<tempdir>` is the system temporary directory (e.g. `/tmp` on macOS, `C:\Users\<user>\AppData\Local\Temp` on Windows).
 
 On Windows, the SDK uses a flat layout under `%LOCALAPPDATA%\musher\` with category subdirectories rather than relying on `platformdirs`, which maps some categories to the same physical path.
 

src/musher/__init__.py

@@ -23,7 +23,6 @@
     MusherError,
     RateLimitError,
     RegistryError,
-    VersionNotFoundError,
 )
 from musher._export import ClaudePluginExport, OpenAIInlineSkill, OpenAILocalSkill
 from musher._handles import (
@@ -81,7 +80,6 @@
     "ResolveResult",
     "SkillHandle",
     "ToolsetHandle",
-    "VersionNotFoundError",
     "__version__",
     "cache_clean",
     "cache_clear",

src/musher/_bundle.py

@@ -138,6 +138,8 @@ def _place_skill_file(
         candidate_root = str(PurePosixPath(*parts[:i]))
         if f"{candidate_root}/SKILL.md" in assets:
             rel = str(PurePosixPath(*parts[i:]))
+            if ".." in PurePosixPath(rel).parts:
+                return
             skill_roots.setdefault(candidate_root, {})[rel] = file_handles[asset.logical_path]
             return
     # Top-level skill file without nested directory

src/musher/_cache.py

@@ -365,6 +365,7 @@ def clear(self) -> None:
         """Remove all cached data."""
         if self._cache_dir.is_dir():
             shutil.rmtree(self._cache_dir)
+        self._tag_written = False
 
     # ── Internal ───────────────────────────────────────────────────
 

src/musher/_client.py

@@ -189,6 +189,10 @@ async def pull(self, ref: str) -> Bundle:
         for layer in result.manifest.layers:
             cached_blob = self._cache.get_blob(layer.content_sha256)
             if cached_blob is not None:
+                if self._config.verify_checksums:
+                    actual_sha = hashlib.sha256(cached_blob).hexdigest()
+                    if actual_sha != layer.content_sha256:
+                        raise IntegrityError(expected=layer.content_sha256, actual=actual_sha)
                 assets[layer.logical_path] = Asset(
                     asset_id=layer.asset_id,
                     logical_path=layer.logical_path,
@@ -233,25 +237,33 @@ def _build_assets_from_pull(
             content = str(item.get("contentText", "")).encode()
             layer = layer_map.get(logical_path)
 
-            # Verify checksum against the resolve manifest
-            if layer and self._config.verify_checksums:
-                actual_sha = hashlib.sha256(content).hexdigest()
-                if actual_sha != layer.content_sha256:
-                    raise IntegrityError(expected=layer.content_sha256, actual=actual_sha)
+            # Always compute actual hash — never cache under an unverified claimed hash
+            actual_sha = hashlib.sha256(content).hexdigest()
 
-            content_sha256 = layer.content_sha256 if layer else hashlib.sha256(content).hexdigest()
-            self._cache.put_blob(content_sha256, content)
+            if layer and self._config.verify_checksums and actual_sha != layer.content_sha256:
+                raise IntegrityError(expected=layer.content_sha256, actual=actual_sha)
+
+            self._cache.put_blob(actual_sha, content)
 
             media_type = str(item.get("mediaType") or "") or (layer.media_type if layer else None)
             assets[logical_path] = Asset(
                 asset_id=layer.asset_id if layer else logical_path,
                 logical_path=logical_path,
                 asset_type=AssetType(str(item["assetType"])),
                 content=content,
-                content_sha256=content_sha256,
+                content_sha256=actual_sha,
                 size_bytes=layer.size_bytes if layer else len(content),
                 media_type=media_type or None,
             )
+
+        # Enforce manifest completeness — all expected layers must be present
+        missing = set(layer_map.keys()) - set(assets.keys())
+        if missing:
+            raise IntegrityError(
+                expected=f"all {len(layer_map)} manifest layers",
+                actual=f"missing {len(missing)} layers: {', '.join(sorted(missing))}",
+            )
+
         return assets
 
     async def _pull_version(self, namespace: str, slug: str, version: str) -> dict[str, object]:

src/musher/_handles.py

@@ -8,12 +8,20 @@
 import tempfile
 import zipfile
 from dataclasses import dataclass, field
-from pathlib import Path
+from pathlib import Path, PurePosixPath
 from typing import cast
 
 from musher._export import ClaudePluginExport, OpenAIInlineSkill, OpenAILocalSkill
 
 
+def _validate_relative_path(relative_path: str) -> None:
+    """Reject paths that could escape the target directory."""
+    p = PurePosixPath(relative_path)
+    if p.is_absolute() or ".." in p.parts:
+        msg = f"Unsafe relative path in skill: {relative_path}"
+        raise ValueError(msg)
+
+
 @dataclass(frozen=True, slots=True)
 class FileHandle:
     """Typed handle to a single file within a bundle."""
@@ -66,6 +74,7 @@ def export_openai_inline_skill(self) -> OpenAIInlineSkill:
         buf = io.BytesIO()
         with zipfile.ZipFile(buf, "w", zipfile.ZIP_DEFLATED) as zf:
             for relative_path, fh in self._files.items():
+                _validate_relative_path(relative_path)
                 zf.writestr(f"{self.name}/{relative_path}", fh.bytes())
         content_b64 = base64.b64encode(buf.getvalue()).decode("ascii")
         return OpenAIInlineSkill(
@@ -81,6 +90,7 @@ def export_path(self, dest: Path | None = None) -> Path:
 
         skill_dir = dest / self.name
         for relative_path, fh in self._files.items():
+            _validate_relative_path(relative_path)
             out = skill_dir / relative_path
             out.parent.mkdir(parents=True, exist_ok=True)
             _ = out.write_bytes(fh.bytes())
@@ -95,6 +105,7 @@ def export_zip(self, dest: Path | None = None) -> Path:
         zip_path = dest / f"{self.name}.zip"
         with zipfile.ZipFile(zip_path, "w", zipfile.ZIP_DEFLATED) as zf:
             for relative_path, fh in self._files.items():
+                _validate_relative_path(relative_path)
                 zf.writestr(f"{self.name}/{relative_path}", fh.bytes())
         return zip_path
 

src/musher/_http.py

@@ -2,6 +2,8 @@
 
 from __future__ import annotations
 
+from datetime import UTC, datetime
+from email.utils import parsedate_to_datetime
 from typing import TYPE_CHECKING
 
 import httpx
@@ -63,6 +65,19 @@ async def close(self) -> None:
             self._client = None
 
 
+def _parse_retry_after(header: str) -> float | None:
+    """Parse Retry-After as delay-seconds or HTTP-date (RFC 9110)."""
+    try:
+        return float(header)
+    except ValueError:
+        pass
+    try:
+        dt = parsedate_to_datetime(header)
+        return max(0.0, (dt - datetime.now(tz=UTC)).total_seconds())
+    except (ValueError, TypeError):
+        return None
+
+
 def _raise_for_status(response: httpx.Response) -> None:
     """Map HTTP error responses to SDK exceptions."""
     if response.is_success:
@@ -78,7 +93,9 @@ def _raise_for_status(response: httpx.Response) -> None:
 
     if status == 429:  # noqa: PLR2004
         retry_after_header: str | None = response.headers.get("Retry-After")  # pyright: ignore[reportAny]
-        raise RateLimitError(retry_after=float(retry_after_header) if retry_after_header else None)
+        raise RateLimitError(
+            retry_after=_parse_retry_after(retry_after_header) if retry_after_header else None
+        )
 
     # Try RFC 9457 Problem Details
     try:

tests/test_cache.py

@@ -238,6 +238,18 @@ def test_clear_noop_if_missing(self, tmp_path: Path):
         cache = BundleCache(cache_dir=tmp_path / "nonexistent")
         cache.clear()  # should not raise
 
+    def test_clear_then_write_recreates_cachedir_tag(self, tmp_path: Path):
+        cache = BundleCache(cache_dir=tmp_path)
+        cache.put_blob("ab" * 32, b"data")
+        tag = tmp_path / "CACHEDIR.TAG"
+        assert tag.is_file()
+        cache.clear()
+        assert not tag.exists()
+        # Write again on the same instance — tag must be recreated
+        cache.put_blob("cd" * 32, b"data2")
+        assert tag.is_file()
+        assert tag.read_text().startswith("Signature: 8a477f597d28d172789f06886806bc55")
+
 
 class TestBlobOverwrite:
     def test_blob_overwrite_succeeds(self, tmp_path: Path):

tests/test_errors.py

@@ -9,8 +9,8 @@
     MusherError,
     RateLimitError,
     RegistryError,
-    VersionNotFoundError,
 )
+from musher._errors import VersionNotFoundError
 
 
 class TestHierarchy: