Fix review findings, add per-project config and multi-target pushremote

Correctness and security fixes found by a review of the initial commit:

- init decided whether a server repository existed from the *local*
  remote.origin.url, so an unlinked project directory skipped the
  confirmation and rm -rf'd the remote history. It now asks the server,
  and aborts when the server cannot be reached.
- Project names and paths were interpolated unquoted into the remote
  shell command strings: a space split one path into two arguments and a
  backtick executed on the git server. Everything now goes through shq(),
  and chained remote commands use && so a failed cd cannot let the next
  command run in the login directory.
- Bare `cd` panicked with an index-out-of-range and took down the shell;
  it now deselects the project.
- Command-line mode set PRJ to the whole path below BASE, so `mgsh push`
  from a subdirectory staged only that subtree and addressed a bogus
  server path. It now truncates at the first path element.
- `list` hardcoded owner and group "git git" in its regex and silently
  printed nothing on any server where the repositories are owned by
  someone else.
- The config parser kept inline "#" comments in values although the
  README and the example file document them, so `mirror = true # ...`
  silently disabled mirroring.
- ~/.mgshrc holds an API token but was created world-readable.
- The mirror token was passed on git's command line, visible in the
  process table; it now goes through GIT_CONFIG_*.
- tag, count and dist ran without a repository and operated on BASE.
- checkout dropped its git options, because the dispatcher strips -x
  flags from the word list.
- REPO was read with a plain `git config`, inheriting a foreign origin
  from an enclosing repository; it is now local-only and, being dead
  state otherwise, no longer recomputed on every prompt.
- getkey consumed a single byte, leaving the rest of a typed answer in
  the tty queue where readline ran it as a command.
- The REPL spun on any readline error that was neither EOF nor interrupt.
- Tab completion cached an empty repository list after one failed ssh.
- Startup did a blocking DNS lookup and three `git config --global`
  writes on every invocation.

New:

- A project may carry its own .mgshrc, overriding the global settings
  while it is active. Resolution order is ~/.mgshrc -> <project>/.mgshrc
  -> MGSH_*; base and the git identity keys stay global. It is read when
  the project changes, and `rescan` reloads it.
- pushremote mirrors to any number of servers, configured as
  remote.<name>.url/key/type/visibility blocks. `pushremote` pushes to
  all of them, `pushremote @name ...` to a selection, and `remotes = ...`
  restricts and orders the set. Each target owns a git remote of the same
  name; a failing target no longer stops the others.
- `config` shows the resolved configuration, its sources and the mirror
  targets with masked tokens; `config -k` lists the setting names.
- gitkey was parsed and documented but never used. It is now the ssh
  identity for the git server, for mgsh's own ssh calls and, via
  GIT_SSH_COMMAND, for the git commands mgsh runs.
