@@ -0,0 +1,165 @@ |
| | 1 | +# Permissions |
| | 2 | + |
| | 3 | +Every authorization decision in shithub flows through one function: |
| | 4 | +`policy.Can(ctx, deps, actor, action, repo) → Decision`. Handlers, |
| | 5 | +hooks, and the SSH/HTTP git transports all funnel through this single |
| | 6 | +entrypoint. No surface reads ownership, visibility, or collaborator |
| | 7 | +state inline; the lint guard at `scripts/lint-policy-boundary.sh` |
| | 8 | +enforces the boundary in CI. |
| | 9 | + |
| | 10 | +## The shape |
| | 11 | + |
| | 12 | +* **Actor** — who's asking. Anonymous, logged-in user (with `IsSuspended` |
| | 13 | + and `IsSiteAdmin` flags), or future org-team principal (S31). |
| | 14 | +* **Action** — what they want to do, drawn from the constant registry |
| | 15 | + in `internal/auth/policy/actions.go`. New actions go in their owning |
| | 16 | + sprint's PR; the matrix test ensures every constant is covered for |
| | 17 | + every actor archetype. |
| | 18 | +* **Resource** — currently a `RepoRef`; issue/pull/org refs land in |
| | 19 | + their owning sprints. The `RepoRef` carries `OwnerOrgID` shape today |
| | 20 | + (zero) so S31 plugs in without retro-fitting the interface. |
| | 21 | +* **Decision** — `{Allow bool; Reason string; Code DenyCode}`. Allow |
| | 22 | + drives control flow; Code lets handlers pick a friendly user-facing |
| | 23 | + message without re-deriving from the resource fields. Reason is for |
| | 24 | + logs and tests, never end-user surfaces. |
| | 25 | + |
| | 26 | +## Role hierarchy |
| | 27 | + |
| | 28 | +Five collaborator tiers, mirroring GitHub: |
| | 29 | + |
| | 30 | +| Role | Implies | Granted by | |
| | 31 | +| ---------- | ------------------------------------------------- | ------------------------- | |
| | 32 | +| `read` | Clone/fetch a private repo, view issues/pulls | invitation by maintainer | |
| | 33 | +| `triage` | read + close/label/assign issues, no code write | invitation by maintainer | |
| | 34 | +| `write` | triage + push, branch create, PR create/comment | invitation by maintainer | |
| | 35 | +| `maintain` | write + most settings (general, branches) | invitation by admin | |
| | 36 | +| `admin` | maintain + delete/transfer/visibility/destructive | invitation by admin/owner | |
| | 37 | + |
| | 38 | +The repo **owner** is implicit `admin` — there's no |
| | 39 | +`repo_collaborators` row for them. `repo_collaborators` lives in |
| | 40 | +migration `0019` and is keyed by `(repo_id, user_id)`. |
| | 41 | + |
| | 42 | +## Action → minimum role table |
| | 43 | + |
| | 44 | +The complete map (also enforced by the matrix test): |
| | 45 | + |
| | 46 | +| Action | Min role on repo | |
| | 47 | +| ------------------------------------- | ---------------- | |
| | 48 | +| `repo:read` | `read` (private) | |
| | 49 | +| `repo:write` | `write` | |
| | 50 | +| `repo:admin` | `admin` | |
| | 51 | +| `repo:settings:general` | `maintain` | |
| | 52 | +| `repo:settings:collaborators` | `admin` | |
| | 53 | +| `repo:settings:branches` | `maintain` | |
| | 54 | +| `repo:archive` | `admin` | |
| | 55 | +| `repo:delete` | `admin` | |
| | 56 | +| `repo:transfer` | `admin` | |
| | 57 | +| `repo:visibility` | `admin` | |
| | 58 | +| `issue:read` | `read` (private) | |
| | 59 | +| `issue:create` | `write` | |
| | 60 | +| `issue:comment` | `write` | |
| | 61 | +| `issue:close` | `triage` | |
| | 62 | +| `issue:label` | `triage` | |
| | 63 | +| `issue:assign` | `triage` | |
| | 64 | +| `pull:read` | `read` (private) | |
| | 65 | +| `pull:create` | `write` | |
| | 66 | +| `pull:merge` | `admin` | |
| | 67 | +| `pull:review` | `write` | |
| | 68 | +| `pull:close` | `write` | |
| | 69 | +| `star:create` | logged in | |
| | 70 | +| `fork:create` | logged in | |
| | 71 | + |
| | 72 | +Read actions on **public** repos are short-circuited to allow before the |
| | 73 | +role check — anyone (anonymous or otherwise) can read a public repo. |
| | 74 | + |
| | 75 | +## Decision precedence |
| | 76 | + |
| | 77 | +`Can()` evaluates in a fixed order; the first matching rule produces |
| | 78 | +the verdict. Ordered from most-decisive to least: |
| | 79 | + |
| | 80 | +1. **Soft-deleted repo** → deny (`DenyRepoDeleted`). Nothing else |
| | 81 | + matters. |
| | 82 | +2. **Site-admin + read action** → allow. (Write actions still go |
| | 83 | + through the rest of the pipeline; broad admin overrides hide bugs |
| | 84 | + and create insider-threat surface.) |
| | 85 | +3. **Suspended actor + write action** → deny (`DenyActorSuspended`). |
| | 86 | + Reads against public repos still allowed. |
| | 87 | +4. **Anonymous + private repo** → deny (`DenyVisibility`). Caller maps |
| | 88 | + to 404, not 403, to avoid existence leak. |
| | 89 | +5. **Public repo + read** → allow. |
| | 90 | +6. Compute effective role (owner ⇒ admin; collaborator ⇒ row.role; |
| | 91 | + else `RoleNone`). |
| | 92 | +7. **Archived repo + write** → deny (`DenyArchived`). Even owners |
| | 93 | + can't push to archived repos. |
| | 94 | +8. **Min role for action** vs effective role. Below threshold + private |
| | 95 | + repo + no role → deny as visibility (404). Below threshold with any |
| | 96 | + role → deny as `DenyRoleTooLow` (403). |
| | 97 | +9. **Login-required actions** (star/fork) on anonymous → deny |
| | 98 | + (`DenyAnonymous`). |
| | 99 | + |
| | 100 | +## Existence-leak guard |
| | 101 | + |
| | 102 | +`policy.Maybe404(decision, repo, actor)` maps a denial to a status |
| | 103 | +code that doesn't reveal whether a private repo exists. Convention: |
| | 104 | + |
| | 105 | +* Allow → 200. |
| | 106 | +* Deny on a private repo, viewer is not the owner → **404**. |
| | 107 | +* Deny on a private repo, viewer **is** the owner (e.g. push to |
| | 108 | + archived) → **403**, since the owner already knows the repo exists. |
| | 109 | +* Deny on a public repo → **403**. |
| | 110 | + |
| | 111 | +Handlers that care about user-facing message tone (e.g. the HTTP git |
| | 112 | +handler's "repository is archived" stderr line) should switch on |
| | 113 | +`Decision.Code` rather than parse `Decision.Reason`. |
| | 114 | + |
| | 115 | +## Per-request memoization |
| | 116 | + |
| | 117 | +`policy.WithCache(ctx)` attaches a request-scoped memo. The web layer |
| | 118 | +wires this in `internal/web/middleware/policy.go::PolicyCache`. Within |
| | 119 | +one request, repeated `Can()` calls for the same `(actor, repo)` pair |
| | 120 | +hit the cache. Across requests there's no cache — staleness is hard to |
| | 121 | +get right and the per-request DB cost of fresh lookups is acceptable. |
| | 122 | + |
| | 123 | +If a handler mutates collaborator state mid-request and re-checks |
| | 124 | +policy in the same flight, call `policy.InvalidateRepo(ctx, repoID)` |
| | 125 | +between the mutation and the re-check. |
| | 126 | + |
| | 127 | +## Site-admin scope |
| | 128 | + |
| | 129 | +`actor.IsSiteAdmin = true` short-circuits to allow on read actions |
| | 130 | +only. Write actions go through the normal role check, which means a |
| | 131 | +site admin who is **not** a collaborator on a private repo cannot |
| | 132 | +push or change settings without explicit impersonation (S34 ships the |
| | 133 | +impersonation surface). Non-impersonated admin write attempts go |
| | 134 | +through Can() like any other request and audit-log loudly via the S05 |
| | 135 | +audit recorder. |
| | 136 | + |
| | 137 | +## Adding a new action |
| | 138 | + |
| | 139 | +1. Add the constant to `internal/auth/policy/actions.go`. |
| | 140 | +2. Append it to `AllActions` in the same file. |
| | 141 | +3. Add a case to `minRoleFor(action)` in `policy.go`. Unknown actions |
| | 142 | + default to `RoleAdmin` (deny by default for strangers). |
| | 143 | +4. The matrix test (`policy_test.go`) iterates `AllActions` and will |
| | 144 | + automatically demand a verdict for every actor × resource × this |
| | 145 | + action combination. Update `mirrorMinRoleFor` in the test file with |
| | 146 | + the same minimum role. |
| | 147 | + |
| | 148 | +If you add an action that involves a new resource type, add a `*Ref` |
| | 149 | +struct in `resources.go` and an adapter in `adapters.go`, then |
| | 150 | +overload `Can` with a new entrypoint (e.g. `CanIssue`, `CanOrg`). |
| | 151 | + |
| | 152 | +## Boundary lint |
| | 153 | + |
| | 154 | +`scripts/lint-policy-boundary.sh` runs as part of `make ci`. It fails |
| | 155 | +when the following patterns appear outside `internal/auth/policy/`, |
| | 156 | +`internal/repos/`, and `internal/web/handlers/repo/` (which constructs |
| | 157 | +the policy actor at the lookup wrapper): |
| | 158 | + |
| | 159 | +* `OwnerUserID == ` or `== row.OwnerUserID` — direct owner equality |
| | 160 | +* `Visibility == reposdb.Repo...` — direct visibility branching |
| | 161 | +* `if X.IsArchived` — archived as a control predicate |
| | 162 | + |
| | 163 | +Test files everywhere are exempt — they legitimately seed state. If a |
| | 164 | +new pattern surfaces (e.g. an issue handler reads `issue.author_id`), |
| | 165 | +extend the script accordingly. |