Re: [PHP-DEV][DISCUSSION] Fix mb_trim inaccurate $character

2024-04-03 Thread youkidearitai
2024年4月4日(木) 6:30 Tim Düsterhus :
>
> Hi
>
> On 4/3/24 10:02, youkidearitai wrote:
> > Therefore, I think require an RFC, I have written a draft an RFC that
> > fixes these issues.
> > https://wiki.php.net/rfc/mb_trim_change_characters
>
> I don't think this (widening the type and changing the default value to
> obtain the *intended* behavior) requires an RFC. It's a bugfix, a bugfix
> with a slightly larger observable impact than other bugfixes.
>
> Best regards
> Tim Düsterhus

Hi
Thank you very much for comment.

I would like to discuss this RFC with all of you, as we have not yet
decided on the correct revision policy.
I created the RFC for that purpose.

Still waiting for your comments!
Thank you.

Yuya

-- 
---
Yuya Hamada (tekimen)
- https://tekitoh-memdhoi.info
- https://github.com/youkidearitai
-


Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-03 Thread Saki Takamachi
Hi Jordan,

> It looks pretty good. It should probably give specific guidelines on how "no 
> scale provided" works for addition, subtraction, multiplication, division, 
> and exponentiation. The section now doesn't make it clear that the returned 
> scale will behave differently for addition (the current example) than for 
> division.

I see, I'll add that later, thanks!

Regards.

Saki

Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-03 Thread Jordan LeDoux
On Wed, Apr 3, 2024 at 7:31 PM Saki Takamachi  wrote:

>
> Thanks, I agree with you that it works a little differently than the
> existing BCMath function, so it needs to be well-documented.
>
> I have summarized the discussion so far, made some corrections, and
> updated the RFC. I would be happy if you could check it out.
>

It looks pretty good. It should probably give specific guidelines on how
"no scale provided" works for addition, subtraction, multiplication,
division, and exponentiation. The section now doesn't make it clear that
the returned scale will behave differently for addition (the current
example) than for division.

Jordan


Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-03 Thread Saki Takamachi
Hi Barney, Jordan,

> Right, if a class is not final but has only final methods (including 
> constructor) and no protected properties then it allows people to extend it 
> but not break any encapsulation or (I think) impose any more BC requirements 
> on the maintainer (unless we care about conflicting with method names of 
> child classes when updating). It lets people just do something a bit like 
> adding 'extension methods' in C#. I like it. People could write e.g. 
> `$bcChildNumber->isPrime()` instead of `BcNumberUtils::isPrime($bcNumber)`

> Yeah, I suspect the most common child class will be something like "Money" 
> that additionally has a parameter denoting what currency the value is in, 
> since that is by far the most common use case for arbitrary precision math in 
> PHP. Making the calculation methods final should do fine in my opinion.

I realize that the purpose of using final here is to prevent users from 
customizing the behavior of operator overloads. In the RFC, there are methods 
that do not involve operator overloading, such as `round`, but do you think 
these should also be made final? Or should only methods related to overloading 
be made final?

IMHO, there are methods such as `powmod` that are computational functions but 
are not related to operator overloading, and from the viewpoint of consistency, 
we believe that all methods should be made final without exception.


> I think that having it be an optional part of the constructor and having a 
> method to modify it is probably sufficient to help with operator overloads. 
> Importantly, it still needs to take that setting and use it intelligently 
> depending on the operation as we've discussed, but that should cover the use 
> cases I was imagining.
> 
> The exact behavior should be very clearly documented and explained though, as 
> this is a different way of interacting with BCMath than the functions. It's 
> better I think, because it takes advantage of the fact that this will be 
> state-aware, which the functions aren't, and is one of the largest benefits 
> of using objects.
> 
> Also, to address an earlier question you had Saki, the term used should be 
> "scale". Scale is the total number of digits to the right of the decimal 
> point. Precision is the total number of significant digits. The BCMath C 
> library works on scale, I believe, so that should carry over here. Most other 
> arbitrary precision libraries in C use precision.

Thanks, I agree with you that it works a little differently than the existing 
BCMath function, so it needs to be well-documented.

I have summarized the discussion so far, made some corrections, and updated the 
RFC. I would be happy if you could check it out.

Regards.

Saki


Re: [PHP-DEV] Proposal: Arbitrary precision native scalar type

2024-04-03 Thread Jordan LeDoux
On Mon, Mar 18, 2024 at 1:35 PM Rowan Tommins [IMSoP] 
wrote:

>
> Where things get more complicated is with *fixed-precision* decimals,
> which is what is generally wanted for something like money. What is the
> correct result of decimal(1.03, precision: 2) / 2 - decimal(0.515, 3)?
> decimal(0.51, 2)? decimal (0.52, 2)? an error? And what about decimal(10) /
> 3?
>
> If you stick to functions / methods, this is slightly less of an issue,
> because you can have decimal(1.03, 2)->dividedBy(2, RoundingMode::DOWN) ==
> decimal(0.51, 2); or decimal(1.03, 2)->split(2) == [ decimal(0.52, 2),
> decimal(0.51, 2) ] Example names taken directly from the brick/money
> package.
>
> At that point, backwards compatibility is less of an issue as well: make
> the new functions convenient to use, but distinct from the existing ones
>
I came back to this discussion after my last reply on the BCMath as an
object thread, because we went through a very similar discussion there with
regard to operators.

I think that, roughly, the fixed precision should be defined on the scalar
itself:

1.234_d3

1.234 is the value, _d makes it a decimal type, and 3 shows that this value
has a precision of 3. (Actually, it has a SCALE of 3, since precision is
significant digits, and also includes the integer portion). But when it
comes to fixed-precision values, it should follow rules very similar to
those we discussed in the BCMath thread:

- Addition and subtraction should return a value that is the largest
scale/precision of any operands in the calculation.
- Division and multiplication should return a value that is the sum of the
scale/precision of any operands + 2 or a default (perhaps configurable)
value if the sum is small, to ensure that rounding occurs correctly. Near
zero, floats have about 12-ish decimal digits of accuracy, and will return
their full accuracy for example.
- Pow is extremely difficult to work with if you allow a decimal in the
exponent. The libraries we have discussed previously handle this
difficulty, but the precision needed often depends on what the calculation
is for, so I'm not entirely sure what the best way to handle this with
operators would be. Minimally, probably the same as division and
multiplication.

But if we have scalar decimal types, we need something like that _d12 to
denote a decimal scalar with a scale/precision of 12.

Jordan


Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-03 Thread Jordan LeDoux
On Tue, Apr 2, 2024 at 5:43 PM Saki Takamachi  wrote:

>
> If make a class final, users will not be able to add arbitrary methods, so
> I think making each method final. Although it is possible to completely
> separate behavior between method and opcode calculations, this is
> inconsistent and confusing to users and should be avoided.
>


Right, if a class is not final but has only final methods (including
> constructor) and no protected properties then it allows people to extend it
> but not break any encapsulation or (I think) impose any more BC
> requirements on the maintainer (unless we care about conflicting with
> method names of child classes when updating). It lets people just do
> something a bit like adding 'extension methods' in C#. I like it. People
> could write e.g. `$bcChildNumber->isPrime()` instead of
> `BcNumberUtils::isPrime($bcNumber)`


Yeah, I suspect the most common child class will be something like "Money"
that additionally has a parameter denoting what currency the value is in,
since that is by far the most common use case for arbitrary precision math
in PHP. Making the calculation methods final should do fine in my opinion.


> How about setting the `$scale` and `$roundMode` that I mentioned earlier
> in the constructor instead of `div` and `pow`? By doing so, we can use any
> scale when calculating with opcodes. If we want to specify a different
> scale for each calculation, you can do it using methods.
> Or would it be more convenient to have a method to reset `$scale` and
> `$roundMode`? It is immutable, so when reset it returns a new instance.
>

I think that having it be an optional part of the constructor and having a
method to modify it is probably sufficient to help with operator overloads.
Importantly, it still needs to take that setting and use it intelligently
depending on the operation as we've discussed, but that should cover the
use cases I was imagining.

The exact behavior should be very clearly documented and explained though,
as this is a different way of interacting with BCMath than the functions.
It's better I think, because it takes advantage of the fact that this will
be state-aware, which the functions aren't, and is one of the largest
benefits of using objects.

Also, to address an earlier question you had Saki, the term used should be
"scale". Scale is the total number of digits to the right of the decimal
point. Precision is the total number of significant digits. The BCMath C
library works on scale, I believe, so that should carry over here. Most
other arbitrary precision libraries in C use precision.

Jordan


Re: [PHP-DEV] [RFC] [Discussion] Deprecate GET/POST sessions

2024-04-03 Thread Kamil Tekiela
If there are no more comments, I would like to put this RFC to vote in
the next two days.


Re: [PHP-DEV][DISCUSSION] Fix mb_trim inaccurate $character

2024-04-03 Thread Tim Düsterhus

Hi

On 4/3/24 10:02, youkidearitai wrote:

Therefore, I think require an RFC, I have written a draft an RFC that
fixes these issues.
https://wiki.php.net/rfc/mb_trim_change_characters


I don't think this (widening the type and changing the default value to 
obtain the *intended* behavior) requires an RFC. It's a bugfix, a bugfix 
with a slightly larger observable impact than other bugfixes.


Best regards
Tim Düsterhus


Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-03 Thread Barney Laurance

On 03/04/2024 01:42, Saki Takamachi wrote:

If make a class final, users will not be able to add arbitrary methods, so I 
think making each method final.



Right, if a class is not final but has only final methods (including 
constructor) and no protected properties then it allows people to extend 
it but not break any encapsulation or (I think) impose any more BC 
requirements on the maintainer (unless we care about conflicting with 
method names of child classes when updating). It lets people just do 
something a bit like adding 'extension methods' in C#. I like it. People 
could write e.g. `$bcChildNumber->isPrime()` instead of 
`BcNumberUtils::isPrime($bcNumber)`


Re: [PHP-DEV] Requiring GPG Commit Signing

2024-04-03 Thread Tim Düsterhus

Hi

[Resending, because my mail server failed to look up php.net. It looks 
good now, I apologize for duplicate copies.]


On 4/3/24 19:28, John Coggeshall wrote:

That's really unfortunate (why even bother). IMO without some sort of web of 
trust verification process for GPG, this just feels like added barriers for no 
actual win. In fact, if anything I think it's more likely to give the project a 
false sense of security.


While it does not *prevent* any attacks, it possibly simplifies an 
investigation:


For example: Did John Doe suddenly start signing with a new key? Or was 
only a single commit signed with a different key?


If John uses a different key for each computer (e.g. one for the work 
laptop and one for the private gaming computer), then the signature 
possibly allows determining which machine was compromised.


These are useful signals to determine the possible scope of an attack.

Best regards
Tim Düsterhus


Re: [PHP-DEV] [RFC][Concept] Data classes (a.k.a. structs)

2024-04-03 Thread Ilija Tovilo
Hi Larry

On Wed, Apr 3, 2024 at 12:03 AM Larry Garfield  wrote:
>
> On Tue, Apr 2, 2024, at 6:04 PM, Ilija Tovilo wrote:
>
> > I think you misunderstood. The intention is to mark both call-site and
> > declaration. Call-site is marked with ->method!(), while declaration
> > is marked with "public mutating function". Call-site is required to
> > avoid the engine complexity, as previously mentioned. But
> > declaration-site is required so that the user (and IDEs) even know
> > that you need to use the special syntax at the call-site.
>
> Ah, OK.  That's... unfortunate, but I defer to you on the implementation 
> complexity.

As I've argued, I believe the different syntax is a positive. This
way, data classes are known to stay unmodified unless:

1. You're explicitly modifying it yourself.
2. You're calling a mutating method, with its associated syntax.
3. You're creating a reference from the value, either explicitly or by
passing it to a by-reference parameter.

By-reference argument passing is the only way that mutations of data
classes can be hidden (given that they look exactly like normal
by-value arguments), and its arguably a flaw of by-reference passing
itself. In all other cases, you can expect your value _not_ to
unexpectedly change. For this reason, I consider it as an alternative
approach to readonly classes.

> > Disallowing ordinary by-ref objects is not trivial without additional
> > performance penalties, and I don't see a good reason for it. Can you
> > provide an example on when that would be problematic?
>
> There's two aspects to it, that I see.
>
> data class A {
>   public function __construct(public string $name) {}
> }
>
> data class B {
>   public function __construct(
> public A $a,
> public PDO $conn,
>   ) {}
> }
>
> $b = new B(new A(), $pdoConnection);
>
> function stuff(B $b2) {
>   $b2->a->name = 'Larry';
>   // This triggers a CoW on $b2, separating it from $b, and also creating a 
> new instance of A.  What about $conn?
>   // Does it get cloned?  That would be bad.  Does it not get cloned?  That 
> seems weird that it's still the same on
>   // a data object.
>
>   $b2->conn->beginTransaction();
>   // This I would say is technically a modification, since the state of the 
> connection is changing.  But then
>   // should this trigger $b2 cloning from $b1?  Neither answer is obvious to 
> me.
> }

IMO, the answer is relatively straight-forward: PDO is a reference
type. For all intents and purposes, when you're passing B to stuff(),
B is copied. Since B::$conn is a "reference" (read pointer), copying B
doesn't copy the connection, only the reference to it. B::$a, however,
is a value type, so copying B also copies A. The fact that this isn't
_exactly_ what happens under the hood due to CoW is an implementation
detail, it doesn't need to change how you think about it. From the
users standpoint, $b and $b2 can already separate values once stuff()
is called.

This is really no different from arrays:

```php
$b = ['a' => ['name' => 'Larry'], 'conn' => $pdoConnection];
$b2 = $b; // $b is detached from $b2, $b['conn'] remains a shared object.
```

> The other aspect is, eg, serialization.  People will come to expect 
> (reasonably) that a data class will have certain properties (in the abstract 
> sense, not lexical sense).  For instance, most classes are serializable, but 
> a few are not.  (Eg, if they have a reference to PDO or a file handle or 
> something unserializable.)  Data classes seem like they should be safe to 
> serialize always, as they're "just data".  If data classes are limited to 
> primitives and data classes internally, that means we can effectively 
> guarantee that they will be serializable, always.  If one of the properties 
> could be a non-serializable object, that assumption breaks.

I'm not sure that's a convincing argument to fully disallow reference
types, especially since it would prevent you from storing
DateTimeImmutables and other immutable values in data classes and thus
break many valid use-cases. That would arguably be very limiting.

> There's probably other similar examples besides serialization where "think of 
> this as data" and "think of this as logic" is how you'd want to think, which 
> leads to different assumptions, which we shouldn't stealthily break.

I think your assumption here is that non-data classes cannot contain
data. This doesn't hold, and especially will not until data classes
become more common. Readonly classes can be considered strict versions
of data classes in terms of mutability, minus some of the other
semantic changes (e.g. identity).

Ilija


Re: [PHP-DEV] Requiring GPG Commit Signing

2024-04-03 Thread John Coggeshall
> > Having GPG key requirements is all fine and dandy I suppose, but my
> > tongue-in-cheek comment above has a real point behind it: GPG keys
> > don't mean jack if you can't trust who owns the key.
>
> GitHub doesn't show the web of trust anyway, just "verified". Command
> line GIT doesn't either, just:
>

That's really unfortunate (why even bother). IMO without some sort of web of 
trust verification process for GPG, this just feels like added barriers for no 
actual win. In fact, if anything I think it's more likely to give the project a 
false sense of security.
Cheers,
John


Re: [PHP-DEV] Requiring GPG Commit Signing

2024-04-03 Thread Derick Rethans
On Tue, 2 Apr 2024, John Coggeshall wrote:

> 
> > So if we want to make sure that something like XY doesn't happen, we
> > have to add some additional restrictions to those GPG keys.
> 
> Looks like all those geeky colleagues of ours back in the day having 
> key-signing parties at conferences were on to something, maybe..

> Let's be clear about something -- having GPG key requirements isn't 
> going to help a situation like XZ. The XZ attack was done by an active 
> maintainer of the project (who arguably manipulated the original 
> maintainer of the project to become a maintainer themselves). It was 
> as much a social engineering attack as anything.

> Having GPG key requirements is all fine and dandy I suppose, but my 
> tongue-in-cheek comment above has a real point behind it: GPG keys 
> don't mean jack if you can't trust who owns the key.

GitHub doesn't show the web of trust anyway, just "verified". Command 
line GIT doesn't either, just:

$ git verify-commit HEAD~1
gpg: Signature made Tue 02 Apr 2024 14:55:21 BST
gpg:using RSA key 5A52880781F755608BF815FC910DEB46F53EA312
…
gpg: aka "Derick Rethans (GitHub) " 
[ultimate]
gpg: aka "Derick Rethans (PHP) " [ultimate]


cheers,
Derick

Re: [PHP-DEV] Requiring GPG Commit Signing

2024-04-03 Thread Derick Rethans
On Tue, 2 Apr 2024, Ayesh Karunaratne wrote:

> > What do y'all think about requiring GPG signed commits for the 
> > php-src repository?
> >
> > I had a look, and this is also something we can enforce through 
> > GitHub as well (by using branch protections).
> 
> +1 from me as well, and quite good timing with all the xz fiasco just 
> last week.
> 
> Git can also sign with SSH keys now, so this is now merely a config 
> update

The issue with SSH keys is that they can not be signed by others to form 
a "web of trust". For example, my key has been signed by several other 
people:

https://keyserver.ubuntu.com/pks/lookup?search=derick%40php.net=on=index

cheers,
Derick


Re: [PHP-DEV] First time contributor (DateTime::setDate PR)

2024-04-03 Thread Derick Rethans
On Tue, 2 Apr 2024, Bilge wrote:

> On 02/04/2024 14:14, Derick Rethans wrote:
> > 
> > On Sun, 31 Mar 2024, Bilge wrote:
> > 
> > > About the PR: I sometimes find it would be useful to only update 
> > > part of the date. The PR makes all parameters to 
> > > DateTime(Immutable)::setDate 
> > >  
> > > optional in a backwards-compatible manner such that we can elect 
> > > to update only the day, month, year or any combination of the 
> > > three (thanks, in part, to named parameters). Without this 
> > > modification, we must always specify all of the day, month and 
> > > year parameters to change the date.
> > As I mentioned to you in Room 11, I am not in favour of adhoc API 
> > changes to Date/Time classes. It has now been nearly 18 years since 
> > they were originally introduced, and they indeed could do with an 
> > overhaul.
> > 
> > I have been colllecting ideas in 
> > https://docs.google.com/document/d/1pxPSRbfATKE4TFWw72K3p7ir-02YQbTf3S3SIxOKWsk/edit#heading=h.2jol7kfhmijb
> > 
> > Having different/better modifiers would also be a good thing to talk 
> > about, albeit perhaps on the four mentioned new classes, instead of 
> > adding them to the already existing DateTime and DateTimeImmutable 
> > classes.
> > 
> > In any case, just allowing setDate to be able to just modify the 
> > month is going to introduce confusion, as this will be counter 
> > intuitive:
> > 
> > $dt = new DateTimeImmutable("2024-03-31");
> > $newDt = $dt->setDate( month: 2 );
> > 
> > It is now representing 2024-03-02.
> > 
> > This might be the right answer, but it might also be that the 
> > developer just cared about the month part (and not the 
> > day-of-month), in which case this is a WTF moment.
> > 
> > Picking mofication APIs is not as trivial as it seems, and I would 
> > like to do it *right*.
> > 
> > Feel free to add comments and wishes to the google doc document. In 
> > the near future, I will be writing up an RFC from this.
> 
> Thanks for your reply!
> 
> Indeed, as per your code snippet, this is a WTF moment I had not accounted for
> and confirm the same result with my patch applied. Generally, my expectation
> here would be the month *must* be set to 2, so if the day portion will be
> invalidated by that change, we should either throw an exception or implicitly
> coerce it into range, i.e. (new DateTime("2024-03-31"))->setDate(month: 2); ==
> 2024-02-29.

We can't do that, as it will be inconsistent with:

modify("+1 month")
// (also 2024-03-02)

> However, I suppose this is not the conversation we're having as
> you do not wish to change this API at all, which I respect.

I think what you're actually after here, is a "YearMonth" type, which 
doesn't concern itself with the day-of-month at all. Setting just the 
month should perhaps just reset the day-of-month to 1 instead. ANd 
setting just the year would result in January 1st, as well.

> Regarding your brainstorm document, I can't understand much of it in its
> current state, and as I am not a subject matter expert, I think you will
> receive much better feedback from others.

It isn't particularly organised yet, and... I also haven't made sense of 
all of it yet.

