Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions oci/skills/errors.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// SPDX-FileCopyrightText: Copyright 2026 Stacklok, Inc.
// SPDX-License-Identifier: Apache-2.0

package skills

import "errors"

// Sentinel errors returned (wrapped) by the packager so callers can classify
// failures with errors.Is instead of matching error message strings. The
// underlying error message is preserved at each call site via fmt.Errorf
// with %w; only the classification is added.
var (
// ErrInvalidSkillDir indicates the skill directory is missing, not a
// directory, or otherwise unsafe to read (e.g. contains path traversal).
ErrInvalidSkillDir = errors.New("invalid skill directory")

// ErrSkillMDMissing indicates SKILL.md is not present in the skill
// directory.
ErrSkillMDMissing = errors.New("SKILL.md missing")

// ErrInvalidFrontmatter indicates the SKILL.md YAML frontmatter is
// missing, malformed, oversized, or missing required fields such as
// the skill name.
ErrInvalidFrontmatter = errors.New("invalid SKILL.md frontmatter")

// ErrTooManyFiles indicates the skill directory exceeds the maximum
// allowed number of files.
ErrTooManyFiles = errors.New("too many files in skill directory")

// ErrSkillTooLarge indicates the skill directory exceeds the maximum
// allowed total size.
ErrSkillTooLarge = errors.New("skill directory too large")

// ErrInvalidSkillFile indicates a per-file issue inside the skill
// directory: a symlink, a non-regular file, or an unreadable entry.
ErrInvalidSkillFile = errors.New("invalid skill file")
)
30 changes: 15 additions & 15 deletions oci/skills/packager.go
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ func readSkillDirectory(dir string) (*skillDirContent, error) {
skillMD, err := os.ReadFile(skillMDPath) //#nosec G304 -- path constructed from user-provided skill directory
if err != nil {
if os.IsNotExist(err) {
return nil, fmt.Errorf("SKILL.md not found in skill directory")
return nil, fmt.Errorf("SKILL.md not found in skill directory: %w", ErrSkillMDMissing)
}
return nil, fmt.Errorf("reading SKILL.md: %w", err)
}
Expand All @@ -238,7 +238,7 @@ func readSkillDirectory(dir string) (*skillDirContent, error) {
}

if fm.Name == "" {
return nil, fmt.Errorf("skill name is required in SKILL.md frontmatter")
return nil, fmt.Errorf("skill name is required in SKILL.md frontmatter: %w", ErrInvalidFrontmatter)
}

files, err := collectSkillFiles(dir)
Expand All @@ -258,17 +258,17 @@ func validateSkillDir(dir string) error {
info, err := os.Stat(dir)
if err != nil {
if os.IsNotExist(err) {
return fmt.Errorf("skill directory not found: %s", dir)
return fmt.Errorf("skill directory not found: %s: %w", dir, ErrInvalidSkillDir)
}
return fmt.Errorf("accessing skill directory: %w", err)
return fmt.Errorf("accessing skill directory: %w: %w", err, ErrInvalidSkillDir)
}
if !info.IsDir() {
return fmt.Errorf("path is not a directory: %s", dir)
return fmt.Errorf("path is not a directory: %s: %w", dir, ErrInvalidSkillDir)
}

cleanDir := filepath.Clean(dir)
if strings.Contains(cleanDir, "..") {
return fmt.Errorf("invalid path: contains path traversal")
return fmt.Errorf("invalid path: contains path traversal: %w", ErrInvalidSkillDir)
}

return nil
Expand Down Expand Up @@ -305,7 +305,7 @@ func collectSkillFiles(dir string) (map[string][]byte, error) {

// Security: reject symlinked directories (WalkDir follows them silently)
if d.Type()&os.ModeSymlink != 0 {
return fmt.Errorf("symlinks not allowed in skill directory: %s", relPath)
return fmt.Errorf("symlinks not allowed in skill directory: %s: %w", relPath, ErrInvalidSkillFile)
}

if d.IsDir() {
Expand All @@ -322,7 +322,7 @@ func collectSkillFiles(dir string) (map[string][]byte, error) {
}

if len(files) >= maxSkillFiles {
return fmt.Errorf("skill directory exceeds maximum of %d files", maxSkillFiles)
return fmt.Errorf("skill directory exceeds maximum of %d files: %w", maxSkillFiles, ErrTooManyFiles)
}

content, err := os.ReadFile(path) //#nosec G304,G122 -- path from WalkDir, symlink-checked
Expand All @@ -332,7 +332,7 @@ func collectSkillFiles(dir string) (map[string][]byte, error) {

totalSize += int64(len(content))
if totalSize > maxSkillTotalSize {
return fmt.Errorf("skill directory exceeds maximum total size of %d bytes", maxSkillTotalSize)
return fmt.Errorf("skill directory exceeds maximum total size of %d bytes: %w", maxSkillTotalSize, ErrSkillTooLarge)
}

files[relPath] = content
Expand All @@ -351,10 +351,10 @@ func validateSkillFile(absPath, relPath string) error {
return fmt.Errorf("checking file type for %s: %w", relPath, err)
}
if fileInfo.Mode()&os.ModeSymlink != 0 {
return fmt.Errorf("symlinks not allowed in skill directory: %s", relPath)
return fmt.Errorf("symlinks not allowed in skill directory: %s: %w", relPath, ErrInvalidSkillFile)
}
if !fileInfo.Mode().IsRegular() {
return fmt.Errorf("non-regular file not allowed in skill directory: %s", relPath)
return fmt.Errorf("non-regular file not allowed in skill directory: %s: %w", relPath, ErrInvalidSkillFile)
}
return nil
}
Expand All @@ -365,26 +365,26 @@ func parseFrontmatter(content []byte) (*frontmatter, error) {

delimiter := []byte("---")
if !bytes.HasPrefix(content, delimiter) {
return nil, fmt.Errorf("SKILL.md must start with YAML frontmatter (---)")
return nil, fmt.Errorf("SKILL.md must start with YAML frontmatter (---): %w", ErrInvalidFrontmatter)
}

rest := content[len(delimiter):]
rest = bytes.TrimPrefix(rest, []byte("\n"))

endIdx := bytes.Index(rest, delimiter)
if endIdx == -1 {
return nil, fmt.Errorf("SKILL.md frontmatter missing closing delimiter (---)")
return nil, fmt.Errorf("SKILL.md frontmatter missing closing delimiter (---): %w", ErrInvalidFrontmatter)
}

fmBytes := rest[:endIdx]

if len(fmBytes) > maxFrontmatterSize {
return nil, fmt.Errorf("frontmatter exceeds maximum size of %d bytes", maxFrontmatterSize)
return nil, fmt.Errorf("frontmatter exceeds maximum size of %d bytes: %w", maxFrontmatterSize, ErrInvalidFrontmatter)
}

var fm frontmatter
if err := yaml.Unmarshal(fmBytes, &fm); err != nil {
return nil, fmt.Errorf("parsing frontmatter YAML: %w", err)
return nil, fmt.Errorf("parsing frontmatter YAML: %w: %w", err, ErrInvalidFrontmatter)
}

return &fm, nil
Expand Down
213 changes: 197 additions & 16 deletions oci/skills/packager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package skills

import (
"bytes"
"context"
"encoding/json"
"fmt"
Expand Down Expand Up @@ -331,14 +332,7 @@ func TestPackager_Package_RejectsSymlinks(t *testing.T) {
t.Parallel()

dir := t.TempDir()
skillMD := `---
name: test-skill
description: A test skill
version: 1.0.0
---
# Test Skill
`
require.NoError(t, os.WriteFile(filepath.Join(dir, "SKILL.md"), []byte(skillMD), 0600))
writeValidSkillMD(t, dir)
require.NoError(t, os.Symlink("/etc/passwd", filepath.Join(dir, "evil_link")))

store, err := NewStore(t.TempDir())
Expand All @@ -356,14 +350,7 @@ func TestPackager_Package_RejectsSymlinkedDirectory(t *testing.T) {
t.Parallel()

dir := t.TempDir()
skillMD := `---
name: test-skill
description: A test skill
version: 1.0.0
---
# Test Skill
`
require.NoError(t, os.WriteFile(filepath.Join(dir, "SKILL.md"), []byte(skillMD), 0600))
writeValidSkillMD(t, dir)
require.NoError(t, os.Symlink("/etc", filepath.Join(dir, "evil_dir")))

store, err := NewStore(t.TempDir())
Expand Down Expand Up @@ -571,6 +558,187 @@ version: 1.0.0
_, err := collectSkillFiles(dir)
require.Error(t, err)
assert.Contains(t, err.Error(), "exceeds maximum")
assert.ErrorIs(t, err, ErrTooManyFiles)
}

func TestPackager_Package_SentinelErrors(t *testing.T) {
t.Parallel()

tests := []struct {
name string
setup func(t *testing.T) string
wantErr error
}{
{
name: "missing skill directory",
setup: func(t *testing.T) string {
t.Helper()
return filepath.Join(t.TempDir(), "does-not-exist")
},
wantErr: ErrInvalidSkillDir,
},
{
name: "path is file not directory",
setup: func(t *testing.T) string {
t.Helper()
f := filepath.Join(t.TempDir(), "not-a-dir")
require.NoError(t, os.WriteFile(f, []byte("x"), 0600))
return f
},
wantErr: ErrInvalidSkillDir,
},
{
name: "path contains traversal",
setup: func(_ *testing.T) string {
return "../no-such-skill-dir"
},
wantErr: ErrInvalidSkillDir,
},
{
name: "missing SKILL.md",
setup: func(t *testing.T) string {
t.Helper()
return t.TempDir()
},
wantErr: ErrSkillMDMissing,
},
{
name: "frontmatter missing opening delimiter",
setup: func(t *testing.T) string {
t.Helper()
dir := t.TempDir()
require.NoError(t, os.WriteFile(
filepath.Join(dir, "SKILL.md"),
[]byte("# no frontmatter\n"),
0600,
))
return dir
},
wantErr: ErrInvalidFrontmatter,
},
{
name: "frontmatter missing closing delimiter",
setup: func(t *testing.T) string {
t.Helper()
dir := t.TempDir()
require.NoError(t, os.WriteFile(
filepath.Join(dir, "SKILL.md"),
[]byte("---\nname: test\n# never closed"),
0600,
))
return dir
},
wantErr: ErrInvalidFrontmatter,
},
{
name: "frontmatter exceeds size limit",
setup: func(t *testing.T) string {
t.Helper()
dir := t.TempDir()
var buf bytes.Buffer
buf.WriteString("---\nname: test\nfiller: ")
buf.Write(bytes.Repeat([]byte("a"), maxFrontmatterSize+1))
buf.WriteString("\n---\n# body\n")
require.NoError(t, os.WriteFile(filepath.Join(dir, "SKILL.md"), buf.Bytes(), 0600))
return dir
},
wantErr: ErrInvalidFrontmatter,
},
{
name: "frontmatter invalid YAML",
setup: func(t *testing.T) string {
t.Helper()
dir := t.TempDir()
require.NoError(t, os.WriteFile(
filepath.Join(dir, "SKILL.md"),
[]byte("---\nname: [unclosed\n---\n# body\n"),
0600,
))
return dir
},
wantErr: ErrInvalidFrontmatter,
},
{
name: "frontmatter missing name",
setup: func(t *testing.T) string {
t.Helper()
dir := t.TempDir()
require.NoError(t, os.WriteFile(
filepath.Join(dir, "SKILL.md"),
[]byte("---\ndescription: nameless skill\n---\n# body\n"),
0600,
))
return dir
},
wantErr: ErrInvalidFrontmatter,
},
{
name: "symlinked file in skill directory",
setup: func(t *testing.T) string {
t.Helper()
dir := t.TempDir()
writeValidSkillMD(t, dir)
require.NoError(t, os.Symlink("/etc/passwd", filepath.Join(dir, "evil_link")))
return dir
},
wantErr: ErrInvalidSkillFile,
},
{
name: "symlinked directory in skill directory",
setup: func(t *testing.T) string {
t.Helper()
dir := t.TempDir()
writeValidSkillMD(t, dir)
require.NoError(t, os.Symlink("/etc", filepath.Join(dir, "evil_dir")))
return dir
},
wantErr: ErrInvalidSkillFile,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()

store, err := NewStore(t.TempDir())
require.NoError(t, err)
packager := NewPackager(store)
opts := PackageOptions{Epoch: time.Unix(0, 0).UTC()}

_, err = packager.Package(context.Background(), tt.setup(t), opts)
require.Error(t, err)
assert.ErrorIs(t, err, tt.wantErr)
})
}
}

// TestCollectSkillFiles_ExceedsMaxSize verifies that the total-size limit
// surfaces ErrSkillTooLarge. Kept separate from the table-driven test because
// it writes >100 MiB to disk.
func TestCollectSkillFiles_ExceedsMaxSize(t *testing.T) {
t.Parallel()

dir := t.TempDir()
writeValidSkillMD(t, dir)

// Stream-write a single file just over maxSkillTotalSize using a 1 MiB
// buffer so we don't hold the whole payload in memory at once.
f, err := os.Create(filepath.Join(dir, "big.bin")) //#nosec G304 -- t.TempDir
require.NoError(t, err)
const chunkSize = 1 << 20 // 1 MiB
chunk := make([]byte, chunkSize)
written := int64(0)
for written <= maxSkillTotalSize {
n, werr := f.Write(chunk)
require.NoError(t, werr)
written += int64(n)
}
require.NoError(t, f.Close())

_, err = collectSkillFiles(dir)
require.Error(t, err)
assert.Contains(t, err.Error(), "exceeds maximum total size")
assert.ErrorIs(t, err, ErrSkillTooLarge)
}

// Helper functions
Expand All @@ -597,6 +765,19 @@ This is a test skill.
return dir
}

func writeValidSkillMD(t *testing.T, dir string) {
t.Helper()

skillMD := `---
name: test-skill
description: A test skill
version: 1.0.0
---
# Test Skill
`
require.NoError(t, os.WriteFile(filepath.Join(dir, "SKILL.md"), []byte(skillMD), 0600))
}

func createTestSkillDirWithScripts(t *testing.T) string {
t.Helper()

Expand Down
Loading