fix(skill): prevent zip-slip path traversal in extractZipToFS (CWE-22)

[sebastiondev]

Summary

This PR fixes a zip-slip path traversal vulnerability (CWE-22) in extractZipToFS in packages/browser-runtime/src/skill/lib/utils/zip-utils.ts. A malicious skill ZIP can contain entries with ../ segments (or absolute paths) that, when extracted, write outside the intended skill target directory inside the ZenFS virtual filesystem.

Vulnerability details

• File: packages/browser-runtime/src/skill/lib/utils/zip-utils.ts
• Function: extractZipToFS
• CWE: CWE-22 (Improper Limitation of a Pathname to a Restricted Directory — "Zip Slip")
• Data flow: skill-storage.saveSkill derives a targetPath like /skills/<name> (the name comes from the ZIP's own SKILL.md frontmatter) and calls extractZipToFS(zipData, targetPath). Pre-fix, each entry's path was joined directly: `${targetPath}/${relativePath}` and written to ZenFS without verifying the resolved location stayed within targetPath. An entry such as ../other-skill/SKILL.md therefore writes into a sibling skill directory.

Fix

Added a small normalizePath() helper (POSIX-style, no Node path dependency to keep it browser-safe) that resolves . and .. segments. Before writing any entry, the resolved full path is normalized and checked against the normalized target with a startsWith(target + "/") guard (the trailing slash prevents prefix-confusion attacks like /skills/foo matching /skills/foobar). Entries that escape the target are skipped with a warning and extraction continues for the remaining valid entries.

The same normalizedFullPath is then used for the mkdir and writeFile calls so the on-disk layout matches what was validated.

Tests

Added zip-utils.test.ts with 24 unit tests covering:

• normalizePath behavior on relative, absolute, dot, and double-dot segments, trailing slashes, and edge cases.
• Zip-slip rejection for ../escape, deep ../../../etc/passwd-style payloads, absolute entry paths (/etc/passwd), sibling-directory escape (../other-skill/SKILL.md), and prefix-confusion (/skills/foobar vs target /skills/foo).
• Positive cases confirming legitimate nested paths inside the target still extract correctly.

All tests pass locally. The diff is intentionally minimal — only zip-utils.ts and the new test file are touched.

 packages/browser-runtime/src/skill/lib/utils/zip-utils.test.ts | 181 +++++++++++++
 packages/browser-runtime/src/skill/lib/utils/zip-utils.ts      |  47 +++-
 2 files changed, 224 insertions(+), 4 deletions(-)

Security analysis

Preconditions: the user must install a crafted skill ZIP through the extension's skill-upload flow.

Why it matters even with that precondition: once a skill is installed, the runtime sandboxes it to its own directory and skills are expected to only touch their own files. Zip-slip breaks that boundary — a malicious skill can overwrite other installed skills' SKILL.md or scripts. That enables persistence and impersonation: even after the malicious skill itself is removed, modified files in unrelated skills remain. This is a real scope/privilege escalation within the skill system, distinct from "the skill you just installed runs code."

Scope of impact: limited to the ZenFS virtual filesystem inside the extension — no host-filesystem reach. Skills already execute scripts via chrome.scripting, so the marginal capability granted by zip-slip is "tamper with other skills," not new RCE. We'd characterize severity as moderate rather than critical, but worth fixing because the mitigation is small and self-contained.

Adversarial review: before submitting, we tried to disprove the finding. We checked whether saveSkill or upstream code already validates entry paths (it doesn't — the target is derived from the ZIP's own metadata and entries are trusted), whether ZenFS itself rejects .. segments (it normalizes and writes wherever the resolved path lands), and whether the fact that skills already run code makes this redundant. It isn't redundant: the attack surface is cross-skill tampering and persistence, which the runtime's per-skill sandboxing is otherwise meant to prevent.

Notes for review

• The normalizePath helper is intentionally inline rather than pulling in path-browserify, to keep the change minimal and dependency-free.
• The continue (skip + warn) behavior preserves backward compatibility for benign ZIPs while neutralizing malicious entries; happy to switch to a hard error if you'd prefer.

cc @lewiswigmore