Hi Matt, On 17.10.2025 20:06, Matt Sicker wrote: > The largest threat of supply chain attacks at this point is from > third party dependencies, especially given the proliferation of > Dependabot updates. Changes made to dependencies are never (or > almost never) reviewed with the same level of rigor as changes made > to our own code.
I agree that third-party dependencies can be a supply chain risk. However, I don’t believe this is a significant concern in our case because: - We only have a small number of dependencies. - We don’t bundle those dependencies. This risk is far more relevant for application developers. For libraries like ours, I think a much more realistic threat is the compromise of one of our committer PATs used in personal projects (where code is not reviewed), which could then be used to push malicious commits to `logging-log4j2`. If I wanted to attack Log4j, that’s exactly the vector I would pursue: obtaining temporary access to a token from one of the ~40 inactive committers. >> - Human oversight: No matter how careful we are, everyone >> occasionally overlooks small details. That’s exactly what peer >> review helps catch. > > This is what I used to do when I could review the commits@ mailing > list. Ever since that was flooded with Dependabot-related chatter, I > have been unable to use the list for post-commit review. It’s not like > this project moves at a super fast pace! We tend to make quarterly > releases which surely gives enough time to browse through commits and > bring up issues to dev@. I agree, Dependabot is a nuisance that needs (and will be addressed). However, reviewing via the `commits@` mailing list, even without Dependabot PRs, has several drawbacks: - It is harder to follow changes: instead of a single squashed commit per PR, we get multiple small commits. - Minor fixes (formatting, missing tests, etc.) “post-merge” generate additional noise in the mailing list. - The process is undocumented, so multiple PMC members may review the same commits redundantly. - Although we theoretically have 22 potential reviewers, in practice the actual number of reviews tends toward zero. > A project with one maintainer using RTC would mean that they can > only accept external contributions and are otherwise not allowed to > merge their own code. The key point is *active* maintainer. Under RTC, we only need one active maintainer, plus another committer who occasionally reviews internal PRs. >> Personally, I’m flexible about the *order* of review (before or after >> commit), as long as every commit is reviewed by someone other than the >> author. If we ever decided to adopt a true CTR model, where every commit >> is traceably reviewed, I’d be fine with that too. > > This is an interesting idea. Making the review process asynchronous > and non-blocking is my main desire. I’m ok with making follow-up > changes (it’s software; we can change it infinitely). What I’m not > fond of is maintaining long-lived branches (an anti-pattern) and > applying different review standards to different changes (which > can happen incidentally based on who decides to review the change > regardless of who made the change). The challenge with asynchronous post-commit review is that it still introduces blockers, just *later* in the process. For example, before any release, *all* commits must be reviewed. That simply moves the bottleneck to release time, and peer pressure may push people to “approve” changes without really reviewing. In this specific case, even if I merged your PR without review, nothing would change for me: I would still have to -1 any release until I independently review all unreviewed commits. Instead of switching to CTR, I suggest we adopt a simple mechanism to ensure timely and fair reviews: process PRs in creation order whenever possible. Some time ago I created a GitHub Project board [1] to help track PR status. We can improve automation later, but for now we can manage the status manually and use assignees to signal who is handling each review. I have already added your PRs to the board and updated their status: - Three PRs are waiting for your responses to review comments. - One PR is assigned to me and waiting for my review. > Suppose someone wanted to contribute code to commons-crypto in order > to support new PQH encryption ciphers recently standardized by NIST > and IETF. Suppose that person is a cryptographer or works in that > space in general. Now also suppose that the Commons PMC has > approximately 0 cryptographic experts. Then nobody feels comfortable > reviewing the code. If nobody on the PMC can understand a contribution well enough to review it, then we have two options: 1. Reject the PR, or 2. Invite the author to become a committer so they can maintain the code. However, relying on a single expert is not sustainable. At least two committers should feel comfortable maintaining a code area. History has shown we frequently need to modify code we didn’t write ourselves. Piotr References: [1] https://github.com/orgs/apache/projects/501/views/1
