Merge pull request #95 from stacklok/improve-build

feat(oci/skills): add sentinel errors for packager failure classification

Author: samuv

oci/skills/errors.go

@@ -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")
+)

oci/skills/packager.go

@@ -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)
 	}
@@ -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)
@@ -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
@@ -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() {
@@ -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
@@ -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
@@ -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
 }
@@ -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

oci/skills/packager_test.go

@@ -4,6 +4,7 @@
 package skills
 
 import (
+	"bytes"
 	"context"
 	"encoding/json"
 	"fmt"
@@ -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())
@@ -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())
@@ -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
@@ -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()