fix: prevent namespace squatting via name-directory binding in skill discovery

[mmizutani]

Summary

Fixes a critical namespace squatting vulnerability in skill discovery that allows attackers to shadow legitimate skills with malicious ones in community skill repositories.

• Enforces name-directory binding in parseSkillMd() — overrides YAML frontmatter name: when it doesn't match the containing directory name (after sanitization)
• Adds duplicate name detection with warnings in discoverSkills() — replaces silent first-match-wins dedup
• Adds directory-name preference in filterSkills() as defense-in-depth — when multiple skills match a filter, prefers skills whose directory matches the requested name
• Extracts sanitizeName() to src/sanitize.ts to avoid circular dependency between skills.ts and installer.ts

Resolve #353

Motivation

The npx skills add command is vulnerable to a namespace squatting attack where:

1. An attacker submits a skill in a directory like aaa-attacker/bird-fake/ with YAML frontmatter name: bird (matching a legitimate skill at steipete/bird/)
2. discoverSkills() deduplicates by frontmatter name: with first-match-wins, and filesystem traversal order (readdir()) typically processes alphabetically-earlier directories first
3. The attacker's skill silently shadows the legitimate one — the user sees no warning

This vulnerability is being actively exploited in the wild. As reported on Reddit, the openclaw/skills community repository contains 100+ malicious skill submissions from accounts including sakaen736jih, gitgoodordietrying, dongsjoa-byte, and others. Reported payloads include C2 callbacks, SSH key injection, and binary downloads.

Root cause

Two design flaws combine to enable the attack:

Flaw	Location	Description
No name-directory binding	parseSkillMd()	YAML frontmatter name: is trusted without validation against the directory name
Silent first-match-wins dedup	discoverSkills()	Duplicate names are silently dropped based on filesystem traversal order

Changes

1. parseSkillMd() — name-directory binding (src/skills.ts)

Added an optional basePath parameter. When provided (from discoverSkills), compares sanitizeName(data.name) against sanitizeName(basename(dirname(skillMdPath))). On mismatch, emits a warning and overrides the name to match the directory.

Exceptions (no false positives):

• Root-level SKILL.md: When dirname(skillMdPath) === basePath (e.g., a single-skill repo clone), validation is skipped since the dirname is a temp directory, not a skill identity
• Legitimate case-mismatch: sanitizeName("My Skill") === "my-skill" — the original frontmatter name is preserved when sanitized forms match
• Direct callers without basePath: listInstalledSkills() and other existing callers that don't pass basePath are unaffected (backward compatible)

2. discoverSkills() — duplicate detection (src/skills.ts)

• Changed seenNames from Set<string> to Map<string, string> (name → path) for tracking
• When a duplicate name is detected, emits a warning showing the accepted and skipped paths
• Passes basePath: searchPath to all parseSkillMd() calls to activate name-directory validation

This applies at all three dedup sites: root skill, priority directory search, and fallback recursive search.

3. filterSkills() — directory-name preference (src/skills.ts)

When multiple skills match a user's filter input, prefers those whose basename(skill.path) matches the input after sanitization. This is a defense-in-depth layer that resolves ambiguity in favor of the skill at the expected directory path.

4. sanitizeName() extraction (src/sanitize.ts)

Extracted sanitizeName() from src/installer.ts to a new src/sanitize.ts module. Both skills.ts and installer.ts now import from sanitize.ts. The original export from installer.ts is preserved via re-export for backward compatibility.

How the fix neutralizes the attack

Before fix:
  aaa-attacker/bird-fake/SKILL.md  →  name: "bird"     ← attacker claims "bird"
  steipete/bird/SKILL.md           →  name: "bird"     ← legitimate, silently dropped
  User gets: attacker's skill installed as "bird"

After fix:
  aaa-attacker/bird-fake/SKILL.md  →  name: "bird-fake" ← overridden to match directory
  steipete/bird/SKILL.md           →  name: "bird"      ← accepted as legitimate
  User gets: legitimate skill installed as "bird"

Test plan

Added tests/namespace-squatting.test.ts with 12 test cases across 3 describe blocks:

parseSkillMd name-directory validation (6 tests)

• Overrides name when frontmatter name doesn't match directory
• Keeps name when frontmatter matches directory exactly
• Keeps original name when match after sanitization (case/spaces)
• Skips validation for root-level SKILL.md (basePath)
• No validation when basePath not provided (backward compat)
• Validates non-root skill even when basePath is provided

discoverSkills namespace squatting prevention (2 tests)

• Prevents attacker from shadowing legitimate skill (end-to-end)
• Warns about duplicate skill names from different directories

filterSkills directory name preference (4 tests)

• Prefers directory-matching skill when multiple share a name
• Returns all matches when no directory matches the filter
• Works normally for single-match scenarios
• Case-insensitive matching regression test

Updated existing tests that relied on name-directory mismatches:

• tests/plugin-manifest-discovery.test.ts — 5 test cases updated to use consistent name-directory bindings
• src/add.test.ts — 1 test case updated for consistent naming

All 299 applicable tests pass.

Files changed

File	Lines	Description
src/sanitize.ts	+22	New module: extracted sanitizeName()
src/skills.ts	+77 −17	Core fix: name-directory binding, duplicate detection, dir preference
src/installer.ts	+3 −26	Import + re-export sanitizeName from sanitize.ts
tests/namespace-squatting.test.ts	+233	New: security fix test suite
tests/plugin-manifest-discovery.test.ts	+12 −12	Updated for name-directory consistency
src/add.test.ts	+4 −4	Updated for name-directory consistency

Security considerations

• No breaking changes for legitimate skills: Skills where name matches the directory (or matches after sanitization) are unaffected
• Backward compatible: Callers of parseSkillMd() that don't pass basePath (e.g., listInstalledSkills()) retain original behavior
• Defense-in-depth: Three independent layers — name binding, duplicate warnings, and filter preference — each individually breaks the exploit chain
• Warning, not rejection: Mismatched skills are still installed (under the directory name) rather than silently dropped, maintaining availability while removing the attacker's ability to impersonate

Related

• Reddit disclosure: PSA: OpenClaw's skills are compromised!
• Affected community repository: openclaw/skills

[vercel]

@mmizutani is attempting to deploy a commit to the Vercel Team on Vercel.

A member of the Team first needs to authorize it.