Changes to Git commit workflow

Nikita Popov

5 years ago

On Mon, Mar 29, 2021 at 12:52 AM Nikita Popov <nikita.ppv@gmail.com> wrote:

> Hi everyone,
>
> Yesterday (2021-03-28) two malicious commits were pushed to the php-src
> repo [1] from the names of Rasmus Lerdorf and myself. We don't yet know how
> exactly this happened, but everything points towards a compromise of the
> git.php.net server (rather than a compromise of an individual git
> account).
>
> While investigation is still underway, we have decided that maintaining
> our own git infrastructure is an unnecessary security risk, and that we
> will discontinue the git.php.net server. Instead, the repositories on
> GitHub, which were previously only mirrors, will become canonical. This
> means that changes should be pushed directly to GitHub rather than to
> git.php.net.
>
> While previously write access to repositories was handled through our
> home-grown karma system, you will now need to be part of the php
> organization on GitHub. If you are not part of the organization yet, or
> don't have access to a repository you should have access to, contact me at
> nikic@php.net with your php.net and GitHub account names, as well as the
> permissions you're currently missing. Membership in the organization
> requires 2FA to be enabled.
>
> This change also means that it is now possible to merge pull requests
> directly from the GitHub web interface.
>
> We're reviewing the repositories for any corruption beyond the two
> referenced commits. Please contact security@php.net if you notice
> anything.
>
> Regards,
> Nikita
>

Assuming you do already have write access on GitHub, you can change the
upstream repository of your local clone by running "git remote set-url
origin git@github.com:php/php-src.git" (replacing php/php-src with the
repository in question).

Nikita

Paul

5 years ago

You might consider requiring commits be signed while you're at it.

On Sun, Mar 28, 2021 at 3:53 PM Nikita Popov <nikita.ppv@gmail.com> wrote:

Sara Golemon

5 years ago

On Sun, Mar 28, 2021 at 6:57 PM Paul Crovella <paul.crovella@gmail.com>
wrote:

> You might consider requiring commits be signed while you're at it.
>
>

I suggested this as well, and even if we don't require it, we should
STRONGLY encourage it.

I've been signing my commits for several years now, it's not even that
hard.

Rasmus Lerdorf

5 years ago

On Sun, Mar 28, 2021 at 17:15 Sara Golemon <pollita@php.net> wrote:

> On Sun, Mar 28, 2021 at 6:57 PM Paul Crovella <paul.crovella@gmail.com>
> wrote:
>
>> You might consider requiring commits be signed while you're at it.
>>
>>
> I suggested this as well, and even if we don't require it, we should
> STRONGLY encourage it.
>
> I've been signing my commits for several years now, it's not even that
> hard.
>

I think for php-src commits we can require it. For doc and other repos we
can make it optional for now until people are more comfortable with it.

-Rasmus

Gabriel Caruso

5 years ago

On Mon, 29 Mar 2021, 03:30 Rasmus Lerdorf, <rasmus@lerdorf.com> wrote:

> On Sun, Mar 28, 2021 at 17:15 Sara Golemon <pollita@php.net> wrote:
>
> > On Sun, Mar 28, 2021 at 6:57 PM Paul Crovella <paul.crovella@gmail.com>
> > wrote:
> >
> >> You might consider requiring commits be signed while you're at it.
> >>
> >>
> > I suggested this as well, and even if we don't require it, we should
> > STRONGLY encourage it.
> >
> > I've been signing my commits for several years now, it's not even that
> > hard.
> >
> I think for php-src commits we can require it. For doc and other repos we
> can make it optional for now until people are more comfortable with it.
>
> -Rasmus

>
> We can require Signed Commits for the main active branches on GitHub:
> https://docs.github.com/en/github/administering-a-repository/about-protected-branches#require-signed-commits

We can create rules that requires that for all active maintained version of
PHP.

We can set that per repo, or in a organization level.

- Gabriel Caruso

Ayesh - PHP.Watch

5 years ago

I think this is a great step forward.
With commit signatures required, I think the person who merges a PR
now needs to do so locally. [GitHub CLI](https://cli.github.com/)
helps me a lot to locally checkout a PR quickly, and then
rebase/squash with my own signature.

Thanks to Levi Morrison and Nikita for the very quick response.

Deleu

5 years ago

I think you only need to handle merges locally if the pull request author
didn't sign their commits:

> You can always push local commits to the branch if the commits are signed

and verified.

> You can also merge signed and verified commits into the branch using a

pull request on GitHub.

> However, you cannot squash and merge a pull request into the branch on

GitHub unless you are the

> author of the pull request. You can squash and merge pull requests

locally. For more information, see

>  "Checking out pull requests locally

<https://docs.github.com/en/github/collaborating-with-issues-and-pull-requests/checking-out-pull-requests-locally>
."

Source:
https://docs.github.com/en/github/administering-a-repository/about-protected-branches#require-signed-commits

On Mon, Mar 29, 2021, 06:19 Ayesh Karunaratne <ayesh@php.watch> wrote:

Paul Dragoonis

5 years ago

On Mon, 29 Mar 2021, 02:30 Rasmus Lerdorf, <rasmus@lerdorf.com> wrote:

> On Sun, Mar 28, 2021 at 17:15 Sara Golemon <pollita@php.net> wrote:
>
> > On Sun, Mar 28, 2021 at 6:57 PM Paul Crovella <paul.crovella@gmail.com>
> > wrote:
> >
> >> You might consider requiring commits be signed while you're at it.
> >>
> >>
> > I suggested this as well, and even if we don't require it, we should
> > STRONGLY encourage it.
> >
> > I've been signing my commits for several years now, it's not even that
> > hard.
> >
> I think for php-src commits we can require it. For doc and other repos we
> can make it optional for now until people are more comfortable with it.
>

Hey Rasmus,

This is a good compromise.

However, if you leave phpweb repo without signed commits then we're at risk
from XSS or similar attacks still, and the surface area is really big
because literally everyone is accessing the site.

Many thanks,
Paul

Paul Dragoonis

5 years ago

On Mon, 29 Mar 2021, 08:51 Paul Dragoonis, <dragoonis@gmail.com> wrote:

>
>
> On Mon, 29 Mar 2021, 02:30 Rasmus Lerdorf, <rasmus@lerdorf.com> wrote:
>
>> On Sun, Mar 28, 2021 at 17:15 Sara Golemon <pollita@php.net> wrote:
>>
>> > On Sun, Mar 28, 2021 at 6:57 PM Paul Crovella <paul.crovella@gmail.com>
>> > wrote:
>> >
>> >> You might consider requiring commits be signed while you're at it.
>> >>
>> >>
>> > I suggested this as well, and even if we don't require it, we should
>> > STRONGLY encourage it.
>> >
>> > I've been signing my commits for several years now, it's not even that
>> > hard.
>> >
>> I think for php-src commits we can require it. For doc and other repos we
>> can make it optional for now until people are more comfortable with it.
>>
>
> Hey Rasmus,
>
> This is a good compromise.
>
> However, if you leave phpweb repo without signed commits then we're at
> risk from XSS or similar attacks still, and the surface area is really big
> because literally everyone is accessing the site.
>
> Many thanks,
> Paul
>

I also wanted to say; back when I was rebuilding our website a few years
ago, when you pushed to master it would automatically deploy this to the
live site.

If we are compromised and we still automatically roll out to production,
this would make it really easy for someone.

Can someone check how we currently do this, and maybe we should reconsider
auto production deploys, even if its temporary, to be on the safe side.

Bishop Bettini

5 years ago

On Sun, Mar 28, 2021 at 8:16 PM Sara Golemon <pollita@php.net> wrote:

> On Sun, Mar 28, 2021 at 6:57 PM Paul Crovella <paul.crovella@gmail.com>
> wrote:
>
> > You might consider requiring commits be signed while you're at it.
> >
> >
> I suggested this as well, and even if we don't require it, we should
> STRONGLY encourage it.
>
> I've been signing my commits for several years now, it's not even that
> hard.
>

I've documented why we need signing, and how to set it up:

https://wiki.php.net/vcs/commit-signing

Feedback welcomed!

Markus Fischer

5 years ago

Hi Bishop,

On 01.04.21 06:54, Bishop Bettini wrote:

> I've documented why we need signing, and how to set it up:
>
> https://wiki.php.net/vcs/commit-signing
>
> Feedback welcomed!

I'm not even the target audience in terms of php-src access, but rarely
have I seen such a good tutorial approach on this (though have not
tested it).

Just wanted to mention this, really great. It mentions it's based on
internal LifeOmic docs, so kudos to them too I guess :)

- Markus

Rowan Collins

5 years ago

On 01/04/2021 05:54, Bishop Bettini wrote:

> I've documented why we need signing, and how to set it up:
>
> https://wiki.php.net/vcs/commit-signing
>
> Feedback welcomed!

This looks great, and very easy to follow.

One edit I would strongly suggest though:

Remove the "Passphrase:" line from the --generate-key command, so that
gpg will prompt interactively for the passphrase using the same entry as
it will use later when signing. You should never include a password or
passphrase in a command if you can avoid it, as it will be visible on
your screen, and stored in plain text in your shell history.


Some additional tips that might be worth adding:

As an advanced setup suggestion, "gpg --full-generate-key" launches a
wizard with a couple of extra prompts.

If you're on Ubuntu and don't have a new enough git (e.g. 18.04LTS ships
with 2.17.1), there is an official PPA to upgrade it; just run: "sudo
add-apt-repository ppa:git-core/ppa && sudo apt update && sudo apt
install git"

Before pushing to github, you can verify the signature on a commit
locally with "git show --show-signature HEAD", or similarly for a tag by
passing the tag name.


Regards,

--
Rowan Tommins
[IMSoP]

Bishop Bettini

5 years ago

On Thu, Apr 1, 2021 at 9:22 AM Rowan Tommins <rowan.collins@gmail.com>
wrote:

> On 01/04/2021 05:54, Bishop Bettini wrote:
> > I've documented why we need signing, and how to set it up:
> >
> > https://wiki.php.net/vcs/commit-signing
> >
> > Feedback welcomed!
>
>
> This looks great, and very easy to follow.
>
> One edit I would strongly suggest though:
>
> Remove the "Passphrase:" line from the --generate-key command, so that
> gpg will prompt interactively for the passphrase using the same entry as
> it will use later when signing. You should never include a password or
> passphrase in a command if you can avoid it, as it will be visible on
> your screen, and stored in plain text in your shell history.
>
>
> Some additional tips that might be worth adding:
>
> As an advanced setup suggestion, "gpg --full-generate-key" launches a
> wizard with a couple of extra prompts.
>
> If you're on Ubuntu and don't have a new enough git (e.g. 18.04LTS ships
> with 2.17.1), there is an official PPA to upgrade it; just run: "sudo
> add-apt-repository ppa:git-core/ppa && sudo apt update && sudo apt
> install git"
>
> Before pushing to github, you can verify the signature on a commit
> locally with "git show --show-signature HEAD", or similarly for a tag by
> passing the tag name.
>

Excellent suggestions. I've updated the guide with these.

I also added a FAQ.

https://wiki.php.net/vcs/commit-signing

Thank you!

Sara Golemon

5 years ago

On Thu, Apr 1, 2021 at 9:21 AM Bishop Bettini <bishop@php.net> wrote:

> I also added a FAQ.
>
>

I disagree with the position this document takes on immortal keys.  We
should encourage best-practices with the knowledge that some people will
weaken their security with an immortal key, not start from a weak position
and suggest that adhering to best practices is "paranoid".

-Sara

Rowan Collins

5 years ago

On 01/04/2021 15:59, Sara Golemon wrote:

> On Thu, Apr 1, 2021 at 9:21 AM Bishop Bettini <bishop@php.net
> <mailto:bishop@php.net>> wrote:
>
>     I also added a FAQ.
>
>
> I disagree with the position this document takes on immortal keys.  We
> should encourage best-practices with the knowledge that some people
> will weaken their security with an immortal key, not start from a weak
> position and suggest that adhering to best practices is "paranoid".

I've been looking around, and most of what I can find says that expiring
a primary key which you use directly for signing has very little value,
because anyone who has the private key and passphrase can change the
expiry date at any time.  See for example:
https://security.stackexchange.com/q/14718/51961

The main use case seems to be when using sub-keys, where the primary key
(with no expiry) is kept offline, and new sub-keys are generated from it
regularly (e.g. once a year) with an appropriate expiry date.

This is based only on a few hours of searching online, however, so I'd
be happy to see a better explanation of how to use expiry effectively.

Regards,

--
Rowan Tommins
[IMSoP]

Sara Golemon

5 years ago

On Thu, Apr 1, 2021 at 11:19 AM Rowan Tommins <rowan.collins@gmail.com>
wrote:

> On 01/04/2021 15:59, Sara Golemon wrote:
> > On Thu, Apr 1, 2021 at 9:21 AM Bishop Bettini <bishop@php.net
> > <mailto:bishop@php.net>> wrote:
> >
> >     I also added a FAQ.
> >
> >
> > I disagree with the position this document takes on immortal keys.  We
> > should encourage best-practices with the knowledge that some people
> > will weaken their security with an immortal key, not start from a weak
> > position and suggest that adhering to best practices is "paranoid".
>
>
> I've been looking around, and most of what I can find says that expiring
> a primary key which you use directly for signing has very little value,
> because anyone who has the private key and passphrase can change the
> expiry date at any time.  See for example:
> https://security.stackexchange.com/q/14718/51961
>
> The main use case seems to be when using sub-keys, where the primary key
> (with no expiry) is kept offline, and new sub-keys are generated from it
> regularly (e.g. once a year) with an appropriate expiry date.
>
> This is based only on a few hours of searching online, however, so I'd
> be happy to see a better explanation of how to use expiry effectively.
>
>

Yeah, I just got told the same offline.  That's.... depressing.  Not
surprising when one thinks about it more, but still depressing.

-Sara

Bishop Bettini

5 years ago

