Found during PR #812 review.
`pkg/skill/skill.go:parseSkill` accepts any string in the `name:` frontmatter field without validation. `s.Name` then flows unsanitized into:
- `filepath.Join(".claude", "skills", s.Name, "SKILL.md")` (Claude renderer)
- `filepath.Join(".cursor", "rules", s.Name+".mdc")` (Cursor renderer)
- and similar paths in Codex / Gemini renderers.
Safe today because the catalog source is `//go:embed` only — every skill is compiled into the binary from a known directory tree under `pkg/skill/data/skills/`. There is no path to inject a malicious `name` field at runtime.
Becomes a real path-traversal vector the moment a future PR introduces:
- `holodeck skill add --file <local-SKILL.md>` (load skill from disk)
- a remote catalog fetcher (HTTP / OCI / Git)
- any other non-embed catalog source
At that point a SKILL.md with `name: ../../etc/cron.d/evil` would let `--global` installs write outside the intended directory.
Cheap fix, low risk, future-proofs the package:
```go
var skillNameRE = regexp.MustCompile(`^[a-z0-9][a-z0-9-]{0,63}$`)
// in parseSkill, after extracting name from frontmatter:
if !skillNameRE.MatchString(s.Name) {
return nil, fmt.Errorf("invalid skill name %q: must match %s", s.Name, skillNameRE)
}
```
Low priority — flag for a future hardening PR before opening the catalog source.
Posted via Team Lead workflow after PE review of PR #812.
Found during PR #812 review.
`pkg/skill/skill.go:parseSkill` accepts any string in the `name:` frontmatter field without validation. `s.Name` then flows unsanitized into:
Safe today because the catalog source is `//go:embed` only — every skill is compiled into the binary from a known directory tree under `pkg/skill/data/skills/`. There is no path to inject a malicious `name` field at runtime.
Becomes a real path-traversal vector the moment a future PR introduces:
At that point a SKILL.md with `name: ../../etc/cron.d/evil` would let `--global` installs write outside the intended directory.
Cheap fix, low risk, future-proofs the package:
```go
var skillNameRE = regexp.MustCompile(`^[a-z0-9][a-z0-9-]{0,63}$`)
// in parseSkill, after extracting name from frontmatter:
if !skillNameRE.MatchString(s.Name) {
return nil, fmt.Errorf("invalid skill name %q: must match %s", s.Name, skillNameRE)
}
```
Low priority — flag for a future hardening PR before opening the catalog source.
Posted via Team Lead workflow after PE review of PR #812.