@@ -0,0 +1,167 @@ |
| 1 | +# Branch protection |
| 2 | + |
| 3 | +S20 ships the branches index, tags index, compare view, and the |
| 4 | +basic branch-protection rule engine enforced by the pre-receive |
| 5 | +hook. |
| 6 | + |
| 7 | +## Routes |
| 8 | + |
| 9 | +| Route | Handler | |
| 10 | +| ------------------------------------------------------- | ----------------------------- | |
| 11 | +| `GET /{owner}/{repo}/branches?filter=active|stale|` | `branchesList` | |
| 12 | +| `GET /{owner}/{repo}/tags` | `tagsList` | |
| 13 | +| `GET /{owner}/{repo}/compare/{base}...{head}` | `compareView` | |
| 14 | +| `GET /{owner}/{repo}/settings/branches` | `settingsBranches` (auth-gated) | |
| 15 | +| `POST /{owner}/{repo}/settings/branches` | upsert rule | |
| 16 | +| `POST /{owner}/{repo}/settings/branches/{id}/delete` | delete rule | |
| 17 | +| `POST /{owner}/{repo}/settings/default-branch` | swap default | |
| 18 | + |
| 19 | +The compare route uses chi's wildcard because chi can't represent the |
| 20 | +literal `...` separator in a route param. The handler parses |
| 21 | +`<base>...<head>` (or `<base>..<head>`) server-side. Cross-repo head |
| 22 | +shape `fork:branch` is accepted but currently treated as local — the |
| 23 | +full cross-repo flow lives with S22 (PRs) + S27 (forks). |
| 24 | + |
| 25 | +## Branches list |
| 26 | + |
| 27 | +For each branch we show: |
| 28 | + |
| 29 | +- Name (linked to `/tree/<branch>`) |
| 30 | +- Last-commit subject + age (from `repogit.HeadOf`) |
| 31 | +- Ahead/behind vs the default branch (from `repogit.AheadBehind`, |
| 32 | + i.e. `git rev-list --left-right --count default...branch`) |
| 33 | +- Default badge + Protected badge (any rule whose pattern matches) |
| 34 | +- "Compare" button → `/compare/<default>...<branch>` |
| 35 | + |
| 36 | +Sort: default first, then by recent activity. Filter: `active` (within |
| 37 | +90 days), `stale` (>90 days), or all. |
| 38 | + |
| 39 | +Counts are recomputed per-request today; the spec proposes caching |
| 40 | +ahead/behind on push (S14's `push:process`). That's a perf-pass item |
| 41 | +and lives with the rest of the cache work in **S36**. |
| 42 | + |
| 43 | +## Tags list |
| 44 | + |
| 45 | +Lightweight: name, OID, subject, author age. Resolution uses |
| 46 | +`repogit.HeadOf(gitDir, "tags/<name>")` — works for both lightweight |
| 47 | +and annotated tags. The "Releases" placeholder note signals that |
| 48 | +first-class releases ship post-MVP. |
| 49 | + |
| 50 | +## Compare view |
| 51 | + |
| 52 | +Inputs: `base` and `head` (defaults to `repo.default_branch` when |
| 53 | +empty). Renders: |
| 54 | + |
| 55 | +- A summary line: ahead/behind counts plus a "Create pull request" |
| 56 | + button when `head` has commits not on `base`. |
| 57 | +- The commits-list (head-side only) via |
| 58 | + `repogit.CommitsBetween(base, head, 250)`. |
| 59 | +- The three-dot diff via S19's renderer fed from |
| 60 | + `diffsource.FromMergeBase`. |
| 61 | + |
| 62 | +Empty state ("nothing to compare") fires when `Ahead == 0`. |
| 63 | + |
| 64 | +## Branch-protection rules |
| 65 | + |
| 66 | +Stored in `branch_protection_rules`: |
| 67 | + |
| 68 | +| Column | Default | Notes | |
| 69 | +| -------------------------- | ------- | -------------------------------------------- | |
| 70 | +| `pattern` | (set) | `filepath.Match` glob | |
| 71 | +| `prevent_force_push` | `true` | enforced | |
| 72 | +| `prevent_deletion` | `true` | enforced | |
| 73 | +| `require_pr_for_push` | `false` | placeholder — column exists, no-op | |
| 74 | +| `allowed_pusher_user_ids` | `{}` | enforced; empty = no restriction | |
| 75 | +| `require_signed_commits` | `false` | placeholder — post-MVP | |
| 76 | +| `status_checks_required` | `{}` | placeholder — S24 wires real checks | |
| 77 | + |
| 78 | +### Pattern matching |
| 79 | + |
| 80 | +`filepath.Match` semantics: |
| 81 | + |
| 82 | +- `*` matches any non-separator chars (does NOT cross `/`) |
| 83 | +- `?` matches a single non-separator char |
| 84 | +- `[abc]` matches one of a/b/c |
| 85 | + |
| 86 | +`release/*` matches `release/v1.0` but NOT `release/v1.0/sub`. The |
| 87 | +matcher is unit-tested in `protection_test.go::TestMatchRule_…`. |
| 88 | + |
| 89 | +When multiple rules match a branch, the **longest pattern wins** |
| 90 | +(alphabetical tiebreaker). Document this in the settings UI when the |
| 91 | +rule list grows complex. |
| 92 | + |
| 93 | +### Enforcement (pre-receive) |
| 94 | + |
| 95 | +The pre-receive hook (`cmd/shithubd/hook.go::hookPreReceiveCmd`): |
| 96 | + |
| 97 | +1. Reads `<old> <new> <ref>` lines from stdin. |
| 98 | +2. Runs the existing S15 policy gate (suspended/archived/deleted). |
| 99 | +3. For each ref update, calls `protection.Enforce`: |
| 100 | + - Skip non-`refs/heads/*` refs (tag protection out of scope). |
| 101 | + - Resolve the longest-matching rule. |
| 102 | + - Apply the gates in order: deletion → force-push → allowed-pushers. |
| 103 | + - Return a `Decision` with `Allow`, `Reason`, and `Pattern`. |
| 104 | +4. On any `Allow=false`: write `protection.FriendlyMessage(d)` to |
| 105 | + stderr; exit non-zero. |
| 106 | + |
| 107 | +**Force-push detection** uses `git merge-base --is-ancestor old new` |
| 108 | +(via `repogit.IsAncestor`). When the old SHA is not an ancestor of |
| 109 | +the new SHA, the update is non-fast-forward → reject. |
| 110 | + |
| 111 | +**Failure mode** (DB unavailable from hook): the rule lookup returns |
| 112 | +an error; the hook prints "transient; retry" to stderr and exits |
| 113 | +non-zero. **Fail closed**, per the S20 lean: better to reject a |
| 114 | +legitimate push than to allow a force-push past a missing rule. |
| 115 | + |
| 116 | +## Default-branch change |
| 117 | + |
| 118 | +`POST /settings/default-branch` validates the target exists, updates |
| 119 | +`repos.default_branch`, then runs `git symbolic-ref HEAD refs/heads/<new>` |
| 120 | +in the bare repo so new clones pick up the new default. The DB row is |
| 121 | +the source of truth; symbolic-ref failure is logged but not rolled |
| 122 | +back (operator can re-run). |
| 123 | + |
| 124 | +Existing clones don't follow the change — that's git's nature, not a |
| 125 | +bug. The settings UI will warn on this when a fuller settings nav |
| 126 | +lands in S32. |
| 127 | + |
| 128 | +## Audit log |
| 129 | + |
| 130 | +Every protection-rule create/update/delete and default-branch change |
| 131 | +emits an audit row through the existing recorder (`audit.TargetRepo`, |
| 132 | +`ActionRepoCreated` placeholder slot — until S20-specific actions |
| 133 | +land, the meta blob carries `action: "default_branch_changed"`/ |
| 134 | +`"create"`/`"update"`/`"delete"` discriminators). |
| 135 | + |
| 136 | +## Tests |
| 137 | + |
| 138 | +- `internal/repos/protection/protection_test.go` — pattern-match |
| 139 | + precedence (longest-prefix, alphabetical tiebreak, no-cross-slash), |
| 140 | + zero-SHA detection. |
| 141 | +- `internal/repos/git/branchops_test.go` — covered by the existing |
| 142 | + log/blame test fixtures (initial commit suffices for AheadBehind + |
| 143 | + IsAncestor smoke). |
| 144 | + |
| 145 | +## Pitfalls handled |
| 146 | + |
| 147 | +- **Tag pushes**: `Enforce` skips `refs/tags/*`; rules only apply to |
| 148 | + branches. Tag protection is its own (post-MVP) concept. |
| 149 | +- **Pattern globs vs regexes**: deliberate; matches GitHub UX. |
| 150 | +- **Pre-receive cache scope**: one DB read per hook invocation. With |
| 151 | + small rule sets this is cheap. A long-lived cache would complicate |
| 152 | + invalidation; defer. |
| 153 | +- **Default-branch change while open clones**: existing clones have a |
| 154 | + stale HEAD; new clones pick up the new default. Standard git |
| 155 | + behavior; documented in the settings UI. |
| 156 | +- **DB unreachable from hook**: fail-closed reject with retry message. |
| 157 | + |
| 158 | +## Deferred to later sprints |
| 159 | + |
| 160 | +- **`require_pr_for_push` enforcement** → S22 (PR engine) wires it. |
| 161 | +- **`require_signed_commits` enforcement** → post-MVP signing surface. |
| 162 | +- **`status_checks_required`** → S24 ships the check engine. |
| 163 | +- **Required reviewers attached to rules** → S23. |
| 164 | +- **Per-team allowed-pushers** → S31 (orgs/teams). |
| 165 | +- **Tag protection** → post-MVP. |
| 166 | +- **Ahead/behind caching on push** → S36. |
| 167 | +- **Cross-repo compare full UI** → S22 + S27. |