On Thu, Apr 1, 2021 at 12:24 PM Sara Golemon <pollita@php.net> wrote:

> On Thu, Apr 1, 2021 at 11:19 AM Rowan Tommins <rowan.collins@gmail.com>
> wrote:
>
> > On 01/04/2021 15:59, Sara Golemon wrote:
> > > On Thu, Apr 1, 2021 at 9:21 AM Bishop Bettini <bishop@php.net
> > > <mailto:bishop@php.net>> wrote:
> > >
> > >     I also added a FAQ.
> > >
> > >
> > > I disagree with the position this document takes on immortal keys.  We
> > > should encourage best-practices with the knowledge that some people
> > > will weaken their security with an immortal key, not start from a weak
> > > position and suggest that adhering to best practices is "paranoid".
> >
> >
> > I've been looking around, and most of what I can find says that expiring
> > a primary key which you use directly for signing has very little value,
> > because anyone who has the private key and passphrase can change the
> > expiry date at any time.  See for example:
> > https://security.stackexchange.com/q/14718/51961
> >
> > The main use case seems to be when using sub-keys, where the primary key
> > (with no expiry) is kept offline, and new sub-keys are generated from it
> > regularly (e.g. once a year) with an appropriate expiry date.
> >
> > This is based only on a few hours of searching online, however, so I'd
> > be happy to see a better explanation of how to use expiry effectively.
> >
> >
> Yeah, I just got told the same offline.  That's.... depressing.  Not
> surprising when one thinks about it more, but still depressing.
>

Appreciate the feedback, Sara and Rowan!

I think there was still opportunity to improve that section, so I adjusted
the language to be less accusatory and highlight the essential limitations.

Jakub Zelenka

5 years ago

On Thu, Apr 1, 2021 at 3:21 PM Bishop Bettini <bishop@php.net> wrote:

> On Thu, Apr 1, 2021 at 9:22 AM Rowan Tommins <rowan.collins@gmail.com>
> wrote:
>
> > On 01/04/2021 05:54, Bishop Bettini wrote:
> > > I've documented why we need signing, and how to set it up:
> > >
> > > https://wiki.php.net/vcs/commit-signing
> > >
> > > Feedback welcomed!
> >
> >
> > This looks great, and very easy to follow.
> >
> > One edit I would strongly suggest though:
> >
> > Remove the "Passphrase:" line from the --generate-key command, so that
> > gpg will prompt interactively for the passphrase using the same entry as
> > it will use later when signing. You should never include a password or
> > passphrase in a command if you can avoid it, as it will be visible on
> > your screen, and stored in plain text in your shell history.
> >
> >
> > Some additional tips that might be worth adding:
> >
> > As an advanced setup suggestion, "gpg --full-generate-key" launches a
> > wizard with a couple of extra prompts.
> >
> > If you're on Ubuntu and don't have a new enough git (e.g. 18.04LTS ships
> > with 2.17.1), there is an official PPA to upgrade it; just run: "sudo
> > add-apt-repository ppa:git-core/ppa && sudo apt update && sudo apt
> > install git"
> >
> > Before pushing to github, you can verify the signature on a commit
> > locally with "git show --show-signature HEAD", or similarly for a tag by
> > passing the tag name.
> >
>
> Excellent suggestions. I've updated the guide with these.
>
> I also added a FAQ.
>
> https://wiki.php.net/vcs/commit-signing
>
>

Nice! It would be great if we start enforcing that. I finally set it up
too. I should have done it long time ago considering that I have been
maintaining gnupg extension for some time. :)

I think it's better to always use subkey for signing commits so that might
be good to add to the tutorial. Also RSA is getting a bit heavy with big
keys so it might be also good to suggest using ECC (e.g. EdDSA) which is
faster and possibly more secure. Although it's still in expert settings but
it works fine and a good tutorial can be found here
https://dev.to/benjaminblack/signing-git-commits-with-modern-encryption-1koh
.

Regards

Jakub

Kalle Sommer Nielsen

5 years ago

Hi Bishop

Den tor. 1. apr. 2021 kl. 07.55 skrev Bishop Bettini <bishop@php.net>:

> I've documented why we need signing, and how to set it up:
>
> https://wiki.php.net/vcs/commit-signing
>
> Feedback welcomed!

Great guide, any chance to add some Windows information here?

--
regards,

Kalle Sommer Nielsen
kalle@php.net

Christoph Becker

5 years ago

On 01.04.2021 at 17:09, Kalle Sommer Nielsen wrote:

> Den tor. 1. apr. 2021 kl. 07.55 skrev Bishop Bettini <bishop@php.net>:
>
>> I've documented why we need signing, and how to set it up:
>>
>> https://wiki.php.net/vcs/commit-signing
>>
>> Feedback welcomed!
>
> Great guide,

Yes, thanks for that, Bishop!

> any chance to add some Windows information here?

For step 1 - 3, that would basically be to download and install
<https://www.gpg4win.org/> and to execute Kleopatra; the GUI should be
pretty much self explaining.

Step 4 (copying to clipboard) might not be supported, but the default
export should work fine (could copy to clipboard from the exported file
then).  Make sure that you don't export the private key here.

Steps 5 - 7 should be identical.

Note that Kleopatra must be running, whenever you want to sign a commit.

--
Christoph M. Becker

Bishop Bettini

5 years ago

On Thu, Apr 1, 2021 at 11:32 AM Christoph M. Becker <cmbecker69@gmx.de>
wrote:

> On 01.04.2021 at 17:09, Kalle Sommer Nielsen wrote:
>
> > Den tor. 1. apr. 2021 kl. 07.55 skrev Bishop Bettini <bishop@php.net>:
> >
> >> I've documented why we need signing, and how to set it up:
> >>
> >> https://wiki.php.net/vcs/commit-signing
> >>
> >> Feedback welcomed!
> >
> > Great guide,
>
> Yes, thanks for that, Bishop!
>
> > any chance to add some Windows information here?
>
> For step 1 - 3, that would basically be to download and install
> <https://www.gpg4win.org/> and to execute Kleopatra; the GUI should be
> pretty much self explaining.
>
> Step 4 (copying to clipboard) might not be supported, but the default
> export should work fine (could copy to clipboard from the exported file
> then).  Make sure that you don't export the private key here.
>
> Steps 5 - 7 should be identical.
>
> Note that Kleopatra must be running, whenever you want to sign a commit.
>

Thanks Christoph! Excellent information.

I will try this locally and update the guide.

Casper Langemeijer

5 years ago

On 01-04-2021 06:54, Bishop Bettini wrote:

> I've documented why we need signing, and how to set it up:
>
> https://wiki.php.net/vcs/commit-signing
>
> Feedback welcomed!

In "Step 5 of 7: Configure git to use that key ID" you set
`git config --global --replace user.signingkey "${GPG_KEYID}"`

I found that git falls back to sign with a key matching the `user.email`
when `commit.gpgsign` is true. So because I have set user.email for my
php related git repo to langemeijer@php.net setting the
`user.signingkey` seems to be unnecessary, and it will pick the right key.

