fix(apps): fail fast on invalid custom-app zip + zip-slip hardening

[ClaydeCode]

Fixes #157.

Custom-app upload validated nothing but the .zip file extension. The archive was written straight into installed_apps/<uploaded-filename>/, a DB row was created and an install task queued — and only then did the worker find out whether the zip was an app at all. A zip whose files sat inside a top-level directory extracted to installed_apps/<name>/<name>/, so no manifest was found at the expected path and the app became "Unknown App", with a row and a directory already on disk. That is what a pilot user hit, and it ended in core failing to start.

What changed

Validate before creating any state. The upload is buffered into a temp dir and inspected there. It must be a readable zip carrying app_meta.json and docker-compose.yml.template at its root, and the manifest must parse into AppMeta. Anything else gets a 400 with a specific message and leaves no row, no directory, no queued task. Only a valid archive is moved into installed_apps/.

Files under a single top-level directory are flattened, not rejected. That is the shape you get from zipping a folder — the pilot user's exact mistake — and it is unambiguous, so extract_app_zip strips the prefix rather than failing. macOS __MACOSX/ resource forks and .DS_Store files are ignored; without that, a folder compressed in Finder looks like it has two top-level directories and the flattening would not have helped the very users this is for.

App name comes from app_meta.json, not from the uploaded filename.

Zip-slip hardening. extract_app_zip replaces the blind extractall(): members are written one at a time and any whose resolved path is not inside the target dir is refused. Store installs and reinstalls go through the same extractor. The app name from the manifest is also path-checked, since it becomes a directory name and a subdomain.

One note for the reviewer: zipfile.extractall() sanitizes ../ members itself, so the original code was not actually exploitable by classic zip slip. The check still earns its place — my per-member extraction gives up that built-in sanitization, and I verified the check has teeth by removing it and watching test_extract_rejects_path_traversal write a file outside the app dir.

Not addressed

Decompression bombs (a small zip declaring a huge app_meta.json) are unchanged from before — out of scope here, happy to file it separately if you want it.

Verification

tests/test_app_zip.py (24 unit tests) plus 5 new endpoint tests in tests/test_app_installation.py, covering each acceptance criterion end-to-end: a nested zip installs and runs, a zip without app_meta.json gets a 400 and leaves no row or directory, and the name comes from the manifest. 42 tests pass across test_app_zip, test_app_installation, test_app_store, test_service_init_apps, test_app_error.

Recommended reading order

1. shard_core/service/app_installation/exceptions.py — new InvalidAppZip
2. shard_core/service/app_installation/app_zip.py — the new validation + extraction module
3. shard_core/service/app_installation/worker.py — extraction now goes through it
4. shard_core/service/app_installation/__init__.py — install_app_from_uploaded_zip takes the validated zip
5. shard_core/web/protected/apps.py — the endpoint
6. tests/test_app_zip.py, tests/test_app_installation.py, agents.md

🤖 Generated with Claude Code