fix(repo-mode): prevent unsafe dir defaults and forced pushes (#85)

[Nivesh353]

Summary

• Repo mode (repo: in the SDK, --repo in the CLI) silently defaulted the
  working directory to process.cwd() when dir was omitted. If that
  directory had no .git of its own, git commands would escape upward and
  run destructive operations (reset --hard, remote set-url) against
  whatever ancestor repo was found instead — corrupting an unrelated project.
• finalize() always committed and pushed on every call, success or failure,
  with no way to opt out — even for pure read-only use.
• Fixes both, closes repo: mode: unsafe dir default can reset an unrelated parent git repo; unconditional push on every call with no read-only mode #85:
  • dir is now required in repo mode; omitting it throws instead of
    silently falling back to cwd.
  • Before touching an existing directory, initLocalSession now verifies
    it's a git repo root in its own right (git rev-parse --show-toplevel,
    compared via realpathSync to handle symlinked paths like macOS's
    /tmp → /private/tmp) and that it already tracks the expected
    remote — refusing to proceed otherwise.
  • Added repo.readOnly (SDK) and --read-only (CLI), which skip
    commitChanges()/push() in finalize() while still stripping the
    embedded token from origin.
• Also fixed two stale test/mcp.test.ts assertions that were checking
  execute() against an old plain-string-return contract; the rest of the
  codebase (e.g. sdk.test.ts) already expects the current structured
  { content, details } shape from buildTool(). Unrelated to repo: mode: unsafe dir default can reset an unrelated parent git repo; unconditional push on every call with no read-only mode #85, found
  while confirming this branch's npm test was clean.

Test plan

• npm run build — no type errors
• npm test — 39/39 passing (was 37/39 on unmodified main; the 2
  failures were the stale mcp.test.ts assertions above, now fixed)
• Manually reproduced the original vulnerability (git-less subfolder
  inside a real repo, pointed at a different real repo) — confirmed it
  threw and left the victim repo untouched, instead of resetting it
• Ran repo mode 3x with readOnly: true (SDK) and --read-only (CLI)
  against a real public repo — 0 pushes landed remotely, confirmed via
  git ls-remote
• Ran repo mode without readOnly — confirmed normal commit/push
  behavior is unchanged (no regression)

[shreyas-lyzr]

Three issues worth addressing before merge — one is a URL normalization bug causing false-positive rejections, one is a latent shell injection in commitChanges, and one is a minor CLI/SDK behavioral inconsistency. The readOnly feature and the git-root validation are solid.

1. cleanUrl does not normalize .git suffixes — remote-mismatch check produces false positives

A repo cloned via https://github.com/org/repo.git stores that URL as its origin. If the caller then passes https://github.com/org/repo (without .git), cleanUrl(existingOrigin) !== cleanUrl(url) evaluates to true and the session is refused with a misleading error, even though both point to the same repo. This is a regression vs. the old code path which would overwrite and proceed.

Fix: normalize .git in cleanUrl.

2. Shell injection via commit message in git commit -m "${commitMsg}"

commitChanges interpolates commitMsg directly into a shell command string passed to execSync. The default value is safe, but the function accepts arbitrary caller-supplied messages — a message containing a double-quote or $() breaks out of the shell argument. Use execFileSync with an argument array for this call, or shell-escape the message before interpolation.

3. CLI --dir guard is fragile compared to SDK enforcement

The SDK throws when dir is omitted in repo mode. The CLI silently derives /tmp/gitagent/<repo-name> when --dir is not passed. The guard used is dir === process.cwd() — if the user explicitly passes --dir $(pwd), the guard is bypassed and their actual cwd is used as the repo dir. Compare rawDir === undefined instead.

Security pass: no new dependencies in this PR, no hardcoded secrets, no obvious injection/authz gaps beyond finding #2 above.

[shreyas-lyzr]

All three findings from the previous review are addressed. The new 'no origin' fix also looks correct. Here's what I verified:

Finding 1 — cleanUrl missing .git strip: Fixed. cleanUrl now strips both the auth prefix and .git suffix (session.ts line 33). The remote-URL comparison at line 93 will correctly match whether or not the caller supplied a .git suffix.

Finding 2 — shell injection in commitChanges: Fixed. execFileSync with an argv array is used at line 177. The commit message is passed as a discrete argument rather than shell-interpolated, so arbitrary content is safe.

Finding 3 — fragile rawDir guard: Fixed. index.ts now tracks rawDir as undefined when --dir is not passed (parseArgs leaves it unset), and the default-dir branch checks rawDir === undefined (line 354) rather than comparing the string value. Explicit --dir matching cwd is now correctly honoured.

New fix — no-origin crash: Correct. When git remote get-url origin fails (empty repo pointed at this URL for the first time), existingOrigin is set to '' and the ternary at line 102 calls git remote add origin ... instead of git remote set-url origin ..., which would have thrown. The subsequent fetch and reset proceed normally. The finalize path (line 191) always calls set-url, which is safe because by that point origin is guaranteed to exist (either from clone, from add, or from the previous set-url).

One pre-existing note (not introduced by this PR, not blocking): the git() helper interpolates aUrl and cleanUrl(url) into a shell string, so a maliciously crafted repo URL with shell metacharacters could inject commands. That pattern exists in main already; it is out of scope here but worth a follow-up issue.

Security pass: no new dependencies, no hardcoded secrets, no new injection surface, no auth/authz gaps. The aUrl interpolation concern pre-dates this branch.

Tests: 39/39 per the PR description; the two previously failing mcp.test.ts assertions are fixed to match the current structured return shape.