fix(org): layer org-level branch protection with repo rules — most-restrictive wins (#727) #728

Merged

jmiller merged 30 commits from fix/727-materialize-org-branch-protection into dev 2026-07-05 04:32:38 +00:00

Conversation 2 Commits 30 Files Changed 27

+604 -178

jmiller commented 2026-07-05 00:16:36 +00:00

Owner

Copy Link

Copy Source

Closes #727.

Approach changed (force-pushed). This PR originally materialized org rules into per-repo rows. Investigation showed that was the wrong model — see below. The branch was reset and reimplemented as runtime layering; the diff is now just two files.

What's actually going on

Org-level branch protection is not inert as the issue states. The single enforcement choke point every push/merge/approval/status/force-push/delete check funnels through — GetFirstMatchProtectedBranchRule (models/git/protected_branch_list.go) — already consults org rules. The issue reporter's grep OrgProtectedBranch missed it because that file calls FindOrgBranchRuleForBranch/ToProtectedBranch (functions) without ever naming the type.

But it only used the org rule as a fallback: if any repo rule matched the branch, the org rule was ignored entirely — so a repo could define a looser rule for a pattern and opt out of the org's protection.

Fix — layer, most-restrictive wins (fail-closed)

Change that one choke point to combine the repo rule and the org rule when both match, returning the stricter constraint per field. The org rule becomes a mandatory floor a repo can only tighten.

• models/git/protected_branch_merge.go (new): mergeMostRestrictive + helpers.
  • allow flags (Can*, *ActionsUser, *DeployKeys) → AND
  • gate/require/block flags (Enable*, Block*, Require*) → OR
  • RequiredApprovals → max
  • required-set lists (status contexts, protected files) → union
  • allow-set lists (whitelists, unprotected files) → intersection (a disabled allowlist means "everyone", so it only constrains when enabled)
• models/git/protected_branch_list.go: GetFirstMatchProtectedBranchRule fetches both rules and merges; returns whichever exists when only one matches. Org lookup factored into getFirstMatchOrgProtectedBranchRule.

Because the choke point feeds all ~15 enforcement sites, no enforcement site needs editing and the existing checks run unchanged on the merged rule.

Safety

Fail-closed: every merge edge errs toward more protection (over-restrict) rather than less, so a mistake can lock people out (recoverable, visible) but cannot silently open a hole. Whitelist intersection across separate user/team OR-criteria can over-restrict in edge cases — documented in the merge function.

Existing tests unaffected: the unit test uses a user-owned fixture repo (no org rules), integration tests set up no org rules — layering only adds restriction when an org rule matches.

⚠️ Verification caveat

No Go toolchain in the authoring environment — not compiled, gofmt'd, vet'd, or tested locally. Hand-verified: tab indentation, no helper-name collisions in package git, every merged field name against the ProtectedBranch struct, and that the reworked choke point preserves prior error-handling. CI must validate build + format + tests before merge, and integration testing of layered enforcement is recommended.

https://claude.ai/code/session_01Wsno14cxE49MstXFs9G5KT

Closes #727. > **Approach changed (force-pushed).** This PR originally *materialized* org rules into per-repo rows. Investigation showed that was the wrong model — see below. The branch was reset and reimplemented as **runtime layering**; the diff is now just two files. ## What's actually going on Org-level branch protection is **not** inert as the issue states. The single enforcement choke point every push/merge/approval/status/force-push/delete check funnels through — `GetFirstMatchProtectedBranchRule` (`models/git/protected_branch_list.go`) — **already** consults org rules. The issue reporter's `grep OrgProtectedBranch` missed it because that file calls `FindOrgBranchRuleForBranch`/`ToProtectedBranch` (functions) without ever naming the type. But it only used the org rule as a **fallback**: if *any* repo rule matched the branch, the org rule was ignored entirely — so a repo could define a looser rule for a pattern and opt out of the org's protection. ## Fix — layer, most-restrictive wins (fail-closed) Change that one choke point to **combine** the repo rule and the org rule when both match, returning the stricter constraint per field. The org rule becomes a mandatory floor a repo can only tighten. - `models/git/protected_branch_merge.go` (new): `mergeMostRestrictive` + helpers. - allow flags (`Can*`, `*ActionsUser`, `*DeployKeys`) → **AND** - gate/require/block flags (`Enable*`, `Block*`, `Require*`) → **OR** - `RequiredApprovals` → **max** - required-set lists (status contexts, protected files) → **union** - allow-set lists (whitelists, unprotected files) → **intersection** (a disabled allowlist means "everyone", so it only constrains when enabled) - `models/git/protected_branch_list.go`: `GetFirstMatchProtectedBranchRule` fetches both rules and merges; returns whichever exists when only one matches. Org lookup factored into `getFirstMatchOrgProtectedBranchRule`. Because the choke point feeds all ~15 enforcement sites, **no enforcement site needs editing** and the existing checks run unchanged on the merged rule. ## Safety **Fail-closed:** every merge edge errs toward *more* protection (over-restrict) rather than less, so a mistake can lock people out (recoverable, visible) but cannot silently open a hole. Whitelist intersection across separate user/team OR-criteria can over-restrict in edge cases — documented in the merge function. Existing tests unaffected: the unit test uses a user-owned fixture repo (no org rules), integration tests set up no org rules — layering only adds restriction when an org rule matches. ## ⚠️ Verification caveat No Go toolchain in the authoring environment — **not compiled, `gofmt`'d, vet'd, or tested locally.** Hand-verified: tab indentation, no helper-name collisions in package `git`, every merged field name against the `ProtectedBranch` struct, and that the reworked choke point preserves prior error-handling. **CI must validate build + format + tests before merge**, and integration testing of layered enforcement is recommended. https://claude.ai/code/session_01Wsno14cxE49MstXFs9G5KT

jmiller referenced this pull request 2026-07-05 00:17:23 +00:00

Org-level branch protection is stored but never enforced (no enforcement/materialization) #727

jmiller changed title from fix(org): materialize org-level branch protection into per-repo rules (#727) to fix(org): layer org-level branch protection with repo rules — most-restrictive wins (#727) 2026-07-05 00:43:02 +00:00

jmiller referenced this pull request 2026-07-05 00:43:26 +00:00

Org-level branch protection is stored but never enforced (no enforcement/materialization) #727

jmiller commented 2026-07-05 01:25:46 +00:00

Author

Owner

Copy Link

Copy Source

Added a second commit: surface the inherited org rules in the repo's Branch Protection settings (read-only), like GitHub shows org rulesets in a repo.

• ProtectedBranchRules handler loads FindOrgProtectedBranchRules for org-owned repos → OrgProtectedBranches.
• templates/repo/settings/branches.tmpl: new read-only "Organization Branch Protection" section — each org rule shows an Organization badge, a lock/read-only marker, and compact indicators (required approvals, signed commits, status checks). No edit/delete controls (managed at the org level).
• 7 en-US locale strings.

Closes the visibility gap noted earlier: the org floor is enforced implicitly at the choke point, so admins had no way to see which org rules applied to their repo. +46 lines, 3 files.

Same caveat: no Go toolchain locally → not compiled/tested; CI must validate. Template/locale render errors surface at runtime, so a quick manual load of the branch-protection settings page on an org repo is worth doing.

Added a second commit: **surface the inherited org rules in the repo's Branch Protection settings** (read-only), like GitHub shows org rulesets in a repo. - `ProtectedBranchRules` handler loads `FindOrgProtectedBranchRules` for org-owned repos → `OrgProtectedBranches`. - `templates/repo/settings/branches.tmpl`: new read-only "Organization Branch Protection" section — each org rule shows an **Organization** badge, a lock/read-only marker, and compact indicators (required approvals, signed commits, status checks). No edit/delete controls (managed at the org level). - 7 en-US locale strings. Closes the visibility gap noted earlier: the org floor is enforced implicitly at the choke point, so admins had no way to see which org rules applied to their repo. +46 lines, 3 files. Same caveat: no Go toolchain locally → not compiled/tested; CI must validate. Template/locale render errors surface at runtime, so a quick manual load of the branch-protection settings page on an org repo is worth doing.

jmiller commented 2026-07-05 02:16:47 +00:00

Author

Owner

Copy Link

Copy Source

Follow-up commit — richer read-only view plus a new capability:

1. Branch deletion is now an org-level ability. OrgProtectedBranch gained CanDelete / EnableDeleteAllowlist / DeleteAllowlistTeamIDs (migration 362), ToProtectedBranch maps them, and the API surface (OrgBranchProtection, CreateOrgBranchProtectionOption, EditOrgBranchProtectionOption + create/edit handlers) exposes enable_delete / enable_delete_allowlist / delete_allowlist_teams. The layering merge already combined the delete fields, so org delete-protection enforces as a floor once populated. (Deletion was previously not removed from the view — it's added as a real ability instead.)

2. The inherited-rule view is now an expandable detail (<details>) rather than three badges: direct push, force-push, branch deletion, merge restriction, required approvals, status checks, and protected-file patterns, with team IDs resolved to names. Still read-only.

+203 lines across model, migration, API DTO/handlers, web handler, template, locale, changelog.

Same caveat: no Go toolchain locally. Hand-verified: struct-field gofmt alignment (name→30 / type→9 in DTO, →19 in model; literal values→col 31), template block-nesting balances, every .Rule.* field exists on OrgProtectedBranch, all 34 template locale keys defined, JSON valid. CI + a manual load of an org repo's Branch Protection page still recommended before merge.

Follow-up commit — richer read-only view **plus** a new capability: **1. Branch deletion is now an org-level ability.** `OrgProtectedBranch` gained `CanDelete` / `EnableDeleteAllowlist` / `DeleteAllowlistTeamIDs` (migration **362**), `ToProtectedBranch` maps them, and the API surface (`OrgBranchProtection`, `CreateOrgBranchProtectionOption`, `EditOrgBranchProtectionOption` + create/edit handlers) exposes `enable_delete` / `enable_delete_allowlist` / `delete_allowlist_teams`. The layering merge already combined the delete fields, so org delete-protection enforces as a floor once populated. *(Deletion was previously not removed from the view — it's added as a real ability instead.)* **2. The inherited-rule view is now an expandable detail** (`<details>`) rather than three badges: direct push, force-push, branch deletion, merge restriction, required approvals, status checks, and protected-file patterns, with team IDs resolved to names. Still read-only. +203 lines across model, migration, API DTO/handlers, web handler, template, locale, changelog. Same caveat: no Go toolchain locally. Hand-verified: struct-field gofmt alignment (name→30 / type→9 in DTO, →19 in model; literal values→col 31), template block-nesting balances, every `.Rule.*` field exists on `OrgProtectedBranch`, all 34 template locale keys defined, JSON valid. CI + a manual load of an org repo's Branch Protection page still recommended before merge.

jmiller referenced this issue from a commit 2026-07-05 02:37:43 +00:00

feat(org): org-level tag protection, layered with per-repo protected tags (#727)

jmiller referenced this pull request 2026-07-05 02:38:17 +00:00

feat(org): org-level tag protection, layered with per-repo protected tags (#727) #729

jmiller referenced this pull request 2026-07-05 02:55:58 +00:00

feat(org): org-level push policy enforced in the pre-receive hook (#727) #730

jmiller referenced this pull request 2026-07-05 03:10:58 +00:00

feat(org): org-level repository defaults applied on repo create/transfer (#727) #731

jmiller referenced this pull request 2026-07-05 03:41:07 +00:00

feat(org): org-level email domain policy for members (#727) #732

jmiller changed target branch from main to dev 2026-07-05 03:58:17 +00:00

jmiller added 30 commits 2026-07-05 03:58:17 +00:00

chore: sync pr-check.yml from Template-Generic [skip ci] 108b19dcc8

release: code security scanner (#552) ... f8a91ed34e

Code security scanner with 22 OWASP pattern detection rules across 7 CWE categories (SQL injection, XSS, command injection, path traversal, insecure deserialization, hardcoded credentials, weak cryptography). Language-filtered scanning for Go, PHP, Python, JS/TS, Java, C#, Ruby.

chore: sync auto-release.yml from Template-Generic [skip ci] 11b2195bdf

fix: remove orphaned deploy-manual workflow [skip ci] 84c6a94333

Merge pull request 'fix: remove orphaned deploy-manual workflow' (#718) from fix/remove-deploy-manual into main ... 78328146e5

fix: remove orphaned deploy-manual workflow [skip ci]

fix: support radio inputs in admin system config form ... 982e45a56e

The system config form JS (config.ts) only mapped checkbox, text, textarea, and datetime-local elements. The fork landing_page.tmpl uses radio inputs for the Mode field, so fillFromSystemConfig() hit unsupportedElement() and threw, aborting all JS init on the admin settings page.

Add radio handling in both directions: fill checks the option whose value matches the config value; collect returns the checked option's value and skips/nulls unchecked radios so a group resolves to exactly one value. Adds a radio-group test case.

fix: preserve server-rendered radio default when config value is empty ... efb0433412

LandingPageType.Mode defaults to "" (Go zero value), and the template renders the home radio as checked for an empty Mode. The initial radio fill would evaluate home.checked = ("home" === "") = false, unchecking the default on a fresh install. Skip assignment when the config value is empty so the server-rendered selection is preserved. Adds a test for the empty-value case.

ci: allow fix/patch branches to target main and guard missing manifest ... b252e9569f

Branch policy in pr-check.yml only allowed fix/* and patch/* to target dev/rc, blocking fix/* PRs to main despite the documented policy. Allow fix/* -> main and patch/* -> main. Also guard the Detect platform step for a missing .mokogitea/manifest.xml (removed in favor of the metadata API) so it no longer aborts the Validate PR job under set -e.

fix: remove dangling mcp-mokogitea-api submodule gitlink ... fc234bc911

The tree carried a gitlink at mcp-mokogitea-api (mode 160000) with no .gitmodules entry, so git submodule update --init --recursive failed with exit 128 at checkout, breaking every PR build/release job. mcp-mokogitea-api is a separate repo, not a submodule; remove the gitlink from the index (keeping the local working-tree clone) and gitignore the path so it can't be re-added.

ci: no-op PR RC Release when updates.xml is absent ... aea4370845

The RC release workflow drives a Joomla-style updates.xml update stream. On a generic repo with no updates.xml, the Determine RC version step ran sed on a missing file and aborted under set -e (exit 2). Detect updates.xml presence and gate the update-stream steps (edit/create-release/commit) on it so the job succeeds and no-ops when there is nothing to package.

Merge pull request 'fix: support radio inputs in admin system config form' (#724) from fix/admin-config-radio into main cb9516b79b

chore: sync auto-release.yml from Template-Generic [skip ci] 844b4b6e81

chore: sync ci-generic.yml from Template-Generic [skip ci] 8fc2700c16

chore: sync cleanup.yml from Template-Generic [skip ci] 51332a587d

chore: sync notify.yml from Template-Generic [skip ci] 296df7792a

chore: sync pr-check.yml from Template-Generic [skip ci] adf0e923df

chore: sync pre-release.yml from Template-Generic [skip ci] 8680fd1cab

chore: sync sync-on-merge.yml from Template-Generic [skip ci] 0c45a2fda3

chore: sync branch-cleanup.yml from Template-Generic [skip ci] 05763eb661

chore: sync ci-generic.yml from Template-Generic [skip ci] 1256f57d40

chore: sync pr-check.yml from Template-Generic [skip ci] 22b0b14ea8

chore: sync ci-generic.yml from Template-Generic [skip ci] d77b82a8af

chore: sync pr-check.yml from Template-Generic [skip ci] ea87b3db97

chore: sync pr-check.yml from Template-Generic [skip ci] 4ed45a5916

fix: render org teams list and make issue type editable (#720, #721) ... 0cc569aef6

#720: org Teams page wrote ctx.Data["OrgListTeams"] but the template iterates .Teams, so no teams rendered. Use the canonical Teams key (matches org/home.go). #721: issue type sidebar gated editing on a FieldEditFlags data key that no handler sets (always nil -> always read-only). Use HasIssuesOrPullsWritePermission like the priority field; the /custom-type endpoint is already protected by reqRepoIssuesOrPullsWriter.

Merge pull request 'fix: render org teams list and make issue type editable (#720, #721)' (#726) from fix/team-list-and-issue-type-editable into main 5ffed39449

chore: sync auto-release.yml from Template-Generic [skip ci] 377dd019be

fix(org): layer org-level branch protection with repo rules, most-restrictive wins (#727) ... 24b3516c1d

Org-level branch protection was already consulted at the single enforcement
choke point `GetFirstMatchProtectedBranchRule`, but only as a FALLBACK: if any
repo-level rule matched the branch, the org rule was ignored entirely. That let
a repo define a looser rule for a pattern and effectively opt out of the org's
protection.

Make the choke point LAYER the two rules instead: when both an org rule and a
repo rule match a branch, return their most-restrictive (fail-closed)
combination, so the org rule is a mandatory floor a repo can only tighten.

- models/git/protected_branch_merge.go: mergeMostRestrictive + helpers. Allow
  flags AND'd; gate/require/block flags OR'd; RequiredApprovals max'd; required
  sets (status contexts, protected files) unioned; allow sets (whitelists,
  unprotected files) intersected. A disabled allowlist means "everyone", so it
  only constrains when enabled.
- models/git/protected_branch_list.go: GetFirstMatchProtectedBranchRule now
  fetches both the repo rule and the org rule and merges when both match;
  returns whichever exists when only one matches. Org lookup factored into
  getFirstMatchOrgProtectedBranchRule.

Supersedes the materialization approach previously proposed for this issue —
the org fallback already existed, so only this one function needed to change.

Fail-closed by design: any merge edge errs toward MORE protection (over-restrict)
rather than less, so it cannot open a hole.

Note: no Go toolchain available locally, so not compiled/gofmt'd/tested here —
relying on CI to validate build, formatting, and tests.

Claude-Session: https://claude.ai/code/session_01Wsno14cxE49MstXFs9G5KT

feat(org): show inherited org branch-protection rules in repo settings (#727) ... 86bd8a2cad

The org "floor" is enforced implicitly at the choke point, so a repo admin
couldn't see which org-level rules apply to their repo. Surface them in the
repo's Branch Protection settings page (read-only), the way GitHub shows
organization rulesets in a repository.

- ProtectedBranchRules handler: when the owner is an org, load
  FindOrgProtectedBranchRules and expose them as OrgProtectedBranches.
- branches.tmpl: new read-only "Organization Branch Protection" section listing
  each org rule with an "Organization" badge, a lock/read-only marker, and
  compact indicators (required approvals, signed commits, status checks). No
  edit/delete controls — these are managed at the org level.
- en-US locale strings.

Note: no Go toolchain available locally, so not compiled/gofmt'd/tested here.

Claude-Session: https://claude.ai/code/session_01Wsno14cxE49MstXFs9G5KT

feat(org): add branch-deletion protection + expandable inherited-rule view (#727) ... 4b68853f08

Two related additions:

1. Branch deletion as an org-level ability. OrgProtectedBranch gained
   CanDelete / EnableDeleteAllowlist / DeleteAllowlistTeamIDs (migration 362),
   ToProtectedBranch maps them, and the API (create/edit/response DTOs +
   handlers) exposes enable_delete / enable_delete_allowlist /
   delete_allowlist_teams. The layering merge already combined delete fields, so
   org delete-protection now enforces once ToProtectedBranch populates them.

2. The repo Branch Protection view now renders each inherited org rule as an
   expandable detail (direct push, force-push, branch deletion, merge, required
   approvals, status checks, protected files) with team names resolved, instead
   of three headline badges. Still read-only.

Note: no Go toolchain available locally, so not compiled/gofmt'd/tested here.
Verified by hand: struct-field gofmt alignment, template block nesting balances,
every .Rule field exists on OrgProtectedBranch, and all locale keys referenced
in the template are defined.

Claude-Session: https://claude.ai/code/session_01Wsno14cxE49MstXFs9G5KT

jmiller merged commit 37fb3703c7 into dev 2026-07-05 04:32:38 +00:00

jmiller deleted branch fix/727-materialize-org-branch-protection 2026-07-05 04:32:41 +00:00

jmiller referenced this pull request 2026-07-05 04:37:45 +00:00

Release: org-wide governance series (#727) — dev → main #733

jmiller referenced this pull request 2026-07-05 06:23:58 +00:00

Expand OpenAPI spec + MCP tooling for org-governance endpoints (#727 series) #738

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

Clear labels

breaking-change

Breaking API or behavior change

ci-cd

CI/CD pipeline changes

config

Configuration changes

dependencies

Dependency updates

deploy-failure

Deployment failed

docker

Docker/container changes

documentation

Documentation changes

feature: wiki

Wiki system enhancements

good first issue

Good for newcomers

health-check

Repo health check result

help wanted

Extra attention needed

mokostandards

Related to MokoStandards framework

push-failure

Git push operation failed

security

Security vulnerability or hardening

size/l

200-500 lines changed

size/m

50-200 lines changed

size/s

10-50 lines changed

size/xl

500+ lines changed

size/xs

< 10 lines changed

standards-drift

Deviates from MokoStandards

standards-update

MokoStandards compliance update

sync-failure

Sync or mirror failed

tech-debt

Technical debt and TODO/FIXME items

upstream

upstream

Inherited from upstream Gitea

work-in-progress

Draft or incomplete work

bug

Something is not working

duplicate

This issue or pull request already exists

enhancement

New feature

help wanted

Need some help

invalid

Something is wrong

question

More information is needed

wontfix

This won't be fixed

No labels

Priority -

Type -

Milestone

No items

No Milestone

Assignees

Clear assignees

jmiller (Jonathan Miller) moko-deploy (Moko Deploy Bot)

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: MokoConsulting/MokoGitea-Fork#728