diff --git a/internal/install/doctor.go b/internal/install/doctor.go index ee982e7..24d5d2b 100644 --- a/internal/install/doctor.go +++ b/internal/install/doctor.go @@ -298,7 +298,8 @@ type doctorRun struct { // use, or "" when Deep is off or the file can't be made. It lives under the // home directory because the scanner only accepts paths under home or /tmp, // and the OS temp dir (e.g. /var/folders on macOS) is neither. It is a git -// repository so the scan exercises the server's git calls. +// repository whose committed copy is empty, so the server's `git diff HEAD` +// succeeds and returns changed lines, as it does for a file edited in a real repo. func (d *doctorRun) deepProbeFile() string { if !d.opts.Deep { return "" @@ -314,12 +315,8 @@ func (d *doctorRun) deepProbeFile() string { d.report.add("scanner", "deep probe", StatusWarn, "could not create a scratch file for the scan call: "+err.Error()) return "" } - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) - defer cancel() - // Best effort: without git the scan simply takes the no-repository path. - // armis:ignore cwe:78 reason:deepDir is a directory this process just created - _ = exec.CommandContext(ctx, "git", "init", "-q", d.deepDir).Run() //nolint:gosec // deepDir is a directory this process just created - file := filepath.Join(d.deepDir, "armis_doctor_probe.py") + file := filepath.Join(d.deepDir, deepProbeName) + initDeepProbeRepo(d.deepDir, file) if err := os.WriteFile(file, []byte("def add(a, b):\n return a + b\n"), 0o600); err != nil { d.report.add("scanner", "deep probe", StatusWarn, "could not create a scratch file for the scan call: "+err.Error()) return "" @@ -328,6 +325,39 @@ func (d *doctorRun) deepProbeFile() string { return file } +const deepProbeName = "armis_doctor_probe.py" + +// initDeepProbeRepo makes dir a git repository with an empty file committed +// at file. Best effort: if git is missing or a step fails, the scan takes the +// no-repository or no-HEAD path instead. +func initDeepProbeRepo(dir, file string) { + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + git := func(args ...string) error { + // armis:ignore cwe:78 reason:fixed git subcommands run in a directory this process just created + return exec.CommandContext(ctx, "git", append([]string{"-C", dir}, args...)...).Run() //nolint:gosec // fixed subcommands in our own scratch dir + } + if err := git("init", "-q"); err != nil { + return + } + if err := os.WriteFile(file, nil, 0o600); err != nil { + return + } + if err := git("add", "--", deepProbeName); err != nil { + return + } + // The user's git config may lack an identity, require signing, or set + // global hooks (possibly an Armis scan hook); none of that should apply + // to a scratch commit. + _ = git( + "-c", "user.name=armis-cli mcp doctor", + "-c", "user.email=mcp-doctor@armis.invalid", + "-c", "commit.gpgsign=false", + "-c", "core.hooksPath="+filepath.Join(dir, ".git", "no-hooks"), + "commit", "-q", "--no-verify", "-m", "mcp doctor probe", + ) +} + // close removes the deep probe's scratch directory. func (d *doctorRun) close() { if d.deepDir != "" { diff --git a/internal/install/doctor_stall_test.go b/internal/install/doctor_stall_test.go index 71ce05a..593db7e 100644 --- a/internal/install/doctor_stall_test.go +++ b/internal/install/doctor_stall_test.go @@ -5,6 +5,7 @@ import ( "context" "errors" "os" + "os/exec" "path/filepath" "runtime" "strings" @@ -150,6 +151,49 @@ func TestHandshakeStallRecordsChildProcess(t *testing.T) { } } +func TestDeepProbeFileDiffsAgainstHEAD(t *testing.T) { + if _, err := exec.LookPath("git"); err != nil { + t.Skip("git not on PATH") + } + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("USERPROFILE", home) + // A global config that would break a naive commit: signing required and + // a pre-commit hook that always fails. + hooks := filepath.Join(home, "hooks") + if err := os.MkdirAll(hooks, 0o750); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(hooks, "pre-commit"), []byte("#!/bin/sh\nexit 1\n"), 0o755); err != nil { //nolint:gosec // test hook must be executable + t.Fatal(err) + } + global := filepath.Join(home, "gitconfig") + cfg := "[commit]\n\tgpgsign = true\n[core]\n\thooksPath = " + filepath.ToSlash(hooks) + "\n" + if err := os.WriteFile(global, []byte(cfg), 0o600); err != nil { + t.Fatal(err) + } + t.Setenv("GIT_CONFIG_GLOBAL", global) + t.Setenv("GIT_CONFIG_NOSYSTEM", "1") + + d := newDoctorRun(DoctorOptions{Deep: true}) + file := d.deepProbeFile() + if file == "" { + t.Fatalf("no probe file; report: %+v", d.report.Checks) + } + out, err := exec.Command("git", "-C", d.deepDir, "diff", "HEAD", "-U0", "--", deepProbeName).CombinedOutput() //nolint:gosec // test-owned dir + if err != nil { + t.Fatalf("git diff HEAD failed: %v\n%s", err, out) + } + if !strings.Contains(string(out), "+def add(a, b):") { + t.Errorf("diff = %q, want the probe content as added lines", out) + } + + d.close() + if _, err := os.Stat(d.deepDir); !os.IsNotExist(err) { + t.Errorf("probe dir %s still exists after close (err=%v)", d.deepDir, err) + } +} + func TestProbeDeepScan(t *testing.T) { d := newDoctorRun(DoctorOptions{Handshake: true, Deep: true, Timeout: 10 * time.Second}) d.deepFile = "/tmp/probe.py"