feat: Add pmg setup doctor command (#290)

* feat: add doctor check runner core types and logic

* feat: add doctor checks for config, binary, directory, and aliases

* feat: add doctor checks for sandbox and security features

* feat: add protection verification check using test malicious packages

* feat: add summarized package manager availability check

* feat: add pmg setup doctor command with compact output

* feat: add PATH shim verification to doctor command

* fix: improve doctor command UX and alias detection

- Capitalize all check messages for consistent output
- Dim passing checks, color warn/fail for visual clarity
- Silence empty Cobra error output on doctor failure
- Remove redundant pmg binary check (self-evident)
- Fix alias IsInstalled to skip commented-out source lines
- Improve protection failure message

* refactor: remove package manager availability check from doctor

* fix: handle os.RemoveAll error in doctor protection check

* refactor: use table layout for setup doctor, extract shared table renderer

Move renderTable, truncate, and visibleWidth helpers from cmd/sandbox
to internal/ui so both sandbox and setup doctor share them. Rewrite
setup doctor output to use the same table structure as sandbox doctor.
Fix VisibleWidth to count runes instead of bytes for correct alignment
with multi-byte UTF-8 characters.

* docs: add pmg setup doctor to README, remove manual verification step

* refactor: use constants for check names, rename and inline doctor helpers

Address PR review comments: extract check name constants, rename
CheckConfigFile to CheckFileExists and CheckDirectoryWritable to
CheckDirectoryExists for reusability, inline trivial wrappers
(CheckSandbox, CheckSecurityFeature, CheckProxyMode), and add
fix hints for all checks with correct config keys.

* refactor: inline simple doctor checks into command layer

* fix: skip protection check when aliases and shims are inactive

Protection checks now fail immediately when shell aliases and shims
are both inactive, instead of falsely passing by running through the
pmg binary directly. Also clean up summary messages to remove
redundant fix hints and truncated paths.
This commit is contained in:
Sahil Bansal
2026-05-26 12:21:39 +05:30
committed by GitHub
parent 8cb7b7d1c0
commit 083f82dd79
10 changed files with 949 additions and 98 deletions
+8 -2
View File
@@ -163,8 +163,14 @@ func (a *AliasManager) IsInstalled() (bool, error) {
continue
}
if strings.Contains(string(data), a.config.RcFileName) {
return true, nil
for _, line := range strings.Split(string(data), "\n") {
trimmed := strings.TrimSpace(line)
if strings.HasPrefix(trimmed, "#") {
continue
}
if strings.Contains(trimmed, a.config.RcFileName) {
return true, nil
}
}
}
}
+136
View File
@@ -0,0 +1,136 @@
package doctor
import (
"fmt"
"os"
"os/exec"
"path/filepath"
"strings"
"github.com/safedep/dry/log"
)
type ProtectionTestCase struct {
PackageManager string
Package string
InstallArgs []string
NeedsVenv bool
}
func ProtectionTestCases() []ProtectionTestCase {
return []ProtectionTestCase{
{
PackageManager: "npm",
Package: "safedep-test-pkg@0.1.3",
InstallArgs: []string{"npm", "install", "--no-cache", "--prefer-online", "safedep-test-pkg@0.1.3"},
},
{
PackageManager: "pip",
Package: "safedep-test-pkg==0.1.4",
InstallArgs: []string{"pip", "install", "--no-cache-dir", "safedep-test-pkg==0.1.4"},
NeedsVenv: true,
},
}
}
func RunProtectionCheck(tc ProtectionTestCase, pmgBinary string) CheckResult {
if _, err := exec.LookPath(tc.PackageManager); err != nil {
return CheckResult{
Status: StatusWarn,
Message: fmt.Sprintf("%s not available — skipping protection test for %s", tc.PackageManager, tc.Package),
}
}
tmpDir, err := os.MkdirTemp("", "pmg-doctor-*")
if err != nil {
return CheckResult{
Status: StatusWarn,
Message: fmt.Sprintf("Could not create temp dir for %s test: %v", tc.PackageManager, err),
}
}
defer func() {
if err := os.RemoveAll(tmpDir); err != nil {
log.Warnf("failed to clean up temp dir %s: %v", tmpDir, err)
}
}()
env := os.Environ()
if tc.NeedsVenv {
venvDir, venvErr := setupVenv(tmpDir)
if venvErr != nil {
return CheckResult{
Status: StatusWarn,
Message: fmt.Sprintf("Could not create venv for %s test: %v", tc.PackageManager, venvErr),
}
}
venvBin := filepath.Join(venvDir, "bin")
env = prependPath(env, venvBin)
}
cmd := exec.Command(pmgBinary, tc.InstallArgs...)
cmd.Dir = tmpDir
cmd.Env = env
_, runErr := cmd.CombinedOutput()
return evaluateProtectionResult(tc.PackageManager, tc.Package, runErr)
}
func setupVenv(baseDir string) (string, error) {
venvDir := filepath.Join(baseDir, "venv")
cmd := exec.Command("python3", "-m", "venv", venvDir)
if output, err := cmd.CombinedOutput(); err != nil {
return "", fmt.Errorf("venv creation failed: %w\n%s", err, string(output))
}
return venvDir, nil
}
func prependPath(env []string, dir string) []string {
result := make([]string, 0, len(env))
for _, e := range env {
if strings.HasPrefix(e, "PATH=") {
e = fmt.Sprintf("PATH=%s%c%s", dir, filepath.ListSeparator, e[5:])
}
result = append(result, e)
}
return result
}
func evaluateProtectionResult(pm string, pkg string, err error) CheckResult {
if err != nil {
if isExecutableNotFound(err) {
return CheckResult{
Status: StatusWarn,
Message: fmt.Sprintf("%s not available — skipping protection test for %s", pm, pkg),
}
}
return CheckResult{
Status: StatusPass,
Message: fmt.Sprintf("Malicious package blocked (%s/%s)", pm, pkg),
}
}
return CheckResult{
Status: StatusFail,
Message: fmt.Sprintf("Failed to block %s/%s — package was installed instead of blocked", pm, pkg),
}
}
func isExecutableNotFound(err error) bool {
if execErr, ok := err.(*exec.Error); ok {
return execErr.Err == exec.ErrNotFound
}
return false
}
func CheckShimScripts(shimDir string, managers []string) (found []string, missing []string) {
for _, pm := range managers {
shimPath := filepath.Join(shimDir, pm)
info, err := os.Stat(shimPath)
if err != nil || info.Mode()&0o111 == 0 {
missing = append(missing, pm)
continue
}
found = append(found, pm)
}
return found, missing
}
+112
View File
@@ -0,0 +1,112 @@
package doctor
import (
"fmt"
"os"
"os/exec"
"path/filepath"
"strings"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestEvaluateProtectionResult(t *testing.T) {
tests := []struct {
name string
pm string
pkg string
err error
wantStatus CheckStatus
}{
{
name: "blocked",
pm: "npm",
pkg: "safedep-test-pkg@0.1.3",
err: fmt.Errorf("exit status 1"),
wantStatus: StatusPass,
},
{
name: "not blocked",
pm: "npm",
pkg: "safedep-test-pkg@0.1.3",
err: nil,
wantStatus: StatusFail,
},
{
name: "pm not found",
pm: "npm",
pkg: "safedep-test-pkg@0.1.3",
err: &exec.Error{Name: "npm", Err: exec.ErrNotFound},
wantStatus: StatusWarn,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
result := evaluateProtectionResult(tt.pm, tt.pkg, tt.err)
assert.Equal(t, tt.wantStatus, result.Status)
})
}
}
func TestProtectionTestCases(t *testing.T) {
cases := ProtectionTestCases()
require.GreaterOrEqual(t, len(cases), 2)
hasNpm := false
hasPip := false
for _, tc := range cases {
if tc.PackageManager == "npm" {
hasNpm = true
assert.False(t, tc.NeedsVenv)
assert.Contains(t, tc.InstallArgs, "--no-cache")
assert.Contains(t, tc.InstallArgs, "--prefer-online")
}
if tc.PackageManager == "pip" {
hasPip = true
assert.True(t, tc.NeedsVenv)
assert.Contains(t, tc.InstallArgs, "--no-cache-dir")
}
}
assert.True(t, hasNpm)
assert.True(t, hasPip)
}
func TestPrependPath(t *testing.T) {
env := []string{"HOME=/home/user", "PATH=/usr/bin:/bin", "TERM=xterm"}
result := prependPath(env, "/tmp/venv/bin")
require.Len(t, result, 3)
assert.Equal(t, "HOME=/home/user", result[0])
assert.True(t, strings.HasPrefix(result[1], "PATH=/tmp/venv/bin"))
assert.Contains(t, result[1], "/usr/bin:/bin")
assert.Equal(t, "TERM=xterm", result[2])
}
func TestSetupVenv(t *testing.T) {
if _, err := exec.LookPath("python3"); err != nil {
t.Skip("python3 not available")
}
tmpDir := t.TempDir()
venvDir, err := setupVenv(tmpDir)
require.NoError(t, err)
pipPath := filepath.Join(venvDir, "bin", "pip")
_, err = os.Stat(pipPath)
assert.NoError(t, err)
}
func TestCheckShimScripts(t *testing.T) {
tmpDir := t.TempDir()
shimDir := filepath.Join(tmpDir, ".pmg", "bin")
require.NoError(t, os.MkdirAll(shimDir, 0o755))
shimPath := filepath.Join(shimDir, "npm")
require.NoError(t, os.WriteFile(shimPath, []byte("#!/bin/sh\nexec pmg npm \"$@\""), 0o755))
found, missing := CheckShimScripts(shimDir, []string{"npm", "pip"})
assert.Equal(t, []string{"npm"}, found)
assert.Equal(t, []string{"pip"}, missing)
}
+53
View File
@@ -0,0 +1,53 @@
package doctor
type CheckStatus int
const (
StatusPass CheckStatus = iota
StatusWarn
StatusFail
)
type CheckResult struct {
Name string
Category string
Status CheckStatus
Message string
}
type Check struct {
Name string
Category string
Run func() CheckResult
}
func RunChecks(checks []Check) []CheckResult {
results := make([]CheckResult, 0, len(checks))
for _, c := range checks {
result := c.Run()
result.Name = c.Name
result.Category = c.Category
results = append(results, result)
}
return results
}
func HasFailures(results []CheckResult) bool {
for _, r := range results {
if r.Status == StatusFail {
return true
}
}
return false
}
func CategorySummary(results []CheckResult) map[string]CheckStatus {
summary := make(map[string]CheckStatus)
for _, r := range results {
current, exists := summary[r.Category]
if !exists || r.Status > current {
summary[r.Category] = r.Status
}
}
return summary
}
+120
View File
@@ -0,0 +1,120 @@
package doctor
import (
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestRunChecks_AllPass(t *testing.T) {
checks := []Check{
{
Name: "test-pass-1",
Category: "Test",
Run: func() CheckResult {
return CheckResult{Status: StatusPass, Message: "all good"}
},
},
{
Name: "test-pass-2",
Category: "Test",
Run: func() CheckResult {
return CheckResult{Status: StatusPass, Message: "also good"}
},
},
}
results := RunChecks(checks)
require.Len(t, results, 2)
assert.Equal(t, StatusPass, results[0].Status)
assert.Equal(t, StatusPass, results[1].Status)
assert.False(t, HasFailures(results))
}
func TestRunChecks_WithFailure(t *testing.T) {
checks := []Check{
{
Name: "test-pass",
Category: "Test",
Run: func() CheckResult {
return CheckResult{Status: StatusPass, Message: "ok"}
},
},
{
Name: "test-fail",
Category: "Test",
Run: func() CheckResult {
return CheckResult{Status: StatusFail, Message: "broken"}
},
},
}
results := RunChecks(checks)
require.Len(t, results, 2)
assert.True(t, HasFailures(results))
}
func TestRunChecks_WarnDoesNotCountAsFailure(t *testing.T) {
checks := []Check{
{
Name: "test-warn",
Category: "Test",
Run: func() CheckResult {
return CheckResult{Status: StatusWarn, Message: "maybe"}
},
},
}
results := RunChecks(checks)
require.Len(t, results, 1)
assert.Equal(t, StatusWarn, results[0].Status)
assert.False(t, HasFailures(results))
}
func TestRunChecks_PreservesCheckMetadata(t *testing.T) {
checks := []Check{
{
Name: "my-check",
Category: "My Category",
Run: func() CheckResult {
return CheckResult{Status: StatusPass, Message: "details"}
},
},
}
results := RunChecks(checks)
require.Len(t, results, 1)
assert.Equal(t, "my-check", results[0].Name)
assert.Equal(t, "My Category", results[0].Category)
assert.Equal(t, "details", results[0].Message)
}
func TestCategorySummary_AllPass(t *testing.T) {
results := []CheckResult{
{Category: "A", Status: StatusPass},
{Category: "A", Status: StatusPass},
}
summary := CategorySummary(results)
require.Len(t, summary, 1)
assert.Equal(t, StatusPass, summary["A"])
}
func TestCategorySummary_FailOverridesWarn(t *testing.T) {
results := []CheckResult{
{Category: "A", Status: StatusPass},
{Category: "A", Status: StatusWarn},
{Category: "A", Status: StatusFail},
}
summary := CategorySummary(results)
assert.Equal(t, StatusFail, summary["A"])
}
func TestCategorySummary_WarnOverridesPass(t *testing.T) {
results := []CheckResult{
{Category: "A", Status: StatusPass},
{Category: "A", Status: StatusWarn},
}
summary := CategorySummary(results)
assert.Equal(t, StatusWarn, summary["A"])
}
+89
View File
@@ -0,0 +1,89 @@
package ui
import (
"fmt"
"io"
"regexp"
"strings"
"unicode/utf8"
)
var ansiEscapeRe = regexp.MustCompile(`\x1b\[[0-9;]*[A-Za-z]`)
func VisibleWidth(s string) int {
return utf8.RuneCountInString(ansiEscapeRe.ReplaceAllString(s, ""))
}
func RenderTable(out io.Writer, rows [][]string, after func(rowIdx int) error) error {
if len(rows) == 0 {
return nil
}
cols := len(rows[0])
widths := make([]int, cols)
for _, row := range rows {
for i, cell := range row {
if w := VisibleWidth(cell); w > widths[i] {
widths[i] = w
}
}
}
for rIdx, row := range rows {
for i, cell := range row {
if i == cols-1 {
if _, err := fmt.Fprint(out, cell); err != nil {
return err
}
continue
}
pad := widths[i] - VisibleWidth(cell)
if _, err := fmt.Fprint(out, cell, strings.Repeat(" ", pad+2)); err != nil {
return err
}
}
if _, err := fmt.Fprintln(out); err != nil {
return err
}
if after != nil {
dataIdx := rIdx - 1
if err := after(dataIdx); err != nil {
return err
}
}
}
return nil
}
func FirstColumnIndent(rows [][]string) string {
if len(rows) == 0 {
return ""
}
w := 0
for _, row := range rows {
if v := VisibleWidth(row[0]); v > w {
w = v
}
}
return strings.Repeat(" ", w+2)
}
func Truncate(s string, n int) string {
runes := []rune(s)
if len(runes) <= n {
return s
}
if n <= 3 {
return string(runes[:n])
}
return string(runes[:n-3]) + "..."
}
func TruncateLeft(s string, n int) string {
runes := []rune(s)
if len(runes) <= n {
return s
}
if n <= 3 {
return string(runes[len(runes)-n:])
}
return "..." + string(runes[len(runes)-(n-3):])
}