diff --git a/pkg/git/client.go b/pkg/git/client.go index 2ff35b87ee..cedd0fbe1a 100644 --- a/pkg/git/client.go +++ b/pkg/git/client.go @@ -37,13 +37,24 @@ type Client interface { // GetFileContent retrieves the content of a file from the repository GetFileContent(repoInfo *RepositoryInfo, path string) ([]byte, error) - // HeadCommitHash returns the commit hash of the HEAD reference. - HeadCommitHash(repoInfo *RepositoryInfo) (string, error) + // HeadCommit returns the hash and signature of the HEAD commit. + HeadCommit(repoInfo *RepositoryInfo) (HeadCommit, error) // Cleanup removes local repository directory Cleanup(ctx context.Context, repoInfo *RepositoryInfo) error } +// HeadCommit describes the HEAD commit of a cloned repository. +type HeadCommit struct { + // Hash is the commit hash. + Hash string + // Signature is the armored signature attached to the commit, empty + // when the commit is unsigned. The signature is UNVERIFIED — it is + // whatever bytes the commit carries; callers must cryptographically + // verify it before treating it as provenance. + Signature string +} + // DefaultGitClient implements Client using go-git type DefaultGitClient struct { // auth is the optional authentication method for cloning @@ -251,16 +262,23 @@ func (*DefaultGitClient) updateRepositoryInfo(repoInfo *RepositoryInfo) error { return nil } -// HeadCommitHash returns the commit hash of the HEAD reference. -func (*DefaultGitClient) HeadCommitHash(repoInfo *RepositoryInfo) (string, error) { +// HeadCommit returns the hash and (unverified) signature of the HEAD +// commit in a single lookup, so both describe the same commit. +func (*DefaultGitClient) HeadCommit(repoInfo *RepositoryInfo) (HeadCommit, error) { if repoInfo == nil || repoInfo.Repository == nil { - return "", ErrNilRepository + return HeadCommit{}, ErrNilRepository } ref, err := repoInfo.Repository.Head() if err != nil { - return "", fmt.Errorf("failed to get HEAD reference: %w", err) + return HeadCommit{}, fmt.Errorf("failed to get HEAD reference: %w", err) } - - return ref.Hash().String(), nil + commit, err := repoInfo.Repository.CommitObject(ref.Hash()) + if err != nil { + return HeadCommit{}, fmt.Errorf("failed to read HEAD commit: %w", err) + } + return HeadCommit{ + Hash: ref.Hash().String(), + Signature: commit.PGPSignature, + }, nil } diff --git a/pkg/git/client_test.go b/pkg/git/client_test.go index 4898caad20..729f5358db 100644 --- a/pkg/git/client_test.go +++ b/pkg/git/client_test.go @@ -104,7 +104,7 @@ func TestDefaultGitClient_GetFileContent_PathTraversal(t *testing.T) { } } -func TestDefaultGitClient_HeadCommitHash_NilInputs(t *testing.T) { +func TestDefaultGitClient_HeadCommit_NilInputs(t *testing.T) { t.Parallel() client := NewDefaultGitClient() @@ -119,14 +119,14 @@ func TestDefaultGitClient_HeadCommitHash_NilInputs(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { t.Parallel() - hash, err := client.HeadCommitHash(tt.repoInfo) + head, err := client.HeadCommit(tt.repoInfo) require.ErrorIs(t, err, ErrNilRepository) - assert.Empty(t, hash) + assert.Empty(t, head) }) } } -func TestDefaultGitClient_HeadCommitHash_Valid(t *testing.T) { +func TestDefaultGitClient_HeadCommit_UnsignedCommit(t *testing.T) { t.Parallel() client := NewDefaultGitClient() @@ -135,10 +135,11 @@ func TestDefaultGitClient_HeadCommitHash_Valid(t *testing.T) { require.NoError(t, err) t.Cleanup(func() { _ = client.Cleanup(t.Context(), repoInfo) }) - hash, err := client.HeadCommitHash(repoInfo) + head, err := client.HeadCommit(repoInfo) require.NoError(t, err) - assert.Len(t, hash, 40, "commit hash should be 40 hex chars") - assert.True(t, isAllHex(hash), "commit hash should be all hex") + assert.Len(t, head.Hash, 40, "commit hash should be 40 hex chars") + assert.True(t, isAllHex(head.Hash), "commit hash should be all hex") + assert.Empty(t, head.Signature, "an unsigned commit must yield an empty signature, not an error") } // isAllHex checks if s is a non-empty lowercase hex string. diff --git a/pkg/git/integration_test.go b/pkg/git/integration_test.go index 907f589dd8..2bb24c5bab 100644 --- a/pkg/git/integration_test.go +++ b/pkg/git/integration_test.go @@ -43,6 +43,56 @@ func initTestRepo(t *testing.T, files map[string]string) string { return dir } +// signTestRepoHead rewrites the HEAD commit of the repo at dir with the given +// armored signature attached, mimicking what gitsign/PGP tooling produces. +func signTestRepoHead(t *testing.T, dir, signature string) { + t.Helper() + + repo, err := gogit.PlainOpen(dir) + require.NoError(t, err) + head, err := repo.Head() + require.NoError(t, err) + commit, err := repo.CommitObject(head.Hash()) + require.NoError(t, err) + + commit.PGPSignature = signature + obj := repo.Storer.NewEncodedObject() + require.NoError(t, commit.Encode(obj)) + hash, err := repo.Storer.SetEncodedObject(obj) + require.NoError(t, err) + require.NoError(t, repo.Storer.SetReference(plumbing.NewHashReference(head.Name(), hash))) +} + +// repoInfoHeadHash returns the HEAD hash of a cloned repo directly via +// go-git, for cross-checking HeadCommit. +func repoInfoHeadHash(t *testing.T, repoInfo *RepositoryInfo) string { + t.Helper() + ref, err := repoInfo.Repository.Head() + require.NoError(t, err) + return ref.Hash().String() +} + +// TestDefaultGitClient_HeadCommit_SignedCommit verifies the armored +// signature stored on a commit survives clone and is returned verbatim. +func TestDefaultGitClient_HeadCommit_SignedCommit(t *testing.T) { + t.Parallel() + + const armoredSig = "-----BEGIN SIGNED MESSAGE-----\nMIIC(test-signature)\n-----END SIGNED MESSAGE-----\n" + repoDir := initTestRepo(t, map[string]string{"test.txt": "content"}) + signTestRepoHead(t, repoDir, armoredSig) + + client := NewDefaultGitClient() + repoInfo, err := client.Clone(t.Context(), &CloneConfig{URL: repoDir}) + require.NoError(t, err) + t.Cleanup(func() { _ = client.Cleanup(t.Context(), repoInfo) }) + + head, err := client.HeadCommit(repoInfo) + require.NoError(t, err) + assert.Equal(t, armoredSig, head.Signature) + assert.Equal(t, repoInfoHeadHash(t, repoInfo), head.Hash, + "hash and signature must describe the same commit") +} + // TestDefaultGitClient_FullWorkflow exercises the complete clone → read → cleanup lifecycle // against a local git repository to verify end-to-end correctness. func TestDefaultGitClient_FullWorkflow(t *testing.T) { diff --git a/pkg/plugins/pluginsvc/install_git.go b/pkg/plugins/pluginsvc/install_git.go index 3712477001..1048c26f08 100644 --- a/pkg/plugins/pluginsvc/install_git.go +++ b/pkg/plugins/pluginsvc/install_git.go @@ -116,10 +116,11 @@ func (s *service) cloneAndCollectPlugin( } defer func() { _ = client.Cleanup(ctx, repoInfo) }() - commitHash, err := client.HeadCommitHash(repoInfo) + head, err := client.HeadCommit(repoInfo) if err != nil { - return nil, nil, "", fmt.Errorf("getting commit hash: %w", err) + return nil, nil, "", fmt.Errorf("getting HEAD commit: %w", err) } + commitHash := head.Hash // Read the plugin manifest. It lives at /.claude-plugin/plugin.json. // We collect the whole file tree first, then locate the manifest among the diff --git a/pkg/skills/gitresolver/resolver.go b/pkg/skills/gitresolver/resolver.go index aaacd13b33..af167edb54 100644 --- a/pkg/skills/gitresolver/resolver.go +++ b/pkg/skills/gitresolver/resolver.go @@ -45,6 +45,11 @@ type ResolveResult struct { Files []FileEntry // CommitHash is the git commit hash (for digest/upgrade detection) CommitHash string + // CommitSignature is the armored signature attached to the resolved + // commit, empty when the commit is unsigned. It is UNVERIFIED here — + // the install-time verifier checks it cryptographically before it is + // trusted or recorded as provenance. + CommitSignature string } // FileEntry represents a single file from the cloned repository. @@ -136,10 +141,11 @@ func (r *defaultResolver) Resolve(ctx context.Context, ref *GitReference) (*Reso } defer client.Cleanup(ctx, repoInfo) //nolint:errcheck // best-effort cleanup - // Get commit hash for digest tracking - commitHash, err := client.HeadCommitHash(repoInfo) + // Get the commit hash (for digest tracking) and its signature in one + // lookup, so both describe the same commit. + head, err := client.HeadCommit(repoInfo) if err != nil { - return nil, fmt.Errorf("getting commit hash: %w", err) + return nil, fmt.Errorf("getting HEAD commit: %w", err) } // Read SKILL.md from the skill path @@ -173,9 +179,10 @@ func (r *defaultResolver) Resolve(ctx context.Context, ref *GitReference) (*Reso } return &ResolveResult{ - SkillConfig: parsed, - Files: files, - CommitHash: commitHash, + SkillConfig: parsed, + Files: files, + CommitHash: head.Hash, + CommitSignature: head.Signature, }, nil } diff --git a/pkg/skills/gitresolver/resolver_test.go b/pkg/skills/gitresolver/resolver_test.go index 22771a1464..4381d0734c 100644 --- a/pkg/skills/gitresolver/resolver_test.go +++ b/pkg/skills/gitresolver/resolver_test.go @@ -328,6 +328,35 @@ func TestResolver_Resolve_CommitRef(t *testing.T) { require.NotNil(t, result) assert.Equal(t, "my-skill", result.SkillConfig.Name) assert.Equal(t, commitHash, result.CommitHash) + assert.Empty(t, result.CommitSignature, "an unsigned commit must resolve with no signature") +} + +func TestResolver_Resolve_SignedCommitSignaturePropagates(t *testing.T) { + t.Parallel() + + const armoredSig = "-----BEGIN SIGNED MESSAGE-----\nMIIC(test-signature)\n-----END SIGNED MESSAGE-----\n" + repoDir := createTestRepo(t, "", validSkillMD) + + // Rewrite HEAD with the signature attached, mimicking gitsign output. + repo, err := gogit.PlainOpen(repoDir) + require.NoError(t, err) + head, err := repo.Head() + require.NoError(t, err) + commit, err := repo.CommitObject(head.Hash()) + require.NoError(t, err) + commit.PGPSignature = armoredSig + obj := repo.Storer.NewEncodedObject() + require.NoError(t, commit.Encode(obj)) + signedHash, err := repo.Storer.SetEncodedObject(obj) + require.NoError(t, err) + require.NoError(t, repo.Storer.SetReference(plumbing.NewHashReference(head.Name(), signedHash))) + + resolver := NewResolver(WithGitClient(git.NewDefaultGitClient())) + result, err := resolver.Resolve(t.Context(), &GitReference{URL: repoDir}) + require.NoError(t, err) + require.NotNil(t, result) + assert.Equal(t, signedHash.String(), result.CommitHash) + assert.Equal(t, armoredSig, result.CommitSignature) } func TestResolver_Resolve_MissingSkillMD(t *testing.T) { @@ -393,8 +422,8 @@ func (*blockingCloneClient) GetFileContent(_ *git.RepositoryInfo, _ string) ([]b return nil, fmt.Errorf("not implemented") } -func (*blockingCloneClient) HeadCommitHash(_ *git.RepositoryInfo) (string, error) { - return "", fmt.Errorf("not implemented") +func (*blockingCloneClient) HeadCommit(_ *git.RepositoryInfo) (git.HeadCommit, error) { + return git.HeadCommit{}, fmt.Errorf("not implemented") } func (*blockingCloneClient) Cleanup(_ context.Context, _ *git.RepositoryInfo) error { diff --git a/pkg/skills/types.go b/pkg/skills/types.go index f908de075e..50b4a27dd8 100644 --- a/pkg/skills/types.go +++ b/pkg/skills/types.go @@ -171,6 +171,10 @@ type InstalledSkill struct { // toolhive.lock.yaml. Only ever true for project-scoped installs. No // omitempty: false is an observable state (unmanaged), not an absence. Managed bool `json:"managed"` + // SigstoreBundle is the serialized Sigstore bundle captured at install + // time, used for offline re-verification during sync. Nil for unsigned + // installs. Never serialized to API responses. + SigstoreBundle []byte `json:"-"` } // SkillIndexEntry represents a single skill entry in a remote skill index. diff --git a/pkg/storage/sqlite/migrations/004_add_skill_sigstore_bundle.sql b/pkg/storage/sqlite/migrations/004_add_skill_sigstore_bundle.sql new file mode 100644 index 0000000000..7c73e8c72d --- /dev/null +++ b/pkg/storage/sqlite/migrations/004_add_skill_sigstore_bundle.sql @@ -0,0 +1,7 @@ +-- +goose Up + +ALTER TABLE installed_skills ADD COLUMN sigstore_bundle BLOB DEFAULT NULL; + +-- +goose Down + +ALTER TABLE installed_skills DROP COLUMN sigstore_bundle; diff --git a/pkg/storage/sqlite/migrations_test.go b/pkg/storage/sqlite/migrations_test.go index dd48c25703..370db0bf16 100644 --- a/pkg/storage/sqlite/migrations_test.go +++ b/pkg/storage/sqlite/migrations_test.go @@ -175,6 +175,50 @@ func TestMigrations_ManagedFlagAppliesOverPriorState(t *testing.T) { assert.True(t, tableExists(t, db.DB(), "installed_skills"), "installed_skills table itself must remain") } +// TestMigrations_SigstoreBundleAppliesOverPriorState verifies migration 004 +// adds installed_skills.sigstore_bundle defaulting to NULL, so rows created by +// an earlier schema read back as unsigned (nil bundle) once the column appears. +func TestMigrations_SigstoreBundleAppliesOverPriorState(t *testing.T) { + t.Parallel() + ctx := t.Context() + dbPath := filepath.Join(t.TempDir(), "test.db") + + db, err := Open(ctx, dbPath) + require.NoError(t, err) + defer db.Close() + + provider := newGooseProvider(t, db.DB()) + + // Roll back to just after migration 003, before "sigstore_bundle" existed. + _, err = provider.DownTo(ctx, 3) + require.NoError(t, err) + + _, err = db.DB().Exec(`INSERT INTO entries (entry_type, name) VALUES ('skill', 'pre-migration-skill')`) + require.NoError(t, err) + _, err = db.DB().Exec(`INSERT INTO installed_skills (entry_id, scope) VALUES (1, 'user')`) + require.NoError(t, err) + + // Re-apply Up: migration 004 adds the column to the pre-existing row. + _, err = provider.Up(ctx) + require.NoError(t, err) + + var bundle []byte + err = db.DB().QueryRow(`SELECT sigstore_bundle FROM installed_skills WHERE entry_id = 1`).Scan(&bundle) + require.NoError(t, err) + assert.Nil(t, bundle, "a row created before migration 004 must default to a NULL bundle") + + // Down for 004 removes the column without disturbing earlier tables. + _, err = provider.DownTo(ctx, 3) + require.NoError(t, err) + var count int + err = db.DB().QueryRow( + `SELECT COUNT(*) FROM pragma_table_info('installed_skills') WHERE name = 'sigstore_bundle'`, + ).Scan(&count) + require.NoError(t, err) + assert.Equal(t, 0, count, "sigstore_bundle column should be dropped by 004 Down") + assert.True(t, tableExists(t, db.DB(), "installed_skills"), "installed_skills table itself must remain") +} + // TestMigrations_DownDropsPluginTables verifies that goose's Down for migration // 002 drops the plugin tables (installed_plugins, plugin_dependencies) while // leaving migration 001's tables (entries, installed_skills, skill_dependencies, diff --git a/pkg/storage/sqlite/skill_store.go b/pkg/storage/sqlite/skill_store.go index 0efd1065a9..29ffca5539 100644 --- a/pkg/storage/sqlite/skill_store.go +++ b/pkg/storage/sqlite/skill_store.go @@ -39,7 +39,8 @@ var _ storage.SkillStore = (*SkillStore)(nil) // skillColumns is the SELECT column list shared by Get and List queries. const skillColumns = `is_.id, e.name, is_.scope, is_.project_root, is_.reference, is_.tag, is_.digest, is_.version, is_.description, is_.author, json(is_.tags), - json(is_.client_apps), is_.status, is_.installed_at, is_.managed` + json(is_.client_apps), is_.status, is_.installed_at, is_.managed, + is_.sigstore_bundle` // errManagedRequiresProjectScope is returned by Create/Update when a // user-scoped skill has Managed set. Managed pins a skill in a project's @@ -94,8 +95,9 @@ func (s *SkillStore) Create(ctx context.Context, skill skills.InstalledSkill) er res, err := tx.ExecContext(ctx, ` INSERT INTO installed_skills ( entry_id, scope, project_root, reference, tag, digest, - version, description, author, tags, client_apps, status, managed - ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, jsonb(?), jsonb(?), ?, ?)`, + version, description, author, tags, client_apps, status, managed, + sigstore_bundle + ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, jsonb(?), jsonb(?), ?, ?, ?)`, entryID, string(skill.Scope), skill.ProjectRoot, @@ -109,6 +111,7 @@ func (s *SkillStore) Create(ctx context.Context, skill skills.InstalledSkill) er clientsJSON, string(skill.Status), boolToInt(skill.Managed), + skill.SigstoreBundle, ) if err != nil { if isUniqueViolation(err) { @@ -280,7 +283,8 @@ func (s *SkillStore) Update(ctx context.Context, skill skills.InstalledSkill) er if _, err := tx.ExecContext(ctx, ` UPDATE installed_skills SET reference = ?, tag = ?, digest = ?, version = ?, description = ?, - author = ?, tags = jsonb(?), client_apps = jsonb(?), status = ?, managed = ? + author = ?, tags = jsonb(?), client_apps = jsonb(?), status = ?, managed = ?, + sigstore_bundle = ? WHERE id = ?`, skill.Reference, skill.Tag, @@ -292,6 +296,7 @@ func (s *SkillStore) Update(ctx context.Context, skill skills.InstalledSkill) er clientsJSON, string(skill.Status), boolToInt(skill.Managed), + skill.SigstoreBundle, installedSkillID, ); err != nil { return fmt.Errorf("updating installed skill: %w", err) @@ -386,12 +391,13 @@ func scanSkillFields(sc scanner) (skills.InstalledSkill, int64, error) { status string installedAtStr string managed int + sigstoreBundle []byte ) err := sc.Scan( &installedSkillID, &name, &scope, &projectRoot, &reference, &tag, &digest, &version, &description, &author, &tagsBlob, - &clientsBlob, &status, &installedAtStr, &managed, + &clientsBlob, &status, &installedAtStr, &managed, &sigstoreBundle, ) if err != nil { if errors.Is(err, sql.ErrNoRows) { @@ -420,15 +426,16 @@ func scanSkillFields(sc scanner) (skills.InstalledSkill, int64, error) { Author: author, Tags: tags, }, - Scope: skills.Scope(scope), - ProjectRoot: projectRoot, - Reference: reference, - Tag: tag, - Digest: digest, - Status: skills.InstallStatus(status), - InstalledAt: installedAt, - Clients: clients, - Managed: managed != 0, + Scope: skills.Scope(scope), + ProjectRoot: projectRoot, + Reference: reference, + Tag: tag, + Digest: digest, + Status: skills.InstallStatus(status), + InstalledAt: installedAt, + Clients: clients, + Managed: managed != 0, + SigstoreBundle: sigstoreBundle, } return sk, installedSkillID, nil diff --git a/pkg/storage/sqlite/skill_store_test.go b/pkg/storage/sqlite/skill_store_test.go index 5c057f055c..7efd267e68 100644 --- a/pkg/storage/sqlite/skill_store_test.go +++ b/pkg/storage/sqlite/skill_store_test.go @@ -73,6 +73,36 @@ func TestSkillStore_Create(t *testing.T) { // InstalledAt is set by the DB DEFAULT, so just assert it is not zero. assert.False(t, got.InstalledAt.IsZero(), "InstalledAt should not be zero") assert.False(t, got.Managed, "Managed should default to false") + assert.Nil(t, got.SigstoreBundle, "SigstoreBundle should default to nil (unsigned)") +} + +func TestSkillStore_SigstoreBundleRoundTrip(t *testing.T) { + t.Parallel() + store := newTestStore(t) + + bundle := []byte(`{"mediaType":"application/vnd.dev.sigstore.bundle.v0.3+json"}`) + sk := testSkill("bundle-test") + sk.SigstoreBundle = bundle + require.NoError(t, store.Create(t.Context(), sk)) + + got, err := store.Get(t.Context(), sk.Metadata.Name, sk.Scope, sk.ProjectRoot) + require.NoError(t, err) + assert.Equal(t, bundle, got.SigstoreBundle) + + // Update replaces the stored bundle (e.g. re-install of a re-signed + // artifact), and can clear it back to nil for an unsigned replacement. + newBundle := []byte(`{"replaced":true}`) + got.SigstoreBundle = newBundle + require.NoError(t, store.Update(t.Context(), got)) + got, err = store.Get(t.Context(), sk.Metadata.Name, sk.Scope, sk.ProjectRoot) + require.NoError(t, err) + assert.Equal(t, newBundle, got.SigstoreBundle) + + got.SigstoreBundle = nil + require.NoError(t, store.Update(t.Context(), got)) + got, err = store.Get(t.Context(), sk.Metadata.Name, sk.Scope, sk.ProjectRoot) + require.NoError(t, err) + assert.Nil(t, got.SigstoreBundle) } func TestSkillStore_ManagedFlagRoundTrip(t *testing.T) {