diff --git a/pygittools/hook_samples/README.md b/pygittools/hook_samples/README.md index 4f61549..001a538 100644 --- a/pygittools/hook_samples/README.md +++ b/pygittools/hook_samples/README.md @@ -30,24 +30,6 @@ Environment variables: - `PYGITWEB_COMMIT_MSG_PATTERN` — message regex (default `^(\S+): (.+)`) - `PYGITWEB_REJECT_MERGE_COMMITS` — set to `1`/`true`/`yes` to reject merge commits -## pre-receive.branch-permissions - -Enforce per-branch write permissions based on `auth.json` `oauth_permissions`. For each -updated branch ref (`refs/heads/*`), the hook: - -- Builds a `project/branch` subject (e.g. `demo/main`, `demo/dev/alice.feature`). -- If any `pgw.write_branch.{scope}` keys match that subject, requires a matching grant. -- Otherwise, falls back to project write (`pgw.write.{project}`) for that project. - -When no usable configuration or context is available, the hook is a no-op and allows -the push to proceed. - -Environment variables: - -- `PYGITWEB_PRINCIPAL` — principal identity string (email or username) to check grants for. -- `PYGITWEB_PROJECT` — project path as known to pygitweb (e.g. `demo`, `team/app`). -- `PYGITWEB_AUTH_CONFIG` — optional auth.json path (default: `~/.pygitweb/auth.json`). - ## post-commit.push-remotes After each local commit, force-with-lease push the current branch to every configured diff --git a/pygittools/hook_samples/pre-receive.branch-permissions b/pygittools/hook_samples/pre-receive.branch-permissions deleted file mode 100644 index 070b018..0000000 --- a/pygittools/hook_samples/pre-receive.branch-permissions +++ /dev/null @@ -1,69 +0,0 @@ -#!/usr/bin/env -S uv run python - -from __future__ import annotations - -__VERSION__ = "1" - -import os -import sys -from pathlib import Path - -from pygittools.hooks import HookResult, RefUpdate, parse_ref_updates -from pygitweb.auth_config import DEFAULT_AUTH_CONFIG_PATH, AuthConfig -from pygitweb.permissions import PermissionsMap, has_branch_write_permission - - -def _normalize_env(name: str) -> str | None: - value = os.environ.get(name) - if value is None: - return None - stripped = value.strip() - return stripped or None - - -def _load_permissions() -> PermissionsMap | None: - config_path_env = _normalize_env("PYGITWEB_AUTH_CONFIG") - path = Path(config_path_env).expanduser() if config_path_env is not None else DEFAULT_AUTH_CONFIG_PATH - try: - text = path.read_text(encoding="utf-8") - except OSError: - return None - try: - config = AuthConfig.model_validate_json(text) - except Exception: - return None - return config.oauth_permissions - - -def _check_updates( - principal: str, - project: str, - updates: list[RefUpdate], - permissions: PermissionsMap, -) -> HookResult: - for _, _, ref_name in updates: - if has_branch_write_permission(permissions, principal, project, ref_name): - continue - sys.stderr.write( - f"Push rejected: {principal!r} lacks branch write permission for project {project!r} ref {ref_name!r}.\n", - ) - return HookResult.FAILURE - return HookResult.SUCCESS - - -def main() -> int: - principal = _normalize_env("PYGITWEB_PRINCIPAL") - project = _normalize_env("PYGITWEB_PROJECT") - if principal is None or project is None: - return int(HookResult.SUCCESS.value) - permissions = _load_permissions() - if permissions is None: - return int(HookResult.SUCCESS.value) - updates = parse_ref_updates(sys.stdin) - result = _check_updates(principal, project, updates, permissions) - return int(result.value) - - -if __name__ == "__main__": - raise SystemExit(main()) - diff --git a/pygitweb/auth.schema.json b/pygitweb/auth.schema.json index 18d877c..7f99ef5 100644 --- a/pygitweb/auth.schema.json +++ b/pygitweb/auth.schema.json @@ -60,7 +60,7 @@ "additionalProperties": { "type": "array", "items": { "type": "string" }, - "description": "Scoped keys: pgw.read.{scope}, pgw.write.{scope}, pgw.read_branch.{scope}, pgw.write_branch.{scope}, pgw.boards.{scope}, pgw.tasks.{scope}, pgw.comments.{scope}, pgw.hooks.{scope}, pgw.mr.create.{scope}, pgw.mr.merge.{scope} (fnmatch on project or project/branch path). Principal * matches any authenticated user." + "description": "Scoped keys: pgw.read.{scope}, pgw.write.{scope}, pgw.write_branch.{project/branch}, pgw.boards.{scope}, pgw.tasks.{scope}, pgw.comments.{scope}, pgw.hooks.{scope}, pgw.mr.create.{scope}, pgw.mr.merge.{scope} (fnmatch). Principal * matches any authenticated user." } }, "local_users": { diff --git a/pygitweb/src/pygitweb/actions.py b/pygitweb/src/pygitweb/actions.py index c07a40c..e098b4a 100644 --- a/pygitweb/src/pygitweb/actions.py +++ b/pygitweb/src/pygitweb/actions.py @@ -21,7 +21,6 @@ from pygittools.merge import MR_REF_PREFIX, MergeRequestStatus, get_merge_reques from python_ripgrep import PySortMode, PySortModeKind from python_ripgrep import search as rg_search -from pygitweb.auth import can_read_branch from pygitweb.config import BLOB_LANG, settings from pygitweb.formatting import age_string, sanitize, to_utf8 from pygitweb.git_helpers import ( @@ -1060,13 +1059,9 @@ def git_history( ) -def git_heads(project: str, token: str | None) -> HTMLResponse: +def git_heads(project: str) -> HTMLResponse: """Heads (branches) list page. Port of git_heads.""" - heads_list = git_get_heads_list(project) - if settings.AUTH: - visible_heads = [(name, ref, oid) for name, ref, oid in heads_list if can_read_branch(project, ref, token)] - else: - visible_heads = heads_list + visible_heads = git_get_heads_list(project) if not visible_heads: raise HTTPException(status_code=404, detail="No heads found") rows: list[list[Cell]] = [] diff --git a/pygitweb/src/pygitweb/auth.py b/pygitweb/src/pygitweb/auth.py index d8ab689..2a021d6 100644 --- a/pygitweb/src/pygitweb/auth.py +++ b/pygitweb/src/pygitweb/auth.py @@ -42,7 +42,11 @@ from pygitweb.auth_oauth import OAUTH_STATE_COOKIE_NAME, oauth_router from pygitweb.config import auth_config, settings from pygitweb.gravatar import gravatar_url from pygitweb.password_hash import hash_password, verify_password -from pygitweb.permissions import Permission, PermissionPrincipal, has_branch_read_permission +from pygitweb.permissions import ( + Permission, + PermissionPrincipal, + branch_permission_subject, +) from pygitweb.projects import git_get_project_owner from pygitweb.sessions import create_session, get_session, revoke_session from pygitweb.templates_env import render_page @@ -184,15 +188,22 @@ def can_write_project(project: str, token: str | None) -> bool: return has_permission(Permission.WRITE, project, token=token) -def can_read_branch(project: str, ref_name: str, token: str | None) -> bool: +def can_push_ref(project: str, principal: PermissionPrincipal, ref_name: str) -> bool: + """Branch push check. Project write is a separate gate. + + Non-branch refs are allowed. When any ``pgw.write_branch`` grant matches the + branch, only those principals may update it. Otherwise project write (including + ownership) is enough. + """ if not settings.AUTH: return True - if _owner_matches(project, token): + subject = branch_permission_subject(project, ref_name) + if subject is None: return True - identity = _principal_identity_for_permissions(token) - if identity is None: - return False - return has_branch_read_permission(auth_config.oauth_permissions, identity, project, ref_name) + permissions = auth_config.oauth_permissions + if permissions.matching_scoped_principals(Permission.WRITE_BRANCH, subject): + return permissions.has_permission(principal.identity, Permission.WRITE_BRANCH, subject) + return principal_can_write_project(project, principal) def ensure_read_project(project: str, token: str | None) -> None: diff --git a/pygitweb/src/pygitweb/hooks_install.py b/pygitweb/src/pygitweb/hooks_install.py index 8460684..2470448 100644 --- a/pygitweb/src/pygitweb/hooks_install.py +++ b/pygitweb/src/pygitweb/hooks_install.py @@ -34,7 +34,6 @@ _SAMPLE_LABELS: dict[str, str] = { "pre-commit.ruff": "Ruff Pre-commit", "commit-msg.pattern": "Commit Message Pattern", "pre-receive.protected-pattern": "Protected Branch Commit Messages", - "pre-receive.branch-permissions": "Branch Write Permissions", "post-commit.push-remotes": "Post-commit Push Remotes", } diff --git a/pygitweb/src/pygitweb/main.py b/pygitweb/src/pygitweb/main.py index 8b22b34..238544f 100644 --- a/pygitweb/src/pygitweb/main.py +++ b/pygitweb/src/pygitweb/main.py @@ -955,7 +955,7 @@ async def dispatch( p, pc = parse_pagination(page, pagecount) return git_history(project, hash_param, file_name, request, p, pc) if action == "heads": - return git_heads(project, token) + return git_heads(project) if action == "tags": p, pc = parse_pagination(page, pagecount) return git_tags(project, request, p, pc) diff --git a/pygitweb/src/pygitweb/permissions.py b/pygitweb/src/pygitweb/permissions.py index ea082f5..36dd6a1 100644 --- a/pygitweb/src/pygitweb/permissions.py +++ b/pygitweb/src/pygitweb/permissions.py @@ -7,6 +7,7 @@ every permission. Scoped keys use a project segment that supports ``fnmatch`` globs (``*``, ``?``, ``[seq]``), e.g. ``pgw.tasks.foo/*``, ``pgw.read.*``, or ``pgw.write.myproj``. +``pgw.write_branch`` scopes are ``project/branch`` (for example ``demo/main``). Principal ``*`` in a grant list matches any authenticated principal. """ @@ -28,7 +29,7 @@ PERMISSION_ADD_PROJECTS = "pgw.addprojects" PERMISSION_SETTINGS = "pgw.settings" _SCOPED_PREFIX = re.compile( - r"^pgw\.(boards|tasks|comments|hooks|mr\.create|mr\.merge|read|write|read_branch|write_branch)\.(.+)$", + r"^pgw\.(boards|tasks|comments|hooks|mr\.create|mr\.merge|read|write|write_branch)\.(.+)$", ) _BRANCH_REF_PREFIX = "refs/heads/" @@ -39,7 +40,6 @@ class Permission(StrEnum): READ = "read" WRITE = "write" - READ_BRANCH = "read_branch" WRITE_BRANCH = "write_branch" BOARDS = "boards" TASKS = "tasks" @@ -63,7 +63,6 @@ class Permission(StrEnum): if self in ( Permission.READ, Permission.WRITE, - Permission.READ_BRANCH, Permission.WRITE_BRANCH, Permission.BOARDS, Permission.TASKS, @@ -137,21 +136,6 @@ def branch_permission_subject(project: str, ref_name: str) -> str | None: return f"{project}/{short_name}" -def has_branch_read_permission( - permissions: PermissionsMap, - principal: str | None, - project: str, - ref_name: str, -) -> bool: - subject = branch_permission_subject(project, ref_name) - if subject is None: - return True - branch_specific = permissions.matching_scoped_principals(Permission.READ_BRANCH, subject) - if branch_specific: - return permissions.has_permission(principal, Permission.READ_BRANCH, subject) - return permissions.has_permission(principal, Permission.READ, project) - - def has_branch_write_permission( permissions: PermissionsMap, principal: str | None, diff --git a/pygitweb/src/pygitweb/permissions_test.py b/pygitweb/src/pygitweb/permissions_test.py index 192f80b..74b0f5b 100644 --- a/pygitweb/src/pygitweb/permissions_test.py +++ b/pygitweb/src/pygitweb/permissions_test.py @@ -6,7 +6,6 @@ from pygitweb.permissions import ( Permission, PermissionsMap, branch_permission_subject, - has_branch_read_permission, has_branch_write_permission, parse_permission_key, permission_key, @@ -59,14 +58,11 @@ def test_parse_mr_scoped_keys() -> None: def test_parse_branch_scoped_keys() -> None: - read = parse_permission_key("pgw.read_branch.team/app-main") - assert read is not None - assert read.permission is Permission.READ_BRANCH - assert read.project_scope == "team/app-main" write = parse_permission_key("pgw.write_branch.dev/alice.*") assert write is not None assert write.permission is Permission.WRITE_BRANCH assert write.project_scope == "dev/alice.*" + assert parse_permission_key("pgw.read_branch.team/app-main") is None def test_project_glob() -> None: @@ -180,22 +176,3 @@ def test_has_branch_write_permission_enforces_branch_specific_rules() -> None: }) assert has_branch_write_permission(m, "branch-writer@example.com", "demo", "refs/heads/main") assert not has_branch_write_permission(m, "writer@example.com", "demo", "refs/heads/main") - - -def test_has_branch_read_permission_falls_back_to_project_scope() -> None: - m = PermissionsMap.model_validate({ - "*": [], - "pgw.read.demo": ["reader@example.com"], - }) - assert has_branch_read_permission(m, "reader@example.com", "demo", "refs/heads/main") - assert not has_branch_read_permission(m, "other@example.com", "demo", "refs/heads/main") - - -def test_has_branch_read_permission_enforces_branch_specific_rules() -> None: - m = PermissionsMap.model_validate({ - "*": [], - "pgw.read.demo": ["reader@example.com"], - "pgw.read_branch.demo/main": ["branch-reader@example.com"], - }) - assert has_branch_read_permission(m, "branch-reader@example.com", "demo", "refs/heads/main") - assert not has_branch_read_permission(m, "reader@example.com", "demo", "refs/heads/main") diff --git a/pygitweb/src/pygitweb/smart_http.py b/pygitweb/src/pygitweb/smart_http.py index 0daa29f..af918aa 100644 --- a/pygitweb/src/pygitweb/smart_http.py +++ b/pygitweb/src/pygitweb/smart_http.py @@ -4,6 +4,7 @@ from __future__ import annotations import gzip import os +import re import subprocess import sys import threading @@ -18,6 +19,7 @@ from fastapi.responses import Response from pygitweb.auth import ( ACCESS_TOKEN_COOKIE_NAME, SmartHttpCredentials, + can_push_ref, can_read_project, load_smart_http_credentials, principal_can_read_project, @@ -35,6 +37,7 @@ http_router = APIRouter(prefix="/http", tags=["smart_http"]) _SMART_HTTP_LOG_LOCK = threading.Lock() _SMART_HTTP_LOG_MODE = 0o600 +_REF_UPDATE_LINE = re.compile(rb"^[0-9a-fA-F]{4,64} [0-9a-fA-F]{4,64} (\S+)$") _SERVICE_CONTENT_TYPES: dict[SmartHttpService, tuple[str, str]] = { "git-upload-pack": ( @@ -125,7 +128,7 @@ def append_smart_http_log(ip: str, actor: str, *, project: str, operation: str) os.chmod(path, _SMART_HTTP_LOG_MODE) -def _begin_smart_http(request: Request, project: str, *, write: bool, operation: str) -> None: +def _begin_smart_http(request: Request, project: str, *, write: bool, operation: str) -> SmartHttpCredentials: creds = load_smart_http_credentials( request.headers.get("authorization"), request.cookies.get(ACCESS_TOKEN_COOKIE_NAME), @@ -135,6 +138,54 @@ def _begin_smart_http(request: Request, project: str, *, write: bool, operation: append_smart_http_log(_request_ip(request), creds.actor, project=project, operation=operation) raise_for_smart_http_credentials(creds) _authorize_smart_http(project, creds, write=write) + return creds + + +def receive_pack_updated_refs(body: bytes) -> list[str]: + """Ref names from a stateless ``git-receive-pack`` command list. + + Stops at the first flush pkt, which separates commands from the pack. Lines that + are not `` `` (capability text, push certificates) are ignored. + """ + refs: list[str] = [] + offset = 0 + total = len(body) + while offset + 4 <= total: + header = body[offset : offset + 4] + try: + size = int(header, 16) + except ValueError as exc: + raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail="Invalid receive-pack request") from exc + if size == 0: + return refs + if size < 4 or offset + size > total: + raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail="Invalid receive-pack request") + payload = body[offset + 4 : offset + size].split(b"\0", 1)[0].rstrip(b"\n") + offset += size + if not payload: + continue + match = _REF_UPDATE_LINE.match(payload) + if match is None: + continue + try: + refs.append(match.group(1).decode("utf-8")) + except UnicodeDecodeError as exc: + raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail="Invalid receive-pack request") from exc + if offset != total: + raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail="Invalid receive-pack request") + return refs + + +def _enforce_branch_pushes(project: str, creds: SmartHttpCredentials, body: bytes) -> None: + if not creds.required or creds.principal is None: + return + for ref_name in receive_pack_updated_refs(body): + if can_push_ref(project, creds.principal, ref_name): + continue + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail=f"Insufficient permissions for {ref_name}", + ) def _authorize_smart_http(project: str, creds: SmartHttpCredentials, *, write: bool) -> None: @@ -266,9 +317,10 @@ async def smart_http_receive_pack( content_encoding: Annotated[str | None, Header(alias="Content-Encoding")] = None, git_protocol: Annotated[str | None, Header(alias="Git-Protocol")] = None, ) -> Response: - _begin_smart_http(request, project, write=True, operation="git-receive-pack") + creds = _begin_smart_http(request, project, write=True, operation="git-receive-pack") raw_body = await request.body() body = _decode_request_body(raw_body, content_encoding) + _enforce_branch_pushes(project, creds, body) result = _run_git_service( "git-receive-pack", project, diff --git a/pygitweb/src/pygitweb/smart_http_test.py b/pygitweb/src/pygitweb/smart_http_test.py index 32649fb..80a0ff8 100644 --- a/pygitweb/src/pygitweb/smart_http_test.py +++ b/pygitweb/src/pygitweb/smart_http_test.py @@ -35,6 +35,7 @@ from pygitweb.smart_http import ( _pkt_line, _service_advertisement, append_smart_http_log, + receive_pack_updated_refs, smart_http_log_path, ) @@ -81,6 +82,18 @@ def _user_with_keys(*, secret: str | None = None, salt: str = "") -> AuthConfig: ) +def _push_commands(*refs: str) -> bytes: + zero = b"0" * 40 + new = b"a" * 40 + body = b"" + for index, ref in enumerate(refs): + line = zero + b" " + new + b" " + ref.encode("ascii") + if index == 0: + line += b"\0report-status" + body += _pkt_line(line + b"\n") + return body + _pkt_flush() + + @contextmanager def _auth_enabled(cfg: AuthConfig) -> Generator[None, None, None]: with ( @@ -185,6 +198,12 @@ class TestSmartHttpHelpers: assert _pkt_line(b"hello\n") == b"000ahello\n" assert _pkt_flush() == b"0000" + def test_receive_pack_updated_refs_stops_at_flush(self) -> None: + body = _push_commands("refs/heads/main", "refs/tags/v1") + b"PACK leftover" + assert receive_pack_updated_refs(body) == ["refs/heads/main", "refs/tags/v1"] + cert = _pkt_line(b"push-cert\0report-status\n") + _pkt_line(b"certificate version 0.1\n") + assert receive_pack_updated_refs(cert + body) == ["refs/heads/main", "refs/tags/v1"] + def test_decode_request_body(self) -> None: raw = b"0014command=ls-refs\n0000" assert _decode_request_body(raw, None) == raw @@ -409,6 +428,64 @@ class TestSmartHttpRoutes: assert fetch.status_code == 404 assert push.status_code == 403 + def test_branch_push_permission_on_receive_pack(self, http_repo_env: dict[str, str]) -> None: + restricted = PermissionsMap.model_validate({ + "*": [], + "pgw.read.*": ["*"], + "pgw.write.*": ["*"], + "pgw.write_branch.demo/main": ["other@example.com"], + }) + allowed = PermissionsMap.model_validate({ + "*": [], + "pgw.read.*": ["*"], + "pgw.write.*": ["*"], + "pgw.write_branch.demo/main": ["alice@example.com"], + }) + client = TestClient(app) + denied_body = _push_commands("refs/heads/main") + with ( + _auth_enabled(_user_with_keys().model_copy(update={"oauth_permissions": restricted})), + patch("pygitweb.smart_http._run_git_service", return_value=b"") as run, + ): + denied = client.post( + f"/http/{http_repo_env['project']}/git-receive-pack", + content=denied_body, + auth=("alice", _WRITE_KEY), + ) + tag = client.post( + f"/http/{http_repo_env['project']}/git-receive-pack", + content=_push_commands("refs/tags/v1"), + auth=("alice", _WRITE_KEY), + ) + assert denied.status_code == 403 + assert "refs/heads/main" in denied.json()["detail"] + assert tag.status_code == 200 + assert run.call_count == 1 + with ( + _auth_enabled(_user_with_keys().model_copy(update={"oauth_permissions": allowed})), + patch("pygitweb.smart_http._run_git_service", return_value=b"") as run_allowed, + ): + accepted = client.post( + f"/http/{http_repo_env['project']}/git-receive-pack", + content=denied_body, + auth=("alice", _WRITE_KEY), + ) + assert accepted.status_code == 200 + assert run_allowed.call_count == 1 + with ( + _auth_enabled(_user_with_keys().model_copy(update={"oauth_permissions": allowed})), + patch("pygitweb.smart_http._run_git_service", return_value=b""), + ): + client.cookies.set( + ACCESS_TOKEN_COOKIE_NAME, + create_session("oauthuser", email="alice@example.com", auth_method="oauth"), + ) + oauth_push = client.post( + f"/http/{http_repo_env['project']}/git-receive-pack", + content=denied_body, + ) + assert oauth_push.status_code == 200 + def test_clone_and_push_over_http(self, http_repo_env: dict[str, str]) -> None: with _http_server() as port: clone_dir = Path(http_repo_env["root"]) / "clone"