[FFmpeg-devel] rebasing security

[Alexander Strasser]

Hi Michael,
hi all!

I think it's a good time to bring stuff like this up for discussion.

On 2025-08-03 21:02 +0200, Michael Niedermayer wrote:
> 
> On Sun, Aug 03, 2025 at 05:31:39PM +0200, Michael Niedermayer wrote:
> [...]
> > The solutions are obvious:
> > 1. ignore security and supply chain attacks
> > 2. use merges not rebases on the server
> > 3. rebase locally, use fast forward only
> > 4. verify on server rebases
> 
> Maybe not everyone understood the problem. So let me try a different
> explanation. Without any signatures.
> 
> In the ML workflow: (for simplicity we assume reviewer and commiter is the same person)
> 1. someone posts a patch
> 2. patch is locally applied or rebased
> 3. commit is reviewed
> 4. commit is tested
> 5. commit is pushed
> 
> Here the only way to get bad code in, is through the reviewer
> If the reviewer doesnt miss anything and his setup is not compromised
> then what he pushes is teh reviewed code
> 
> if its manipulated after its pushed git should light up like a christmess tree
> on the next "git pull --rebase"
> 
> 
> With the rebase on webapp (gitlab or forgejo) workflow
> 1. someone posts a pull request
> 2. pr is reviewed
> 3. pr is approved
> 4. pr is rebased
> 5. pr is tested
> 6, pr is pushed
> 
> now here of course the same reviewer trust or compromised scenarios exist
> but we also have an extra one and that is the server
> because the server strips the signatures during rebase it can modify the
> commit. And this happens after review and because a rebase was litterally
> requested by the reviewer its not likely to be noticed as something out of
> place

If I understand the original point you wanted to discuss correctly,
than this is not a question of rebase or merge but one of letting
**commits happen on the forge**. If it happens it bears the
possibility of modification on the server the forge is running on.

TL;DR: I think it's fine the way it's setup now.

I'm not against letting rebase/merges happen on the server because
otherwise we would lose a lot of advantages and comfort we get by
using a forge for PRs.

Only alternative I see is to do PRs on the forge and doing merging
manually by the same person that ensures reviewed PR is not changed
and pushes (after rebase or with a clean merge commit) from their
machine.

Just to be clear here I'm talking about stuff happening for
rebase/merge on the server. There is another thing that is
done sometimes that I'm against, that is creating and pushing
commits from runners. That is a way too dangerous practice IMHO
and I would argue we should never do it.


Best regards
  Alexander

PS.
Maybe there are some less conventional possibilities I'm missing, that
could be implemented. So if you see any that seem worth to pursue it
might be interesting.