From 8b4b33db2dfe2f707336b4912dfef97205cbafbe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ulises=20Gasc=C3=B3n?= Date: Tue, 3 Jun 2025 15:13:50 +0200 Subject: [PATCH 1/8] docs: add ADR for branch protection Related https://github.com/expressjs/security-wg/issues/2 --- docs/adr/367-branch-protection-md | 106 ++++++++++++++++++++++++++++++ 1 file changed, 106 insertions(+) create mode 100644 docs/adr/367-branch-protection-md diff --git a/docs/adr/367-branch-protection-md b/docs/adr/367-branch-protection-md new file mode 100644 index 0000000..ae930cb --- /dev/null +++ b/docs/adr/367-branch-protection-md @@ -0,0 +1,106 @@ +# ADR 367: Enforce Branch Protection by Default Across All Repositories + +## Status + +Accepted + +## Submitters + +- @UlisesGascon + +## Decision Owners + +- @expressjs/express-tc + +## Context + +Branch protection rules in GitHub enforce key policies—such as required reviews, status checks, and linear history—that help safeguard critical branches like `main`, `master` or version specific. Despite the clear benefits, adoption across repositories has been inconsistent, leading to: + +- Risk of direct or accidental pushes to protected branches +- Increased likelihood of unreviewed or unsafe code merges +- Reduced ability to enforce standardized development workflows + +GitHub allows fine-grained control over branch protections, including the ability for administrators (tc members, repo captains...) to bypass rules when needed (e.g., urgent hotfixes or security patches). + +This decision aims to normalize a secure-by-default stance while maintaining flexibility for urgent, responsible exceptions. + +## Decision + +**Branch protection rules will be enabled by default on all repositories.** + +Key rules to be applied: + +- Protect the `main`, `master`, and `v*` branches +- Require pull request reviews before merging (at least 1 approval) +- Require all status checks to pass before merging + +Optional rules: +- Enforce linear history (disable merge commits, allow squash/rebase) +- Restrict who can push directly to the protected branches + +### Exceptions: + +- Tc members and repo captains will retain the ability to **temporarily bypass branch protections** in critical scenarios: + - Security fixes that require immediate deployment + - Emergency production incidents +- These exceptions must be documented in the relevant PR or release communication + +## Rationale + +### Alternatives Considered: + +- **Opt-in branch protection**: Rejected due to inconsistent adoption and higher risk of accidental pushes. +- **No admin bypass**: Rejected because it adds overhead in urgent situations and may block time-sensitive fixes. + +### Pros and Cons: + +**Pros:** + +- Stronger default security posture +- Better alignment with compliance and audit requirements +- Reduced chance of unsafe or accidental changes + +**Cons:** + +- May slow down development for repos lacking automation (e.g., CI failures delaying merges) +- Risk of misuse of admin bypass unless monitored + +**Why is this the best option?** + +This decision strikes a balance between **security and agility**, using defaults that promote safe practices while preserving escape hatches for legitimate exceptions. + +## Consequences + +### Positive Impact: + +- Improved consistency and safety across all codebases +- Reduced likelihood of accidental or unreviewed code merges +- Easier to audit and enforce development standards across org + +### Negative Impact: + +- Initial friction for projects with a single captain or no commiters (at least 1 approval per PR) +- Possible misuse of admin bypass if not documented + + +## Implementation + +### Phase 1: Rollout to Active Repos + +- Identify all active repositories +- Apply default protection rules using GitHub API or organization policies +- Notify repo maintainers of the change + +### Phase 2: Harden Governance + +- Periodic review of admin bypass usage using the Scorecard Monitor ([ref](https://github.com/expressjs/security-wg/pull/70)) +- Enforce admin bypass documentation for emergency changes + +## References + +- [GitHub Docs: About protected branches](https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/defining-the-mergeability-of-pull-requests/about-protected-branches) +- [OSSF Scorecard: Branch Protection](https://github.com/ossf/scorecard/blob/main/docs/checks.md#branch-protection) + +## Changelog + +- **[2025-06-03]**: @UlisesGascon – Initial proposal and adoption for org-wide branch protection default From 942e14173a85dabc4a6645e695b54960aca74b20 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ulises=20Gasc=C3=B3n?= Date: Tue, 3 Jun 2025 18:00:20 +0200 Subject: [PATCH 2/8] Update docs/adr/367-branch-protection-md Co-authored-by: Jordan Harband --- docs/adr/367-branch-protection-md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/adr/367-branch-protection-md b/docs/adr/367-branch-protection-md index ae930cb..615cd1f 100644 --- a/docs/adr/367-branch-protection-md +++ b/docs/adr/367-branch-protection-md @@ -14,7 +14,7 @@ Accepted ## Context -Branch protection rules in GitHub enforce key policies—such as required reviews, status checks, and linear history—that help safeguard critical branches like `main`, `master` or version specific. Despite the clear benefits, adoption across repositories has been inconsistent, leading to: +Branch protection rules in GitHub enforce key policies—such as required reviews, status checks, and linear history—that help safeguard critical branches like `main`, `master` or version branches/tags. Despite the clear benefits, adoption across repositories has been inconsistent, leading to: - Risk of direct or accidental pushes to protected branches - Increased likelihood of unreviewed or unsafe code merges From 3c40ac7d2d00811f311cd8d91bc5b71f48630c44 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ulises=20Gasc=C3=B3n?= Date: Tue, 3 Jun 2025 18:00:49 +0200 Subject: [PATCH 3/8] Update docs/adr/367-branch-protection-md Co-authored-by: Wes Todd --- docs/adr/367-branch-protection-md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/adr/367-branch-protection-md b/docs/adr/367-branch-protection-md index 615cd1f..c8b22a7 100644 --- a/docs/adr/367-branch-protection-md +++ b/docs/adr/367-branch-protection-md @@ -30,7 +30,7 @@ This decision aims to normalize a secure-by-default stance while maintaining fle Key rules to be applied: -- Protect the `main`, `master`, and `v*` branches +- Protect release branches, this includes: `main`, `master`, and `v*` branches/tags - Require pull request reviews before merging (at least 1 approval) - Require all status checks to pass before merging From e6e1cc44b9fb39c812227066ddb4603ae3cb9322 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ulises=20Gasc=C3=B3n?= Date: Tue, 3 Jun 2025 18:00:57 +0200 Subject: [PATCH 4/8] Update docs/adr/367-branch-protection-md Co-authored-by: Jordan Harband --- docs/adr/367-branch-protection-md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/adr/367-branch-protection-md b/docs/adr/367-branch-protection-md index c8b22a7..0df9a31 100644 --- a/docs/adr/367-branch-protection-md +++ b/docs/adr/367-branch-protection-md @@ -79,7 +79,7 @@ This decision strikes a balance between **security and agility**, using defaults ### Negative Impact: -- Initial friction for projects with a single captain or no commiters (at least 1 approval per PR) +- Initial friction for projects with a single captain or no committers (at least 1 approval per PR) - Possible misuse of admin bypass if not documented From 249a0f1504b81b36ae9be011874c57211935de91 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ulises=20Gasc=C3=B3n?= Date: Tue, 3 Jun 2025 18:01:09 +0200 Subject: [PATCH 5/8] Update docs/adr/367-branch-protection-md Co-authored-by: Chris de Almeida --- docs/adr/367-branch-protection-md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/adr/367-branch-protection-md b/docs/adr/367-branch-protection-md index 0df9a31..e442a9a 100644 --- a/docs/adr/367-branch-protection-md +++ b/docs/adr/367-branch-protection-md @@ -88,8 +88,8 @@ This decision strikes a balance between **security and agility**, using defaults ### Phase 1: Rollout to Active Repos - Identify all active repositories -- Apply default protection rules using GitHub API or organization policies - Notify repo maintainers of the change +- Apply default protection rules using GitHub API or organization policies ### Phase 2: Harden Governance From 9ffca785aa2f8db4ac6174b43b1c9b0468a0bfa8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ulises=20Gasc=C3=B3n?= Date: Tue, 3 Jun 2025 18:01:58 +0200 Subject: [PATCH 6/8] Update docs/adr/367-branch-protection-md Co-authored-by: Wes Todd --- docs/adr/367-branch-protection-md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/adr/367-branch-protection-md b/docs/adr/367-branch-protection-md index e442a9a..7866a56 100644 --- a/docs/adr/367-branch-protection-md +++ b/docs/adr/367-branch-protection-md @@ -36,7 +36,7 @@ Key rules to be applied: Optional rules: - Enforce linear history (disable merge commits, allow squash/rebase) -- Restrict who can push directly to the protected branches +- Restrict who can push directly to the protected branches to people who can release (captains, TC, etc) ### Exceptions: From 938799634afe310c8e7ed5cbfb788d6a47f20bd0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ulises=20Gasc=C3=B3n?= Date: Tue, 3 Jun 2025 18:02:10 +0200 Subject: [PATCH 7/8] Update docs/adr/367-branch-protection-md Co-authored-by: Wes Todd --- docs/adr/367-branch-protection-md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/adr/367-branch-protection-md b/docs/adr/367-branch-protection-md index 7866a56..8099bc2 100644 --- a/docs/adr/367-branch-protection-md +++ b/docs/adr/367-branch-protection-md @@ -40,7 +40,7 @@ Optional rules: ### Exceptions: -- Tc members and repo captains will retain the ability to **temporarily bypass branch protections** in critical scenarios: +- TC members and Repo Captains will retain the ability to **temporarily bypass branch protections** in critical scenarios: - Security fixes that require immediate deployment - Emergency production incidents - These exceptions must be documented in the relevant PR or release communication From 9ff47646284d7cbdf53f1bee256060ddfda1a79b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ulises=20Gasc=C3=B3n?= Date: Tue, 3 Jun 2025 18:02:20 +0200 Subject: [PATCH 8/8] Update docs/adr/367-branch-protection-md Co-authored-by: Wes Todd --- docs/adr/367-branch-protection-md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/adr/367-branch-protection-md b/docs/adr/367-branch-protection-md index 8099bc2..f5abeb2 100644 --- a/docs/adr/367-branch-protection-md +++ b/docs/adr/367-branch-protection-md @@ -43,6 +43,9 @@ Optional rules: - TC members and Repo Captains will retain the ability to **temporarily bypass branch protections** in critical scenarios: - Security fixes that require immediate deployment - Emergency production incidents + - When CI is clearly broken for unrelated reasons + - To fix history when necessary + - When a reasonable case is made where the exception makes sense - These exceptions must be documented in the relevant PR or release communication ## Rationale