Files
executus/skillpack/manifest_test.go
steve 9bb5d143f7
executus CI / test (pull_request) Successful in 3m30s
fix(skillpack): address review — symlink read, git arg-injection, dup-subscribe, nil panics
Real issues from the PR review:
- security: readTree now skips symlinks (a pack with SKILL.md -> /etc/passwd
  or scripts/x -> ../../.ssh/id_rsa could read host files); covers file and
  dir symlinks, incl. within a git subpath
- security: GitSource rejects url/ref beginning with '-' (git arg injection)
  and clones with '--' separator; --filter=blob:none (blobless partial clone)
  instead of full-history clone
- correctness: Subscribe no longer swallows a non-ErrNotFound store error from
  GetByName (would create a duplicate subscription); handles *GitSource as well
  as GitSource in the URL/subpath extraction
- correctness: pinTo no longer renames a subscription, so Apply can't silently
  collide two subscriptions when an upstream pack changes its name
- validation: isKebab rejects leading/trailing/consecutive hyphens; BOM-
  prefixed SKILL.md now parses (matches the doc comment)
- robustness: Catalog/Activate/renderPackBody/Stage guard nil/malformed packs
- test cleanup: Syncer.Store field renamed Cache (collided with the Store
  interface); test NewID returns distinct ids
- tests: symlink-skip, BOM, strict-kebab, nil-pack-safety

Deferred (advisory perf, documented): PackCache stores raw trees so activation
re-parses; CheckAll is serial. Both fine at expected scale.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-04 20:41:44 -04:00

97 lines
2.8 KiB
Go

package skillpack
import (
"strings"
"testing"
)
const goodManifest = `---
name: pdf-processing
description: Extract text and tables from PDF files and fill forms.
license: MIT
allowed-tools: [Read, Bash]
metadata:
version: 1.2.0
---
# PDF Processing
Use pdfplumber for extraction.
`
func TestParseManifest_Good(t *testing.T) {
m, err := ParseManifest([]byte(goodManifest))
if err != nil {
t.Fatalf("ParseManifest: %v", err)
}
if m.Name != "pdf-processing" {
t.Errorf("name = %q", m.Name)
}
if !strings.HasPrefix(m.Description, "Extract text") {
t.Errorf("description = %q", m.Description)
}
if m.License != "MIT" {
t.Errorf("license = %q", m.License)
}
if len(m.AllowedTools) != 2 || m.AllowedTools[0] != "Read" || m.AllowedTools[1] != "Bash" {
t.Errorf("allowed-tools = %v", m.AllowedTools)
}
if m.Metadata["version"] != "1.2.0" {
t.Errorf("metadata version = %q", m.Metadata["version"])
}
if !strings.Contains(m.Body, "pdfplumber") || strings.Contains(m.Body, "---") {
t.Errorf("body not cleanly extracted: %q", m.Body)
}
}
func TestParseManifest_AllowedToolsScalar(t *testing.T) {
m, err := ParseManifest([]byte("---\nname: n\ndescription: d\nallowed-tools: \"Read, Bash , Grep\"\n---\nbody\n"))
if err != nil {
t.Fatal(err)
}
if len(m.AllowedTools) != 3 || m.AllowedTools[2] != "Grep" {
t.Errorf("scalar allowed-tools = %v", m.AllowedTools)
}
}
func TestParseManifest_Errors(t *testing.T) {
cases := map[string]string{
"no frontmatter": "# just a heading\n",
"unterminated": "---\nname: x\ndescription: y\n",
"missing name": "---\ndescription: y\n---\nb\n",
"missing desc": "---\nname: x\n---\nb\n",
"bad name uppercase": "---\nname: PdfProcessing\ndescription: d\n---\nb\n",
"bad name space": "---\nname: pdf processing\ndescription: d\n---\nb\n",
"bad name leading -": "---\nname: -pdf\ndescription: d\n---\nb\n",
"bad name trailing-": "---\nname: pdf-\ndescription: d\n---\nb\n",
"bad name double -": "---\nname: pdf--tools\ndescription: d\n---\nb\n",
"bad yaml": "---\nname: [unclosed\n---\nb\n",
}
for label, in := range cases {
if _, err := ParseManifest([]byte(in)); err == nil {
t.Errorf("%s: expected error, got nil", label)
}
}
}
func TestParseManifest_LeadingBlanksAndCRLF(t *testing.T) {
in := "\r\n\n---\r\nname: ok-name\r\ndescription: fine\r\n---\r\nbody line\r\n"
m, err := ParseManifest([]byte(in))
if err != nil {
t.Fatalf("tolerant parse: %v", err)
}
if m.Name != "ok-name" || m.Body != "body line" {
t.Errorf("got name=%q body=%q", m.Name, m.Body)
}
}
func TestParseManifest_BOM(t *testing.T) {
in := append([]byte{0xEF, 0xBB, 0xBF}, []byte("---\nname: bom-ok\ndescription: d\n---\nbody\n")...)
m, err := ParseManifest(in)
if err != nil {
t.Fatalf("BOM-prefixed SKILL.md should parse: %v", err)
}
if m.Name != "bom-ok" {
t.Errorf("name = %q", m.Name)
}
}