madt_auth_use_ssh skips DirScope.resolve and can mutate a foreign primary checkout's remote #262

New issue

Closed

opened 2026-07-19 03:17:23 +00:00 by hexajon · 0 comments

hexajon commented

2026-07-19 03:17:23 +00:00

(Migrated from codeberg.org)

Pre-release audit finding: madt_auth_use_ssh takes dir= but is a documented no-resolve site - the handler passes the raw dir straight to svcgit.RemoteUseSSH (internal/mcp/tools_auth.go:526-533; exemption recorded at internal/mcp/tools_auth.go:44-50 and internal/parity/dirscope_drift.go:179-183). RemoteUseSSH itself performs no foreign-checkout check either.

Consequence: the tool can flip the origin remote URL (HTTPS <-> SSH) of a FOREIGN PRIMARY checkout - mutating exactly the shared state ADR 0019 declares immutable. Blast radius is small (a reversible protocol flip of the same remote, no token exposure), but it breaks the checkout owner's HTTPS token-injection path and any hook expectations until flipped back, and it is the one dir-taking mutation that bypasses the guard chain every other tool routes through.

The recorded exemption rationale ("SSH auth is user-managed outside the HTTPS token model") justifies skipping credential resolution - it does not address the foreign-mutation angle at all.

Fix, either direction works:

Route the handler through DirScope.resolve and refuse foreign targets like the other local-git mutations do (drop the entry from dirScopeNoResolveSites; the parity guard then enforces it), or
Keep the exemption but make it deliberate: amend ADR 0019 to record that remote-protocol flips on foreign checkouts are permitted and why, and extend the dirscope_drift.go reason to cover the foreign-mutation angle.

Option 1 seems right - there is no obvious use case for flipping a checkout you do not own.

Acceptance: either the tool refuses a foreign dir (with a test alongside the existing foreign-guard tests), or ADR 0019 + the drift-guard reason explicitly record the exemption.

Pre-release audit finding: `madt_auth_use_ssh` takes `dir=` but is a documented no-resolve site - the handler passes the raw dir straight to `svcgit.RemoteUseSSH` (`internal/mcp/tools_auth.go:526-533`; exemption recorded at `internal/mcp/tools_auth.go:44-50` and `internal/parity/dirscope_drift.go:179-183`). `RemoteUseSSH` itself performs no foreign-checkout check either. Consequence: the tool can flip the origin remote URL (HTTPS <-> SSH) of a FOREIGN PRIMARY checkout - mutating exactly the shared state ADR 0019 declares immutable. Blast radius is small (a reversible protocol flip of the same remote, no token exposure), but it breaks the checkout owner's HTTPS token-injection path and any hook expectations until flipped back, and it is the one dir-taking mutation that bypasses the guard chain every other tool routes through. The recorded exemption rationale ("SSH auth is user-managed outside the HTTPS token model") justifies skipping credential resolution - it does not address the foreign-mutation angle at all. Fix, either direction works: 1. Route the handler through `DirScope.resolve` and refuse foreign targets like the other local-git mutations do (drop the entry from `dirScopeNoResolveSites`; the parity guard then enforces it), or 2. Keep the exemption but make it deliberate: amend ADR 0019 to record that remote-protocol flips on foreign checkouts are permitted and why, and extend the `dirscope_drift.go` reason to cover the foreign-mutation angle. Option 1 seems right - there is no obvious use case for flipping a checkout you do not own. Acceptance: either the tool refuses a foreign dir (with a test alongside the existing foreign-guard tests), or ADR 0019 + the drift-guard reason explicitly record the exemption.

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

sixfold-space/madtea#262

No description provided.

Rows
Columns