push: a branch value with a leading + force-pushes past ValidateRef #637

New issue

Closed

opened 2026-09-25 15:50:29 +00:00 by hexajon · 0 comments

hexajon commented

2026-09-25 15:50:29 +00:00

Owner

What

This is a second force path that ADR 0029 does not allow, next to #635. It needs no force flag and no lease.

madt_push branch= and madtea push <branch> pass the branch value to git push -u origin <branch>. In a git refspec, a leading + forces the update. The git-push manual says a refspec is "an optional plus +, followed by the source object", and that a non-fast-forward "update can be overridden by adding ... the optional leading + to a refspec". git also accepts +foo as a branch name: git check-ref-format --branch '+foo' exits 0.

The comment at internal/git/push.go:14-18 gives "a definite --force-class abuse" as one reason for the leading-dash check. A leading + is the same abuse in refspec form, and nothing checks for it.

The path, read from the code

internal/mcp/tools_localgit.go:650 passes input.Branch unchanged to svcgit.Push.
internal/service/git/push.go:125 calls ValidateRef. internal/git/validate.go:20 rejects only a leading -, so +main passes.
The identity guard and the signing guard (internal/service/git/push.go:134 and :140) read the commits on +main. That ref does not resolve, so the read fails. Both guards pass on a read error (internal/service/git/identity_guard.go:76-86, internal/service/git/signing_guard.go:115-117). The identity guard comment assumes that "the push that follows fails on its own". With a leading +, the push does not fail.
internal/git/push.go:22 runs git push -u origin +main. git reads that as a forced push of the local main.

So madt_push branch="+main" force-pushes main, with force unset. Forge branch protection can refuse a force push to a protected branch. An unprotected branch has no such guard.

Reconcile also reaches GitPushDir: realGitOps.Push (internal/service/orchestrate/types.go:205) pushes the PR head ref that comes from the forge. I have not traced that path further.

I have not reproduced this end to end yet. Each step above comes from the code and the git manual.

Acceptance

Every path that reaches git push refuses a branch value with a leading + before git runs.
A test against a local bare remote shows the refusal and shows that the remote branch did not change.
Together with #635, no madtea surface can make a forced push.

## What This is a second force path that ADR 0029 does not allow, next to #635. It needs no `force` flag and no lease. `madt_push branch=` and `madtea push <branch>` pass the branch value to `git push -u origin <branch>`. In a git refspec, a leading `+` forces the update. The git-push manual says a refspec is "an optional plus +, followed by the source object", and that a non-fast-forward "update can be overridden by adding ... the optional leading + to a refspec". git also accepts `+foo` as a branch name: `git check-ref-format --branch '+foo'` exits 0. The comment at `internal/git/push.go:14-18` gives "a definite --force-class abuse" as one reason for the leading-dash check. A leading `+` is the same abuse in refspec form, and nothing checks for it. ## The path, read from the code 1. `internal/mcp/tools_localgit.go:650` passes `input.Branch` unchanged to `svcgit.Push`. 2. `internal/service/git/push.go:125` calls `ValidateRef`. `internal/git/validate.go:20` rejects only a leading `-`, so `+main` passes. 3. The identity guard and the signing guard (`internal/service/git/push.go:134` and `:140`) read the commits on `+main`. That ref does not resolve, so the read fails. Both guards pass on a read error (`internal/service/git/identity_guard.go:76-86`, `internal/service/git/signing_guard.go:115-117`). The identity guard comment assumes that "the push that follows fails on its own". With a leading `+`, the push does not fail. 4. `internal/git/push.go:22` runs `git push -u origin +main`. git reads that as a forced push of the local `main`. So `madt_push branch="+main"` force-pushes `main`, with `force` unset. Forge branch protection can refuse a force push to a protected branch. An unprotected branch has no such guard. Reconcile also reaches `GitPushDir`: `realGitOps.Push` (`internal/service/orchestrate/types.go:205`) pushes the PR head ref that comes from the forge. I have not traced that path further. I have not reproduced this end to end yet. Each step above comes from the code and the git manual. ## Acceptance - Every path that reaches `git push` refuses a branch value with a leading `+` before git runs. - A test against a local bare remote shows the refusal and shows that the remote branch did not change. - Together with #635, no madtea surface can make a forced push.

hexajon added the

bug

label

2026-09-25 15:50:29 +00:00

hexajon referenced this issue

2026-09-25 15:50:49 +00:00

push --force: the lease does not protect commits that madt_status fetched #635

hexajon referenced this issue

2026-10-20 03:28:55 +00:00

push: five git push calls run outside the guarded Writer #833

hexajon referenced this issue from a pull request that will close it,

2026-10-20 16:40:49 +00:00