From 6afeea2beaa7bf14a0e3de4757b8669e68b54795 Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Tue, 23 Jun 2026 19:13:08 -0500 Subject: [PATCH] harden(pets): host-pin asset downloads + sanitize slug paths install_pet now refuses spritesheet/pet.json URLs that aren't on a petdex host (matching thumbnail_png's existing _is_petdex_host guard), so a spoofed manifest can't redirect a download at an arbitrary host. Slugs are normalized to a single path segment before indexing into pets_dir(), closing a path-traversal vector in load_pet/remove_pet/install_pet. --- agent/pet/store.py | 35 +++++++++++++++++++++++++++++++---- 1 file changed, 31 insertions(+), 4 deletions(-) diff --git a/agent/pet/store.py b/agent/pet/store.py index 46cc3bc9cac..8f9f8376865 100644 --- a/agent/pet/store.py +++ b/agent/pet/store.py @@ -84,9 +84,25 @@ def _resolve_spritesheet(directory: Path, meta: dict) -> Path: return directory / "spritesheet.webp" +def _safe_slug(slug: str) -> str: + """Normalize a slug to a single bare path segment. + + Pet slugs index into ``pets_dir()//`` for load/remove, so a value + carrying path separators (``../``, absolute paths) could escape the pets + directory. Strip every separator and reject ``.``/``..`` so callers can + only ever name a direct child of the pets directory. + """ + segment = Path(str(slug).strip()).name + if segment in ("", ".", ".."): + return "" + return segment + + def load_pet(slug: str) -> InstalledPet | None: """Return the :class:`InstalledPet` for *slug*, or ``None`` if absent.""" - slug = slug.strip() + slug = _safe_slug(slug) + if not slug: + return None directory = pets_dir() / slug if not directory.is_dir(): return None @@ -135,7 +151,9 @@ def install_pet(slug: str, *, force: bool = False, timeout: float = _DOWNLOAD_TI """ from agent.pet.manifest import find_entry - slug = slug.strip() + slug = _safe_slug(slug) + if not slug: + raise PetStoreError("invalid pet slug") existing = load_pet(slug) if existing and existing.exists and not force: return existing @@ -144,6 +162,12 @@ def install_pet(slug: str, *, force: bool = False, timeout: float = _DOWNLOAD_TI if entry is None: raise PetStoreError(f"pet '{slug}' is not in the petdex manifest") + # Host-pin every asset URL to petdex. The manifest is trusted (HTTPS from + # petdex.dev), but pin the asset hosts too so a compromised/spoofed manifest + # can't redirect the download at an arbitrary host. Matches thumbnail_png. + if not _is_petdex_host(entry.spritesheet_url): + raise PetStoreError(f"refusing non-petdex spritesheet host for '{slug}'") + directory = pets_dir() / slug directory.mkdir(parents=True, exist_ok=True) @@ -155,7 +179,7 @@ def install_pet(slug: str, *, force: bool = False, timeout: float = _DOWNLOAD_TI # Fetch the upstream pet.json if present; otherwise synthesize a minimal # one so the local layout is self-describing. meta: dict = {} - if entry.pet_json_url: + if entry.pet_json_url and _is_petdex_host(entry.pet_json_url): try: meta = _download_json(entry.pet_json_url, timeout=timeout) except Exception as exc: # noqa: BLE001 - non-fatal, fall back below @@ -274,7 +298,10 @@ def remove_pet(slug: str) -> bool: """Delete an installed pet directory. Returns True if anything was removed.""" import shutil - directory = pets_dir() / slug.strip() + slug = _safe_slug(slug) + if not slug: + return False + directory = pets_dir() / slug if not directory.is_dir(): return False shutil.rmtree(directory, ignore_errors=True)