Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 26 additions & 8 deletions pkg/git/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}
15 changes: 8 additions & 7 deletions pkg/git/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand All @@ -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()

Expand All @@ -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.
Expand Down
50 changes: 50 additions & 0 deletions pkg/git/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
5 changes: 3 additions & 2 deletions pkg/plugins/pluginsvc/install_git.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 <path>/.claude-plugin/plugin.json.
// We collect the whole file tree first, then locate the manifest among the
Expand Down
19 changes: 13 additions & 6 deletions pkg/skills/gitresolver/resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Comment thread
samuv marked this conversation as resolved.

// FileEntry represents a single file from the cloned repository.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}

Expand Down
33 changes: 31 additions & 2 deletions pkg/skills/gitresolver/resolver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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 {
Expand Down
4 changes: 4 additions & 0 deletions pkg/skills/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:"-"`
}
Comment thread
samuv marked this conversation as resolved.

// SkillIndexEntry represents a single skill entry in a remote skill index.
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
44 changes: 44 additions & 0 deletions pkg/storage/sqlite/migrations_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Loading
Loading