diff --git a/cmd/gitcd/init.go b/cmd/gitcd/init.go index 0e16b96..de8cd5d 100644 --- a/cmd/gitcd/init.go +++ b/cmd/gitcd/init.go @@ -4,6 +4,7 @@ package main import ( + "bufio" "context" "errors" "flag" @@ -14,23 +15,54 @@ import ( "strings" "time" + "github.com/ostendorf-cloud/gitcd/internal/config" "github.com/ostendorf-cloud/gitcd/internal/git" "github.com/ostendorf-cloud/gitcd/internal/gitconfig" + "gopkg.in/yaml.v3" ) -// runInit implements "gitcd init": it adds exactly one "include.path" line -// to the user's global git configuration, pointing at the include file -// "gitcd apply" renders, so every identity profile rule actually takes -// effect. It refuses when a global identity key already sits after the -// last existing "[include]" section, since "git config --global --add" -// inserts a new key into that section rather than at the end of the file, -// and a key sitting after it would keep beating every includeIf value -// gitcd adds, silently. -func runInit(args []string, stdout, stderr io.Writer) int { +// defaultMovedProfileName is the profile "init" moves a global identity +// key into when "--profile" does not say otherwise. targetProfileConflict +// enforces that the chosen name, this default or an explicit "--profile", +// is always a dedicated profile, never an existing one chosen by +// guesswork, and never the profile defaultProfile already names: merging +// into whichever profile happens to already exist would silently +// overwrite a key that profile set on purpose, the moment it happened to +// share a name with one of IdentityKeys, and landing in defaultProfile +// would make the leaked identity the default identity again, every +// unmatched repository's own, exactly the bug this move exists to fix. +const defaultMovedProfileName = "global" + +// runInit implements "gitcd init". Beyond adding the "include.path" line +// described below, it first detects every identity key the global git +// configuration sets directly: "user.useConfigOnly" only stops git from +// guessing a default identity, it never suppresses a value that is +// already configured, so a key sitting anywhere in this file, matched by +// gitcd's own includeIf chain or not, defeats the refusal every profile's +// "useConfigOnly" is meant to provide for a repository no rule matches. +// "init" reports every such key, names the profile it would belong in, +// and, only when asked, moves it there and removes it from the global +// file, with a backup of both files it touches. +// +// "init" also adds exactly one "include.path" line to the user's global +// git configuration, pointing at the include file "gitcd apply" renders, +// so every identity profile rule actually takes effect. It refuses when a +// global identity key already sits after the last existing "[include]" +// section, since "git config --global --add" inserts a new key into that +// section rather than at the end of the file, and a key sitting after it +// would keep beating every includeIf value gitcd adds, silently. +func runInit(args []string, stdin io.Reader, stdout, stderr io.Writer) int { fset := flag.NewFlagSet("gitcd init", flag.ContinueOnError) fset.SetOutput(stderr) dryRun := fset.Bool("dry-run", false, "print what would change without writing anything") verbose := fset.Bool("verbose", false, "enable debug logging on stderr") + moveIdentity := fset.String( + "move-identity", "", + `what to do with an identity key found directly in the global git configuration: +"yes" moves it without a prompt, "ask" prompts on stderr and reads the answer from stdin, +"no", the default, only reports it`, + ) + profileFlag := fset.String("profile", "", "profile a moved identity key is written into (default \""+defaultMovedProfileName+"\")") if err := fset.Parse(args); err != nil { return 2 } @@ -39,6 +71,17 @@ func runInit(args []string, stdout, stderr io.Writer) int { fset.Usage() return 2 } + switch *moveIdentity { + case "", "no", "yes", "ask": + default: + fmt.Fprintf(stderr, "gitcd init: --move-identity must be \"yes\", \"ask\" or \"no\", got %q\n", *moveIdentity) + return 2 + } + target := defaultMovedProfileName + if *profileFlag != "" { + target = *profileFlag + } + logger := newLogger(*verbose, stderr) ctx := context.Background() @@ -62,6 +105,66 @@ func runInit(args []string, stdout, stderr io.Writer) int { return 1 } + // A single backup of globalPath as it was found, shared between the + // identity move below and the include line further down: both can + // write to globalPath in the same run, and a second, later backup + // would either collide with the first, since both resolve to the + // same once per second timestamp, or, if it did not collide, record + // an already modified file instead of the original one. + var backupPath string + backupGlobalConfig := func() error { + if backupPath != "" || len(content) == 0 { + return nil + } + backupPath = globalPath + ".gitcd-backup-" + time.Now().UTC().Format("20060102T150405Z") + if err := os.WriteFile(backupPath, content, 0o600); err != nil { + backupPath = "" + return fmt.Errorf("back up %s: %w", globalPath, err) + } + logger.Debug("backed up the global git configuration file", "path", backupPath) + fmt.Fprintf(stdout, "backed up %s to %s\n", globalPath, backupPath) + return nil + } + + if found := git.FindGlobalIdentityValues(content, git.IdentityKeys); len(found) > 0 { + reportGlobalIdentityKeys(stdout, globalPath, found, target) + + move := decideMove(*moveIdentity, stdin, stderr, found, target) + if move { + cfgPath, err := config.Path() + if err != nil { + fmt.Fprintf(stderr, "gitcd init: %v\n", err) + return 1 + } + conflict, err := targetProfileConflict(cfgPath, target) + if err != nil { + fmt.Fprintf(stderr, "gitcd init: %v\n", err) + return 1 + } + if conflict != "" { + fmt.Fprintf(stderr, "gitcd init: %s\n", conflict) + return 1 + } + } + switch { + case !move: + fmt.Fprintln(stdout, "not moving them: pass --move-identity=yes to move them without a prompt, or --move-identity=ask to be asked") + case *dryRun: + fmt.Fprintf(stdout, "would move %s into profile %q and remove them from %s\n", describeIdentityKeys(found), target, globalPath) + default: + if err := moveGlobalIdentity(ctx, found, target, globalPath, backupGlobalConfig, stdout); err != nil { + fmt.Fprintf(stderr, "gitcd init: %v\n", err) + return 1 + } + refreshed, err := os.ReadFile(globalPath) + if err != nil { + fmt.Fprintf(stderr, "gitcd init: read %s: %v\n", globalPath, err) + return 1 + } + content = refreshed + } + } + if alreadyIncluded(content, includePath) { fmt.Fprintf(stdout, "gitcd is already initialized: %s already includes %s\n", globalPath, includePath) return 0 @@ -86,14 +189,9 @@ func runInit(args []string, stdout, stderr io.Writer) int { return 0 } - if len(content) > 0 { - backupPath := globalPath + ".gitcd-backup-" + time.Now().UTC().Format("20060102T150405Z") - if err := os.WriteFile(backupPath, content, 0o600); err != nil { - fmt.Fprintf(stderr, "gitcd init: back up %s: %v\n", globalPath, err) - return 1 - } - logger.Debug("backed up the global git configuration file", "path", backupPath) - fmt.Fprintf(stdout, "backed up %s to %s\n", globalPath, backupPath) + if err := backupGlobalConfig(); err != nil { + fmt.Fprintf(stderr, "%v\n", err) + return 1 } if err := os.MkdirAll(filepath.Dir(includePath), 0o755); err != nil { @@ -117,3 +215,278 @@ func runInit(args []string, stdout, stderr io.Writer) int { func alreadyIncluded(content []byte, includePath string) bool { return len(content) > 0 && strings.Contains(string(content), includePath) } + +// reportGlobalIdentityKeys prints every key found, its value, and the +// profile it would belong in, to stdout: the acceptance this command's +// detection side exists for is naming them, regardless of whether a move +// follows. +func reportGlobalIdentityKeys(stdout io.Writer, globalPath string, found []git.GlobalIdentityValue, target string) { + fmt.Fprintf(stdout, "%s sets %s directly, outside any profile:\n", globalPath, describeIdentityKeys(found)) + for _, v := range found { + fmt.Fprintf(stdout, " %s = %s\n", v.Key, v.Value) + } + fmt.Fprintln(stdout, `"user.useConfigOnly" only stops git from guessing a default identity; it does`) + fmt.Fprintln(stdout, "not suppress a value already configured here, so every repository gitcd") + fmt.Fprintln(stdout, "manages, including one no rule matches, still gets an identity from this file.") + fmt.Fprintf(stdout, "They belong in profile %q instead.\n", target) +} + +// describeIdentityKeys joins found's key names for a one line message. +func describeIdentityKeys(found []git.GlobalIdentityValue) string { + names := make([]string, len(found)) + for i, v := range found { + names[i] = v.Key + } + return strings.Join(names, ", ") +} + +// targetProfileConflict reports, as a message ready to print, why "init" +// must refuse to move an identity key into profile target, or "" when +// target is clear to use. It reads cfgPath itself, with its own, lenient +// parse into only the two fields this check needs, rather than going +// through config.LoadFrom: this is purely a name collision check, which +// must not block on some unrelated problem the rest of the file carries, +// a problem moveGlobalIdentity's own, stricter validation is already +// responsible for catching once there is something to write. The decoder +// stays loose on purpose: it is never given KnownFields, so a field this +// struct does not name, "totallyUnknownKey" for instance, is simply +// ignored here rather than failing this parse, keeping that failure mode +// for the stricter check downstream, the one already responsible for it. +// +// A failure of this lenient parse itself, meaning the two fields it does +// name do not even decode, a "profiles" that is a list rather than a +// mapping for instance, is refused rather than waved through: a parse +// this forgiving failing at all means the file's shape could not be +// understood well enough to say there is no conflict, and "no conflict" +// must never be the answer to "I could not tell." +// +// Two collisions are refused, matching pull request 12's review: +// - target already names defaultProfile. Never allowed, with no +// override: the found keys are, by definition, a leaked identity that +// "useConfigOnly" could not suppress; landing them in defaultProfile +// would make that same leaked identity the default for every +// repository no rule matches, again, the exact bug this move exists +// to fix, just rearmed in a broader form. +// - target already names an existing profile. Never allowed either, +// with no override flag: AddProfileKeys overwrites a key by name, so +// merging into a profile that already exists would silently replace +// a value chosen on purpose the moment it happened to share a key +// name with one of git.IdentityKeys. A dedicated profile costs +// nothing to create under a name nothing else uses yet, so there is +// no legitimate reason to merge instead, and the refusal names the +// conflict so the user can rerun with a different "--profile". +func targetProfileConflict(cfgPath, target string) (string, error) { + raw, err := os.ReadFile(cfgPath) + if err != nil { + if errors.Is(err, os.ErrNotExist) { + return "", nil + } + return "", fmt.Errorf("read %s: %w", cfgPath, err) + } + + var doc struct { + Profiles map[string]map[string]any `yaml:"profiles"` + DefaultProfile string `yaml:"defaultProfile"` + } + if err := yaml.Unmarshal(raw, &doc); err != nil { + return fmt.Sprintf( + "%s does not parse well enough to check profile %q for a conflict: %v\n"+ + "refusing the move rather than risking a silent overwrite or a silently rearmed default", + cfgPath, target, err, + ), nil + } + + if doc.DefaultProfile != "" && doc.DefaultProfile == target { + return fmt.Sprintf( + "profile %q is the defaultProfile in %s: moving the leaked identity there would make every repository no rule\n"+ + "matches inherit it again, the exact bug this move exists to fix\n"+ + "choose a different name with --profile, or clear defaultProfile first and run \"gitcd apply\" again", + target, cfgPath, + ), nil + } + if _, ok := doc.Profiles[target]; ok { + return fmt.Sprintf( + "profile %q already exists in %s: moving into it could silently overwrite a value chosen on purpose\n"+ + "choose a different, not yet used name with --profile", + target, cfgPath, + ), nil + } + return "", nil +} + +// decideMove reports whether found should be moved, given moveIdentity, +// the "--move-identity" flag's value: "yes" moves without asking, "no" +// and the unset default never move and never read stdin, and "ask" prompts +// on stderr and reads the answer from stdin, defaulting to "no" on +// anything other than an explicit "y" or "yes", EOF included, so a closed +// or empty stdin, the shape a script or CI job's stdin commonly has, +// resolves to the safe answer without blocking. +func decideMove(moveIdentity string, stdin io.Reader, stderr io.Writer, found []git.GlobalIdentityValue, target string) bool { + switch moveIdentity { + case "yes": + return true + case "ask": + fmt.Fprintf(stderr, "move %s into profile %q and remove them from the global git configuration? [y/N] ", describeIdentityKeys(found), target) + line, _ := bufio.NewReader(stdin).ReadString('\n') + answer := strings.ToLower(strings.TrimSpace(line)) + return answer == "y" || answer == "yes" + default: + return false + } +} + +// moveGlobalIdentity moves found into profile target: it writes the +// updated profile into the configuration file config.Path resolves, +// backing it up first when it already exists, validates that the +// updated file actually loads, before removing each key from the global +// git configuration, backing that up through backupGlobalConfig first. +// Persisting the new copy before touching the original is deliberate: if +// the persist step fails, or the persisted copy does not load, the +// global configuration is never touched at all, and if a later removal +// fails partway through, every value is already safely recorded in the +// profile, not lost. +func moveGlobalIdentity( + ctx context.Context, found []git.GlobalIdentityValue, target, globalPath string, backupGlobalConfig func() error, stdout io.Writer, +) error { + cfgPath, err := config.Path() + if err != nil { + return err + } + + existingYAML, err := os.ReadFile(cfgPath) + if err != nil && !errors.Is(err, os.ErrNotExist) { + return fmt.Errorf("read %s: %w", cfgPath, err) + } + if len(existingYAML) > 0 { + cfgBackupPath := cfgPath + ".gitcd-backup-" + time.Now().UTC().Format("20060102T150405Z") + if err := os.WriteFile(cfgBackupPath, existingYAML, 0o600); err != nil { + return fmt.Errorf("back up %s: %w", cfgPath, err) + } + fmt.Fprintf(stdout, "backed up %s to %s\n", cfgPath, cfgBackupPath) + } + + keys := make(map[string]string, len(found)) + for _, v := range found { + keys[v.Key] = v.Value + } + updatedYAML, err := config.AddProfileKeys(existingYAML, target, keys) + if err != nil { + return fmt.Errorf("%s: %w", cfgPath, err) + } + if err := os.MkdirAll(filepath.Dir(cfgPath), 0o755); err != nil { + return fmt.Errorf("create %s: %w", filepath.Dir(cfgPath), err) + } + + // Written to a temporary file first and validated with config.LoadFrom + // before it ever reaches cfgPath: writing an updated configuration + // that then fails to load, for instance because the file already + // carried an unknown key, must never reach the point of removing the + // identity from the global git configuration below, the comment on + // this function's own name already argues for persisting before + // destroying, and persisting something unreadable does not satisfy + // that argument. Validating against a temporary file, rather than + // cfgPath itself, means a failure here leaves cfgPath exactly as it + // was, with nothing left to restore. + // + // The temporary file gets an unpredictable name from os.CreateTemp, + // in the same directory as cfgPath so the later rename stays on one + // filesystem, rather than a fixed, guessable name: a fixed name is a + // symlink another process or a leftover from a previous run could + // already occupy, and os.WriteFile follows a symlink wherever it + // finds one, which would render this validation copy through that + // link instead of writing an ordinary file gitcd fully controls. + tmpFile, err := os.CreateTemp(filepath.Dir(cfgPath), filepath.Base(cfgPath)+".gitcd-validate-tmp-*") + if err != nil { + return fmt.Errorf("create a temporary file next to %s: %w", cfgPath, err) + } + tmpPath := tmpFile.Name() + defer func() { _ = os.Remove(tmpPath) }() + // os.CreateTemp creates its file 0o600; the configuration file this + // temporary copy stands in for is written 0o644, so match that + // before the rename below carries this file's own mode over to + // cfgPath. + if err := tmpFile.Chmod(0o644); err != nil { + _ = tmpFile.Close() + return fmt.Errorf("set permissions on %s: %w", tmpPath, err) + } + if _, err := tmpFile.Write(updatedYAML); err != nil { + _ = tmpFile.Close() + return fmt.Errorf("write %s: %w", tmpPath, err) + } + if err := tmpFile.Close(); err != nil { + return fmt.Errorf("write %s: %w", tmpPath, err) + } + + cfg, err := config.LoadFrom(tmpPath) + if err != nil { + return fmt.Errorf("the updated configuration would not load, aborting before touching %s: %w", globalPath, err) + } + // A cheap belt and braces on top of LoadFrom succeeding: LoadFrom not + // erroring only proves the rendered file is valid YAML and a valid + // gitcd configuration, not that this particular edit actually landed + // in it, which is exactly the gap a "profiles:" key decoding to a + // "!!null" scalar opened. identityKeysLanded parses the same bytes + // that are about to replace cfgPath, through the same loader every + // other command uses, so it catches a future shape of this class + // regardless of where in the render path it originates, and it runs + // before the rename and before the unset loop below, so a failure + // here guarantees the global keys are never removed. + if err := identityKeysLanded(cfg, target, found); err != nil { + return fmt.Errorf("the write that was supposed to move %s did not, aborting before touching %s: %w", describeIdentityKeys(found), globalPath, err) + } + + // cfgPath may itself be a symlink, the common shape for a + // configuration tracked in a separate dotfiles repository: renaming + // a file onto a symlink's own path replaces the link with an + // ordinary file, silently detaching it from whatever it pointed at. + // Resolving it first and writing through the resolved path keeps the + // link itself, and the dotfiles copy it points at, intact. A cfgPath + // that does not exist yet, the first run on a machine with no + // configuration file at all, is not a symlink to resolve, and is + // written at cfgPath directly. + writeTarget := cfgPath + if resolved, err := filepath.EvalSymlinks(cfgPath); err == nil { + writeTarget = resolved + } else if !errors.Is(err, os.ErrNotExist) { + return fmt.Errorf("resolve %s: %w", cfgPath, err) + } + if err := os.Rename(tmpPath, writeTarget); err != nil { + return fmt.Errorf("write %s: %w", writeTarget, err) + } + fmt.Fprintf(stdout, "moved %s into profile %q in %s\n", describeIdentityKeys(found), target, writeTarget) + + if err := backupGlobalConfig(); err != nil { + return err + } + for _, v := range found { + if err := git.ConfigUnsetGlobal(ctx, v.Key); err != nil { + return fmt.Errorf("remove %s from %s: %w", v.Key, globalPath, err) + } + } + fmt.Fprintf(stdout, "removed %s from %s\n", describeIdentityKeys(found), globalPath) + fmt.Fprintf( + stdout, + "profile %q is not applied automatically: add a rule, or set it as the defaultProfile, then run \"gitcd apply\"\n"+ + "to reverse this, restore the backups above, or set the keys shown earlier back with \"git config --global\"\n", + target, + ) + return nil +} + +// identityKeysLanded reports an error naming the first of found's keys +// that profile target does not carry in cfg with exactly the value found +// recorded, or nil when every one of them does. cfg is expected to come +// from parsing the exact bytes moveGlobalIdentity is about to rename into +// place, so a non-nil result here means that write does not contain what +// it claims to, and the caller must abort before going anywhere near the +// global git configuration's own keys. +func identityKeysLanded(cfg config.Config, target string, found []git.GlobalIdentityValue) error { + profile := cfg.Profiles[target] + for _, v := range found { + got, ok := profile[v.Key] + if !ok || got != v.Value { + return fmt.Errorf("profile %q does not carry %s", target, v.Key) + } + } + return nil +} diff --git a/cmd/gitcd/init_test.go b/cmd/gitcd/init_test.go index d03b10e..c391230 100644 --- a/cmd/gitcd/init_test.go +++ b/cmd/gitcd/init_test.go @@ -5,10 +5,16 @@ package main import ( "bytes" + "context" + "errors" "os" + "os/exec" "path/filepath" "strings" "testing" + + "github.com/ostendorf-cloud/gitcd/internal/config" + "github.com/ostendorf-cloud/gitcd/internal/git" ) // globalConfigPath mirrors setupEnv's own choice of global git configuration @@ -148,6 +154,746 @@ func TestRunInit_DryRunChangesNothing(t *testing.T) { } } +func TestRunInit_ReportsGlobalIdentityKeysWithoutMovingByDefault(t *testing.T) { + home := setupEnv(t) + globalPath := globalConfigPath(home) + + var stdout, stderr bytes.Buffer + code := run([]string{"init"}, &stdout, &stderr) + if code != 0 { + t.Fatalf("init = %d, want 0, stderr: %s", code, stderr.String()) + } + if !strings.Contains(stdout.String(), "user.email") { + t.Fatalf("init stdout = %q, want it to name user.email, set globally by setupEnv", stdout.String()) + } + if !strings.Contains(stdout.String(), `"global"`) { + t.Fatalf("init stdout = %q, want it to name the profile a move would use", stdout.String()) + } + + after, err := os.ReadFile(globalPath) + if err != nil { + t.Fatalf("read %s: %v", globalPath, err) + } + if !strings.Contains(string(after), "email") { + t.Fatalf("init removed the global identity key despite no --move-identity flag: %q", after) + } + // The include line is still added: reporting a leaked identity key + // must not block the rest of "init" from doing its ordinary job. + includePath := filepath.Join(home, ".config", "gitcd", "includes.gitconfig") + if !strings.Contains(string(after), includePath) { + t.Fatalf("global config after init = %q, want it to include %q", after, includePath) + } + + if _, err := os.Stat(filepath.Join(home, ".gitcd.yaml")); err == nil { + t.Fatal("init created ~/.gitcd.yaml despite no --move-identity flag, want it left untouched") + } +} + +func TestRunInit_MovesIdentityKeysWhenExplicitlyToldYes(t *testing.T) { + home := setupEnv(t) + globalPath := globalConfigPath(home) + + var stdout, stderr bytes.Buffer + code := runInit([]string{"--move-identity=yes"}, strings.NewReader(""), &stdout, &stderr) + if code != 0 { + t.Fatalf("init --move-identity=yes = %d, want 0, stderr: %s", code, stderr.String()) + } + + after, err := os.ReadFile(globalPath) + if err != nil { + t.Fatalf("read %s: %v", globalPath, err) + } + if strings.Contains(string(after), "Test User") || strings.Contains(string(after), "[email protected]") { + t.Fatalf("global config after a move = %q, want the identity keys removed", after) + } + + cfgPath := filepath.Join(home, ".gitcd.yaml") + cfgContent, err := os.ReadFile(cfgPath) + if err != nil { + t.Fatalf("init did not write %s: %v", cfgPath, err) + } + if !strings.Contains(string(cfgContent), "[email protected]") { + t.Fatalf("%s = %q, want the moved email under profiles.global", cfgPath, cfgContent) + } + + // The global git config must have a backup of its pre-move content, + // so the move is reversible. gitcd.yaml did not exist before this + // run, so no backup of it is expected here; a pre-existing + // gitcd.yaml getting backed up is covered separately below. + gitBackupEntries, err := os.ReadDir(filepath.Dir(globalPath)) + if err != nil { + t.Fatalf("read dir %s: %v", filepath.Dir(globalPath), err) + } + var sawGitBackup bool + for _, e := range gitBackupEntries { + if strings.HasPrefix(e.Name(), filepath.Base(globalPath)+".gitcd-backup-") { + sawGitBackup = true + } + } + if !sawGitBackup { + t.Fatalf("no backup of %s found, entries: %v", globalPath, gitBackupEntries) + } +} + +func TestRunInit_MoveBacksUpAnExistingGitcdYAMLBeforeEditingIt(t *testing.T) { + home := setupEnv(t) + cfgPath := filepath.Join(home, ".gitcd.yaml") + writeFile(t, cfgPath, "profiles:\n work:\n user.email: work@example.test\nrules:\n - host: github.com\n profile: work\n") + + var stdout, stderr bytes.Buffer + code := runInit([]string{"--move-identity=yes"}, strings.NewReader(""), &stdout, &stderr) + if code != 0 { + t.Fatalf("init --move-identity=yes = %d, want 0, stderr: %s", code, stderr.String()) + } + + after, err := os.ReadFile(cfgPath) + if err != nil { + t.Fatalf("read %s: %v", cfgPath, err) + } + if !strings.Contains(string(after), "work@example.test") { + t.Fatalf("%s after a move = %q, want the pre-existing profile untouched", cfgPath, after) + } + if !strings.Contains(string(after), "[email protected]") { + t.Fatalf("%s after a move = %q, want the moved email under profiles.global", cfgPath, after) + } + + entries, err := os.ReadDir(home) + if err != nil { + t.Fatalf("read dir %s: %v", home, err) + } + var backup string + for _, e := range entries { + if strings.HasPrefix(e.Name(), filepath.Base(cfgPath)+".gitcd-backup-") { + backup = filepath.Join(home, e.Name()) + } + } + if backup == "" { + t.Fatalf("no backup of the pre-existing %s found, entries: %v", cfgPath, entries) + } + backupContent, err := os.ReadFile(backup) + if err != nil { + t.Fatalf("read backup %s: %v", backup, err) + } + if !strings.Contains(string(backupContent), "work@example.test") || strings.Contains(string(backupContent), "[email protected]") { + t.Fatalf("backup content = %q, want it to hold the configuration exactly as it was before the move", backupContent) + } +} + +func TestRunInit_AsksInteractivelyAndMovesOnYesAnswer(t *testing.T) { + home := setupEnv(t) + + var stdout, stderr bytes.Buffer + code := runInit([]string{"--move-identity=ask"}, strings.NewReader("y\n"), &stdout, &stderr) + if code != 0 { + t.Fatalf("init --move-identity=ask (y) = %d, want 0, stderr: %s", code, stderr.String()) + } + if !strings.Contains(stderr.String(), "[y/N]") { + t.Fatalf("init --move-identity=ask stderr = %q, want a prompt", stderr.String()) + } + + after, err := os.ReadFile(globalConfigPath(home)) + if err != nil { + t.Fatalf("read global config: %v", err) + } + if strings.Contains(string(after), "[email protected]") { + t.Fatalf("global config after answering yes = %q, want the identity key removed", after) + } +} + +func TestRunInit_AsksInteractivelyAndSkipsOnNoAnswer(t *testing.T) { + home := setupEnv(t) + globalPath := globalConfigPath(home) + before, err := os.ReadFile(globalPath) + if err != nil { + t.Fatalf("read %s: %v", globalPath, err) + } + + var stdout, stderr bytes.Buffer + code := runInit([]string{"--move-identity=ask"}, strings.NewReader("n\n"), &stdout, &stderr) + if code != 0 { + t.Fatalf("init --move-identity=ask (n) = %d, want 0, stderr: %s", code, stderr.String()) + } + + after, err := os.ReadFile(globalPath) + if err != nil { + t.Fatalf("read %s: %v", globalPath, err) + } + // The include line is still added on a declined move; only the + // identity keys must survive untouched. + if !strings.Contains(string(after), "[email protected]") { + t.Fatalf("global config after declining = %q, want the identity key left in place", after) + } + if string(before) == string(after) { + t.Fatal("init made no change at all after declining the move, want the include line still added") + } +} + +func TestRunInit_AsksInteractivelyDefaultsToNoOnClosedStdin(t *testing.T) { + // A script or CI job commonly runs with an empty or closed stdin, + // never a terminal: "--move-identity=ask" must resolve to the safe + // "no" answer there instead of blocking forever waiting for a line + // that will never arrive. + home := setupEnv(t) + + var stdout, stderr bytes.Buffer + code := runInit([]string{"--move-identity=ask"}, strings.NewReader(""), &stdout, &stderr) + if code != 0 { + t.Fatalf("init --move-identity=ask (closed stdin) = %d, want 0, stderr: %s", code, stderr.String()) + } + + after, err := os.ReadFile(globalConfigPath(home)) + if err != nil { + t.Fatalf("read global config: %v", err) + } + if !strings.Contains(string(after), "[email protected]") { + t.Fatalf("global config after a closed stdin prompt = %q, want the identity key left in place", after) + } +} + +func TestRunInit_DryRunDescribesTheMoveWithoutWriting(t *testing.T) { + home := setupEnv(t) + globalPath := globalConfigPath(home) + before, err := os.ReadFile(globalPath) + if err != nil { + t.Fatalf("read %s: %v", globalPath, err) + } + + var stdout, stderr bytes.Buffer + code := runInit([]string{"--dry-run", "--move-identity=yes"}, strings.NewReader(""), &stdout, &stderr) + if code != 0 { + t.Fatalf("init --dry-run --move-identity=yes = %d, want 0, stderr: %s", code, stderr.String()) + } + if !strings.Contains(stdout.String(), "would move") { + t.Fatalf("init --dry-run stdout = %q, want it to describe the move", stdout.String()) + } + + after, err := os.ReadFile(globalPath) + if err != nil { + t.Fatalf("read %s: %v", globalPath, err) + } + if string(after) != string(before) { + t.Fatalf("init --dry-run changed the global config:\nbefore: %q\nafter: %q", before, after) + } + if _, err := os.Stat(filepath.Join(home, ".gitcd.yaml")); err == nil { + t.Fatal("init --dry-run created ~/.gitcd.yaml, want it left untouched") + } +} + +// TestRunInit_UnmatchedRepositoryRefusesToCommitAfterMove is the +// end-to-end acceptance proof for issue 8: reproduces the exact scenario +// a global identity defeats "useConfigOnly" for (a global user.name and +// user.email, a rule for one host only, no defaultProfile), moves the +// leaked keys out through "gitcd init --move-identity=yes", runs "gitcd +// apply", and then attempts a real commit, through the actual git +// binary, in a repository whose remote matches no rule. Before the move, +// this same sequence commits successfully under the leaked global +// identity; the assertion below is that it now fails instead. +func TestRunInit_UnmatchedRepositoryRefusesToCommitAfterMove(t *testing.T) { + home := setupEnv(t) + writeFile(t, filepath.Join(home, ".gitcd.yaml"), ` +rules: + - host: github.com + profile: work + +profiles: + work: + user.email: work@example.test +`) + + var stdout, stderr bytes.Buffer + if code := run([]string{"init", "--move-identity=yes"}, &stdout, &stderr); code != 0 { + t.Fatalf("init --move-identity=yes = %d, want 0, stderr: %s", code, stderr.String()) + } + stdout.Reset() + stderr.Reset() + if code := run([]string{"apply"}, &stdout, &stderr); code != 0 { + t.Fatalf("apply = %d, want 0, stderr: %s", code, stderr.String()) + } + + // Built by hand rather than through initRepository, which commits as + // part of its own setup: after the move, that commit would already + // be the one expected to fail, leaving nothing left for this test's + // own, deliberate commit attempt to prove. + repo := filepath.Join(home, "elsewhere", "unmatched") + if err := os.MkdirAll(repo, 0o755); err != nil { + t.Fatalf("create %s: %v", repo, err) + } + runGit(t, repo, "init", "--initial-branch=main") + runGit(t, repo, "remote", "add", "origin", "git@gitlab.com:someone/unmatched.git") + writeFile(t, filepath.Join(repo, "README.md"), "example\n") + runGit(t, repo, "add", "README.md") + + cmd := exec.CommandContext(context.Background(), "git", "-c", "core.editor=true", "commit", "-m", "fix: probe an unmatched repository") + cmd.Dir = repo + out, err := cmd.CombinedOutput() + if err == nil { + t.Fatalf("commit in an unmatched repository succeeded after moving the leaked identity, want it to fail: %s", out) + } + + effectiveEmail := runGitShowOriginAllowFailure(t, repo, "user.email") + if effectiveEmail != "" { + t.Fatalf("user.email resolved to %q in the unmatched repository, want it unset", effectiveEmail) + } +} + +// runGitShowOriginAllowFailure runs "git -C dir config --get key", +// returning its trimmed stdout, or "" when git exits 1 because the key +// carries no value at all, which is the expected outcome here, not a +// test failure. +func runGitShowOriginAllowFailure(t *testing.T, dir, key string) string { + t.Helper() + cmd := exec.CommandContext(context.Background(), "git", "-C", dir, "config", "--get", key) + out, err := cmd.Output() + if err != nil { + var exitErr *exec.ExitError + if errors.As(err, &exitErr) && exitErr.ExitCode() == 1 { + return "" + } + t.Fatalf("git -C %s config --get %s: %v", dir, key, err) + } + return strings.TrimSpace(string(out)) +} + +// TestRunInit_RefusesToTargetADeliberateProfileThatIsAlsoTheDefault is +// the exact reproduction pull request 12's review returned as a blocker: +// a profile that already carries deliberate values and is also +// defaultProfile, named explicitly with "--profile". Moving the leaked +// global identity there would both overwrite the deliberate values and +// make the leaked identity the default for every repository no rule +// matches, re-arming the bug this command exists to fix in a broader +// form. "init" must refuse instead, and must not change the file. +func TestRunInit_RefusesToTargetADeliberateProfileThatIsAlsoTheDefault(t *testing.T) { + home := setupEnv(t) + cfgPath := filepath.Join(home, ".gitcd.yaml") + writeFile(t, cfgPath, ` +profiles: + everyday: + user.name: Deliberate Name + user.email: deliberate@example.com +defaultProfile: everyday +`) + before, err := os.ReadFile(cfgPath) + if err != nil { + t.Fatalf("read %s: %v", cfgPath, err) + } + + var stdout, stderr bytes.Buffer + code := runInit([]string{"--move-identity=yes", "--profile=everyday"}, strings.NewReader(""), &stdout, &stderr) + if code == 0 { + t.Fatalf("init --move-identity=yes --profile=everyday = 0, want a refusal when the target is the defaultProfile, stdout: %s", stdout.String()) + } + if !strings.Contains(stderr.String(), "defaultProfile") { + t.Fatalf("init stderr = %q, want it to name the defaultProfile conflict", stderr.String()) + } + + after, err := os.ReadFile(cfgPath) + if err != nil { + t.Fatalf("read %s: %v", cfgPath, err) + } + if string(after) != string(before) { + t.Fatalf("init changed %s despite refusing:\nbefore: %q\nafter: %q", cfgPath, before, after) + } + + globalAfter, err := os.ReadFile(globalConfigPath(home)) + if err != nil { + t.Fatalf("read global config: %v", err) + } + if !strings.Contains(string(globalAfter), "Test User") || !strings.Contains(string(globalAfter), "[email protected]") { + t.Fatalf("init removed the global identity despite refusing the move: %q", globalAfter) + } +} + +// TestRunInit_RefusesToOverwriteAPreExistingProfileNamedGlobal covers the +// review's second reproduction of the same blocker: no "--profile" flag +// at all, but a profile already sits under the default target name and +// already carries deliberate values. "init" must refuse rather than +// silently overwriting them. +func TestRunInit_RefusesToOverwriteAPreExistingProfileNamedGlobal(t *testing.T) { + home := setupEnv(t) + cfgPath := filepath.Join(home, ".gitcd.yaml") + writeFile(t, cfgPath, ` +profiles: + global: + user.email: deliberate@example.test + user.name: Deliberate Name +`) + before, err := os.ReadFile(cfgPath) + if err != nil { + t.Fatalf("read %s: %v", cfgPath, err) + } + + var stdout, stderr bytes.Buffer + code := runInit([]string{"--move-identity=yes"}, strings.NewReader(""), &stdout, &stderr) + if code == 0 { + t.Fatalf("init --move-identity=yes = 0, want a refusal when profile %q already exists, stdout: %s", defaultMovedProfileName, stdout.String()) + } + if !strings.Contains(stderr.String(), "already exists") { + t.Fatalf("init stderr = %q, want it to name the existing profile conflict", stderr.String()) + } + + after, err := os.ReadFile(cfgPath) + if err != nil { + t.Fatalf("read %s: %v", cfgPath, err) + } + if string(after) != string(before) { + t.Fatalf("init changed %s despite refusing:\nbefore: %q\nafter: %q", cfgPath, before, after) + } +} + +// TestRunInit_AbortsTheMoveWhenTheUpdatedConfigWouldNotLoad covers the +// review's "should": a pre-existing ~/.gitcd.yaml that already carries an +// unknown key still carries it once AddProfileKeys merges the moved +// identity in, so the result does not load either. "init" must abort +// before removing anything from the global git configuration, rather +// than exiting 0 with the identity gone from everywhere and a gitcd +// configuration that does not load. +func TestRunInit_AbortsTheMoveWhenTheUpdatedConfigWouldNotLoad(t *testing.T) { + home := setupEnv(t) + globalPath := globalConfigPath(home) + cfgPath := filepath.Join(home, ".gitcd.yaml") + writeFile(t, cfgPath, "totallyUnknownKey: 1\n") + + beforeCfg, err := os.ReadFile(cfgPath) + if err != nil { + t.Fatalf("read %s: %v", cfgPath, err) + } + beforeGlobal, err := os.ReadFile(globalPath) + if err != nil { + t.Fatalf("read %s: %v", globalPath, err) + } + + var stdout, stderr bytes.Buffer + code := runInit([]string{"--move-identity=yes"}, strings.NewReader(""), &stdout, &stderr) + if code == 0 { + t.Fatalf("init --move-identity=yes = 0, want a failure when the updated configuration would not load, stdout: %s", stdout.String()) + } + + afterCfg, err := os.ReadFile(cfgPath) + if err != nil { + t.Fatalf("read %s: %v", cfgPath, err) + } + if string(afterCfg) != string(beforeCfg) { + t.Fatalf("init changed %s despite aborting:\nbefore: %q\nafter: %q", cfgPath, beforeCfg, afterCfg) + } + + afterGlobal, err := os.ReadFile(globalPath) + if err != nil { + t.Fatalf("read %s: %v", globalPath, err) + } + if string(afterGlobal) != string(beforeGlobal) { + t.Fatalf("init removed the global identity despite aborting:\nbefore: %q\nafter: %q", beforeGlobal, afterGlobal) + } + + entries, err := os.ReadDir(home) + if err != nil { + t.Fatalf("read dir %s: %v", home, err) + } + for _, e := range entries { + if strings.HasPrefix(e.Name(), filepath.Base(cfgPath)+".gitcd-validate-tmp") { + t.Fatalf("init left a temporary validation file behind: %s", e.Name()) + } + } +} + +// TestRunInit_NeverClaimsSuccessWhileLosingAMoveIntoABareProfilesKey is +// the exact reproduction pull request 12's review returned as this +// round's blocker: a hand written ~/.gitcd.yaml that declares "profiles:" +// and leaves it empty, the shape "profiles:\n" decodes to a "!!null" +// scalar node rather than a mapping node. "init --move-identity=yes" must +// not be able to print that it moved the identity, remove it from the +// global git configuration, exit 0, and leave ~/.gitcd.yaml byte for byte +// unchanged: either the move genuinely lands in the file, or the command +// refuses and leaves everything, including the global identity, exactly +// as it was. +func TestRunInit_NeverClaimsSuccessWhileLosingAMoveIntoABareProfilesKey(t *testing.T) { + home := setupEnv(t) + globalPath := globalConfigPath(home) + cfgPath := filepath.Join(home, ".gitcd.yaml") + writeFile(t, cfgPath, "root: ~/Code\nprofiles:\n") + + beforeCfg, err := os.ReadFile(cfgPath) + if err != nil { + t.Fatalf("read %s: %v", cfgPath, err) + } + beforeGlobal, err := os.ReadFile(globalPath) + if err != nil { + t.Fatalf("read %s: %v", globalPath, err) + } + + var stdout, stderr bytes.Buffer + code := runInit([]string{"--move-identity=yes"}, strings.NewReader(""), &stdout, &stderr) + + afterCfg, err := os.ReadFile(cfgPath) + if err != nil { + t.Fatalf("read %s: %v", cfgPath, err) + } + afterGlobal, err := os.ReadFile(globalPath) + if err != nil { + t.Fatalf("read %s: %v", globalPath, err) + } + cfgCarriesMove := strings.Contains(string(afterCfg), "[email protected]") + globalKeysGone := !strings.Contains(string(afterGlobal), "Test User") && !strings.Contains(string(afterGlobal), "[email protected]") + + // The one outcome that must never happen, regardless of exit code: + // the identity gone from the global git configuration while + // ~/.gitcd.yaml does not actually carry it. + if globalKeysGone && !cfgCarriesMove { + t.Fatalf( + "init removed the global identity without %s carrying the move: exit=%d stdout=%q stderr=%q cfg after=%q", + cfgPath, code, stdout.String(), stderr.String(), afterCfg, + ) + } + + switch code { + case 0: + if !cfgCarriesMove { + t.Fatalf("init exited 0 without %s carrying the moved identity: %q", cfgPath, afterCfg) + } + if !globalKeysGone { + t.Fatalf("init exited 0 without removing the global identity: %q", afterGlobal) + } + default: + if string(afterCfg) != string(beforeCfg) { + t.Fatalf("init refused but still changed %s:\nbefore: %q\nafter: %q", cfgPath, beforeCfg, afterCfg) + } + if string(afterGlobal) != string(beforeGlobal) { + t.Fatalf("init refused but still changed the global git configuration:\nbefore: %q\nafter: %q", beforeGlobal, afterGlobal) + } + } +} + +// TestRunInit_DoesNotFollowAPreexistingSymlinkAtTheValidationTempPath +// covers the review's third "should": the validation copy used to be +// written to a fixed, predictable path next to cfgPath with os.WriteFile, +// which follows a symlink already sitting there. A symlink planted at +// that exact legacy name, pointing anywhere else, must not have its +// target overwritten by this command. +func TestRunInit_DoesNotFollowAPreexistingSymlinkAtTheValidationTempPath(t *testing.T) { + home := setupEnv(t) + cfgPath := filepath.Join(home, ".gitcd.yaml") + + canary := filepath.Join(home, "canary.txt") + writeFile(t, canary, "do not touch\n") + + legacyTmpPath := cfgPath + ".gitcd-validate-tmp" + if err := os.Symlink(canary, legacyTmpPath); err != nil { + t.Fatalf("symlink %s -> %s: %v", legacyTmpPath, canary, err) + } + + var stdout, stderr bytes.Buffer + code := runInit([]string{"--move-identity=yes"}, strings.NewReader(""), &stdout, &stderr) + if code != 0 { + t.Fatalf("init --move-identity=yes = %d, want 0, stderr: %s", code, stderr.String()) + } + + canaryAfter, err := os.ReadFile(canary) + if err != nil { + t.Fatalf("read %s: %v", canary, err) + } + if string(canaryAfter) != "do not touch\n" { + t.Fatalf("canary content = %q, want it untouched: the validation step wrote through a preexisting symlink at its old, predictable path", canaryAfter) + } + + linkTarget, err := os.Readlink(legacyTmpPath) + if err != nil { + t.Fatalf("readlink %s: %v", legacyTmpPath, err) + } + if linkTarget != canary { + t.Fatalf("legacy symlink target = %q, want it left pointing at %q", linkTarget, canary) + } + + cfgContent, err := os.ReadFile(cfgPath) + if err != nil { + t.Fatalf("init did not write %s: %v", cfgPath, err) + } + if !strings.Contains(string(cfgContent), "[email protected]") { + t.Fatalf("%s = %q, want the moved email under profiles.global", cfgPath, cfgContent) + } +} + +// TestRunInit_WritesThroughASymlinkedGitcdYAMLRatherThanReplacingIt +// covers the review's second "should": ~/.gitcd.yaml symlinked into a +// dotfiles repository, the common setup. The rename this command used to +// do landed directly on the symlink's own path, replacing the link with +// an ordinary file and leaving the dotfiles copy holding the stale +// content. The link, and the dotfiles copy it points at, must both still +// carry the move afterwards. +func TestRunInit_WritesThroughASymlinkedGitcdYAMLRatherThanReplacingIt(t *testing.T) { + home := setupEnv(t) + dotfilesCopy := filepath.Join(home, "dotfiles", "gitcd.yaml") + writeFile(t, dotfilesCopy, "profiles:\n work:\n user.email: work@example.test\n") + + cfgPath := filepath.Join(home, ".gitcd.yaml") + if err := os.Symlink(dotfilesCopy, cfgPath); err != nil { + t.Fatalf("symlink %s -> %s: %v", cfgPath, dotfilesCopy, err) + } + + var stdout, stderr bytes.Buffer + code := runInit([]string{"--move-identity=yes"}, strings.NewReader(""), &stdout, &stderr) + if code != 0 { + t.Fatalf("init --move-identity=yes = %d, want 0, stderr: %s", code, stderr.String()) + } + + info, err := os.Lstat(cfgPath) + if err != nil { + t.Fatalf("lstat %s: %v", cfgPath, err) + } + if info.Mode()&os.ModeSymlink == 0 { + t.Fatalf("%s is no longer a symlink after the move, want the dotfiles link kept intact", cfgPath) + } + linkTarget, err := os.Readlink(cfgPath) + if err != nil { + t.Fatalf("readlink %s: %v", cfgPath, err) + } + if linkTarget != dotfilesCopy { + t.Fatalf("%s now points at %q, want it still pointing at %q", cfgPath, linkTarget, dotfilesCopy) + } + + dotfilesContent, err := os.ReadFile(dotfilesCopy) + if err != nil { + t.Fatalf("read %s: %v", dotfilesCopy, err) + } + if !strings.Contains(string(dotfilesContent), "work@example.test") { + t.Fatalf("%s = %q, want the pre-existing profile untouched", dotfilesCopy, dotfilesContent) + } + if !strings.Contains(string(dotfilesContent), "[email protected]") { + t.Fatalf("%s = %q, the dotfiles copy itself must carry the moved identity", dotfilesCopy, dotfilesContent) + } + + // Reading through the symlink must agree with reading the dotfiles + // copy directly: both name the same file. + viaLink, err := os.ReadFile(cfgPath) + if err != nil { + t.Fatalf("read %s: %v", cfgPath, err) + } + if string(viaLink) != string(dotfilesContent) { + t.Fatalf("reading %s and %s disagree:\nvia link: %q\ndotfiles: %q", cfgPath, dotfilesCopy, viaLink, dotfilesContent) + } +} + +// TestRunInit_MultipleDocumentsRefusalNamesTheFile covers the nit left +// standing across both review rounds: ErrMultipleDocuments's own message +// never names the file it refused, so the printed refusal gave no path +// to look at, even though the configuration may sit at either of two +// candidate locations. +func TestRunInit_MultipleDocumentsRefusalNamesTheFile(t *testing.T) { + home := setupEnv(t) + cfgPath := filepath.Join(home, ".gitcd.yaml") + writeFile(t, cfgPath, + "profiles:\n work:\n user.email: work@example.test\n"+ + "---\n"+ + "profiles:\n personal:\n user.email: personal@example.test\n", + ) + + var stdout, stderr bytes.Buffer + code := runInit([]string{"--move-identity=yes"}, strings.NewReader(""), &stdout, &stderr) + if code == 0 { + t.Fatalf("init --move-identity=yes = 0, want a refusal for a multi document file, stdout: %s", stdout.String()) + } + if !strings.Contains(stderr.String(), "more than one YAML document") { + t.Fatalf("init stderr = %q, want it to name the multi document refusal", stderr.String()) + } + if !strings.Contains(stderr.String(), cfgPath) { + t.Fatalf("init stderr = %q, want it to name %s, the file it refused", stderr.String(), cfgPath) + } +} + +// TestTargetProfileConflict_RefusesWhenTheLenientParseItselfFails covers +// the first "should": targetProfileConflict used to treat any parse +// failure of its own, lenient decode as "no conflict", waving a move +// through for a file it could not actually make sense of. +func TestTargetProfileConflict_RefusesWhenTheLenientParseItselfFails(t *testing.T) { + home := setupEnv(t) + cfgPath := filepath.Join(home, ".gitcd.yaml") + writeFile(t, cfgPath, "profiles:\n - not\n - a\n - mapping\n") + + msg, err := targetProfileConflict(cfgPath, "global") + if err != nil { + t.Fatalf("targetProfileConflict() returned an unexpected error: %v", err) + } + if msg == "" { + t.Fatal("targetProfileConflict() = \"\" (no conflict), want a refusal when the file does not even parse") + } +} + +// TestTargetProfileConflict_AnUnrelatedUnknownKeyAloneIsNotAConflict +// confirms the fix above stays independent of config.LoadFrom's own, +// stricter unknown key rejection: a key this lenient parse does not name +// at all must not, by itself, read as a parse failure. +func TestTargetProfileConflict_AnUnrelatedUnknownKeyAloneIsNotAConflict(t *testing.T) { + home := setupEnv(t) + cfgPath := filepath.Join(home, ".gitcd.yaml") + writeFile(t, cfgPath, "totallyUnknownKey: 1\n") + + msg, err := targetProfileConflict(cfgPath, "global") + if err != nil { + t.Fatalf("targetProfileConflict() returned an unexpected error: %v", err) + } + if msg != "" { + t.Fatalf("targetProfileConflict() = %q, want no conflict for a file whose only problem is unrelated to profiles or defaultProfile", msg) + } +} + +// TestTargetProfileConflict_AParseFailureIsNotMaskedByAnUnrelatedUnknownKey +// combines both: a genuinely malformed "profiles" key must still be +// refused even alongside an unrelated unknown key, rather than either +// masking the other. +func TestTargetProfileConflict_AParseFailureIsNotMaskedByAnUnrelatedUnknownKey(t *testing.T) { + home := setupEnv(t) + cfgPath := filepath.Join(home, ".gitcd.yaml") + writeFile(t, cfgPath, "totallyUnknownKey: 1\nprofiles:\n - not\n - a\n - mapping\n") + + msg, err := targetProfileConflict(cfgPath, "global") + if err != nil { + t.Fatalf("targetProfileConflict() returned an unexpected error: %v", err) + } + if msg == "" { + t.Fatal("targetProfileConflict() = \"\" (no conflict), want the malformed profiles key refused alongside an unrelated unknown key") + } +} + +// TestIdentityKeysLanded is the unit level proof for the blocker's belt +// and braces guarantee: moveGlobalIdentity must abort, rather than go on +// to remove anything from the global git configuration, whenever the +// configuration it is about to rename into place does not actually carry +// every key it claims to have moved. +func TestIdentityKeysLanded(t *testing.T) { + t.Parallel() + found := []git.GlobalIdentityValue{{Key: "user.email", Value: "[email protected]"}} + + t.Run("reports an error when the profile does not carry the moved key", func(t *testing.T) { + t.Parallel() + cfg := config.Config{Profiles: map[string]config.Profile{"global": {}}} + if err := identityKeysLanded(cfg, "global", found); err == nil { + t.Fatal("identityKeysLanded() = nil, want an error when the profile does not carry the key") + } + }) + + t.Run("reports an error when the profile does not exist at all", func(t *testing.T) { + t.Parallel() + cfg := config.Config{} + if err := identityKeysLanded(cfg, "global", found); err == nil { + t.Fatal("identityKeysLanded() = nil, want an error when the profile does not exist at all") + } + }) + + t.Run("reports an error when the value does not match what was moved", func(t *testing.T) { + t.Parallel() + cfg := config.Config{Profiles: map[string]config.Profile{"global": {"user.email": "wrong@example.test"}}} + if err := identityKeysLanded(cfg, "global", found); err == nil { + t.Fatal("identityKeysLanded() = nil, want an error when the value does not match") + } + }) + + t.Run("accepts a profile carrying every moved key with its exact value", func(t *testing.T) { + t.Parallel() + cfg := config.Config{Profiles: map[string]config.Profile{"global": {"user.email": "[email protected]"}}} + if err := identityKeysLanded(cfg, "global", found); err != nil { + t.Fatalf("identityKeysLanded() = %v, want nil when every moved key matches", err) + } + }) +} + func TestRunInit_NeverWritesToTheRealHomeDirectory(t *testing.T) { // A sanity check on this test file's own isolation, not gitcd's // behaviour: every init test above must run with os.UserHomeDir() diff --git a/cmd/gitcd/main.go b/cmd/gitcd/main.go index 8e563c5..284c967 100644 --- a/cmd/gitcd/main.go +++ b/cmd/gitcd/main.go @@ -54,7 +54,7 @@ func run(args []string, stdout, stderr io.Writer) int { case "path": return runPath(args[1:], stdout, stderr) case "init": - return runInit(args[1:], stdout, stderr) + return runInit(args[1:], os.Stdin, stdout, stderr) case "apply": return runApply(args[1:], stdout, stderr) case "profile": diff --git a/docs/concepts/gitcd.md b/docs/concepts/gitcd.md index d0383c1..53de138 100644 --- a/docs/concepts/gitcd.md +++ b/docs/concepts/gitcd.md @@ -49,6 +49,18 @@ identity: a generated `includeIf` config for the common case, and a pinned local config write for the explicit override, so a repository always has one clear, inspectable reason for the identity it carries. +- `user.useConfigOnly` only stops git from guessing `user.name` and `user.email` from the + account's username and hostname; it never suppresses a value that is already configured, + from any source. `apply` writes it twice for that reason: unconditionally at the top of the + generated include file, so a repository no rule matches and no `defaultProfile` covers still + refuses to commit, and again inside every profile file, redundant for a matched repository + but needed on its own wherever a profile file is read outside that include chain. Neither + copy protects against a `user.name`, `user.email`, `user.signingkey`, `gpg.format` or + `commit.gpgsign` set directly in the user's own global git configuration, outside gitcd's + generated files: that value is already "configuration" by git's own definition, and applies + to every repository, one some rule matches or not. `init` detects exactly this case, reports + every such key, and, only when asked, moves it into a profile and removes it from the global + file, with a backup. - `internal/scan` walks the configured root into repository values for `list`, `doctor` and `status` to consume, treating a `.git` file, not only a `.git` directory, as a repository boundary, so a linked worktree is never descended into by mistake. diff --git a/internal/config/config.go b/internal/config/config.go index 9ccd683..cfedcf3 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -226,6 +226,25 @@ func defaultForHome(home string) (Config, error) { return cfg, nil } +// Path returns the configuration file Load reads: the first of its +// candidate paths that exists, or its primary candidate, "~/.gitcd.yaml" +// resolved against the calling user's home directory, when neither does. +// A caller that edits the file a person maintains by hand, "init" moving +// a global identity key into a profile for instance, writes here, so an +// edit always lands on the same file Load would read back. +func Path() (string, error) { + home, err := os.UserHomeDir() + if err != nil { + return "", fmt.Errorf("config: find the home directory: %w", err) + } + for _, path := range candidatePaths(home) { + if _, err := os.Stat(path); err == nil { + return path, nil + } + } + return candidatePaths(home)[0], nil +} + // candidatePaths lists, in the order Load tries them, the paths a // configuration file may live at for a user whose home directory is home. func candidatePaths(home string) []string { diff --git a/internal/config/config_test.go b/internal/config/config_test.go index c6eabbc..884f803 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -304,6 +304,47 @@ hosts: }) } +func TestPath(t *testing.T) { + t.Run("the primary candidate when neither file exists", func(t *testing.T) { + home := withHome(t) + + got, err := config.Path() + if err != nil { + t.Fatalf("Path() returned an unexpected error: %v", err) + } + if want := filepath.Join(home, ".gitcd.yaml"); got != want { + t.Fatalf("Path() = %q, want %q", got, want) + } + }) + + t.Run("the file Load would read when it already exists", func(t *testing.T) { + home := withHome(t) + writeFile(t, filepath.Join(home, ".gitcd.yaml"), "root: ~/elsewhere\n") + + got, err := config.Path() + if err != nil { + t.Fatalf("Path() returned an unexpected error: %v", err) + } + if want := filepath.Join(home, ".gitcd.yaml"); got != want { + t.Fatalf("Path() = %q, want %q", got, want) + } + }) + + t.Run("the xdg fallback when only it exists", func(t *testing.T) { + home := withHome(t) + xdgPath := filepath.Join(home, ".config", "gitcd", "config.yaml") + writeFile(t, xdgPath, "root: ~/elsewhere\n") + + got, err := config.Path() + if err != nil { + t.Fatalf("Path() returned an unexpected error: %v", err) + } + if got != xdgPath { + t.Fatalf("Path() = %q, want %q", got, xdgPath) + } + }) +} + func TestLoad_Profiles(t *testing.T) { t.Run("no file ships useConfigOnly true and no profiles", func(t *testing.T) { withHome(t) diff --git a/internal/config/edit.go b/internal/config/edit.go new file mode 100644 index 0000000..7e46a99 --- /dev/null +++ b/internal/config/edit.go @@ -0,0 +1,220 @@ +// Copyright 2026 Simon Ostendorf +// SPDX-License-Identifier: Apache-2.0 + +package config + +import ( + "bytes" + "errors" + "fmt" + "io" + "sort" + + "gopkg.in/yaml.v3" +) + +// AddProfileKeys adds or overwrites key/value pairs under profiles.name in +// the YAML document existing holds, creating the top level "profiles" +// map, name's own map, or the document itself, whichever is missing, and +// preserving every comment and the rest of the document's structure +// otherwise. existing may be empty, in which case a fresh document is +// built. +// +// This is the file a person edits by hand, not one gitcd owns and +// regenerates, so the edit goes through gopkg.in/yaml.v3's comment +// preserving Node tree rather than a decode into a plain struct followed +// by a full re-encode, which would silently drop every comment the file +// carried. +func AddProfileKeys(existing []byte, name string, keys map[string]string) ([]byte, error) { + doc, err := parseOrEmptyDocument(existing) + if err != nil { + return nil, err + } + + root := doc.Content[0] + if root.Kind != yaml.MappingNode { + return nil, fmt.Errorf("config: the existing configuration is not a YAML mapping at its top level") + } + + profiles, err := mappingChild(root, "profiles") + if err != nil { + return nil, err + } + profile, err := mappingChild(profiles, name) + if err != nil { + return nil, err + } + for _, key := range sortedStringKeys(keys) { + setScalarChild(profile, key, keys[key]) + } + + out, err := yaml.Marshal(doc) + if err != nil { + return nil, fmt.Errorf("config: render the updated configuration: %w", err) + } + return out, nil +} + +// ErrMultipleDocuments is wrapped into the error AddProfileKeys returns +// when the existing configuration holds more than one YAML document: +// gopkg.in/yaml.v3 decodes and re-encodes only the first one, which would +// silently drop every document after it from the file this command +// rewrites. +var ErrMultipleDocuments = errors.New("config: the existing configuration holds more than one YAML document") + +// parseOrEmptyDocument parses existing as a single YAML document node, or +// returns a fresh document holding an empty mapping when existing is +// empty: a configuration file that does not exist yet, or exists but is +// blank, is not a parse error, matching LoadFrom's own treatment of a +// missing file. +// +// existing holding more than one YAML document is refused with +// ErrMultipleDocuments rather than edited: this package's own +// yaml.v3.Node round trip, like config.LoadFrom, only ever sees the +// first document, so silently writing back just that one would discard +// every document after it with nothing printed to say so. A person who +// deliberately split the file this way is expected to combine it into +// one document by hand before running a command that edits it. +func parseOrEmptyDocument(existing []byte) (*yaml.Node, error) { + if len(existing) == 0 { + return &yaml.Node{ + Kind: yaml.DocumentNode, + Content: []*yaml.Node{{Kind: yaml.MappingNode, Tag: "!!map"}}, + }, nil + } + + dec := yaml.NewDecoder(bytes.NewReader(existing)) + + var doc yaml.Node + if err := dec.Decode(&doc); err != nil && !errors.Is(err, io.EOF) { + return nil, fmt.Errorf("config: parse the existing configuration: %w", err) + } + + if err := nextDocumentExists(dec); err != nil { + return nil, err + } + + if doc.Kind == 0 || len(doc.Content) == 0 { + // A document holding only comments, or nothing at all, decodes + // into a node with no content: the same empty case as a missing + // file, just already having gone through the parser. + return &yaml.Node{ + Kind: yaml.DocumentNode, + HeadComment: doc.HeadComment, + Content: []*yaml.Node{{Kind: yaml.MappingNode, Tag: "!!map"}}, + }, nil + } + return &doc, nil +} + +// nextDocumentExists reports, as ErrMultipleDocuments, whether dec still +// holds a second YAML document after the one parseOrEmptyDocument already +// decoded. A trailing, empty document, the shape a lone "---" end marker +// with nothing meaningful after it decodes to, is not treated as a real +// second document: only one that itself carries content counts. +func nextDocumentExists(dec *yaml.Decoder) error { + var next yaml.Node + err := dec.Decode(&next) + switch { + case errors.Is(err, io.EOF): + return nil + case err != nil: + return fmt.Errorf("config: parse the existing configuration: %w", err) + case isEmptyDocument(&next): + return nil + default: + return ErrMultipleDocuments + } +} + +// isEmptyDocument reports whether doc, a yaml.Node decoded as a whole +// document, carries no real content: either no document node at all, or +// a document node wrapping nothing but an implicit "!!null" scalar, the +// shape a bare "---" document separator with nothing meaningful after it +// decodes to. +func isEmptyDocument(doc *yaml.Node) bool { + if doc.Kind == 0 || len(doc.Content) == 0 { + return true + } + content := doc.Content[0] + return content.Kind == yaml.ScalarNode && content.Tag == "!!null" +} + +// errNotFound is returned internally by mappingLookup when key is not +// among parent's children; mappingChild and setScalarChild both treat it +// as "create one" rather than an error, so it never escapes this file. +var errNotFound = errors.New("config: key not found in mapping") + +// mappingChild returns parent's child mapping node keyed name, appending +// both the key and a fresh, empty mapping value when parent does not +// already carry one. +// +// A child that already exists but decoded as an empty "!!null" scalar is +// turned into a mapping node in place rather than returned as is: that is +// exactly the shape a bare "profiles:" or "defaultProfile:", with nothing +// after the colon, decodes to, and appending a key/value pair to a +// scalar node's Content is silently ignored by yaml.Marshal, which let +// "init" report a move that never reached the file. A child that exists +// as some other, non-mapping, non-null shape, a list or a quoted string +// for instance, is refused instead of silently coerced, since turning it +// into a mapping would discard whatever a person deliberately wrote +// there. +func mappingChild(parent *yaml.Node, name string) (*yaml.Node, error) { + value, err := mappingLookup(parent, name) + if errors.Is(err, errNotFound) { + key := &yaml.Node{Kind: yaml.ScalarNode, Tag: "!!str", Value: name} + value = &yaml.Node{Kind: yaml.MappingNode, Tag: "!!map"} + parent.Content = append(parent.Content, key, value) + return value, nil + } + switch { + case value.Kind == yaml.MappingNode: + return value, nil + case value.Kind == yaml.ScalarNode && value.Tag == "!!null": + value.Kind = yaml.MappingNode + value.Tag = "!!map" + value.Value = "" + value.Style = 0 + value.Content = nil + return value, nil + default: + return nil, fmt.Errorf("config: %q is not a YAML mapping in the existing configuration", name) + } +} + +// setScalarChild sets parent's child scalar value keyed key to value, +// updating an existing entry's value node in place so any style or +// comment that particular node already carries is kept, or appending a +// freshly built key/value pair when parent carries no such key yet. +func setScalarChild(parent *yaml.Node, key, value string) { + if existing, err := mappingLookup(parent, key); err == nil { + existing.SetString(value) + return + } + keyNode := &yaml.Node{Kind: yaml.ScalarNode, Tag: "!!str", Value: key} + valueNode := &yaml.Node{} + valueNode.SetString(value) + parent.Content = append(parent.Content, keyNode, valueNode) +} + +// mappingLookup returns parent's value node keyed key, or errNotFound +// when parent, a mapping node, carries no such key. +func mappingLookup(parent *yaml.Node, key string) (*yaml.Node, error) { + for i := 0; i+1 < len(parent.Content); i += 2 { + if parent.Content[i].Value == key { + return parent.Content[i+1], nil + } + } + return nil, errNotFound +} + +// sortedStringKeys returns m's keys sorted, so a caller writing them into +// a document a person reads gets a deterministic order. +func sortedStringKeys(m map[string]string) []string { + keys := make([]string, 0, len(m)) + for k := range m { + keys = append(keys, k) + } + sort.Strings(keys) + return keys +} diff --git a/internal/config/edit_test.go b/internal/config/edit_test.go new file mode 100644 index 0000000..fb3a712 --- /dev/null +++ b/internal/config/edit_test.go @@ -0,0 +1,186 @@ +// Copyright 2026 Simon Ostendorf +// SPDX-License-Identifier: Apache-2.0 + +package config_test + +import ( + "errors" + "path/filepath" + "strings" + "testing" + + "github.com/ostendorf-cloud/gitcd/internal/config" +) + +func TestAddProfileKeys(t *testing.T) { + t.Parallel() + + t.Run("builds a fresh document when none exists yet", func(t *testing.T) { + t.Parallel() + out, err := config.AddProfileKeys(nil, "global", map[string]string{"user.email": "me@example.test"}) + if err != nil { + t.Fatalf("AddProfileKeys() returned an unexpected error: %v", err) + } + + path := filepath.Join(t.TempDir(), "gitcd.yaml") + writeFile(t, path, string(out)) + cfg, err := config.LoadFrom(path) + if err != nil { + t.Fatalf("LoadFrom() the result returned an unexpected error: %v", err) + } + if got, want := cfg.Profiles["global"]["user.email"], "me@example.test"; got != want { + t.Fatalf("profiles.global.user.email = %q, want %q", got, want) + } + }) + + t.Run("adds a new profile without disturbing an existing one", func(t *testing.T) { + t.Parallel() + existing := "profiles:\n work:\n user.email: work@example.test\n" + + out, err := config.AddProfileKeys([]byte(existing), "global", map[string]string{"user.name": "Jane Doe"}) + if err != nil { + t.Fatalf("AddProfileKeys() returned an unexpected error: %v", err) + } + + path := filepath.Join(t.TempDir(), "gitcd.yaml") + writeFile(t, path, string(out)) + cfg, err := config.LoadFrom(path) + if err != nil { + t.Fatalf("LoadFrom() the result returned an unexpected error: %v", err) + } + if got, want := cfg.Profiles["work"]["user.email"], "work@example.test"; got != want { + t.Fatalf("profiles.work.user.email = %q, want %q, the existing profile must be untouched", got, want) + } + if got, want := cfg.Profiles["global"]["user.name"], "Jane Doe"; got != want { + t.Fatalf("profiles.global.user.name = %q, want %q", got, want) + } + }) + + t.Run("overwrites an existing key in the named profile rather than duplicating it", func(t *testing.T) { + t.Parallel() + existing := "profiles:\n global:\n user.email: old@example.test\n user.name: Old Name\n" + + out, err := config.AddProfileKeys([]byte(existing), "global", map[string]string{"user.email": "new@example.test"}) + if err != nil { + t.Fatalf("AddProfileKeys() returned an unexpected error: %v", err) + } + + path := filepath.Join(t.TempDir(), "gitcd.yaml") + writeFile(t, path, string(out)) + cfg, err := config.LoadFrom(path) + if err != nil { + t.Fatalf("LoadFrom() the result returned an unexpected error: %v", err) + } + if got, want := cfg.Profiles["global"]["user.email"], "new@example.test"; got != want { + t.Fatalf("profiles.global.user.email = %q, want %q", got, want) + } + if got, want := cfg.Profiles["global"]["user.name"], "Old Name"; got != want { + t.Fatalf("profiles.global.user.name = %q, want %q, an untouched key must survive", got, want) + } + }) + + t.Run("preserves a comment elsewhere in the document", func(t *testing.T) { + t.Parallel() + existing := "# a hand written note a person left for themselves\nroot: ~/Code\n" + + out, err := config.AddProfileKeys([]byte(existing), "global", map[string]string{"user.email": "me@example.test"}) + if err != nil { + t.Fatalf("AddProfileKeys() returned an unexpected error: %v", err) + } + if !strings.Contains(string(out), "a hand written note a person left for themselves") { + t.Fatalf("AddProfileKeys() dropped the document's own comment:\n%s", out) + } + }) + + t.Run("writes into a bare profiles key with no value rather than silently losing the write", func(t *testing.T) { + t.Parallel() + // pull request 12's exact reproduction: a hand written file that + // declares "profiles:" and leaves it empty. That decodes to a + // "!!null" scalar node, not a mapping node, so appending the new + // profile's key and value to it must not be silently swallowed by + // yaml.Marshal the way it was before this fix. + existing := "root: /tmp/example/Code\nprofiles:\n" + + out, err := config.AddProfileKeys([]byte(existing), "global", map[string]string{"user.email": "me@example.test"}) + if err != nil { + t.Fatalf("AddProfileKeys() returned an unexpected error for a bare \"profiles:\" key: %v", err) + } + + path := filepath.Join(t.TempDir(), "gitcd.yaml") + writeFile(t, path, string(out)) + cfg, err := config.LoadFrom(path) + if err != nil { + t.Fatalf("LoadFrom() the result returned an unexpected error: %v", err) + } + if got, want := cfg.Profiles["global"]["user.email"], "me@example.test"; got != want { + t.Fatalf("profiles.global.user.email = %q, want %q; AddProfileKeys() must not report success while losing the write", got, want) + } + }) + + t.Run("writes into a profile name already present with no value the same way", func(t *testing.T) { + t.Parallel() + // The same "!!null" scalar shape, one call deeper: "profiles" + // already exists, but the target profile name sits under it as a + // bare key, the shape a person placing a profile name before + // filling it in would plausibly write. + existing := "profiles:\n global:\n" + + out, err := config.AddProfileKeys([]byte(existing), "global", map[string]string{"user.name": "Jane Doe"}) + if err != nil { + t.Fatalf("AddProfileKeys() returned an unexpected error for a bare profile name: %v", err) + } + + path := filepath.Join(t.TempDir(), "gitcd.yaml") + writeFile(t, path, string(out)) + cfg, err := config.LoadFrom(path) + if err != nil { + t.Fatalf("LoadFrom() the result returned an unexpected error: %v", err) + } + if got, want := cfg.Profiles["global"]["user.name"], "Jane Doe"; got != want { + t.Fatalf("profiles.global.user.name = %q, want %q; AddProfileKeys() must not report success while losing the write", got, want) + } + }) + + t.Run("rejects a document that is not a mapping at its top level", func(t *testing.T) { + t.Parallel() + _, err := config.AddProfileKeys([]byte("- not\n- a\n- mapping\n"), "global", map[string]string{"user.email": "me@example.test"}) + if err == nil { + t.Fatal("AddProfileKeys() succeeded, want an error for a non mapping top level document") + } + }) + + t.Run("refuses a file holding more than one YAML document rather than dropping the rest", func(t *testing.T) { + t.Parallel() + existing := "profiles:\n work:\n user.email: work@example.test\n" + + "---\n" + + "profiles:\n personal:\n user.email: personal@example.test\n" + + _, err := config.AddProfileKeys([]byte(existing), "global", map[string]string{"user.email": "me@example.test"}) + if err == nil { + t.Fatal("AddProfileKeys() succeeded, want a refusal for a multi document file") + } + if !errors.Is(err, config.ErrMultipleDocuments) { + t.Fatalf("AddProfileKeys() error = %v, want it to wrap config.ErrMultipleDocuments", err) + } + }) + + t.Run("accepts a file ending in a bare document separator with nothing after it", func(t *testing.T) { + t.Parallel() + existing := "profiles:\n work:\n user.email: work@example.test\n---\n" + + out, err := config.AddProfileKeys([]byte(existing), "global", map[string]string{"user.email": "me@example.test"}) + if err != nil { + t.Fatalf("AddProfileKeys() returned an unexpected error for a trailing empty document: %v", err) + } + + path := filepath.Join(t.TempDir(), "gitcd.yaml") + writeFile(t, path, string(out)) + cfg, err := config.LoadFrom(path) + if err != nil { + t.Fatalf("LoadFrom() the result returned an unexpected error: %v", err) + } + if got, want := cfg.Profiles["work"]["user.email"], "work@example.test"; got != want { + t.Fatalf("profiles.work.user.email = %q, want %q", got, want) + } + }) +} diff --git a/internal/git/config.go b/internal/git/config.go index 44c249c..dc010f0 100644 --- a/internal/git/config.go +++ b/internal/git/config.go @@ -112,6 +112,21 @@ func ConfigUnsetLocal(ctx context.Context, path, key string) error { return nil } +// ConfigUnsetGlobal runs "git config --global --unset-all key", removing +// every global value key carries. It wraps ErrConfigKeyNotSet, rather than +// failing, when key already carries no global value. +func ConfigUnsetGlobal(ctx context.Context, key string) error { + cmd := exec.CommandContext(ctx, "git", "config", "--global", "--unset-all", key) + if out, err := cmd.CombinedOutput(); err != nil { + var exitErr *exec.ExitError + if errors.As(err, &exitErr) && exitErr.ExitCode() == 5 { + return fmt.Errorf("git: unset global %s: %w", key, ErrConfigKeyNotSet) + } + return fmt.Errorf("git: unset global %s: %w: %s", key, err, strings.TrimSpace(string(out))) + } + return nil +} + // ConfigValue is one git config key's effective value together with the // file git read it from. type ConfigValue struct { @@ -187,8 +202,10 @@ func revParsePath(ctx context.Context, path, arg string) (string, error) { var configSectionHeader = regexp.MustCompile(`^\s*\[\s*([A-Za-z0-9_.-]+)(?:\s+"([^"]*)")?\s*\]`) // configKeyLine matches a git config key assignment line, capturing the -// key name. -var configKeyLine = regexp.MustCompile(`^\s*([A-Za-z][A-Za-z0-9-]*)\s*(?:=.*)?$`) +// key name, whether it carries an explicit "=" (group 2, literally "=" or +// unmatched), and, when it does, the value's raw text after it (group 3), +// not yet unquoted. +var configKeyLine = regexp.MustCompile(`^\s*([A-Za-z][A-Za-z0-9-]*)\s*(?:(=)\s*(.*))?$`) // IdentityKeyAfterInclude reports whether any of keys sits, in file order, // after the last bare "[include]" section (no subsection) that content @@ -199,6 +216,15 @@ var configKeyLine = regexp.MustCompile(`^\s*([A-Za-z][A-Za-z0-9-]*)\s*(?:=.*)?$` // silently. found is false, with no error, when content carries no bare // "[include]" section at all, or when no key in keys sits after the one it // does carry. +// +// This is a narrower, ordering only concern: a key sitting before the +// include is "safe" by this function's own definition only in the sense +// that it is never overridden by a value gitcd's own includeIf chain +// sets. It says nothing about FindGlobalIdentityValues' concern, which is +// broader and applies regardless of position: any of these keys set +// directly anywhere in this file is already "configuration" by git's own +// definition, so it is used for every repository, matched by some rule or +// not, with or without ever reaching this function's notion of "after". func IdentityKeyAfterInclude(content []byte, keys []string) (key string, found bool, err error) { lines := strings.Split(string(content), "\n") @@ -245,6 +271,90 @@ func IdentityKeyAfterInclude(content []byte, keys []string) (key string, found b return "", false, nil } +// GlobalIdentityValue is one identity key the global git configuration +// sets directly, with the value it carries. +type GlobalIdentityValue struct { + Key string + Value string +} + +// FindGlobalIdentityValues scans content, the raw text of the user's +// global git configuration file, for every key in keys that it sets +// directly, in content's own file order, regardless of position relative +// to any "[include]" or "[includeIf]" section: a key set this way is +// already "configuration" by git's own definition, so +// "user.useConfigOnly" never suppresses it, and it applies to every +// repository gitcd manages, one some rule matches or not, exactly the gap +// IdentityKeyAfterInclude's narrower, ordering only check does not cover. +// The returned slice is ordered the same as keys, one entry per key that +// is set, carrying the last value assigned to it when content sets it +// more than once, consistent with git's own "last value wins" semantics +// for a single valued key. +func FindGlobalIdentityValues(content []byte, keys []string) []GlobalIdentityValue { + wanted := make(map[string]bool, len(keys)) + for _, k := range keys { + wanted[strings.ToLower(k)] = true + } + + values := make(map[string]string, len(keys)) + section := "" + for _, line := range strings.Split(string(content), "\n") { + if m := configSectionHeader.FindStringSubmatch(line); m != nil { + section = strings.ToLower(m[1]) + continue + } + if section == "" { + continue + } + m := configKeyLine.FindStringSubmatch(line) + if m == nil { + continue + } + full := section + "." + strings.ToLower(m[1]) + if !wanted[full] { + continue + } + if m[2] != "=" { + // A bare key with no "=" at all carries the implicit boolean + // value "true", the same as "git config --bool" reports it. + values[full] = "true" + continue + } + values[full] = parseConfigValue(m[3]) + } + + var found []GlobalIdentityValue + for _, k := range keys { + if v, ok := values[strings.ToLower(k)]; ok { + found = append(found, GlobalIdentityValue{Key: k, Value: v}) + } + } + return found +} + +// parseConfigValue extracts a key assignment's value from raw, the text a +// configKeyLine match captures after its "=". A value quoted with a +// single pair of double quotes is unquoted, undoing quoteValue's own +// escaping of "\\" and "\""; git's own handling of a quoted value's +// escape sequences is otherwise reproduced only for those two characters, +// which is all gitconfig.quoteValue, the renderer on the other side of +// this value, ever produces, and all the identity keys this package cares +// about are ever expected to carry. An unquoted value is trimmed of +// surrounding whitespace only, with no attempt to strip a trailing +// comment: this package's other textual checks are equally light, and an +// identity value carrying an inline "#" or ";" comment is not a shape any +// of them are expected to see. +func parseConfigValue(raw string) string { + raw = strings.TrimSpace(raw) + if len(raw) >= 2 && raw[0] == '"' && raw[len(raw)-1] == '"' { + inner := raw[1 : len(raw)-1] + inner = strings.ReplaceAll(inner, `\"`, `"`) + inner = strings.ReplaceAll(inner, `\\`, `\`) + return inner + } + return raw +} + // sortedConfigPairs returns pairs's keys sorted, so a caller building // "-c key=value" arguments gets a deterministic order. func sortedConfigPairs(pairs map[string]string) []string { diff --git a/internal/git/config_test.go b/internal/git/config_test.go index 48f3206..6a8375e 100644 --- a/internal/git/config_test.go +++ b/internal/git/config_test.go @@ -339,3 +339,126 @@ func TestIdentityKeyAfterInclude(t *testing.T) { // --add" always inserts a new key into that last matching section, so an // identity key sitting between two include sections ends up before the // insertion point, safe, while one sitting after both does not. + +func TestFindGlobalIdentityValues(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + content string + want []git.GlobalIdentityValue + }{ + { + name: "no identity key at all", + content: "[core]\n\teditor = vim\n", + want: nil, + }, + { + name: "a key before any include, the position IdentityKeyAfterInclude calls safe", + content: "[user]\n\tname = Test User\n\temail = test@example.test\n" + "[include]\n\tpath = /other.gitconfig\n", + want: []git.GlobalIdentityValue{ + {Key: "user.name", Value: "Test User"}, + {Key: "user.email", Value: "test@example.test"}, + }, + }, + { + name: "a key after an include is found too, same as before it", + content: "[include]\n\tpath = /other.gitconfig\n" + "[user]\n\tsigningkey = ssh-ed25519 AAAA\n", + want: []git.GlobalIdentityValue{ + {Key: "user.signingkey", Value: "ssh-ed25519 AAAA"}, + }, + }, + { + name: "a quoted value is unquoted, escaped characters undone", + content: "[user]\n\tname = \"Jane \\\"The Dev\\\" Doe\"\n", + want: []git.GlobalIdentityValue{ + {Key: "user.name", Value: `Jane "The Dev" Doe`}, + }, + }, + { + name: "a bare boolean key with no equals sign carries true", + content: "[commit]\n\tgpgsign\n", + want: []git.GlobalIdentityValue{ + {Key: "commit.gpgsign", Value: "true"}, + }, + }, + { + name: "a key under an unrelated section is ignored", + content: "[alias]\n\tname = \"Test\"\n", + want: nil, + }, + { + name: "an includeIf section is scanned too, not only a bare include", + content: "[includeIf \"gitdir:/some/path/\"]\n\tsigningkey = ssh-ed25519 AAAA\n", + want: nil, + }, + { + name: "the last value wins when a key is set more than once", + content: "[user]\n\temail = first@example.test\n[user]\n\temail = second@example.test\n", + want: []git.GlobalIdentityValue{ + {Key: "user.email", Value: "second@example.test"}, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got := git.FindGlobalIdentityValues([]byte(tt.content), git.IdentityKeys) + if len(got) != len(tt.want) { + t.Fatalf("FindGlobalIdentityValues() = %+v, want %+v", got, tt.want) + } + for i := range got { + if got[i] != tt.want[i] { + t.Fatalf("FindGlobalIdentityValues()[%d] = %+v, want %+v", i, got[i], tt.want[i]) + } + } + }) + } +} + +// TestFindGlobalIdentityValues_IncludeIfSectionNameRequiresNoSubsection +// documents a deliberate narrowing shared with IdentityKeyAfterInclude: +// "[includeIf \"...\"]" carries a subsection, the condition itself, so it +// is never mistaken for the section "includeif" when looking for an +// identity key, but it is also never itself scanned as a section named +// after its condition; the key lines under it belong to the subsectioned +// header's own section name, "includeif", which "user.name" and friends +// never collide with. This test exists so a future change to how +// sections are tracked has something to catch a regression against. +func TestFindGlobalIdentityValues_IncludeIfSectionNameRequiresNoSubsection(t *testing.T) { + t.Parallel() + content := "[includeIf \"gitdir:/some/path/\"]\n\tpath = /conditional.gitconfig\n[user]\n\temail = after@example.test\n" + got := git.FindGlobalIdentityValues([]byte(content), git.IdentityKeys) + want := []git.GlobalIdentityValue{{Key: "user.email", Value: "after@example.test"}} + if len(got) != 1 || got[0] != want[0] { + t.Fatalf("FindGlobalIdentityValues() = %+v, want %+v", got, want) + } +} + +func TestConfigUnsetGlobal(t *testing.T) { + setupGitEnv(t) + + t.Run("removes a value that was set", func(t *testing.T) { + if err := git.ConfigAddGlobal(context.Background(), "user.signingkey", "ssh-ed25519 AAAA"); err != nil { + t.Fatalf("ConfigAddGlobal() returned an unexpected error: %v", err) + } + + if err := git.ConfigUnsetGlobal(context.Background(), "user.signingkey"); err != nil { + t.Fatalf("ConfigUnsetGlobal() returned an unexpected error: %v", err) + } + + // A second unset of the same key now wraps ErrConfigKeyNotSet, + // which only happens once git itself reports no value left: the + // behavioural proof that the first call actually removed it. + if err := git.ConfigUnsetGlobal(context.Background(), "user.signingkey"); !errors.Is(err, git.ErrConfigKeyNotSet) { + t.Fatalf("ConfigUnsetGlobal() after removal, error = %v, want it to wrap ErrConfigKeyNotSet", err) + } + }) + + t.Run("unset a key that is not set wraps ErrConfigKeyNotSet instead of failing", func(t *testing.T) { + err := git.ConfigUnsetGlobal(context.Background(), "user.doesnotexist") + if !errors.Is(err, git.ErrConfigKeyNotSet) { + t.Fatalf("ConfigUnsetGlobal() error = %v, want it to wrap ErrConfigKeyNotSet", err) + } + }) +} diff --git a/internal/gitconfig/gitconfig.go b/internal/gitconfig/gitconfig.go index c29a333..194cd04 100644 --- a/internal/gitconfig/gitconfig.go +++ b/internal/gitconfig/gitconfig.go @@ -118,15 +118,35 @@ func RenderProfile(p config.Profile, useConfigOnly bool) (string, error) { return b.String(), nil } -// renderIncludes renders the include file: one plain "[include]" for -// cfg.DefaultProfile, then one "[includeIf]" block per URL form and one -// "[includeIf \"gitdir/i:...\"]" block, per rule, in ascending specificity -// order, so a later, more specific block always wins over an earlier, less -// specific one, regardless of the configuration file's own rule order. +// renderIncludes renders the include file: an unconditional +// "user.useConfigOnly" when cfg.UseConfigOnly is set, then one plain +// "[include]" for cfg.DefaultProfile, then one "[includeIf]" block per URL +// form and one "[includeIf \"gitdir/i:...\"]" block, per rule, in ascending +// specificity order, so a later, more specific block always wins over an +// earlier, less specific one, regardless of the configuration file's own +// rule order. +// +// The unconditional block is load bearing, not decorative: every other +// "useConfigOnly" this package writes sits inside a profile file, reached +// only through an includeIf condition, so it only ever applies to a +// repository some rule, or the default profile, already matches. A +// repository no rule matches, and no default profile covers, reaches +// neither, so without this unconditional copy it carries no +// "useConfigOnly" at all, and git falls back to guessing an identity from +// the account's username and hostname, silently, exactly the outcome a +// repository with an undetermined identity is supposed to refuse. +// Verified directly against git 2.54.0: with this block removed, a +// repository under neither a rule nor a default profile committed +// successfully under a guessed identity despite every profile file +// itself setting "useConfigOnly = true". func renderIncludes(cfg config.Config, home string) string { var b strings.Builder b.WriteString(generatedHeader) + if cfg.UseConfigOnly { + b.WriteString("\n[user]\n\tuseConfigOnly = true\n") + } + if cfg.DefaultProfile != "" { fmt.Fprintf(&b, "\n[include]\n\tpath = %s\n", quoteValue(ProfilePath(home, cfg.DefaultProfile))) } diff --git a/internal/gitconfig/gitconfig_test.go b/internal/gitconfig/gitconfig_test.go index 0423927..fa70115 100644 --- a/internal/gitconfig/gitconfig_test.go +++ b/internal/gitconfig/gitconfig_test.go @@ -297,6 +297,72 @@ func TestRender_HasconfigFormsPerRule(t *testing.T) { } } +// TestRender_UnconditionalUseConfigOnly guards the one block in the +// include file that applies to a repository no rule matches and no +// default profile covers: every other "useConfigOnly" this package +// writes sits inside a profile file, reached only through an includeIf +// condition that such a repository never satisfies. +func TestRender_UnconditionalUseConfigOnly(t *testing.T) { + t.Parallel() + + t.Run("present before every includeIf block when useConfigOnly is on", func(t *testing.T) { + t.Parallel() + cfg := config.Config{ + Root: "/home/simon/Code", + Default: "{host}/{owner}/{repo}", + UseConfigOnly: true, + Profiles: map[string]config.Profile{"private": {"user.email": "simon@example.test"}}, + Rules: []config.Rule{{Host: "github.com", Profile: "private"}}, + } + rendered, err := gitconfig.Render(cfg, "/home/simon") + if err != nil { + t.Fatalf("Render() returned an unexpected error: %v", err) + } + includes := rendered[gitconfig.IncludePath("/home/simon")] + + unconditional := strings.Index(includes, "[user]\n\tuseConfigOnly = true") + if unconditional == -1 { + t.Fatalf("Render() include file has no unconditional useConfigOnly block:\n%s", includes) + } + if firstIncludeIf := strings.Index(includes, "[includeIf"); firstIncludeIf != -1 && firstIncludeIf < unconditional { + t.Fatalf("Render() places the unconditional useConfigOnly block after an includeIf block:\n%s", includes) + } + }) + + t.Run("absent when the config opts out", func(t *testing.T) { + t.Parallel() + cfg := config.Config{ + Root: "/home/simon/Code", + Default: "{host}/{owner}/{repo}", + UseConfigOnly: false, + Profiles: map[string]config.Profile{"private": {"user.email": "simon@example.test"}}, + Rules: []config.Rule{{Host: "github.com", Profile: "private"}}, + } + rendered, err := gitconfig.Render(cfg, "/home/simon") + if err != nil { + t.Fatalf("Render() returned an unexpected error: %v", err) + } + includes := rendered[gitconfig.IncludePath("/home/simon")] + + if strings.Contains(includes, "useConfigOnly") { + t.Fatalf("Render() include file carries useConfigOnly despite opting out:\n%s", includes) + } + }) + + t.Run("present even with no profiles and no rules at all", func(t *testing.T) { + t.Parallel() + cfg := config.Config{Root: "/home/simon/Code", Default: "{host}/{owner}/{repo}", UseConfigOnly: true} + rendered, err := gitconfig.Render(cfg, "/home/simon") + if err != nil { + t.Fatalf("Render() returned an unexpected error: %v", err) + } + includes := rendered[gitconfig.IncludePath("/home/simon")] + if !strings.Contains(includes, "[user]\n\tuseConfigOnly = true") { + t.Fatalf("Render() with no profiles and no rules = %q, want the unconditional useConfigOnly block still present", includes) + } + }) +} + func TestRender_GitdirBlock(t *testing.T) { t.Parallel() diff --git a/internal/gitconfig/testdata/render.golden b/internal/gitconfig/testdata/render.golden index 2404289..e644678 100644 --- a/internal/gitconfig/testdata/render.golden +++ b/internal/gitconfig/testdata/render.golden @@ -1,6 +1,9 @@ === /home/simon/.config/gitcd/includes.gitconfig === # Generated by "gitcd apply". Edits here are overwritten the next time it runs. +[user] + useConfigOnly = true + [include] path = "/home/simon/.config/gitcd/profiles/private.gitconfig"