From 8b6be78f7adf38f7a0e0ea6f1d0c1e8a6e67112f Mon Sep 17 00:00:00 2001 From: Samuel K Date: Thu, 30 Jul 2026 19:29:26 -0500 Subject: [PATCH 1/4] fix(git): stop auto-installing git-lfs during clone setup Plain git never installs missing tools on your behalf: with no LFS filter available it just leaves pointer stubs, silently. SetupLFS diverged from that by trying to apt/GitHub-release install git-lfs on every clone, which only ever fails in unprivileged environments and adds noisy permission-denied logs plus a wasted network round trip. Drop the install attempt so devsy matches git's own default behavior: use git-lfs when it's already present, fall back to pointer stubs when it's not. --- pkg/git/installer.go | 5 ----- pkg/git/lfs.go | 11 ++++------- pkg/git/lfs_test.go | 27 +++++++++++++++++++++++++++ 3 files changed, 31 insertions(+), 12 deletions(-) diff --git a/pkg/git/installer.go b/pkg/git/installer.go index 015ca4dec..2d52c3e67 100644 --- a/pkg/git/installer.go +++ b/pkg/git/installer.go @@ -15,11 +15,6 @@ func InstallBinary(ctx context.Context) error { return newInstaller(defaultRunner).ensure(ctx, gitTool) } -// InstallLFS installs the git-lfs binary if it is not already available. -func InstallLFS(ctx context.Context) error { - return newInstaller(defaultRunner).ensure(ctx, lfsTool) -} - // tool identifies a git-related binary the installer can provide. type tool struct { binary string diff --git a/pkg/git/lfs.go b/pkg/git/lfs.go index 1a7034935..77344545b 100644 --- a/pkg/git/lfs.go +++ b/pkg/git/lfs.go @@ -51,13 +51,10 @@ func (r *Repo) SetupLFS(ctx context.Context, mode LFSMode) { } if !command.Exists(binGitLFS) { - if err := InstallLFS(ctx); err != nil { - log.Warnf( - "repository uses git-lfs but it could not be installed, LFS files will be pointer stubs: %v", - err, - ) - return - } + log.Info( + "repository uses git-lfs but the binary is not installed, LFS files will be pointer stubs", + ) + return } if err := r.lfs(ctx, "install", "--local"); err != nil { diff --git a/pkg/git/lfs_test.go b/pkg/git/lfs_test.go index c2b8116c9..3743535fb 100644 --- a/pkg/git/lfs_test.go +++ b/pkg/git/lfs_test.go @@ -3,6 +3,7 @@ package git import ( "context" "os" + "os/exec" "path/filepath" "strings" "testing" @@ -100,6 +101,32 @@ func TestSetupLFSModeCommands(t *testing.T) { } } +func TestSetupLFSSkipsWhenBinaryMissing(t *testing.T) { + hideGitLFSFromPath(t) + + dir, fake := newLFSRepo(t) + At(dir, WithRunner(fake)).SetupLFS(context.Background(), LFSFull) + + if got := lfsSubcommands(fake); got != nil { + t.Errorf("lfs subcommands = %v, want none", got) + } +} + +func hideGitLFSFromPath(t *testing.T) { + t.Helper() + + gitPath, err := exec.LookPath("git") + if err != nil { + t.Skip("git not found on PATH") + } + + binDir := t.TempDir() + if err := os.Symlink(gitPath, filepath.Join(binDir, "git")); err != nil { + t.Fatal(err) + } + t.Setenv("PATH", binDir) +} + func TestRepoUsesLFS(t *testing.T) { cases := []struct { name string From bd4d081e6b059030012b8184a1e2af58c857b1a2 Mon Sep 17 00:00:00 2001 From: Samuel K Date: Thu, 30 Jul 2026 19:42:25 -0500 Subject: [PATCH 2/4] fix(git): gate git-lfs auto-install on provisioned (non-local) hosts Reinstate git-lfs auto-install, but only where devsy.ensureGit already draws the same line for the git binary itself: devsy-provisioned remote hosts it controls, never a user's local environment. Adds WithAllowLFSInstall/SetupLFS(..., allowInstall) so the decision is made once at the call site (mirroring ensureGit's isLocalAgent check) instead of unconditionally inside SetupLFS, which is what caused every clone in an unprivileged host to fail apt/GitHub-release installs and log permission-denied noise. --- pkg/agent/workspace.go | 20 ++++++++++++++++--- pkg/agent/workspace_test.go | 23 ++++++++++++++++++++++ pkg/git/clone.go | 8 ++++++++ pkg/git/installer.go | 5 +++++ pkg/git/lfs.go | 28 +++++++++++++++++++++------ pkg/git/lfs_test.go | 38 ++++++++++++++++++++++++++++++++++--- pkg/git/repo.go | 2 +- 7 files changed, 111 insertions(+), 13 deletions(-) diff --git a/pkg/agent/workspace.go b/pkg/agent/workspace.go index c535868ee..6aadf75b3 100644 --- a/pkg/agent/workspace.go +++ b/pkg/agent/workspace.go @@ -313,12 +313,21 @@ func ensureGit(ctx context.Context, agentConfig *provider2.ProviderAgentConfig) if command.Exists("git") { return nil } - if local, _ := agentConfig.Local.Bool(); local { + if isLocalAgent(agentConfig) { return errors.New("git not installed: install git and add it to PATH") } return git.InstallBinary(ctx) } +// isLocalAgent reports whether the agent is running in the user's own local +// environment, as opposed to a devsy-provisioned remote/cloud host. Devsy +// only installs missing tools on hosts it provisions and controls, never +// into a user's local environment. +func isLocalAgent(agentConfig *provider2.ProviderAgentConfig) bool { + local, _ := agentConfig.Local.Bool() + return local +} + func setupGitSSH( options provider2.CLIOptions, agentConfig *provider2.ProviderAgentConfig, @@ -418,7 +427,8 @@ func cloneViaGit(ctx context.Context, p CloneWorkspaceParams, extraEnv []string) repo := git.At(p.WorkspaceDir, git.WithStrictHostKeyChecking(p.Options.StrictHostKeyChecking), git.WithEnv(extraEnv)) - if err := repo.CloneFromInfo(ctx, gitInfo, p.Helper, getGitOptions(p.Options)...); err != nil { + gitOpts := getGitOptions(p.Options, p.AgentConfig) + if err := repo.CloneFromInfo(ctx, gitInfo, p.Helper, gitOpts...); err != nil { return failedClone(p.WorkspaceDir, "clone repository", err) } return nil @@ -452,7 +462,10 @@ func applyDevsyIgnore(workspaceDir string) error { return nil } -func getGitOptions(options provider2.CLIOptions) []git.Option { +func getGitOptions( + options provider2.CLIOptions, + agentConfig *provider2.ProviderAgentConfig, +) []git.Option { var gitOpts []git.Option if options.GitCloneStrategy != "" { gitOpts = append(gitOpts, git.WithCloneStrategy(options.GitCloneStrategy)) @@ -468,6 +481,7 @@ func getGitOptions(options provider2.CLIOptions) []git.Option { } else { gitOpts = append(gitOpts, git.WithLFSMode(options.GitLFSMode)) } + gitOpts = append(gitOpts, git.WithAllowLFSInstall(!isLocalAgent(agentConfig))) if options.GitCloneRecursiveSubmodules { gitOpts = append(gitOpts, git.WithRecursiveSubmodules()) } diff --git a/pkg/agent/workspace_test.go b/pkg/agent/workspace_test.go index 039baf80f..cda85ebf7 100644 --- a/pkg/agent/workspace_test.go +++ b/pkg/agent/workspace_test.go @@ -2,10 +2,33 @@ package agent import ( "testing" + + provider2 "github.com/devsy-org/devsy/pkg/provider" + "github.com/devsy-org/devsy/pkg/types" ) const explicitAgentDir = "/some/dir" +func TestIsLocalAgent(t *testing.T) { + cases := []struct { + name string + local types.StrBool + want bool + }{ + {"local true", "true", true}, + {"local false", "false", false}, + {"unset defaults to remote", "", false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := isLocalAgent(&provider2.ProviderAgentConfig{Local: tc.local}) + if got != tc.want { + t.Errorf("isLocalAgent(%q) = %v, want %v", tc.local, got, tc.want) + } + }) + } +} + func withContainerDetector(t *testing.T, fn func() bool) { t.Helper() prev := containerDetector diff --git a/pkg/git/clone.go b/pkg/git/clone.go index 2a683e30d..3553794a8 100644 --- a/pkg/git/clone.go +++ b/pkg/git/clone.go @@ -107,12 +107,20 @@ func WithSkipLFS() Option { return WithLFSMode(LFSSkip) } +// WithAllowLFSInstall permits SetupLFS to install the git-lfs binary itself +// when it's missing. Reserve this for environments Devsy provisions and +// controls; never enable it for a user's local environment. +func WithAllowLFSInstall(allow bool) Option { + return func(c *cloneConfig) { c.allowLFSInstall = allow } +} + // cloneConfig is the resolved set of clone options. type cloneConfig struct { strategy CloneStrategy branch string credentialHelper string recurseSubmodules bool + allowLFSInstall bool lfsMode LFSMode } diff --git a/pkg/git/installer.go b/pkg/git/installer.go index 2d52c3e67..015ca4dec 100644 --- a/pkg/git/installer.go +++ b/pkg/git/installer.go @@ -15,6 +15,11 @@ func InstallBinary(ctx context.Context) error { return newInstaller(defaultRunner).ensure(ctx, gitTool) } +// InstallLFS installs the git-lfs binary if it is not already available. +func InstallLFS(ctx context.Context) error { + return newInstaller(defaultRunner).ensure(ctx, lfsTool) +} + // tool identifies a git-related binary the installer can provide. type tool struct { binary string diff --git a/pkg/git/lfs.go b/pkg/git/lfs.go index 77344545b..85f269b6e 100644 --- a/pkg/git/lfs.go +++ b/pkg/git/lfs.go @@ -41,8 +41,15 @@ func cloneEnvForLFS() []string { return nil } -// SetupLFS configures Git LFS in the repository. -func (r *Repo) SetupLFS(ctx context.Context, mode LFSMode) { +// lfsInstaller installs git-lfs; overridable in tests to avoid a real +// package-manager/network install attempt. +var lfsInstaller = InstallLFS + +// SetupLFS configures Git LFS in the repository. When git-lfs isn't +// installed, it's only installed automatically if allowInstall is set, +// matching ensureGit's rule of never installing tools into a user's local +// environment. +func (r *Repo) SetupLFS(ctx context.Context, mode LFSMode, allowInstall bool) { if mode == LFSSkip { return } @@ -51,10 +58,19 @@ func (r *Repo) SetupLFS(ctx context.Context, mode LFSMode) { } if !command.Exists(binGitLFS) { - log.Info( - "repository uses git-lfs but the binary is not installed, LFS files will be pointer stubs", - ) - return + if !allowInstall { + log.Info( + "repository uses git-lfs but the binary is not installed, LFS files will be pointer stubs", + ) + return + } + if err := lfsInstaller(ctx); err != nil { + log.Warnf( + "repository uses git-lfs but it could not be installed, LFS files will be pointer stubs: %v", + err, + ) + return + } } if err := r.lfs(ctx, "install", "--local"); err != nil { diff --git a/pkg/git/lfs_test.go b/pkg/git/lfs_test.go index 3743535fb..018713d4e 100644 --- a/pkg/git/lfs_test.go +++ b/pkg/git/lfs_test.go @@ -2,6 +2,7 @@ package git import ( "context" + "errors" "os" "os/exec" "path/filepath" @@ -90,7 +91,7 @@ func TestSetupLFSModeCommands(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { dir, fake := newLFSRepo(t) - At(dir, WithRunner(fake)).SetupLFS(context.Background(), tc.mode) + At(dir, WithRunner(fake)).SetupLFS(context.Background(), tc.mode, false) var got []string for _, args := range lfsSubcommands(fake) { @@ -101,17 +102,48 @@ func TestSetupLFSModeCommands(t *testing.T) { } } -func TestSetupLFSSkipsWhenBinaryMissing(t *testing.T) { +func TestSetupLFSSkipsWhenBinaryMissingAndInstallNotAllowed(t *testing.T) { hideGitLFSFromPath(t) + stubLFSInstaller(t, func(context.Context) error { + t.Fatal("lfsInstaller must not be called when allowInstall is false") + return nil + }) dir, fake := newLFSRepo(t) - At(dir, WithRunner(fake)).SetupLFS(context.Background(), LFSFull) + At(dir, WithRunner(fake)).SetupLFS(context.Background(), LFSFull, false) if got := lfsSubcommands(fake); got != nil { t.Errorf("lfs subcommands = %v, want none", got) } } +func TestSetupLFSInstallsWhenBinaryMissingAndInstallAllowed(t *testing.T) { + hideGitLFSFromPath(t) + + var called bool + stubLFSInstaller(t, func(context.Context) error { + called = true + return errors.New("install unavailable in this environment") + }) + + dir, fake := newLFSRepo(t) + At(dir, WithRunner(fake)).SetupLFS(context.Background(), LFSFull, true) + + if !called { + t.Error("lfsInstaller was not called despite allowInstall being true") + } + if got := lfsSubcommands(fake); got != nil { + t.Errorf("lfs subcommands = %v, want none (install failed)", got) + } +} + +func stubLFSInstaller(t *testing.T, fn func(context.Context) error) { + t.Helper() + original := lfsInstaller + lfsInstaller = fn + t.Cleanup(func() { lfsInstaller = original }) +} + func hideGitLFSFromPath(t *testing.T) { t.Helper() diff --git a/pkg/git/repo.go b/pkg/git/repo.go index 656a90bb4..8703213f6 100644 --- a/pkg/git/repo.go +++ b/pkg/git/repo.go @@ -173,7 +173,7 @@ func (r *Repo) CloneFromInfo( // Bare clones have no worktree to hydrate. if c.strategy != BareCloneStrategy { - r.SetupLFS(ctx, c.lfsMode) + r.SetupLFS(ctx, c.lfsMode, c.allowLFSInstall) } return nil } From ac62b5445980fbfe8c2c591d5170b49dcf683ec8 Mon Sep 17 00:00:00 2001 From: Samuel K Date: Thu, 30 Jul 2026 19:44:08 -0500 Subject: [PATCH 3/4] chore(git): trim comments to bare minimum --- pkg/agent/workspace.go | 4 ---- pkg/git/clone.go | 4 +--- pkg/git/lfs.go | 8 ++------ 3 files changed, 3 insertions(+), 13 deletions(-) diff --git a/pkg/agent/workspace.go b/pkg/agent/workspace.go index 6aadf75b3..2cf59985b 100644 --- a/pkg/agent/workspace.go +++ b/pkg/agent/workspace.go @@ -319,10 +319,6 @@ func ensureGit(ctx context.Context, agentConfig *provider2.ProviderAgentConfig) return git.InstallBinary(ctx) } -// isLocalAgent reports whether the agent is running in the user's own local -// environment, as opposed to a devsy-provisioned remote/cloud host. Devsy -// only installs missing tools on hosts it provisions and controls, never -// into a user's local environment. func isLocalAgent(agentConfig *provider2.ProviderAgentConfig) bool { local, _ := agentConfig.Local.Bool() return local diff --git a/pkg/git/clone.go b/pkg/git/clone.go index 3553794a8..55c7a18a1 100644 --- a/pkg/git/clone.go +++ b/pkg/git/clone.go @@ -107,9 +107,7 @@ func WithSkipLFS() Option { return WithLFSMode(LFSSkip) } -// WithAllowLFSInstall permits SetupLFS to install the git-lfs binary itself -// when it's missing. Reserve this for environments Devsy provisions and -// controls; never enable it for a user's local environment. +// WithAllowLFSInstall permits SetupLFS to install the git-lfs binary itself. func WithAllowLFSInstall(allow bool) Option { return func(c *cloneConfig) { c.allowLFSInstall = allow } } diff --git a/pkg/git/lfs.go b/pkg/git/lfs.go index 85f269b6e..88ac31785 100644 --- a/pkg/git/lfs.go +++ b/pkg/git/lfs.go @@ -41,14 +41,10 @@ func cloneEnvForLFS() []string { return nil } -// lfsInstaller installs git-lfs; overridable in tests to avoid a real -// package-manager/network install attempt. +// lfsInstaller is overridable in tests. var lfsInstaller = InstallLFS -// SetupLFS configures Git LFS in the repository. When git-lfs isn't -// installed, it's only installed automatically if allowInstall is set, -// matching ensureGit's rule of never installing tools into a user's local -// environment. +// SetupLFS configures Git LFS in the repository. func (r *Repo) SetupLFS(ctx context.Context, mode LFSMode, allowInstall bool) { if mode == LFSSkip { return From 71210411bab2c507b65da2064798c9e4c1b9b6d5 Mon Sep 17 00:00:00 2001 From: Samuel K Date: Thu, 30 Jul 2026 19:53:26 -0500 Subject: [PATCH 4/4] fix(git): extract ensureLFSBinary to satisfy cyclop complexity limit SetupLFS's cyclomatic complexity hit 9 (max 8) after the allowInstall branch. Split the missing-binary handling into its own function. --- pkg/git/lfs.go | 35 +++++++++++++++++++++-------------- 1 file changed, 21 insertions(+), 14 deletions(-) diff --git a/pkg/git/lfs.go b/pkg/git/lfs.go index 88ac31785..b97b9aaff 100644 --- a/pkg/git/lfs.go +++ b/pkg/git/lfs.go @@ -53,20 +53,8 @@ func (r *Repo) SetupLFS(ctx context.Context, mode LFSMode, allowInstall bool) { return } - if !command.Exists(binGitLFS) { - if !allowInstall { - log.Info( - "repository uses git-lfs but the binary is not installed, LFS files will be pointer stubs", - ) - return - } - if err := lfsInstaller(ctx); err != nil { - log.Warnf( - "repository uses git-lfs but it could not be installed, LFS files will be pointer stubs: %v", - err, - ) - return - } + if !command.Exists(binGitLFS) && !ensureLFSBinary(ctx, allowInstall) { + return } if err := r.lfs(ctx, "install", "--local"); err != nil { @@ -83,6 +71,25 @@ func (r *Repo) SetupLFS(ctx context.Context, mode LFSMode, allowInstall bool) { } } +// ensureLFSBinary installs git-lfs when allowInstall is set, reporting +// whether the binary is now available. +func ensureLFSBinary(ctx context.Context, allowInstall bool) bool { + if !allowInstall { + log.Info( + "repository uses git-lfs but the binary is not installed, LFS files will be pointer stubs", + ) + return false + } + if err := lfsInstaller(ctx); err != nil { + log.Warnf( + "repository uses git-lfs but it could not be installed, LFS files will be pointer stubs: %v", + err, + ) + return false + } + return true +} + // lfs runs a `git lfs ` subcommand in the repository, returning any // captured output alongside the error for diagnostics. func (r *Repo) lfs(ctx context.Context, args ...string) error {