Skip to content

fix(security): skill install writes through symlinks shipped in scripts/ #753

Open

Description

@luongnv89

Problem

executeInstall (src/installer-core.ts) copies an installed skill with fs.promises.cp({ recursive: true }), which keeps symlinks as they are, including absolute ones. git clone keeps them too. Then installScriptDependencies runs npm install in <skill>/scripts/, and npm writes through any symlinks the skill shipped:

  • scripts/package-lock.json -> <absolute path>: npm overwrites the target with lockfile JSON and keeps the target's file mode. Part of that JSON comes from the skill (e.g. a file: dependency's license string is copied in verbatim). If the target is an executable on PATH or a shell rc file, the skill-controlled text runs later. Reproduced in a sandbox on npm 11.19.0: a 0755 tool was overwritten, and running it from bash executed a $(touch PWNED) payload.
  • scripts/node_modules/.bin -> <absolute dir>: npm creates bin links (e.g. git) in the chosen directory.

The attacker has to guess an absolute path (e.g. /root, /home/runner, /usr/local/bin). This predates #751/#752 (it also exists on main). The hardening in #752 doesn't cover it, because it is a filesystem issue, not an npm config issue.

This breaks the goal in .oss-scanner/threat_model.md that installing a skill must not lead to running code from it.

Fix direction

  • During the copy in executeInstall, refuse (or skip, with a warning) symlinks that resolve outside the skill directory.
  • At minimum, before running npm, refuse to install script dependencies if scripts/package-lock.json, scripts/npm-shrinkwrap.json, or anything under scripts/node_modules is a symlink. Consider deleting any shipped scripts/node_modules.
  • Add a real-npm regression test: package-lock.json is a symlink to a temp-dir file, and the test asserts that file is unchanged. Keep the test sandboxed (see the companion issue about npm_config_prefix).

Acceptance Criteria

  • Installing a skill that ships a symlink pointing outside the skill directory doesn't write to the symlink's target.
  • npm is never run in a scripts/ directory whose lockfile or node_modules contains such symlinks.
  • A regression test fails without the fix.

Found during review of #752.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions