1. 50
  1.  

    1. 3

      Very creative bug. Happy to see the author getting a good reward, but something like this could’ve been so devastating. Hard to say if it was valued high enough

      1. 1

        But we just created a pull request where the “base branch” is a commit hash, not a branch. And anyone can create a new commit hash in the base repository, since GitHub shares commits between forks.

        I can’t wrap my mind around it. What does “the base repository” refer to? I don’t think you can commit to the repo being forked without upfront write access to it.

        My understanding is that when you update the base branch of the pull request from step (2) to the commit hash from step (3), you essentially point the base branch to a commit in your own fork, which may contain malicious Actions workflow code, but I fail to understand how does this have anything to do with the base-branch-may-be-a-commit-hash bug. Can’t I just point the base branch to a new branch in my fork?

        1. 6

          I don’t think you can commit to the repo being forked without upfront write access to it.

          See the end: “because GitHub shares commits between forks”. For reasons of efficiency, all repos that are related by forking are actually a single repo in the GitHub backend, and they just have their branches and tags namespaced. (And all the ref names are rewritten by their custom Git server on the way in and out, I guess.) If you fork some repo, make a commit, and then go to https://github.com/original-owner/reponame/commit/yourcommithashgoeshere you will see your commit, even though it “doesn’t exist” in the original repo. The UI does say “This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository”, but the logic for PR base refs (prior to the bug fix) didn’t care about checking that: it would just look up the commit, find it, and happily consider it to be part of the upstream repo.

          Can’t I just point the base branch to a new branch in my fork?

          No, it won’t accept a ref from another repo (barring the weird behavior discussed above). The field just contains a ref name, not owner/repo/ref. So you have to fool it into checking out something that it thinks is in the upstream repo but actually is controlled by a third party.

          1. 1

            No, it won’t accept a ref from another repo (barring the weird behavior discussed above).

            I suppose you meant to say “a ref from another namespace in the same underlying repo”? There is no “another repo” on GitHub’s backend after all.

            Anyway, I think the fundamental issue is that commit hashes are not namespaced. I hope they have fixed that as well.

            1. 3

              Clearly you know what I mean :-P

              And no, the commit hashes aren’t namespaced and likely won’t be. I guess they just have to recognize (and probably are recognizing at the moment) that any time they’re parsing a refspec that has to respect the namespacing, they need to filter down to the right kinds of refs.