This won't work when user.email is different from the identity in
user.signingkey, but that situation should be avoided.

Kalle Sommer Nielsen

5 years ago

Den man. 29. mar. 2021 kl. 01.52 skrev Nikita Popov <nikita.ppv@gmail.com>:

> While previously write access to repositories was handled through our home-grown karma system, you will now need to be part of the php organization on GitHub. If you are not part of the organization yet, or don't have access to a repository you should have access to, contact me at nikic@php.net with your php.net and GitHub account names, as well as the permissions you're currently missing. Membership in the organization requires 2FA to be enabled.

How will this work for SVNROOT karma holders to grant karma to others
going forward?

--
regards,

Kalle Sommer Nielsen
kalle@php.net

Max Semenik

5 years ago

On Mon, Mar 29, 2021 at 1:53 AM Nikita Popov <nikita.ppv@gmail.com> wrote:

> changes should be pushed directly to GitHub rather than to git.php.net.

Would it also make sense if direct pushes (bypassing the pull requests
system) were disallowed completely? I can see multiple problems with direct
pushes:

1) Someone trying to sneak in malicious code, like in the current incident
(rarest but most damaging problem)
2) One dev pushes something, another dev disagrees, while the discussion
continues, the potentially problematic commit stays on. Pre-merge reviews
are much more natural for discussions.
3) Direct push bypasses CI to break tests, PRs can't be merged until the
cause is identified and resolved (seems to be happening quite often)

In my roughly one year around, I can now recall instances of each of these
problems in php-src.

--
Best regards,
Max Semenik

Girgias

5 years ago

On Mon, 29 Mar 2021 at 12:50, Max Semenik <maxsem.wiki@gmail.com> wrote:

> On Mon, Mar 29, 2021 at 1:53 AM Nikita Popov <nikita.ppv@gmail.com> wrote:
>
> > changes should be pushed directly to GitHub rather than to git.php.net.
>
>
> Would it also make sense if direct pushes (bypassing the pull requests
> system) were disallowed completely? I can see multiple problems with direct
> pushes:
>
> 1) Someone trying to sneak in malicious code, like in the current incident
> (rarest but most damaging problem)
> 2) One dev pushes something, another dev disagrees, while the discussion
> continues, the potentially problematic commit stays on. Pre-merge reviews
> are much more natural for discussions.
> 3) Direct push bypasses CI to break tests, PRs can't be merged until the
> cause is identified and resolved (seems to be happening quite often)
>
> In my roughly one year around, I can now recall instances of each of these
> problems in php-src.
>
> --
> Best regards,
> Max Semenik
>

