From 1126cdc7753e79f48f7625102ce23ec21e5f340d Mon Sep 17 00:00:00 2001 From: matthew-pilot Date: Thu, 28 May 2026 08:57:38 +0000 Subject: [PATCH] fix: add Ed25519 detached-signature verification to manifest/skill fetch (PILOT-111) manifest.go: add ManifestPublicKey to fetcher, getOrVerify() fetches .sig alongside each resource and verifies before accepting. Nil key preserves backward-compatible behavior (no verification). skillinject.go: add ManifestPublicKey ed25519.PublicKey to Config. zz_extra_branches_test.go: three tests covering valid-sig, wrong-key, and missing-.sig paths. --- manifest.go | 30 +++++++++++-- skillinject.go | 6 +++ zz_extra_branches_test.go | 95 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 128 insertions(+), 3 deletions(-) diff --git a/manifest.go b/manifest.go index f658c4d..d74e6d5 100644 --- a/manifest.go +++ b/manifest.go @@ -4,6 +4,7 @@ package skillinject import ( "context" + "crypto/ed25519" "encoding/json" "fmt" "io" @@ -124,6 +125,7 @@ type fetcher struct { httpClient *http.Client manifestURL string repoBase string + publicKey ed25519.PublicKey // nil = skip verification (backward compat) } func newFetcher(cfg Config) *fetcher { @@ -142,7 +144,7 @@ func newFetcher(cfg Config) *fetcher { if !strings.HasSuffix(rb, "/") { rb += "/" } - return &fetcher{httpClient: c, manifestURL: mu, repoBase: rb} + return &fetcher{httpClient: c, manifestURL: mu, repoBase: rb, publicKey: cfg.ManifestPublicKey} } func (f *fetcher) get(ctx context.Context, url string) ([]byte, error) { @@ -165,7 +167,7 @@ func (f *fetcher) get(ctx context.Context, url string) ([]byte, error) { // fetchManifest grabs and parses the manifest from the configured URL. func (f *fetcher) fetchManifest(ctx context.Context) (*Manifest, error) { - body, err := f.get(ctx, f.manifestURL) + body, err := f.getOrVerify(ctx, f.manifestURL) if err != nil { return nil, fmt.Errorf("fetch manifest: %w", err) } @@ -190,7 +192,29 @@ func (f *fetcher) fetchManifest(ctx context.Context) (*Manifest, error) { func (f *fetcher) fetchRepoFile(ctx context.Context, relPath string) ([]byte, error) { relPath = strings.TrimPrefix(relPath, "/") url := f.repoBase + relPath - return f.get(ctx, url) + return f.getOrVerify(ctx, url) +} + +// getOrVerify returns the body at url. When f.publicKey is set, it also +// fetches .sig and verifies the detached Ed25519 signature before +// returning. Without a public key, behavior matches get() exactly +// (backward compatible). +func (f *fetcher) getOrVerify(ctx context.Context, url string) ([]byte, error) { + body, err := f.get(ctx, url) + if err != nil { + return nil, err + } + if f.publicKey == nil { + return body, nil + } + sig, err := f.get(ctx, url+".sig") + if err != nil { + return nil, fmt.Errorf("fetch signature %s.sig: %w", url, err) + } + if !ed25519.Verify(f.publicKey, body, sig) { + return nil, fmt.Errorf("ed25519 signature verification failed for %s", url) + } + return body, nil } // expandHome resolves "~/" in a manifest path against the user's home dir. diff --git a/skillinject.go b/skillinject.go index 746ede2..5a8750d 100644 --- a/skillinject.go +++ b/skillinject.go @@ -14,6 +14,7 @@ package skillinject import ( "context" + "crypto/ed25519" "crypto/sha256" "encoding/hex" "fmt" @@ -51,6 +52,11 @@ type Config struct { RepoBaseURL string // HTTPClient overrides the HTTP client used for fetching. HTTPClient *http.Client + // ManifestPublicKey, when set, enables Ed25519 detached-signature + // verification on manifest + all fetched repo files. The daemon + // fetches .sig alongside each resource and verifies before + // accepting. Nil (default) preserves the pre-verification behavior. + ManifestPublicKey ed25519.PublicKey } // Run blocks running scan/reconcile ticks until ctx is cancelled. The diff --git a/zz_extra_branches_test.go b/zz_extra_branches_test.go index 0c5c027..d99a380 100644 --- a/zz_extra_branches_test.go +++ b/zz_extra_branches_test.go @@ -29,6 +29,7 @@ package skillinject import ( "context" + "crypto/ed25519" "net/http" "net/http/httptest" "os" @@ -330,6 +331,100 @@ func TestReconcilePluginAllowList_IdenticalNoop(t *testing.T) { } } +// getOrVerify: with a valid public key, fetches file + .sig and verifies. +func TestFetchManifest_WithValidSignature(t *testing.T) { + t.Parallel() + pub, priv, err := ed25519.GenerateKey(nil) + if err != nil { + t.Fatalf("generate key: %v", err) + } + body := []byte(`{"version": 1, "entrypoint": "x", "tools": [{"name":"x"}]}`) + sig := ed25519.Sign(priv, body) + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if strings.HasSuffix(r.URL.Path, ".sig") { + _, _ = w.Write(sig) + } else { + _, _ = w.Write(body) + } + })) + defer srv.Close() + + f := newFetcher(Config{ + ManifestURL: srv.URL + "/m.json", + ManifestPublicKey: pub, + }) + m, err := f.fetchManifest(context.Background()) + if err != nil { + t.Fatalf("expected success with valid signature; got %v", err) + } + if m.Version != 1 { + t.Errorf("version = %d; want 1", m.Version) + } +} + +// getOrVerify: wrong public key → signature verification fails. +func TestFetchManifest_WrongPublicKeyFails(t *testing.T) { + t.Parallel() + _, priv, err := ed25519.GenerateKey(nil) + if err != nil { + t.Fatalf("generate key: %v", err) + } + wrongPub, _, _ := ed25519.GenerateKey(nil) + + body := []byte(`{"version": 1, "entrypoint": "x", "tools": [{"name":"x"}]}`) + sig := ed25519.Sign(priv, body) + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if strings.HasSuffix(r.URL.Path, ".sig") { + _, _ = w.Write(sig) + } else { + _, _ = w.Write(body) + } + })) + defer srv.Close() + + f := newFetcher(Config{ + ManifestURL: srv.URL + "/m.json", + ManifestPublicKey: wrongPub, + }) + _, err = f.fetchManifest(context.Background()) + if err == nil { + t.Fatal("expected signature verification failure") + } + if !strings.Contains(err.Error(), "signature verification failed") { + t.Errorf("unexpected error: %v", err) + } +} + +// getOrVerify: missing .sig file returns a wrapped fetch error. +func TestFetchManifest_MissingSigFileErrors(t *testing.T) { + t.Parallel() + pub, _, _ := ed25519.GenerateKey(nil) + body := []byte(`{"version": 1, "entrypoint": "x", "tools": [{"name":"x"}]}`) + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if strings.HasSuffix(r.URL.Path, ".sig") { + http.NotFound(w, r) + return + } + _, _ = w.Write(body) + })) + defer srv.Close() + + f := newFetcher(Config{ + ManifestURL: srv.URL + "/m.json", + ManifestPublicKey: pub, + }) + _, err := f.fetchManifest(context.Background()) + if err == nil { + t.Fatal("expected error when .sig is missing") + } + if !strings.Contains(err.Error(), "fetch signature") { + t.Errorf("unexpected error: %v", err) + } +} + // reconcilePluginAllowList: merge failure surfaces as Outcome.Action=Error // with Err populated. Trigger by writing malformed JSON into the config // at a state Drift will mis-classify as needing rewrite (note: