Re: [PHP-DEV] Changes to Git commit workflow

[Casper Langemeijer]

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 langemei...@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.


--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php

Re: [PHP-DEV] Changes to Git commit workflow

[Jakub Zelenka]

On Thu, Apr 1, 2021 at 3:21 PM Bishop Bettini  wrote:

> On Thu, Apr 1, 2021 at 9:22 AM Rowan Tommins 
> 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

Re: [PHP-DEV] Changes to Git commit workflow

[Nikita Popov]

On Thu, Apr 1, 2021 at 10:17 PM CHU Zhaowei  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-
> From: Nikita Popov 
> Sent: Monday, March 29, 2021 6:52 AM
> To: PHP internals ; PHP Doc Mailing List <
> php...@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
> ni...@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 secur...@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
>
>
>
>

RE: [PHP-DEV] Changes to Git commit workflow

[CHU Zhaowei]

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  
Sent: Monday, March 29, 2021 6:52 AM
To: PHP internals ; PHP Doc Mailing List 

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 ni...@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 secur...@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

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php

Re: [PHP-DEV] Changes to Git commit workflow

[Bishop Bettini]

On Thu, Apr 1, 2021 at 12:24 PM Sara Golemon  wrote:

> On Thu, Apr 1, 2021 at 11:19 AM Rowan Tommins 
> wrote:
>
> > On 01/04/2021 15:59, Sara Golemon wrote:
> > > On Thu, Apr 1, 2021 at 9:21 AM Bishop Bettini  > > > 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.

Re: [PHP-DEV] Changes to Git commit workflow

[Sara Golemon]

On Thu, Apr 1, 2021 at 11:19 AM Rowan Tommins 
wrote:

> On 01/04/2021 15:59, Sara Golemon wrote:
> > On Thu, Apr 1, 2021 at 9:21 AM Bishop Bettini  > > 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

Re: [PHP-DEV] Changes to Git commit workflow

[Rowan Tommins]

On 01/04/2021 15:59, Sara Golemon wrote:
On Thu, Apr 1, 2021 at 9:21 AM Bishop Bettini > 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]

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php

Re: [PHP-DEV] Changes to Git commit workflow

[Bishop Bettini]

On Thu, Apr 1, 2021 at 11:32 AM Christoph M. Becker 
wrote:

> On 01.04.2021 at 17:09, Kalle Sommer Nielsen wrote:
>
> > Den tor. 1. apr. 2021 kl. 07.55 skrev Bishop Bettini :
> >
> >> 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
>  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.

Re: [PHP-DEV] Changes to Git commit workflow

[Christoph M. Becker]

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

> Den tor. 1. apr. 2021 kl. 07.55 skrev Bishop Bettini :
>
>> 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
 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

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php

Re: [PHP-DEV] Changes to Git commit workflow

[Kalle Sommer Nielsen]

Hi Bishop

Den tor. 1. apr. 2021 kl. 07.55 skrev Bishop Bettini :
> 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
ka...@php.net

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php

Re: [PHP-DEV] Changes to Git commit workflow

[Sara Golemon]

On Thu, Apr 1, 2021 at 9:21 AM Bishop Bettini  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

Re: [PHP-DEV] Changes to Git commit workflow

[Bishop Bettini]

On Thu, Apr 1, 2021 at 9:22 AM Rowan Tommins 
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!

Re: [PHP-DEV] Changes to Git commit workflow

[Rowan Tommins]

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]

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php

Re: [PHP-DEV] Changes to Git commit workflow

[Markus Fischer]

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

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php

Re: [PHP-DEV] Changes to Git commit workflow

[Bishop Bettini]

On Sun, Mar 28, 2021 at 8:16 PM Sara Golemon  wrote:

> On Sun, Mar 28, 2021 at 6:57 PM Paul Crovella 
> 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!

Re: [PHP-DEV] Changes to Git commit workflow

[Sara Golemon]

On Tue, Mar 30, 2021 at 10:13 AM Christoph M. Becker 
wrote:

> > https://github.com/php/php-src/blob/master/NEWS which is a guaranteed
> merge
> > conflict between branches.
>
> Nope, see . :)
>
>
GAME. CHANGED.
This message constitutes your EFT (Entirely Fungible Token) for a beer if I
even meet you in person.

-Sara

Re: [PHP-DEV] Changes to Git commit workflow

[Christoph M. Becker]

On 30.03.2021 at 15:50, Sara Golemon wrote:

> On Tue, Mar 30, 2021 at 8:34 AM Mike Schinkel  wrote:
>
>> When you speak of NEWS, do you mean this?
>> 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.

Nope, see . :)

--
Christoph M. Becker

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php

Re: [PHP-DEV] Changes to Git commit workflow

[Sara Golemon]

On Tue, Mar 30, 2021 at 9:08 AM Mike Schinkel  wrote:

> On Mar 30, 2021, at 9:50 AM, Sara Golemon  wrote:
>
> On Tue, Mar 30, 2021 at 8:34 AM Mike Schinkel  wrote:
> > When you speak of NEWS, do you mean this?
> > 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
>
>
> Is there any chance the team would consider modifying the workflow a bit?
>
>
I thought about this (admittedly briefly) while replying and while I think
fixing NEWS would be... not impossible, I think the configure.ac and
php_version.h conflicts are a little more dicey.
Granted those are only a problem for RMs and we can build an escape hatch
for that.
But then conflicts in the rest of code aren't exactly uncommon, especially
in areas under heavy development like ext/opcache.
Basically, this is a hard problem and it's going to be a hard problem.

One alternative might be to switch to a pick model rather than a merge
model, but that just kicks the problem out a layer, it doesn't solve it.
What it does do is add a new opportunity for fixes to get missed.  -1 from
me on that.

-Sara

Re: [PHP-DEV] Changes to Git commit workflow

[Mike Schinkel]

> On Mar 30, 2021, at 9:50 AM, Sara Golemon  wrote:
> 
> On Tue, Mar 30, 2021 at 8:34 AM Mike Schinkel  > wrote:
> > When you speak of NEWS, do you mean this?
> > 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 
Gotcha. Thanks for the details on that Sara and George.

Is there any chance the team would consider modifying the workflow a bit?  I 
only ask if my input is helpful. If not, please disregard.

When teams I have been on in the past have found frequent merge conflicts we've 
restructured to move the frequently conflicting items to their own repo.

Then the high-conflict repo gets its own set of workflow rules and we used 
CI/CD and more recently GitHub Actions to incorporate the "releases" from the 
high-conflict repo into the main low-conflict repo.

Given those files you mentioned they seems to be about release management — and 
maybe harder for someone to inject a back-door into — so maybe a "release 
management" repo that gets merged in to php-src with GitHub Actions at 
appropriate times?

Again, please feel free to tell me to butt out if my thoughts are not being 
helpful here.

-Mike

Re: [PHP-DEV] Changes to Git commit workflow

[G. P. B.]

On Tue, 30 Mar 2021 at 14:34, Mike Schinkel  wrote:

>
>
> > On Mar 30, 2021, at 8:29 AM, Jakub Zelenka  wrote:
> >
> >
> >
> > On Tue, Mar 30, 2021 at 1:21 PM Jakub Zelenka  bu...@php.net>> wrote:
> >
> >
> > On Tue, Mar 30, 2021 at 12:47 PM Mike Schinkel  > wrote:
> >
> >
> > > On Mar 30, 2021, at 5:51 AM, Derick Rethans  der...@php.net>> wrote:
> > >
> > > On 30 March 2021 10:43:41 BST, Max Semenik  > wrote:
> > >> On Tue, Mar 30, 2021 at 3:29 AM Stanislav Malyshev
> > >> mailto:smalys...@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 
>
> -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

Re: [PHP-DEV] Changes to Git commit workflow

[Sara Golemon]

On Tue, Mar 30, 2021 at 8:34 AM Mike Schinkel  wrote:
> When you speak of NEWS, do you mean this?
> 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

Re: [PHP-DEV] Changes to Git commit workflow

[Mike Schinkel]

> On Mar 30, 2021, at 8:29 AM, Jakub Zelenka  wrote:
> 
> 
> 
> On Tue, Mar 30, 2021 at 1:21 PM Jakub Zelenka  > wrote:
> 
> 
> On Tue, Mar 30, 2021 at 12:47 PM Mike Schinkel  > wrote:
> 
> 
> > On Mar 30, 2021, at 5:51 AM, Derick Rethans  > > wrote:
> > 
> > On 30 March 2021 10:43:41 BST, Max Semenik  > > wrote:
> >> On Tue, Mar 30, 2021 at 3:29 AM Stanislav Malyshev
> >> mailto:smalys...@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.
> 

When you speak of NEWS, do you mean this?

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 

Re: [PHP-DEV] Changes to Git commit workflow

[Jakub Zelenka]

On Tue, Mar 30, 2021 at 1:21 PM Jakub Zelenka  wrote:

>
>
> On Tue, Mar 30, 2021 at 12:47 PM Mike Schinkel 
> wrote:
>
>>
>>
>> > On Mar 30, 2021, at 5:51 AM, Derick Rethans  wrote:
>> >
>> > On 30 March 2021 10:43:41 BST, Max Semenik 
>> wrote:
>> >> On Tue, Mar 30, 2021 at 3:29 AM Stanislav Malyshev
>> >> 
>> >> 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

Re: [PHP-DEV] Changes to Git commit workflow

[Jakub Zelenka]

On Tue, Mar 30, 2021 at 12:47 PM Mike Schinkel  wrote:

>
>
> > On Mar 30, 2021, at 5:51 AM, Derick Rethans  wrote:
> >
> > On 30 March 2021 10:43:41 BST, Max Semenik 
> wrote:
> >> On Tue, Mar 30, 2021 at 3:29 AM Stanislav Malyshev
> >> 
> >> 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

Re: [PHP-DEV] Changes to Git commit workflow

[Mike Schinkel]

> On Mar 30, 2021, at 5:51 AM, Derick Rethans  wrote:
> 
> On 30 March 2021 10:43:41 BST, Max Semenik  wrote:
>> On Tue, Mar 30, 2021 at 3:29 AM Stanislav Malyshev
>> 
>> 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  

— 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 

— Heres a simply article showing how to automate with Javascript and Probot:
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/ 

[3] https://www.gitops.tech/  
[4] https://harness.io/blog/devops/what-is-gitops/ 



> 
> It sounds like an annoying hurdle.
> 
> cheers,
> Derick
> 
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: https://www.php.net/unsub.php
> 

Re: [PHP-DEV] Changes to Git commit workflow

[Derick Rethans]

On 30 March 2021 10:43:41 BST, Max Semenik  wrote:
>On Tue, Mar 30, 2021 at 3:29 AM Stanislav Malyshev
>
>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

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php

Re: [PHP-DEV] Changes to Git commit workflow

[Max Semenik]

On Tue, Mar 30, 2021 at 3:29 AM Stanislav Malyshev 
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

Re: [PHP-DEV] Changes to Git commit workflow

[Stanislav Malyshev]

Hi!

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

On Mon, Mar 29, 2021 at 1:53 AM Nikita Popov  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
smalys...@gmail.com

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php

Re: [PHP-DEV] Changes to Git commit workflow

[G. P. B.]

On Mon, 29 Mar 2021 at 12:50, Max Semenik  wrote:

> On Mon, Mar 29, 2021 at 1:53 AM Nikita Popov  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

Re: [PHP-DEV] Changes to Git commit workflow

[Max Semenik]

On Mon, Mar 29, 2021 at 1:53 AM Nikita Popov  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

Re: [PHP-DEV] Changes to Git commit workflow

[Paul Dragoonis]

On Mon, 29 Mar 2021, 08:51 Paul Dragoonis,  wrote:

>
>
> On Mon, 29 Mar 2021, 02:30 Rasmus Lerdorf,  wrote:
>
>> On Sun, Mar 28, 2021 at 17:15 Sara Golemon  wrote:
>>
>> > On Sun, Mar 28, 2021 at 6:57 PM Paul Crovella 
>> > 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.


>
>
>
>> -Rasmus
>>
>

Re: [PHP-DEV] Changes to Git commit workflow

[Paul Dragoonis]

On Mon, 29 Mar 2021, 02:30 Rasmus Lerdorf,  wrote:

> On Sun, Mar 28, 2021 at 17:15 Sara Golemon  wrote:
>
> > On Sun, Mar 28, 2021 at 6:57 PM Paul Crovella 
> > 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




> -Rasmus
>

Re: [PHP-DEV] Changes to Git commit workflow

[Deleu]

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

."

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  wrote:

> 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.
>
> >
> > On Mon, 29 Mar 2021, 03:30 Rasmus Lerdorf,  wrote:
> >
> > > On Sun, Mar 28, 2021 at 17:15 Sara Golemon  wrote:
> > >
> > > > On Sun, Mar 28, 2021 at 6:57 PM Paul Crovella <
> paul.crove...@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
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: https://www.php.net/unsub.php
>
>

Re: [PHP-DEV] Changes to Git commit workflow

[Ayesh Karunaratne]

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.

>
> On Mon, 29 Mar 2021, 03:30 Rasmus Lerdorf,  wrote:
>
> > On Sun, Mar 28, 2021 at 17:15 Sara Golemon  wrote:
> >
> > > On Sun, Mar 28, 2021 at 6:57 PM Paul Crovella 
> > > 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

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php

Re: [PHP-DEV] Changes to Git commit workflow

[Gabriel Caruso]

On Mon, 29 Mar 2021, 03:30 Rasmus Lerdorf,  wrote:

> On Sun, Mar 28, 2021 at 17:15 Sara Golemon  wrote:
>
> > On Sun, Mar 28, 2021 at 6:57 PM Paul Crovella 
> > 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

Re: [PHP-DEV] Changes to Git commit workflow

[Rasmus Lerdorf]

On Sun, Mar 28, 2021 at 17:15 Sara Golemon  wrote:

> On Sun, Mar 28, 2021 at 6:57 PM Paul Crovella 
> 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

Re: [PHP-DEV] Changes to Git commit workflow

[Sara Golemon]

On Sun, Mar 28, 2021 at 6:57 PM Paul Crovella 
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.

Re: [PHP-DEV] Changes to Git commit workflow

[Paul Crovella]

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

On Sun, Mar 28, 2021 at 3:53 PM Nikita Popov  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
> ni...@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 secur...@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

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php

[PHP-DEV] Changes to Git commit workflow

[Nikita Popov]

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
ni...@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 secur...@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