- config, count, login and cloneall work from the command line too.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
2026-07-26 11:37:49 +02:00
co-authored by Claude Opus 5
parent 5562055695
commit ae8c7a3ec0
11 changed files with 1480 additions and 203 deletions
+465
View File
@@ -2,7 +2,9 @@ package main
import (
"os"
"os/exec"
"path/filepath"
"reflect"
"strconv"
"strings"
"testing"
@@ -115,6 +117,251 @@ base=/tmp/src
}
}
func TestParseConfigInlineComments(t *testing.T) {
rc := `
editor = code # fallback opener for ` + "`open`" + `
mirror = true # ` + "`push`" + ` also mirrors via pushremote
gitport = 22 # ssh port
remotekey = abc#123
remoteurl = "https://git.example.com" # quoted, comment after
gitname = ' Spaced # Name '
gitemail = # value is only a comment
`
m := parseConfig(rc)
checks := map[string]string{
"editor": "code",
"mirror": "true",
"gitport": "22",
"remotekey": "abc#123", // '#' not preceded by space stays part of the value
"remoteurl": "https://git.example.com",
"gitname": " Spaced # Name ",
"gitemail": "",
}
for k, want := range checks {
if m[k] != want {
t.Errorf("parseConfig[%q] = %q, want %q", k, m[k], want)
}
}
if !truthy(m["mirror"]) {
t.Errorf("mirror with a trailing comment must stay truthy, got %q", m["mirror"])
}
}
func TestLsEntry(t *testing.T) {
cases := []struct{ line, suffix, want string }{
// ownership is not assumed: any user/group must list
{"drwxr-xr-x 7 git git 4096 Sep 28 2016 myproj.git", ".git", "Sep 28 2016 myproj"},
{"drwxr-xr-x 7 deploy deploy 4096 Sep 28 2016 myproj.git", ".git", "Sep 28 2016 myproj"},
{"drwxr-xr-x 7 mike staff 4096 Sep 28 2016 myproj.git", ".git", "Sep 28 2016 myproj"},
{"drwxr-xr-x. 7 git users 4096 Sep 28 2016 myproj.git", ".git", "Sep 28 2016 myproj"},
// archives only match the archive suffix, and vice versa
{"-rw-r--r-- 1 git git 512 Sep 28 2016 myproj.git.tar.gz", ".git.tar.gz", "Sep 28 2016 myproj"},
{"-rw-r--r-- 1 git git 512 Sep 28 2016 myproj.git.tar.gz", ".git", ""},
{"drwxr-xr-x 7 git git 4096 Sep 28 2016 myproj.git", ".git.tar.gz", ""},
// non-entries
{"total 48", ".git", ""},
{"", ".git", ""},
{"drwxr-xr-x 7 git git 4096 Sep 28 2016 notes", ".git", ""},
}
for _, c := range cases {
if got := lsEntry(c.line, c.suffix); got != c.want {
t.Errorf("lsEntry(%q, %q) = %q, want %q", c.line, c.suffix, got, c.want)
}
}
}
func TestLsEntrySymlink(t *testing.T) {
// a symlinked bare repo lists its target too — only the link name counts
in := "lrwxrwxrwx 1 git git 14 Sep 28 2016 myproj.git -> /srv/other.git"
if got := lsEntry(in, ".git"); got != "Sep 28 2016 myproj" {
t.Errorf("lsEntry(symlink) = %q, want %q", got, "Sep 28 2016 myproj")
}
// and a symlink to something that is not a repo must not match
if got := lsEntry("lrwxrwxrwx 1 git git 5 Sep 28 2016 notes -> x.git", ".git"); got != "" {
t.Errorf("lsEntry(non-repo symlink) = %q, want empty", got)
}
}
func TestMaskSecret(t *testing.T) {
cases := []struct{ in, want string }{
{"", "(unset)"},
{"ab", "**"},
{"abcd", "****"},
{"abcdef", "ab**ef"},
}
for _, c := range cases {
if got := maskSecret(c.in); got != c.want {
t.Errorf("maskSecret(%q) = %q, want %q", c.in, got, c.want)
}
}
// a real-length token must not leak its middle
tok := strings.Repeat("s3cr3t", 6)
if got := maskSecret(tok); strings.Contains(got, "s3cr3ts3cr3t") || len(got) != len(tok) {
t.Errorf("maskSecret leaked or resized: %q", got)
}
}
func TestSSHKeyPath(t *testing.T) {
home := t.TempDir()
t.Setenv("HOME", home)
old := cfg
defer func() { cfg = old }()
cfg = Config{}
if got := sshKeyPath(); got != "" {
t.Errorf("no gitkey should yield no identity, got %q", got)
}
cfg = Config{GitKey: "mgit_rsa"} // bare name -> ~/.ssh
if want := filepath.Join(home, ".ssh", "mgit_rsa"); sshKeyPath() != want {
t.Errorf("sshKeyPath() = %q, want %q", sshKeyPath(), want)
}
cfg = Config{GitKey: "~/keys/id"} // ~/-relative
if want := filepath.Join(home, "keys", "id"); sshKeyPath() != want {
t.Errorf("sshKeyPath() = %q, want %q", sshKeyPath(), want)
}
cfg = Config{GitKey: "/etc/keys/id"} // absolute -> as given
if sshKeyPath() != "/etc/keys/id" {
t.Errorf("sshKeyPath() = %q, want /etc/keys/id", sshKeyPath())
}
// the identity must reach git through the environment, quoted
if env := gitEnv(); len(env) == 0 {
t.Fatal("gitEnv() returned no environment for a configured key")
} else if last := env[len(env)-1]; last != `GIT_SSH_COMMAND=ssh -i '/etc/keys/id'` {
t.Errorf("gitEnv() last entry = %q", last)
}
cfg = Config{}
if gitEnv() != nil {
t.Error("gitEnv() must inherit (nil) when no key is configured")
}
}
func TestProjectRCIgnored(t *testing.T) {
dir := t.TempDir()
// no repository yet: the .gitignore that `init` would use is what counts
if projectRCIgnored(dir) {
t.Error("no .gitignore should not count as ignored")
}
if err := os.WriteFile(filepath.Join(dir, ".gitignore"), []byte("*.o\n/"+projectRC+"\n"), 0644); err != nil {
t.Fatal(err)
}
if !projectRCIgnored(dir) {
t.Error("a .gitignore listing /" + projectRC + " should count as ignored")
}
}
func TestShq(t *testing.T) {
cases := []struct{ in, want string }{
{"myproj", "'myproj'"},
{"my project", "'my project'"},
{"it's", `'it'\''s'`},
{"`rm -rf ~`", "'`rm -rf ~`'"},
{"$(id)", "'$(id)'"},
{"", "''"},
}
for _, c := range cases {
if got := shq(c.in); got != c.want {
t.Errorf("shq(%q) = %q, want %q", c.in, got, c.want)
}
}
// the quoted form must survive a real shell as exactly one argument
out, err := exec.Command("/bin/sh", "-c", "printf '[%s]' "+shq("a b`id`'c")).Output()
if err != nil {
t.Fatal(err)
}
if string(out) != "[a b`id`'c]" {
t.Errorf("shq did not round-trip through /bin/sh: %q", out)
}
}
func TestValidProject(t *testing.T) {
ok := []string{"myproj", "my project", "a.b", "x-1_2"}
bad := []string{"", ".", "..", ".hidden", "foo/bar", "../etc", `foo\bar`}
for _, s := range ok {
if !validProject(s) {
t.Errorf("validProject(%q) = false, want true", s)
}
}
for _, s := range bad {
if validProject(s) {
t.Errorf("validProject(%q) = true, want false", s)
}
}
}
func TestProjectFromCwd(t *testing.T) {
base := t.TempDir()
// t.TempDir may hand back a symlinked path (/var -> /private/var on macOS);
// Getwd reports the resolved one, so compare like for like.
base, err := filepath.EvalSymlinks(base)
if err != nil {
t.Fatal(err)
}
deep := filepath.Join(base, "foo", "src", "lib")
if err := os.MkdirAll(deep, 0755); err != nil {
t.Fatal(err)
}
oldBase, oldWd := BASE, mustGetwd(t)
defer func() { BASE = oldBase; os.Chdir(oldWd) }()
BASE = base
cases := []struct{ dir, want string }{
{deep, "foo"}, // deep inside a project -> the project
{filepath.Join(base, "foo"), "foo"}, // project root
{base, ""}, // BASE itself -> no project
{filepath.Dir(base), ""}, // outside BASE -> no project
}
for _, c := range cases {
if err := os.Chdir(c.dir); err != nil {
t.Fatal(err)
}
if got := projectFromCwd(); got != c.want {
t.Errorf("projectFromCwd() in %s = %q, want %q", c.dir, got, c.want)
}
}
}
func mustGetwd(t *testing.T) string {
t.Helper()
wd, err := os.Getwd()
if err != nil {
t.Fatal(err)
}
return wd
}
func TestCdCommand(t *testing.T) {
base := t.TempDir()
if err := os.MkdirAll(filepath.Join(base, "notes"), 0755); err != nil {
t.Fatal(err)
}
oldBase, oldPrj := BASE, PRJ
defer func() { BASE = oldBase; PRJ = oldPrj }()
BASE, PRJ = base, ""
// bare `cd` must deselect the project, not panic on a missing argument
PRJ = "notes"
runCommand("cd")
if PRJ != "" {
t.Errorf("bare cd: PRJ = %q, want empty", PRJ)
}
runCommand("cd notes")
if PRJ != "notes" {
t.Errorf("cd notes: PRJ = %q, want notes", PRJ)
}
// a path with a separator would escape BASE and is rejected
runCommand("cd ../etc")
if PRJ != "notes" {
t.Errorf("cd ../etc changed PRJ to %q", PRJ)
}
runCommand("cd nosuchproject")
if PRJ != "" {
t.Errorf("cd to a missing project: PRJ = %q, want empty", PRJ)
}
}
func TestApplyConfig(t *testing.T) {
c := Config{GitName: "Original Name"}
applyConfig(&c, map[string]string{
@@ -133,6 +380,224 @@ func TestApplyConfig(t *testing.T) {
}
}
func TestRemoteTargetsFromConfig(t *testing.T) {
rc := `
remote.gitea.url = https://git.example.com
remote.gitea.key = tok-gitea
remote.hub.url = https://github.com
remote.hub.key = tok-hub
remote.hub.type = github
remote.hub.visibility = public
remote.broken.url = https://nowhere.example # no key -> unusable
`
var c Config
applyConfig(&c, parseConfig(rc))
usable, incomplete := c.mirrorTargets()
if len(usable) != 2 {
t.Fatalf("mirrorTargets usable = %d, want 2 (%+v)", len(usable), usable)
}
// key order is deterministic: gitea before hub
if usable[0].Name != "gitea" || usable[1].Name != "hub" {
t.Errorf("target order = %q,%q, want gitea,hub", usable[0].Name, usable[1].Name)
}
if usable[1].Type != "github" || usable[1].Vis != "public" {
t.Errorf("hub target = %+v, want type github / visibility public", usable[1])
}
if len(incomplete) != 1 || incomplete[0] != "broken" {
t.Errorf("incomplete = %v, want [broken]", incomplete)
}
}
func TestMirrorTargetsLegacyAndSelection(t *testing.T) {
// the flat remoteurl/remotekey pair stays supported, as target "public"
var c Config
applyConfig(&c, parseConfig("remoteurl = https://git.example.com\nremotekey = tok\n"))
usable, _ := c.mirrorTargets()
if len(usable) != 1 || usable[0].Name != legacyRemoteName {
t.Fatalf("legacy flat config = %+v, want one target named %q", usable, legacyRemoteName)
}
// `remotes` restricts and reorders the set
rc := `
remoteurl = https://git.example.com
remotekey = tok
remote.hub.url = https://github.com
remote.hub.key = tok2
remotes = hub, public
`
var c2 Config
applyConfig(&c2, parseConfig(rc))
usable, _ = c2.mirrorTargets()
if len(usable) != 2 || usable[0].Name != "hub" || usable[1].Name != "public" {
t.Fatalf("remotes selection = %+v, want hub,public", usable)
}
var c3 Config
applyConfig(&c3, parseConfig(rc+"remotes = hub\n"))
usable, _ = c3.mirrorTargets()
if len(usable) != 1 || usable[0].Name != "hub" {
t.Fatalf("narrowed selection = %+v, want only hub", usable)
}
}
func TestParsePushRemoteArgs(t *testing.T) {
cases := []struct {
in string
names []string
desc string
}{
{"", nil, ""},
{"a fix", nil, "a fix"},
{"@hub", []string{"hub"}, ""},
{"@hub a fix", []string{"hub"}, "a fix"},
{"@hub @gitea a fix", []string{"hub", "gitea"}, "a fix"},
{"a fix @hub", nil, "a fix @hub"}, // only leading @words select
{"@", nil, ""},
}
for _, c := range cases {
names, desc := parsePushRemoteArgs(c.in)
if strings.Join(names, ",") != strings.Join(c.names, ",") || desc != c.desc {
t.Errorf("parsePushRemoteArgs(%q) = %v,%q, want %v,%q", c.in, names, desc, c.names, c.desc)
}
}
}
func TestPickRemotes(t *testing.T) {
all := []RemoteTarget{{Name: "gitea"}, {Name: "hub"}}
if got := pickRemotes(all, nil); len(got) != 2 {
t.Errorf("no selection should keep all, got %+v", got)
}
got := pickRemotes(all, []string{"HUB"}) // names are case-insensitive
if len(got) != 1 || got[0].Name != "hub" {
t.Errorf("pickRemotes(HUB) = %+v, want hub", got)
}
if got := pickRemotes(all, []string{"nope"}); len(got) != 0 {
t.Errorf("unknown name should select nothing, got %+v", got)
}
}
func TestResolveProjectConfig(t *testing.T) {
dir := t.TempDir()
global := Config{
Base: "/base", GitHost: "global.example", GitPort: "22", GitUser: "git",
GitPath: "/home/git", GitName: "Global Name", Editor: "vi",
Remotes: []RemoteTarget{{Name: "gitea", URL: "https://gitea.example", Key: "tok"}},
}
// no project file -> unchanged
if got := resolveConfig(global, dir); got.GitHost != "global.example" {
t.Fatalf("without a project file GitHost = %q", got.GitHost)
}
rc := `
githost = project.example
editor = code
base = /somewhere/else
gitname = Project Name
remote.hub.url = https://github.com
remote.hub.key = tok2
remote.gitea.visibility = public
`
if err := os.WriteFile(filepath.Join(dir, projectRC), []byte(rc), 0600); err != nil {
t.Fatal(err)
}
got := resolveConfig(global, dir)
if got.GitHost != "project.example" || got.Editor != "code" {
t.Errorf("project overrides not applied: host=%q editor=%q", got.GitHost, got.Editor)
}
// base and the git identity stay global
if got.Base != "/base" {
t.Errorf("project must not override base, got %q", got.Base)
}
if got.GitName != "Global Name" {
t.Errorf("project must not override gitname, got %q", got.GitName)
}
// a project adds a target and refines a field of a global one
targets, _ := got.mirrorTargets()
if len(targets) != 2 {
t.Fatalf("targets = %+v, want gitea and hub", targets)
}
if targets[0].Name != "gitea" || targets[0].Vis != "public" || targets[0].Key != "tok" {
t.Errorf("gitea target = %+v, want visibility public with the global key", targets[0])
}
if targets[1].Name != "hub" || targets[1].URL != "https://github.com" {
t.Errorf("hub target = %+v", targets[1])
}
// the global configuration must be untouched by the overlay
if global.GitHost != "global.example" || len(global.Remotes) != 1 || global.Remotes[0].Vis != "" {
t.Errorf("resolveConfig mutated the global config: %+v", global)
}
// MGSH_* still wins over the project file
t.Setenv("MGSH_GITHOST", "env.example")
if got := resolveConfig(global, dir); got.GitHost != "env.example" {
t.Errorf("env override lost against project file, got %q", got.GitHost)
}
}
// TestEveryConfigKeyHasEnvOverride keeps the documented settings, the `config`
// command and applyEnv in step: every key mgsh reports must really be
// overridable through its MGSH_* variable.
func TestEveryConfigKeyHasEnvOverride(t *testing.T) {
for _, key := range configKeys() {
probe := "probe-" + key
t.Setenv(envName(key), probe)
var c Config
applyEnv(&c)
if !configHasValue(c, probe) {
t.Errorf("%s does not override the %q setting", envName(key), key)
}
t.Setenv(envName(key), "")
}
}
// configHasValue reports whether any string field of c equals want.
func configHasValue(c Config, want string) bool {
v := reflect.ValueOf(c)
for i := 0; i < v.NumField(); i++ {
if f := v.Field(i); f.Kind() == reflect.String && f.String() == want {
return true
}
}
return false
}
// TestCheckoutForwardsOptions covers the option-stripping trap: mgsh pulls
// `-x` flags out of the word list, so a command that forwards to git has to use
// the raw fields or `checkout -b topic` silently loses its flag.
func TestCheckoutForwardsOptions(t *testing.T) {
base := t.TempDir()
dir := filepath.Join(base, "proj")
if err := os.MkdirAll(dir, 0755); err != nil {
t.Fatal(err)
}
for _, args := range [][]string{
{"init", "-q"},
{"-c", "user.name=t", "-c", "user.email=t@e", "commit", "-q", "--allow-empty", "-m", "x"},
} {
if out, err := exec.Command("git", append([]string{"-C", dir}, args...)...).CombinedOutput(); err != nil {
t.Fatalf("git %v: %v\n%s", args, err, out)
}
}
oldBase, oldPrj, oldDir := BASE, PRJ, DIR
defer func() { BASE, PRJ, DIR = oldBase, oldPrj, oldDir }()
BASE, PRJ, DIR = base, "proj", dir
runCommand("checkout -b topic")
out, err := exec.Command("git", "-C", dir, "rev-parse", "--abbrev-ref", "HEAD").Output()
if err != nil {
t.Fatal(err)
}
if got := strings.TrimSpace(string(out)); got != "topic" {
t.Errorf("after `checkout -b topic` HEAD is %q, want topic", got)
}
}
func TestMissingRequired(t *testing.T) {
full := Config{Base: "/b", GitHost: "h", GitPort: "22", GitUser: "u", GitPath: "/p"}
if m := full.missingRequired(); len(m) != 0 {