> In particular, I cannot glean which four classes you are referring to 
> in that document. Yet what I do find interesting is the notion of 
> adding setters to DateTimeImmutable. For my particular 
> use-case—producing a collection of dates incrementing by year in a 
> Twig template—a trivial year setter would do just fine, with the 
> significant caveat that it must implement fluent interface, because I 
> need to call it in an expression context (returning a value), not a 
> statement context (executing a void function separately). Not that 
> Twig cannot execute statements, but it just becomes more verbose, 
> cumbersome and less template-like.
> 
> If you were happy for me to add getters and fluent setters for year, 
> I'd be happy to work on that PR, but for month we're back to the same 
> problem outlined in the opening paragraph (and I suppose the same 
> problem occasionally applies to year, if the day happens to be set to 
> the leap day). Otherwise, I'll be happy to read over your RFC when 
> it's ready.

Feel free to add your suggestion (anywhere) in the document, preferrably 
at the bottom :-)

I can envision that we'll have some actual setters and getters, beyond 
just add, sub, and modify (which is really powerful).

But what is important to understand is rather *why* a specific method is 
useful, and perhaps even more important is what you're actual goal is.

cheers,
Derick

-- 
https://derickrethans.nl | https://xdebug.org | https://dram.io

Author of Xdebug. Like it? Consider supporting me: https://xdebug.org/support

mastodon: @derickr@phpc.social @xdebug@phpc.social

[PHP-DEV][DISCUSSION] Fix mb_trim inaccurate $character

2024-04-03 Thread youkidearitai
Hi, Internals
Found two problem with mb_trim, mb_ltrim and mb_rtrim functions.

- https://github.com/php/php-src/issues/13789
- https://github.com/php/php-src/issues/13815

Therefore, I think require an RFC, I have written a draft an RFC that
fixes these issues.
https://wiki.php.net/rfc/mb_trim_change_characters

Feel free to comments on this.

Regards
Yuya

-- 
---
Yuya Hamada (tekimen)
- https://tekitoh-memdhoi.info
- https://github.com/youkidearitai
-


Re: [PHP-DEV] Requiring GPG Commit Signing

2024-04-03 Thread Rob Landers
On Tue, Apr 2, 2024, at 21:40, Rowan Tommins [IMSoP] wrote:
> On 02/04/2024 20:02, Ilija Tovilo wrote:
>> But, does it matter? I'm not sure we look at some commits closer than
>> others, based on its author. It's true that it might be easier to
>> identify malicious commits if they all come from the same user, but it
>> wouldn't prevent them.
> 
> 
> It's like the difference between stealing someone's credit card, and cloning 
> the card of everyone who comes into the shop: in the first case, someone 
> needs to check their credit card statements carefully; in the second, you'll 
> have a hard job even working out who to contact.
> 
> Similarly, if you discover a compromised key or signing account, you can look 
> for uses of that key or account, which might be a tiny number from a non-core 
> contributor; if you discover a compromised account pushing unsigned commits, 
> you have to audit every commit in the repository.
> 
> I agree it's not a complete solution, but no security measure is; it's always 
> about reducing the attack surface or limiting the damage.
> 
> Regards,
> 
> -- 
> Rowan Tommins
> [IMSoP]

FWIW, I store my signing and ssh keys on yubikeys. Even then, when I managed to 
lose one several years ago, revoking the certificate on GitHub was relatively 
straightforward. Further, it marked every commit made by that key (ever) as 
unverified.

So, in the very least, if a key were to be compromised, any open PRs would need 
to be resigned by the author to get them back in good standing; if verified 
commits are required. 

This, of course, makes GitHub the "single point of failure." If someone were to 
gain access to my GH (even on an unattended laptop), they could add their own 
keys, and on another computer; log in with another account, and then push 
commits with my email address and their gpg key. GitHub would show them as 
verified (IIRC) and from me. This is by design, since "in theory," I could have 
pushed my commits to a coworker's git repo, who then pushed it to GH.

Terrifying stuff... but it is even more terrifying than not having signed 
commits at all, since literally anyone can push a commit with anyone's name on 
it and nobody would even know it was a counterfeit. So, at least requiring 
signed commits makes the bar that much higher to counterfeit/hide malicious 
commits.

If this stuff terrifies you too, I recommend turning on vigilant mode: 
https://docs.github.com/en/authentication/managing-commit-signature-verification/displaying-verification-statuses-for-all-of-your-commits

— Rob