Skip to content

Commit 3fdf9a4

Browse files
committed
Refactor object signer registration
Entire-Checkpoint: a70d97c363c8 Entire-Checkpoint: 983194b9963d
1 parent 53c7d2f commit 3fdf9a4

File tree

14 files changed

+293
-133
lines changed

14 files changed

+293
-133
lines changed

cmd/entire/cli/checkpoint/checkpoint_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -545,7 +545,7 @@ func TestWriteCommitted_MergesVercelConfigOnMetadataBranch(t *testing.T) {
545545
}
546546

547547
store := NewGitStore(repo)
548-
commitHash, err := store.createCommit(treeHash, plumbing.ZeroHash, "Initialize metadata branch", "Test", "test@test.com")
548+
commitHash, err := store.createCommit(context.Background(), treeHash, plumbing.ZeroHash, "Initialize metadata branch", "Test", "test@test.com")
549549
if err != nil {
550550
t.Fatalf("createCommit() error = %v", err)
551551
}

cmd/entire/cli/checkpoint/committed.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1641,6 +1641,7 @@ func GetGitAuthorFromRepo(repo *git.Repository) (name, email string) {
16411641

16421642
// If not found in local config, try global config
16431643
if name == "" || email == "" {
1644+
//nolint:staticcheck // the v6 is not yet released, revisit once it is.
16441645
globalCfg, err := config.LoadConfig(config.GlobalScope)
16451646
if err == nil {
16461647
if name == "" {
@@ -1703,11 +1704,12 @@ func CreateCommit(ctx context.Context, repo *git.Repository, treeHash, parentHas
17031704
// If no signer is registered, signing is disabled via settings, or signing fails,
17041705
// the commit is left unsigned and the error is logged.
17051706
func SignCommitBestEffort(ctx context.Context, commit *object.Commit) {
1706-
if !settings.IsSignCheckpointCommitsEnabled(ctx) {
1707+
// Check plugin availability first (in-memory) before hitting disk for settings.
1708+
if !plugin.Has(plugin.ObjectSigner()) {
17071709
return
17081710
}
17091711

1710-
if !plugin.Has(plugin.ObjectSigner()) {
1712+
if !settings.IsSignCheckpointCommitsEnabled(ctx) {
17111713
return
17121714
}
17131715

@@ -1732,6 +1734,7 @@ func SignCommitBestEffort(ctx context.Context, commit *object.Commit) {
17321734
logging.Warn(ctx, "failed to read encoded commit", slog.String("error", err.Error()))
17331735
return
17341736
}
1737+
defer r.Close()
17351738

17361739
sig, err := signer.Sign(r)
17371740
if err != nil {
Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,154 @@
1+
package checkpoint
2+
3+
import (
4+
"context"
5+
"errors"
6+
"io"
7+
"os"
8+
"path/filepath"
9+
"testing"
10+
"time"
11+
12+
"github.com/entireio/cli/cmd/entire/cli/paths"
13+
14+
"github.com/go-git/go-git/v6/plumbing"
15+
"github.com/go-git/go-git/v6/plumbing/object"
16+
"github.com/go-git/go-git/v6/x/plugin"
17+
)
18+
19+
type stubSigner struct {
20+
sig []byte
21+
err error
22+
}
23+
24+
func (s *stubSigner) Sign(_ io.Reader) ([]byte, error) {
25+
return s.sig, s.err
26+
}
27+
28+
func setupSigningEnv(t *testing.T, disableSigning bool) {
29+
t.Helper()
30+
31+
dir := t.TempDir()
32+
33+
// Minimal git repo so paths.WorktreeRoot resolves.
34+
if err := os.MkdirAll(filepath.Join(dir, ".git"), 0o755); err != nil {
35+
t.Fatal(err)
36+
}
37+
38+
entireDir := filepath.Join(dir, ".entire")
39+
if err := os.MkdirAll(entireDir, 0o755); err != nil {
40+
t.Fatal(err)
41+
}
42+
43+
if disableSigning {
44+
content := `{"sign_checkpoint_commits": false}`
45+
if err := os.WriteFile(filepath.Join(entireDir, "settings.json"), []byte(content), 0o644); err != nil {
46+
t.Fatal(err)
47+
}
48+
}
49+
50+
paths.ClearWorktreeRootCache()
51+
t.Chdir(dir)
52+
t.Cleanup(func() {
53+
resetPluginEntry("object-signer")
54+
paths.ClearWorktreeRootCache()
55+
})
56+
}
57+
58+
func newTestCommit() *object.Commit {
59+
sig := object.Signature{
60+
Name: "Test",
61+
Email: "test@test.com",
62+
When: time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC),
63+
}
64+
return &object.Commit{
65+
TreeHash: plumbing.ZeroHash,
66+
Author: sig,
67+
Committer: sig,
68+
Message: "test commit",
69+
}
70+
}
71+
72+
func TestSignCommitBestEffort_Signs(t *testing.T) { //nolint:paralleltest // t.Chdir requires non-parallel
73+
setupSigningEnv(t, false)
74+
75+
err := plugin.Register(plugin.ObjectSigner(), func() plugin.Signer {
76+
return &stubSigner{sig: []byte("FAKESIG")}
77+
})
78+
if err != nil {
79+
t.Fatal(err)
80+
}
81+
82+
commit := newTestCommit()
83+
SignCommitBestEffort(context.Background(), commit)
84+
85+
if commit.Signature != "FAKESIG" {
86+
t.Errorf("expected signature %q, got %q", "FAKESIG", commit.Signature)
87+
}
88+
}
89+
90+
func TestSignCommitBestEffort_SkipsWhenDisabled(t *testing.T) { //nolint:paralleltest // t.Chdir requires non-parallel
91+
setupSigningEnv(t, true)
92+
93+
err := plugin.Register(plugin.ObjectSigner(), func() plugin.Signer {
94+
t.Fatal("signer should not be called when signing is disabled")
95+
return nil
96+
})
97+
if err != nil {
98+
t.Fatal(err)
99+
}
100+
101+
commit := newTestCommit()
102+
SignCommitBestEffort(context.Background(), commit)
103+
104+
if commit.Signature != "" {
105+
t.Errorf("expected empty signature, got %q", commit.Signature)
106+
}
107+
}
108+
109+
func TestSignCommitBestEffort_ErrorIsBestEffort(t *testing.T) { //nolint:paralleltest // t.Chdir requires non-parallel
110+
setupSigningEnv(t, false)
111+
112+
err := plugin.Register(plugin.ObjectSigner(), func() plugin.Signer {
113+
return &stubSigner{err: errors.New("signing failed")}
114+
})
115+
if err != nil {
116+
t.Fatal(err)
117+
}
118+
119+
commit := newTestCommit()
120+
SignCommitBestEffort(context.Background(), commit)
121+
122+
if commit.Signature != "" {
123+
t.Errorf("expected empty signature after error, got %q", commit.Signature)
124+
}
125+
}
126+
127+
func TestSignCommitBestEffort_NoSignerRegistered(t *testing.T) { //nolint:paralleltest // t.Chdir requires non-parallel
128+
setupSigningEnv(t, false)
129+
130+
commit := newTestCommit()
131+
SignCommitBestEffort(context.Background(), commit)
132+
133+
if commit.Signature != "" {
134+
t.Errorf("expected empty signature without signer, got %q", commit.Signature)
135+
}
136+
}
137+
138+
func TestSignCommitBestEffort_NilSigner(t *testing.T) { //nolint:paralleltest // t.Chdir requires non-parallel
139+
setupSigningEnv(t, false)
140+
141+
err := plugin.Register(plugin.ObjectSigner(), func() plugin.Signer {
142+
return nil
143+
})
144+
if err != nil {
145+
t.Fatal(err)
146+
}
147+
148+
commit := newTestCommit()
149+
SignCommitBestEffort(context.Background(), commit)
150+
151+
if commit.Signature != "" {
152+
t.Errorf("expected empty signature with nil signer, got %q", commit.Signature)
153+
}
154+
}

cmd/entire/cli/checkpoint/global_test.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,9 @@ func TestMain(m *testing.M) {
1515
// For tests, ensure that go-git always gets empty Configs for both
1616
// system and global scopes. This way the current environment does not
1717
// impact the tests.
18-
err := plugin.Register(plugin.ConfigLoader(), config.NewEmpty)
18+
err := plugin.Register(plugin.ConfigLoader(), func() plugin.ConfigSource {
19+
return config.NewEmpty()
20+
})
1921
if err != nil {
2022
panic(fmt.Errorf("failed to register config storers: %w", err))
2123
}

cmd/entire/cli/checkpoint/v2_store_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -908,7 +908,7 @@ func TestV2GitStore_UpdateCommitted_PreservesExistingTaskMetadataInFullCurrent(t
908908
require.NoError(t, err)
909909

910910
authorName, authorEmail := GetGitAuthorFromRepo(repo)
911-
commitHash, err := CreateCommit(repo, newRootHash, parentHash,
911+
commitHash, err := CreateCommit(ctx, repo, newRootHash, parentHash,
912912
fmt.Sprintf("Checkpoint: %s (task metadata)\n", cpID), authorName, authorEmail)
913913
require.NoError(t, err)
914914
require.NoError(t, repo.Storer.SetReference(plumbing.NewHashReference(refName, commitHash)))

cmd/entire/cli/global_test.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,9 @@ func TestMain(m *testing.M) {
1515
// Register a default ConfigSource so tests that call ConfigScoped
1616
// (directly or indirectly via Commit/CreateTag) don't fail with
1717
// "no config loader registered".
18-
err := plugin.Register(plugin.ConfigLoader(), config.NewEmpty)
18+
err := plugin.Register(plugin.ConfigLoader(), func() plugin.ConfigSource {
19+
return config.NewEmpty()
20+
})
1921
if err != nil {
2022
panic(fmt.Errorf("failed to register config storers: %w", err))
2123
}

cmd/entire/cli/objectsigner.go

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
package cli
2+
3+
import (
4+
"context"
5+
"fmt"
6+
"net"
7+
"os"
8+
"sync"
9+
10+
"github.com/go-git/go-git/v6/config"
11+
"github.com/go-git/go-git/v6/x/plugin"
12+
"github.com/go-git/x/plugin/objectsigner/auto"
13+
"golang.org/x/crypto/ssh/agent"
14+
)
15+
16+
var registerObjectSignerOnce sync.Once
17+
18+
func RegisterObjectSigner() {
19+
registerObjectSignerOnce.Do(func() {
20+
//nolint:errcheck,gosec // best-effort; if registration fails, commits are left unsigned
21+
plugin.Register(plugin.ObjectSigner(), func() plugin.Signer {
22+
cfgSource, err := plugin.Get(plugin.ConfigLoader())
23+
if err != nil {
24+
// No config loader registered; signing not possible.
25+
return nil
26+
}
27+
28+
sysCfg := loadScopedConfig(cfgSource, config.SystemScope)
29+
globalCfg := loadScopedConfig(cfgSource, config.GlobalScope)
30+
31+
// Merge system then global so that global settings take precedence.
32+
merged := config.Merge(sysCfg, globalCfg)
33+
34+
if !merged.Commit.GpgSign.IsTrue() {
35+
return nil
36+
}
37+
38+
cfg := auto.Config{
39+
SigningKey: merged.User.SigningKey,
40+
Format: auto.Format(merged.GPG.Format),
41+
SSHAgent: connectSSHAgent(),
42+
}
43+
44+
signer, err := auto.FromConfig(cfg)
45+
if err != nil {
46+
fmt.Fprintf(os.Stderr, "warning: failed to create object signer: %v\n", err)
47+
return nil
48+
}
49+
50+
return signer
51+
})
52+
})
53+
}
54+
55+
// connectSSHAgent connects to the SSH agent via SSH_AUTH_SOCK.
56+
// Returns nil if the agent is unavailable.
57+
func connectSSHAgent() agent.Agent { //nolint:ireturn // must return the ssh agent interface
58+
sock := os.Getenv("SSH_AUTH_SOCK")
59+
if sock == "" {
60+
return nil
61+
}
62+
63+
var d net.Dialer
64+
conn, err := d.DialContext(context.Background(), "unix", sock)
65+
if err != nil {
66+
return nil
67+
}
68+
69+
return agent.NewClient(conn)
70+
}
71+
72+
var scopeName = map[config.Scope]string{
73+
config.GlobalScope: "global",
74+
config.SystemScope: "system",
75+
}
76+
77+
func loadScopedConfig(source plugin.ConfigSource, scope config.Scope) *config.Config {
78+
name := scopeName[scope]
79+
80+
storer, err := source.Load(scope)
81+
if err != nil {
82+
fmt.Fprintf(os.Stderr, "warning: failed to load %s git config: %v\n", name, err)
83+
return config.NewConfig()
84+
}
85+
86+
cfg, err := storer.Config()
87+
if err != nil {
88+
fmt.Fprintf(os.Stderr, "warning: failed to parse %s git config: %v\n", name, err)
89+
return config.NewConfig()
90+
}
91+
92+
return cfg
93+
}

cmd/entire/cli/root_test.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,11 @@ import (
44
"bytes"
55
"runtime"
66
"strings"
7+
"sync"
78
"testing"
89

910
"github.com/entireio/cli/cmd/entire/cli/versioninfo"
11+
"github.com/go-git/go-git/v6/x/plugin"
1012
"github.com/spf13/cobra"
1113
)
1214

@@ -68,6 +70,21 @@ func TestVersionFlag_ContainsExpectedInfo(t *testing.T) {
6870
}
6971
}
7072

73+
func TestRegisterObjectSigner_RegistersPlugin(t *testing.T) {
74+
resetPluginEntry("object-signer")
75+
registerObjectSignerOnce = sync.Once{}
76+
t.Cleanup(func() {
77+
resetPluginEntry("object-signer")
78+
registerObjectSignerOnce = sync.Once{}
79+
})
80+
81+
RegisterObjectSigner()
82+
83+
if !plugin.Has(plugin.ObjectSigner()) {
84+
t.Fatal("expected object signer plugin to be registered")
85+
}
86+
}
87+
7188
func TestPersistentPostRun_SkipsHiddenParent(t *testing.T) {
7289
t.Parallel()
7390

0 commit comments

Comments
 (0)