Repository navigation
fix(githttp): refuse a branch tip that is not a commit before the ref store takes it - #860
Conversation
…re takes it
A command could set a refs/heads/* value to a blob, a tree or an annotated
tag. The update hook's CAS stored it, and git then refused the same write
("trying to write non-commit object ... to branch"). The ref store and git
disagreed from then on: every read of the repository answered 500, and
every later update of the branch lost its CAS.
Pre-receive and the update hook now read the object types of the new values
with one git child and refuse a branch tip that is not a commit, with a
per-ref ng. The refusal comes before the policy check, so a refused tip
also stores no pack. The REST write routes cannot set such a tip, because
each takes its value from a peeled commit or a new commit. Tests pin the
two routes that take a revision from the caller.
_Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-sonnet-5-5` • Thinking: `xhigh`_
Change-Id: Ied5fdb6a1554de5dd027fe6997fcca03168e6b35
Signed-off-by: Thomas Kosiewski <tk@coder.com>
…e-commit Change-Id: I22d9d7c829bb9cf79a078ead469add156207aa4d Signed-off-by: Thomas Kosiewski <tk@coder.com>
…s nothing _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-sonnet-5-5` • Thinking: `xhigh`_ Change-Id: I6700e2f23ee3d0d330fd348c3a45af239950e249 Signed-off-by: Thomas Kosiewski <tk@coder.com>
receive-pack refuses a command whose new value it lacks before the update hook runs, so the tip check has no kind to refuse for a missing object. The in-process update hook tests name objects that no repository holds. _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-sonnet-5-5` • Thinking: `xhigh`_ Change-Id: Iaaf151f4246262a13f69767c62a1506c808dcb14 Signed-off-by: Thomas Kosiewski <tk@coder.com>
…heck preReceive and updateHook call the new check and it adds four declarations. None of them changes a push-hold or schedule step that the model describes. _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-sonnet-5-5` • Thinking: `xhigh`_ Change-Id: Ib8e639b85241f97157ff16ed8a3617e8d6dbdab0 Signed-off-by: Thomas Kosiewski <tk@coder.com>
…ntee for branch tips Drop the merge row that checked nothing (a merge source is a branch name, so any 40-hex id answers 404). Add a fake-git unit test for the fail-closed type read and put objectKinds in the no-replace-objects test. Name the push rules of the refusal in the docs. Signed-off-by: Thomas Kosiewski <tk@coder.com> --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-sonnet-5-5` • Thinking: `xhigh`_ Change-Id: I154ffc8eab7c86163ec50915a511622c5e64dc3f
…e-commit Change-Id: Ic8d2eb7342ddf565c5523f913ed89de1e40b4d31 Signed-off-by: Thomas Kosiewski <tk@coder.com>
Signed-off-by: Thomas Kosiewski <tk@coder.com> --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-sonnet-5-5` • Thinking: `xhigh`_ Change-Id: I2e9ea0f1a0aae59bf212e4c0de7ff857293d86ba
|
@codex review |
|
@codex security review |
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
…he policy The mixed push now runs under no-force-push and moves main to a blob. A hook that asked the policy first would walk the ancestry of a blob. Signed-off-by: Thomas Kosiewski <tk@coder.com> --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-sonnet-5-5` • Thinking: `xhigh`_ Change-Id: I47c86578bd237bb852d97514157ee627cf0fa44b
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
…e-commit Change-Id: Ifa160c07aeb2a2b3564ce8f75713b7fcba0097d5 Signed-off-by: Thomas Kosiewski <tk@coder.com>
Signed-off-by: Thomas Kosiewski <tk@coder.com> --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-sonnet-5-5` • Thinking: `xhigh`_ Change-Id: Ica3d5fee2a5e0c5f3c4c854f3524dfbd136c0af3
|
@codex review |
|
@codex security review |
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
Why
A push command can set a
refs/heads/*value to a blob, a tree or an annotated tag. The update hook's CAS stores that value in the ref store, and git then refuses the same write (trying to write non-commit object ... to branch). From then on the store and git disagree. Every read of the repository answers 500, because hydration cannot write the branch, and every later update of the branch loses its CAS. Agit:writetoken is enough, and no ref policy has to be set.This is a robustness fix: client input leaves the server in a state that it cannot read.
What changed
ng(update: <ref> must point at a commit, not a blob). The refusal comes before the policy check, so a refused tip also stores no pack, and an--atomicpush that carries one lands nothing.--atomicand whole-push-decline rules as a policy denial.Scope
branches/createresolves its base throughResolveRef, which peels a tag to its commit and answers 404 for a blob or a tree. Merge reads its source and target as branch names.commit-pack,diff-commit,restore-commitand the merge commit write a new commit. One test pins the one route that takes a caller's revision (branches/create).repair-refsremoves refs by name and reads no object types, so repairing such a ref is a separate change.Verification
TestBranchTipThatIsNoCommitIsRefused(blob, tree and annotated tag as the tip of a created or updated branch, underno-force-push,--atomic, and a mixed push; no pack stored for a refused tip; tags and a fast-forward still land). It fails onorigin/mainin every row, and passes with the fix on the local, PostgreSQL and S3-WAL lanes.osv.shnode with a raw receive-pack client. Onorigin/maineach of four pushes getsng ... invalid new value provided,ls-remoteandclonethen answer 500, and serve logs 8 hydrate failures. With the fix each getsng ... must point at a commit,ls-remoteand a fresh clone work, and serve logs none.GIT_NO_REPLACE_OBJECTS=1. Each fails when its guard is removed.no-force-pushand movesmainto a blob, so an update hook that asks the policy before the type fails it.internal/githttpand its importers pass.Generated with
xum• Model:anthropic:claude-sonnet-5-5• Thinking:xhigh