From d613104617ecac10d2ef13c1ab420a372d9572db Mon Sep 17 00:00:00 2001 From: renatoathaydes Date: Thu, 22 Oct 2026 11:38:49 +0200 Subject: [PATCH 1/3] Give a workspace the git identity of its repository DevPod gave the dev container the identity from `git config --global`, which only reads ~/.gitconfig and doesn't follow its include or includeIf directives. A work identity kept in an includeIf block was ignored, so workspaces committed with the global (often personal) email. `devpod up` now resolves the identity where the workspace comes from (its local folder, or the directory `devpod up` runs in) with `git -C config`, which includes the repository's config and the matching includes, falling back to `git config --global --includes`. It saves the identity with the workspace when it has none yet, so later connections from any directory use the same one, and the tunnel server serves it to the container. The new `--git-user-name` and `--git-user-email` options of `devpod up` set it explicitly. The container still only fills in a name or email it doesn't have yet. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 12 +++ cmd/build.go | 1 + cmd/up.go | 5 + .../developing-in-workspaces/credentials.mdx | 17 +++ pkg/agent/tunnelserver/tunnelserver.go | 28 +++-- pkg/agent/tunnelserver/tunnelserver_test.go | 50 +++++++++ pkg/gitcredentials/gitcredentials.go | 31 +++++- pkg/gitcredentials/gituser_test.go | 85 +++++++++++++++ pkg/provider/workspace.go | 11 ++ pkg/workspace/gituser.go | 56 ++++++++++ pkg/workspace/gituser_test.go | 100 ++++++++++++++++++ pkg/workspace/workspace.go | 12 +++ 12 files changed, 398 insertions(+), 10 deletions(-) create mode 100644 pkg/gitcredentials/gituser_test.go create mode 100644 pkg/workspace/gituser.go create mode 100644 pkg/workspace/gituser_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 41b6b707d..a9141ae38 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,8 +7,20 @@ Add new entries to the top of the "Unreleased" section. ## Unreleased +### Added + +- `devpod up --git-user-name` and `--git-user-email` set the git identity that the workspace's dev container + gets, and save it with the workspace. + ### Fixed +- The git identity that DevPod gives a dev container is the one git uses where the workspace comes from (its local + folder, or the directory `devpod up` runs in), including the repository's config and matching `include` and + `includeIf` directives. DevPod used to read only `~/.gitconfig` without its includes, an issue inherited from + upstream, so a work identity kept in an `includeIf` block was ignored and workspaces committed with the + global (often personal) email. `devpod up` saves the identity with the workspace when it creates it, so later + connections from anywhere use the same one; workspaces created before get it on their next `devpod up`. + - `devpod up` of a Kubernetes workspace recreates the workspace's pod when it is dead (its phase is `Failed`, `Succeeded` or `Unknown`, its dev container has terminated, or it is being deleted), keeping the workspace's persistent volume claim. It used to reuse such a pod, for example after the node restarted, and then time out diff --git a/cmd/build.go b/cmd/build.go index 855f366ae..9c9dace35 100644 --- a/cmd/build.go +++ b/cmd/build.go @@ -89,6 +89,7 @@ func NewBuildCmd(flags *flags.GlobalFlags) *cobra.Command { cmd.DevContainerPath, sshConfigPath, nil, + nil, cmd.UID, false, log.Default, diff --git a/cmd/up.go b/cmd/up.go index 3c37aefbd..82efb455a 100644 --- a/cmd/up.go +++ b/cmd/up.go @@ -58,6 +58,8 @@ type UpCmd struct { SSHConfigPath string + GitUser provider2.WorkspaceGitUser + DotfilesSource string DotfilesScript string DotfilesScriptEnv []string // Key=Value to pass to install script @@ -123,6 +125,8 @@ func NewUpCmd(f *flags.GlobalFlags) *cobra.Command { upCmd.Flags().Var(&cmd.GitCloneStrategy, "git-clone-strategy", "The git clone strategy DevPod uses to checkout git based workspaces. Can be full (default), blobless, treeless or shallow") upCmd.Flags().BoolVar(&cmd.GitCloneRecursiveSubmodules, "git-clone-recursive-submodules", false, "If true will clone git submodule repositories recursively") upCmd.Flags().StringVar(&cmd.GitSSHSigningKey, "git-ssh-signing-key", "", "The ssh key to use when signing git commits. Used to explicitly setup DevPod's ssh signature forwarding with given key. Should be same format as value of `git config user.signingkey`") + upCmd.Flags().StringVar(&cmd.GitUser.Name, "git-user-name", "", "The git user.name to give the workspace's container. If empty, uses the one git uses in the directory DevPod runs in (or the workspace's local folder) when the workspace has none saved yet") + upCmd.Flags().StringVar(&cmd.GitUser.Email, "git-user-email", "", "The git user.email to give the workspace's container. If empty, uses the one git uses in the directory DevPod runs in (or the workspace's local folder) when the workspace has none saved yet") upCmd.Flags().StringVar(&cmd.FallbackImage, "fallback-image", "", "The fallback image to use if no devcontainer configuration has been detected") upCmd.Flags().BoolVar(&cmd.DisableDaemon, "disable-daemon", false, "If enabled, will not install a daemon into the target machine to track activity") upCmd.Flags().StringVar(&cmd.Source, "source", "", "Optional source for the workspace. E.g. git:https://github.com/my-org/my-repo") @@ -1095,6 +1099,7 @@ func (cmd *UpCmd) prepareClient(ctx context.Context, devPodConfig *config.Config cmd.DevContainerPath, cmd.SSHConfigPath, source, + &cmd.GitUser, cmd.UID, true, logger, diff --git a/docs/pages/developing-in-workspaces/credentials.mdx b/docs/pages/developing-in-workspaces/credentials.mdx index 0cfea03c9..3794d1b63 100644 --- a/docs/pages/developing-in-workspaces/credentials.mdx +++ b/docs/pages/developing-in-workspaces/credentials.mdx @@ -20,6 +20,23 @@ Both only work while you are connected to the workspace, because the credentials workspaces that must keep working without you, such as ones running AI agents, see [Run Workspaces Unattended](./unattended-workspaces.mdx). +## Git identity + +DevPod gives the dev container your git `user.name` and `user.email`, unless the container already has them (for +example from its own start command). `devpod up` saves the identity with the workspace when it creates it: the +one git uses in the workspace's local folder or, for other sources, in the directory you run `devpod up` from. That +includes the repository's config and the `include` and `includeIf` directives that match it, so running +`devpod up` from inside a work repository gives the workspace your work identity. Outside a repository it's your +global identity. + +Running `devpod up` again doesn't change the saved identity. To set it explicitly, pass it to `devpod up`: + +``` +devpod up --git-user-email me+devpod@company.com --git-user-name "My Name" my-workspace +``` + +A dev container that already has an identity keeps it; use `devpod up --recreate` to give it the new one. + ## Docker credentials DevPod will make docker registry credentials available inside the dev container through a [docker credentials helper](https://docs.docker.com/engine/reference/commandline/login/#credential-helpers). This allows you to pull and push images from and to private registries from within the dev container. diff --git a/pkg/agent/tunnelserver/tunnelserver.go b/pkg/agent/tunnelserver/tunnelserver.go index 3e7939397..da6a76196 100644 --- a/pkg/agent/tunnelserver/tunnelserver.go +++ b/pkg/agent/tunnelserver/tunnelserver.go @@ -186,12 +186,7 @@ func (t *tunnelServer) DockerCredentials(ctx context.Context, message *tunnel.Me } func (t *tunnelServer) GitUser(ctx context.Context, empty *tunnel.Empty) (*tunnel.Message, error) { - gitUser, err := gitcredentials.GetUser("") - if err != nil { - return nil, err - } - - out, err := json.Marshal(gitUser) + out, err := json.Marshal(t.gitUser()) if err != nil { return nil, err } @@ -201,6 +196,27 @@ func (t *tunnelServer) GitUser(ctx context.Context, empty *tunnel.Empty) (*tunne }, nil } +// gitUser returns the identity `devpod up` saved with the workspace. A value it couldn't +// find, or a workspace saved before DevPod kept the identity, gets the global one. +func (t *tunnelServer) gitUser() *gitcredentials.GitUser { + gitUser := &gitcredentials.GitUser{} + if t.workspace != nil { + gitUser.Name = t.workspace.GitUser.Name + gitUser.Email = t.workspace.GitUser.Email + } + if gitUser.Name == "" || gitUser.Email == "" { + hostUser := gitcredentials.GetHostUser("") + if gitUser.Name == "" { + gitUser.Name = hostUser.Name + } + if gitUser.Email == "" { + gitUser.Email = hostUser.Email + } + } + + return gitUser +} + func (t *tunnelServer) GitCredentials(ctx context.Context, message *tunnel.Message) (*tunnel.Message, error) { if !t.allowGitCredentials { return nil, fmt.Errorf("git credentials forbidden") diff --git a/pkg/agent/tunnelserver/tunnelserver_test.go b/pkg/agent/tunnelserver/tunnelserver_test.go index 52f308ed7..6d0aaef0e 100644 --- a/pkg/agent/tunnelserver/tunnelserver_test.go +++ b/pkg/agent/tunnelserver/tunnelserver_test.go @@ -2,10 +2,14 @@ package tunnelserver import ( "context" + "os" + "path/filepath" "testing" "github.com/loft-sh/devpod/pkg/agent/tunnel" "github.com/loft-sh/devpod/pkg/log" + provider2 "github.com/loft-sh/devpod/pkg/provider" + "gotest.tools/assert" ) // TestGitCredentials_NoWorkspace checks that a git credentials request to a @@ -30,3 +34,49 @@ func TestGitCredentials_NoWorkspace(t *testing.T) { // a panic is not. _, _ = s.GitCredentials(context.Background(), request) } + +// TestGitUser checks that the container gets the identity saved with the +// workspace, and the global one for what the workspace doesn't have. +func TestGitUser(t *testing.T) { + home := t.TempDir() + globalConfig := filepath.Join(home, ".gitconfig") + err := os.WriteFile(globalConfig, []byte("[user]\n\tname = Global\n\temail = global@personal.com\n"), 0600) + assert.NilError(t, err) + t.Setenv("HOME", home) + t.Setenv("XDG_CONFIG_HOME", filepath.Join(home, ".config")) + t.Setenv("GIT_CONFIG_GLOBAL", globalConfig) + t.Setenv("GIT_CONFIG_NOSYSTEM", "1") + + testCases := []struct { + name string + workspace *provider2.Workspace + expected string + }{ + { + name: "saved identity", + workspace: &provider2.Workspace{GitUser: provider2.WorkspaceGitUser{Name: "Work", Email: "me@company.com"}}, + expected: `{"name":"Work","email":"me@company.com"}`, + }, + { + name: "partly saved identity", + workspace: &provider2.Workspace{GitUser: provider2.WorkspaceGitUser{Email: "me@company.com"}}, + expected: `{"name":"Global","email":"me@company.com"}`, + }, + { + name: "workspace saved before DevPod kept the identity", + workspace: &provider2.Workspace{}, + expected: `{"name":"Global","email":"global@personal.com"}`, + }, + { + name: "no workspace", + expected: `{"name":"Global","email":"global@personal.com"}`, + }, + } + + for _, testCase := range testCases { + s := New(log.Discard, WithWorkspace(testCase.workspace)) + response, err := s.GitUser(context.Background(), &tunnel.Empty{}) + assert.NilError(t, err, testCase.name) + assert.Equal(t, response.Message, testCase.expected, testCase.name) + } +} diff --git a/pkg/gitcredentials/gitcredentials.go b/pkg/gitcredentials/gitcredentials.go index 5424a3f7c..2b4a336a9 100644 --- a/pkg/gitcredentials/gitcredentials.go +++ b/pkg/gitcredentials/gitcredentials.go @@ -190,14 +190,37 @@ func GetUser(userName string) (*GitUser, error) { scopeArgs = append(scopeArgs, "--global") } - // we ignore the error here, because if email is empty we don't care - name, _ := exec.Command("git", append(scopeArgs[:], "user.name")...).Output() + // --file and --global name a single file, so git only follows its includes with --includes + scopeArgs = append(scopeArgs, "--includes") + + return readUser(scopeArgs), nil +} + +// GetHostUser returns the git identity that git uses in dir on this machine. That includes the +// repository's config and the include and includeIf directives that match it; outside a +// repository it is the system and global config. If dir is empty or git can't run there, it +// falls back to the global config and its includes. +func GetHostUser(dir string) *GitUser { + if dir != "" { + if info, err := os.Stat(dir); err == nil && info.IsDir() { + return readUser([]string{"-C", dir, "config"}) + } + } + + return readUser([]string{"config", "--global", "--includes"}) +} + +func readUser(configArgs []string) *GitUser { + gitUser := &GitUser{} + + // we ignore the error here, because git fails if the value isn't set + name, _ := exec.Command("git", append(configArgs, "user.name")...).Output() gitUser.Name = strings.TrimSpace(string(name)) - email, _ := exec.Command("git", append(scopeArgs[:], "user.email")...).Output() + email, _ := exec.Command("git", append(configArgs, "user.email")...).Output() gitUser.Email = strings.TrimSpace(string(email)) - return gitUser, nil + return gitUser } func GetCredentials(requestObj *GitCredentials) (*GitCredentials, error) { diff --git a/pkg/gitcredentials/gituser_test.go b/pkg/gitcredentials/gituser_test.go new file mode 100644 index 000000000..b0657ea45 --- /dev/null +++ b/pkg/gitcredentials/gituser_test.go @@ -0,0 +1,85 @@ +package gitcredentials + +import ( + "os" + "os/exec" + "path/filepath" + "testing" + + "gotest.tools/assert" +) + +// setupIncludeIfConfig gives git a global config with a personal identity and an includeIf that +// gives repositories under the returned work directory a work email, as in the bug report. +func setupIncludeIfConfig(t *testing.T) (workDir string) { + t.Helper() + + // resolve symlinks (such as macOS's /var) so the gitdir pattern matches + root, err := filepath.EvalSymlinks(t.TempDir()) + assert.NilError(t, err) + home := filepath.Join(root, "home") + workDir = filepath.Join(home, "work") + assert.NilError(t, os.MkdirAll(workDir, 0755)) + + globalConfig := filepath.Join(home, ".gitconfig") + writeFile(t, globalConfig, `[user] + name = Me + email = me@personal.com +[includeIf "gitdir:`+workDir+`/"] + path = `+filepath.Join(workDir, ".gitconfig")+"\n") + writeFile(t, filepath.Join(workDir, ".gitconfig"), "[user]\n\temail = me@company.com\n") + + t.Setenv("HOME", home) + t.Setenv("XDG_CONFIG_HOME", filepath.Join(home, ".config")) + t.Setenv("GIT_CONFIG_GLOBAL", globalConfig) + t.Setenv("GIT_CONFIG_NOSYSTEM", "1") + + return workDir +} + +func writeFile(t *testing.T, path, content string) { + t.Helper() + assert.NilError(t, os.WriteFile(path, []byte(content), 0600)) +} + +func gitInit(t *testing.T, dir string) { + t.Helper() + out, err := exec.Command("git", "init", "--quiet", dir).CombinedOutput() + assert.NilError(t, err, string(out)) +} + +func TestGetHostUser_FollowsIncludeIfOfRepository(t *testing.T) { + repo := filepath.Join(setupIncludeIfConfig(t), "repo") + gitInit(t, repo) + + assert.DeepEqual(t, GetHostUser(repo), &GitUser{Name: "Me", Email: "me@company.com"}) + // a subdirectory of the repository is in it too + subDir := filepath.Join(repo, "sub") + assert.NilError(t, os.Mkdir(subDir, 0755)) + assert.DeepEqual(t, GetHostUser(subDir), &GitUser{Name: "Me", Email: "me@company.com"}) +} + +func TestGetHostUser_UsesRepositoryConfig(t *testing.T) { + repo := filepath.Join(setupIncludeIfConfig(t), "..", "other") + gitInit(t, repo) + out, err := exec.Command("git", "-C", repo, "config", "user.email", "me@repo.com").CombinedOutput() + assert.NilError(t, err, string(out)) + + assert.DeepEqual(t, GetHostUser(repo), &GitUser{Name: "Me", Email: "me@repo.com"}) +} + +func TestGetHostUser_OutsideRepositoryUsesGlobalConfig(t *testing.T) { + workDir := setupIncludeIfConfig(t) + + // the includeIf only matches repositories + assert.DeepEqual(t, GetHostUser(workDir), &GitUser{Name: "Me", Email: "me@personal.com"}) + assert.DeepEqual(t, GetHostUser(filepath.Join(workDir, "missing")), &GitUser{Name: "Me", Email: "me@personal.com"}) + assert.DeepEqual(t, GetHostUser(""), &GitUser{Name: "Me", Email: "me@personal.com"}) +} + +func TestGetHostUser_GlobalFollowsIncludes(t *testing.T) { + workDir := setupIncludeIfConfig(t) + writeFile(t, os.Getenv("GIT_CONFIG_GLOBAL"), "[include]\n\tpath = "+filepath.Join(workDir, ".gitconfig")+"\n") + + assert.DeepEqual(t, GetHostUser(""), &GitUser{Email: "me@company.com"}) +} diff --git a/pkg/provider/workspace.go b/pkg/provider/workspace.go index 3356d48f5..ccefdb03a 100644 --- a/pkg/provider/workspace.go +++ b/pkg/provider/workspace.go @@ -49,6 +49,9 @@ type Workspace struct { // DevContainerConfig holds the config for the devcontainer.json. DevContainerConfig *devcontainerconfig.DevContainerConfig `json:"devContainerConfig,omitempty"` + // GitUser is the git identity to give the dev container, if it doesn't have one yet + GitUser WorkspaceGitUser `json:"gitUser,omitempty"` + // CreationTimestamp is the timestamp when this workspace was created CreationTimestamp types.Time `json:"creationTimestamp,omitempty"` @@ -73,6 +76,14 @@ type WorkspaceIDEConfig struct { Options map[string]config.OptionValue `json:"options,omitempty"` } +type WorkspaceGitUser struct { + // Name is the git user.name + Name string `json:"name,omitempty"` + + // Email is the git user.email + Email string `json:"email,omitempty"` +} + type WorkspaceMachineConfig struct { // ID is the machine ID to use for this workspace ID string `json:"machineId,omitempty"` diff --git a/pkg/workspace/gituser.go b/pkg/workspace/gituser.go new file mode 100644 index 000000000..64c3671f4 --- /dev/null +++ b/pkg/workspace/gituser.go @@ -0,0 +1,56 @@ +package workspace + +import ( + "fmt" + + "github.com/loft-sh/devpod/pkg/gitcredentials" + "github.com/loft-sh/devpod/pkg/log" + providerpkg "github.com/loft-sh/devpod/pkg/provider" +) + +// ConfigureGitUser saves the git identity that the workspace's container gets if it doesn't have +// one yet. A name or email in gitUser replaces the saved one. One that is neither given nor saved +// is the one git uses where the workspace comes from: its local folder, or else dir, the +// directory `devpod up` runs in. That way the repository's config and its includeIf matches +// count, and running `devpod up` again from somewhere else doesn't change the identity. +func ConfigureGitUser(workspace *providerpkg.Workspace, gitUser providerpkg.WorkspaceGitUser, dir string, log log.Logger) error { + if workspace.Source.LocalFolder != "" { + dir = workspace.Source.LocalFolder + } + + newGitUser := resolveGitUser(workspace.GitUser, gitUser, dir) + if newGitUser == workspace.GitUser { + return nil + } + + log.Debugf("Use git identity '%s <%s>' for workspace %s", newGitUser.Name, newGitUser.Email, workspace.ID) + workspace.GitUser = newGitUser + err := providerpkg.SaveWorkspaceConfig(workspace) + if err != nil { + return fmt.Errorf("save workspace: %w", err) + } + + return nil +} + +func resolveGitUser(saved, given providerpkg.WorkspaceGitUser, dir string) providerpkg.WorkspaceGitUser { + gitUser := saved + if given.Name != "" { + gitUser.Name = given.Name + } + if given.Email != "" { + gitUser.Email = given.Email + } + + if gitUser.Name == "" || gitUser.Email == "" { + hostUser := gitcredentials.GetHostUser(dir) + if gitUser.Name == "" { + gitUser.Name = hostUser.Name + } + if gitUser.Email == "" { + gitUser.Email = hostUser.Email + } + } + + return gitUser +} diff --git a/pkg/workspace/gituser_test.go b/pkg/workspace/gituser_test.go new file mode 100644 index 000000000..e7b07801f --- /dev/null +++ b/pkg/workspace/gituser_test.go @@ -0,0 +1,100 @@ +package workspace + +import ( + "fmt" + "os" + "os/exec" + "path/filepath" + "testing" + + "github.com/loft-sh/devpod/pkg/config" + "github.com/loft-sh/devpod/pkg/log" + providerpkg "github.com/loft-sh/devpod/pkg/provider" + "gotest.tools/assert" +) + +// setupGitConfig gives git a global identity and returns a repository with its own email. +func setupGitConfig(t *testing.T) (repo string) { + t.Helper() + + root := t.TempDir() + globalConfig := filepath.Join(root, ".gitconfig") + assert.NilError(t, os.WriteFile(globalConfig, []byte("[user]\n\tname = Me\n\temail = me@personal.com\n"), 0600)) + t.Setenv("HOME", root) + t.Setenv("XDG_CONFIG_HOME", filepath.Join(root, ".config")) + t.Setenv("GIT_CONFIG_GLOBAL", globalConfig) + t.Setenv("GIT_CONFIG_NOSYSTEM", "1") + t.Setenv(config.DEVPOD_HOME, filepath.Join(root, ".devpod")) + + repo = filepath.Join(root, "repo") + out, err := exec.Command("git", "init", "--quiet", repo).CombinedOutput() + assert.NilError(t, err, string(out)) + out, err = exec.Command("git", "-C", repo, "config", "user.email", "me@company.com").CombinedOutput() + assert.NilError(t, err, string(out)) + + return repo +} + +func TestConfigureGitUser(t *testing.T) { + repo := setupGitConfig(t) + + testCases := []struct { + name string + workspace providerpkg.Workspace + given providerpkg.WorkspaceGitUser + dir string + expected providerpkg.WorkspaceGitUser + }{ + { + name: "new workspace gets the identity of the directory devpod runs in", + dir: repo, + expected: providerpkg.WorkspaceGitUser{Name: "Me", Email: "me@company.com"}, + }, + { + name: "a local folder wins over the directory devpod runs in", + workspace: providerpkg.Workspace{Source: providerpkg.WorkspaceSource{LocalFolder: repo}}, + expected: providerpkg.WorkspaceGitUser{Name: "Me", Email: "me@company.com"}, + }, + { + name: "outside a repository the global identity is used", + dir: t.TempDir(), + expected: providerpkg.WorkspaceGitUser{Name: "Me", Email: "me@personal.com"}, + }, + { + name: "the saved identity is kept", + workspace: providerpkg.Workspace{GitUser: providerpkg.WorkspaceGitUser{Name: "Saved", Email: "saved@company.com"}}, + dir: repo, + expected: providerpkg.WorkspaceGitUser{Name: "Saved", Email: "saved@company.com"}, + }, + { + name: "the given identity replaces the saved one", + workspace: providerpkg.Workspace{GitUser: providerpkg.WorkspaceGitUser{Name: "Saved", Email: "saved@company.com"}}, + given: providerpkg.WorkspaceGitUser{Email: "me+devpod@company.com"}, + dir: repo, + expected: providerpkg.WorkspaceGitUser{Name: "Saved", Email: "me+devpod@company.com"}, + }, + { + name: "a value that isn't given is resolved", + given: providerpkg.WorkspaceGitUser{Name: "Given"}, + dir: repo, + expected: providerpkg.WorkspaceGitUser{Name: "Given", Email: "me@company.com"}, + }, + } + + for i, testCase := range testCases { + workspace := testCase.workspace + workspace.ID = fmt.Sprintf("test-%d", i) + workspace.Context = "default" + assert.NilError(t, ConfigureGitUser(&workspace, testCase.given, testCase.dir, log.Discard), testCase.name) + assert.Equal(t, workspace.GitUser, testCase.expected, testCase.name) + + saved, err := providerpkg.LoadWorkspaceConfig(workspace.Context, workspace.ID) + if testCase.expected == testCase.workspace.GitUser { + // nothing changed, so nothing was saved + assert.Assert(t, err != nil, testCase.name) + } else { + assert.NilError(t, err, testCase.name) + assert.Equal(t, saved.GitUser, testCase.expected, testCase.name) + } + } +} diff --git a/pkg/workspace/workspace.go b/pkg/workspace/workspace.go index 8f540d350..a70a3ddc1 100644 --- a/pkg/workspace/workspace.go +++ b/pkg/workspace/workspace.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "os" "sort" "strings" @@ -36,6 +37,7 @@ func Resolve( devContainerPath string, sshConfigPath string, source *providerpkg.WorkspaceSource, + gitUser *providerpkg.WorkspaceGitUser, uid string, changeLastUsed bool, log log.Logger, @@ -101,6 +103,16 @@ func Resolve( } } + // configure the git identity of the container, unless the caller doesn't start one + if gitUser != nil { + // an empty directory makes DevPod use the global git identity + cwd, _ := os.Getwd() + err = ConfigureGitUser(workspace, *gitUser, cwd, log) + if err != nil { + return nil, fmt.Errorf("configure git user: %w", err) + } + } + // create workspace client client, err := getWorkspaceClient(devPodConfig, provider, workspace, machine, log) if err != nil { From a79305fe02554f65a944f0c6288f0a63de86f4c0 Mon Sep 17 00:00:00 2001 From: renatoathaydes Date: Thu, 22 Oct 2026 11:45:16 +0200 Subject: [PATCH 2/3] Fix git identity tests on Windows The tests wrote Windows paths into git config files, where git takes a backslash as an escape character and rejected the file. Write them with forward slashes, which git accepts and gitdir patterns need, and match the gitdir case insensitively, like Windows paths. Co-Authored-By: Claude Opus 5.5 --- pkg/gitcredentials/gituser_test.go | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/pkg/gitcredentials/gituser_test.go b/pkg/gitcredentials/gituser_test.go index b0657ea45..65f938e64 100644 --- a/pkg/gitcredentials/gituser_test.go +++ b/pkg/gitcredentials/gituser_test.go @@ -21,12 +21,13 @@ func setupIncludeIfConfig(t *testing.T) (workDir string) { workDir = filepath.Join(home, "work") assert.NilError(t, os.MkdirAll(workDir, 0755)) + // gitdir/i because Windows paths are case insensitive globalConfig := filepath.Join(home, ".gitconfig") writeFile(t, globalConfig, `[user] name = Me email = me@personal.com -[includeIf "gitdir:`+workDir+`/"] - path = `+filepath.Join(workDir, ".gitconfig")+"\n") +[includeIf "gitdir/i:`+configPath(workDir)+`/"] + path = `+configPath(filepath.Join(workDir, ".gitconfig"))+"\n") writeFile(t, filepath.Join(workDir, ".gitconfig"), "[user]\n\temail = me@company.com\n") t.Setenv("HOME", home) @@ -37,6 +38,12 @@ func setupIncludeIfConfig(t *testing.T) (workDir string) { return workDir } +// configPath writes path for a git config file: git takes a backslash as an escape character, and +// gitdir patterns use forward slashes on Windows too. +func configPath(path string) string { + return filepath.ToSlash(path) +} + func writeFile(t *testing.T, path, content string) { t.Helper() assert.NilError(t, os.WriteFile(path, []byte(content), 0600)) @@ -79,7 +86,7 @@ func TestGetHostUser_OutsideRepositoryUsesGlobalConfig(t *testing.T) { func TestGetHostUser_GlobalFollowsIncludes(t *testing.T) { workDir := setupIncludeIfConfig(t) - writeFile(t, os.Getenv("GIT_CONFIG_GLOBAL"), "[include]\n\tpath = "+filepath.Join(workDir, ".gitconfig")+"\n") + writeFile(t, os.Getenv("GIT_CONFIG_GLOBAL"), "[include]\n\tpath = "+configPath(filepath.Join(workDir, ".gitconfig"))+"\n") assert.DeepEqual(t, GetHostUser(""), &GitUser{Email: "me@company.com"}) } From 510101bfaef80f0d708f9307843a9678cf5d2a9b Mon Sep 17 00:00:00 2001 From: renatoathaydes Date: Thu, 22 Oct 2026 12:23:46 +0200 Subject: [PATCH 3/3] Only save a git identity that was given or found in a repository Address review findings on the git identity change: - `devpod up` saved whatever identity it found, also outside any repository (e.g. from the desktop app), and never looked it up again, so a workspace could keep a personal or outdated identity for good. It now only saves a value given with --git-user-name/--git-user-email or found in the workspace's repository. Otherwise the container gets the current global identity when it connects. An empty flag value removes the saved one. - Check that the directory is in a repository git can use with `git rev-parse`, and log why not at debug level, so a repository git doesn't trust (safe.directory) no longer silently gives the global identity. - SetUser ran `git config` through a shell with the value in double quotes, so a name with a quote or `$` broke the container's credentials server. It now runs git directly on the same config file GetUser reads. - `devpod up` passes the directory it runs in to Resolve instead of Resolve reading the process's working directory. - Keep the tests from finding a repository that the temporary directory is in, and cover the new cases. - Docs and CHANGELOG: existing containers keep their identity until `devpod up --recreate`, the container only checks the user's ~/.gitconfig, and safe.directory. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 17 ++-- cmd/up.go | 17 +++- .../developing-in-workspaces/credentials.mdx | 25 ++++-- pkg/agent/tunnelserver/tunnelserver.go | 10 +-- pkg/agent/tunnelserver/tunnelserver_test.go | 2 +- pkg/gitcredentials/gitcredentials.go | 88 +++++++++---------- pkg/gitcredentials/gituser_test.go | 61 ++++++++++--- pkg/workspace/gituser.go | 44 +++++++--- pkg/workspace/gituser_test.go | 69 ++++++++++----- pkg/workspace/workspace.go | 9 +- 10 files changed, 219 insertions(+), 123 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a9141ae38..a38877f9e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,17 +10,18 @@ Add new entries to the top of the "Unreleased" section. ### Added - `devpod up --git-user-name` and `--git-user-email` set the git identity that the workspace's dev container - gets, and save it with the workspace. + gets, and save it with the workspace. An empty value removes the saved one. ### Fixed -- The git identity that DevPod gives a dev container is the one git uses where the workspace comes from (its local - folder, or the directory `devpod up` runs in), including the repository's config and matching `include` and - `includeIf` directives. DevPod used to read only `~/.gitconfig` without its includes, an issue inherited from - upstream, so a work identity kept in an `includeIf` block was ignored and workspaces committed with the - global (often personal) email. `devpod up` saves the identity with the workspace when it creates it, so later - connections from anywhere use the same one; workspaces created before get it on their next `devpod up`. - +- The git identity that DevPod gives a dev container is the one git uses in the repository the workspace comes + from (its local folder, or the directory `devpod up` runs in), including the repository's config and matching + `include` and `includeIf` directives. DevPod used to read only `~/.gitconfig` without its includes, an issue + inherited from upstream, so a work identity kept in an `includeIf` block was ignored and workspaces committed + with the global (often personal) email. `devpod up` saves the repository's identity with the workspace, so + later connections from anywhere use the same one; outside a repository it saves nothing, and the container + gets the global identity (now with its includes) when it connects. A dev container that already has an + identity keeps it, including those of existing workspaces: use `devpod up --recreate` to give it the new one. - `devpod up` of a Kubernetes workspace recreates the workspace's pod when it is dead (its phase is `Failed`, `Succeeded` or `Unknown`, its dev container has terminated, or it is being deleted), keeping the workspace's persistent volume claim. It used to reuse such a pod, for example after the node restarted, and then time out diff --git a/cmd/up.go b/cmd/up.go index 82efb455a..3f0a3695c 100644 --- a/cmd/up.go +++ b/cmd/up.go @@ -58,7 +58,9 @@ type UpCmd struct { SSHConfigPath string - GitUser provider2.WorkspaceGitUser + GitUserName string + GitUserEmail string + GitUser workspace2.GitUserOptions DotfilesSource string DotfilesScript string @@ -87,6 +89,15 @@ func NewUpCmd(f *flags.GlobalFlags) *cobra.Command { ctx, cancel := WithSignals(cobraCmd.Context()) defer cancel() + // an empty directory makes DevPod use the global git identity + cmd.GitUser.Dir, _ = os.Getwd() + if cobraCmd.Flags().Changed("git-user-name") { + cmd.GitUser.Name = &cmd.GitUserName + } + if cobraCmd.Flags().Changed("git-user-email") { + cmd.GitUser.Email = &cmd.GitUserEmail + } + client, logger, err := cmd.prepareClient(ctx, devPodConfig, args) if err != nil { return fmt.Errorf("prepare workspace client: %w", err) @@ -125,8 +136,8 @@ func NewUpCmd(f *flags.GlobalFlags) *cobra.Command { upCmd.Flags().Var(&cmd.GitCloneStrategy, "git-clone-strategy", "The git clone strategy DevPod uses to checkout git based workspaces. Can be full (default), blobless, treeless or shallow") upCmd.Flags().BoolVar(&cmd.GitCloneRecursiveSubmodules, "git-clone-recursive-submodules", false, "If true will clone git submodule repositories recursively") upCmd.Flags().StringVar(&cmd.GitSSHSigningKey, "git-ssh-signing-key", "", "The ssh key to use when signing git commits. Used to explicitly setup DevPod's ssh signature forwarding with given key. Should be same format as value of `git config user.signingkey`") - upCmd.Flags().StringVar(&cmd.GitUser.Name, "git-user-name", "", "The git user.name to give the workspace's container. If empty, uses the one git uses in the directory DevPod runs in (or the workspace's local folder) when the workspace has none saved yet") - upCmd.Flags().StringVar(&cmd.GitUser.Email, "git-user-email", "", "The git user.email to give the workspace's container. If empty, uses the one git uses in the directory DevPod runs in (or the workspace's local folder) when the workspace has none saved yet") + upCmd.Flags().StringVar(&cmd.GitUserName, "git-user-name", "", "The git user.name to give the workspace's container, saved with the workspace. An empty value removes the saved one. Without one, DevPod saves the one git uses in the workspace's repository: its local folder, or the directory DevPod runs in") + upCmd.Flags().StringVar(&cmd.GitUserEmail, "git-user-email", "", "The git user.email to give the workspace's container, saved with the workspace. An empty value removes the saved one. Without one, DevPod saves the one git uses in the workspace's repository: its local folder, or the directory DevPod runs in") upCmd.Flags().StringVar(&cmd.FallbackImage, "fallback-image", "", "The fallback image to use if no devcontainer configuration has been detected") upCmd.Flags().BoolVar(&cmd.DisableDaemon, "disable-daemon", false, "If enabled, will not install a daemon into the target machine to track activity") upCmd.Flags().StringVar(&cmd.Source, "source", "", "Optional source for the workspace. E.g. git:https://github.com/my-org/my-repo") diff --git a/docs/pages/developing-in-workspaces/credentials.mdx b/docs/pages/developing-in-workspaces/credentials.mdx index 3794d1b63..443a32b67 100644 --- a/docs/pages/developing-in-workspaces/credentials.mdx +++ b/docs/pages/developing-in-workspaces/credentials.mdx @@ -22,20 +22,29 @@ workspaces that must keep working without you, such as ones running AI agents, s ## Git identity -DevPod gives the dev container your git `user.name` and `user.email`, unless the container already has them (for -example from its own start command). `devpod up` saves the identity with the workspace when it creates it: the -one git uses in the workspace's local folder or, for other sources, in the directory you run `devpod up` from. That -includes the repository's config and the `include` and `includeIf` directives that match it, so running -`devpod up` from inside a work repository gives the workspace your work identity. Outside a repository it's your -global identity. +DevPod gives the dev container your git `user.name` and `user.email`, unless the container user's `~/.gitconfig` +already has them (for example from the container's own start command). It uses the identity git uses in the +repository the workspace comes from: the workspace's local folder or, for other sources, the directory you run +`devpod up` from. That includes the repository's config and the `include` and `includeIf` directives that match +it, so running `devpod up` from inside a work repository gives the workspace your work identity. -Running `devpod up` again doesn't change the saved identity. To set it explicitly, pass it to `devpod up`: +`devpod up` saves that identity with the workspace, and running it again, from anywhere, doesn't change it. If +`devpod up` doesn't run in a repository (for example from the desktop app, for a git source), it saves nothing, +and the container gets your global identity, following the global config's includes, when it connects. + +Git ignores a repository it doesn't trust, for example one on a network share or owned by another user, and then +DevPod can't use its identity. Add it to git's +[`safe.directory`](https://git-scm.com/docs/git-config#Documentation/git-config.txt-safedirectory) to fix that. + +To set the identity explicitly, pass it to `devpod up`. An empty value removes the saved one, so that DevPod looks +it up again: ``` devpod up --git-user-email me+devpod@company.com --git-user-name "My Name" my-workspace ``` -A dev container that already has an identity keeps it; use `devpod up --recreate` to give it the new one. +A dev container that already has an identity keeps it, and so do the dev containers of workspaces created before +DevPod saved the identity. Use `devpod up --recreate` to give the container the new one. ## Docker credentials diff --git a/pkg/agent/tunnelserver/tunnelserver.go b/pkg/agent/tunnelserver/tunnelserver.go index da6a76196..274d08838 100644 --- a/pkg/agent/tunnelserver/tunnelserver.go +++ b/pkg/agent/tunnelserver/tunnelserver.go @@ -196,8 +196,8 @@ func (t *tunnelServer) GitUser(ctx context.Context, empty *tunnel.Empty) (*tunne }, nil } -// gitUser returns the identity `devpod up` saved with the workspace. A value it couldn't -// find, or a workspace saved before DevPod kept the identity, gets the global one. +// gitUser returns the identity `devpod up` saved with the workspace. A value it didn't save, +// because it wasn't given and the workspace doesn't come from a repository, is the global one. func (t *tunnelServer) gitUser() *gitcredentials.GitUser { gitUser := &gitcredentials.GitUser{} if t.workspace != nil { @@ -205,12 +205,12 @@ func (t *tunnelServer) gitUser() *gitcredentials.GitUser { gitUser.Email = t.workspace.GitUser.Email } if gitUser.Name == "" || gitUser.Email == "" { - hostUser := gitcredentials.GetHostUser("") + globalUser := gitcredentials.GetGlobalUser() if gitUser.Name == "" { - gitUser.Name = hostUser.Name + gitUser.Name = globalUser.Name } if gitUser.Email == "" { - gitUser.Email = hostUser.Email + gitUser.Email = globalUser.Email } } diff --git a/pkg/agent/tunnelserver/tunnelserver_test.go b/pkg/agent/tunnelserver/tunnelserver_test.go index 6d0aaef0e..5e63a491d 100644 --- a/pkg/agent/tunnelserver/tunnelserver_test.go +++ b/pkg/agent/tunnelserver/tunnelserver_test.go @@ -63,7 +63,7 @@ func TestGitUser(t *testing.T) { expected: `{"name":"Global","email":"me@company.com"}`, }, { - name: "workspace saved before DevPod kept the identity", + name: "workspace without a saved identity", workspace: &provider2.Workspace{}, expected: `{"name":"Global","email":"global@personal.com"}`, }, diff --git a/pkg/gitcredentials/gitcredentials.go b/pkg/gitcredentials/gitcredentials.go index 2b4a336a9..e4daf3539 100644 --- a/pkg/gitcredentials/gitcredentials.go +++ b/pkg/gitcredentials/gitcredentials.go @@ -145,68 +145,64 @@ func ToString(credentials *GitCredentials) string { } 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) - } + if user.Name == "" && user.Email == "" { + return nil + } - out, err := exec.Command(args[0], args[1:]...).CombinedOutput() - if err != nil { - return fmt.Errorf("set user.name '%s': %w", strings.Join(args, " "), command.WrapCommandError(out, err)) - } + gitConfigPath, err := getGlobalGitConfigPath(userName) + if err != nil { + return err } - 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) + + err = file.MkdirAll(userName, filepath.Dir(gitConfigPath), 0755) + if err != nil { + return err + } + + // run git without a shell, so that the values need no quoting + for _, setting := range [][2]string{{"user.name", user.Name}, {"user.email", user.Email}} { + if setting[1] == "" { + continue } - out, err := exec.Command(args[0], args[1:]...).CombinedOutput() + out, err := exec.Command("git", "config", "--file", gitConfigPath, setting[0], setting[1]).CombinedOutput() if err != nil { - return fmt.Errorf("set user.email '%s': %w", strings.Join(args, " "), command.WrapCommandError(out, err)) + return fmt.Errorf("set %s: %w", setting[0], command.WrapCommandError(out, err)) } } - return nil + + return file.Chown(userName, gitConfigPath) } func GetUser(userName string) (*GitUser, error) { - gitUser := &GitUser{} - - scopeArgs := []string{"config"} - if userName != "" { - p, err := getGlobalGitConfigPath(userName) - if err != nil { - return gitUser, fmt.Errorf("get git global dir for %s: %w", userName, err) - } - scopeArgs = append(scopeArgs, "--file", p) - } else { - scopeArgs = append(scopeArgs, "--global") + gitConfigPath, err := getGlobalGitConfigPath(userName) + if err != nil { + return &GitUser{}, fmt.Errorf("get git global dir for %s: %w", userName, err) } - // --file and --global name a single file, so git only follows its includes with --includes - scopeArgs = append(scopeArgs, "--includes") - - return readUser(scopeArgs), nil + // --file names a single file, so git only follows its includes with --includes + return readUser([]string{"config", "--file", gitConfigPath, "--includes"}), nil } -// GetHostUser returns the git identity that git uses in dir on this machine. That includes the -// repository's config and the include and includeIf directives that match it; outside a -// repository it is the system and global config. If dir is empty or git can't run there, it -// falls back to the global config and its includes. -func GetHostUser(dir string) *GitUser { - if dir != "" { - if info, err := os.Stat(dir); err == nil && info.IsDir() { - return readUser([]string{"-C", dir, "config"}) - } +// GetRepositoryUser returns the git identity that git uses in the repository that dir is in. That +// includes the repository's config and the include and includeIf directives that match it. It +// fails if dir isn't in a repository that git can use, for example one that git doesn't trust +// because another user owns it (see git's safe.directory). +func GetRepositoryUser(dir string) (*GitUser, error) { + if dir == "" { + return nil, fmt.Errorf("no directory") } + out, err := exec.Command("git", "-C", dir, "rev-parse", "--git-dir").CombinedOutput() + if err != nil { + return nil, command.WrapCommandError(out, err) + } + + return readUser([]string{"-C", dir, "config"}), nil +} + +// GetGlobalUser returns the git identity of the global git config, following its includes. +func GetGlobalUser() *GitUser { return readUser([]string{"config", "--global", "--includes"}) } diff --git a/pkg/gitcredentials/gituser_test.go b/pkg/gitcredentials/gituser_test.go index 65f938e64..c816f55bd 100644 --- a/pkg/gitcredentials/gituser_test.go +++ b/pkg/gitcredentials/gituser_test.go @@ -4,6 +4,7 @@ import ( "os" "os/exec" "path/filepath" + "runtime" "testing" "gotest.tools/assert" @@ -34,6 +35,8 @@ func setupIncludeIfConfig(t *testing.T) (workDir string) { t.Setenv("XDG_CONFIG_HOME", filepath.Join(home, ".config")) t.Setenv("GIT_CONFIG_GLOBAL", globalConfig) t.Setenv("GIT_CONFIG_NOSYSTEM", "1") + // keep git from finding a repository the temporary directory is in + t.Setenv("GIT_CEILING_DIRECTORIES", root) return workDir } @@ -55,38 +58,72 @@ func gitInit(t *testing.T, dir string) { assert.NilError(t, err, string(out)) } -func TestGetHostUser_FollowsIncludeIfOfRepository(t *testing.T) { +func TestGetRepositoryUser_FollowsIncludeIf(t *testing.T) { repo := filepath.Join(setupIncludeIfConfig(t), "repo") gitInit(t, repo) - assert.DeepEqual(t, GetHostUser(repo), &GitUser{Name: "Me", Email: "me@company.com"}) + gitUser, err := GetRepositoryUser(repo) + assert.NilError(t, err) + assert.DeepEqual(t, gitUser, &GitUser{Name: "Me", Email: "me@company.com"}) + // a subdirectory of the repository is in it too subDir := filepath.Join(repo, "sub") assert.NilError(t, os.Mkdir(subDir, 0755)) - assert.DeepEqual(t, GetHostUser(subDir), &GitUser{Name: "Me", Email: "me@company.com"}) + gitUser, err = GetRepositoryUser(subDir) + assert.NilError(t, err) + assert.DeepEqual(t, gitUser, &GitUser{Name: "Me", Email: "me@company.com"}) } -func TestGetHostUser_UsesRepositoryConfig(t *testing.T) { +func TestGetRepositoryUser_UsesRepositoryConfig(t *testing.T) { repo := filepath.Join(setupIncludeIfConfig(t), "..", "other") gitInit(t, repo) out, err := exec.Command("git", "-C", repo, "config", "user.email", "me@repo.com").CombinedOutput() assert.NilError(t, err, string(out)) - assert.DeepEqual(t, GetHostUser(repo), &GitUser{Name: "Me", Email: "me@repo.com"}) + gitUser, err := GetRepositoryUser(repo) + assert.NilError(t, err) + assert.DeepEqual(t, gitUser, &GitUser{Name: "Me", Email: "me@repo.com"}) } -func TestGetHostUser_OutsideRepositoryUsesGlobalConfig(t *testing.T) { +func TestGetRepositoryUser_FailsOutsideRepository(t *testing.T) { workDir := setupIncludeIfConfig(t) - // the includeIf only matches repositories - assert.DeepEqual(t, GetHostUser(workDir), &GitUser{Name: "Me", Email: "me@personal.com"}) - assert.DeepEqual(t, GetHostUser(filepath.Join(workDir, "missing")), &GitUser{Name: "Me", Email: "me@personal.com"}) - assert.DeepEqual(t, GetHostUser(""), &GitUser{Name: "Me", Email: "me@personal.com"}) + for _, dir := range []string{workDir, filepath.Join(workDir, "missing"), ""} { + _, err := GetRepositoryUser(dir) + assert.Assert(t, err != nil, dir) + } } -func TestGetHostUser_GlobalFollowsIncludes(t *testing.T) { +func TestGetRepositoryUser_FailsForUntrustedRepository(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("Git for Windows checks the owner of a repository differently") + } + + repo := filepath.Join(setupIncludeIfConfig(t), "repo") + gitInit(t, repo) + // git then takes the repository to belong to another user + t.Setenv("GIT_TEST_ASSUME_DIFFERENT_OWNER", "1") + + _, err := GetRepositoryUser(repo) + assert.ErrorContains(t, err, "dubious ownership") +} + +func TestGetGlobalUser(t *testing.T) { workDir := setupIncludeIfConfig(t) + assert.DeepEqual(t, GetGlobalUser(), &GitUser{Name: "Me", Email: "me@personal.com"}) + writeFile(t, os.Getenv("GIT_CONFIG_GLOBAL"), "[include]\n\tpath = "+configPath(filepath.Join(workDir, ".gitconfig"))+"\n") + assert.DeepEqual(t, GetGlobalUser(), &GitUser{Email: "me@company.com"}) +} - assert.DeepEqual(t, GetHostUser(""), &GitUser{Email: "me@company.com"}) +func TestSetUser_TakesValuesLiterally(t *testing.T) { + setupIncludeIfConfig(t) + + // SetUser and GetUser use $XDG_CONFIG_HOME/git/config, whose directory doesn't exist yet + gitUser := &GitUser{Name: `Ann "Nan' $HOME`, Email: "`me`@company.com"} + assert.NilError(t, SetUser("", gitUser)) + + saved, err := GetUser("") + assert.NilError(t, err) + assert.DeepEqual(t, saved, gitUser) } diff --git a/pkg/workspace/gituser.go b/pkg/workspace/gituser.go index 64c3671f4..527f021cb 100644 --- a/pkg/workspace/gituser.go +++ b/pkg/workspace/gituser.go @@ -8,17 +8,30 @@ import ( providerpkg "github.com/loft-sh/devpod/pkg/provider" ) +// GitUserOptions are the `devpod up` options for the git identity of the workspace's container. +type GitUserOptions struct { + // Name and Email replace the saved values unless they are nil. An empty value removes the + // saved one, so that DevPod looks it up again. + Name *string + Email *string + + // Dir is the directory `devpod up` runs in + Dir string +} + // ConfigureGitUser saves the git identity that the workspace's container gets if it doesn't have -// one yet. A name or email in gitUser replaces the saved one. One that is neither given nor saved -// is the one git uses where the workspace comes from: its local folder, or else dir, the +// one yet. A name or email in options replaces the saved one. One that is neither given nor saved +// is the one git uses in the repository the workspace comes from: its local folder, or else the // directory `devpod up` runs in. That way the repository's config and its includeIf matches -// count, and running `devpod up` again from somewhere else doesn't change the identity. -func ConfigureGitUser(workspace *providerpkg.Workspace, gitUser providerpkg.WorkspaceGitUser, dir string, log log.Logger) error { +// count, and running `devpod up` again from somewhere else doesn't change the identity. Outside a +// repository nothing is saved, so the container gets the global identity when it connects. +func ConfigureGitUser(workspace *providerpkg.Workspace, options GitUserOptions, log log.Logger) error { + dir := options.Dir if workspace.Source.LocalFolder != "" { dir = workspace.Source.LocalFolder } - newGitUser := resolveGitUser(workspace.GitUser, gitUser, dir) + newGitUser := resolveGitUser(workspace.GitUser, options, dir, log) if newGitUser == workspace.GitUser { return nil } @@ -33,22 +46,27 @@ func ConfigureGitUser(workspace *providerpkg.Workspace, gitUser providerpkg.Work return nil } -func resolveGitUser(saved, given providerpkg.WorkspaceGitUser, dir string) providerpkg.WorkspaceGitUser { +func resolveGitUser(saved providerpkg.WorkspaceGitUser, options GitUserOptions, dir string, log log.Logger) providerpkg.WorkspaceGitUser { gitUser := saved - if given.Name != "" { - gitUser.Name = given.Name + if options.Name != nil { + gitUser.Name = *options.Name } - if given.Email != "" { - gitUser.Email = given.Email + if options.Email != nil { + gitUser.Email = *options.Email } if gitUser.Name == "" || gitUser.Email == "" { - hostUser := gitcredentials.GetHostUser(dir) + repositoryUser, err := gitcredentials.GetRepositoryUser(dir) + if err != nil { + log.Debugf("Not using the git identity of %q: %v", dir, err) + return gitUser + } + if gitUser.Name == "" { - gitUser.Name = hostUser.Name + gitUser.Name = repositoryUser.Name } if gitUser.Email == "" { - gitUser.Email = hostUser.Email + gitUser.Email = repositoryUser.Email } } diff --git a/pkg/workspace/gituser_test.go b/pkg/workspace/gituser_test.go index e7b07801f..eb9322c2d 100644 --- a/pkg/workspace/gituser_test.go +++ b/pkg/workspace/gituser_test.go @@ -17,13 +17,16 @@ import ( func setupGitConfig(t *testing.T) (repo string) { t.Helper() - root := t.TempDir() + root, err := filepath.EvalSymlinks(t.TempDir()) + assert.NilError(t, err) globalConfig := filepath.Join(root, ".gitconfig") assert.NilError(t, os.WriteFile(globalConfig, []byte("[user]\n\tname = Me\n\temail = me@personal.com\n"), 0600)) t.Setenv("HOME", root) t.Setenv("XDG_CONFIG_HOME", filepath.Join(root, ".config")) t.Setenv("GIT_CONFIG_GLOBAL", globalConfig) t.Setenv("GIT_CONFIG_NOSYSTEM", "1") + // keep git from finding a repository the temporary directory is in + t.Setenv("GIT_CEILING_DIRECTORIES", root) t.Setenv(config.DEVPOD_HOME, filepath.Join(root, ".devpod")) repo = filepath.Join(root, "repo") @@ -37,47 +40,71 @@ func setupGitConfig(t *testing.T) (repo string) { func TestConfigureGitUser(t *testing.T) { repo := setupGitConfig(t) + // git's ceiling only stops it above the temporary directory, so not the directory itself + notRepo := filepath.Join(filepath.Dir(repo), "other") + assert.NilError(t, os.Mkdir(notRepo, 0755)) + saved := providerpkg.WorkspaceGitUser{Name: "Saved", Email: "saved@company.com"} + empty := "" + devpodEmail := "me+devpod@company.com" + givenName := "Given" testCases := []struct { name string workspace providerpkg.Workspace - given providerpkg.WorkspaceGitUser - dir string + options GitUserOptions expected providerpkg.WorkspaceGitUser }{ { - name: "new workspace gets the identity of the directory devpod runs in", - dir: repo, + name: "new workspace gets the identity of the repository devpod runs in", + options: GitUserOptions{Dir: repo}, expected: providerpkg.WorkspaceGitUser{Name: "Me", Email: "me@company.com"}, }, { name: "a local folder wins over the directory devpod runs in", workspace: providerpkg.Workspace{Source: providerpkg.WorkspaceSource{LocalFolder: repo}}, + options: GitUserOptions{Dir: notRepo}, expected: providerpkg.WorkspaceGitUser{Name: "Me", Email: "me@company.com"}, }, { - name: "outside a repository the global identity is used", - dir: t.TempDir(), - expected: providerpkg.WorkspaceGitUser{Name: "Me", Email: "me@personal.com"}, + name: "outside a repository nothing is saved", + options: GitUserOptions{Dir: notRepo}, + }, + { + name: "without a directory nothing is saved", + options: GitUserOptions{}, }, { name: "the saved identity is kept", - workspace: providerpkg.Workspace{GitUser: providerpkg.WorkspaceGitUser{Name: "Saved", Email: "saved@company.com"}}, - dir: repo, - expected: providerpkg.WorkspaceGitUser{Name: "Saved", Email: "saved@company.com"}, + workspace: providerpkg.Workspace{GitUser: saved}, + options: GitUserOptions{Dir: repo}, + expected: saved, + }, + { + name: "a given value replaces the saved one", + workspace: providerpkg.Workspace{GitUser: saved}, + options: GitUserOptions{Email: &devpodEmail, Dir: notRepo}, + expected: providerpkg.WorkspaceGitUser{Name: "Saved", Email: devpodEmail}, + }, + { + name: "a value that isn't given is looked up", + options: GitUserOptions{Name: &givenName, Dir: repo}, + expected: providerpkg.WorkspaceGitUser{Name: givenName, Email: "me@company.com"}, + }, + { + name: "a given value is saved outside a repository", + options: GitUserOptions{Email: &devpodEmail, Dir: notRepo}, + expected: providerpkg.WorkspaceGitUser{Email: devpodEmail}, }, { - name: "the given identity replaces the saved one", - workspace: providerpkg.Workspace{GitUser: providerpkg.WorkspaceGitUser{Name: "Saved", Email: "saved@company.com"}}, - given: providerpkg.WorkspaceGitUser{Email: "me+devpod@company.com"}, - dir: repo, - expected: providerpkg.WorkspaceGitUser{Name: "Saved", Email: "me+devpod@company.com"}, + name: "an empty value removes the saved one, which is looked up again", + workspace: providerpkg.Workspace{GitUser: saved}, + options: GitUserOptions{Email: &empty, Dir: repo}, + expected: providerpkg.WorkspaceGitUser{Name: "Saved", Email: "me@company.com"}, }, { - name: "a value that isn't given is resolved", - given: providerpkg.WorkspaceGitUser{Name: "Given"}, - dir: repo, - expected: providerpkg.WorkspaceGitUser{Name: "Given", Email: "me@company.com"}, + name: "an empty value outside a repository removes the saved one", + workspace: providerpkg.Workspace{GitUser: saved}, + options: GitUserOptions{Name: &empty, Email: &empty, Dir: notRepo}, }, } @@ -85,7 +112,7 @@ func TestConfigureGitUser(t *testing.T) { workspace := testCase.workspace workspace.ID = fmt.Sprintf("test-%d", i) workspace.Context = "default" - assert.NilError(t, ConfigureGitUser(&workspace, testCase.given, testCase.dir, log.Discard), testCase.name) + assert.NilError(t, ConfigureGitUser(&workspace, testCase.options, log.Discard), testCase.name) assert.Equal(t, workspace.GitUser, testCase.expected, testCase.name) saved, err := providerpkg.LoadWorkspaceConfig(workspace.Context, workspace.ID) diff --git a/pkg/workspace/workspace.go b/pkg/workspace/workspace.go index a70a3ddc1..73c971b3c 100644 --- a/pkg/workspace/workspace.go +++ b/pkg/workspace/workspace.go @@ -4,7 +4,6 @@ import ( "context" "errors" "fmt" - "os" "sort" "strings" @@ -37,7 +36,7 @@ func Resolve( devContainerPath string, sshConfigPath string, source *providerpkg.WorkspaceSource, - gitUser *providerpkg.WorkspaceGitUser, + gitUser *GitUserOptions, uid string, changeLastUsed bool, log log.Logger, @@ -103,11 +102,9 @@ func Resolve( } } - // configure the git identity of the container, unless the caller doesn't start one + // configure the git identity of the container, unless the caller doesn't connect to it if gitUser != nil { - // an empty directory makes DevPod use the global git identity - cwd, _ := os.Getwd() - err = ConfigureGitUser(workspace, *gitUser, cwd, log) + err = ConfigureGitUser(workspace, *gitUser, log) if err != nil { return nil, fmt.Errorf("configure git user: %w", err) }