diff --git a/go.mod b/go.mod index 84f6ca832f..ef99b7df04 100644 --- a/go.mod +++ b/go.mod @@ -46,7 +46,7 @@ require ( github.com/shirou/gopsutil/v4 v4.26.2 github.com/spf13/viper v1.21.0 github.com/stacklok/toolhive-catalog v0.20260423.0 - github.com/stacklok/toolhive-core v0.0.16 + github.com/stacklok/toolhive-core v0.0.17 github.com/stretchr/testify v1.11.1 github.com/swaggo/swag/v2 v2.0.0-rc5 github.com/tailscale/hujson v0.0.0-20260302212456-ecc657c15afd @@ -307,7 +307,7 @@ require ( modernc.org/libc v1.70.0 // indirect modernc.org/mathutil v1.7.1 // indirect modernc.org/memory v1.11.0 // indirect - oras.land/oras-go/v2 v2.6.0 // indirect + oras.land/oras-go/v2 v2.6.0 sigs.k8s.io/json v0.0.0-20250730193827-2d320260d730 // indirect sigs.k8s.io/randfill v1.0.0 // indirect sigs.k8s.io/structured-merge-diff/v6 v6.3.2-0.20260122202528-d9cc6641c482 // indirect diff --git a/go.sum b/go.sum index 46b4b8b9be..d9e379dd9e 100644 --- a/go.sum +++ b/go.sum @@ -799,8 +799,8 @@ github.com/spf13/viper v1.21.0 h1:x5S+0EU27Lbphp4UKm1C+1oQO+rKx36vfCoaVebLFSU= github.com/spf13/viper v1.21.0/go.mod h1:P0lhsswPGWD/1lZJ9ny3fYnVqxiegrlNrEmgLjbTCAY= github.com/stacklok/toolhive-catalog v0.20260423.0 h1:W3qbLqbLhPqclYGrxYtm5jDYurjcEMRAhr7k/qfxm4s= github.com/stacklok/toolhive-catalog v0.20260423.0/go.mod h1:bfIGimrf8SJJGT3+WcJmFKJ3JB2vz9RPykn2r6nKVNw= -github.com/stacklok/toolhive-core v0.0.16 h1:Td/o/zrO3Pbr6tkx4r0k8Bqn9DySlwgah2vknE8tj5A= -github.com/stacklok/toolhive-core v0.0.16/go.mod h1:iHp39rCHZxXzU5FmXKK9aoH48NE+K0ob+MtdTS9CvAk= +github.com/stacklok/toolhive-core v0.0.17 h1:yGKXntWyw5ZO5GMxfSHi9doJhSXA8w5ORSXWveJ3OGc= +github.com/stacklok/toolhive-core v0.0.17/go.mod h1:o/zVzleR/xNCNXdTwNx8A41hApu0GZsHZS42qcXYUr8= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.2.0/go.mod h1:qt09Ya8vawLte6SNmTgCsAVtYtaKzEcn8ATUoHMkEqE= diff --git a/pkg/skills/skillsvc/build.go b/pkg/skills/skillsvc/build.go index bcfe32f1c2..eb89da02a1 100644 --- a/pkg/skills/skillsvc/build.go +++ b/pkg/skills/skillsvc/build.go @@ -40,6 +40,18 @@ func (s *service) Build(ctx context.Context, opts skills.BuildOptions) (*skills. } result, err := s.packager.Package(ctx, opts.Path, ociskills.DefaultPackageOptions()) if err != nil { + // User-input failures (missing SKILL.md, bad frontmatter, symlinks, + // size/count limits, unreadable directory) are surfaced as 400 with + // the packager's message intact. Anything else is a real 500. + switch { + case errors.Is(err, ociskills.ErrSkillMDMissing), + errors.Is(err, ociskills.ErrInvalidFrontmatter), + errors.Is(err, ociskills.ErrInvalidSkillDir), + errors.Is(err, ociskills.ErrInvalidSkillFile), + errors.Is(err, ociskills.ErrTooManyFiles), + errors.Is(err, ociskills.ErrSkillTooLarge): + return nil, httperr.WithCode(err, http.StatusBadRequest) + } return nil, fmt.Errorf("packaging skill: %w", err) } diff --git a/pkg/skills/skillsvc/build_test.go b/pkg/skills/skillsvc/build_test.go index d30ede4aa3..ae53da68bd 100644 --- a/pkg/skills/skillsvc/build_test.go +++ b/pkg/skills/skillsvc/build_test.go @@ -83,12 +83,13 @@ func TestBuild(t *testing.T) { t.Parallel() tests := []struct { - name string - opts skills.BuildOptions - setup func(*gomock.Controller) (ociskills.SkillPackager, *ociskills.Store) - wantCode int - wantRef string - wantErr string + name string + opts skills.BuildOptions + setup func(*gomock.Controller) (ociskills.SkillPackager, *ociskills.Store) + wantCode int + wantRef string + wantErr string + wantErrIs error }{ { name: "nil packager returns 500", @@ -158,6 +159,76 @@ func TestBuild(t *testing.T) { }, wantErr: "packaging skill", }, + { + name: "missing SKILL.md returns 400", + opts: skills.BuildOptions{Path: "/some/dir"}, + setup: func(ctrl *gomock.Controller) (ociskills.SkillPackager, *ociskills.Store) { + ociStore, err := ociskills.NewStore(t.TempDir()) + require.NoError(t, err) + p := ocimocks.NewMockSkillPackager(ctrl) + p.EXPECT().Package(gomock.Any(), "/some/dir", gomock.Any()). + Return(nil, fmt.Errorf("reading skill directory: %w", ociskills.ErrSkillMDMissing)) + return p, ociStore + }, + wantCode: http.StatusBadRequest, + wantErrIs: ociskills.ErrSkillMDMissing, + }, + { + name: "invalid frontmatter returns 400", + opts: skills.BuildOptions{Path: "/some/dir"}, + setup: func(ctrl *gomock.Controller) (ociskills.SkillPackager, *ociskills.Store) { + ociStore, err := ociskills.NewStore(t.TempDir()) + require.NoError(t, err) + p := ocimocks.NewMockSkillPackager(ctrl) + p.EXPECT().Package(gomock.Any(), "/some/dir", gomock.Any()). + Return(nil, fmt.Errorf("parsing frontmatter YAML: %w", ociskills.ErrInvalidFrontmatter)) + return p, ociStore + }, + wantCode: http.StatusBadRequest, + wantErrIs: ociskills.ErrInvalidFrontmatter, + }, + { + name: "empty name in frontmatter returns 400", + opts: skills.BuildOptions{Path: "/some/dir"}, + setup: func(ctrl *gomock.Controller) (ociskills.SkillPackager, *ociskills.Store) { + ociStore, err := ociskills.NewStore(t.TempDir()) + require.NoError(t, err) + p := ocimocks.NewMockSkillPackager(ctrl) + p.EXPECT().Package(gomock.Any(), "/some/dir", gomock.Any()). + Return(nil, fmt.Errorf("skill name is required in SKILL.md frontmatter: %w", ociskills.ErrInvalidFrontmatter)) + return p, ociStore + }, + wantCode: http.StatusBadRequest, + wantErrIs: ociskills.ErrInvalidFrontmatter, + }, + { + name: "symlink in skill dir returns 400", + opts: skills.BuildOptions{Path: "/some/dir"}, + setup: func(ctrl *gomock.Controller) (ociskills.SkillPackager, *ociskills.Store) { + ociStore, err := ociskills.NewStore(t.TempDir()) + require.NoError(t, err) + p := ocimocks.NewMockSkillPackager(ctrl) + p.EXPECT().Package(gomock.Any(), "/some/dir", gomock.Any()). + Return(nil, fmt.Errorf("symlinks not allowed in skill directory: sub/link: %w", ociskills.ErrInvalidSkillFile)) + return p, ociStore + }, + wantCode: http.StatusBadRequest, + wantErrIs: ociskills.ErrInvalidSkillFile, + }, + { + name: "oversized dir returns 400", + opts: skills.BuildOptions{Path: "/some/dir"}, + setup: func(ctrl *gomock.Controller) (ociskills.SkillPackager, *ociskills.Store) { + ociStore, err := ociskills.NewStore(t.TempDir()) + require.NoError(t, err) + p := ocimocks.NewMockSkillPackager(ctrl) + p.EXPECT().Package(gomock.Any(), "/some/dir", gomock.Any()). + Return(nil, fmt.Errorf("skill directory exceeds maximum total size: %w", ociskills.ErrSkillTooLarge)) + return p, ociStore + }, + wantCode: http.StatusBadRequest, + wantErrIs: ociskills.ErrSkillTooLarge, + }, { name: "successful build with explicit tag", opts: skills.BuildOptions{Path: "/some/dir", Tag: "v1.0.0"}, @@ -243,6 +314,9 @@ func TestBuild(t *testing.T) { if tt.wantCode != 0 { require.Error(t, err) assert.Equal(t, tt.wantCode, httperr.Code(err)) + if tt.wantErrIs != nil { + assert.ErrorIs(t, err, tt.wantErrIs) + } return } if tt.wantErr != "" {