fix(git): apply M3 adversarial-review findings (9 confirmed)

Security/robustness:
- WithToken now resets the inherited credential.helper (`-c credential.helper=`)
  so a token clone never persists the token to disk (e.g. ~/.git-credentials via
  the `store` helper), and places the GIT_ASKPASS shim under the XDG runtime dir
  (exec-permitting) instead of a possibly-noexec /tmp.
- hardenedEnv sets `GIT_SSH_COMMAND=ssh -o BatchMode=yes -o ConnectTimeout=10`
  when no TTY is attached (and not preset), so an unknown host key or a
  passphrase-protected key with no agent fails fast per-repo instead of hanging
  the parallel batch (acceptance #6) — still honoring ~/.ssh/config + ProxyJump.

Correctness:
- `ws clone` validates the existing remote with git.SameRemote (ssh/https/
  shorthand equivalence) and treats an unreadable origin as a per-repo failure
  rather than a silent pass.
- `ws git` now follows the spec grammar `git <args> -- [names]` (ArgsLenAtDash)
  and supports the repo-name subset.
- status table: show the conflicts count when non-zero; render AHEAD/BEHIND as
  "-" for an upstream-gone branch instead of a misleading "+0/-0".
- ExpandURL trims the repo:/shorthand value (no leaked leading space).

New tests: SameRemote equivalence, credential.helper reset, URL trimming.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: gustavobertoi

go.mod

@@ -15,6 +15,7 @@ require (
 	github.com/moby/moby/client v0.4.1
 	github.com/spf13/cobra v1.10.2
 	golang.org/x/sync v0.20.0
+	golang.org/x/term v0.44.0
 	modernc.org/sqlite v1.52.0
 )
 
@@ -76,7 +77,7 @@ require (
 	go.opentelemetry.io/otel/trace v1.35.0 // indirect
 	go.yaml.in/yaml/v4 v4.0.0-rc.4 // indirect
 	golang.org/x/crypto v0.52.0 // indirect
-	golang.org/x/sys v0.45.0 // indirect
+	golang.org/x/sys v0.46.0 // indirect
 	golang.org/x/text v0.37.0 // indirect
 	modernc.org/libc v1.72.3 // indirect
 	modernc.org/mathutil v1.7.1 // indirect

go.sum

@@ -172,8 +172,10 @@ golang.org/x/sync v0.20.0 h1:e0PTpb7pjO8GAtTs2dQ6jYa5BWYlMuX047Dco/pItO4=
 golang.org/x/sync v0.20.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0=
 golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
-golang.org/x/sys v0.45.0 h1:dO4czNzziLiiXplLQgBCEpCvXQ3dnkn0SdaZSYdQ+FY=
-golang.org/x/sys v0.45.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw=
+golang.org/x/sys v0.46.0 h1:noSf2Fq6F8DBgS+LysIkx7rIExoNHJsxOAtPp4rthXw=
+golang.org/x/sys v0.46.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw=
+golang.org/x/term v0.44.0 h1:0rLvDRCtNj0gZkyIXhCyOb2OAzEhLVqc4B+hrsBhrmc=
+golang.org/x/term v0.44.0/go.mod h1:7ze4MdzUzLXpSAoFP1H0bOI9aXDqveSvatT5vKcFh2Y=
 golang.org/x/text v0.37.0 h1:Cqjiwd9eSg8e0QAkyCaQTNHFIIzWtidPahFWR83rTrc=
 golang.org/x/text v0.37.0/go.mod h1:a5sjxXGs9hsn/AJVwuElvCAo9v8QYLzvavO5z2PiM38=
 golang.org/x/tools v0.44.0 h1:UP4ajHPIcuMjT1GqzDWRlalUEoY+uzoZKnhOjbIPD2c=

internal/cli/ws.go

@@ -193,7 +193,8 @@ func branchLabel(s *git.Status) string {
 }
 
 func abLabel(s *git.Status) string {
-	if s.Upstream == "" {
+	// No upstream, or the upstream was deleted: ahead/behind is unknown.
+	if s.Upstream == "" || s.UpstreamGone {
 		return "-"
 	}
 	return fmt.Sprintf("+%d/-%d", s.Ahead, s.Behind)
@@ -203,6 +204,10 @@ func stateLabel(s *git.Status) string {
 	if !s.Dirty() {
 		return "clean"
 	}
+	if s.Conflicts > 0 {
+		return fmt.Sprintf("dirty (%d staged, %d unstaged, %d untracked, %d conflicts)",
+			s.Staged, s.Unstaged, s.Untracked, s.Conflicts)
+	}
 	return fmt.Sprintf("dirty (%d staged, %d unstaged, %d untracked)", s.Staged, s.Unstaged, s.Untracked)
 }
 
@@ -230,11 +235,17 @@ func newWsCloneCmd(g *GlobalOpts) *cobra.Command {
 					return fmt.Errorf("no git URL declared")
 				}
 				if gx.IsRepo(ctx, r.dir) {
-					// Idempotent: validate the remote matches the expected URL.
-					if cur, err := gx.RemoteURL(ctx, r.dir); err == nil && cur != r.url {
+					// Idempotent: validate the remote matches (tolerating
+					// ssh/https/shorthand equivalence). A failure to read origin is
+					// a per-repo error, not a silent pass.
+					cur, err := gx.RemoteURL(ctx, r.dir)
+					if err != nil {
+						return fmt.Errorf("exists but cannot read its origin remote: %w", err)
+					}
+					if !git.SameRemote(cur, r.url) {
 						return fmt.Errorf("exists but origin is %q, expected %q", cur, r.url)
 					}
-					return nil // already cloned
+					return nil // already cloned, correct remote
 				}
 				return gx.Clone(ctx, r.url, r.dir, git.CloneOptions{})
 			})
@@ -285,13 +296,19 @@ func newWsSyncCmd(g *GlobalOpts) *cobra.Command {
 func newWsGitCmd(g *GlobalOpts) *cobra.Command {
 	var jobs int
 	cmd := &cobra.Command{
-		Use:   "git -- <git args...>",
-		Short: "Run an arbitrary git command across every repo",
+		Use:   "git <git args...> [-- names...]",
+		Short: "Run an arbitrary git command across repos (subset after --)",
 		RunE: func(cmd *cobra.Command, args []string) error {
-			if len(args) == 0 {
-				return fmt.Errorf("usage: ws git -- <git args...>")
+			// Grammar: `ws git <git args...> -- [all|name...]`. Everything before
+			// the -- is the git command; names after it select a repo subset.
+			gitArgs, names := args, []string(nil)
+			if dash := cmd.ArgsLenAtDash(); dash >= 0 {
+				gitArgs, names = args[:dash], args[dash:]
+			}
+			if len(gitArgs) == 0 {
+				return fmt.Errorf("usage: %s ws git <git args...> [-- names...]", rootName(cmd))
 			}
-			repos, err := loadRepos(nil)
+			repos, err := loadRepos(names)
 			if err != nil {
 				return err
 			}
@@ -306,7 +323,7 @@ func newWsGitCmd(g *GlobalOpts) *cobra.Command {
 				if !gx.IsRepo(ctx, r.dir) {
 					return fmt.Errorf("not cloned")
 				}
-				return gx.Run(ctx, r.dir, args...)
+				return gx.Run(ctx, r.dir, gitArgs...)
 			})
 			return reportResults(cmd, g, "git", results)
 		},

internal/git/askpass.go

@@ -5,18 +5,33 @@ import (
 	"os"
 	"path/filepath"
 	"strings"
+
+	"github.com/open-source-cloud/devstack/internal/xdg"
 )
 
 // WithToken returns a copy of the Git handle that authenticates HTTPS remotes
 // with token via a generated GIT_ASKPASS shim, plus a cleanup func that removes
-// the shim. The token is written to a 0600 file in a private temp dir and never
-// embedded in the URL or .git/config or visible in `ps` (spec 06). The secrets
-// provider supplies the token (M4); this is the injection mechanism.
+// the shim. The token is written to a 0600 file in a private dir and never
+// embedded in the URL, .git/config, or visible in `ps` (spec 06). It also
+// disables any inherited credential.helper for these invocations, so the token
+// is NOT persisted to disk (e.g. ~/.git-credentials by the `store` helper) after
+// a successful clone. The secrets provider supplies the token (M4); this is the
+// injection mechanism.
 func (g *Git) WithToken(token string) (*Git, func(), error) {
 	if token == "" {
 		return g, func() {}, nil
 	}
-	dir, err := os.MkdirTemp("", "devstack-askpass-")
+	// Place the shim under the XDG runtime dir (typically an exec-permitting
+	// tmpfs) rather than /tmp, which may be mounted noexec on hardened systems
+	// (GIT_ASKPASS must be executable). Fall back to the default temp dir if the
+	// runtime dir is unavailable.
+	base := xdg.RuntimeDir()
+	if base != "" {
+		if err := os.MkdirAll(base, 0o700); err != nil {
+			base = ""
+		}
+	}
+	dir, err := os.MkdirTemp(base, "devstack-askpass-")
 	if err != nil {
 		return nil, nil, fmt.Errorf("create askpass dir: %w", err)
 	}
@@ -37,7 +52,14 @@ func (g *Git) WithToken(token string) (*Git, func(), error) {
 		return nil, nil, fmt.Errorf("write askpass shim: %w", err)
 	}
 
-	cp := &Git{bin: g.bin, env: append(append([]string{}, g.env...), "GIT_ASKPASS="+shim)}
+	cp := &Git{
+		bin: g.bin,
+		env: append(append([]string{}, g.env...), "GIT_ASKPASS="+shim),
+		// Reset the inherited credential.helper list so a successful clone's
+		// `git credential approve` has no helper to persist the token to
+		// (e.g. the plaintext `store` helper writing ~/.git-credentials).
+		configArgs: append(append([]string{}, g.configArgs...), "-c", "credential.helper="),
+	}
 	return cp, cleanup, nil
 }
 

internal/git/askpass_test.go

@@ -44,13 +44,28 @@ func TestWithTokenShim(t *testing.T) {
 		t.Errorf("token file perms = %v, want 0600", perm)
 	}
 
+	// The token handle disables inherited credential persistence so a successful
+	// clone never writes the token to ~/.git-credentials (the `store` helper).
+	if !containsPair(tg.configArgs, "-c", "credential.helper=") {
+		t.Errorf("token handle should reset credential.helper, got configArgs=%v", tg.configArgs)
+	}
+
 	// cleanup removes the whole shim dir.
 	cleanup()
 	if _, err := os.Stat(askpass); !os.IsNotExist(err) {
 		t.Error("cleanup should remove the shim")
 	}
 }
 
+func containsPair(s []string, a, b string) bool {
+	for i := 0; i+1 < len(s); i++ {
+		if s[i] == a && s[i+1] == b {
+			return true
+		}
+	}
+	return false
+}
+
 func TestWithTokenEmptyIsNoop(t *testing.T) {
 	g := testGit(t)
 	g2, cleanup, err := g.WithToken("")

internal/git/gitx.go

@@ -15,6 +15,8 @@ import (
 	"os/exec"
 	"strconv"
 	"strings"
+
+	"golang.org/x/term"
 )
 
 // MinVersion is the git floor (spec 06). 2.30 is conservative (porcelain=v2
@@ -25,6 +27,9 @@ var MinVersion = Version{2, 30}
 type Git struct {
 	bin string
 	env []string
+	// configArgs are `-c key=val` pairs prepended to every invocation (used by
+	// WithToken to disable inherited credential persistence).
+	configArgs []string
 }
 
 // Version is a major.minor pair for the git floor check.
@@ -58,19 +63,45 @@ func New() (*Git, error) {
 }
 
 // hardenedEnv augments the process env so git never blocks on an interactive
-// prompt and always emits stable, parseable output (spec 06).
+// prompt and always emits stable, parseable output (spec 06). GIT_TERMINAL_PROMPT
+// and GCM_INTERACTIVE cover git's own prompts and Git Credential Manager (the
+// HTTPS path); for the SSH path, BatchMode is added when NO terminal is attached
+// so an unknown host key or a passphrase-protected key with no agent fails fast
+// per-repo instead of hanging the parallel batch — while still honoring
+// ~/.ssh/config (IdentityFile, Host aliases, ProxyJump) and known_hosts. When a
+// TTY IS attached, interactive passphrase/host-key entry is left intact, and a
+// pre-existing GIT_SSH_COMMAND is always respected.
 func hardenedEnv(base []string) []string {
-	return append(base,
+	env := append(base,
 		"GIT_TERMINAL_PROMPT=0", // never prompt on the terminal
 		"GCM_INTERACTIVE=never", // Git Credential Manager: never pop UI
 		"LC_ALL=C",              // stable, parseable, locale-independent output
 	)
+	if !hasEnv(base, "GIT_SSH_COMMAND") && !term.IsTerminal(int(os.Stderr.Fd())) {
+		env = append(env, "GIT_SSH_COMMAND=ssh -o BatchMode=yes -o ConnectTimeout=10")
+	}
+	return env
+}
+
+// hasEnv reports whether env already defines key.
+func hasEnv(env []string, key string) bool {
+	prefix := key + "="
+	for _, e := range env {
+		if strings.HasPrefix(e, prefix) {
+			return true
+		}
+	}
+	return false
 }
 
 // run executes git in dir (empty = inherit CWD), capturing stdout. A failure is
 // wrapped with the args + captured stderr so it is self-debuggable (§7.6).
 func (g *Git) run(ctx context.Context, dir string, args ...string) ([]byte, error) {
-	cmd := exec.CommandContext(ctx, g.bin, args...)
+	full := args
+	if len(g.configArgs) > 0 {
+		full = append(append([]string{}, g.configArgs...), args...)
+	}
+	cmd := exec.CommandContext(ctx, g.bin, full...)
 	cmd.Dir = dir
 	cmd.Env = g.env
 	var stdout, stderr strings.Builder

internal/git/url.go

@@ -26,21 +26,63 @@ func ExpandURL(spec string) string {
 	}
 	// Explicit passthrough.
 	if rest, ok := strings.CutPrefix(spec, "repo:"); ok {
-		return rest
+		return strings.TrimSpace(rest)
 	}
 	// Already a full URL or scp-like remote or a path.
 	if looksLikeURL(spec) {
 		return spec
 	}
 	if scheme, rest, ok := strings.Cut(spec, ":"); ok {
 		if host, known := knownHosts[scheme]; known {
-			path := strings.TrimSuffix(rest, ".git")
+			path := strings.TrimSuffix(strings.TrimSpace(rest), ".git")
 			return "git@" + host + ":" + path + ".git"
 		}
 	}
 	return spec
 }
 
+// SameRemote reports whether two git remote specs point at the same repository,
+// tolerating ssh/https/scp/shorthand differences (e.g. git@github.com:a/b.git
+// and https://github.com/a/b are the same). Used by `ws clone` so an existing
+// clone reached via a different transport is not flagged as a URL mismatch.
+func SameRemote(a, b string) bool {
+	ah, ap := canonicalRemote(a)
+	bh, bp := canonicalRemote(b)
+	return ah == bh && ap == bp
+}
+
+// canonicalRemote reduces a remote spec to a (host, path) identity. A purely
+// local path has an empty host and its cleaned path.
+func canonicalRemote(s string) (host, path string) {
+	s = ExpandURL(strings.TrimSpace(s))
+	switch {
+	case strings.Contains(s, "://"):
+		s = s[strings.Index(s, "://")+3:]
+		if at := strings.IndexByte(s, '@'); at >= 0 {
+			if slash := strings.IndexByte(s, '/'); slash < 0 || at < slash {
+				s = s[at+1:]
+			}
+		}
+		host, path, _ = strings.Cut(s, "/")
+		host, _, _ = strings.Cut(host, ":") // drop :port
+	case strings.HasPrefix(s, "/"), strings.HasPrefix(s, "."), strings.HasPrefix(s, "~"):
+		return "", strings.TrimSuffix(s, ".git") // local path
+	default:
+		// scp-style git@host:path
+		if at := strings.IndexByte(s, '@'); at >= 0 {
+			s = s[at+1:]
+		}
+		if h, p, ok := strings.Cut(s, ":"); ok {
+			host, path = h, p
+		} else {
+			return "", strings.TrimSuffix(s, ".git")
+		}
+	}
+	host = strings.ToLower(host)
+	path = strings.TrimSuffix(strings.TrimPrefix(path, "/"), ".git")
+	return host, path
+}
+
 // looksLikeURL reports whether spec is already a transport URL, an scp-style
 // remote (git@host:path), or a filesystem path.
 func looksLikeURL(spec string) bool {

internal/git/url_test.go

@@ -21,3 +21,35 @@ func TestExpandURL(t *testing.T) {
 		}
 	}
 }
+
+func TestExpandURLTrimsRepoPassthrough(t *testing.T) {
+	if got := ExpandURL("repo: git@host:acme/x.git"); got != "git@host:acme/x.git" {
+		t.Errorf("repo: passthrough leaked whitespace: %q", got)
+	}
+	if got := ExpandURL("github: acme/api"); got != "git@github.com:acme/api.git" {
+		t.Errorf("shorthand leaked whitespace: %q", got)
+	}
+}
+
+func TestSameRemote(t *testing.T) {
+	same := [][2]string{
+		{"git@github.com:acme/api.git", "https://github.com/acme/api"},
+		{"git@github.com:acme/api.git", "https://github.com/acme/api.git"},
+		{"github:acme/api", "https://github.com/acme/api"},
+		{"ssh://git@github.com:22/acme/api.git", "git@github.com:acme/api.git"},
+	}
+	for _, p := range same {
+		if !SameRemote(p[0], p[1]) {
+			t.Errorf("SameRemote(%q, %q) = false, want true", p[0], p[1])
+		}
+	}
+	diff := [][2]string{
+		{"git@github.com:acme/api.git", "git@github.com:acme/web.git"},
+		{"git@github.com:acme/api.git", "git@gitlab.com:acme/api.git"},
+	}
+	for _, p := range diff {
+		if SameRemote(p[0], p[1]) {
+			t.Errorf("SameRemote(%q, %q) = true, want false", p[0], p[1])
+		}
+	}
+}