Although I agree that as a general rule many things should go through a PR,
I think making it mandatory is going overboard, there is no need for a PR
when
one does a simple type fix in a document or comment, or trivial
refactorings.
I also don't know how this would interact with bug fixes needing to be
merged
upwards or cherry picks from the security repo which most of us have (and
don't deserve) access to.

Moreover, we have some flaky tests so CI can break for no reason and the
CI matrix for master has a more complex nightly pipeline which does not run
in a PR and thus make certain issues only apparent after the merge.

IMHO making it required that direct pushes need to be signed commits is
a better way forward.

Best regards,

George P. Banyard

Stas Malyshev

5 years ago

Hi!

On 3/29/21 4:49 AM, Max Semenik wrote:

> On Mon, Mar 29, 2021 at 1:53 AM Nikita Popov <nikita.ppv@gmail.com> wrote:
>
>> changes should be pushed directly to GitHub rather than to git.php.net.
>
>
> Would it also make sense if direct pushes (bypassing the pull requests
> system) were disallowed completely? I can see multiple problems with direct
> pushes:

This is possible. In fact, there are Git bots that make it easier (e.g.
prow: https://github.com/kubernetes/test-infra/tree/master/prow) - I am
using such system over Github at my $DAYJOB and it's generally working
well. It even has its own built-in karma-like system. However, it has
some downsides, as the experience shows:

1. Quick management patches, typofixes, release management patches, etc.
become more high friction processes.
2. Setup and configuration of such system involves some time investment
by some knowledgeable people, and it has certain learning curve (though
once it is set up, it's pretty easy to use).
3. Somebody knowledgeable needs to maintain it, as periodically bots can
get stuck and need a gentle kick to continue.
4. CI needs to be very stable and clean for having CI pass as gateway to
merge, otherwise a flaky test can block all work in the repo for days.
5. Managing multiple active branches can be a pain.

None of these are critical, and we could start small and build it
incrementally, of course.

--
Stas Malyshev
smalyshev@gmail.com

Max Semenik

5 years ago

On Tue, Mar 30, 2021 at 3:29 AM Stanislav Malyshev <smalyshev@gmail.com>
wrote:

> Hi!
>
> On 3/29/21 4:49 AM, Max Semenik wrote:
> > Would it also make sense if direct pushes (bypassing the pull requests
> > system) were disallowed completely? I can see multiple problems with
> direct
> > pushes:
>
> This is possible. In fact, there are Git bots that make it easier (e.g.
> prow: https://github.com/kubernetes/test-infra/tree/master/prow) - I am
> using such system over Github at my $DAYJOB and it's generally working
> well. It even has its own built-in karma-like system. However, it has
> some downsides, as the experience shows:
>
> 1. Quick management patches, typofixes, release management patches, etc.
> become more high friction processes.
> 2. Setup and configuration of such system involves some time investment
> by some knowledgeable people, and it has certain learning curve (though
> once it is set up, it's pretty easy to use).
> 3. Somebody knowledgeable needs to maintain it, as periodically bots can
> get stuck and need a gentle kick to continue.
> 4. CI needs to be very stable and clean for having CI pass as gateway to
> merge, otherwise a flaky test can block all work in the repo for days.
> 5. Managing multiple active branches can be a pain.
>
> None of these are critical, and we could start small and build it
> incrementally, of course.
>

We don't even have to use bots - GitHub allows you to require passing CI
and/or approving reviews to merge.

--
Best regards,
Max Semenik

Derick Rethans

5 years ago

On 30 March 2021 10:43:41 BST, Max Semenik <maxsem.wiki@gmail.com> wrote:

>On Tue, Mar 30, 2021 at 3:29 AM Stanislav Malyshev
><smalyshev@gmail.com>
>wrote:
>
>> Hi!
>>
>> On 3/29/21 4:49 AM, Max Semenik wrote:
>> > Would it also make sense if direct pushes (bypassing the pull
>requests
>> > system) were disallowed completely? I can see multiple problems
>with
>> direct
>> > pushes:
>>
>> This is possible. In fact, there are Git bots that make it easier
>(e.g.
>> prow: https://github.com/kubernetes/test-infra/tree/master/prow) - I
>am
>> using such system over Github at my $DAYJOB and it's generally
>working
>> well. It even has its own built-in karma-like system. However, it has
>> some downsides, as the experience shows:
>>
>> 1. Quick management patches, typofixes, release management patches,
>etc.
>> become more high friction processes.
>> 2. Setup and configuration of such system involves some time
>investment
>> by some knowledgeable people, and it has certain learning curve
>(though
>> once it is set up, it's pretty easy to use).
>> 3. Somebody knowledgeable needs to maintain it, as periodically bots
>can
>> get stuck and need a gentle kick to continue.
>> 4. CI needs to be very stable and clean for having CI pass as gateway
>to
>> merge, otherwise a flaky test can block all work in the repo for
>days.
>> 5. Managing multiple active branches can be a pain.
>>
>> None of these are critical, and we could start small and build it
>> incrementally, of course.
>>
>
>We don't even have to use bots - GitHub allows you to require passing
>CI
>and/or approving reviews to merge.

How well does that work for merging up fixes from an older bug fix branch up through PHP 7.4, PHP 8.0, and then into master?

Or for things like new timezone definitions, which is now automated, and would then require a pointless PR?

It sounds like an annoying hurdle.

cheers,
Derick

Mike Schinkel

5 years ago

> On Mar 30, 2021, at 5:51 AM, Derick Rethans <derick@php.net> wrote:
>
> On 30 March 2021 10:43:41 BST, Max Semenik <maxsem.wiki@gmail.com> wrote:
>> On Tue, Mar 30, 2021 at 3:29 AM Stanislav Malyshev
>> <smalyshev@gmail.com>
>> wrote:
>>
>>> Hi!
>>>
>>> On 3/29/21 4:49 AM, Max Semenik wrote:
>>>> Would it also make sense if direct pushes (bypassing the pull
>> requests
>>>> system) were disallowed completely? I can see multiple problems
>> with
>>> direct
>>>> pushes:
>>>
>>> This is possible. In fact, there are Git bots that make it easier
>> (e.g.
>>> prow: https://github.com/kubernetes/test-infra/tree/master/prow) - I
>> am
>>> using such system over Github at my $DAYJOB and it's generally
>> working
>>> well. It even has its own built-in karma-like system. However, it has
>>> some downsides, as the experience shows:
>>>
>>> 1. Quick management patches, typofixes, release management patches,
>> etc.
>>> become more high friction processes.
>>> 2. Setup and configuration of such system involves some time
>> investment
>>> by some knowledgeable people, and it has certain learning curve
>> (though
>>> once it is set up, it's pretty easy to use).
>>> 3. Somebody knowledgeable needs to maintain it, as periodically bots
>> can
>>> get stuck and need a gentle kick to continue.
>>> 4. CI needs to be very stable and clean for having CI pass as gateway
>> to
>>> merge, otherwise a flaky test can block all work in the repo for
>> days.
>>> 5. Managing multiple active branches can be a pain.
>>>
>>> None of these are critical, and we could start small and build it
>>> incrementally, of course.
>>>
>>
>> We don't even have to use bots - GitHub allows you to require passing
>> CI
>> and/or approving reviews to merge.
>
> How well does that work for merging up fixes from an older bug fix branch up through PHP 7.4, PHP 8.0, and then into master?
>
> Or for things like new timezone definitions, which is now automated, and would then require a pointless PR?

Accepting some PRs can be automated.  Repos can be protects on Github via per-branch rules[1] where permissions and requirements can be assigned.

PRs are actually a foundational component of GitOps[2] which is an emerging best practice for managing infrastructure. It was initially for Kubernetes deployments but has become recognized as being beneficial[3] for automating software CI/CD[4] and other workflows.

I have actually been developing GitOps pipelines for my current client since February.

If you are unfamiliar with GitOps you might consider reading about from the links below?

----------------

Also, I just googled and found these, so I cannot endorse them having never used them, but they apparently show what can be done with GitHub PR and Merge Automation:

— Bulldozer is an auto-merge bot:
https://github.com/palantir/bulldozer <https://github.com/palantir/bulldozer>

— BorsNG is a merge bot for GitHub pull requests
https://bors.tech/

— Policy-bot evidentially allows merge rules to be declaratively specified, a real plus because then everyone who needs to know could see how the approval process works and who has access, at least by group:
https://github.com/palantir/policy-bot <https://github.com/palantir/policy-bot>

— Heres a simply article showing how to automate with Javascript and Probot:
https://freeletics.engineering/2019/09/09/automating-your-github-routine.html <https://freeletics.engineering/2019/09/09/automating-your-github-routine.html>

Hope this helps.

-Mike

[1] https://docs.github.com/en/github/administering-a-repository/managing-a-branch-protection-rule
[2] https://www.weave.works/technologies/gitops/ <https://www.weave.works/technologies/gitops/>
[3] https://www.gitops.tech/ <https://www.gitops.tech/>
[4] https://harness.io/blog/devops/what-is-gitops/ <https://harness.io/blog/devops/what-is-gitops/>

Jakub Zelenka

5 years ago

On Tue, Mar 30, 2021 at 12:47 PM Mike Schinkel <mike@newclarity.net> wrote:

>
>
> > On Mar 30, 2021, at 5:51 AM, Derick Rethans <derick@php.net> wrote:
> >
> > On 30 March 2021 10:43:41 BST, Max Semenik <maxsem.wiki@gmail.com>
> wrote:
> >> On Tue, Mar 30, 2021 at 3:29 AM Stanislav Malyshev
> >> <smalyshev@gmail.com>
> >> wrote:
> >>
> >>> Hi!
> >>>
> >>> On 3/29/21 4:49 AM, Max Semenik wrote:
> >>>> Would it also make sense if direct pushes (bypassing the pull
> >> requests
> >>>> system) were disallowed completely? I can see multiple problems
> >> with
> >>> direct
> >>>> pushes:
> >>>
> >>> This is possible. In fact, there are Git bots that make it easier
> >> (e.g.
> >>> prow: https://github.com/kubernetes/test-infra/tree/master/prow) - I
> >> am
> >>> using such system over Github at my $DAYJOB and it's generally
> >> working
> >>> well. It even has its own built-in karma-like system. However, it has
> >>> some downsides, as the experience shows:
> >>>
> >>> 1. Quick management patches, typofixes, release management patches,
> >> etc.
> >>> become more high friction processes.
> >>> 2. Setup and configuration of such system involves some time
> >> investment
> >>> by some knowledgeable people, and it has certain learning curve
> >> (though
> >>> once it is set up, it's pretty easy to use).
> >>> 3. Somebody knowledgeable needs to maintain it, as periodically bots
> >> can
> >>> get stuck and need a gentle kick to continue.
> >>> 4. CI needs to be very stable and clean for having CI pass as gateway
> >> to
> >>> merge, otherwise a flaky test can block all work in the repo for
> >> days.
> >>> 5. Managing multiple active branches can be a pain.
> >>>
> >>> None of these are critical, and we could start small and build it
> >>> incrementally, of course.
> >>>
> >>
> >> We don't even have to use bots - GitHub allows you to require passing
> >> CI
> >> and/or approving reviews to merge.
> >
> > How well does that work for merging up fixes from an older bug fix
> branch up through PHP 7.4, PHP 8.0, and then into master?
> >
> > Or for things like new timezone definitions, which is now automated, and
> would then require a pointless PR?
>
> Accepting some PRs can be automated.  Repos can be protects on Github via
> per-branch rules[1] where permissions and requirements can be assigned.
>
> PRs are actually a foundational component of GitOps[2] which is an
> emerging best practice for managing infrastructure. It was initially for
> Kubernetes deployments but has become recognized as being beneficial[3] for
> automating software CI/CD[4] and other workflows.
>
>

The problem is that this is not gonna easily work with the current PHP
workflow and merging changes up. For example currently if you merge PR to
PHP-7.4 and then you merge PHP-7.4 to PHP-8.0, you get most of the time
conflict in NEWS that needs to be manually resolved. We would have to make
the bot that is able to resolve those conflicts or come up with a different
way how to handle NEWS. However sometimes there are code conflicts as well
which the bot cannot resolve. I think the only solution would be to stop
merging the branches up, do PR's always against master and then just
cherry-pick the commit to lower branches (it could be potentially done by
bot automatically in some / most cases) but again that would require some
changes in the NEWS handling and possibly other things.

Cheers

Jakub

Jakub Zelenka

5 years ago

On Tue, Mar 30, 2021 at 1:21 PM Jakub Zelenka <bukka@php.net> wrote:

>
>
> On Tue, Mar 30, 2021 at 12:47 PM Mike Schinkel <mike@newclarity.net>
> wrote:
>
>>
>>
>> > On Mar 30, 2021, at 5:51 AM, Derick Rethans <derick@php.net> wrote:
>> >
>> > On 30 March 2021 10:43:41 BST, Max Semenik <maxsem.wiki@gmail.com>
>> wrote:
>> >> On Tue, Mar 30, 2021 at 3:29 AM Stanislav Malyshev
>> >> <smalyshev@gmail.com>
>> >> wrote:
>> >>
>> >>> Hi!
>> >>>
>> >>> On 3/29/21 4:49 AM, Max Semenik wrote:
>> >>>> Would it also make sense if direct pushes (bypassing the pull
>> >> requests
>> >>>> system) were disallowed completely? I can see multiple problems
>> >> with
>> >>> direct
>> >>>> pushes:
>> >>>
>> >>> This is possible. In fact, there are Git bots that make it easier
>> >> (e.g.
>> >>> prow: https://github.com/kubernetes/test-infra/tree/master/prow) - I
>> >> am
>> >>> using such system over Github at my $DAYJOB and it's generally
>> >> working
>> >>> well. It even has its own built-in karma-like system. However, it has
>> >>> some downsides, as the experience shows:
>> >>>
>> >>> 1. Quick management patches, typofixes, release management patches,
>> >> etc.
>> >>> become more high friction processes.
>> >>> 2. Setup and configuration of such system involves some time
>> >> investment
>> >>> by some knowledgeable people, and it has certain learning curve
>> >> (though
>> >>> once it is set up, it's pretty easy to use).
>> >>> 3. Somebody knowledgeable needs to maintain it, as periodically bots
>> >> can
>> >>> get stuck and need a gentle kick to continue.
>> >>> 4. CI needs to be very stable and clean for having CI pass as gateway
>> >> to
>> >>> merge, otherwise a flaky test can block all work in the repo for
>> >> days.
>> >>> 5. Managing multiple active branches can be a pain.
>> >>>
>> >>> None of these are critical, and we could start small and build it
>> >>> incrementally, of course.
>> >>>
>> >>
>> >> We don't even have to use bots - GitHub allows you to require passing
>> >> CI
>> >> and/or approving reviews to merge.
>> >
>> > How well does that work for merging up fixes from an older bug fix
>> branch up through PHP 7.4, PHP 8.0, and then into master?
>> >
>> > Or for things like new timezone definitions, which is now automated,
>> and would then require a pointless PR?
>>
>> Accepting some PRs can be automated.  Repos can be protects on Github via
>> per-branch rules[1] where permissions and requirements can be assigned.
>>
>> PRs are actually a foundational component of GitOps[2] which is an
>> emerging best practice for managing infrastructure. It was initially for
>> Kubernetes deployments but has become recognized as being beneficial[3] for
>> automating software CI/CD[4] and other workflows.
>>
>>
> The problem is that this is not gonna easily work with the current PHP
> workflow and merging changes up. For example currently if you merge PR to
> PHP-7.4 and then you merge PHP-7.4 to PHP-8.0, you get most of the time
> conflict in NEWS that needs to be manually resolved. We would have to make
> the bot that is able to resolve those conflicts or come up with a different
> way how to handle NEWS. However sometimes there are code conflicts as well
> which the bot cannot resolve. I think the only solution would be to stop
> merging the branches up, do PR's always against master and then just
> cherry-pick the commit to lower branches (it could be potentially done by
> bot automatically in some / most cases) but again that would require some
> changes in the NEWS handling and possibly other things.
>
>

Actually if we needed to do cherry-pick that conflicts, we would still need
PR's for lower branches like having multiple PR's for the same thing that
requires conflicting implementation (cannot be cherry-picked by bot).

Think having everything going through the PR's has certainly benefit as we
could require reviews for each PR which would be certainly good for
security. A similar worklfow is done in OpenSSL for example except it's a
bit different flow there because there are not too many fixes and changes
in lower branches.

Cheers

Jakub

Mike Schinkel

5 years ago

> On Mar 30, 2021, at 8:29 AM, Jakub Zelenka <bukka@php.net> wrote:
>
>
>
> On Tue, Mar 30, 2021 at 1:21 PM Jakub Zelenka <bukka@php.net <mailto:bukka@php.net>> wrote:
>
>
> On Tue, Mar 30, 2021 at 12:47 PM Mike Schinkel <mike@newclarity.net <mailto:mike@newclarity.net>> wrote:
>
>
> > On Mar 30, 2021, at 5:51 AM, Derick Rethans <derick@php.net <mailto:derick@php.net>> wrote:
> >
> > On 30 March 2021 10:43:41 BST, Max Semenik <maxsem.wiki@gmail.com <mailto:maxsem.wiki@gmail.com>> wrote:
> >> On Tue, Mar 30, 2021 at 3:29 AM Stanislav Malyshev
> >> <smalyshev@gmail.com <mailto:smalyshev@gmail.com>>
> >> wrote:
> >>
> >>> Hi!
> >>>
> >>> On 3/29/21 4:49 AM, Max Semenik wrote:
> >>>> Would it also make sense if direct pushes (bypassing the pull
> >> requests
> >>>> system) were disallowed completely? I can see multiple problems
> >> with
> >>> direct
> >>>> pushes:
> >>>
> >>> This is possible. In fact, there are Git bots that make it easier
> >> (e.g.
> >>> prow: https://github.com/kubernetes/test-infra/tree/master/prow <https://github.com/kubernetes/test-infra/tree/master/prow>) - I
> >> am
> >>> using such system over Github at my $DAYJOB and it's generally
> >> working
> >>> well. It even has its own built-in karma-like system. However, it has
> >>> some downsides, as the experience shows:
> >>>
> >>> 1. Quick management patches, typofixes, release management patches,
> >> etc.
> >>> become more high friction processes.
> >>> 2. Setup and configuration of such system involves some time
> >> investment
> >>> by some knowledgeable people, and it has certain learning curve
> >> (though
> >>> once it is set up, it's pretty easy to use).
> >>> 3. Somebody knowledgeable needs to maintain it, as periodically bots
> >> can
> >>> get stuck and need a gentle kick to continue.
> >>> 4. CI needs to be very stable and clean for having CI pass as gateway
> >> to
> >>> merge, otherwise a flaky test can block all work in the repo for
> >> days.
> >>> 5. Managing multiple active branches can be a pain.
> >>>
> >>> None of these are critical, and we could start small and build it
> >>> incrementally, of course.
> >>>
> >>
> >> We don't even have to use bots - GitHub allows you to require passing
> >> CI
> >> and/or approving reviews to merge.
> >
> > How well does that work for merging up fixes from an older bug fix branch up through PHP 7.4, PHP 8.0, and then into master?
> >
> > Or for things like new timezone definitions, which is now automated, and would then require a pointless PR?
>
> Accepting some PRs can be automated.  Repos can be protects on Github via per-branch rules[1] where permissions and requirements can be assigned.
>
> PRs are actually a foundational component of GitOps[2] which is an emerging best practice for managing infrastructure. It was initially for Kubernetes deployments but has become recognized as being beneficial[3] for automating software CI/CD[4] and other workflows.
>
>
> The problem is that this is not gonna easily work with the current PHP workflow and merging changes up. For example currently if you merge PR to PHP-7.4 and then you merge PHP-7.4 to PHP-8.0, you get most of the time conflict in NEWS that needs to be manually resolved. We would have to make the bot that is able to resolve those conflicts or come up with a different way how to handle NEWS. However sometimes there are code conflicts as well which the bot cannot resolve. I think the only solution would be to stop merging the branches up, do PR's always against master and then just cherry-pick the commit to lower branches (it could be potentially done by bot automatically in some / most cases) but again that would require some changes in the NEWS handling and possibly other things.
>
>
> Actually if we needed to do cherry-pick that conflicts, we would still need PR's for lower branches like having multiple PR's for the same thing that requires conflicting implementation (cannot be cherry-picked by bot).
>
> Think having everything going through the PR's has certainly benefit as we could require reviews for each PR which would be certainly good for security. A similar worklfow is done in OpenSSL for example except it's a bit different flow there because there are not too many fixes and changes in lower branches.
>

When you speak of NEWS, do you mean this?

https://github.com/php/web-news <https://github.com/php/web-news>

If yes, then no problem.  Not only can different branches have different rules, different repos definitely can.  So worse case NEWS could be manual, but php-src could require PRs, if that is the best that could be done initially.

And as for special manual cases, I would be surprised is there wouldn't be a way that is not terribly hard to deal with special cases. After all, those working in GitOps are often dealing with enterprise use-cases and lots of legacy code and tons of special cases.

So it is really more of a divide-and-conquer approach; automate what can be and manually handle what cannot be automated. At least until you are able to upgrade the workflow, if ever.

-Mike

P.S. If nobody else has sufficient expertise here, maybe this would be a way I could finally start contributing something tangible to PHP since I have yet to learn how to add to or update the source code in C for PHP?

Sara Golemon

5 years ago

On Tue, Mar 30, 2021 at 8:34 AM Mike Schinkel <mike@newclarity.net> wrote:

> When you speak of NEWS, do you mean this?
> https://github.com/php/web-news <https://github.com/php/web-news>
>

Negative. NEWS in all caps invariably refers to
https://github.com/php/php-src/blob/master/NEWS which is a guaranteed merge
conflict between branches.

There are a few other places that mostly impact RMs like main/php_version.h
and configure.ac

-Sara

Girgias

5 years ago

On Tue, 30 Mar 2021 at 14:34, Mike Schinkel <mike@newclarity.net> wrote:

>
>
> > On Mar 30, 2021, at 8:29 AM, Jakub Zelenka <bukka@php.net> wrote:
> >
> >
> >
> > On Tue, Mar 30, 2021 at 1:21 PM Jakub Zelenka <bukka@php.net <mailto:
> bukka@php.net>> wrote:
> >
> >
> > On Tue, Mar 30, 2021 at 12:47 PM Mike Schinkel <mike@newclarity.net
> <mailto:mike@newclarity.net>> wrote:
> >
> >
> > > On Mar 30, 2021, at 5:51 AM, Derick Rethans <derick@php.net <mailto:
> derick@php.net>> wrote:
> > >
> > > On 30 March 2021 10:43:41 BST, Max Semenik <maxsem.wiki@gmail.com
> <mailto:maxsem.wiki@gmail.com>> wrote:
> > >> On Tue, Mar 30, 2021 at 3:29 AM Stanislav Malyshev
> > >> <smalyshev@gmail.com <mailto:smalyshev@gmail.com>>
> > >> wrote:
> > >>
> > >>> Hi!
> > >>>
> > >>> On 3/29/21 4:49 AM, Max Semenik wrote:
> > >>>> Would it also make sense if direct pushes (bypassing the pull
> > >> requests
> > >>>> system) were disallowed completely? I can see multiple problems
> > >> with
> > >>> direct
> > >>>> pushes:
> > >>>
> > >>> This is possible. In fact, there are Git bots that make it easier
> > >> (e.g.
> > >>> prow: https://github.com/kubernetes/test-infra/tree/master/prow <
> https://github.com/kubernetes/test-infra/tree/master/prow>) - I
> > >> am
> > >>> using such system over Github at my $DAYJOB and it's generally
> > >> working
> > >>> well. It even has its own built-in karma-like system. However, it has
> > >>> some downsides, as the experience shows:
> > >>>
> > >>> 1. Quick management patches, typofixes, release management patches,
> > >> etc.
> > >>> become more high friction processes.
> > >>> 2. Setup and configuration of such system involves some time
> > >> investment
> > >>> by some knowledgeable people, and it has certain learning curve
> > >> (though
> > >>> once it is set up, it's pretty easy to use).
> > >>> 3. Somebody knowledgeable needs to maintain it, as periodically bots
> > >> can
> > >>> get stuck and need a gentle kick to continue.
> > >>> 4. CI needs to be very stable and clean for having CI pass as gateway
> > >> to
> > >>> merge, otherwise a flaky test can block all work in the repo for
> > >> days.
> > >>> 5. Managing multiple active branches can be a pain.
> > >>>
> > >>> None of these are critical, and we could start small and build it
> > >>> incrementally, of course.
> > >>>
> > >>
> > >> We don't even have to use bots - GitHub allows you to require passing
> > >> CI
> > >> and/or approving reviews to merge.
> > >
> > > How well does that work for merging up fixes from an older bug fix
> branch up through PHP 7.4, PHP 8.0, and then into master?
> > >
> > > Or for things like new timezone definitions, which is now automated,
> and would then require a pointless PR?
> >
> > Accepting some PRs can be automated.  Repos can be protects on Github
> via per-branch rules[1] where permissions and requirements can be assigned.
> >
> > PRs are actually a foundational component of GitOps[2] which is an
> emerging best practice for managing infrastructure. It was initially for
> Kubernetes deployments but has become recognized as being beneficial[3] for
> automating software CI/CD[4] and other workflows.
> >
> >
> > The problem is that this is not gonna easily work with the current PHP
> workflow and merging changes up. For example currently if you merge PR to
> PHP-7.4 and then you merge PHP-7.4 to PHP-8.0, you get most of the time
> conflict in NEWS that needs to be manually resolved. We would have to make
> the bot that is able to resolve those conflicts or come up with a different
> way how to handle NEWS. However sometimes there are code conflicts as well
> which the bot cannot resolve. I think the only solution would be to stop
> merging the branches up, do PR's always against master and then just
> cherry-pick the commit to lower branches (it could be potentially done by
> bot automatically in some / most cases) but again that would require some
> changes in the NEWS handling and possibly other things.
> >
> >
> > Actually if we needed to do cherry-pick that conflicts, we would still
> need PR's for lower branches like having multiple PR's for the same thing
> that requires conflicting implementation (cannot be cherry-picked by bot).
> >
> > Think having everything going through the PR's has certainly benefit as
> we could require reviews for each PR which would be certainly good for
> security. A similar worklfow is done in OpenSSL for example except it's a
> bit different flow there because there are not too many fixes and changes
> in lower branches.
> >
>
> When you speak of NEWS, do you mean this?
>
> https://github.com/php/web-news <https://github.com/php/web-news>
>
> -Mike
>

No he is talking about the NEWS file at the root of every branch which is
the changelog for what bugfixes have been applied to the branch.

Regards,

George P. Banyard

Benjamin Morel

5 years ago

>
> Hi everyone,
>
> Yesterday (2021-03-28) two malicious commits were pushed to the php-src
> repo [1] from the names of Rasmus Lerdorf and myself. We don't yet know how
> exactly this happened, but everything points towards a compromise of the
> git.php.net server (rather than a compromise of an individual git
> account).
>

That is scary. Can you disclose the contents of the commits? Are they
specially designed to open a security hole, or to be harmful in another way?

> While investigation is still underway, we have decided that maintaining
> our own git infrastructure is an unnecessary security risk, and that we
> will discontinue the git.php.net server. Instead, the repositories on
> GitHub, which were previously only mirrors, will become canonical. This
> means that changes should be pushed directly to GitHub rather than to
> git.php.net.
>

This change will be welcome anyway!

— Benjamin

Björn Larsson

5 years ago

Den 2021-03-29 kl. 23:10, skrev Benjamin Morel:

>>
>> Hi everyone,
>>
>> Yesterday (2021-03-28) two malicious commits were pushed to the php-src
>> repo [1] from the names of Rasmus Lerdorf and myself. We don't yet know how
>> exactly this happened, but everything points towards a compromise of the
>> git.php.net server (rather than a compromise of an individual git
>> account).
>>
>
> That is scary. Can you disclose the contents of the commits? Are they
> specially designed to open a security hole, or to be harmful in another way?
>

An article from The Hacker News and a tweet from Zerodium about the
incident:
-https://thehackernews.com/2021/03/phps-git-server-hacked-to-insert-secret.html
-https://twitter.com/cBekrar/status/1376469666084757506

r//Björn L

Jan Ehrhardt

5 years ago

Nikita Popov in php.internals (Mon, 29 Mar 2021 00:52:24 +0200):

>We're reviewing the repositories for any corruption beyond the two
>referenced commits.

Will PHP 8.0.4 and 7.4.17 be postponed because of this? They haven't been
released yet. The usual day for tagging always was Tuesday or Wednesday.

--
Jan

Sebastian Bergmann

5 years ago

Am 01.04.2021 um 09:58 schrieb Jan Ehrhardt:

> Will PHP 8.0.4 and 7.4.17 be postponed because of this? They haven't been
> released yet. The usual day for tagging always was Tuesday or Wednesday.

Yes, see https://twitter.com/official_php/status/1377339882645905408

Sara Golemon

5 years ago

On Thu, Apr 1, 2021 at 2:59 AM Sebastian Bergmann <sebastian@php.net> wrote:

> Am 01.04.2021 um 09:58 schrieb Jan Ehrhardt:
> > Will PHP 8.0.4 and 7.4.17 be postponed because of this? They haven't been
> > released yet. The usual day for tagging always was Tuesday or Wednesday.
>
> Yes, see https://twitter.com/official_php/status/1377339882645905408
>
>

Or even the front page of php.net, this was announced on Tuesday when
tagging normally would have taken place.

-Sara

=?utf-8?B?5rGf5rmW5aSn6Jm+5LuB?=

5 years ago

Hi Nikita,

Can I check with you if the hook integrating with bugs.php.net still works? I just committed a bug fix on doc-zh repo, and the bug report has been updated. Thanks.

Regards,
CHU Zhaowei

-----Original Message-----
From: Nikita Popov <nikita.ppv@gmail.com>
Sent: Monday, March 29, 2021 6:52 AM
To: PHP internals <internals@lists.php.net>; PHP Doc Mailing List <phpdoc@lists.php.net>
Subject: [PHP-DEV] Changes to Git commit workflow

Hi everyone,

Yesterday (2021-03-28) two malicious commits were pushed to the php-src repo [1] from the names of Rasmus Lerdorf and myself. We don't yet know how exactly this happened, but everything points towards a compromise of the git.php.net server (rather than a compromise of an individual git account).

While investigation is still underway, we have decided that maintaining our own git infrastructure is an unnecessary security risk, and that we will discontinue the git.php.net server. Instead, the repositories on GitHub, which were previously only mirrors, will become canonical. This means that changes should be pushed directly to GitHub rather than to git.php.net.

While previously write access to repositories was handled through our home-grown karma system, you will now need to be part of the php organization on GitHub. If you are not part of the organization yet, or don't have access to a repository you should have access to, contact me at nikic@php.net with your php.net and GitHub account names, as well as the permissions you're currently missing. Membership in the organization requires 2FA to be enabled.

This change also means that it is now possible to merge pull requests directly from the GitHub web interface.

We're reviewing the repositories for any corruption beyond the two referenced commits. Please contact security@php.net if you notice anything.

Regards,
Nikita

[1]:
https://github.com/php/php-src/commit/c730aa26bd52829a49f2ad284b181b7e82a68d7d
and
https://github.com/php/php-src/commit/2b0f239b211c7544ebc7a4cd2c977a5b7a11ed8a

Nikita Popov

5 years ago

On Thu, Apr 1, 2021 at 10:17 PM CHU Zhaowei <me@jhdxr.com> wrote:

> Hi Nikita,
>
> Can I check with you if the hook integrating with bugs.php.net still
> works? I just committed a bug fix on doc-zh repo, and the bug report has
> been updated. Thanks.
>
> Regards,
> CHU Zhaowei
>

This issue is probably fixed by
https://github.com/php/web-master/commit/d0cac5411f97ec9df5995a632c20da770a77dedb
.

Nikita

-----Original Message-----