9bb5d143f7
executus CI / test (pull_request) Successful in 3m30s
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>
178 lines
5.0 KiB
Go
178 lines
5.0 KiB
Go
package skillpack
|
|
|
|
import (
|
|
"context"
|
|
"fmt"
|
|
"testing"
|
|
"time"
|
|
)
|
|
|
|
// fakeSource returns a caller-controlled tree, so sync behavior is tested with
|
|
// no filesystem or git.
|
|
type fakeSource struct {
|
|
tree Tree
|
|
ref string
|
|
err error
|
|
}
|
|
|
|
func (f *fakeSource) Fetch(context.Context, string) (Tree, string, error) {
|
|
return f.tree, f.ref, f.err
|
|
}
|
|
func (f *fakeSource) Kind() string { return "fake" }
|
|
func (f *fakeSource) String() string { return "fake://pack" }
|
|
|
|
func packTree(name, body string) Tree {
|
|
return Tree{ManifestName: []byte("---\nname: " + name + "\ndescription: does " + name + "\n---\n" + body + "\n")}
|
|
}
|
|
|
|
func newTestSyncer(src *fakeSource) *Syncer {
|
|
n := 0
|
|
return &Syncer{
|
|
Cache: NewMemoryPackCache(),
|
|
Subs: NewMemory(),
|
|
Now: func() time.Time { return time.Unix(1000, 0) },
|
|
NewID: func() string { n++; return fmt.Sprintf("id-%d", n) },
|
|
SourceFor: func(*Subscription) (Source, error) { return src, nil },
|
|
}
|
|
}
|
|
|
|
func TestSubscribeAndPin(t *testing.T) {
|
|
ctx := context.Background()
|
|
src := &fakeSource{tree: packTree("alpha", "v1"), ref: "sha-v1"}
|
|
y := newTestSyncer(src)
|
|
|
|
sub, err := y.Subscribe(ctx, src, "main", "steve")
|
|
if err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if sub.Name != "alpha" || sub.PinnedSourceRef != "sha-v1" || sub.PinnedBy != "steve" {
|
|
t.Fatalf("bad pin: %+v", sub)
|
|
}
|
|
if sub.HasPending() {
|
|
t.Fatal("fresh subscription should have no pending update")
|
|
}
|
|
// pinned tree is cached under its digest
|
|
if _, err := y.Cache.Get(ctx, sub.PinnedDigest); err != nil {
|
|
t.Fatalf("pinned tree not cached: %v", err)
|
|
}
|
|
}
|
|
|
|
func TestSubscribe_DuplicateName(t *testing.T) {
|
|
ctx := context.Background()
|
|
src := &fakeSource{tree: packTree("alpha", "v1"), ref: "r"}
|
|
y := newTestSyncer(src)
|
|
if _, err := y.Subscribe(ctx, src, "", "s"); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if _, err := y.Subscribe(ctx, src, "", "s"); err == nil {
|
|
t.Fatal("expected duplicate-name error")
|
|
}
|
|
}
|
|
|
|
func TestCheck_RecordsPendingButDoesNotMovePin(t *testing.T) {
|
|
ctx := context.Background()
|
|
src := &fakeSource{tree: packTree("alpha", "v1"), ref: "sha-v1"}
|
|
y := newTestSyncer(src)
|
|
sub, _ := y.Subscribe(ctx, src, "main", "s")
|
|
pinnedBefore := sub.PinnedDigest
|
|
|
|
// upstream changes
|
|
src.tree = packTree("alpha", "v2-new-instructions")
|
|
src.ref = "sha-v2"
|
|
|
|
updated, err := y.Check(ctx, sub.ID)
|
|
if err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if !updated.HasPending() {
|
|
t.Fatal("expected a pending update after upstream change")
|
|
}
|
|
if updated.PinnedDigest != pinnedBefore {
|
|
t.Fatal("Check must NOT move the pin — that is the supply-chain guard")
|
|
}
|
|
if updated.PendingSourceRef != "sha-v2" {
|
|
t.Errorf("pending ref = %q", updated.PendingSourceRef)
|
|
}
|
|
// the pending tree is cached, ready for Apply
|
|
if _, err := y.Cache.Get(ctx, updated.PendingDigest); err != nil {
|
|
t.Fatalf("pending tree not cached: %v", err)
|
|
}
|
|
}
|
|
|
|
func TestCheck_ClearsStalePendingWhenUpstreamMatches(t *testing.T) {
|
|
ctx := context.Background()
|
|
src := &fakeSource{tree: packTree("alpha", "v1"), ref: "r1"}
|
|
y := newTestSyncer(src)
|
|
sub, _ := y.Subscribe(ctx, src, "main", "s")
|
|
|
|
src.tree = packTree("alpha", "v2")
|
|
src.ref = "r2"
|
|
sub, _ = y.Check(ctx, sub.ID) // records pending
|
|
if !sub.HasPending() {
|
|
t.Fatal("precondition: pending expected")
|
|
}
|
|
// upstream reverts to the pinned content
|
|
src.tree = packTree("alpha", "v1")
|
|
src.ref = "r1"
|
|
sub, _ = y.Check(ctx, sub.ID)
|
|
if sub.HasPending() {
|
|
t.Fatal("pending should be cleared once upstream matches the pin again")
|
|
}
|
|
}
|
|
|
|
func TestApply_MovesPinAndClearsPending(t *testing.T) {
|
|
ctx := context.Background()
|
|
src := &fakeSource{tree: packTree("alpha", "v1"), ref: "sha-v1"}
|
|
y := newTestSyncer(src)
|
|
sub, _ := y.Subscribe(ctx, src, "main", "s")
|
|
|
|
src.tree = packTree("alpha", "v2")
|
|
src.ref = "sha-v2"
|
|
sub, _ = y.Check(ctx, sub.ID)
|
|
pendingDigest := sub.PendingDigest
|
|
|
|
applied, err := y.Apply(ctx, sub.ID, "admin")
|
|
if err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if applied.PinnedDigest != pendingDigest {
|
|
t.Fatal("Apply must move the pin to the pending digest")
|
|
}
|
|
if applied.PinnedSourceRef != "sha-v2" || applied.PinnedBy != "admin" {
|
|
t.Errorf("bad post-apply pin: %+v", applied)
|
|
}
|
|
if applied.HasPending() {
|
|
t.Fatal("Apply must clear the pending update")
|
|
}
|
|
}
|
|
|
|
func TestApply_NoPending(t *testing.T) {
|
|
ctx := context.Background()
|
|
src := &fakeSource{tree: packTree("alpha", "v1"), ref: "r"}
|
|
y := newTestSyncer(src)
|
|
sub, _ := y.Subscribe(ctx, src, "", "s")
|
|
if _, err := y.Apply(ctx, sub.ID, "admin"); err == nil {
|
|
t.Fatal("expected error applying with no pending update")
|
|
}
|
|
}
|
|
|
|
func TestCheckAll(t *testing.T) {
|
|
ctx := context.Background()
|
|
src := &fakeSource{tree: packTree("alpha", "v1"), ref: "r1"}
|
|
y := newTestSyncer(src)
|
|
sub, _ := y.Subscribe(ctx, src, "main", "s")
|
|
|
|
if pend, errs := y.CheckAll(ctx); len(pend) != 0 || len(errs) != 0 {
|
|
t.Fatalf("no change: pend=%v errs=%v", pend, errs)
|
|
}
|
|
src.tree = packTree("alpha", "v2")
|
|
src.ref = "r2"
|
|
pend, errs := y.CheckAll(ctx)
|
|
if len(errs) != 0 {
|
|
t.Fatalf("errs: %v", errs)
|
|
}
|
|
if len(pend) != 1 || pend[0].ID != sub.ID {
|
|
t.Fatalf("expected 1 pending, got %v", pend)
|
|
}
|
|
}
|