Two mistakes in the size support, reported from a real server. The remote command used "2>/dev/null" to silence du. That is sh syntax, and the git user's login shell need not be sh: in csh it parses as an argument "2" followed by a redirection of stdout, so du was handed a file named "2", complained, and exited non-zero. The redirection is gone -- without it there is no bogus argument to trip over, and the command now uses nothing that differs between sh and csh. Worse, the exit status of the chain is the *last* command's, so that failing du made sshOut return an error and `list` threw away a listing that had arrived perfectly intact. It now reports a failure only when nothing usable came back at all; a listing that parsed is shown whatever the exit status, simply without the size column. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
394 lines
13 KiB
Go
394 lines
13 KiB
Go
package main
|
|
|
|
// remotecmd_test.go — what mgsh actually sends to the git server.
|
|
//
|
|
// These are the only commands that can destroy data (`rm -rf` on the server),
|
|
// and they are the hardest to check by reading: the remote command is a single
|
|
// string handed to a login shell, so a missing quote silently changes which
|
|
// paths it touches. sshExec and yesno are swapped for recorders, and the tests
|
|
// assert on the exact command strings that would have been sent.
|
|
|
|
import (
|
|
"errors"
|
|
"os"
|
|
"os/exec"
|
|
"path/filepath"
|
|
"strings"
|
|
"testing"
|
|
)
|
|
|
|
// fakeServer replaces the ssh transport for the duration of the test. reply
|
|
// returns the stdout for a remote command (and optionally an error); a nil
|
|
// reply answers every command with empty output and success.
|
|
func fakeServer(t *testing.T, reply func(cmd string) (string, error)) *[]string {
|
|
t.Helper()
|
|
var sent []string
|
|
old := sshExec
|
|
sshExec = func(remote string, capture bool) ([]byte, error) {
|
|
sent = append(sent, remote)
|
|
if reply == nil {
|
|
return nil, nil
|
|
}
|
|
out, err := reply(remote)
|
|
return []byte(out), err
|
|
}
|
|
t.Cleanup(func() { sshExec = old })
|
|
return &sent
|
|
}
|
|
|
|
// fakeAnswers replaces the y/n prompt, recording the questions asked and
|
|
// answering each one with answer.
|
|
func fakeAnswers(t *testing.T, answer bool) *[]string {
|
|
t.Helper()
|
|
var asked []string
|
|
old := yesno
|
|
yesno = func(prompt string, def bool) bool {
|
|
asked = append(asked, prompt)
|
|
return answer
|
|
}
|
|
t.Cleanup(func() { yesno = old })
|
|
return &asked
|
|
}
|
|
|
|
// useProject points the globals at a fresh project directory under a temp base.
|
|
func useProject(t *testing.T, name string) string {
|
|
t.Helper()
|
|
base := t.TempDir()
|
|
dir := filepath.Join(base, name)
|
|
if err := os.MkdirAll(dir, 0755); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
oldBase, oldPrj, oldDir, oldCfg := BASE, PRJ, DIR, cfg
|
|
t.Cleanup(func() { BASE, PRJ, DIR, cfg = oldBase, oldPrj, oldDir, oldCfg })
|
|
|
|
BASE, PRJ, DIR = base, name, dir
|
|
cfg = Config{
|
|
Base: base, GitHost: "git.example", GitPort: "22",
|
|
GitUser: "git", GitPath: "/home/git",
|
|
}
|
|
return dir
|
|
}
|
|
|
|
// findCmd returns the first recorded command containing sub, or "".
|
|
func findCmd(sent []string, sub string) string {
|
|
for _, c := range sent {
|
|
if strings.Contains(c, sub) {
|
|
return c
|
|
}
|
|
}
|
|
return ""
|
|
}
|
|
|
|
// TestInitKeepsServerRepoWhenDeclined is the regression test for the worst bug
|
|
// in this command set: `init` used the *local* remote.origin.url to decide
|
|
// whether a server repository existed, so an unlinked project directory made it
|
|
// skip the prompt and wipe the remote history.
|
|
func TestInitKeepsServerRepoWhenDeclined(t *testing.T) {
|
|
useProject(t, "notes") // deliberately no .git, so there is no local origin
|
|
sent := fakeServer(t, func(cmd string) (string, error) {
|
|
if strings.HasPrefix(cmd, "/bin/ls") {
|
|
return "notes.git\nother.git\n", nil // the repo DOES exist there
|
|
}
|
|
return "", nil
|
|
})
|
|
asked := fakeAnswers(t, false) // ... and the user declines
|
|
|
|
runCommand("init")
|
|
|
|
if len(*asked) == 0 {
|
|
t.Fatal("init did not ask before overwriting an existing server repository")
|
|
}
|
|
if c := findCmd(*sent, "rm -rf"); c != "" {
|
|
t.Fatalf("init destroyed the server repository after the user declined: %q", c)
|
|
}
|
|
if c := findCmd(*sent, "git --bare init"); c != "" {
|
|
t.Fatalf("init re-created the repository after the user declined: %q", c)
|
|
}
|
|
}
|
|
|
|
// TestInitAbortsWhenServerUnreachable: an unreachable server must not be read
|
|
// as "no repository there" — that would turn a network glitch into data loss.
|
|
func TestInitAbortsWhenServerUnreachable(t *testing.T) {
|
|
useProject(t, "notes")
|
|
sent := fakeServer(t, func(cmd string) (string, error) {
|
|
return "", errors.New("ssh: connect to host git.example port 22: Network is unreachable")
|
|
})
|
|
asked := fakeAnswers(t, true) // even a "yes" must not get that far
|
|
|
|
runCommand("init")
|
|
|
|
if c := findCmd(*sent, "rm -rf"); c != "" {
|
|
t.Fatalf("init sent %q although the server could not be listed", c)
|
|
}
|
|
if len(*asked) != 0 {
|
|
t.Fatalf("init asked %q despite not knowing the server state", (*asked)[0])
|
|
}
|
|
}
|
|
|
|
// TestInitOnFreshProjectCreatesRepo: with nothing on the server, `init` goes
|
|
// ahead without asking and builds the bare repository at the configured path.
|
|
func TestInitOnFreshProjectCreatesRepo(t *testing.T) {
|
|
useProject(t, "notes")
|
|
sent := fakeServer(t, func(cmd string) (string, error) {
|
|
if strings.HasPrefix(cmd, "/bin/ls") {
|
|
return "other.git\n", nil // notes.git is not there
|
|
}
|
|
return "", nil
|
|
})
|
|
asked := fakeAnswers(t, false)
|
|
|
|
runCommand("init")
|
|
|
|
for _, prompt := range *asked {
|
|
if strings.Contains(prompt, "overwrite existing repository") {
|
|
t.Errorf("init asked about overwriting although nothing was there: %q", prompt)
|
|
}
|
|
}
|
|
if findCmd(*sent, "rm -rf "+shq("/home/git/notes.git")) == "" {
|
|
t.Errorf("init did not clear the target path, sent: %q", *sent)
|
|
}
|
|
if c := findCmd(*sent, "git --bare init"); !strings.Contains(c, shq("/home/git/notes.git")) {
|
|
t.Errorf("init did not create the bare repository at the configured path: %q", c)
|
|
}
|
|
}
|
|
|
|
// TestRemoteCommandsQuoteProjectNames covers the injection the review found: a
|
|
// project name reaches the server inside a single shell string, so a space
|
|
// turns one path into two arguments and a backtick runs on the server.
|
|
func TestRemoteCommandsQuoteProjectNames(t *testing.T) {
|
|
const evil = "my 'weird' project"
|
|
useProject(t, evil)
|
|
sent := fakeServer(t, func(cmd string) (string, error) {
|
|
if strings.HasPrefix(cmd, "/bin/ls") {
|
|
return evil + ".git\n", nil
|
|
}
|
|
return "", nil
|
|
})
|
|
fakeAnswers(t, true) // confirm the overwrite
|
|
|
|
runCommand("init")
|
|
runCommand("push")
|
|
runCommand("archive")
|
|
|
|
if len(*sent) == 0 {
|
|
t.Fatal("no remote commands recorded")
|
|
}
|
|
for _, c := range *sent {
|
|
// the bare name must never appear outside single quotes: every
|
|
// occurrence has to be preceded by the quote shq() adds
|
|
for _, idx := range indexesOf(c, evil) {
|
|
if idx == 0 || c[idx-1] != '\'' {
|
|
t.Errorf("unquoted project name in remote command: %q", c)
|
|
break
|
|
}
|
|
}
|
|
}
|
|
// spot-check the destructive one in full
|
|
want := "rm -rf " + shq("/home/git/"+evil+".git")
|
|
if findCmd(*sent, want) == "" {
|
|
t.Errorf("expected %q among the sent commands, got %q", want, *sent)
|
|
}
|
|
}
|
|
|
|
// TestArchiveRequiresServerRepo: `archive` copies the server-side repository,
|
|
// so a project that was never pushed must produce a clear message rather than a
|
|
// raw `cp -r` failure.
|
|
func TestArchiveRequiresServerRepo(t *testing.T) {
|
|
useProject(t, "notes")
|
|
sent := fakeServer(t, func(cmd string) (string, error) {
|
|
return "", nil // empty server
|
|
})
|
|
|
|
runCommand("archive")
|
|
|
|
if c := findCmd(*sent, "cp -r"); c != "" {
|
|
t.Errorf("archive copied although the server has no such repository: %q", c)
|
|
}
|
|
}
|
|
|
|
// TestArchiveNamesAndQuotesSnapshot checks the snapshot pipeline end to end:
|
|
// the three remote commands, their order, and their quoting.
|
|
func TestArchiveNamesAndQuotesSnapshot(t *testing.T) {
|
|
useProject(t, "notes")
|
|
sent := fakeServer(t, func(cmd string) (string, error) {
|
|
if strings.HasPrefix(cmd, "/bin/ls") {
|
|
return "notes.git\n", nil
|
|
}
|
|
return "", nil
|
|
})
|
|
|
|
runCommand("archive before rewrite")
|
|
|
|
if len(*sent) != 4 { // ls, cp, tar, rm
|
|
t.Fatalf("expected 4 remote commands, got %d: %q", len(*sent), *sent)
|
|
}
|
|
cp, tar, rm := (*sent)[1], (*sent)[2], (*sent)[3]
|
|
|
|
// name is <project>_<stamp>_<sanitised comment>
|
|
stamp := archiveStamp()
|
|
name := "notes_" + stamp + "_before_rewrite"
|
|
if !strings.HasPrefix(cp, "cp -r "+shq("notes.git")+" ") {
|
|
t.Errorf("cp command = %q", cp)
|
|
}
|
|
if !strings.Contains(cp, shq("archive/"+name+".git")) {
|
|
t.Errorf("cp target = %q, want it to contain %q", cp, name)
|
|
}
|
|
// `cd archive && tar`, not `cd archive;tar`: a failed cd must not let tar
|
|
// run in the login directory
|
|
if !strings.HasPrefix(tar, "cd archive && tar ") || !strings.Contains(tar, shq(name+".git.tar.gz")) {
|
|
t.Errorf("tar command = %q", tar)
|
|
}
|
|
if rm != "rm -rf "+shq("archive/"+name+".git") {
|
|
t.Errorf("cleanup command = %q", rm)
|
|
}
|
|
}
|
|
|
|
// TestCloneRefusesUnknownRepository: `clone` must not start a git clone for a
|
|
// repository the server does not list.
|
|
func TestCloneRefusesUnknownRepository(t *testing.T) {
|
|
useProject(t, "notes")
|
|
PRJ = ""
|
|
sent := fakeServer(t, func(cmd string) (string, error) {
|
|
return "other.git\n", nil
|
|
})
|
|
|
|
runCommand("clone notes")
|
|
|
|
if len(*sent) != 1 || !strings.HasPrefix((*sent)[0], "/bin/ls") {
|
|
t.Errorf("clone did more than look: %q", *sent)
|
|
}
|
|
if PRJ != "" {
|
|
t.Errorf("clone selected %q although nothing was cloned", PRJ)
|
|
}
|
|
}
|
|
|
|
// TestPushCommitsAndTouchesServerRepo drives the whole `push` sequence against
|
|
// a real local bare repository: stage, commit, push, then bump the mtime of the
|
|
// server-side repository so `list` can sort by last activity. The project name
|
|
// contains a space, so the touch also has to be quoted.
|
|
func TestPushCommitsAndTouchesServerRepo(t *testing.T) {
|
|
dir := useProject(t, "my project")
|
|
bare := filepath.Join(t.TempDir(), "origin.git")
|
|
mustGit(t, "", "init", "--bare", "-q", bare)
|
|
mustGit(t, dir, "init", "-q")
|
|
mustGit(t, dir, "config", "user.name", "t")
|
|
mustGit(t, dir, "config", "user.email", "t@e")
|
|
if err := os.WriteFile(filepath.Join(dir, "a.txt"), []byte("hi\n"), 0644); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
mustGit(t, dir, "add", ".")
|
|
mustGit(t, dir, "commit", "-q", "-m", "first")
|
|
mustGit(t, dir, "remote", "add", "origin", bare)
|
|
mustGit(t, dir, "push", "-q", "-u", "origin", "HEAD")
|
|
|
|
if err := os.WriteFile(filepath.Join(dir, "b.txt"), []byte("more\n"), 0644); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
sent := fakeServer(t, nil)
|
|
|
|
runCommand("push a second file")
|
|
|
|
want := "touch " + shq("/home/git/my project.git")
|
|
if findCmd(*sent, want) == "" {
|
|
t.Fatalf("expected %q among the sent commands, got %q", want, *sent)
|
|
}
|
|
// the new file really made it into the bare repository, with the
|
|
// "[user@host] comment" message mgsh builds
|
|
out, err := gitCapture(bare, "log", "-1", "--format=%s")
|
|
if err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if subj := strings.TrimSpace(out); !strings.HasSuffix(subj, "] a second file") {
|
|
t.Errorf("commit subject = %q, want it to end in %q", subj, "] a second file")
|
|
}
|
|
if files, err := gitCapture(bare, "ls-tree", "--name-only", "HEAD"); err != nil {
|
|
t.Fatal(err)
|
|
} else if !strings.Contains(files, "b.txt") {
|
|
t.Errorf("pushed tree = %q, want it to contain b.txt", files)
|
|
}
|
|
}
|
|
|
|
// indexesOf returns every start index of sub in s.
|
|
func indexesOf(s, sub string) []int {
|
|
var out []int
|
|
for i := 0; ; {
|
|
j := strings.Index(s[i:], sub)
|
|
if j < 0 {
|
|
return out
|
|
}
|
|
out = append(out, i+j)
|
|
i += j + 1
|
|
}
|
|
}
|
|
|
|
// mustGit runs a git command in dir, failing the test on error.
|
|
func mustGit(t *testing.T, dir string, args ...string) {
|
|
t.Helper()
|
|
out, err := exec.Command("git", append([]string{"-C", dir}, args...)...).CombinedOutput()
|
|
if err != nil {
|
|
t.Fatalf("git %v: %v\n%s", args, err, out)
|
|
}
|
|
}
|
|
|
|
// TestListSurvivesFailingDu: `list` chains the listing and `du` into one remote
|
|
// command, and the exit status is the *last* command's. A server whose du fails
|
|
// — a shell that mis-parses the arguments, a du that is not there, a permission
|
|
// problem — must still get its repositories listed.
|
|
func TestListSurvivesFailingDu(t *testing.T) {
|
|
useProject(t, "x")
|
|
fakeServer(t, func(cmd string) (string, error) {
|
|
return "total 4\n" +
|
|
"drwxr-xr-x 7 git git 4096 Jan 3 14:32 notes.git\n" +
|
|
"drwxr-xr-x 7 git git 4096 Sep 28 2016 website.git\n" +
|
|
listMarker + "\n",
|
|
errors.New("exit status 1") // du blew up, ls did not
|
|
})
|
|
|
|
out := captureStdout(t, func() { runCommand("list") })
|
|
|
|
if strings.Contains(out, "could not list") {
|
|
t.Errorf("a failing du discarded a good listing:\n%s", out)
|
|
}
|
|
for _, want := range []string{"notes", "website", "2 repositories"} {
|
|
if !strings.Contains(out, want) {
|
|
t.Errorf("listing missing %q:\n%s", want, out)
|
|
}
|
|
}
|
|
// without sizes there must be no size column, not a column of zeroes
|
|
if strings.Contains(out, "0B") {
|
|
t.Errorf("zero sizes shown when du produced none:\n%s", out)
|
|
}
|
|
}
|
|
|
|
// TestListReportsATrulyFailedListing: when nothing usable came back, the error
|
|
// still has to surface.
|
|
func TestListReportsATrulyFailedListing(t *testing.T) {
|
|
useProject(t, "x")
|
|
fakeServer(t, func(cmd string) (string, error) {
|
|
return "", errors.New("ssh: connect failed")
|
|
})
|
|
out := captureStdout(t, func() { runCommand("list") })
|
|
if !strings.Contains(out, "could not list") {
|
|
t.Errorf("a failed listing was not reported:\n%s", out)
|
|
}
|
|
}
|
|
|
|
// TestListSendsNoShellSpecificSyntax guards the bug this replaced: the remote
|
|
// command is run by the git user's login shell, which may be csh, where
|
|
// "2>/dev/null" is an argument followed by a redirection rather than a
|
|
// redirection of stderr.
|
|
func TestListSendsNoShellSpecificSyntax(t *testing.T) {
|
|
useProject(t, "x")
|
|
sent := fakeServer(t, func(cmd string) (string, error) { return "", nil })
|
|
captureStdout(t, func() { runCommand("list") })
|
|
|
|
if len(*sent) == 0 {
|
|
t.Fatal("list sent nothing")
|
|
}
|
|
for _, c := range *sent {
|
|
if strings.Contains(c, "2>") || strings.Contains(c, "&>") {
|
|
t.Errorf("remote command uses sh-only redirection: %q", c)
|
|
}
|
|
}
|
|
}
|