From 37f9cb5cb03b5ba4c48cb6d7bef95930cf429a64 Mon Sep 17 00:00:00 2001 From: Samuel K Date: Sat, 4 Jul 2026 12:43:52 -0500 Subject: [PATCH 1/3] refactor(git): repo/runner architecture, native LFS handling, ctx threading Rework pkg/git around a Repo handle bound to path+env with an injectable Runner seam, replacing scattered exec calls. Adds: - Repo with Clone/CloneFromInfo/Fetch/Checkout/Reset/Config/CredentialFill - strategy-based tool installer with git-lfs GitHub-release fallback - typed CommandError + consistent git-binary preflight - three-way Git LFS mode (full/setup-only/skip) via --git-lfs-mode - depth-bounded LFS .gitattributes detection Moves credential-helper config and gitcredentials off ad-hoc file/shell surgery onto the git config seam (fixing a SetUser shell-injection), and inverts the pkg/download -> gitcredentials dependency so download stays a leaf. Threads context.Context through the download/provider/workspace clone chains to real command handlers. --- cmd/internal/agent_daemon.go | 2 +- .../agentcontainer/credentials_server.go | 13 +- cmd/internal/agentcontainer/setup.go | 10 +- cmd/internal/agentworkspace/build.go | 3 +- .../agentworkspace/install_dotfiles.go | 9 +- cmd/internal/agentworkspace/setup_gpg.go | 2 +- cmd/internal/agentworkspace/up.go | 123 +++-- cmd/internal/check_provider_update.go | 5 +- cmd/internal/container_tunnel.go | 2 +- cmd/internal/get_provider_name.go | 2 +- cmd/internal/git_credentials.go | 17 +- cmd/pro/login.go | 20 +- cmd/pro/update_provider.go | 2 +- cmd/provider/add.go | 3 +- cmd/provider/set_source.go | 17 +- cmd/workspace/build.go | 4 + cmd/workspace/ssh.go | 24 +- cmd/workspace/ssh_test.go | 13 +- cmd/workspace/up/up_client.go | 2 +- cmd/workspace/up/up_flags.go | 4 + cmd/workspace/workspace.go | 5 +- pkg/agent/tunnelserver/tunnelserver.go | 4 +- pkg/agent/workspace.go | 20 +- pkg/config/pathmanager.go | 4 + pkg/config/pathmanager_darwin.go | 5 + pkg/config/pathmanager_linux.go | 4 + pkg/config/pathmanager_linux_test.go | 1 + pkg/config/pathmanager_windows.go | 11 + pkg/daemon/platform/local_server.go | 2 +- pkg/devcontainer/helpers.go | 31 +- pkg/devcontainer/setup/dotfiles.go | 3 +- pkg/devcontainer/setup/setup.go | 11 +- pkg/download/download.go | 118 +++-- pkg/driver/custom/custom.go | 5 +- pkg/git/args.go | 35 ++ pkg/git/clone.go | 198 ++++---- pkg/git/clone_test.go | 122 +++++ pkg/git/config.go | 131 ++++++ pkg/git/config_test.go | 104 +++++ pkg/git/git.go | 122 +---- pkg/git/install.go | 77 ---- pkg/git/installer.go | 145 ++++++ pkg/git/installer_release.go | 228 ++++++++++ pkg/git/installer_test.go | 165 +++++++ pkg/git/lfs.go | 114 +++++ pkg/git/lfs_test.go | 212 +++++++++ pkg/git/repo.go | 229 ++++++++++ pkg/git/repo_test.go | 176 ++++++++ pkg/git/runner.go | 122 +++++ pkg/git/runner_test.go | 50 +++ pkg/gitcredentials/gitcredentials.go | 425 +++++++++--------- pkg/gitcredentials/gitcredentials_test.go | 146 +++++- pkg/gitsshsigning/utils.go | 25 +- pkg/gitsshsigning/utils_test.go | 20 +- pkg/provider/download.go | 40 +- pkg/provider/resolve.go | 28 +- pkg/provider/workspace.go | 1 + pkg/tunnel/services.go | 15 +- pkg/tunnel/services_test.go | 11 +- pkg/workspace/provider.go | 34 +- pkg/workspace/provider_update.go | 10 +- pkg/workspace/provider_versions.go | 9 +- 62 files changed, 2694 insertions(+), 801 deletions(-) create mode 100644 pkg/git/args.go create mode 100644 pkg/git/clone_test.go create mode 100644 pkg/git/config.go create mode 100644 pkg/git/config_test.go delete mode 100644 pkg/git/install.go create mode 100644 pkg/git/installer.go create mode 100644 pkg/git/installer_release.go create mode 100644 pkg/git/installer_test.go create mode 100644 pkg/git/lfs.go create mode 100644 pkg/git/lfs_test.go create mode 100644 pkg/git/repo.go create mode 100644 pkg/git/repo_test.go create mode 100644 pkg/git/runner.go create mode 100644 pkg/git/runner_test.go diff --git a/cmd/internal/agent_daemon.go b/cmd/internal/agent_daemon.go index f153cdd8d..f615ba9ec 100644 --- a/cmd/internal/agent_daemon.go +++ b/cmd/internal/agent_daemon.go @@ -178,7 +178,7 @@ func (cmd *DaemonCmd) runShutdownCommand( workspace *provider2.AgentWorkspaceInfo, ) { // get environ - environ, err := custom.ToEnvironWithBinaries(workspace) + environ, err := custom.ToEnvironWithBinaries(ctx, workspace) if err != nil { log.Errorf("%v", err) return diff --git a/cmd/internal/agentcontainer/credentials_server.go b/cmd/internal/agentcontainer/credentials_server.go index 09b534ae2..718e11807 100644 --- a/cmd/internal/agentcontainer/credentials_server.go +++ b/cmd/internal/agentcontainer/credentials_server.go @@ -123,14 +123,17 @@ func (cmd *CredentialsServerCmd) Run(ctx context.Context, port int) error { if err != nil { return err } - err = gitcredentials.ConfigureHelper(binaryPath, cmd.User, port) + err = gitcredentials.ConfigureHelper(ctx, binaryPath, cmd.User, port) if err != nil { return fmt.Errorf("configure git helper: %w", err) } - // cleanup when we are done + // cleanup when we are done. This defer runs after the server loop + // returns on shutdown, when ctx is already canceled — use an uncanceled + // context so the helper is actually removed instead of aborting early. + cleanupCtx := context.WithoutCancel(ctx) defer func(userName string) { - _ = gitcredentials.RemoveHelper(userName) + _ = gitcredentials.RemoveHelper(cleanupCtx, userName) }(cmd.User) } @@ -165,7 +168,7 @@ func configureGitUserLocally( client tunnel.TunnelClient, ) error { // get local credentials - localGitUser, err := gitcredentials.GetUser(userName, "") + localGitUser, err := gitcredentials.GetUser(ctx, userName, "") if err != nil { return err } else if localGitUser.Name != "" && localGitUser.Email != "" { @@ -194,7 +197,7 @@ func configureGitUserLocally( } // set git user - err = gitcredentials.SetUser(userName, gitUser) + err = gitcredentials.SetUser(ctx, userName, gitUser) if err != nil { return fmt.Errorf("set git user & email: %w", err) } diff --git a/cmd/internal/agentcontainer/setup.go b/cmd/internal/agentcontainer/setup.go index cd1c04253..9cc0e1ad2 100644 --- a/cmd/internal/agentcontainer/setup.go +++ b/cmd/internal/agentcontainer/setup.go @@ -805,18 +805,14 @@ func configureSystemGitCredentials( ) _ = os.Setenv(config2.EnvGitHelperPort, strconv.Itoa(serverPort)) - err = git.CommandContext(ctx, git.GetDefaultExtraEnv(false), "config", "--system", "--add", - "credential.helper", gitCredentials). - Run() - if err != nil { + gitConfig := git.At("", git.WithStrictHostKeyChecking(false)).Config() + if err = gitConfig.Add(ctx, "credential.helper", gitCredentials, git.ScopeSystem); err != nil { return nil, fmt.Errorf("add git credential helper: %w", err) } cleanup := func() { log.Debug("unset setup system credential helper") - err = git.CommandContext(ctx, git.GetDefaultExtraEnv(false), "config", "--system", "--unset", "credential.helper"). - Run() - if err != nil { + if err = gitConfig.Unset(ctx, "credential.helper", git.ScopeSystem); err != nil { log.Errorf("unset system credential helper %v", err) } } diff --git a/cmd/internal/agentworkspace/build.go b/cmd/internal/agentworkspace/build.go index acd685e87..d4522e134 100644 --- a/cmd/internal/agentworkspace/build.go +++ b/cmd/internal/agentworkspace/build.go @@ -59,8 +59,7 @@ func (cmd *BuildCmd) Run(ctx context.Context) error { // initialize the workspace cancelCtx, cancel := context.WithCancel(ctx) defer cancel() - _, credentialsDir, err := initWorkspace(initWorkspaceParams{ - ctx: cancelCtx, + _, credentialsDir, err := initWorkspace(cancelCtx, initWorkspaceParams{ workspaceInfo: workspaceInfo, debug: cmd.Debug, shouldInstallDaemon: false, diff --git a/cmd/internal/agentworkspace/install_dotfiles.go b/cmd/internal/agentworkspace/install_dotfiles.go index 171b5d0ad..2e5c94abe 100644 --- a/cmd/internal/agentworkspace/install_dotfiles.go +++ b/cmd/internal/agentworkspace/install_dotfiles.go @@ -56,13 +56,8 @@ func (cmd *InstallDotfilesCmd) Run(ctx context.Context) error { log.Infof("Cloning dotfiles %s", cmd.Repository) gitInfo := git.NormalizeRepository(cmd.Repository) - if err := git.CloneRepository( - ctx, - gitInfo, - targetDir, - "", - cmd.StrictHostKeyChecking, - ); err != nil { + if err := git.At(targetDir, git.WithStrictHostKeyChecking(cmd.StrictHostKeyChecking)). + CloneFromInfo(ctx, gitInfo, ""); err != nil { return err } } else { diff --git a/cmd/internal/agentworkspace/setup_gpg.go b/cmd/internal/agentworkspace/setup_gpg.go index 76332e268..f034a3420 100644 --- a/cmd/internal/agentworkspace/setup_gpg.go +++ b/cmd/internal/agentworkspace/setup_gpg.go @@ -74,7 +74,7 @@ func (cmd *SetupGPGCmd) Run(ctx context.Context) error { if gpgConf.GitKey != "" { log.Debugf("Setup git signing key") - if err := gitcredentials.SetupGpgGitKey(gpgConf.GitKey); err != nil { + if err := gitcredentials.SetupGpgGitKey(ctx, gpgConf.GitKey); err != nil { log.Warnf("Setup git signing key failed (non-fatal): %v", err) } } diff --git a/cmd/internal/agentworkspace/up.go b/cmd/internal/agentworkspace/up.go index d52e2680e..d63a5528f 100644 --- a/cmd/internal/agentworkspace/up.go +++ b/cmd/internal/agentworkspace/up.go @@ -74,8 +74,7 @@ func (cmd *UpCmd) Run(ctx context.Context) error { cancelCtx, cancel := context.WithCancel(ctx) defer cancel() - tunnelClient, credentialsDir, err := initWorkspace(initWorkspaceParams{ - ctx: cancelCtx, + tunnelClient, credentialsDir, err := initWorkspace(cancelCtx, initWorkspaceParams{ workspaceInfo: workspaceInfo, debug: cmd.Debug, shouldInstallDaemon: cmd.shouldInstallDaemon(workspaceInfo), @@ -215,6 +214,7 @@ func CreateRunner( } func InitContentFolder( + ctx context.Context, workspaceInfo *provider.AgentWorkspaceInfo, ) (bool, error) { exists, err := contentFolderExists(workspaceInfo.ContentFolder) @@ -229,7 +229,7 @@ func InitContentFolder( return false, err } - if err := downloadWorkspaceBinaries(workspaceInfo); err != nil { + if err := downloadWorkspaceBinaries(ctx, workspaceInfo); err != nil { _ = os.RemoveAll(workspaceInfo.ContentFolder) return false, err } @@ -265,6 +265,7 @@ func createContentFolder(path string) error { } func downloadWorkspaceBinaries( + ctx context.Context, workspaceInfo *provider.AgentWorkspaceInfo, ) error { binariesDir, err := agent.GetAgentBinariesDir( @@ -280,7 +281,7 @@ func downloadWorkspaceBinaries( ) } - _, err = provider.DownloadBinaries(workspaceInfo.Agent.Binaries, binariesDir) + _, err = provider.DownloadBinaries(ctx, workspaceInfo.Agent.Binaries, binariesDir) if err != nil { return fmt.Errorf( "error downloading workspace %s binaries: %w", @@ -293,7 +294,6 @@ func downloadWorkspaceBinaries( } type workspaceInitializer struct { - ctx context.Context workspaceInfo *provider.AgentWorkspaceInfo debug bool shouldInstallDaemon bool @@ -304,39 +304,40 @@ type workspaceInitializer struct { } type initWorkspaceParams struct { - ctx context.Context workspaceInfo *provider.AgentWorkspaceInfo debug bool shouldInstallDaemon bool } -func initWorkspace(params initWorkspaceParams) (tunnel.TunnelClient, string, error) { +func initWorkspace( + ctx context.Context, + params initWorkspaceParams, +) (tunnel.TunnelClient, string, error) { init := &workspaceInitializer{ - ctx: params.ctx, workspaceInfo: params.workspaceInfo, debug: params.debug, shouldInstallDaemon: params.shouldInstallDaemon, } - if err := init.initialize(); err != nil { + if err := init.initialize(ctx); err != nil { return nil, init.dockerCredentialsDir, err } return init.tunnelClient, init.dockerCredentialsDir, nil } -func (w *workspaceInitializer) initialize() error { - if err := w.initializeTunnel(); err != nil { +func (w *workspaceInitializer) initialize(ctx context.Context) error { + if err := w.initializeTunnel(ctx); err != nil { return err } - if err := w.setupCredentials(); err != nil { + if err := w.setupCredentials(ctx); err != nil { log.Warnf("failed to set up docker/git credentials (continuing without them): %v", err) } dockerErrChan := w.installDockerAsync() - if err := w.prepareWorkspaceContent(); err != nil { + if err := w.prepareWorkspaceContent(ctx); err != nil { return err } @@ -346,7 +347,7 @@ func (w *workspaceInitializer) initialize() error { return err } - w.tryConfigureDockerDaemon() + w.tryConfigureDockerDaemon(ctx) return nil } @@ -358,12 +359,12 @@ func (w *workspaceInitializer) setupDaemonIfNeeded() { } } -func (w *workspaceInitializer) tryConfigureDockerDaemon() { +func (w *workspaceInitializer) tryConfigureDockerDaemon(ctx context.Context) { if !w.shouldConfigureDockerDaemon() { log.Debug("skipping configuring docker daemon") return } - if err := configureDockerDaemon(w.ctx); err != nil { + if err := configureDockerDaemon(ctx); err != nil { log.Warn( "could not find docker daemon config file, if using the registry cache, " + "ensure the daemon is configured with containerd-snapshotter=true, " + @@ -372,25 +373,24 @@ func (w *workspaceInitializer) tryConfigureDockerDaemon() { } } -func (w *workspaceInitializer) initializeTunnel() error { +func (w *workspaceInitializer) initializeTunnel(ctx context.Context) error { client, err := tunnelserver.NewTunnelClient(os.Stdin, os.Stdout, true, 0) if err != nil { return fmt.Errorf("error creating tunnel client: %w", err) } w.tunnelClient = client - w.logger = tunnelserver.NewTunnelLogger(w.ctx, w.tunnelClient, w.debug) + w.logger = tunnelserver.NewTunnelLogger(ctx, w.tunnelClient, w.debug) log.Debugf("created logger") - if _, err := w.tunnelClient.Ping(w.ctx, &tunnel.Empty{}); err != nil { + if _, err := w.tunnelClient.Ping(ctx, &tunnel.Empty{}); err != nil { return fmt.Errorf("ping client: %w", err) } return nil } -func (w *workspaceInitializer) setupCredentials() error { - dockerCredentialsDir, gitCredentialsHelper, err := configureCredentials(credentialsConfig{ - ctx: w.ctx, +func (w *workspaceInitializer) setupCredentials(ctx context.Context) error { + dockerCredentialsDir, gitCredentialsHelper, err := configureCredentials(ctx, credentialsConfig{ workspaceInfo: w.workspaceInfo, client: w.tunnelClient, }) @@ -489,9 +489,8 @@ func (w *workspaceInitializer) isDockerInstallDisabled() bool { return err == nil && !install } -func (w *workspaceInitializer) prepareWorkspaceContent() error { - return prepareWorkspace(prepareWorkspaceParams{ - ctx: w.ctx, +func (w *workspaceInitializer) prepareWorkspaceContent(ctx context.Context) error { + return prepareWorkspace(ctx, prepareWorkspaceParams{ workspaceInfo: w.workspaceInfo, client: w.tunnelClient, gitHelper: w.gitCredentialsHelper, @@ -530,7 +529,6 @@ func (w *workspaceInitializer) shouldConfigureDockerDaemon() bool { } type prepareWorkspaceParams struct { - ctx context.Context workspaceInfo *provider.AgentWorkspaceInfo client tunnel.TunnelClient gitHelper string @@ -539,7 +537,7 @@ type prepareWorkspaceParams struct { // prepareWorkspace initializes the workspace content folder and downloads/prepares the workspace source. // Note: This function modifies params.workspaceInfo.ContentFolder when platform is enabled with a local folder. -func prepareWorkspace(params prepareWorkspaceParams) error { +func prepareWorkspace(ctx context.Context, params prepareWorkspaceParams) error { if params.workspaceInfo.CLIOptions.Platform.Enabled && params.workspaceInfo.Workspace.Source.LocalFolder != "" { params.workspaceInfo.ContentFolder = agent.GetAgentWorkspaceContentDir( @@ -547,7 +545,7 @@ func prepareWorkspace(params prepareWorkspaceParams) error { ) } - exists, err := InitContentFolder(params.workspaceInfo) + exists, err := InitContentFolder(ctx, params.workspaceInfo) if err != nil { return err } @@ -556,45 +554,44 @@ func prepareWorkspace(params prepareWorkspaceParams) error { return nil } - if params.workspaceInfo.Workspace.Source.GitRepository != "" { - return prepareGitWorkspace(prepareGitWorkspaceParams{ - ctx: params.ctx, + return prepareWorkspaceSource(ctx, params, exists) +} + +// prepareWorkspaceSource dispatches on the workspace source type (git, local +// folder, image, or container). +func prepareWorkspaceSource(ctx context.Context, params prepareWorkspaceParams, exists bool) error { + source := params.workspaceInfo.Workspace.Source + switch { + case source.GitRepository != "": + return prepareGitWorkspace(ctx, prepareGitWorkspaceParams{ workspaceInfo: params.workspaceInfo, gitHelper: params.gitHelper, exists: exists, logger: params.logger, }) - } - - if params.workspaceInfo.Workspace.Source.LocalFolder != "" { - return prepareLocalWorkspace(params.ctx, params.workspaceInfo, params.client) - } - - if params.workspaceInfo.Workspace.Source.Image != "" { + case source.LocalFolder != "": + return prepareLocalWorkspace(ctx, params.workspaceInfo, params.client) + case source.Image != "": params.logger.Debugf("prepare image") - return prepareImage( - params.workspaceInfo.ContentFolder, - params.workspaceInfo.Workspace.Source.Image, - ) - } - - if params.workspaceInfo.Workspace.Source.Container != "" { + return prepareImage(params.workspaceInfo.ContentFolder, source.Image) + case source.Container != "": params.logger.Debugf("workspace is a container, nothing to do") return nil + default: + return fmt.Errorf( + "either workspace repository, image, container or local-folder is required", + ) } - - return fmt.Errorf("either workspace repository, image, container or local-folder is required") } type prepareGitWorkspaceParams struct { - ctx context.Context workspaceInfo *provider.AgentWorkspaceInfo gitHelper string exists bool logger tunnelserver.Logger } -func prepareGitWorkspace(params prepareGitWorkspaceParams) error { +func prepareGitWorkspace(ctx context.Context, params prepareGitWorkspaceParams) error { if params.workspaceInfo.CLIOptions.Reset { params.logger.Info("resetting git based workspace, removing old content folder") if err := os.RemoveAll(params.workspaceInfo.ContentFolder); err != nil { @@ -619,7 +616,7 @@ func prepareGitWorkspace(params prepareGitWorkspaceParams) error { } return agent.CloneRepositoryForWorkspace( - params.ctx, + ctx, ¶ms.workspaceInfo.Workspace.Source, ¶ms.workspaceInfo.Agent, params.workspaceInfo.ContentFolder, @@ -675,18 +672,17 @@ func ensureLastDevContainerJson(workspaceInfo *provider.AgentWorkspaceInfo) erro } type credentialsConfig struct { - ctx context.Context workspaceInfo *provider.AgentWorkspaceInfo client tunnel.TunnelClient } -func configureCredentials(cfg credentialsConfig) (string, string, error) { +func configureCredentials(ctx context.Context, cfg credentialsConfig) (string, string, error) { if cfg.workspaceInfo.Agent.InjectDockerCredentials != config.BoolTrue && cfg.workspaceInfo.Agent.InjectGitCredentials != config.BoolTrue { return "", "", nil } - serverPort, err := credentials.StartCredentialsServer(cfg.ctx, cfg.client) + serverPort, err := credentials.StartCredentialsServer(ctx, cfg.client) if err != nil { return "", "", err } @@ -700,15 +696,9 @@ func configureCredentials(cfg credentialsConfig) (string, string, error) { return "", "", fmt.Errorf("workspace folder is not set") } - dockerCredentials := "" - if cfg.workspaceInfo.Agent.InjectDockerCredentials == config.BoolTrue { - dockerCredentials, err = dockercredentials.ConfigureCredentialsMachine( - cfg.workspaceInfo.Origin, - serverPort, - ) - if err != nil { - return "", "", err - } + dockerCredentials, err := configureDockerCredentials(cfg, serverPort) + if err != nil { + return "", "", err } gitCredentials := "" @@ -724,6 +714,15 @@ func configureCredentials(cfg credentialsConfig) (string, string, error) { return dockerCredentials, gitCredentials, nil } +// configureDockerCredentials sets up the docker credential helper machine when +// requested, returning the resulting credentials string (empty when disabled). +func configureDockerCredentials(cfg credentialsConfig, serverPort int) (string, error) { + if cfg.workspaceInfo.Agent.InjectDockerCredentials != config.BoolTrue { + return "", nil + } + return dockercredentials.ConfigureCredentialsMachine(cfg.workspaceInfo.Origin, serverPort) +} + func installDaemon(workspaceInfo *provider.AgentWorkspaceInfo) error { if len(workspaceInfo.Agent.Exec.Shutdown) == 0 { return nil diff --git a/cmd/internal/check_provider_update.go b/cmd/internal/check_provider_update.go index 8a92841ee..471dede3f 100644 --- a/cmd/internal/check_provider_update.go +++ b/cmd/internal/check_provider_update.go @@ -75,7 +75,7 @@ func (cmd *CheckProviderUpdateCmd) Run( return errProviderNotFound } - latestProviderConfig, err := loadLatestProvider(providerSourceRaw) + latestProviderConfig, err := loadLatestProvider(ctx, providerSourceRaw) if err != nil { return err } @@ -108,9 +108,10 @@ func (cmd *CheckProviderUpdateCmd) Run( } func loadLatestProvider( + ctx context.Context, providerSourceRaw string, ) (*provider.ProviderConfig, error) { - providerRaw, _, err := provider.ResolveProvider(providerSourceRaw) + providerRaw, _, err := provider.ResolveProvider(ctx, providerSourceRaw) if err != nil { return nil, fmt.Errorf("resolve provider: %w", err) } diff --git a/cmd/internal/container_tunnel.go b/cmd/internal/container_tunnel.go index aa8879265..f4aa52cc5 100644 --- a/cmd/internal/container_tunnel.go +++ b/cmd/internal/container_tunnel.go @@ -61,7 +61,7 @@ func (cmd *ContainerTunnelCmd) Run(ctx context.Context) error { } // make sure content folder exists - _, err = agentworkspace.InitContentFolder(workspaceInfo) + _, err = agentworkspace.InitContentFolder(ctx, workspaceInfo) if err != nil { return err } diff --git a/cmd/internal/get_provider_name.go b/cmd/internal/get_provider_name.go index 7006914b6..bfd4db160 100644 --- a/cmd/internal/get_provider_name.go +++ b/cmd/internal/get_provider_name.go @@ -35,7 +35,7 @@ func (cmd *GetProviderNameCmd) Run(ctx context.Context, args []string) error { return fmt.Errorf("provider is missing") } - providerRaw, _, err := provider.ResolveProvider(args[0]) + providerRaw, _, err := provider.ResolveProvider(ctx, args[0]) if err != nil { return fmt.Errorf("resolve provider: %w", err) } diff --git a/cmd/internal/git_credentials.go b/cmd/internal/git_credentials.go index fafa6e0ee..3d1e81b46 100644 --- a/cmd/internal/git_credentials.go +++ b/cmd/internal/git_credentials.go @@ -55,25 +55,18 @@ func (cmd *GitCredentialsCmd) Run(ctx context.Context, args []string) error { return err } - credentialsReq, err := gitcredentials.Parse(string(raw)) - if err != nil { - return err - } - - // try to get the credentials from the workspace server first - credentials := getCredentialsFromWorkspaceServer(credentialsReq) + credentialsReq := gitcredentials.ParseCredentials(string(raw)) + credentials := getCredentialsFromWorkspaceServer(&credentialsReq) if credentials == nil && cmd.Port != 0 { - // try to get the credentials from the local machine - credentials = getCredentialsFromLocalMachine(credentialsReq, cmd.Port) + credentials = getCredentialsFromLocalMachine(&credentialsReq, cmd.Port) } - // if we still don't have credentials, just return nothing if credentials == nil { return nil } - // print response to stdout - fmt.Print(gitcredentials.ToString(credentials)) + // git's credential helper protocol reads the response from stdout verbatim. + _, _ = os.Stdout.WriteString(credentials.Encode()) return nil } diff --git a/cmd/pro/login.go b/cmd/pro/login.go index e81dcd763..6324f28aa 100644 --- a/cmd/pro/login.go +++ b/cmd/pro/login.go @@ -86,7 +86,7 @@ func (cmd *LoginCmd) Run(ctx context.Context, fullURL string) error { return err } - devsyConfig, err = cmd.ensureProvider(devsyConfig, currentInstance, fullURL) + devsyConfig, err = cmd.ensureProvider(ctx, devsyConfig, currentInstance, fullURL) if err != nil { return err } @@ -173,6 +173,7 @@ func (cmd *LoginCmd) resolveNewProviderName(devsyConfig *config.Config, host str } func (cmd *LoginCmd) ensureProvider( + ctx context.Context, devsyConfig *config.Config, currentInstance *provider.ProInstance, fullURL string, @@ -188,7 +189,7 @@ func (cmd *LoginCmd) ensureProvider( CreationTimestamp: types.Now(), } - if err := cmd.addProviderByVersion(devsyConfig, fullURL); err != nil { + if err := cmd.addProviderByVersion(ctx, devsyConfig, fullURL); err != nil { return nil, err } @@ -199,7 +200,11 @@ func (cmd *LoginCmd) ensureProvider( return config.LoadConfig(devsyConfig.DefaultContext, cmd.Provider) } -func (cmd *LoginCmd) addProviderByVersion(devsyConfig *config.Config, fullURL string) error { +func (cmd *LoginCmd) addProviderByVersion( + ctx context.Context, + devsyConfig *config.Config, + fullURL string, +) error { remoteVersion, err := platform.GetDevsyVersion(fullURL) if err != nil { return err @@ -213,10 +218,10 @@ func (cmd *LoginCmd) addProviderByVersion(devsyConfig *config.Config, fullURL st if rv.LT(semver.Version{Major: 0, Minor: 6, Patch: 999}) && remoteVersion != versionpkg.DevVersion { log.Debug("remote version < 0.7.0, installing proxy provider") - return cmd.addLoftProvider(devsyConfig, fullURL) + return cmd.addLoftProvider(ctx, devsyConfig, fullURL) } - _, err = workspace.AddProvider(devsyConfig, cmd.Provider, "pro") + _, err = workspace.AddProvider(ctx, devsyConfig, cmd.Provider, "pro") return err } @@ -263,6 +268,7 @@ func (cmd *LoginCmd) loginAndConfigure( } func (cmd *LoginCmd) addLoftProvider( + ctx context.Context, devsyConfig *config.Config, url string, ) error { @@ -278,7 +284,7 @@ func (cmd *LoginCmd) addLoftProvider( // is development? if cmd.ProviderSource == config.RepoSlug+"@v0.0.0" { log.Debugf("Add development provider") - _, err = workspace.AddProviderRaw(workspace.ProviderParams{ + _, err = workspace.AddProviderRaw(ctx, workspace.ProviderParams{ DevsyConfig: devsyConfig, ProviderName: cmd.Provider, Source: &provider.ProviderSource{}, @@ -288,7 +294,7 @@ func (cmd *LoginCmd) addLoftProvider( return err } } else { - _, err = workspace.AddProvider(devsyConfig, cmd.Provider, cmd.ProviderSource) + _, err = workspace.AddProvider(ctx, devsyConfig, cmd.Provider, cmd.ProviderSource) if err != nil { return err } diff --git a/cmd/pro/update_provider.go b/cmd/pro/update_provider.go index 4f3fc82fe..3918f1199 100644 --- a/cmd/pro/update_provider.go +++ b/cmd/pro/update_provider.go @@ -72,7 +72,7 @@ func (cmd *UpdateProviderCmd) Run(ctx context.Context, args []string) error { } providerSource = splitted[0] + "@" + newVersion - _, err = workspace.UpdateProvider(devsyConfig, provider.Name, providerSource) + _, err = workspace.UpdateProvider(ctx, devsyConfig, provider.Name, providerSource) if err != nil { return fmt.Errorf("update provider %s: %w", provider.Name, err) } diff --git a/cmd/provider/add.go b/cmd/provider/add.go index 472ef84ea..abe24fec4 100644 --- a/cmd/provider/add.go +++ b/cmd/provider/add.go @@ -90,6 +90,7 @@ func (cmd *AddCmd) Run(ctx context.Context, devsyConfig *config.Config, args []s return fmt.Errorf("provider %s does not exist", cmd.FromExisting) } providerWithOptions, err := workspace.CloneProvider( + ctx, devsyConfig, providerName, cmd.FromExisting, @@ -109,7 +110,7 @@ func (cmd *AddCmd) Run(ctx context.Context, devsyConfig *config.Config, args []s return fmt.Errorf("specify either a URL or path, " + "e.g. devsy provider add https://path/to/my/provider.yaml") } - c, err := workspace.AddProvider(devsyConfig, providerName, args[0]) + c, err := workspace.AddProvider(ctx, devsyConfig, providerName, args[0]) if err != nil { return err } diff --git a/cmd/provider/set_source.go b/cmd/provider/set_source.go index 13255d835..d06bef79a 100644 --- a/cmd/provider/set_source.go +++ b/cmd/provider/set_source.go @@ -50,7 +50,7 @@ func NewSetSourceCmd(flags *flags.GlobalFlags) *cobra.Command { func (cmd *SetSourceCmd) Run(ctx context.Context, devsyConfig *config.Config, args []string) error { if cmd.Version != "" { - return cmd.runPinVersion(devsyConfig, args) + return cmd.runPinVersion(ctx, devsyConfig, args) } if len(args) != 1 && len(args) != 2 { @@ -63,7 +63,7 @@ func (cmd *SetSourceCmd) Run(ctx context.Context, devsyConfig *config.Config, ar providerSource = args[1] } - providerConfig, err := workspace.UpdateProvider(devsyConfig, args[0], providerSource) + providerConfig, err := workspace.UpdateProvider(ctx, devsyConfig, args[0], providerSource) if err != nil { return err } @@ -88,7 +88,11 @@ func (cmd *SetSourceCmd) Run(ctx context.Context, devsyConfig *config.Config, ar return writeDefaultProvider(cmd.Context, providerConfig.Name) } -func (cmd *SetSourceCmd) runPinVersion(devsyConfig *config.Config, args []string) error { +func (cmd *SetSourceCmd) runPinVersion( + ctx context.Context, + devsyConfig *config.Config, + args []string, +) error { if len(args) == 0 { return fmt.Errorf("provider name must be provided when using --version") } @@ -96,7 +100,12 @@ func (cmd *SetSourceCmd) runPinVersion(devsyConfig *config.Config, args []string return fmt.Errorf("--version and a source argument are mutually exclusive") } providerName := args[0] - if err := workspace.SetProviderVersion(devsyConfig, providerName, cmd.Version); err != nil { + if err := workspace.SetProviderVersion( + ctx, + devsyConfig, + providerName, + cmd.Version, + ); err != nil { return err } log.Infof("pinned provider %s to version %s", providerName, cmd.Version) diff --git a/cmd/workspace/build.go b/cmd/workspace/build.go index 79b0f632b..6e58252bb 100644 --- a/cmd/workspace/build.go +++ b/cmd/workspace/build.go @@ -95,6 +95,10 @@ func NewBuildCmd(flags *flags.GlobalFlags) *cobra.Command { buildCmd.Flags(). BoolVar(&cmd.GitCloneRecursiveSubmodules, "git-clone-recursive-submodules", false, "If true will clone git submodule repositories recursively") + buildCmd.Flags(). + Var(&cmd.GitLFSMode, "git-lfs-mode", + "How Devsy handles Git LFS after cloning. Can be full (default, download LFS "+ + "content), setup-only (configure LFS but leave pointer files) or skip (ignore LFS)") buildCmd.Flags(). StringVar(&cmd.ImageName, "image-name", "", "Alternative name for the built image") diff --git a/cmd/workspace/ssh.go b/cmd/workspace/ssh.go index 227654841..052b93f9d 100644 --- a/cmd/workspace/ssh.go +++ b/cmd/workspace/ssh.go @@ -19,6 +19,7 @@ import ( client2 "github.com/devsy-org/devsy/pkg/client" "github.com/devsy-org/devsy/pkg/client/clientimplementation" "github.com/devsy-org/devsy/pkg/config" + "github.com/devsy-org/devsy/pkg/git" "github.com/devsy-org/devsy/pkg/gpg" "github.com/devsy-org/devsy/pkg/log" "github.com/devsy-org/devsy/pkg/port" @@ -698,7 +699,7 @@ func (cmd *SSHCmd) setupGPGAgent( gpgExtraSocketPath := strings.TrimSpace(string(gpgExtraSocketBytes)) log.Debugf("[GPG] detected gpg-agent socket path %s", gpgExtraSocketPath) - gitKey := gpgSigningKey() + gitKey := gpgSigningKey(ctx) cmd.ReverseForwardPorts = append(cmd.ReverseForwardPorts, gpgExtraSocketPath) @@ -756,39 +757,34 @@ func (cmd *SSHCmd) setupGPGAgent( // gpgSigningKey returns the user's GPG signing key from git config, // or empty string if no key is configured or the signing format is SSH // (SSH signing keys are handled by the separate SSH signature helper). -func gpgSigningKey() string { - format, err := exec.Command("git", "config", "--get", "gpg.format").Output() - formatStr := "" - if err == nil { - formatStr = strings.TrimSpace(string(format)) - } +func gpgSigningKey(ctx context.Context) string { + config := git.At("").Config() + formatStr, _ := config.Get(ctx, "gpg.format", git.ScopeDefault) if formatStr == "ssh" { log.Debugf( - "[GPG] gpg.format is ssh, skipping GPG signing key (handled by SSH signing helper)", + "gpg.format is ssh, skipping GPG signing key", ) return "" } - key, err := exec.Command("git", "config", "--get", "user.signingKey").Output() + result, err := config.Get(ctx, "user.signingKey", git.ScopeDefault) if err != nil { - log.Debugf("[GPG] no git signkey detected, skipping") + log.Debugf("no git signkey detected, skipping") return "" } - result := strings.TrimSpace(string(key)) - // GPG key IDs are hex fingerprints, not file paths. If the signing key // looks like a file path and the format isn't x509 (which legitimately // uses certificate file paths via gpgsm), it's an SSH key. if (strings.HasPrefix(result, "/") || strings.HasPrefix(result, "~")) && formatStr != "x509" { log.Debugf( - "[GPG] signing key %s looks like a file path, skipping (not a GPG key ID)", + "signing key %s looks like a file path, skipping", result, ) return "" } - log.Debugf("[GPG] detected git sign key %s", result) + log.Debugf("detected git sign key %s", result) return result } diff --git a/cmd/workspace/ssh_test.go b/cmd/workspace/ssh_test.go index 776bffebd..05b10ccd5 100644 --- a/cmd/workspace/ssh_test.go +++ b/cmd/workspace/ssh_test.go @@ -1,6 +1,7 @@ package workspace import ( + "context" "os" "path/filepath" "testing" @@ -24,7 +25,7 @@ func writeGitConfig(t *testing.T, content string) { func TestGpgSigningKey_GPGFormat(t *testing.T) { writeGitConfig(t, "[user]\n\tsigningKey = TESTKEY123\n") - result := gpgSigningKey() + result := gpgSigningKey(context.Background()) assert.Equal(t, "TESTKEY123", result) } @@ -33,30 +34,30 @@ func TestGpgSigningKey_SSHFormat_Skipped(t *testing.T) { t, "[gpg]\n\tformat = ssh\n[user]\n\tsigningKey = /home/user/.ssh/id_ed25519.pub\n", ) - result := gpgSigningKey() + result := gpgSigningKey(context.Background()) assert.Empty(t, result) } func TestGpgSigningKey_NoKeyConfigured(t *testing.T) { writeGitConfig(t, "[user]\n\tname = Test\n") - result := gpgSigningKey() + result := gpgSigningKey(context.Background()) assert.Empty(t, result) } func TestGpgSigningKey_X509Format_Returned(t *testing.T) { writeGitConfig(t, "[gpg]\n\tformat = x509\n[user]\n\tsigningKey = /path/to/cert\n") - result := gpgSigningKey() + result := gpgSigningKey(context.Background()) assert.Equal(t, "/path/to/cert", result) } func TestGpgSigningKey_SSHKeyPath_Skipped(t *testing.T) { writeGitConfig(t, "[user]\n\tsigningKey = /home/user/.ssh/id_ed25519.pub\n") - result := gpgSigningKey() + result := gpgSigningKey(context.Background()) assert.Empty(t, result) } func TestGpgSigningKey_TildeKeyPath_Skipped(t *testing.T) { writeGitConfig(t, "[user]\n\tsigningKey = ~/.ssh/id_ed25519.pub\n") - result := gpgSigningKey() + result := gpgSigningKey(context.Background()) assert.Empty(t, result) } diff --git a/cmd/workspace/up/up_client.go b/cmd/workspace/up/up_client.go index 2269ae265..539782622 100644 --- a/cmd/workspace/up/up_client.go +++ b/cmd/workspace/up/up_client.go @@ -97,7 +97,7 @@ func (cmd *UpCmd) prepareClient( } if !cmd.Platform.Enabled { proInstance := workspace2.GetProInstance(devsyConfig, client.Provider()) - if err := workspace2.CheckProviderUpdate(devsyConfig, proInstance); err != nil { + if err := workspace2.CheckProviderUpdate(ctx, devsyConfig, proInstance); err != nil { return nil, err } } diff --git a/cmd/workspace/up/up_flags.go b/cmd/workspace/up/up_flags.go index 7d1c4f9ee..bea0242d7 100644 --- a/cmd/workspace/up/up_flags.go +++ b/cmd/workspace/up/up_flags.go @@ -159,6 +159,10 @@ func (cmd *UpCmd) registerGitFlags(upCmd *cobra.Command) { upCmd.Flags(). BoolVar(&cmd.GitCloneRecursiveSubmodules, "git-clone-recursive-submodules", false, "If true will clone git submodule repositories recursively") + upCmd.Flags(). + Var(&cmd.GitLFSMode, "git-lfs-mode", + "How Devsy handles Git LFS after cloning. Can be full (default, download LFS "+ + "content), setup-only (configure LFS but leave pointer files) or skip (ignore LFS)") upCmd.Flags(). StringVar(&cmd.GitSSHSigningKey, "git-ssh-signing-key", "", "The ssh key to use when signing git commits. Used to explicitly setup Devsy's ssh signature "+ diff --git a/cmd/workspace/workspace.go b/cmd/workspace/workspace.go index 68c4f1798..4152bc894 100644 --- a/cmd/workspace/workspace.go +++ b/cmd/workspace/workspace.go @@ -9,8 +9,9 @@ import ( // NewWorkspaceCmd builds the 'devsy workspace' parent command. func NewWorkspaceCmd(globalFlags *flags.GlobalFlags) *cobra.Command { cmd := &cobra.Command{ - Use: "workspace", - Short: "Manage devcontainer workspaces", + Use: "workspace", + Aliases: []string{"ws"}, + Short: "Manage devcontainer workspaces", } cmd.AddCommand(up.NewUpCmd(globalFlags)) cmd.AddCommand(NewStopCmd(globalFlags)) diff --git a/pkg/agent/tunnelserver/tunnelserver.go b/pkg/agent/tunnelserver/tunnelserver.go index e6ef5da3a..637fcf4b3 100644 --- a/pkg/agent/tunnelserver/tunnelserver.go +++ b/pkg/agent/tunnelserver/tunnelserver.go @@ -232,7 +232,7 @@ func (t *tunnelServer) GitUser(ctx context.Context, empty *tunnel.Empty) (*tunne if t.workspace != nil { workingDir = t.workspace.Source.LocalFolder } - gitUser, err := gitcredentials.GetUser("", workingDir) + gitUser, err := gitcredentials.GetUser(ctx, "", workingDir) if err != nil { return nil, err } @@ -304,7 +304,7 @@ func (t *tunnelServer) GitCredentials( log.Warn("workspace is not available for git credentials") } - response, err := gitcredentials.GetCredentials(credentials) + response, err := gitcredentials.GetCredentials(ctx, credentials) if err != nil { return nil, fmt.Errorf("get git response: %w", err) } diff --git a/pkg/agent/workspace.go b/pkg/agent/workspace.go index 38e241786..bdb8c569c 100644 --- a/pkg/agent/workspace.go +++ b/pkg/agent/workspace.go @@ -381,6 +381,7 @@ func CloneRepositoryForWorkspace( defer func() { if helper != "" { if err := gitcredentials.RemoveHelperFromPath( + ctx, gitcredentials.GetLocalGitConfigPath(workspaceDir), ); err != nil { log.Errorf("Remove git credential helper: %v", err) @@ -396,7 +397,7 @@ func CloneRepositoryForWorkspace( "seems like git isn't installed on your system. Make sure to install git and make it available in the PATH", ) } - if err := git.InstallBinary(); err != nil { + if err := git.InstallBinary(ctx); err != nil { return err } } @@ -512,14 +513,10 @@ func CloneRepositoryForWorkspace( if options.Platform.GitSkipLFS { log.Info("Skipping Git LFS") } - err := git.CloneRepositoryWithEnv( - ctx, - gitInfo, - extraEnv, - workspaceDir, - helper, - options.StrictHostKeyChecking, - getGitOptions(options)...) + repo := git.At(workspaceDir, + git.WithStrictHostKeyChecking(options.StrictHostKeyChecking), + git.WithEnv(extraEnv)) + err := repo.CloneFromInfo(ctx, gitInfo, helper, getGitOptions(options)...) if err != nil { // cleanup workspace dir if clone failed, otherwise we won't try to clone again when rebuilding this workspace if cleanupErr := cleanupWorkspaceDir(workspaceDir); cleanupErr != nil { @@ -563,8 +560,11 @@ func getGitOptions(options provider2.CLIOptions) []git.Option { git.WithCloneStrategy(git.CloneStrategy(options.Platform.GitCloneStrategy)), ) } + // Platform.GitSkipLFS forces skipping; otherwise honor the requested LFS mode. if options.Platform.GitSkipLFS { - gitOpts = append(gitOpts, git.WithSkipLFS()) + gitOpts = append(gitOpts, git.WithLFSMode(git.LFSSkip)) + } else { + gitOpts = append(gitOpts, git.WithLFSMode(options.GitLFSMode)) } if options.GitCloneRecursiveSubmodules { gitOpts = append(gitOpts, git.WithRecursiveSubmodules()) diff --git a/pkg/config/pathmanager.go b/pkg/config/pathmanager.go index 006f702be..9d9997d60 100644 --- a/pkg/config/pathmanager.go +++ b/pkg/config/pathmanager.go @@ -33,6 +33,10 @@ type PathManager interface { StateDir() (string, error) RuntimeDir() (string, error) + // SystemBinDir is a directory on the user's PATH where standalone tool + // binaries (e.g. a git-lfs release fallback) can be installed. + SystemBinDir() (string, error) + // Config paths. ConfigFilePath() (string, error) diff --git a/pkg/config/pathmanager_darwin.go b/pkg/config/pathmanager_darwin.go index 20cce0a95..6ae22c907 100644 --- a/pkg/config/pathmanager_darwin.go +++ b/pkg/config/pathmanager_darwin.go @@ -68,3 +68,8 @@ func (d *darwinPathManager) RuntimeDir() (string, error) { } return dir, nil } + +// SystemBinDir returns a PATH directory for installing standalone tool binaries. +func (d *darwinPathManager) SystemBinDir() (string, error) { + return "/usr/local/bin", nil +} diff --git a/pkg/config/pathmanager_linux.go b/pkg/config/pathmanager_linux.go index 5861646f2..3b9732243 100644 --- a/pkg/config/pathmanager_linux.go +++ b/pkg/config/pathmanager_linux.go @@ -58,3 +58,7 @@ func (l *linuxPathManager) StateDir() (string, error) { func (l *linuxPathManager) RuntimeDir() (string, error) { return ensureDir(filepath.Join(os.TempDir(), fmt.Sprintf("%s-%d", RepoName, os.Getuid()))) } + +func (l *linuxPathManager) SystemBinDir() (string, error) { + return "/usr/local/bin", nil +} diff --git a/pkg/config/pathmanager_linux_test.go b/pkg/config/pathmanager_linux_test.go index 58d0b0455..4b06a87d5 100644 --- a/pkg/config/pathmanager_linux_test.go +++ b/pkg/config/pathmanager_linux_test.go @@ -50,6 +50,7 @@ func TestLinuxDefaults(t *testing.T) { {"CacheDir", pm.CacheDir, filepath.Join(home, ".cache", RepoName)}, {"StateDir", pm.StateDir, filepath.Join(home, "."+RepoName, "state")}, {"RuntimeDir", pm.RuntimeDir, wantRuntime}, + {"SystemBinDir", pm.SystemBinDir, "/usr/local/bin"}, } for _, tt := range tests { diff --git a/pkg/config/pathmanager_windows.go b/pkg/config/pathmanager_windows.go index c23da1b8f..37879b528 100644 --- a/pkg/config/pathmanager_windows.go +++ b/pkg/config/pathmanager_windows.go @@ -58,3 +58,14 @@ func (w *windowsPathManager) StateDir() (string, error) { func (w *windowsPathManager) RuntimeDir() (string, error) { return ensureDir(filepath.Join(os.TempDir(), RepoName)) } + +// SystemBinDir returns a PATH directory for installing standalone tool binaries. +// %LOCALAPPDATA%\Microsoft\WindowsApps is on PATH by default. +func (w *windowsPathManager) SystemBinDir() (string, error) { + localAppData := os.Getenv("LOCALAPPDATA") + if localAppData == "" { + return "", fmt.Errorf("system bin dir: LOCALAPPDATA environment variable is not set") + } + + return filepath.Join(localAppData, "Microsoft", "WindowsApps"), nil +} diff --git a/pkg/daemon/platform/local_server.go b/pkg/daemon/platform/local_server.go index aab6d0985..5e14f279f 100644 --- a/pkg/daemon/platform/local_server.go +++ b/pkg/daemon/platform/local_server.go @@ -721,7 +721,7 @@ func (l *localServer) getGitCredentials( protocol = "https" } - credentials, err := gitcredentials.GetCredentials(&gitcredentials.GitCredentials{ + credentials, err := gitcredentials.GetCredentials(r.Context(), &gitcredentials.GitCredentials{ Protocol: protocol, Host: host, }) diff --git a/pkg/devcontainer/helpers.go b/pkg/devcontainer/helpers.go index 9c3b67711..27ea57652 100644 --- a/pkg/devcontainer/helpers.go +++ b/pkg/devcontainer/helpers.go @@ -1,7 +1,6 @@ package devcontainer import ( - "bufio" "context" "io/fs" "path/filepath" @@ -128,14 +127,8 @@ func findFilesInGitRepo( SubPath: opts.gitSubDir, } log.Debugf("Cloning Git repository into %s", opts.tmpDirPath) - err := git.CloneRepository( - ctx, - gitInfo, - opts.tmpDirPath, - "", - opts.strictHostKeyChecking, - git.WithCloneStrategy(git.BareCloneStrategy), - ) + err := git.At(opts.tmpDirPath, git.WithStrictHostKeyChecking(opts.strictHostKeyChecking)). + CloneFromInfo(ctx, gitInfo, "", git.WithCloneStrategy(git.BareCloneStrategy)) if err != nil { return nil, err } @@ -146,22 +139,13 @@ func findFilesInGitRepo( if opts.gitBranch != "" { ref = opts.gitBranch } - // git ls-tree -r --full-name --name-only $REF - lsArgs := []string{"ls-tree", "-r", "--full-name", "--name-only", ref} - lsCmd := git.CommandContext(ctx, git.GetDefaultExtraEnv(opts.strictHostKeyChecking), lsArgs...) - lsCmd.Dir = opts.tmpDirPath - stdout, err := lsCmd.StdoutPipe() - if err != nil { - return nil, err - } - err = lsCmd.Start() + repo := git.At(opts.tmpDirPath, git.WithStrictHostKeyChecking(opts.strictHostKeyChecking)) + paths, err := repo.LsTree(ctx, ref) if err != nil { return nil, err } - scanner := bufio.NewScanner(stdout) - for scanner.Scan() { - path := scanner.Text() + for _, path := range paths { depth := strings.Count(path, string(filepath.Separator)) if depth > opts.maxDepth { continue @@ -171,11 +155,6 @@ func findFilesInGitRepo( } } - err = lsCmd.Wait() - if err != nil { - return nil, err - } - return result, nil } diff --git a/pkg/devcontainer/setup/dotfiles.go b/pkg/devcontainer/setup/dotfiles.go index 2e68b9d1d..b283d51df 100644 --- a/pkg/devcontainer/setup/dotfiles.go +++ b/pkg/devcontainer/setup/dotfiles.go @@ -67,7 +67,8 @@ func cloneDotfiles(ctx context.Context, repo, targetDir string) error { log.Infof("Cloning dotfiles %s", repo) gitInfo := git.NormalizeRepository(repo) - return git.CloneRepository(ctx, gitInfo, targetDir, "", false) + return git.At(targetDir, git.WithStrictHostKeyChecking(false)). + CloneFromInfo(ctx, gitInfo, "") } func installDotfiles( diff --git a/pkg/devcontainer/setup/setup.go b/pkg/devcontainer/setup/setup.go index 05d5c0021..9c1e70d3f 100644 --- a/pkg/devcontainer/setup/setup.go +++ b/pkg/devcontainer/setup/setup.go @@ -189,6 +189,7 @@ func setupOptionalFeatures(ctx context.Context, cfg *ContainerSetupConfig) { if cfg.PlatformOptions != nil { if err := setupPlatformGitCredentials( + ctx, config.GetRemoteUser(cfg.SetupInfo), cfg.PlatformOptions, ); err != nil { @@ -513,6 +514,7 @@ func markerFileExists(markerName string, markerContent string) (bool, error) { } func setupPlatformGitCredentials( + ctx context.Context, userName string, platformOptions *devsy.PlatformOptions, ) error { @@ -524,10 +526,10 @@ func setupPlatformGitCredentials( // setup platform git user if platformOptions.UserCredentials.GitUser != "" && platformOptions.UserCredentials.GitEmail != "" { - gitUser, err := gitcredentials.GetUser(userName, "") + gitUser, err := gitcredentials.GetUser(ctx, userName, "") if err == nil && gitUser.Name == "" && gitUser.Email == "" { log.Info("Setup workspace git user and email") - err := gitcredentials.SetUser(userName, &gitcredentials.GitUser{ + err := gitcredentials.SetUser(ctx, userName, &gitcredentials.GitUser{ Name: platformOptions.UserCredentials.GitUser, Email: platformOptions.UserCredentials.GitEmail, }) @@ -538,7 +540,7 @@ func setupPlatformGitCredentials( } // setup platform git http credentials - err := setupPlatformGitHTTPCredentials(userName, platformOptions) + err := setupPlatformGitHTTPCredentials(ctx, userName, platformOptions) if err != nil { log.Errorf("Error setting up platform git http credentials: %v", err) } @@ -553,6 +555,7 @@ func setupPlatformGitCredentials( } func setupPlatformGitHTTPCredentials( + ctx context.Context, userName string, platformOptions *devsy.PlatformOptions, ) error { @@ -565,7 +568,7 @@ func setupPlatformGitHTTPCredentials( if err != nil { return err } - err = gitcredentials.ConfigureHelper(binaryPath, userName, -1) + err = gitcredentials.ConfigureHelper(ctx, binaryPath, userName, -1) if err != nil { return fmt.Errorf("configure git helper: %w", err) } diff --git a/pkg/download/download.go b/pkg/download/download.go index d0d28fba5..f89027cef 100644 --- a/pkg/download/download.go +++ b/pkg/download/download.go @@ -1,6 +1,7 @@ package download import ( + "context" "encoding/json" "fmt" "io" @@ -8,11 +9,28 @@ import ( "net/url" "strings" - "github.com/devsy-org/devsy/pkg/gitcredentials" devsyhttp "github.com/devsy-org/devsy/pkg/http" "github.com/devsy-org/devsy/pkg/log" ) +// CredentialResolver resolves a username/password (token) for a host, used to +// authenticate downloads of private assets. +type CredentialResolver interface { + Resolve(ctx context.Context, protocol, host, path string) (username, password string, err error) +} + +type options struct { + resolver CredentialResolver +} + +// Option configures a download. +type Option func(*options) + +// WithCredentialResolver enables authenticated retries for private assets. +func WithCredentialResolver(resolver CredentialResolver) Option { + return func(o *options) { o.resolver = resolver } +} + // HTTPStatusError wraps HTTP status code errors for better error handling. type HTTPStatusError struct { StatusCode int @@ -57,8 +75,8 @@ func sanitizeURL(raw string) string { return scheme + "://" + afterAt } -func Head(rawURL string) (int, error) { - req, err := http.NewRequest(http.MethodHead, rawURL, nil) +func Head(ctx context.Context, rawURL string) (int, error) { + req, err := http.NewRequestWithContext(ctx, http.MethodHead, rawURL, nil) if err != nil { return 0, err } @@ -72,19 +90,19 @@ func Head(rawURL string) (int, error) { return resp.StatusCode, nil } -func File(rawURL string) (io.ReadCloser, error) { - parsed, err := url.Parse(rawURL) - if err != nil { - return nil, err +func File(ctx context.Context, rawURL string, opts ...Option) (io.ReadCloser, error) { + cfg := &options{} + for _, opt := range opts { + opt(cfg) } - req, err := http.NewRequest(http.MethodGet, rawURL, nil) + parsed, err := url.Parse(rawURL) if err != nil { return nil, err } - if parsed.Host == "github.com" { - body, err := tryGithubPrivateDownload(parsed) + if parsed.Host == "github.com" && cfg.resolver != nil { + body, err := fetchGithubPrivateRelease(ctx, parsed, cfg.resolver) if err != nil { return nil, err } @@ -93,10 +111,22 @@ func File(rawURL string) (io.ReadCloser, error) { } } + return getURL(ctx, rawURL) +} + +// getURL performs an anonymous GET and returns the response body, mapping +// non-2xx/3xx responses to an HTTPStatusError. +func getURL(ctx context.Context, rawURL string) (io.ReadCloser, error) { + req, err := http.NewRequestWithContext(ctx, http.MethodGet, rawURL, nil) + if err != nil { + return nil, err + } + resp, err := devsyhttp.GetHTTPClient().Do(req) if err != nil { return nil, fmt.Errorf("download file: %w", err) - } else if resp.StatusCode >= 400 { + } + if resp.StatusCode >= 400 { body, _ := io.ReadAll(io.LimitReader(resp.Body, 1024)) _ = resp.Body.Close() return nil, &HTTPStatusError{StatusCode: resp.StatusCode, URL: rawURL, Body: string(body)} @@ -105,12 +135,17 @@ func File(rawURL string) (io.ReadCloser, error) { return resp.Body, nil } -// tryGithubPrivateDownload attempts to download a GitHub release asset using -// git credentials when the URL returns a 404 (indicating a private repo). -// Returns (nil, nil) if the URL is not a private GitHub release or credentials -// are unavailable, allowing the caller to fall through to a normal download. -func tryGithubPrivateDownload(parsed *url.URL) (io.ReadCloser, error) { - code, err := Head(parsed.String()) +// fetchGithubPrivateRelease attempts to download a GitHub release asset using +// credentials from the resolver when the URL returns a 404 (indicating a +// private repo). Returns (nil, nil) if the URL is not a private GitHub release +// or credentials are unavailable, allowing the caller to fall through to a +// normal download. +func fetchGithubPrivateRelease( + ctx context.Context, + parsed *url.URL, + resolver CredentialResolver, +) (io.ReadCloser, error) { + code, err := Head(ctx, parsed.String()) if err != nil { return nil, err } @@ -123,18 +158,14 @@ func tryGithubPrivateDownload(parsed *url.URL) (io.ReadCloser, error) { return nil, nil } - log.Debugf("Try to find credentials for github") - credentials, err := gitcredentials.GetCredentials(&gitcredentials.GitCredentials{ - Protocol: parsed.Scheme, - Host: parsed.Host, - Path: parsed.Path, - }) - if err != nil || credentials == nil || credentials.Password == "" { + log.Debugf("try to find credentials for github") + _, password, err := resolver.Resolve(ctx, parsed.Scheme, parsed.Host, parsed.Path) + if err != nil || password == "" { return nil, nil } - log.Debugf("Make request with credentials") - return downloadGithubRelease(ref, credentials.Password) + log.Debugf("make request with credentials") + return downloadGithubRelease(ctx, ref, password) } type githubReleaseRef struct { @@ -150,13 +181,17 @@ type GithubReleaseAsset struct { Name string `json:"name,omitempty"` } -func downloadGithubRelease(ref githubReleaseRef, token string) (io.ReadCloser, error) { - assetID, err := fetchGithubReleaseAssetID(ref, token) +func downloadGithubRelease( + ctx context.Context, + ref githubReleaseRef, + token string, +) (io.ReadCloser, error) { + assetID, err := fetchGithubReleaseAssetID(ctx, ref, token) if err != nil { return nil, err } - return downloadGithubAsset(ref.org, ref.repo, assetID, token) + return downloadGithubAsset(ctx, ref, assetID, token) } func (ref githubReleaseRef) releaseURL() string { @@ -183,8 +218,8 @@ func (ref githubReleaseRef) releaseURL() string { }).String() } -func githubAPIGetJSON(apiURL, token string) ([]byte, error) { - req, err := http.NewRequest(http.MethodGet, apiURL, nil) +func githubAPIGetJSON(ctx context.Context, apiURL, token string) ([]byte, error) { + req, err := http.NewRequestWithContext(ctx, http.MethodGet, apiURL, nil) if err != nil { return nil, err } @@ -209,10 +244,14 @@ func githubAPIGetJSON(apiURL, token string) ([]byte, error) { return io.ReadAll(resp.Body) } -func fetchGithubReleaseAssetID(ref githubReleaseRef, token string) (int, error) { +func fetchGithubReleaseAssetID( + ctx context.Context, + ref githubReleaseRef, + token string, +) (int, error) { releaseURL := ref.releaseURL() - raw, err := githubAPIGetJSON(releaseURL, token) + raw, err := githubAPIGetJSON(ctx, releaseURL, token) if err != nil { return 0, err } @@ -231,11 +270,16 @@ func fetchGithubReleaseAssetID(ref githubReleaseRef, token string) (int, error) return 0, fmt.Errorf("couldn't find asset %s in github release (%s)", ref.file, releaseURL) } -func downloadGithubAsset(org, repo string, assetID int, token string) (io.ReadCloser, error) { +func downloadGithubAsset( + ctx context.Context, + ref githubReleaseRef, + assetID int, + token string, +) (io.ReadCloser, error) { assetPath := fmt.Sprintf( "/repos/%s/%s/releases/assets/%d", - url.PathEscape(org), - url.PathEscape(repo), + url.PathEscape(ref.org), + url.PathEscape(ref.repo), assetID, ) assetURL := (&url.URL{ @@ -244,7 +288,7 @@ func downloadGithubAsset(org, repo string, assetID int, token string) (io.ReadCl Path: assetPath, }).String() - req, err := http.NewRequest(http.MethodGet, assetURL, nil) + req, err := http.NewRequestWithContext(ctx, http.MethodGet, assetURL, nil) if err != nil { return nil, err } diff --git a/pkg/driver/custom/custom.go b/pkg/driver/custom/custom.go index de92925de..52b1edee9 100644 --- a/pkg/driver/custom/custom.go +++ b/pkg/driver/custom/custom.go @@ -294,7 +294,7 @@ func (c *customDriver) runCommand( log.Debugf("Run %s driver command: %s", name, strings.Join(command, " ")) // get environ - environ, err := ToEnvironWithBinaries(c.workspaceInfo) + environ, err := ToEnvironWithBinaries(ctx, c.workspaceInfo) if err != nil { return err } @@ -318,6 +318,7 @@ func (c *customDriver) runCommand( } func ToEnvironWithBinaries( + ctx context.Context, workspace *provider.AgentWorkspaceInfo, ) ([]string, error) { // get binaries dir @@ -330,7 +331,7 @@ func ToEnvironWithBinaries( } // download binaries - agentBinaries, err := provider.DownloadBinaries(workspace.Agent.Binaries, binariesDir) + agentBinaries, err := provider.DownloadBinaries(ctx, workspace.Agent.Binaries, binariesDir) if err != nil { return nil, fmt.Errorf( "error downloading workspace %s binaries: %w", diff --git a/pkg/git/args.go b/pkg/git/args.go new file mode 100644 index 000000000..e306ab43d --- /dev/null +++ b/pkg/git/args.go @@ -0,0 +1,35 @@ +package git + +// Shared git subcommands, flags, and binary names used to build command lines. +// Centralized so repeated literals stay consistent (and satisfy goconst). +const ( + binGit = "git" + binGitLFS = "git-lfs" + + subClone = "clone" + subConfig = "config" + + flagDepth1 = "--depth=1" + flagBranch = "--branch" + flagConfig = "--config" + flagProgress = "--progress" + flagGlobal = "--global" + flagSystem = "--system" + flagFile = "--file" + flagGet = "--get" + + lfsInstall = "install" + pkgUpdate = "update" + + gitAttributesFile = ".gitattributes" + + // Install strategy names. + strategyApt = "apt" + strategyApk = "apk" + strategyRelease = "github-release" + + // GOOS values the release installer recognizes. + osLinux = "linux" + osDarwin = "darwin" + osWindows = "windows" +) diff --git a/pkg/git/clone.go b/pkg/git/clone.go index 8a25356a8..2a683e30d 100644 --- a/pkg/git/clone.go +++ b/pkg/git/clone.go @@ -1,13 +1,12 @@ package git import ( - "context" "fmt" - "github.com/devsy-org/devsy/pkg/log" "github.com/spf13/pflag" ) +// CloneStrategy selects how much history and object data a clone fetches. type CloneStrategy string const ( @@ -18,122 +17,143 @@ const ( BareCloneStrategy CloneStrategy = "bare" ) -type Cloner interface { - Clone( - ctx context.Context, - repository string, - targetDir string, - extraArgs, extraEnv []string, - ) error +// strategyArgs is the single source of truth mapping each strategy to its +// `git clone` flags. It drives both argument construction and flag validation. +var strategyArgs = map[CloneStrategy][]string{ + FullCloneStrategy: nil, + BloblessCloneStrategy: {"--filter=blob:none"}, + TreelessCloneStrategy: {"--filter=tree:0"}, + ShallowCloneStrategy: {flagDepth1}, + BareCloneStrategy: {"--bare", flagDepth1}, } -type Option func(*cloner) +// Option configures a clone performed by Repo.Clone or Repo.CloneFromInfo. +type Option func(*cloneConfig) +// WithCloneStrategy selects the clone strategy. An empty strategy is treated as +// FullCloneStrategy. func WithCloneStrategy(strategy CloneStrategy) Option { - return func(c *cloner) { - if strategy == "" { - strategy = FullCloneStrategy - } - c.cloneStrategy = strategy - } + return func(c *cloneConfig) { c.strategy = strategy } } -func WithRecursiveSubmodules() Option { - return func(c *cloner) { - c.extraArgs = append(c.extraArgs, "--recurse-submodules") - } +// WithBranch clones only the named branch. +func WithBranch(branch string) Option { + return func(c *cloneConfig) { c.branch = branch } } -func WithSkipLFS() Option { - return func(c *cloner) { - c.skipLFS = true - } +// WithCredentialHelper configures a git credential helper for the clone. +func WithCredentialHelper(helper string) Option { + return func(c *cloneConfig) { c.credentialHelper = helper } } -func NewClonerWithOpts(options ...Option) Cloner { - cloner := &cloner{ - cloneStrategy: FullCloneStrategy, - } - for _, opt := range options { - opt(cloner) - } - return cloner +// WithRecursiveSubmodules clones submodules recursively. +func WithRecursiveSubmodules() Option { + return func(c *cloneConfig) { c.recurseSubmodules = true } } -func NewCloner(strategy CloneStrategy) Cloner { - return NewClonerWithOpts(WithCloneStrategy(strategy)) +// LFSMode controls how Git LFS is handled after a clone. +type LFSMode int + +const ( + // LFSFull registers the LFS filters and downloads LFS content. This matches + // git's default clone behavior and is the zero value. + LFSFull LFSMode = iota + // LFSSetupOnly registers the LFS filters but does not download content, + // leaving pointer stubs. Future checkouts/pulls will hydrate on demand. + LFSSetupOnly + // LFSSkip does nothing: no filters, no download. + LFSSkip +) + +const lfsModeSkip = "skip" + +// lfsModeNames maps each LFSMode to its CLI string, and back via Set. +var lfsModeNames = map[LFSMode]string{ + LFSFull: "full", + LFSSetupOnly: "setup-only", + LFSSkip: lfsModeSkip, } -var _ pflag.Value = (*CloneStrategy)(nil) +// LFSMode implements pflag.Value so it can back a CLI flag. +var _ pflag.Value = (*LFSMode)(nil) -func (s *CloneStrategy) Set(v string) error { - switch v { - case string(FullCloneStrategy), - string(BloblessCloneStrategy), - string(TreelessCloneStrategy), - string(ShallowCloneStrategy), - string(BareCloneStrategy): - { - *s = CloneStrategy(v) +func (m *LFSMode) Set(v string) error { + for mode, name := range lfsModeNames { + if name == v { + *m = mode return nil } - default: - return fmt.Errorf("CloneStrategy %s not supported", v) } + return fmt.Errorf("unsupported git-lfs mode %q (want full, setup-only, or skip)", v) } -func (s *CloneStrategy) Type() string { - return "cloneStrategy" +func (m *LFSMode) Type() string { + return "lfsMode" } -func (s *CloneStrategy) String() string { - return string(*s) +func (m LFSMode) String() string { + return lfsModeNames[m] } -type cloner struct { - extraArgs []string - cloneStrategy CloneStrategy - skipLFS bool +// WithLFSMode selects how Git LFS is handled after the clone. The default is +// LFSFull. +func WithLFSMode(mode LFSMode) Option { + return func(c *cloneConfig) { c.lfsMode = mode } } -var _ Cloner = &cloner{} +// WithSkipLFS disables Git LFS smudge and hydration for the clone. Equivalent +// to WithLFSMode(LFSSkip). +func WithSkipLFS() Option { + return WithLFSMode(LFSSkip) +} + +// cloneConfig is the resolved set of clone options. +type cloneConfig struct { + strategy CloneStrategy + branch string + credentialHelper string + recurseSubmodules bool + lfsMode LFSMode +} -func (c *cloner) initialArgs() []string { - switch c.cloneStrategy { - case BloblessCloneStrategy: - return []string{"clone", "--filter=blob:none"} - case TreelessCloneStrategy: - return []string{"clone", "--filter=tree:0"} - case ShallowCloneStrategy: - return []string{"clone", "--depth=1"} - case BareCloneStrategy: - return []string{"clone", "--bare", "--depth=1"} - case FullCloneStrategy: - default: +func newCloneConfig(options ...Option) cloneConfig { + var c cloneConfig + for _, opt := range options { + opt(&c) } - return []string{"clone"} -} - -func (c *cloner) Clone( - ctx context.Context, - repository string, - targetDir string, - extraArgs, extraEnv []string, -) error { - args := c.initialArgs() - args = append(args, extraArgs...) - args = append(args, c.extraArgs...) - args = append(args, repository, targetDir) - args = append(args, "--progress") - - if c.skipLFS { - extraEnv = append(extraEnv, "GIT_LFS_SKIP_SMUDGE=1") + return c +} + +// args returns the `git clone` arguments for the config and given repository and target directory. +func (c cloneConfig) args(repository, targetDir string) []string { + args := append([]string{subClone}, strategyArgs[c.strategy]...) + if c.branch != "" { + args = append(args, flagBranch, c.branch) + } + if c.credentialHelper != "" { + args = append(args, flagConfig, "credential.helper="+c.credentialHelper) + } + if c.recurseSubmodules { + args = append(args, "--recurse-submodules") } + return append(args, repository, targetDir, flagProgress) +} - w := log.Writer(log.LevelInfo) - gitCommand := CommandContext(ctx, extraEnv, args...) - gitCommand.Stdout = w - gitCommand.Stderr = w +// CloneStrategy implements pflag.Value for CloneStrategy. +var _ pflag.Value = (*CloneStrategy)(nil) - return gitCommand.Run() +func (s *CloneStrategy) Set(v string) error { + if _, ok := strategyArgs[CloneStrategy(v)]; !ok { + return fmt.Errorf("unsupported clone strategy %q", v) + } + *s = CloneStrategy(v) + return nil +} + +func (s *CloneStrategy) Type() string { + return "cloneStrategy" +} + +func (s *CloneStrategy) String() string { + return string(*s) } diff --git a/pkg/git/clone_test.go b/pkg/git/clone_test.go new file mode 100644 index 000000000..0a2e0365e --- /dev/null +++ b/pkg/git/clone_test.go @@ -0,0 +1,122 @@ +package git + +import ( + "testing" + + "gotest.tools/assert" + "gotest.tools/assert/cmp" +) + +const ( + credHelperStore = "credential.helper=store" // #nosec G101 -- not a credential, a git config flag + testDir = "dir" + testBranch = "dev" + testRepo = "repo" +) + +type cloneArgsCase struct { + name string + options []Option + expected []string +} + +var cloneArgsCases = []cloneArgsCase{ + {"default full clone", nil, []string{subClone, testRepo, testDir, flagProgress}}, + { + "blobless strategy", + []Option{WithCloneStrategy(BloblessCloneStrategy)}, + []string{subClone, "--filter=blob:none", testRepo, testDir, flagProgress}, + }, + { + "treeless strategy", + []Option{WithCloneStrategy(TreelessCloneStrategy)}, + []string{subClone, "--filter=tree:0", testRepo, testDir, flagProgress}, + }, + { + "shallow strategy", + []Option{WithCloneStrategy(ShallowCloneStrategy)}, + []string{subClone, flagDepth1, testRepo, testDir, flagProgress}, + }, + { + "bare strategy", + []Option{WithCloneStrategy(BareCloneStrategy)}, + []string{subClone, "--bare", flagDepth1, testRepo, testDir, flagProgress}, + }, + { + "branch", + []Option{WithBranch(testBranch)}, + []string{subClone, flagBranch, testBranch, testRepo, testDir, flagProgress}, + }, + { + "credential helper", + []Option{WithCredentialHelper("store")}, + []string{subClone, flagConfig, credHelperStore, testRepo, testDir, flagProgress}, + }, + { + "recurse submodules", + []Option{WithRecursiveSubmodules()}, + []string{subClone, "--recurse-submodules", testRepo, testDir, flagProgress}, + }, + { + "all options compose in flag order", + []Option{ + WithCloneStrategy(ShallowCloneStrategy), + WithBranch(testBranch), + WithCredentialHelper("store"), + WithRecursiveSubmodules(), + }, + []string{ + subClone, flagDepth1, flagBranch, testBranch, + flagConfig, credHelperStore, "--recurse-submodules", + testRepo, testDir, flagProgress, + }, + }, +} + +func TestCloneConfigArgs(t *testing.T) { + for _, tc := range cloneArgsCases { + t.Run(tc.name, func(t *testing.T) { + c := newCloneConfig(tc.options...) + assert.Check(t, cmp.DeepEqual(tc.expected, c.args(testRepo, testDir))) + }) + } +} + +func TestCloneStrategySetValidates(t *testing.T) { + valid := []string{"", "blobless", "treeless", "shallow", "bare"} + for _, v := range valid { + var s CloneStrategy + assert.NilError(t, s.Set(v)) + assert.Equal(t, v, s.String()) + } + + var s CloneStrategy + assert.Assert(t, s.Set("nonsense") != nil) +} + +func TestCloneStrategyType(t *testing.T) { + var s CloneStrategy + assert.Equal(t, "cloneStrategy", s.Type()) +} + +func TestLFSModeFlagValue(t *testing.T) { + cases := map[string]LFSMode{ + "full": LFSFull, + "setup-only": LFSSetupOnly, + "skip": LFSSkip, + } + for in, want := range cases { + var m LFSMode + assert.NilError(t, m.Set(in)) + assert.Equal(t, want, m) + assert.Equal(t, in, m.String()) + } + + // Zero value renders as the default ("full") for flag help. + var zero LFSMode + assert.Equal(t, "full", zero.String()) + assert.Equal(t, "lfsMode", zero.Type()) + + var m LFSMode + assert.Assert(t, m.Set("nonsense") != nil) +} diff --git a/pkg/git/config.go b/pkg/git/config.go new file mode 100644 index 000000000..7deea0460 --- /dev/null +++ b/pkg/git/config.go @@ -0,0 +1,131 @@ +package git + +import ( + "context" + "errors" + "fmt" + "strings" +) + +// ConfigScope selects which git configuration file an operation targets. +type ConfigScope struct { + flag string + file string +} + +var ( + // ScopeDefault targets the default config file (repository, global, or system). + ScopeDefault = ConfigScope{} + // ScopeLocal targets the repository's .git/config. + ScopeLocal = ConfigScope{flag: "--local"} + // ScopeGlobal targets the user's global config. + ScopeGlobal = ConfigScope{flag: flagGlobal} + // ScopeSystem targets the system-wide config. + ScopeSystem = ConfigScope{flag: flagSystem} +) + +// ScopeFile targets a specific config file. +func ScopeFile(path string) ConfigScope { + return ConfigScope{flag: flagFile, file: path} +} + +func (s ConfigScope) args() []string { + switch { + case s.flag == "": + return nil + case s.file != "": + return []string{s.flag, s.file} + default: + return []string{s.flag} + } +} + +// Config exposes `git config` operations scoped to a repository. Obtain one via +// Repo.Config. +type Config struct { + repo *Repo +} + +// Config returns a handle for `git config` operations on the repository. +func (r *Repo) Config() *Config { + return &Config{repo: r} +} + +// Get retrieves the value of a config key in the given scope. An absent key is not an error: it returns ("", nil). +func (c *Config) Get(ctx context.Context, key string, scope ConfigScope) (string, error) { + args := append([]string{subConfig}, scope.args()...) + args = append(args, flagGet, key) + res, err := c.repo.run(ctx, args...) + value := strings.TrimSpace(string(res.Stdout)) + if err != nil { + // `git config --get` exits with code 1 when the key does not exist. + var cmdErr *CommandError + if errors.As(err, &cmdErr) && cmdErr.ExitCode == 1 { + return "", nil + } + return value, fmt.Errorf("get git config %q: %w", key, err) + } + return value, nil +} + +// Add sets a config key to value in the given scope. +func (c *Config) Add(ctx context.Context, key, value string, scope ConfigScope) error { + args := append([]string{subConfig}, scope.args()...) + args = append(args, "--add", key, value) + if _, err := c.repo.run(ctx, args...); err != nil { + return fmt.Errorf("add git config %q: %w", key, err) + } + return nil +} + +// Set sets a config key to value in the given scope, replacing any existing value. +func (c *Config) Set(ctx context.Context, key, value string, scope ConfigScope) error { + args := append([]string{subConfig}, scope.args()...) + args = append(args, key, value) + if _, err := c.repo.run(ctx, args...); err != nil { + return fmt.Errorf("set git config %q: %w", key, err) + } + return nil +} + +// Unset removes a config key in the given scope. +func (c *Config) Unset(ctx context.Context, key string, scope ConfigScope) error { + args := append([]string{subConfig}, scope.args()...) + args = append(args, "--unset", key) + if _, err := c.repo.run(ctx, args...); err != nil { + return fmt.Errorf("unset git config %q: %w", key, err) + } + return nil +} + +// UnsetAll removes all values of a multi-valued config key in the given scope. An absent key is not an error. +func (c *Config) UnsetAll(ctx context.Context, key string, scope ConfigScope) error { + args := append([]string{subConfig}, scope.args()...) + args = append(args, "--unset-all", key) + if _, err := c.repo.run(ctx, args...); err != nil { + // Exit code 5 means the key (or section) does not exist. + var cmdErr *CommandError + if errors.As(err, &cmdErr) && cmdErr.ExitCode == 5 { + return nil + } + return fmt.Errorf("unset-all git config %q: %w", key, err) + } + return nil +} + +// GetAll retrieves all values of a multi-valued config key in the given scope. +// An absent key is not an error: it returns (nil, nil). +func (c *Config) GetAll(ctx context.Context, key string, scope ConfigScope) ([]string, error) { + args := append([]string{subConfig}, scope.args()...) + args = append(args, flagGet+"-all", key) + res, err := c.repo.run(ctx, args...) + if err != nil { + // Exit code 1 means the key does not exist. + var cmdErr *CommandError + if errors.As(err, &cmdErr) && cmdErr.ExitCode == 1 { + return nil, nil + } + return nil, fmt.Errorf("get-all git config %q: %w", key, err) + } + return splitLines(res.Stdout), nil +} diff --git a/pkg/git/config_test.go b/pkg/git/config_test.go new file mode 100644 index 000000000..2dac332fb --- /dev/null +++ b/pkg/git/config_test.go @@ -0,0 +1,104 @@ +package git + +import ( + "context" + "errors" + "testing" + + "gotest.tools/assert" +) + +func TestConfigScopeArgs(t *testing.T) { + testCases := []struct { + name string + scope ConfigScope + expected []string + }{ + {"default", ScopeDefault, nil}, + {"local", ScopeLocal, []string{"--local"}}, + {"global", ScopeGlobal, []string{"--global"}}, + {"system", ScopeSystem, []string{flagSystem}}, + {"file", ScopeFile("/tmp/x.gitconfig"), []string{"--file", "/tmp/x.gitconfig"}}, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + assert.DeepEqual(t, tc.expected, tc.scope.args()) + }) + } +} + +func TestConfigGet(t *testing.T) { + fake := &fakeRunner{stdout: []byte(" ssh\n")} + config := At("/tmp/repo", WithRunner(fake)).Config() + + val, err := config.Get(context.Background(), "gpg.format", ScopeDefault) + assert.NilError(t, err) + assert.Equal(t, "ssh", val) // trimmed + assert.DeepEqual(t, []string{subConfig, flagGet, "gpg.format"}, fake.lastArgs()) +} + +func TestConfigGetWithScope(t *testing.T) { + fake := &fakeRunner{stdout: []byte("user@example.com")} + config := At("", WithRunner(fake)).Config() + + _, err := config.Get(context.Background(), "user.email", ScopeFile("/home/u/.gitconfig")) + assert.NilError(t, err) + assert.DeepEqual(t, + []string{subConfig, "--file", "/home/u/.gitconfig", flagGet, "user.email"}, + fake.lastArgs()) +} + +func TestConfigAddSystemScope(t *testing.T) { + fake := &fakeRunner{} + config := At("", WithRunner(fake)).Config() + + err := config.Add(context.Background(), "credential.helper", "!helper", ScopeSystem) + assert.NilError(t, err) + assert.DeepEqual(t, + []string{subConfig, flagSystem, "--add", "credential.helper", "!helper"}, + fake.lastArgs()) +} + +func TestConfigSetGlobalScope(t *testing.T) { + fake := &fakeRunner{} + config := At("", WithRunner(fake)).Config() + + err := config.Set(context.Background(), "user.signingKey", "KEYID", ScopeGlobal) + assert.NilError(t, err) + assert.DeepEqual(t, + []string{subConfig, "--global", "user.signingKey", "KEYID"}, + fake.lastArgs()) +} + +func TestConfigUnsetSystemScope(t *testing.T) { + fake := &fakeRunner{} + config := At("", WithRunner(fake)).Config() + + err := config.Unset(context.Background(), "credential.helper", ScopeSystem) + assert.NilError(t, err) + assert.DeepEqual(t, + []string{subConfig, flagSystem, "--unset", "credential.helper"}, + fake.lastArgs()) +} + +func TestConfigGetAbsentKeyIsNotError(t *testing.T) { + // `git config --get` exits 1 with no output when the key is absent. + fake := &fakeRunner{err: &CommandError{ExitCode: 1}} + config := At("/tmp/repo", WithRunner(fake)).Config() + + val, err := config.Get(context.Background(), "user.name", ScopeDefault) + assert.NilError(t, err) + assert.Equal(t, "", val) +} + +func TestConfigGetRealFailurePropagates(t *testing.T) { + fake := &fakeRunner{err: &CommandError{ExitCode: 128, Stderr: "fatal: bad config"}} + config := At("/tmp/repo", WithRunner(fake)).Config() + + _, err := config.Get(context.Background(), "user.name", ScopeDefault) + assert.Assert(t, err != nil) + + var cmdErr *CommandError + assert.Assert(t, errors.As(err, &cmdErr)) + assert.Equal(t, 128, cmdErr.ExitCode) +} diff --git a/pkg/git/git.go b/pkg/git/git.go index 9b26089c2..afc707c08 100644 --- a/pkg/git/git.go +++ b/pkg/git/git.go @@ -2,7 +2,6 @@ package git import ( "context" - "fmt" "os" "os/exec" "regexp" @@ -10,7 +9,6 @@ import ( "time" "github.com/devsy-org/devsy/pkg/command" - "github.com/devsy-org/devsy/pkg/log" ) const ( @@ -94,8 +92,7 @@ func PingRepository(str string, extraEnv []string) bool { timeoutCtx, cancel := context.WithTimeout(context.Background(), time.Second*10) defer cancel() - _, err := CommandContext(timeoutCtx, extraEnv, "ls-remote", "--quiet", str).CombinedOutput() - return err == nil + return At("", WithEnv(extraEnv)).LsRemote(timeoutCtx, str) == nil } func GetBranchNameForPR(ref string) string { @@ -119,24 +116,6 @@ type GitInfo struct { SubPath string } -func CloneRepository( - ctx context.Context, - gitInfo *GitInfo, - targetDir string, - helper string, - strictHostKeyChecking bool, - cloneOptions ...Option, -) error { - return CloneRepositoryWithEnv( - ctx, - gitInfo, - nil, - targetDir, - helper, - strictHostKeyChecking, - cloneOptions...) -} - func GetDefaultExtraEnv(strictHostKeyChecking bool) []string { newExtraEnv := []string{"GIT_TERMINAL_PROMPT=0"} sshArgs := "GIT_SSH_COMMAND=ssh -oBatchMode=yes -oStrictHostKeyChecking=" @@ -147,102 +126,3 @@ func GetDefaultExtraEnv(strictHostKeyChecking bool) []string { } return append(newExtraEnv, sshArgs) } - -func CloneRepositoryWithEnv( - ctx context.Context, - gitInfo *GitInfo, - extraEnv []string, - targetDir string, - helper string, - strictHostKeyChecking bool, - cloneOptions ...Option, -) error { - cloner := NewClonerWithOpts(cloneOptions...) - - // make sure to append the extra env so that they override existing env vars if set - extraEnv = append(GetDefaultExtraEnv(strictHostKeyChecking), extraEnv...) - - extraArgs := []string{} - if helper != "" { - extraArgs = append(extraArgs, "--config", "credential.helper="+helper) - } - - if gitInfo.Branch != "" { - extraArgs = append(extraArgs, "--branch", gitInfo.Branch) - } - - if err := cloner.Clone( - ctx, - gitInfo.Repository, - targetDir, - extraArgs, - extraEnv, - ); err != nil { - return err - } - - if gitInfo.PR != "" { - return checkoutPR(ctx, gitInfo, extraEnv, targetDir) - } - - if gitInfo.Commit != "" { - return checkoutCommit(ctx, gitInfo, extraEnv, targetDir) - } - - return nil -} - -func checkoutPR( - ctx context.Context, - gitInfo *GitInfo, - extraEnv []string, - targetDir string, -) error { - log.Debugf("Fetching pull request : %s", gitInfo.PR) - - prBranch := GetBranchNameForPR(gitInfo.PR) - - // Try to fetch the pull request by - // checking out the reference GitHub set up for it. Afterwards, switch to it. - // See [this doc](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally#modifying-an-inactive-pull-request-locally) - // Command args: `git fetch origin pull/996/head:PR996` - fetchArgs := []string{"fetch", "origin", gitInfo.PR + ":" + prBranch} - fetchCmd := CommandContext(ctx, extraEnv, fetchArgs...) - fetchCmd.Dir = targetDir - if err := fetchCmd.Run(); err != nil { - return fmt.Errorf("fetch pull request reference: %w", err) - } - - // git switch PR996 - switchArgs := []string{"switch", prBranch} - switchCmd := CommandContext(ctx, extraEnv, switchArgs...) - switchCmd.Dir = targetDir - if err := switchCmd.Run(); err != nil { - return fmt.Errorf("switch to branch: %w", err) - } - - return nil -} - -func checkoutCommit( - ctx context.Context, - gitInfo *GitInfo, - extraEnv []string, - targetDir string, -) error { - stdout := log.Writer(log.LevelInfo) - stderr := log.Writer(log.LevelError) - defer func() { _ = stdout.Close() }() - defer func() { _ = stderr.Close() }() - - args := []string{"reset", "--hard", gitInfo.Commit} - gitCommand := CommandContext(ctx, extraEnv, args...) - gitCommand.Dir = targetDir - gitCommand.Stdout = stdout - gitCommand.Stderr = stderr - if err := gitCommand.Run(); err != nil { - return fmt.Errorf("reset head to commit: %w", err) - } - - return nil -} diff --git a/pkg/git/install.go b/pkg/git/install.go deleted file mode 100644 index 00c5a3a73..000000000 --- a/pkg/git/install.go +++ /dev/null @@ -1,77 +0,0 @@ -package git - -import ( - "fmt" - "io" - "os/exec" - - "github.com/devsy-org/devsy/pkg/command" - "github.com/devsy-org/devsy/pkg/log" -) - -func InstallBinary() error { - writer := log.Writer(log.LevelInfo) - errwriter := log.Writer(log.LevelError) - defer func() { _ = writer.Close() }() - defer func() { _ = errwriter.Close() }() - - // try to install git via apt / apk - switch { - case command.Exists("apt"): - if err := installGitWithApt(writer, errwriter); err != nil { - return err - } - case command.Exists("apk"): - if err := installGitWithApk(writer, errwriter); err != nil { - return err - } - default: - // TODO: use golang git implementation - return fmt.Errorf("couldn't find a package manager to install git") - } - - // is git available now? - if !command.Exists("git") { - return fmt.Errorf("couldn't install git") - } - - log.Infof("installed git") - - return nil -} - -func installGitWithApt(writer, errwriter io.Writer) error { - log.Infof("Git command is missing, try to install git with apt") - - if err := runCmd(writer, errwriter, "apt", "update"); err != nil { - return fmt.Errorf("run apt update: %w", err) - } - - if err := runCmd(writer, errwriter, "apt", "-y", "install", "git"); err != nil { - return fmt.Errorf("run apt install git -y: %w", err) - } - - return nil -} - -func installGitWithApk(writer, errwriter io.Writer) error { - log.Infof("Git command is missing, try to install git with apk") - - if err := runCmd(writer, errwriter, "apk", "update"); err != nil { - return fmt.Errorf("run apk update: %w", err) - } - - if err := runCmd(writer, errwriter, "apk", "add", "git"); err != nil { - return fmt.Errorf("run apk add git: %w", err) - } - - return nil -} - -func runCmd(stdout, stderr io.Writer, name string, args ...string) error { - cmd := exec.Command(name, args...) // #nosec G204 -- args are internally constructed - cmd.Stdout = stdout - cmd.Stderr = stderr - - return cmd.Run() -} diff --git a/pkg/git/installer.go b/pkg/git/installer.go new file mode 100644 index 000000000..0abc77f59 --- /dev/null +++ b/pkg/git/installer.go @@ -0,0 +1,145 @@ +package git + +import ( + "context" + "errors" + "fmt" + + "github.com/devsy-org/devsy/pkg/command" + "github.com/devsy-org/devsy/pkg/log" +) + +// InstallBinary installs the git binary if it is not already available, using a +// system package manager. +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 + pkg string + release *releaseSource +} + +var ( + gitTool = tool{binary: binGit, pkg: binGit} + lfsTool = tool{binary: binGitLFS, pkg: binGitLFS, release: &gitLFSRelease} +) + +// installStrategy installs a package. +type installStrategy interface { + name() string + usable() bool + install(ctx context.Context, t tool) error +} + +// Installer installs git-related tools using an ordered list of strategies. +type Installer struct { + strategies []installStrategy +} + +// newInstaller builds an Installer with the default strategies. +func newInstaller(runner Runner) *Installer { + return &Installer{ + strategies: []installStrategy{ + &pkgManagerStrategy{ + manager: strategyApt, runner: runner, + installArgs: func(pkg string) [][]string { + return [][]string{{pkgUpdate}, {"-y", lfsInstall, pkg}} + }, + }, + &pkgManagerStrategy{ + manager: strategyApk, runner: runner, + installArgs: func(pkg string) [][]string { + return [][]string{{pkgUpdate}, {"add", pkg}} + }, + }, + &releaseStrategy{}, + }, + } +} + +// ensure installs the tool if it is not already available. Strategies are tried +// in order; a strategy that fails (e.g. apt present but no root/network) does +// not abort the sequence, so a later fallback like the GitHub release can still +// succeed. +func (i *Installer) ensure(ctx context.Context, t tool) error { + if command.Exists(t.binary) { + return nil + } + + tried := []string{} + var errs []error + for _, s := range i.strategies { + if !s.usable() { + continue + } + tried = append(tried, s.name()) + if err := s.install(ctx, t); err != nil { + errs = append(errs, fmt.Errorf("%s: %w", s.name(), err)) + continue + } + if command.Exists(t.binary) { + log.Infof("installed %s via %s", t.binary, s.name()) + return nil + } + errs = append( + errs, + fmt.Errorf("%s: reported success but %s is not on PATH", s.name(), t.binary), + ) + } + + if len(tried) == 0 { + return fmt.Errorf("no usable strategy to install %s", t.binary) + } + return fmt.Errorf("install %s (tried: %v): %w", t.binary, tried, errors.Join(errs...)) +} + +// pkgManagerStrategy installs via a system package manager (apt, apk). +type pkgManagerStrategy struct { + manager string + runner Runner + installArgs func(pkg string) [][]string +} + +func (s *pkgManagerStrategy) name() string { return s.manager } + +func (s *pkgManagerStrategy) usable() bool { return command.Exists(s.manager) } + +func (s *pkgManagerStrategy) install(ctx context.Context, t tool) error { + log.Infof("installing %s with %s", t.pkg, s.manager) + w := log.Writer(log.LevelInfo) + defer func() { _ = w.Close() }() + + for _, args := range s.installArgs(t.pkg) { + if _, err := s.runner.Run(ctx, RunOptions{ + Binary: s.manager, + Args: args, + Stdout: w, + Stderr: w, + }); err != nil { + return err + } + } + return nil +} + +// releaseStrategy installs a tool by downloading its GitHub release asset. +type releaseStrategy struct{} + +func (releaseStrategy) name() string { return strategyRelease } + +func (releaseStrategy) usable() bool { return true } + +func (releaseStrategy) install(ctx context.Context, t tool) error { + if t.release == nil { + return fmt.Errorf("%s has no release download available", t.binary) + } + return t.release.install(ctx, t.binary) +} diff --git a/pkg/git/installer_release.go b/pkg/git/installer_release.go new file mode 100644 index 000000000..515c9a176 --- /dev/null +++ b/pkg/git/installer_release.go @@ -0,0 +1,228 @@ +package git + +import ( + "bytes" + "context" + "crypto/sha256" + "encoding/hex" + "fmt" + "io" + "os" + "path/filepath" + "runtime" + + "github.com/devsy-org/devsy/pkg/config" + "github.com/devsy-org/devsy/pkg/download" + "github.com/devsy-org/devsy/pkg/extract" + "github.com/devsy-org/devsy/pkg/log" +) + +// releaseSource describes how to fetch a tool binary from a GitHub release. +type releaseSource struct { + // version is the release tag without the leading "v" (e.g. "3.5.1"). + version string + // assetName builds the release asset filename for the given GOOS/GOARCH. + assetName func(goos, goarch, version string) (string, error) + // downloadURL builds the download URL for a resolved asset filename. + downloadURL func(version, asset string) string + // checksums maps asset filename to its expected lowercase-hex SHA-256. The + // download is rejected if its digest is absent or does not match, pinning + // the exact bytes we install (defense against a compromised release host). + checksums map[string]string + // binaryInArchive is the name of the executable within the extracted + // archive (it may sit under a versioned subdirectory). + binaryInArchive string + // installDir is the directory the binary is placed into; it must be on PATH. + // Empty defaults to the platform's SystemBinDir (see pkg/config). + installDir string +} + +// gitLFSRelease pins a git-lfs release used when no package manager is present. +// git-lfs publishes per-OS/arch archives (.tar.gz on linux, .zip on darwin and +// windows) each containing a single git-lfs binary (git-lfs.exe on windows). +var gitLFSRelease = releaseSource{ + version: "3.5.1", + binaryInArchive: binGitLFS, + assetName: func(goos, goarch, version string) (string, error) { + ext, ok := map[string]string{ + osLinux: "tar.gz", + osDarwin: "zip", + osWindows: "zip", + }[goos] + if !ok { + return "", fmt.Errorf("unsupported OS %q for git-lfs release download", goos) + } + switch goarch { + case "amd64", "arm64": + default: + return "", fmt.Errorf( + "unsupported architecture %q for git-lfs release download", + goarch, + ) + } + return fmt.Sprintf("git-lfs-%s-%s-v%s.%s", goos, goarch, version, ext), nil + }, + downloadURL: func(version, asset string) string { + return fmt.Sprintf( + "https://github.com/git-lfs/git-lfs/releases/download/v%s/%s", + version, asset, + ) + }, + // SHA-256 digests from git-lfs v3.5.1 sha256sums.asc for the OS/arch pairs + // assetName can resolve. Update alongside version. + checksums: map[string]string{ + "git-lfs-linux-amd64-v3.5.1.tar.gz": "6f28eb19faa7a968882dca190d92adc82493378b933958d67ceaeb9ebe4d731e", + "git-lfs-linux-arm64-v3.5.1.tar.gz": "4f8700aacaa0fd26ae5300fb0996aed14d1fd0ce1a63eb690629c132ff5163a9", + "git-lfs-darwin-amd64-v3.5.1.zip": "23f6c768e22a33dcbb57d6cb67d318dc0edc2b16ac04b15faa803a74a31e8c42", + "git-lfs-darwin-arm64-v3.5.1.zip": "1570833e5011290dff12a18416580bfed576bc797b7b521122916e09adf4622d", + "git-lfs-windows-amd64-v3.5.1.zip": "94435072f6b3a6f9064b277760c8340e432b5ede0db8205d369468b9be52c6b6", + "git-lfs-windows-arm64-v3.5.1.zip": "54fb4a04a5597ebdae83b2873adb363c2e2b7022b8b2ce813cc0f198c12f8a61", + }, +} + +// install downloads the release asset for the current platform, extracts the +// binary, and places it on PATH under installDir. +func (s *releaseSource) install(ctx context.Context, binary string) error { + asset, err := s.assetName(runtime.GOOS, runtime.GOARCH, s.version) + if err != nil { + return err + } + wantSum, ok := s.checksums[asset] + if !ok { + return fmt.Errorf("no pinned checksum for %s release asset %q", binary, asset) + } + + installDir := s.installDir + if installDir == "" { + installDir, err = config.DefaultPathManager().SystemBinDir() + if err != nil { + return fmt.Errorf("resolve install dir for %s: %w", binary, err) + } + } + + req := fetchRequest{ + binary: binary, + url: s.downloadURL(s.version, asset), + wantSum: wantSum, + // git-lfs on windows ships as git-lfs.exe inside the archive. + execName: executableName(s.binaryInArchive, runtime.GOOS), + } + src, cleanup, err := s.fetchBinary(ctx, req) + if err != nil { + return err + } + defer cleanup() + + if err := os.MkdirAll(installDir, 0o750); err != nil { + return fmt.Errorf("create install dir %q: %w", installDir, err) + } + dst := filepath.Join(installDir, req.execName) + if err := moveExecutable(src, dst); err != nil { + return fmt.Errorf("install %s to %q: %w", binary, dst, err) + } + return nil +} + +// fetchRequest describes a single release-asset download and extraction. +type fetchRequest struct { + binary string // human-facing tool name for logs/errors + url string // resolved asset download URL + wantSum string // pinned lowercase-hex SHA-256 of the asset + execName string // executable filename to locate inside the archive +} + +// executableName returns the platform-specific executable filename, appending +// the .exe extension on windows. +func executableName(base, goos string) string { + if goos == osWindows { + return base + ".exe" + } + return base +} + +// fetchBinary downloads the release asset, verifies it against the pinned +// SHA-256, then extracts it into a temp dir and returns the path to the +// extracted binary plus a cleanup func for the temp dir. +func (s *releaseSource) fetchBinary( + ctx context.Context, + req fetchRequest, +) (path string, cleanup func(), err error) { + log.Infof("downloading %s %s release from %s", req.binary, s.version, req.url) + body, err := download.File(ctx, req.url) + if err != nil { + return "", nil, fmt.Errorf("download %s release: %w", req.binary, err) + } + defer func() { _ = body.Close() }() + + archive, err := readVerified(body, req.wantSum) + if err != nil { + return "", nil, fmt.Errorf("verify %s release: %w", req.binary, err) + } + + tmpDir, err := os.MkdirTemp("", "git-lfs-release-") + if err != nil { + return "", nil, err + } + cleanup = func() { _ = os.RemoveAll(tmpDir) } + + if err := extract.Extract(archive, tmpDir); err != nil { + cleanup() + return "", nil, fmt.Errorf("extract %s release: %w", req.binary, err) + } + + src, err := findBinary(tmpDir, req.execName) + if err != nil { + cleanup() + return "", nil, err + } + return src, cleanup, nil +} + +// readVerified reads r fully and returns a reader over its bytes only if the +// content's SHA-256 matches wantSum (lowercase hex). +func readVerified(r io.Reader, wantSum string) (io.Reader, error) { + data, err := io.ReadAll(r) + if err != nil { + return nil, err + } + sum := sha256.Sum256(data) + if got := hex.EncodeToString(sum[:]); got != wantSum { + return nil, fmt.Errorf("checksum mismatch: got %s, want %s", got, wantSum) + } + return bytes.NewReader(data), nil +} + +// findBinary locates a named executable anywhere under root (git-lfs tarballs +// place it in a versioned subdirectory). +func findBinary(root, name string) (string, error) { + var found string + err := filepath.WalkDir(root, func(path string, d os.DirEntry, err error) error { + if err != nil { + return nil + } + if !d.IsDir() && d.Name() == name { + found = path + return filepath.SkipAll + } + return nil + }) + if err != nil { + return "", err + } + if found == "" { + return "", fmt.Errorf("binary %q not found in downloaded archive", name) + } + return found, nil +} + +// moveExecutable copies src to dst with executable permissions. Both paths are +// internally constructed (temp extraction dir and the configured install dir), +// not user input. +func moveExecutable(src, dst string) error { + data, err := os.ReadFile(src) // #nosec G304 -- src is within our temp extraction dir + if err != nil { + return err + } + // #nosec G306,G703 -- dst is internally constructed; an executable must be world-executable + return os.WriteFile(dst, data, 0o755) +} diff --git a/pkg/git/installer_test.go b/pkg/git/installer_test.go new file mode 100644 index 000000000..6ea52a548 --- /dev/null +++ b/pkg/git/installer_test.go @@ -0,0 +1,165 @@ +package git + +import ( + "context" + "fmt" + "strings" + "testing" + + "gotest.tools/assert" +) + +func TestPkgManagerStrategyAptArgs(t *testing.T) { + fake := &fakeRunner{} + s := &pkgManagerStrategy{ + manager: strategyApt, + runner: fake, + installArgs: func(pkg string) [][]string { + return [][]string{{pkgUpdate}, {"-y", "install", pkg}} + }, + } + + assert.NilError(t, s.install(context.Background(), lfsTool)) + assert.Equal(t, 2, len(fake.calls)) + assert.Equal(t, "apt", fake.calls[0].Binary) + assert.DeepEqual(t, []string{pkgUpdate}, fake.calls[0].Args) + assert.DeepEqual(t, []string{"-y", "install", "git-lfs"}, fake.calls[1].Args) +} + +func TestPkgManagerStrategyStopsOnError(t *testing.T) { + fake := &fakeRunner{err: fmt.Errorf("boom")} + s := &pkgManagerStrategy{ + manager: strategyApk, + runner: fake, + installArgs: func(pkg string) [][]string { + return [][]string{{pkgUpdate}, {"add", pkg}} + }, + } + + err := s.install(context.Background(), lfsTool) + assert.Assert(t, err != nil) + // The second command must not run after the first fails. + assert.Equal(t, 1, len(fake.calls)) +} + +func TestReleaseStrategyRequiresReleaseSource(t *testing.T) { + // git has no release source; the release strategy must refuse it. + err := releaseStrategy{}.install(context.Background(), gitTool) + assert.Assert(t, err != nil) +} + +// fakeStrategy is a test installStrategy with configurable behavior. +type fakeStrategy struct { + label string + isUsable bool + installer func() error + installed *bool // set true by install to simulate the binary appearing +} + +func (f *fakeStrategy) name() string { return f.label } +func (f *fakeStrategy) usable() bool { return f.isUsable } +func (f *fakeStrategy) install(context.Context, tool) error { + err := f.installer() + if err == nil && f.installed != nil { + *f.installed = true + } + return err +} + +func TestEnsureFallsThroughOnInstallFailure(t *testing.T) { + // A usable strategy that fails must not abort the sequence; a later + // strategy should still get a chance and can succeed. + firstTried, secondTried := false, false + installed := false + + inst := &Installer{strategies: []installStrategy{ + &fakeStrategy{ + label: strategyApt, + isUsable: true, + installer: func() error { + firstTried = true + return fmt.Errorf("no root") + }, + }, + &fakeStrategy{ + label: strategyRelease, + isUsable: true, + installed: &installed, + installer: func() error { + secondTried = true + return nil + }, + }, + }} + + // command.Exists reports the real git-lfs; guard the assertion on the + // simulated flag instead, which the second strategy sets. + _ = inst.ensure(context.Background(), tool{binary: "definitely-not-a-real-binary-xyz"}) + assert.Assert(t, firstTried) + assert.Assert(t, secondTried) + assert.Assert(t, installed) +} + +func TestEnsureAggregatesErrorsWhenAllFail(t *testing.T) { + inst := &Installer{strategies: []installStrategy{ + &fakeStrategy{label: strategyApt, isUsable: true, installer: func() error { + return fmt.Errorf("apt boom") + }}, + &fakeStrategy{label: strategyRelease, isUsable: true, installer: func() error { + return fmt.Errorf("release boom") + }}, + }} + + err := inst.ensure(context.Background(), tool{binary: "definitely-not-a-real-binary-xyz"}) + assert.Assert(t, err != nil) + // Both strategy failures must surface, not just the first. + assert.Assert(t, strings.Contains(err.Error(), "apt boom")) + assert.Assert(t, strings.Contains(err.Error(), "release boom")) +} + +func TestGitLFSReleaseAsset(t *testing.T) { + // linux uses .tar.gz; darwin and windows publish .zip. + cases := map[string]string{ + "linux/amd64": "git-lfs-linux-amd64-v3.5.1.tar.gz", + "darwin/arm64": "git-lfs-darwin-arm64-v3.5.1.zip", + "windows/amd64": "git-lfs-windows-amd64-v3.5.1.zip", + } + for platform, want := range cases { + goos, goarch, _ := strings.Cut(platform, "/") + asset, err := gitLFSRelease.assetName(goos, goarch, "3.5.1") + assert.NilError(t, err) + assert.Equal(t, want, asset) + assert.Equal( + t, + "https://github.com/git-lfs/git-lfs/releases/download/v3.5.1/"+want, + gitLFSRelease.downloadURL("3.5.1", asset), + ) + } +} + +func TestGitLFSReleaseAssetUnsupportedPlatform(t *testing.T) { + _, err := gitLFSRelease.assetName("plan9", "amd64", "3.5.1") + assert.Assert(t, err != nil) + + _, err = gitLFSRelease.assetName("linux", "mips", "3.5.1") + assert.Assert(t, err != nil) +} + +func TestGitLFSReleaseAllAssetsHaveChecksums(t *testing.T) { + // Every asset assetName can resolve must have a pinned checksum, or install + // fails closed at runtime. + for _, goos := range []string{osLinux, osDarwin, osWindows} { + for _, goarch := range []string{"amd64", "arm64"} { + asset, err := gitLFSRelease.assetName(goos, goarch, gitLFSRelease.version) + assert.NilError(t, err) + _, ok := gitLFSRelease.checksums[asset] + assert.Assert(t, ok, "missing checksum for %s", asset) + } + } +} + +func TestExecutableName(t *testing.T) { + assert.Equal(t, "git-lfs", executableName("git-lfs", osLinux)) + assert.Equal(t, "git-lfs", executableName("git-lfs", osDarwin)) + assert.Equal(t, "git-lfs.exe", executableName("git-lfs", osWindows)) +} diff --git a/pkg/git/lfs.go b/pkg/git/lfs.go new file mode 100644 index 000000000..8c9a8f64b --- /dev/null +++ b/pkg/git/lfs.go @@ -0,0 +1,114 @@ +package git + +import ( + "bytes" + "context" + "fmt" + "io/fs" + "os" + "path/filepath" + "strings" + + "github.com/devsy-org/devsy/pkg/command" + "github.com/devsy-org/devsy/pkg/log" +) + +// lfsFilterMarker is the .gitattributes token that marks a path as LFS-tracked. +var lfsFilterMarker = []byte("filter=lfs") + +// smudgeSkippedForClone reports whether the clone should set GIT_LFS_SKIP_SMUDGE. +func smudgeSkippedForClone(mode LFSMode) bool { + if mode == LFSFull && !command.Exists(binGitLFS) { + log.Info("git-lfs not found, skipping LFS smudge; LFS files will be pointer stubs") + } + return true +} + +// SetupLFS configures Git LFS in the repository. +func (r *Repo) SetupLFS(ctx context.Context, mode LFSMode) { + if mode == LFSSkip { + return + } + if !repoUsesLFS(r.path) { + return + } + + 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 + } + } + + if err := r.lfs(ctx, "install", "--local"); err != nil { + log.Warnf("git-lfs install failed, LFS files may be pointer stubs: %v", err) + return + } + + if mode == LFSSetupOnly { + return + } + + if err := r.lfs(ctx, "pull"); err != nil { + log.Warnf("git-lfs pull failed, LFS files may be pointer stubs: %v", err) + } +} + +// 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 { + res, err := r.run(ctx, append([]string{"lfs"}, args...)...) + if err != nil { + return fmt.Errorf("%w: %s%s", err, res.Stdout, res.Stderr) + } + return nil +} + +// lfsDetectMaxDepth bounds how deep repoUsesLFS descends looking for .gitattributes. +const lfsDetectMaxDepth = 6 + +// repoUsesLFS reports whether any .gitattributes file within lfsDetectMaxDepth +// levels of the worktree declares an LFS filter. +func repoUsesLFS(root string) bool { + found := false + _ = filepath.WalkDir(root, func(path string, d fs.DirEntry, err error) error { + if err != nil { + return nil + } + if d.IsDir() { + if d.Name() == ".git" { + return filepath.SkipDir + } + if dirDepth(root, path) > lfsDetectMaxDepth { + return filepath.SkipDir + } + return nil + } + if d.Name() != gitAttributesFile { + return nil + } + // #nosec G304,G122 -- path is within the freshly cloned worktree we control + data, err := os.ReadFile(path) + if err != nil { + return nil + } + if bytes.Contains(data, lfsFilterMarker) { + found = true + return filepath.SkipAll + } + return nil + }) + return found +} + +// dirDepth returns how many directory levels dir is below root (root itself is 0). +func dirDepth(root, dir string) int { + rel, err := filepath.Rel(root, dir) + if err != nil || rel == "." { + return 0 + } + return strings.Count(rel, string(filepath.Separator)) + 1 +} diff --git a/pkg/git/lfs_test.go b/pkg/git/lfs_test.go new file mode 100644 index 000000000..14f4bfde8 --- /dev/null +++ b/pkg/git/lfs_test.go @@ -0,0 +1,212 @@ +package git + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/devsy-org/devsy/pkg/command" + "gotest.tools/assert" +) + +func TestLFSModeResolution(t *testing.T) { + cases := []struct { + name string + opts []Option + want LFSMode + }{ + {"default is full", nil, LFSFull}, + {"explicit full", []Option{WithLFSMode(LFSFull)}, LFSFull}, + {"setup only", []Option{WithLFSMode(LFSSetupOnly)}, LFSSetupOnly}, + {"skip", []Option{WithLFSMode(LFSSkip)}, LFSSkip}, + {"WithSkipLFS maps to skip", []Option{WithSkipLFS()}, LFSSkip}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := newCloneConfig(tc.opts...).lfsMode; got != tc.want { + t.Errorf("lfsMode = %v, want %v", got, tc.want) + } + }) + } +} + +func TestSmudgeAlwaysSkippedForClone(t *testing.T) { + // The smudge filter is skipped at clone time for every mode; hydration is + // done explicitly afterward (or not at all). + for _, mode := range []LFSMode{LFSFull, LFSSetupOnly, LFSSkip} { + if !smudgeSkippedForClone(mode) { + t.Errorf("smudgeSkippedForClone(%v) = false, want true", mode) + } + } +} + +// lfsSubcommands returns the `lfs ...` invocations a fakeRunner recorded. +func lfsSubcommands(fake *fakeRunner) [][]string { + var out [][]string + for _, c := range fake.calls { + if len(c.Args) > 0 && c.Args[0] == "lfs" { + out = append(out, c.Args) + } + } + return out +} + +// newLFSRepo returns a temp worktree that declares LFS (so SetupLFS proceeds +// past detection) and a fakeRunner capturing its git invocations. +func newLFSRepo(t *testing.T) (string, *fakeRunner) { + t.Helper() + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, gitAttributesFile), + []byte("*.bin filter=lfs diff=lfs merge=lfs -text\n"), 0o600); err != nil { + t.Fatal(err) + } + return dir, &fakeRunner{} +} + +func TestSetupLFSModeCommands(t *testing.T) { + if !command.Exists(binGitLFS) { + t.Skip("git-lfs not installed") + } + + cases := []struct { + name string + mode LFSMode + want []string // expected joined `lfs ...` subcommands + }{ + {"full installs and pulls", LFSFull, []string{"lfs install --local", "lfs pull"}}, + {"setup-only installs, no pull", LFSSetupOnly, []string{"lfs install --local"}}, + {"skip does nothing", LFSSkip, nil}, + } + + 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) + + var got []string + for _, args := range lfsSubcommands(fake) { + got = append(got, strings.Join(args, " ")) + } + assert.DeepEqual(t, tc.want, got) + }) + } +} + +func TestRepoUsesLFS(t *testing.T) { + cases := []struct { + name string + files map[string]string + expected bool + }{ + { + name: "no gitattributes", + files: map[string]string{"README.md": "hi"}, + expected: false, + }, + { + name: "gitattributes without lfs", + files: map[string]string{gitAttributesFile: "*.txt text\n"}, + expected: false, + }, + { + name: "root gitattributes with lfs", + files: map[string]string{ + gitAttributesFile: "*.bin filter=lfs diff=lfs merge=lfs -text\n", + }, + expected: true, + }, + { + name: "nested gitattributes with lfs", + files: map[string]string{ + "services/a/.gitattributes": "data/x.jsonl filter=lfs diff=lfs merge=lfs -text\n", + }, + expected: true, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + dir := t.TempDir() + for rel, content := range tc.files { + p := filepath.Join(dir, rel) + if err := os.MkdirAll(filepath.Dir(p), 0o750); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(p, []byte(content), 0o600); err != nil { + t.Fatal(err) + } + } + if got := repoUsesLFS(dir); got != tc.expected { + t.Errorf("repoUsesLFS = %v, want %v", got, tc.expected) + } + }) + } +} + +func TestRepoUsesLFSSkipsGitDir(t *testing.T) { + dir := t.TempDir() + gitDir := filepath.Join(dir, ".git") + if err := os.MkdirAll(gitDir, 0o750); err != nil { + t.Fatal(err) + } + // A .gitattributes inside .git must not trigger detection. + if err := os.WriteFile( + filepath.Join(gitDir, gitAttributesFile), + []byte("x filter=lfs\n"), + 0o600, + ); err != nil { + t.Fatal(err) + } + if repoUsesLFS(dir) { + t.Error("repoUsesLFS should ignore .git contents") + } +} + +func TestRepoUsesLFSRespectsMaxDepth(t *testing.T) { + // A .gitattributes at exactly the max depth is found; one level deeper is not. + atLimit := strings.Repeat("d/", lfsDetectMaxDepth) // lfsDetectMaxDepth dirs deep + tooDeep := strings.Repeat("d/", lfsDetectMaxDepth+1) // one level beyond the bound + content := "x filter=lfs diff=lfs merge=lfs -text\n" + + cases := []struct { + name string + rel string + expected bool + }{ + {"at max depth", filepath.Join(filepath.FromSlash(atLimit), gitAttributesFile), true}, + {"beyond max depth", filepath.Join(filepath.FromSlash(tooDeep), gitAttributesFile), false}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + dir := t.TempDir() + p := filepath.Join(dir, tc.rel) + if err := os.MkdirAll(filepath.Dir(p), 0o750); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(p, []byte(content), 0o600); err != nil { + t.Fatal(err) + } + if got := repoUsesLFS(dir); got != tc.expected { + t.Errorf("repoUsesLFS = %v, want %v", got, tc.expected) + } + }) + } +} + +func TestDirDepth(t *testing.T) { + root := filepath.FromSlash("/tmp/repo") + cases := map[string]int{ + "/tmp/repo": 0, + "/tmp/repo/a": 1, + "/tmp/repo/a/b": 2, + "/tmp/repo/a/b/c/d": 4, + } + for dir, want := range cases { + if got := dirDepth(root, filepath.FromSlash(dir)); got != want { + t.Errorf("dirDepth(%q) = %d, want %d", dir, got, want) + } + } +} diff --git a/pkg/git/repo.go b/pkg/git/repo.go new file mode 100644 index 000000000..dd7232694 --- /dev/null +++ b/pkg/git/repo.go @@ -0,0 +1,229 @@ +package git + +import ( + "context" + "fmt" + "strings" + + "github.com/devsy-org/devsy/pkg/log" +) + +// Repo is a handle to a git repository at a filesystem path. +type Repo struct { + path string + env []string + runner Runner +} + +// RepoOption configures a Repo. +type RepoOption func(*Repo) + +// WithEnv appends extra environment entries applied to every operation. +func WithEnv(env []string) RepoOption { + return func(r *Repo) { + r.env = append(r.env, env...) + } +} + +// WithStrictHostKeyChecking appends the default SSH/terminal environment honoring the given policy. +func WithStrictHostKeyChecking(strict bool) RepoOption { + return WithEnv(GetDefaultExtraEnv(strict)) +} + +// WithRunner injects a command Runner, primarily for testing. +func WithRunner(runner Runner) RepoOption { + return func(r *Repo) { + r.runner = runner + } +} + +// At returns a Repo rooted at path. +func At(path string, opts ...RepoOption) *Repo { + r := &Repo{path: path, runner: defaultRunner} + for _, opt := range opts { + opt(r) + } + if r.runner == nil { + r.runner = defaultRunner + } + return r +} + +// Path returns the repository's filesystem path. +func (r *Repo) Path() string { + return r.path +} + +// ResetMode selects how Reset updates the index and working tree. +type ResetMode string + +const ( + ResetSoft ResetMode = "--soft" + ResetMixed ResetMode = "--mixed" + ResetHard ResetMode = "--hard" +) + +// Fetch fetches the given refspec from origin. +func (r *Repo) Fetch(ctx context.Context, refspec string) error { + if err := r.runLogged(ctx, "fetch", "origin", refspec); err != nil { + return fmt.Errorf("fetch %q: %w", refspec, err) + } + return nil +} + +// Switch switches the working tree to an existing branch. +func (r *Repo) Switch(ctx context.Context, branch string) error { + if _, err := r.run(ctx, "switch", branch); err != nil { + return fmt.Errorf("switch to branch %q: %w", branch, err) + } + return nil +} + +// CheckoutPR fetches a pull request head reference into a local branch and +// switches to it. prRef is a reference like "pull/996/head"; the local branch +// is derived via GetBranchNameForPR (e.g. "PR996"). This mirrors GitHub's +// documented "checking out pull requests locally" flow. +func (r *Repo) CheckoutPR(ctx context.Context, prRef string) error { + log.Debugf("Fetching pull request : %s", prRef) + + prBranch := GetBranchNameForPR(prRef) + if err := r.Fetch(ctx, prRef+":"+prBranch); err != nil { + return err + } + return r.Switch(ctx, prBranch) +} + +// Reset moves HEAD to commit using the given mode. +func (r *Repo) Reset(ctx context.Context, commit string, mode ResetMode) error { + if err := r.runLogged(ctx, "reset", string(mode), commit); err != nil { + return fmt.Errorf("reset head to %q: %w", commit, err) + } + return nil +} + +// LsRemote reports whether the remote repository is reachable. +func (r *Repo) LsRemote(ctx context.Context, repository string) error { + if _, err := r.run(ctx, "ls-remote", "--quiet", repository); err != nil { + return fmt.Errorf("ls-remote %q: %w", repository, err) + } + return nil +} + +// LsTree lists the files tracked at ref, recursively, as repo-relative paths. +func (r *Repo) LsTree(ctx context.Context, ref string) ([]string, error) { + res, err := r.run(ctx, "ls-tree", "-r", "--full-name", "--name-only", ref) + if err != nil { + return nil, fmt.Errorf("ls-tree %q: %w", ref, err) + } + return splitLines(res.Stdout), nil +} + +// Clone clones repository into the repo's path using the given options. +func (r *Repo) Clone(ctx context.Context, repository string, options ...Option) error { + return r.cloneWith(ctx, repository, newCloneConfig(options...)) +} + +// CloneFromInfo clones the repository described by gitInfo into the repo's path, +// using the given credential helper and options. +func (r *Repo) CloneFromInfo( + ctx context.Context, + gitInfo *GitInfo, + helper string, + options ...Option, +) error { + options = append( + []Option{WithBranch(gitInfo.Branch), WithCredentialHelper(helper)}, + options..., + ) + c := newCloneConfig(options...) + + if err := r.cloneWith(ctx, gitInfo.Repository, c); err != nil { + return err + } + + switch { + case gitInfo.PR != "": + if err := r.CheckoutPR(ctx, gitInfo.PR); err != nil { + return err + } + case gitInfo.Commit != "": + if err := r.Reset(ctx, gitInfo.Commit, ResetHard); err != nil { + return err + } + } + + // Bare clones have no worktree to hydrate. + if c.strategy != BareCloneStrategy { + r.SetupLFS(ctx, c.lfsMode) + } + return nil +} + +// CredentialFill runs `git credential fill`, feeding request on stdin and +// returning git's response. Used to resolve credentials via configured helpers. +func (r *Repo) CredentialFill(ctx context.Context, request string) (string, error) { + res, err := r.runner.Run(ctx, RunOptions{ + Dir: r.path, + Env: r.env, + Args: []string{"credential", "fill"}, + Stdin: strings.NewReader(request), + }) + if err != nil { + return "", fmt.Errorf("git credential fill: %w", err) + } + return string(res.Stdout), nil +} + +// run executes a git subcommand in the repository, capturing its output. +func (r *Repo) run(ctx context.Context, args ...string) (RunResult, error) { + return r.runner.Run(ctx, RunOptions{ + Dir: r.path, + Env: r.env, + Args: args, + }) +} + +// runLogged executes a git subcommand streaming its output to the logs. +func (r *Repo) runLogged(ctx context.Context, args ...string) error { + w := log.Writer(log.LevelInfo) + defer func() { _ = w.Close() }() + + _, err := r.runner.Run(ctx, RunOptions{ + Dir: r.path, + Env: r.env, + Args: args, + Stdout: w, + Stderr: w, + }) + return err +} + +// cloneWith executes a `git clone` with the given config, streaming output to the logs. +func (r *Repo) cloneWith(ctx context.Context, repository string, c cloneConfig) error { + w := log.Writer(log.LevelInfo) + defer func() { _ = w.Close() }() + + env := append([]string{}, r.env...) + if smudgeSkippedForClone(c.lfsMode) { + env = append(env, "GIT_LFS_SKIP_SMUDGE=1") + } + + _, err := r.runner.Run(ctx, RunOptions{ + Env: env, + Args: c.args(repository, r.path), + Stdout: w, + Stderr: w, + }) + return err +} + +// splitLines splits command output into non-empty trimmed lines. +func splitLines(b []byte) []string { + var lines []string + for line := range strings.SplitSeq(string(b), "\n") { + if trimmed := strings.TrimSpace(line); trimmed != "" { + lines = append(lines, trimmed) + } + } + return lines +} diff --git a/pkg/git/repo_test.go b/pkg/git/repo_test.go new file mode 100644 index 000000000..8abad6c15 --- /dev/null +++ b/pkg/git/repo_test.go @@ -0,0 +1,176 @@ +package git + +import ( + "context" + "testing" + + "gotest.tools/assert" + "gotest.tools/assert/cmp" +) + +// Shared literals used across Repo tests. +const ( + testRepoURL = "git@host:org/repo.git" + testPRRef = "pull/996/head" + testPRLocal = "PR996" + testPRRefSpec = testPRRef + ":" + testPRLocal + testCommit = "abc123" + subFetch = "fetch" + originRemote = "origin" + testTarget = "/tmp/target" +) + +// fakeRunner records the invocations it receives and returns canned output, +// letting git operations be tested without a real repository. +type fakeRunner struct { + calls []RunOptions + stdout []byte + err error +} + +func (f *fakeRunner) Run(_ context.Context, opts RunOptions) (RunResult, error) { + f.calls = append(f.calls, opts) + return RunResult{Stdout: f.stdout}, f.err +} + +func (f *fakeRunner) lastArgs() []string { + if len(f.calls) == 0 { + return nil + } + return f.calls[len(f.calls)-1].Args +} + +func TestRepoFetch(t *testing.T) { + fake := &fakeRunner{} + repo := At("/tmp/repo", WithRunner(fake)) + + assert.NilError(t, repo.Fetch(context.Background(), testPRRefSpec)) + assert.DeepEqual(t, []string{subFetch, originRemote, testPRRefSpec}, fake.lastArgs()) + assert.Equal(t, "/tmp/repo", fake.calls[0].Dir) +} + +func TestRepoReset(t *testing.T) { + fake := &fakeRunner{} + repo := At("/tmp/repo", WithRunner(fake)) + + assert.NilError(t, repo.Reset(context.Background(), testCommit, ResetHard)) + assert.DeepEqual(t, []string{"reset", "--hard", testCommit}, fake.lastArgs()) +} + +func TestRepoCheckoutPR(t *testing.T) { + fake := &fakeRunner{} + repo := At("/tmp/repo", WithRunner(fake)) + + assert.NilError(t, repo.CheckoutPR(context.Background(), testPRRef)) + // Expect a fetch of the PR ref into a local branch, then a switch to it. + assert.DeepEqual(t, []string{subFetch, originRemote, testPRRefSpec}, fake.calls[0].Args) + assert.DeepEqual(t, []string{"switch", testPRLocal}, fake.calls[1].Args) +} + +func TestRepoLsRemote(t *testing.T) { + fake := &fakeRunner{} + repo := At("", WithRunner(fake)) + + assert.NilError(t, repo.LsRemote(context.Background(), testRepoURL)) + assert.DeepEqual(t, []string{"ls-remote", "--quiet", testRepoURL}, fake.lastArgs()) +} + +func TestRepoLsTree(t *testing.T) { + fake := &fakeRunner{stdout: []byte("a/b.txt\n\n c.txt \nd/e/f.go\n")} + repo := At("/tmp/repo", WithRunner(fake)) + + paths, err := repo.LsTree(context.Background(), "HEAD") + assert.NilError(t, err) + assert.DeepEqual( + t, + []string{"ls-tree", "-r", "--full-name", "--name-only", "HEAD"}, + fake.lastArgs(), + ) + // Blank lines dropped, surrounding whitespace trimmed. + assert.DeepEqual(t, []string{"a/b.txt", "c.txt", "d/e/f.go"}, paths) +} + +func TestRepoCloneArgsThroughRunner(t *testing.T) { + fake := &fakeRunner{} + repo := At(testTarget, WithRunner(fake)) + + err := repo.Clone(context.Background(), testRepoURL, + WithCloneStrategy(ShallowCloneStrategy), + WithBranch(testBranch), + ) + assert.NilError(t, err) + assert.DeepEqual(t, []string{ + subClone, "--depth=1", flagBranch, testBranch, + testRepoURL, testTarget, flagProgress, + }, fake.lastArgs()) +} + +func TestRepoEnvThreadedToRunner(t *testing.T) { + fake := &fakeRunner{} + repo := At("/tmp/repo", WithRunner(fake), WithEnv([]string{"GIT_TERMINAL_PROMPT=0"})) + + assert.NilError(t, repo.Fetch(context.Background(), "main")) + assert.Assert(t, cmp.Contains(fake.calls[0].Env, "GIT_TERMINAL_PROMPT=0")) +} + +func TestRepoEnvOptionsCompose(t *testing.T) { + fake := &fakeRunner{} + // Both env-setting options must contribute; neither should overwrite the other. + repo := At("/tmp/repo", + WithRunner(fake), + WithStrictHostKeyChecking(false), + WithEnv([]string{"CUSTOM=1"}), + ) + + assert.NilError(t, repo.Fetch(context.Background(), "main")) + env := fake.calls[0].Env + assert.Assert(t, cmp.Contains(env, "GIT_TERMINAL_PROMPT=0")) // from strict-host-key option + assert.Assert(t, cmp.Contains(env, "CUSTOM=1")) // from WithEnv +} + +func TestRepoCloneFromInfoBranch(t *testing.T) { + fake := &fakeRunner{} + repo := At(testTarget, WithRunner(fake)) + info := &GitInfo{Repository: testRepoURL, Branch: testBranch} + + assert.NilError(t, repo.CloneFromInfo(context.Background(), info, "")) + // Branch becomes a clone flag; no separate checkout call. + assert.DeepEqual(t, []string{ + subClone, flagBranch, testBranch, + testRepoURL, testTarget, flagProgress, + }, fake.calls[0].Args) +} + +func TestRepoCloneFromInfoCommit(t *testing.T) { + fake := &fakeRunner{} + repo := At(testTarget, WithRunner(fake)) + info := &GitInfo{Repository: testRepoURL, Commit: testCommit} + + assert.NilError(t, repo.CloneFromInfo(context.Background(), info, "")) + // clone, then hard reset to the commit. + assert.Equal(t, subClone, fake.calls[0].Args[0]) + assert.DeepEqual(t, []string{"reset", "--hard", testCommit}, fake.calls[1].Args) +} + +func TestRepoCloneFromInfoPR(t *testing.T) { + fake := &fakeRunner{} + repo := At(testTarget, WithRunner(fake)) + info := &GitInfo{Repository: testRepoURL, PR: testPRRef} + + assert.NilError(t, repo.CloneFromInfo(context.Background(), info, "")) + assert.Equal(t, subClone, fake.calls[0].Args[0]) + assert.DeepEqual(t, []string{subFetch, originRemote, testPRRefSpec}, fake.calls[1].Args) + assert.DeepEqual(t, []string{"switch", testPRLocal}, fake.calls[2].Args) +} + +func TestRepoCloneFromInfoHelper(t *testing.T) { + fake := &fakeRunner{} + repo := At(testTarget, WithRunner(fake)) + info := &GitInfo{Repository: "https://host/org/repo.git"} + + assert.NilError(t, repo.CloneFromInfo(context.Background(), info, "store")) + assert.DeepEqual(t, []string{ + subClone, flagConfig, "credential.helper=store", + "https://host/org/repo.git", testTarget, flagProgress, + }, fake.calls[0].Args) +} diff --git a/pkg/git/runner.go b/pkg/git/runner.go new file mode 100644 index 000000000..691856a24 --- /dev/null +++ b/pkg/git/runner.go @@ -0,0 +1,122 @@ +package git + +import ( + "bytes" + "context" + "errors" + "fmt" + "io" + "os" + "os/exec" + "strings" + + "github.com/devsy-org/devsy/pkg/command" +) + +// ErrGitNotFound is returned when the git binary is not available on PATH. +var ErrGitNotFound = errors.New("git binary not found in PATH") + +// CommandError describes a failed git invocation. +type CommandError struct { + Args []string + ExitCode int + Stderr string + Err error +} + +func (e *CommandError) Error() string { + msg := fmt.Sprintf("git %s: %v", strings.Join(e.Args, " "), e.Err) + if e.Stderr != "" { + msg += ": " + e.Stderr + } + return msg +} + +func (e *CommandError) Unwrap() error { + return e.Err +} + +// Runner executes a single git subcommand. +type Runner interface { + Run(ctx context.Context, opts RunOptions) (RunResult, error) +} + +// RunOptions describes one command invocation. +type RunOptions struct { + Binary string + Dir string + Env []string + Args []string + Stdin io.Reader + Stdout io.Writer + Stderr io.Writer +} + +// RunResult holds captured output. +type RunResult struct { + Stdout []byte + Stderr []byte +} + +// execRunner runs git as a subprocess. +type execRunner struct{} + +var _ Runner = execRunner{} + +func (execRunner) Run(ctx context.Context, opts RunOptions) (RunResult, error) { + binary := opts.Binary + if binary == "" { + binary = binGit + } + if !command.Exists(binary) { + if binary == binGit { + return RunResult{}, &CommandError{Args: opts.Args, ExitCode: -1, Err: ErrGitNotFound} + } + return RunResult{}, &CommandError{ + Args: opts.Args, ExitCode: -1, + Err: fmt.Errorf("%q binary not found in PATH", binary), + } + } + + cmd := exec.CommandContext( + ctx, + binary, + opts.Args...) // #nosec G204 -- binary is an internal constant (git or a package manager) + cmd.Env = append(os.Environ(), opts.Env...) + cmd.Dir = opts.Dir + cmd.Stdin = opts.Stdin + + var outBuf, errBuf bytes.Buffer + if opts.Stdout != nil { + cmd.Stdout = opts.Stdout + } else { + cmd.Stdout = &outBuf + } + // Always capture stderr into errBuf so CommandError carries git's message, + // teeing to the caller's writer when one is provided. + if opts.Stderr != nil { + cmd.Stderr = io.MultiWriter(opts.Stderr, &errBuf) + } else { + cmd.Stderr = &errBuf + } + + err := cmd.Run() + result := RunResult{Stdout: outBuf.Bytes(), Stderr: errBuf.Bytes()} + if err != nil { + cmdErr := &CommandError{ + Args: opts.Args, + ExitCode: -1, + Stderr: strings.TrimSpace(errBuf.String()), + Err: err, + } + var exitErr *exec.ExitError + if errors.As(err, &exitErr) { + cmdErr.ExitCode = exitErr.ExitCode() + } + return result, cmdErr + } + return result, nil +} + +// defaultRunner is used by Repo when no runner is injected. +var defaultRunner Runner = execRunner{} diff --git a/pkg/git/runner_test.go b/pkg/git/runner_test.go new file mode 100644 index 000000000..4e9cefd24 --- /dev/null +++ b/pkg/git/runner_test.go @@ -0,0 +1,50 @@ +package git + +import ( + "context" + "errors" + "strings" + "testing" + + "gotest.tools/assert" +) + +func TestCommandErrorUnwrapAndMessage(t *testing.T) { + inner := errors.New("exit status 1") + e := &CommandError{ + Args: []string{subConfig, flagGet, "missing.key"}, + ExitCode: 1, + Stderr: "boom", + Err: inner, + } + assert.Assert(t, errors.Is(e, inner)) + assert.Assert(t, cmpContains(e.Error(), "config --get missing.key")) + assert.Assert(t, cmpContains(e.Error(), "boom")) +} + +func cmpContains(haystack, needle string) bool { + return strings.Contains(haystack, needle) +} + +// TestExecRunnerExitCode exercises the real git binary to confirm exit codes +// are captured into CommandError. `git config --get` of an absent key in a +// non-repo directory exits 1. +func TestExecRunnerExitCode(t *testing.T) { + runner := execRunner{} + if _, err := runner.Run( + context.Background(), + RunOptions{Args: []string{"--version"}}, + ); err != nil { + t.Skipf("git not available: %v", err) + } + + _, err := runner.Run(context.Background(), RunOptions{ + Dir: t.TempDir(), + Args: []string{subConfig, flagGet, "no.such.key.exists"}, + }) + assert.Assert(t, err != nil) + + var cmdErr *CommandError + assert.Assert(t, errors.As(err, &cmdErr)) + assert.Equal(t, 1, cmdErr.ExitCode) +} diff --git a/pkg/gitcredentials/gitcredentials.go b/pkg/gitcredentials/gitcredentials.go index 91aee2f5f..f5a8a2b3e 100644 --- a/pkg/gitcredentials/gitcredentials.go +++ b/pkg/gitcredentials/gitcredentials.go @@ -3,7 +3,7 @@ package gitcredentials import ( "context" "fmt" - netUrl "net/url" + netURL "net/url" "os" "os/exec" "path/filepath" @@ -13,9 +13,10 @@ import ( "github.com/devsy-org/devsy/pkg/config" "github.com/devsy-org/devsy/pkg/file" "github.com/devsy-org/devsy/pkg/git" - "github.com/devsy-org/devsy/pkg/scanner" ) +// GitCredentials is the git credential-helper record. Its wire form is git's +// documented key=value line protocol (see gitcredentials(7)). type GitCredentials struct { Protocol string `json:"protocol,omitempty"` URL string `json:"url,omitempty"` @@ -25,235 +26,247 @@ type GitCredentials struct { Password string `json:"password,omitempty"` } +// GitUser is a git identity (user.name / user.email). type GitUser struct { Name string `json:"name,omitempty"` Email string `json:"email,omitempty"` } -func ConfigureHelper(binaryPath, userName string, port int) error { - gitConfigPath, err := getGlobalGitConfigPath(userName) - if err != nil { - return err - } - - out, err := os.ReadFile(gitConfigPath) - if err != nil && !os.IsNotExist(err) { - return err +// Encode renders the credentials in git's key=value line format, terminated by +// a trailing newline. Only non-empty fields are emitted, matching git's helper +// protocol. +func (c GitCredentials) Encode() string { + var b strings.Builder + writeField := func(key, value string) { + if value != "" { + b.WriteString(key) + b.WriteByte('=') + b.WriteString(value) + b.WriteByte('\n') + } } + writeField("protocol", c.Protocol) + writeField("url", c.URL) + writeField("path", c.Path) + writeField("host", c.Host) + writeField("username", c.Username) + writeField("password", c.Password) + return b.String() +} - helper := fmt.Sprintf(`helper = !'%s' internal agent git-credentials`, binaryPath) - if port != -1 { - helper += fmt.Sprintf(` --port %d`, port) +// ParseCredentials parses git's key=value credential line format. It is the +// inverse of Encode. Unknown keys are ignored, matching git's tolerant parsing. +func ParseCredentials(raw string) GitCredentials { + var c GitCredentials + fields := map[string]*string{ + "protocol": &c.Protocol, + "url": &c.URL, + "path": &c.Path, + "host": &c.Host, + "username": &c.Username, + "password": &c.Password, } - - config := string(out) - if !strings.Contains(config, helper) { - content := removeCredentialHelper(config) + fmt.Sprintf(` -[credential] - %s -`, helper) - - err = os.WriteFile(gitConfigPath, []byte(content), 0o600) - if err != nil { - return fmt.Errorf("write git config: %w", err) + for line := range strings.SplitSeq(raw, "\n") { + key, value, ok := strings.Cut(strings.TrimSpace(line), "=") + if !ok { + continue } - - err = file.Chown(userName, gitConfigPath) - if err != nil { - return err + if field, known := fields[key]; known { + *field = value } } + return c +} - return nil +// credentialHelperValue builds the `credential.helper` value that runs devsy's +// own credential helper subcommand. +func credentialHelperValue(binaryPath string, port int) string { + helper := fmt.Sprintf(`!'%s' internal agent git-credentials`, binaryPath) + if port != -1 { + helper += fmt.Sprintf(` --port %d`, port) + } + return helper } -func RemoveHelper(userName string) error { +// ConfigureHelper installs credential helper into the user's global git +// config, replacing any previously configured helper. +func ConfigureHelper(ctx context.Context, binaryPath, userName string, port int) error { gitConfigPath, err := getGlobalGitConfigPath(userName) if err != nil { return err } - return RemoveHelperFromPath(gitConfigPath) -} + cfg := git.At("").Config() + scope := git.ScopeFile(gitConfigPath) + helper := credentialHelperValue(binaryPath, port) -func RemoveHelperFromPath(gitConfigPath string) error { - out, err := os.ReadFile(gitConfigPath) - if err != nil && !os.IsNotExist(err) { + existing, err := cfg.GetAll(ctx, "credential.helper", scope) + if err != nil { return err } + if len(existing) == 1 && existing[0] == helper { + return nil + } - if strings.Contains(string(out), "[credential]") { - err = os.WriteFile(gitConfigPath, []byte(removeCredentialHelper(string(out))), 0o600) - if err != nil { - return fmt.Errorf("write git config: %w", err) - } + if err := cfg.UnsetAll(ctx, "credential.helper", scope); err != nil { + return err + } + if err := cfg.Add(ctx, "credential.helper", helper, scope); err != nil { + return err } - return nil + return file.Chown(userName, gitConfigPath) } -func Parse(raw string) (*GitCredentials, error) { - credentials := &GitCredentials{} - lines := strings.SplitSeq(raw, "\n") - for line := range lines { - line = strings.TrimSpace(line) - splitted := strings.Split(line, "=") - if len(splitted) == 1 { - continue - } - - switch splitted[0] { - case "protocol": - credentials.Protocol = strings.Join(splitted[1:], "=") - case "host": - credentials.Host = strings.Join(splitted[1:], "=") - case "username": - credentials.Username = strings.Join(splitted[1:], "=") - case "password": - credentials.Password = strings.Join(splitted[1:], "=") - case "url": - credentials.URL = strings.Join(splitted[1:], "=") - case "path": - credentials.Path = strings.Join(splitted[1:], "=") - } +// RemoveHelper removes credential helper from the user's global config. +func RemoveHelper(ctx context.Context, userName string) error { + gitConfigPath, err := getGlobalGitConfigPath(userName) + if err != nil { + return err } + return RemoveHelperFromPath(ctx, gitConfigPath) +} - return credentials, nil +// RemoveHelperFromPath removes the credential.helper setting from the config at +// path, leaving any other credential.* settings intact. +func RemoveHelperFromPath(ctx context.Context, gitConfigPath string) error { + return git.At("").Config(). + UnsetAll(ctx, "credential.helper", git.ScopeFile(gitConfigPath)) } -func ToString(credentials *GitCredentials) string { - request := []string{} - if credentials.Protocol != "" { - request = append(request, "protocol="+credentials.Protocol) - } - if credentials.URL != "" { - request = append(request, "url="+credentials.URL) - } - if credentials.Path != "" { - request = append(request, "path="+credentials.Path) - } - if credentials.Host != "" { - request = append(request, "host="+credentials.Host) - } - if credentials.Username != "" { - request = append(request, "username="+credentials.Username) - } - if credentials.Password != "" { - request = append(request, "password="+credentials.Password) +// SetUser sets the global git identity for the given OS user. +func SetUser(ctx context.Context, userName string, user *GitUser) error { + scope, err := identityScope(userName) + if err != nil { + return err } - return strings.Join(request, "\n") + "\n" -} - -func SetUser(userName string, user *GitUser) error { - if user.Name != "" { - shellCommand := fmt.Sprintf(`git config --global user.name "%s"`, user.Name) - args := []string{} - if userName != "" { - args = append(args, "su", userName, "-c", shellCommand) - } else { - args = append(args, "sh", "-c", shellCommand) + cfg := git.At("").Config() + fields := []struct{ key, value string }{ + {"user.name", user.Name}, + {"user.email", user.Email}, + } + wrote := false + for _, f := range fields { + if f.value == "" { + continue } - - out, err := exec.Command(args[0], args[1:]...).CombinedOutput() - if err != nil { - return fmt.Errorf( - "set user.name %q: %w", - strings.Join(args, " "), - command.WrapCommandError(out, err), - ) + if err := cfg.Set(ctx, f.key, f.value, scope); err != nil { + return err } + wrote = true } - if user.Email != "" { - shellCommand := fmt.Sprintf(`git config --global user.email "%s"`, user.Email) - args := []string{} - if userName != "" { - args = append(args, "su", userName, "-c", shellCommand) - } else { - args = append(args, "sh", "-c", shellCommand) - } - out, err := exec.Command(args[0], args[1:]...).CombinedOutput() + // Only reassign ownership when we actually wrote the named user's config. + // Chowning unconditionally would fail on the not-yet-created file when + // neither field was provided. + if wrote && userName != "" { + path, err := getGlobalGitConfigPath(userName) if err != nil { - return fmt.Errorf( - "set user.email %q: %w", - strings.Join(args, " "), - command.WrapCommandError(out, err), - ) + return err } + return file.Chown(userName, path) } return nil } -func GetUser(userName string, workingDir string) (*GitUser, error) { - gitUser := &GitUser{} - - scopeArgs := []string{"config"} +// GetUser reads the git identity visible from workingDir, or the named user's +// global identity when workingDir is empty. +func GetUser(ctx context.Context, userName, workingDir string) (*GitUser, error) { + scope := git.ScopeGlobal + if workingDir != "" { + scope = git.ScopeDefault + } if userName != "" { - p, err := getGlobalGitConfigPath(userName) + path, err := getGlobalGitConfigPath(userName) if err != nil { - return gitUser, fmt.Errorf("get git global dir for %s: %w", userName, err) + return nil, fmt.Errorf("get git global dir for %s: %w", userName, err) } - scopeArgs = append(scopeArgs, "--file", p) - } else if workingDir == "" { - scopeArgs = append(scopeArgs, "--global") + scope = git.ScopeFile(path) } - nameCmd := exec.Command("git", append(scopeArgs, "user.name")...) // #nosec G204 - emailCmd := exec.Command("git", append(scopeArgs, "user.email")...) // #nosec G204 + cfg := git.At(workingDir).Config() + name, _ := cfg.Get(ctx, "user.name", scope) + email, _ := cfg.Get(ctx, "user.email", scope) + return &GitUser{Name: name, Email: email}, nil +} - if workingDir != "" { - nameCmd.Dir = workingDir - emailCmd.Dir = workingDir +// identityScope resolves the config scope for setting a user's identity: the +// named user's global config file, or the current user's global config. +func identityScope(userName string) (git.ConfigScope, error) { + if userName == "" { + return git.ScopeGlobal, nil } - - // we ignore the error here, because if email is empty we don't care - name, _ := nameCmd.Output() - gitUser.Name = strings.TrimSpace(string(name)) - - email, _ := emailCmd.Output() - gitUser.Email = strings.TrimSpace(string(email)) - - return gitUser, nil + path, err := getGlobalGitConfigPath(userName) + if err != nil { + return git.ConfigScope{}, err + } + return git.ScopeFile(path), nil } -func GetCredentials(requestObj *GitCredentials) (*GitCredentials, error) { - // run in git helper mode if we have a port - gitHelperPort := os.Getenv(config.EnvGitHelperPort) - if gitHelperPort != "" { - binaryPath, err := os.Executable() - if err != nil { - return nil, err - } +// GetCredentials resolves credentials for a request, using devsy's credential +// server when a helper port is configured, otherwise `git credential fill`. +func GetCredentials(ctx context.Context, request *GitCredentials) (*GitCredentials, error) { + if port := os.Getenv(config.EnvGitHelperPort); port != "" { + return credentialsViaServer(ctx, request, port) + } + return credentialsViaGit(ctx, request) +} - //nolint:gosec // binaryPath is from os.Executable(), not user input - c := exec.Command( - binaryPath, - "internal", - "agent", - "git-credentials", - "--port", - gitHelperPort, - "get", - ) - - c.Stdin = strings.NewReader(ToString(requestObj)) - stdout, err := c.Output() - if err != nil { - return nil, err - } +// credentialsViaServer asks devsy's own credential helper process (used inside +// agent containers, addressed by port). +func credentialsViaServer( + ctx context.Context, + request *GitCredentials, + port string, +) (*GitCredentials, error) { + binaryPath, err := os.Executable() + if err != nil { + return nil, err + } - return Parse(string(stdout)) + //nolint:gosec // binaryPath is from os.Executable(), not user input + cmd := exec.CommandContext( + ctx, + binaryPath, + "internal", "agent", "git-credentials", "--port", port, "get", + ) + cmd.Stdin = strings.NewReader(request.Encode()) + stdout, err := cmd.Output() + if err != nil { + return nil, err } + creds := ParseCredentials(string(stdout)) + return &creds, nil +} - // use local credentials if not - c := git.CommandContext(context.TODO(), git.GetDefaultExtraEnv(false), "credential", "fill") - c.Stdin = strings.NewReader(ToString(requestObj)) - stdout, err := c.Output() +// credentialsViaGit resolves credentials through `git credential fill`, which +// consults the host's configured credential helpers. +func credentialsViaGit(ctx context.Context, request *GitCredentials) (*GitCredentials, error) { + stdout, err := git.At("", git.WithStrictHostKeyChecking(false)). + CredentialFill(ctx, request.Encode()) if err != nil { return nil, err } - return Parse(string(stdout)) + creds := ParseCredentials(stdout) + return &creds, nil +} + +// Resolver adapts GetCredentials to download.CredentialResolver, letting the +// download package authenticate private assets without depending on this +// package (dependency inversion). +type Resolver struct{} + +// Resolve implements download.CredentialResolver. +func (Resolver) Resolve( + ctx context.Context, + protocol, host, path string, +) (username, password string, err error) { + creds, err := GetCredentials(ctx, &GitCredentials{Protocol: protocol, Host: host, Path: path}) + if err != nil || creds == nil { + return "", "", err + } + return creds.Username, creds.Password, nil } type GetHttpPathParameters struct { @@ -263,73 +276,38 @@ type GetHttpPathParameters struct { Repository string } -// GetHTTPPath checks for gits `credential.useHttpPath` setting for a given host+protocol and returns the path component -// of `GitCredential` if the setting is true. +// GetHTTPPath returns the repository path component when git's +// `credential..useHttpPath` is enabled for the host+protocol, else "". func GetHTTPPath(ctx context.Context, params GetHttpPathParameters) (string, error) { - // No need to look up the HTTP Path if we already have one if params.CurrentPath != "" { return params.CurrentPath, nil } - // Check if we need to respect gits `credential.useHttpPath` - // The actual format for the key is `credential.$PROTOCOL://$HOST.useHttpPath`, i.e. `credential.https://github.com.useHttpPath` - // We need to ignore the error as git will always exit with 1 if the key doesn't exist configKey := fmt.Sprintf("credential.%s://%s.useHttpPath", params.Protocol, params.Host) - out, _ := git.CommandContext(ctx, git.GetDefaultExtraEnv(false), "config", "--get", configKey). - Output() - if strings.TrimSpace(string(out)) != config.BoolTrue { + value, _ := git.At("", git.WithStrictHostKeyChecking(false)). + Config().Get(ctx, configKey, git.ScopeDefault) + if value != config.BoolTrue { return "", nil } - // We can assume the GitRepository is always HTTP(S) based as otherwise we wouldn't - // request credentials for it - url, err := netUrl.Parse(params.Repository) + + parsed, err := netURL.Parse(params.Repository) if err != nil { return "", fmt.Errorf("parse workspace repository: %w", err) } - - return url.Path, nil + return parsed.Path, nil } -func SetupGpgGitKey(gitSignKey string) error { - gitConfigCmd := exec.Command( - "git", - []string{"config", "--global", "user.signingKey", gitSignKey}...) - - out, err := gitConfigCmd.Output() - if err != nil { - return fmt.Errorf("git signkey: %s: %w", string(out), err) +// SetupGpgGitKey sets the global signing key. +func SetupGpgGitKey(ctx context.Context, gitSignKey string) error { + if err := git.At("").Config(). + Set(ctx, "user.signingKey", gitSignKey, git.ScopeGlobal); err != nil { + return fmt.Errorf("git signkey: %w", err) } - return nil } -func removeCredentialHelper(content string) string { - scan := scanner.NewScanner(strings.NewReader(content)) - - isCredential := false - out := []string{} - for scan.Scan() { - line := scan.Text() - if strings.TrimSpace(line) == "[credential]" { - isCredential = true - continue - } else if isCredential { - trimmed := strings.TrimSpace(line) - if len(trimmed) > 0 && trimmed[0] == '[' { - isCredential = false - } else { - continue - } - } - - out = append(out, line) - } - - return strings.Join(out, "\n") -} - -// getGlobalGitConfigPath resolves the global git config for the specified user according to -// https://git-scm.com/docs/git-config/#Documentation/git-config.txt-XDGCONFIGHOMEgitconfig +// getGlobalGitConfigPath resolves the global git config for the specified user +// per https://git-scm.com/docs/git-config/#Documentation/git-config.txt-XDGCONFIGHOMEgitconfig func getGlobalGitConfigPath(userName string) (string, error) { if xdgConfigHome := os.Getenv("XDG_CONFIG_HOME"); xdgConfigHome != "" { return filepath.Join(xdgConfigHome, "git", "config"), nil @@ -339,11 +317,10 @@ func getGlobalGitConfigPath(userName string) (string, error) { if err != nil { return "", fmt.Errorf("get homedir for %s: %w", userName, err) } - return filepath.Join(home, ".gitconfig"), nil } -// GetLocalGitConfigPath resolves the local git config for the specified repository path. +// GetLocalGitConfigPath resolves the local git config for a repository path. func GetLocalGitConfigPath(repoPath string) string { return filepath.Join(repoPath, ".git", "config") } diff --git a/pkg/gitcredentials/gitcredentials_test.go b/pkg/gitcredentials/gitcredentials_test.go index 8eef68fbc..fb8e670f8 100644 --- a/pkg/gitcredentials/gitcredentials_test.go +++ b/pkg/gitcredentials/gitcredentials_test.go @@ -1,10 +1,12 @@ package gitcredentials import ( + "context" "fmt" "os" "os/exec" "path/filepath" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -12,12 +14,12 @@ import ( ) func TestGetUser_WorkingDirResolvesIncludeIf(t *testing.T) { - tmpHome := t.TempDir() + tmpHome := tempDirResolved(t) t.Setenv("HOME", tmpHome) t.Setenv("XDG_CONFIG_HOME", "") t.Setenv("GIT_CONFIG_NOSYSTEM", "1") - projectDir := t.TempDir() + projectDir := tempDirResolved(t) //nolint:gosec // test-only, args are constants require.NoError(t, exec.Command("git", "init", projectDir).Run()) @@ -35,12 +37,23 @@ func TestGetUser_WorkingDirResolvesIncludeIf(t *testing.T) { path = %s `, projectDir, projectConfigPath), 0o600)) - user, err := GetUser("", projectDir) + user, err := GetUser(context.Background(), "", projectDir) require.NoError(t, err) assert.Equal(t, "Project User", user.Name) assert.Equal(t, "project@example.com", user.Email) } +// tempDirResolved returns a t.TempDir() with symlinks resolved. On macOS the +// temp dir lives under /var (a symlink to /private/var); git's `includeIf +// gitdir:` matches against the resolved path, so tests that build such patterns +// must use the resolved form or they silently fail to match. +func tempDirResolved(t *testing.T) string { + t.Helper() + dir, err := filepath.EvalSymlinks(t.TempDir()) + require.NoError(t, err) + return dir +} + func TestGetUser_EmptyWorkingDirUsesGlobal(t *testing.T) { tmpHome := t.TempDir() t.Setenv("HOME", tmpHome) @@ -53,7 +66,7 @@ func TestGetUser_EmptyWorkingDirUsesGlobal(t *testing.T) { email = global@example.com `), 0o600)) - user, err := GetUser("", "") + user, err := GetUser(context.Background(), "", "") require.NoError(t, err) assert.Equal(t, "Global User", user.Name) assert.Equal(t, "global@example.com", user.Email) @@ -85,8 +98,131 @@ func TestGetUser_WorkingDirWithNoMatchingIncludeIfUsesGlobal(t *testing.T) { //nolint:gosec // test-only, args are constants require.NoError(t, exec.Command("git", "init", projectDir).Run()) - user, err := GetUser("", projectDir) + user, err := GetUser(context.Background(), "", projectDir) require.NoError(t, err) assert.Equal(t, "Global User", user.Name) assert.Equal(t, "global@example.com", user.Email) } + +func TestCredentialsCodecRoundTrip(t *testing.T) { + original := GitCredentials{ + Protocol: "https", + Host: "github.com", + Path: "org/repo.git", + Username: "user", + Password: "tok=en", // value containing '=' must survive + } + + decoded := ParseCredentials(original.Encode()) + assert.Equal(t, original, decoded) +} + +func TestEncodeOmitsEmptyFields(t *testing.T) { + encoded := GitCredentials{Protocol: "https", Host: "github.com"}.Encode() + assert.Equal(t, "protocol=https\nhost=github.com\n", encoded) +} + +func TestParseCredentialsIgnoresUnknownAndBlankLines(t *testing.T) { + creds := ParseCredentials("protocol=https\n\ngarbage\nquit=1\nhost=example.com\n") + assert.Equal(t, "https", creds.Protocol) + assert.Equal(t, "example.com", creds.Host) +} + +func TestSetAndGetUserRoundTrip(t *testing.T) { + tmpHome := t.TempDir() + t.Setenv("HOME", tmpHome) + t.Setenv("XDG_CONFIG_HOME", "") + t.Setenv("GIT_CONFIG_NOSYSTEM", "1") + + ctx := context.Background() + name := `Injector"; touch /tmp/pwned #` + require.NoError(t, SetUser(ctx, "", &GitUser{Name: name, Email: "x@example.com"})) + + user, err := GetUser(ctx, "", "") + require.NoError(t, err) + assert.Equal(t, name, user.Name) + assert.Equal(t, "x@example.com", user.Email) +} + +func TestSetUserEmptyIdentityIsNoOp(t *testing.T) { + tmpHome := t.TempDir() + t.Setenv("HOME", tmpHome) + t.Setenv("XDG_CONFIG_HOME", "") + t.Setenv("GIT_CONFIG_NOSYSTEM", "1") + + // A named user with an empty identity must not attempt to chown a config + // file that was never written (regression: unconditional chown failed with + // ENOENT, aborting container credential/signing setup). + require.NoError(t, SetUser(context.Background(), "", &GitUser{})) + assert.NoFileExists(t, filepath.Join(tmpHome, ".gitconfig")) +} + +// helperTestEnv points git's global config at a temp HOME so ConfigureHelper / +// RemoveHelper operate on an isolated file. It returns the config path. +func helperTestEnv(t *testing.T) string { + t.Helper() + tmpHome := t.TempDir() + t.Setenv("HOME", tmpHome) + t.Setenv("XDG_CONFIG_HOME", "") + t.Setenv("GIT_CONFIG_NOSYSTEM", "1") + return filepath.Join(tmpHome, ".gitconfig") +} + +func gitConfigGetAll(t *testing.T, path, key string) []string { + t.Helper() + // #nosec G204 -- test-only, path/key are test-controlled + out, err := exec.Command("git", "config", "--file", path, "--get-all", key).Output() + if err != nil { + return nil + } + var vals []string + for line := range strings.SplitSeq(strings.TrimSpace(string(out)), "\n") { + if line != "" { + vals = append(vals, line) + } + } + return vals +} + +func TestConfigureHelperInstallsAndIsIdempotent(t *testing.T) { + path := helperTestEnv(t) + + require.NoError(t, ConfigureHelper(context.Background(), "/usr/local/bin/devsy", "", 8022)) + want := `!'/usr/local/bin/devsy' internal agent git-credentials --port 8022` + assert.Equal(t, []string{want}, gitConfigGetAll(t, path, "credential.helper")) + + require.NoError(t, ConfigureHelper(context.Background(), "/usr/local/bin/devsy", "", 8022)) + assert.Equal(t, []string{want}, gitConfigGetAll(t, path, "credential.helper")) +} + +func TestConfigureHelperReplacesExisting(t *testing.T) { + path := helperTestEnv(t) + // #nosec G204 -- test-only, path is test-controlled + cmd := exec.Command("git", "config", "--file", path, "--add", "credential.helper", "store") + require.NoError(t, cmd.Run()) + + require.NoError(t, ConfigureHelper(context.Background(), "/usr/local/bin/devsy", "", -1)) + want := `!'/usr/local/bin/devsy' internal agent git-credentials` + assert.Equal(t, []string{want}, gitConfigGetAll(t, path, "credential.helper")) +} + +func TestRemoveHelperPreservesOtherCredentialKeys(t *testing.T) { + path := helperTestEnv(t) + + require.NoError(t, ConfigureHelper(context.Background(), "/usr/local/bin/devsy", "", 1)) + // #nosec G204 -- test-only, path is test-controlled + cmd := exec.Command("git", "config", "--file", path, "credential.useHttpPath", "true") + require.NoError(t, cmd.Run()) + + require.NoError(t, RemoveHelperFromPath(context.Background(), path)) + + assert.Empty(t, gitConfigGetAll(t, path, "credential.helper")) + assert.Equal(t, []string{"true"}, gitConfigGetAll(t, path, "credential.useHttpPath")) +} + +func TestRemoveHelperOnMissingConfigIsNoError(t *testing.T) { + require.NoError( + t, + RemoveHelperFromPath(context.Background(), filepath.Join(t.TempDir(), "does-not-exist")), + ) +} diff --git a/pkg/gitsshsigning/utils.go b/pkg/gitsshsigning/utils.go index 92cc25f8e..6670c8803 100644 --- a/pkg/gitsshsigning/utils.go +++ b/pkg/gitsshsigning/utils.go @@ -1,8 +1,9 @@ package gitsshsigning import ( - "os/exec" - "strings" + "context" + + "github.com/devsy-org/devsy/pkg/git" ) const ( @@ -11,28 +12,18 @@ const ( GPGFormatSSH = "ssh" ) -func ExtractGitConfiguration(workingDir string) (string, string, error) { - format, err := readGitConfigValue(GPGFormatConfigKey, workingDir) +func ExtractGitConfiguration(ctx context.Context, workingDir string) (string, string, error) { + config := git.At(workingDir).Config() + + format, err := config.Get(ctx, GPGFormatConfigKey, git.ScopeDefault) if err != nil { return "", "", err } - signingKey, err := readGitConfigValue(UsersSigningKeyConfigKey, workingDir) + signingKey, err := config.Get(ctx, UsersSigningKeyConfigKey, git.ScopeDefault) if err != nil { return "", "", err } return format, signingKey, nil } - -func readGitConfigValue(key string, workingDir string) (string, error) { - cmd := exec.Command("git", "config", "--get", key) - if workingDir != "" { - cmd.Dir = workingDir - } - output, err := cmd.Output() - if err != nil { - return "", err - } - return strings.TrimSpace(string(output)), nil -} diff --git a/pkg/gitsshsigning/utils_test.go b/pkg/gitsshsigning/utils_test.go index c7cb6324a..cfb792ee2 100644 --- a/pkg/gitsshsigning/utils_test.go +++ b/pkg/gitsshsigning/utils_test.go @@ -1,6 +1,7 @@ package gitsshsigning import ( + "context" "fmt" "os" "os/exec" @@ -12,12 +13,12 @@ import ( ) func TestExtractGitConfiguration_WorkingDirResolvesIncludeIf(t *testing.T) { - tmpHome := t.TempDir() + tmpHome := tempDirResolved(t) t.Setenv("HOME", tmpHome) t.Setenv("XDG_CONFIG_HOME", "") t.Setenv("GIT_CONFIG_NOSYSTEM", "1") - projectDir := t.TempDir() + projectDir := tempDirResolved(t) //nolint:gosec // test-only, args are constants require.NoError(t, exec.Command("git", "init", projectDir).Run()) @@ -33,7 +34,7 @@ func TestExtractGitConfiguration_WorkingDirResolvesIncludeIf(t *testing.T) { path = %s `, projectDir, projectConfigPath), 0o600)) - format, signingKey, err := ExtractGitConfiguration(projectDir) + format, signingKey, err := ExtractGitConfiguration(context.Background(), projectDir) require.NoError(t, err) assert.Equal(t, GPGFormatSSH, format) assert.Equal(t, "/path/to/signing.pub", signingKey) @@ -56,8 +57,19 @@ func TestExtractGitConfiguration_WorkingDirWithGlobalSigningConfig(t *testing.T) signingkey = /global/key.pub `), 0o600)) - format, signingKey, err := ExtractGitConfiguration(projectDir) + format, signingKey, err := ExtractGitConfiguration(context.Background(), projectDir) require.NoError(t, err) assert.Equal(t, GPGFormatSSH, format) assert.Equal(t, "/global/key.pub", signingKey) } + +// tempDirResolved returns a t.TempDir() with symlinks resolved. On macOS the +// temp dir lives under /var (a symlink to /private/var); git's `includeIf +// gitdir:` matches against the resolved path, so tests that build such patterns +// must use the resolved form or they silently fail to match. +func tempDirResolved(t *testing.T) string { + t.Helper() + dir, err := filepath.EvalSymlinks(t.TempDir()) + require.NoError(t, err) + return dir +} diff --git a/pkg/provider/download.go b/pkg/provider/download.go index 142b05d14..fab388e96 100644 --- a/pkg/provider/download.go +++ b/pkg/provider/download.go @@ -1,6 +1,7 @@ package provider import ( + "context" "errors" "fmt" "io" @@ -16,6 +17,7 @@ import ( "github.com/devsy-org/devsy/pkg/copy" "github.com/devsy-org/devsy/pkg/download" "github.com/devsy-org/devsy/pkg/extract" + "github.com/devsy-org/devsy/pkg/gitcredentials" "github.com/devsy-org/devsy/pkg/hash" "github.com/devsy-org/devsy/pkg/log" "k8s.io/client-go/util/retry" @@ -103,12 +105,13 @@ func GetBinaries(context string, config *ProviderConfig) (map[string]string, err } func DownloadBinaries( + ctx context.Context, binaries map[string][]*ProviderBinary, targetFolder string, ) (map[string]string, error) { retBinaries := map[string]string{} for binaryName, binaryLocations := range binaries { - binaryPath, err := downloadBinaryForPlatform(binaryName, binaryLocations, targetFolder) + binaryPath, err := downloadBinaryForPlatform(ctx, binaryName, binaryLocations, targetFolder) if err != nil { return nil, err } @@ -119,6 +122,7 @@ func DownloadBinaries( } func downloadBinaryForPlatform( + ctx context.Context, binaryName string, binaryLocations []*ProviderBinary, targetFolder string, @@ -137,7 +141,7 @@ func downloadBinaryForPlatform( } // try to download the binary - binaryPath, err := downloadWithRetry(binaryName, binary, binaryTargetFolder) + binaryPath, err := downloadWithRetry(ctx, binaryName, binary, binaryTargetFolder) if err != nil { return "", err } @@ -149,13 +153,14 @@ func downloadBinaryForPlatform( } func downloadWithRetry( + ctx context.Context, binaryName string, binary *ProviderBinary, targetFolder string, ) (string, error) { var binaryPath string err := retry.OnError(downloadBackoff, isRetriableError, func() error { - path, err := downloadBinary(binaryName, binary, targetFolder) + path, err := downloadBinary(ctx, binaryName, binary, targetFolder) if err != nil { return err } @@ -389,6 +394,7 @@ func isRemotePath(p string) bool { } func downloadBinary( + ctx context.Context, binaryName string, binary *ProviderBinary, targetFolder string, @@ -397,7 +403,7 @@ func downloadBinary( if err := os.MkdirAll(targetFolder, dirPerms); err != nil { return "", fmt.Errorf("create folder: %w", err) } - return downloadRemoteBinary(binaryName, binary, targetFolder) + return downloadRemoteBinary(ctx, binaryName, binary, targetFolder) } if _, err := os.Stat(binary.Path); err == nil { @@ -434,6 +440,7 @@ func handleNonHTTPBinary(binary *ProviderBinary, targetFolder string) (string, e } func downloadRemoteBinary( + ctx context.Context, binaryName string, binary *ProviderBinary, targetFolder string, @@ -442,9 +449,9 @@ func downloadRemoteBinary( var err error if binary.ArchivePath != "" { - targetPath, err = downloadArchive(binaryName, binary, targetFolder) + targetPath, err = downloadArchive(ctx, binaryName, binary, targetFolder) } else { - targetPath, err = downloadFile(binaryName, binary, targetFolder) + targetPath, err = downloadFile(ctx, binaryName, binary, targetFolder) } if err != nil { @@ -462,6 +469,7 @@ func downloadRemoteBinary( } func downloadFile( + ctx context.Context, binaryName string, binary *ProviderBinary, targetFolder string, @@ -473,17 +481,22 @@ func downloadFile( // (could be partial download from previous failed attempt) _ = os.Remove(targetPath) - return downloadAndSaveFile(binaryName, binary, targetPath) + return downloadAndSaveFile(ctx, binaryName, binary, targetPath) } func downloadAndSaveFile( + ctx context.Context, binaryName string, binary *ProviderBinary, targetPath string, ) (string, error) { log.Infof("downloading binary %s from %s", binaryName, binary.Path) - body, err := download.File(binary.Path) + body, err := download.File( + ctx, + binary.Path, + download.WithCredentialResolver(gitcredentials.Resolver{}), + ) if err != nil { return "", fmt.Errorf("download binary: %w", err) } @@ -505,6 +518,7 @@ func downloadAndSaveFile( } func downloadArchive( + ctx context.Context, binaryName string, binary *ProviderBinary, targetFolder string, @@ -518,7 +532,7 @@ func downloadArchive( // (could be partial download from previous failed attempt) _ = os.Remove(targetPath) - return extractArchive(archiveDownloadParams{ + return extractArchive(ctx, archiveDownloadParams{ binaryName: binaryName, binary: binary, targetFolder: targetFolder, @@ -533,10 +547,14 @@ type archiveDownloadParams struct { targetPath string } -func extractArchive(params archiveDownloadParams) (string, error) { +func extractArchive(ctx context.Context, params archiveDownloadParams) (string, error) { log.Infof("downloading binary %s from %s", params.binaryName, params.binary.Path) - body, err := download.File(params.binary.Path) + body, err := download.File( + ctx, + params.binary.Path, + download.WithCredentialResolver(gitcredentials.Resolver{}), + ) if err != nil { return "", err } diff --git a/pkg/provider/resolve.go b/pkg/provider/resolve.go index acece6455..dc087dc87 100644 --- a/pkg/provider/resolve.go +++ b/pkg/provider/resolve.go @@ -1,14 +1,17 @@ package provider import ( + "context" "fmt" "io" + "net/http" "os" "path/filepath" "strings" "github.com/devsy-org/devsy/pkg/config" "github.com/devsy-org/devsy/pkg/download" + "github.com/devsy-org/devsy/pkg/gitcredentials" devsyhttp "github.com/devsy-org/devsy/pkg/http" "github.com/devsy-org/devsy/pkg/log" "github.com/devsy-org/devsy/providers" @@ -18,6 +21,7 @@ import ( // bytes and a ProviderSource describing the origin (internal, URL, file, or // GitHub release). func ResolveProvider( + ctx context.Context, providerSource string, ) ([]byte, *ProviderSource, error) { retSource := &ProviderSource{Raw: strings.TrimSpace(providerSource)} @@ -27,6 +31,7 @@ func ResolveProvider( } if out, err := tryResolveURLProvider( + ctx, providerSource, retSource, ); hasOutputOrError( @@ -40,7 +45,7 @@ func ResolveProvider( return out, retSource, err } - out, source, err := downloadProviderGithub(providerSource) + out, source, err := downloadProviderGithub(ctx, providerSource) if len(out) > 0 || err != nil { return out, source, err } @@ -55,10 +60,11 @@ func hasOutputOrError(out []byte, err error) bool { } func tryResolveURLProvider( + ctx context.Context, providerSource string, retSource *ProviderSource, ) ([]byte, error) { - out, ok, err := resolveURLProvider(providerSource, retSource) + out, ok, err := resolveURLProvider(ctx, providerSource, retSource) if !ok { return nil, nil } @@ -77,6 +83,7 @@ func tryResolveFileProvider( } func downloadProviderGithub( + ctx context.Context, originalPath string, ) ([]byte, *ProviderSource, error) { path := strings.TrimPrefix(originalPath, "github.com/") @@ -100,7 +107,11 @@ func downloadProviderGithub( requestURL := buildGithubURL(path, release) - body, err := download.File(requestURL) + body, err := download.File( + ctx, + requestURL, + download.WithCredentialResolver(gitcredentials.Resolver{}), + ) if err != nil { return nil, nil, fmt.Errorf("download: %w", err) } @@ -130,6 +141,7 @@ func resolveInternalProvider( } func resolveURLProvider( + ctx context.Context, providerSource string, retSource *ProviderSource, ) ([]byte, bool, error) { @@ -139,7 +151,7 @@ func resolveURLProvider( } log.Infof("downloading provider from %s", providerSource) - out, err := downloadProvider(providerSource) + out, err := downloadProvider(ctx, providerSource) if err != nil { return nil, true, fmt.Errorf("download provider: %w", err) } @@ -176,8 +188,12 @@ func resolveFileProvider( return out, true, nil } -func downloadProvider(url string) ([]byte, error) { - resp, err := devsyhttp.GetHTTPClient().Get(url) +func downloadProvider(ctx context.Context, url string) ([]byte, error) { + req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) + if err != nil { + return nil, err + } + resp, err := devsyhttp.GetHTTPClient().Do(req) if err != nil { return nil, fmt.Errorf("download binary: %w", err) } diff --git a/pkg/provider/workspace.go b/pkg/provider/workspace.go index d3862f423..7f8c769f7 100644 --- a/pkg/provider/workspace.go +++ b/pkg/provider/workspace.go @@ -227,6 +227,7 @@ type CLIOptions struct { DaemonInterval string `json:"daemonInterval,omitempty"` GitCloneStrategy git.CloneStrategy `json:"gitCloneStrategy,omitempty"` GitCloneRecursiveSubmodules bool `json:"gitCloneRecursive,omitempty"` + GitLFSMode git.LFSMode `json:"gitLFSMode,omitempty"` FallbackImage string `json:"fallbackImage,omitempty"` GitSSHSigningKey string `json:"gitSshSigningKey,omitempty"` SSHAuthSockID string `json:"sshAuthSockID,omitempty"` // ID to use when looking for SSH_AUTH_SOCK, defaults to a new random ID if not set (only used for browser IDEs) diff --git a/pkg/tunnel/services.go b/pkg/tunnel/services.go index afba69008..cce6fc8c7 100644 --- a/pkg/tunnel/services.go +++ b/pkg/tunnel/services.go @@ -124,10 +124,15 @@ func runTunnelServer(ctx context.Context, cancel context.CancelFunc, p tunnelSer // When explicitKey is set (from --git-ssh-signing-key flag), it takes // precedence over the host's .gitconfig. This ensures signing works // even when user.signingkey is not in the host git configuration. -func addGitSSHSigningKey(command string, explicitKey string, workingDir string) string { +func addGitSSHSigningKey( + ctx context.Context, + command string, + explicitKey string, + workingDir string, +) string { userSigningKey := explicitKey if userSigningKey == "" { - format, extracted, err := gitsshsigning.ExtractGitConfiguration(workingDir) + format, extracted, err := gitsshsigning.ExtractGitConfiguration(ctx, workingDir) if err != nil { log.Debugf("failed to extract git configuration: %v", err) return command @@ -143,7 +148,7 @@ func addGitSSHSigningKey(command string, explicitKey string, workingDir string) } // buildCredentialsCommand builds the credentials server command. -func buildCredentialsCommand(opts RunServicesOptions) string { +func buildCredentialsCommand(ctx context.Context, opts RunServicesOptions) string { command := fmt.Sprintf( "%s%s internal agent container credentials-server --user %s", agent.ContainerAgentEnvPrefix, @@ -158,7 +163,7 @@ func buildCredentialsCommand(opts RunServicesOptions) string { if opts.Workspace != nil { workingDir = opts.Workspace.Source.LocalFolder } - command = addGitSSHSigningKey(command, opts.GitSSHSigningKey, workingDir) + command = addGitSSHSigningKey(ctx, command, opts.GitSSHSigningKey, workingDir) } if opts.ConfigureDockerCredentials { command += " --configure-docker-helper" @@ -209,7 +214,7 @@ func runServicesIteration( writer := log.Writer(log.LevelDebug) defer func() { _ = writer.Close() }() - command := buildCredentialsCommand(opts) + command := buildCredentialsCommand(ctx, opts) err = devssh.Run(cancelCtx, devssh.RunOptions{ Client: opts.ContainerClient, diff --git a/pkg/tunnel/services_test.go b/pkg/tunnel/services_test.go index d397e4857..6c2bd317e 100644 --- a/pkg/tunnel/services_test.go +++ b/pkg/tunnel/services_test.go @@ -1,6 +1,7 @@ package tunnel import ( + "context" "encoding/base64" "testing" "time" @@ -58,7 +59,7 @@ const testBaseCommand = "devsy internal agent container credentials-server --use func TestAddGitSSHSigningKey_ExplicitKey(t *testing.T) { command := testBaseCommand - result := addGitSSHSigningKey(command, "/path/to/key.pub", "") + result := addGitSSHSigningKey(context.Background(), command, "/path/to/key.pub", "") encoded := base64.StdEncoding.EncodeToString([]byte("/path/to/key.pub")) assert.Equal(t, command+" --git-user-signing-key "+encoded, result) @@ -69,7 +70,7 @@ func TestAddGitSSHSigningKey_ExplicitKeyTakesPrecedence(t *testing.T) { // of what ExtractGitConfiguration might return from host .gitconfig. command := testBaseCommand explicitKey := "/explicit/key.pub" - result := addGitSSHSigningKey(command, explicitKey, "") + result := addGitSSHSigningKey(context.Background(), command, explicitKey, "") encoded := base64.StdEncoding.EncodeToString([]byte(explicitKey)) assert.Equal(t, command+" --git-user-signing-key "+encoded, result) @@ -82,7 +83,7 @@ func TestAddGitSSHSigningKey_EmptyExplicitKey_FallsBackToHostConfig(t *testing.T t.Setenv("HOME", tmpHome) t.Setenv("XDG_CONFIG_HOME", tmpHome) - result := addGitSSHSigningKey(command, "", "") + result := addGitSSHSigningKey(context.Background(), command, "", "") assert.Equal(t, command, result) assert.NotContains(t, result, "--git-user-signing-key") @@ -94,7 +95,7 @@ func TestBuildCredentialsCommand_IncludesSigningKey(t *testing.T) { ConfigureGitSSHSignatureHelper: true, GitSSHSigningKey: "/my/key.pub", } - command := buildCredentialsCommand(opts) + command := buildCredentialsCommand(context.Background(), opts) encoded := base64.StdEncoding.EncodeToString([]byte("/my/key.pub")) assert.Contains(t, command, "--git-user-signing-key "+encoded) @@ -106,7 +107,7 @@ func TestBuildCredentialsCommand_NoSigningKey(t *testing.T) { User: "testuser", ConfigureGitSSHSignatureHelper: false, } - command := buildCredentialsCommand(opts) + command := buildCredentialsCommand(context.Background(), opts) assert.NotContains(t, command, "--git-user-signing-key") } diff --git a/pkg/workspace/provider.go b/pkg/workspace/provider.go index 76068a016..ce7dca684 100644 --- a/pkg/workspace/provider.go +++ b/pkg/workspace/provider.go @@ -104,15 +104,16 @@ func ProviderFromHost( } func AddProvider( + ctx context.Context, devsyConfig *config.Config, providerName, providerSourceRaw string, ) (*provider.ProviderConfig, error) { - providerRaw, providerSource, err := provider.ResolveProvider(providerSourceRaw) + providerRaw, providerSource, err := provider.ResolveProvider(ctx, providerSourceRaw) if err != nil { return nil, err } - return AddProviderRaw(ProviderParams{ + return AddProviderRaw(ctx, ProviderParams{ DevsyConfig: devsyConfig, ProviderName: providerName, Source: providerSource, @@ -120,8 +121,8 @@ func AddProvider( }) } -func AddProviderRaw(p ProviderParams) (*provider.ProviderConfig, error) { - providerConfig, err := installRawProvider(p) +func AddProviderRaw(ctx context.Context, p ProviderParams) (*provider.ProviderConfig, error) { + providerConfig, err := installRawProvider(ctx, p) if err != nil { return nil, err } @@ -143,6 +144,7 @@ func AddProviderRaw(p ProviderParams) (*provider.ProviderConfig, error) { } func UpdateProvider( + ctx context.Context, devsyConfig *config.Config, providerName, providerSourceRaw string, ) (*provider.ProviderConfig, error) { @@ -158,12 +160,12 @@ func UpdateProvider( providerSourceRaw = s } - providerRaw, providerSource, err := provider.ResolveProvider(providerSourceRaw) + providerRaw, providerSource, err := provider.ResolveProvider(ctx, providerSourceRaw) if err != nil { return nil, err } - return updateProvider(ProviderParams{ + return updateProvider(ctx, ProviderParams{ DevsyConfig: devsyConfig, ProviderName: providerName, Raw: providerRaw, @@ -172,6 +174,7 @@ func UpdateProvider( } func CloneProvider( + ctx context.Context, devsyConfig *config.Config, providerName, providerSourceRaw string, ) (*ProviderWithOptions, error) { @@ -181,6 +184,7 @@ func CloneProvider( } providerConfig, err := installProvider( + ctx, ProviderParams{ DevsyConfig: devsyConfig, ProviderName: providerName, @@ -288,12 +292,12 @@ func loadProviderEntry( return nil } -func installRawProvider(p ProviderParams) (*provider.ProviderConfig, error) { +func installRawProvider(ctx context.Context, p ProviderParams) (*provider.ProviderConfig, error) { providerConfig, err := provider.ParseProvider(bytes.NewReader(p.Raw)) if err != nil { return nil, err } - return installProvider(ProviderParams{ + return installProvider(ctx, ProviderParams{ DevsyConfig: p.DevsyConfig, ProviderName: p.ProviderName, Source: p.Source, @@ -301,6 +305,7 @@ func installRawProvider(p ProviderParams) (*provider.ProviderConfig, error) { } func installProvider( + ctx context.Context, p ProviderParams, providerConfig *provider.ProviderConfig, ) (*provider.ProviderConfig, error) { @@ -317,14 +322,14 @@ func installProvider( return nil, err } - if err := downloadAndSaveProvider(p, providerConfig); err != nil { + if err := downloadAndSaveProvider(ctx, p, providerConfig); err != nil { return nil, err } return providerConfig, nil } -func updateProvider(p ProviderParams) (*provider.ProviderConfig, error) { +func updateProvider(ctx context.Context, p ProviderParams) (*provider.ProviderConfig, error) { providerConfig, err := parseAndValidateProvider(p) if err != nil { return nil, err @@ -336,7 +341,7 @@ func updateProvider(p ProviderParams) (*provider.ProviderConfig, error) { return nil, err } - if err := downloadAndSaveProvider(p, providerConfig); err != nil { + if err := downloadAndSaveProvider(ctx, p, providerConfig); err != nil { return nil, err } @@ -380,7 +385,11 @@ func checkProviderNotExists(devsyConfig *config.Config, providerName string) err return nil } -func downloadAndSaveProvider(p ProviderParams, providerConfig *provider.ProviderConfig) error { +func downloadAndSaveProvider( + ctx context.Context, + p ProviderParams, + providerConfig *provider.ProviderConfig, +) error { binariesDir, err := provider.GetProviderBinariesDir( p.DevsyConfig.DefaultContext, providerConfig.Name, @@ -395,6 +404,7 @@ func downloadAndSaveProvider(p ProviderParams, providerConfig *provider.Provider } if _, err := provider.DownloadBinaries( + ctx, providerConfig.Binaries, binariesDir, ); err != nil { diff --git a/pkg/workspace/provider_update.go b/pkg/workspace/provider_update.go index 1844c7794..429be2895 100644 --- a/pkg/workspace/provider_update.go +++ b/pkg/workspace/provider_update.go @@ -1,6 +1,7 @@ package workspace import ( + "context" "fmt" "strings" @@ -15,6 +16,7 @@ import ( // CheckProviderUpdate currently only ensures the local provider is in sync with the remote for Devsy Pro instances. // Potentially auto-upgrade other providers in the future. func CheckProviderUpdate( + ctx context.Context, devsyConfig *config.Config, proInstance *provider2.ProInstance, ) error { @@ -27,10 +29,11 @@ func CheckProviderUpdate( return nil } - return checkProviderUpdateForInstance(devsyConfig, proInstance) + return checkProviderUpdateForInstance(ctx, devsyConfig, proInstance) } func checkProviderUpdateForInstance( + ctx context.Context, devsyConfig *config.Config, proInstance *provider2.ProInstance, ) error { @@ -63,7 +66,7 @@ func checkProviderUpdateForInstance( newVersion, ) - return applyProviderUpdate(devsyConfig, proInstance.Provider, newVersion) + return applyProviderUpdate(ctx, devsyConfig, proInstance.Provider, newVersion) } func providerVersionNeedsUpdate(newVersion, currentVersion string) (bool, error) { @@ -79,6 +82,7 @@ func providerVersionNeedsUpdate(newVersion, currentVersion string) (bool, error) } func applyProviderUpdate( + ctx context.Context, devsyConfig *config.Config, providerName, newVersion string, ) error { @@ -93,7 +97,7 @@ func applyProviderUpdate( } providerSource = splitted[0] + "@" + newVersion - _, err = UpdateProvider(devsyConfig, providerName, providerSource) + _, err = UpdateProvider(ctx, devsyConfig, providerName, providerSource) if err != nil { return fmt.Errorf("update provider %s: %w", providerName, err) } diff --git a/pkg/workspace/provider_versions.go b/pkg/workspace/provider_versions.go index 5e3965f0c..c6a84c25e 100644 --- a/pkg/workspace/provider_versions.go +++ b/pkg/workspace/provider_versions.go @@ -1,6 +1,7 @@ package workspace import ( + "context" "errors" "fmt" @@ -27,7 +28,11 @@ func ListProviderVersions( } // SetProviderVersion switches the provider to the given tag. -func SetProviderVersion(devsyConfig *config.Config, providerName, tag string) error { +func SetProviderVersion( + ctx context.Context, + devsyConfig *config.Config, + providerName, tag string, +) error { source, err := ResolveProviderSource(devsyConfig, providerName) if err != nil { return fmt.Errorf("resolve provider source: %w", err) @@ -36,7 +41,7 @@ func SetProviderVersion(devsyConfig *config.Config, providerName, tag string) er if err != nil { return err } - _, err = UpdateProvider(devsyConfig, providerName, rewritten) + _, err = UpdateProvider(ctx, devsyConfig, providerName, rewritten) return err } From 35ab9f1cf12ff9d7ece2d5a300c3a92291785741 Mon Sep 17 00:00:00 2001 From: Samuel K Date: Sun, 5 Jul 2026 05:58:00 -0500 Subject: [PATCH 2/3] chore: cleanup comments Signed-off-by: Samuel K --- pkg/git/config.go | 4 --- pkg/git/{args.go => const.go} | 2 -- pkg/git/git.go | 7 +++-- pkg/git/installer.go | 6 +---- pkg/git/installer_release.go | 32 +++++------------------ pkg/git/repo.go | 7 ++--- pkg/git/runner.go | 5 +--- pkg/gitcredentials/gitcredentials.go | 10 ++----- pkg/gitcredentials/gitcredentials_test.go | 9 ------- 9 files changed, 15 insertions(+), 67 deletions(-) rename pkg/git/{args.go => const.go} (80%) diff --git a/pkg/git/config.go b/pkg/git/config.go index 7deea0460..b1da3b8bb 100644 --- a/pkg/git/config.go +++ b/pkg/git/config.go @@ -14,13 +14,9 @@ type ConfigScope struct { } var ( - // ScopeDefault targets the default config file (repository, global, or system). ScopeDefault = ConfigScope{} - // ScopeLocal targets the repository's .git/config. ScopeLocal = ConfigScope{flag: "--local"} - // ScopeGlobal targets the user's global config. ScopeGlobal = ConfigScope{flag: flagGlobal} - // ScopeSystem targets the system-wide config. ScopeSystem = ConfigScope{flag: flagSystem} ) diff --git a/pkg/git/args.go b/pkg/git/const.go similarity index 80% rename from pkg/git/args.go rename to pkg/git/const.go index e306ab43d..355a50c27 100644 --- a/pkg/git/args.go +++ b/pkg/git/const.go @@ -1,7 +1,5 @@ package git -// Shared git subcommands, flags, and binary names used to build command lines. -// Centralized so repeated literals stay consistent (and satisfy goconst). const ( binGit = "git" binGitLFS = "git-lfs" diff --git a/pkg/git/git.go b/pkg/git/git.go index afc707c08..a812fa58a 100644 --- a/pkg/git/git.go +++ b/pkg/git/git.go @@ -15,14 +15,13 @@ const ( CommitDelimiter string = "@sha256:" PullRequestReference string = "pull/([0-9]+)/head" SubPathDelimiter string = "@subpath:" + repoBaseRegEx string = `((?:(?:https?|git|ssh|file):\/\/)?\/?(?:[^@\/\n]+@)?(?:[^:\/\n]+)(?:[:\/][^\/\n]+)+(?:\.git)?)` ) // WARN: Make sure this matches the regex in /desktop/src/views/Workspaces/CreateWorkspace/CreateWorkspaceInput.tsx! var ( - // Updated regex pattern to support SSH-style Git URLs. - repoBaseRegEx = `((?:(?:https?|git|ssh|file):\/\/)?\/?(?:[^@\/\n]+@)?(?:[^:\/\n]+)(?:[:\/][^\/\n]+)+(?:\.git)?)` - branchRegEx = regexp.MustCompile(`^` + repoBaseRegEx + `@([a-zA-Z0-9\./\-\_]+)$`) - commitRegEx = regexp.MustCompile( + branchRegEx = regexp.MustCompile(`^` + repoBaseRegEx + `@([a-zA-Z0-9\./\-\_]+)$`) + commitRegEx = regexp.MustCompile( `^` + repoBaseRegEx + regexp.QuoteMeta(CommitDelimiter) + `([a-zA-Z0-9]+)$`, ) prReferenceRegEx = regexp.MustCompile(`^` + repoBaseRegEx + `@(` + PullRequestReference + `)$`) diff --git a/pkg/git/installer.go b/pkg/git/installer.go index 0abc77f59..015ca4dec 100644 --- a/pkg/git/installer.go +++ b/pkg/git/installer.go @@ -65,10 +65,7 @@ func newInstaller(runner Runner) *Installer { } } -// ensure installs the tool if it is not already available. Strategies are tried -// in order; a strategy that fails (e.g. apt present but no root/network) does -// not abort the sequence, so a later fallback like the GitHub release can still -// succeed. +// ensure installs the tool if it is not already available. func (i *Installer) ensure(ctx context.Context, t tool) error { if command.Exists(t.binary) { return nil @@ -101,7 +98,6 @@ func (i *Installer) ensure(ctx context.Context, t tool) error { return fmt.Errorf("install %s (tried: %v): %w", t.binary, tried, errors.Join(errs...)) } -// pkgManagerStrategy installs via a system package manager (apt, apk). type pkgManagerStrategy struct { manager string runner Runner diff --git a/pkg/git/installer_release.go b/pkg/git/installer_release.go index 515c9a176..9258ad685 100644 --- a/pkg/git/installer_release.go +++ b/pkg/git/installer_release.go @@ -17,29 +17,15 @@ import ( "github.com/devsy-org/devsy/pkg/log" ) -// releaseSource describes how to fetch a tool binary from a GitHub release. type releaseSource struct { - // version is the release tag without the leading "v" (e.g. "3.5.1"). version string - // assetName builds the release asset filename for the given GOOS/GOARCH. assetName func(goos, goarch, version string) (string, error) - // downloadURL builds the download URL for a resolved asset filename. downloadURL func(version, asset string) string - // checksums maps asset filename to its expected lowercase-hex SHA-256. The - // download is rejected if its digest is absent or does not match, pinning - // the exact bytes we install (defense against a compromised release host). checksums map[string]string - // binaryInArchive is the name of the executable within the extracted - // archive (it may sit under a versioned subdirectory). binaryInArchive string - // installDir is the directory the binary is placed into; it must be on PATH. - // Empty defaults to the platform's SystemBinDir (see pkg/config). installDir string } -// gitLFSRelease pins a git-lfs release used when no package manager is present. -// git-lfs publishes per-OS/arch archives (.tar.gz on linux, .zip on darwin and -// windows) each containing a single git-lfs binary (git-lfs.exe on windows). var gitLFSRelease = releaseSource{ version: "3.5.1", binaryInArchive: binGitLFS, @@ -68,8 +54,6 @@ var gitLFSRelease = releaseSource{ version, asset, ) }, - // SHA-256 digests from git-lfs v3.5.1 sha256sums.asc for the OS/arch pairs - // assetName can resolve. Update alongside version. checksums: map[string]string{ "git-lfs-linux-amd64-v3.5.1.tar.gz": "6f28eb19faa7a968882dca190d92adc82493378b933958d67ceaeb9ebe4d731e", "git-lfs-linux-arm64-v3.5.1.tar.gz": "4f8700aacaa0fd26ae5300fb0996aed14d1fd0ce1a63eb690629c132ff5163a9", @@ -104,7 +88,6 @@ func (s *releaseSource) install(ctx context.Context, binary string) error { binary: binary, url: s.downloadURL(s.version, asset), wantSum: wantSum, - // git-lfs on windows ships as git-lfs.exe inside the archive. execName: executableName(s.binaryInArchive, runtime.GOOS), } src, cleanup, err := s.fetchBinary(ctx, req) @@ -125,10 +108,10 @@ func (s *releaseSource) install(ctx context.Context, binary string) error { // fetchRequest describes a single release-asset download and extraction. type fetchRequest struct { - binary string // human-facing tool name for logs/errors - url string // resolved asset download URL - wantSum string // pinned lowercase-hex SHA-256 of the asset - execName string // executable filename to locate inside the archive + binary string + url string + wantSum string + execName string } // executableName returns the platform-specific executable filename, appending @@ -178,8 +161,7 @@ func (s *releaseSource) fetchBinary( return src, cleanup, nil } -// readVerified reads r fully and returns a reader over its bytes only if the -// content's SHA-256 matches wantSum (lowercase hex). +// readVerified reads all data from r, computes its SHA-256, and compares it to wantSum. func readVerified(r io.Reader, wantSum string) (io.Reader, error) { data, err := io.ReadAll(r) if err != nil { @@ -215,9 +197,7 @@ func findBinary(root, name string) (string, error) { return found, nil } -// moveExecutable copies src to dst with executable permissions. Both paths are -// internally constructed (temp extraction dir and the configured install dir), -// not user input. +// moveExecutable copies src to dst with executable permissions. func moveExecutable(src, dst string) error { data, err := os.ReadFile(src) // #nosec G304 -- src is within our temp extraction dir if err != nil { diff --git a/pkg/git/repo.go b/pkg/git/repo.go index dd7232694..133c25dff 100644 --- a/pkg/git/repo.go +++ b/pkg/git/repo.go @@ -79,12 +79,9 @@ func (r *Repo) Switch(ctx context.Context, branch string) error { return nil } -// CheckoutPR fetches a pull request head reference into a local branch and -// switches to it. prRef is a reference like "pull/996/head"; the local branch -// is derived via GetBranchNameForPR (e.g. "PR996"). This mirrors GitHub's -// documented "checking out pull requests locally" flow. +// CheckoutPR fetches the given pull request refspec and switches to a local branch for it. func (r *Repo) CheckoutPR(ctx context.Context, prRef string) error { - log.Debugf("Fetching pull request : %s", prRef) + log.Debugf("fetching pull request: %s", prRef) prBranch := GetBranchNameForPR(prRef) if err := r.Fetch(ctx, prRef+":"+prBranch); err != nil { diff --git a/pkg/git/runner.go b/pkg/git/runner.go index 691856a24..8a777ad03 100644 --- a/pkg/git/runner.go +++ b/pkg/git/runner.go @@ -92,8 +92,6 @@ func (execRunner) Run(ctx context.Context, opts RunOptions) (RunResult, error) { } else { cmd.Stdout = &outBuf } - // Always capture stderr into errBuf so CommandError carries git's message, - // teeing to the caller's writer when one is provided. if opts.Stderr != nil { cmd.Stderr = io.MultiWriter(opts.Stderr, &errBuf) } else { @@ -109,8 +107,7 @@ func (execRunner) Run(ctx context.Context, opts RunOptions) (RunResult, error) { Stderr: strings.TrimSpace(errBuf.String()), Err: err, } - var exitErr *exec.ExitError - if errors.As(err, &exitErr) { + if exitErr, ok := errors.AsType[*exec.ExitError](err); ok { cmdErr.ExitCode = exitErr.ExitCode() } return result, cmdErr diff --git a/pkg/gitcredentials/gitcredentials.go b/pkg/gitcredentials/gitcredentials.go index f5a8a2b3e..35293e2a5 100644 --- a/pkg/gitcredentials/gitcredentials.go +++ b/pkg/gitcredentials/gitcredentials.go @@ -157,9 +157,7 @@ func SetUser(ctx context.Context, userName string, user *GitUser) error { wrote = true } - // Only reassign ownership when we actually wrote the named user's config. - // Chowning unconditionally would fail on the not-yet-created file when - // neither field was provided. + // If config was written, chown the config file to the named user. if wrote && userName != "" { path, err := getGlobalGitConfigPath(userName) if err != nil { @@ -191,8 +189,7 @@ func GetUser(ctx context.Context, userName, workingDir string) (*GitUser, error) return &GitUser{Name: name, Email: email}, nil } -// identityScope resolves the config scope for setting a user's identity: the -// named user's global config file, or the current user's global config. +// identityScope resolves the config scope for setting a user's identity. func identityScope(userName string) (git.ConfigScope, error) { if userName == "" { return git.ScopeGlobal, nil @@ -252,9 +249,6 @@ func credentialsViaGit(ctx context.Context, request *GitCredentials) (*GitCreden return &creds, nil } -// Resolver adapts GetCredentials to download.CredentialResolver, letting the -// download package authenticate private assets without depending on this -// package (dependency inversion). type Resolver struct{} // Resolve implements download.CredentialResolver. diff --git a/pkg/gitcredentials/gitcredentials_test.go b/pkg/gitcredentials/gitcredentials_test.go index fb8e670f8..86cddeb08 100644 --- a/pkg/gitcredentials/gitcredentials_test.go +++ b/pkg/gitcredentials/gitcredentials_test.go @@ -43,10 +43,6 @@ func TestGetUser_WorkingDirResolvesIncludeIf(t *testing.T) { assert.Equal(t, "project@example.com", user.Email) } -// tempDirResolved returns a t.TempDir() with symlinks resolved. On macOS the -// temp dir lives under /var (a symlink to /private/var); git's `includeIf -// gitdir:` matches against the resolved path, so tests that build such patterns -// must use the resolved form or they silently fail to match. func tempDirResolved(t *testing.T) string { t.Helper() dir, err := filepath.EvalSymlinks(t.TempDir()) @@ -150,15 +146,10 @@ func TestSetUserEmptyIdentityIsNoOp(t *testing.T) { t.Setenv("XDG_CONFIG_HOME", "") t.Setenv("GIT_CONFIG_NOSYSTEM", "1") - // A named user with an empty identity must not attempt to chown a config - // file that was never written (regression: unconditional chown failed with - // ENOENT, aborting container credential/signing setup). require.NoError(t, SetUser(context.Background(), "", &GitUser{})) assert.NoFileExists(t, filepath.Join(tmpHome, ".gitconfig")) } -// helperTestEnv points git's global config at a temp HOME so ConfigureHelper / -// RemoveHelper operate on an isolated file. It returns the config path. func helperTestEnv(t *testing.T) string { t.Helper() tmpHome := t.TempDir() From 56ac939d62c26dbcd8e3a8ec8fec385a462a6b1b Mon Sep 17 00:00:00 2001 From: Samuel K Date: Sun, 5 Jul 2026 06:18:12 -0500 Subject: [PATCH 3/3] style: update GetUser docstring Signed-off-by: Samuel K --- pkg/git/config.go | 6 +- pkg/git/git.go | 11 +- pkg/git/git_test.go | 439 ++++++++++++++------------- pkg/git/installer_release.go | 16 +- pkg/gitcredentials/gitcredentials.go | 3 +- 5 files changed, 238 insertions(+), 237 deletions(-) diff --git a/pkg/git/config.go b/pkg/git/config.go index b1da3b8bb..b40b61fb0 100644 --- a/pkg/git/config.go +++ b/pkg/git/config.go @@ -15,9 +15,9 @@ type ConfigScope struct { var ( ScopeDefault = ConfigScope{} - ScopeLocal = ConfigScope{flag: "--local"} - ScopeGlobal = ConfigScope{flag: flagGlobal} - ScopeSystem = ConfigScope{flag: flagSystem} + ScopeLocal = ConfigScope{flag: "--local"} + ScopeGlobal = ConfigScope{flag: flagGlobal} + ScopeSystem = ConfigScope{flag: flagSystem} ) // ScopeFile targets a specific config file. diff --git a/pkg/git/git.go b/pkg/git/git.go index a812fa58a..1193dbd98 100644 --- a/pkg/git/git.go +++ b/pkg/git/git.go @@ -2,8 +2,6 @@ package git import ( "context" - "os" - "os/exec" "regexp" "strings" "time" @@ -15,7 +13,8 @@ const ( CommitDelimiter string = "@sha256:" PullRequestReference string = "pull/([0-9]+)/head" SubPathDelimiter string = "@subpath:" - repoBaseRegEx string = `((?:(?:https?|git|ssh|file):\/\/)?\/?(?:[^@\/\n]+@)?(?:[^:\/\n]+)(?:[:\/][^\/\n]+)+(?:\.git)?)` + repoBaseRegEx string = `((?:(?:https?|git|ssh|file):\/\/)?\/?(?:[^@\/\n]+@)?` + + `(?:[^:\/\n]+)(?:[:\/][^\/\n]+)+(?:\.git)?)` ) // WARN: Make sure this matches the regex in /desktop/src/views/Workspaces/CreateWorkspace/CreateWorkspaceInput.tsx! @@ -77,12 +76,6 @@ func canonicalizeURL(str string) string { return "https://" + str } -func CommandContext(ctx context.Context, extraEnv []string, args ...string) *exec.Cmd { - cmd := exec.CommandContext(ctx, "git", args...) - cmd.Env = append(os.Environ(), extraEnv...) - return cmd -} - func PingRepository(str string, extraEnv []string) bool { if !command.Exists("git") { return false diff --git a/pkg/git/git_test.go b/pkg/git/git_test.go index ad09c56f6..db73297ff 100644 --- a/pkg/git/git_test.go +++ b/pkg/git/git_test.go @@ -7,6 +7,15 @@ import ( "gotest.tools/assert/cmp" ) +const ( + repoDevsyHTTPS = "https://github.com/devsy-org/devsy.git" + repoDevsyNoProtoSlash = "https://github.com/devsy-org/devsy-without-protocol-with-slash.git" + repoFileProject = "file:///workspace/projects/project" + branchTestBranch = "test-branch" + testCommitSHA = "905ffb0" + testSubpathVal = "/test/path" +) + type testCaseNormalizeRepository struct { in string expectedPRReference string @@ -21,221 +30,221 @@ type testCaseGetBranchNameForPR struct { expectedBranch string } -func TestNormalizeRepository(t *testing.T) { - testCases := []testCaseNormalizeRepository{ - { - in: "ssh://github.com/devsy-org/devsy.git", - expectedRepo: "ssh://github.com/devsy-org/devsy.git", - expectedPRReference: "", - expectedBranch: "", - expectedCommit: "", - expectedSubpath: "", - }, - { - in: "ssh://git@github.com/devsy-org/devsy.git", - expectedRepo: "ssh://git@github.com/devsy-org/devsy.git", - expectedPRReference: "", - expectedBranch: "", - expectedCommit: "", - expectedSubpath: "", - }, - { - in: "git@github.com:devsy-org/devsy-without-branch.git", - expectedRepo: "git@github.com:devsy-org/devsy-without-branch.git", - expectedPRReference: "", - expectedBranch: "", - expectedCommit: "", - expectedSubpath: "", - }, - { - in: "https://github.com/devsy-org/devsy.git", - expectedRepo: "https://github.com/devsy-org/devsy.git", - expectedPRReference: "", - expectedBranch: "", - expectedCommit: "", - expectedSubpath: "", - }, - { - in: "github.com/devsy-org/devsy.git", - expectedRepo: "https://github.com/devsy-org/devsy.git", - expectedPRReference: "", - expectedBranch: "", - expectedCommit: "", - expectedSubpath: "", - }, - { - in: "github.com/devsy-org/devsy.git@test-branch", - expectedRepo: "https://github.com/devsy-org/devsy.git", - expectedPRReference: "", - expectedBranch: "test-branch", - expectedCommit: "", - expectedSubpath: "", - }, - { - in: "git@github.com:devsy-org/devsy-with-branch.git@test-branch", - expectedRepo: "git@github.com:devsy-org/devsy-with-branch.git", - expectedPRReference: "", - expectedBranch: "test-branch", - expectedCommit: "", - expectedSubpath: "", - }, - { - in: "git@github.com:devsy-org/devsy-with-branch.git@test_branch", - expectedRepo: "git@github.com:devsy-org/devsy-with-branch.git", - expectedPRReference: "", - expectedBranch: "test_branch", - expectedCommit: "", - expectedSubpath: "", - }, - { - in: "ssh://git@github.com:devsy-org/devsy.git@test_branch", - expectedRepo: "ssh://git@github.com:devsy-org/devsy.git", - expectedPRReference: "", - expectedBranch: "test_branch", - expectedCommit: "", - expectedSubpath: "", - }, - { - in: "github.com/devsy-org/devsy-without-protocol-with-slash.git@user/branch", - expectedRepo: "https://github.com/devsy-org/devsy-without-protocol-with-slash.git", - expectedPRReference: "", - expectedBranch: "user/branch", - expectedCommit: "", - expectedSubpath: "", - }, - { - in: "git@github.com:devsy-org/devsy-with-slash.git@user/branch", - expectedRepo: "git@github.com:devsy-org/devsy-with-slash.git", - expectedPRReference: "", - expectedBranch: "user/branch", - expectedCommit: "", - expectedSubpath: "", - }, - { - in: "github.com/devsy-org/devsy.git@sha256:905ffb0", - expectedRepo: "https://github.com/devsy-org/devsy.git", - expectedPRReference: "", - expectedBranch: "", - expectedCommit: "905ffb0", - expectedSubpath: "", - }, - { - in: "git@github.com:devsy-org/devsy.git@sha256:905ffb0", - expectedRepo: "git@github.com:devsy-org/devsy.git", - expectedPRReference: "", - expectedBranch: "", - expectedCommit: "905ffb0", - expectedSubpath: "", - }, - { - in: "github.com/devsy-org/devsy.git@pull/996/head", - expectedRepo: "https://github.com/devsy-org/devsy.git", - expectedPRReference: "pull/996/head", - expectedBranch: "", - expectedCommit: "", - expectedSubpath: "", - }, - { - in: "git@github.com:devsy-org/devsy.git@pull/996/head", - expectedRepo: "git@github.com:devsy-org/devsy.git", - expectedPRReference: "pull/996/head", - expectedBranch: "", - expectedCommit: "", - expectedSubpath: "", - }, - { - in: "github.com/devsy-org/devsy-without-protocol-with-slash.git@subpath:/test/path", - expectedRepo: "https://github.com/devsy-org/devsy-without-protocol-with-slash.git", - expectedPRReference: "", - expectedBranch: "", - expectedCommit: "", - expectedSubpath: "/test/path", - }, - { - in: "github.com/devsy-org/devsy-without-protocol-with-slash.git@subpath:/test/path/", - expectedRepo: "https://github.com/devsy-org/devsy-without-protocol-with-slash.git", - expectedPRReference: "", - expectedBranch: "", - expectedCommit: "", - expectedSubpath: "/test/path", - }, - { - in: "https://my_prefix@github.com/devsy-org/devsy.git@test-branch", - expectedRepo: "https://my_prefix@github.com/devsy-org/devsy.git", - expectedPRReference: "", - expectedBranch: "test-branch", - expectedCommit: "", - expectedSubpath: "", - }, - { - in: "https://test@dev.azure.com/org/project/_git/repo@dev", - expectedRepo: "https://test@dev.azure.com/org/project/_git/repo", - expectedPRReference: "", - expectedBranch: "dev", - expectedCommit: "", - expectedSubpath: "", - }, - { - in: "https://test@dev.azure.com/org/project/_git/repo@sha256:905ffb0", - expectedRepo: "https://test@dev.azure.com/org/project/_git/repo", - expectedPRReference: "", - expectedBranch: "", - expectedCommit: "905ffb0", - expectedSubpath: "", - }, - { - in: "git@ssh.dev.azure.com:v3/org/project/repo@dev", - expectedRepo: "git@ssh.dev.azure.com:v3/org/project/repo", - expectedPRReference: "", - expectedBranch: "dev", - expectedCommit: "", - expectedSubpath: "", - }, - { - in: "file:///workspace/projects/project", - expectedRepo: "file:///workspace/projects/project", - expectedPRReference: "", - expectedBranch: "", - expectedCommit: "", - expectedSubpath: "", - }, - { - in: "file:///workspace/projects/project@dev", - expectedRepo: "file:///workspace/projects/project", - expectedPRReference: "", - expectedBranch: "dev", - expectedCommit: "", - expectedSubpath: "", - }, - { - in: "file:///workspace/projects/project@sha256:905ffb0", - expectedRepo: "file:///workspace/projects/project", - expectedPRReference: "", - expectedBranch: "", - expectedCommit: "905ffb0", - expectedSubpath: "", - }, - { - in: "file:///workspace/projects/project@subpath:/test/path", - expectedRepo: "file:///workspace/projects/project", - expectedPRReference: "", - expectedBranch: "", - expectedCommit: "", - expectedSubpath: "/test/path", - }, - { - // WorkspaceSource.String emits "git:"; round-tripping that - // through NormalizeRepository must not produce "https://git:https://...". - in: "git:https://github.com/devsy-org/devsy.git", - expectedRepo: "https://github.com/devsy-org/devsy.git", - expectedPRReference: "", - expectedBranch: "", - expectedCommit: "", - expectedSubpath: "", - }, - } +var normalizeRepositoryCases = []testCaseNormalizeRepository{ + { + in: "ssh://github.com/devsy-org/devsy.git", + expectedRepo: "ssh://github.com/devsy-org/devsy.git", + expectedPRReference: "", + expectedBranch: "", + expectedCommit: "", + expectedSubpath: "", + }, + { + in: "ssh://git@github.com/devsy-org/devsy.git", + expectedRepo: "ssh://git@github.com/devsy-org/devsy.git", + expectedPRReference: "", + expectedBranch: "", + expectedCommit: "", + expectedSubpath: "", + }, + { + in: "git@github.com:devsy-org/devsy-without-branch.git", + expectedRepo: "git@github.com:devsy-org/devsy-without-branch.git", + expectedPRReference: "", + expectedBranch: "", + expectedCommit: "", + expectedSubpath: "", + }, + { + in: repoDevsyHTTPS, + expectedRepo: repoDevsyHTTPS, + expectedPRReference: "", + expectedBranch: "", + expectedCommit: "", + expectedSubpath: "", + }, + { + in: "github.com/devsy-org/devsy.git", + expectedRepo: repoDevsyHTTPS, + expectedPRReference: "", + expectedBranch: "", + expectedCommit: "", + expectedSubpath: "", + }, + { + in: "github.com/devsy-org/devsy.git@test-branch", + expectedRepo: repoDevsyHTTPS, + expectedPRReference: "", + expectedBranch: branchTestBranch, + expectedCommit: "", + expectedSubpath: "", + }, + { + in: "git@github.com:devsy-org/devsy-with-branch.git@test-branch", + expectedRepo: "git@github.com:devsy-org/devsy-with-branch.git", + expectedPRReference: "", + expectedBranch: branchTestBranch, + expectedCommit: "", + expectedSubpath: "", + }, + { + in: "git@github.com:devsy-org/devsy-with-branch.git@test_branch", + expectedRepo: "git@github.com:devsy-org/devsy-with-branch.git", + expectedPRReference: "", + expectedBranch: "test_branch", + expectedCommit: "", + expectedSubpath: "", + }, + { + in: "ssh://git@github.com:devsy-org/devsy.git@test_branch", + expectedRepo: "ssh://git@github.com:devsy-org/devsy.git", + expectedPRReference: "", + expectedBranch: "test_branch", + expectedCommit: "", + expectedSubpath: "", + }, + { + in: "github.com/devsy-org/devsy-without-protocol-with-slash.git@user/branch", + expectedRepo: repoDevsyNoProtoSlash, + expectedPRReference: "", + expectedBranch: "user/branch", + expectedCommit: "", + expectedSubpath: "", + }, + { + in: "git@github.com:devsy-org/devsy-with-slash.git@user/branch", + expectedRepo: "git@github.com:devsy-org/devsy-with-slash.git", + expectedPRReference: "", + expectedBranch: "user/branch", + expectedCommit: "", + expectedSubpath: "", + }, + { + in: "github.com/devsy-org/devsy.git@sha256:905ffb0", + expectedRepo: repoDevsyHTTPS, + expectedPRReference: "", + expectedBranch: "", + expectedCommit: testCommitSHA, + expectedSubpath: "", + }, + { + in: "git@github.com:devsy-org/devsy.git@sha256:905ffb0", + expectedRepo: "git@github.com:devsy-org/devsy.git", + expectedPRReference: "", + expectedBranch: "", + expectedCommit: testCommitSHA, + expectedSubpath: "", + }, + { + in: "github.com/devsy-org/devsy.git@pull/996/head", + expectedRepo: repoDevsyHTTPS, + expectedPRReference: testPRRef, + expectedBranch: "", + expectedCommit: "", + expectedSubpath: "", + }, + { + in: "git@github.com:devsy-org/devsy.git@pull/996/head", + expectedRepo: "git@github.com:devsy-org/devsy.git", + expectedPRReference: testPRRef, + expectedBranch: "", + expectedCommit: "", + expectedSubpath: "", + }, + { + in: "github.com/devsy-org/devsy-without-protocol-with-slash.git@subpath:/test/path", + expectedRepo: repoDevsyNoProtoSlash, + expectedPRReference: "", + expectedBranch: "", + expectedCommit: "", + expectedSubpath: testSubpathVal, + }, + { + in: "github.com/devsy-org/devsy-without-protocol-with-slash.git@subpath:/test/path/", + expectedRepo: repoDevsyNoProtoSlash, + expectedPRReference: "", + expectedBranch: "", + expectedCommit: "", + expectedSubpath: testSubpathVal, + }, + { + in: "https://my_prefix@github.com/devsy-org/devsy.git@test-branch", + expectedRepo: "https://my_prefix@github.com/devsy-org/devsy.git", + expectedPRReference: "", + expectedBranch: branchTestBranch, + expectedCommit: "", + expectedSubpath: "", + }, + { + in: "https://test@dev.azure.com/org/project/_git/repo@dev", + expectedRepo: "https://test@dev.azure.com/org/project/_git/repo", + expectedPRReference: "", + expectedBranch: testBranch, + expectedCommit: "", + expectedSubpath: "", + }, + { + in: "https://test@dev.azure.com/org/project/_git/repo@sha256:905ffb0", + expectedRepo: "https://test@dev.azure.com/org/project/_git/repo", + expectedPRReference: "", + expectedBranch: "", + expectedCommit: testCommitSHA, + expectedSubpath: "", + }, + { + in: "git@ssh.dev.azure.com:v3/org/project/repo@dev", + expectedRepo: "git@ssh.dev.azure.com:v3/org/project/repo", + expectedPRReference: "", + expectedBranch: testBranch, + expectedCommit: "", + expectedSubpath: "", + }, + { + in: repoFileProject, + expectedRepo: repoFileProject, + expectedPRReference: "", + expectedBranch: "", + expectedCommit: "", + expectedSubpath: "", + }, + { + in: "file:///workspace/projects/project@dev", + expectedRepo: repoFileProject, + expectedPRReference: "", + expectedBranch: testBranch, + expectedCommit: "", + expectedSubpath: "", + }, + { + in: "file:///workspace/projects/project@sha256:905ffb0", + expectedRepo: repoFileProject, + expectedPRReference: "", + expectedBranch: "", + expectedCommit: testCommitSHA, + expectedSubpath: "", + }, + { + in: "file:///workspace/projects/project@subpath:/test/path", + expectedRepo: repoFileProject, + expectedPRReference: "", + expectedBranch: "", + expectedCommit: "", + expectedSubpath: testSubpathVal, + }, + { + // WorkspaceSource.String emits "git:"; round-tripping that + // through NormalizeRepository must not produce "https://git:https://...". + in: "git:https://github.com/devsy-org/devsy.git", + expectedRepo: repoDevsyHTTPS, + expectedPRReference: "", + expectedBranch: "", + expectedCommit: "", + expectedSubpath: "", + }, +} - for _, testCase := range testCases { +func TestNormalizeRepository(t *testing.T) { + for _, testCase := range normalizeRepositoryCases { got := NormalizeRepository(testCase.in) assert.Check(t, cmp.Equal(testCase.expectedRepo, got.Repository)) assert.Check(t, cmp.Equal(testCase.expectedPRReference, got.PR)) @@ -248,7 +257,7 @@ func TestNormalizeRepository(t *testing.T) { func TestGetBranchNameForPRReference(t *testing.T) { testCases := []testCaseGetBranchNameForPR{ { - in: "pull/996/head", + in: testPRRef, expectedBranch: "PR996", }, { diff --git a/pkg/git/installer_release.go b/pkg/git/installer_release.go index 9258ad685..f0e331b3e 100644 --- a/pkg/git/installer_release.go +++ b/pkg/git/installer_release.go @@ -18,12 +18,12 @@ import ( ) type releaseSource struct { - version string - assetName func(goos, goarch, version string) (string, error) - downloadURL func(version, asset string) string - checksums map[string]string + version string + assetName func(goos, goarch, version string) (string, error) + downloadURL func(version, asset string) string + checksums map[string]string binaryInArchive string - installDir string + installDir string } var gitLFSRelease = releaseSource{ @@ -85,9 +85,9 @@ func (s *releaseSource) install(ctx context.Context, binary string) error { } req := fetchRequest{ - binary: binary, - url: s.downloadURL(s.version, asset), - wantSum: wantSum, + binary: binary, + url: s.downloadURL(s.version, asset), + wantSum: wantSum, execName: executableName(s.binaryInArchive, runtime.GOOS), } src, cleanup, err := s.fetchBinary(ctx, req) diff --git a/pkg/gitcredentials/gitcredentials.go b/pkg/gitcredentials/gitcredentials.go index 35293e2a5..62e10073d 100644 --- a/pkg/gitcredentials/gitcredentials.go +++ b/pkg/gitcredentials/gitcredentials.go @@ -168,8 +168,7 @@ func SetUser(ctx context.Context, userName string, user *GitUser) error { return nil } -// GetUser reads the git identity visible from workingDir, or the named user's -// global identity when workingDir is empty. +// GetUser retrieves the global git identity for the given OS user. func GetUser(ctx context.Context, userName, workingDir string) (*GitUser, error) { scope := git.ScopeGlobal if workingDir != "" {