diff --git a/README.md b/README.md index bc5cc02d..0a8c85b0 100644 --- a/README.md +++ b/README.md @@ -16,7 +16,7 @@ The platform exposes four subsystems: - **Actions** — outbound SCM operations initiated by Sentry code. The `SourceCodeManager` class provides 70+ methods covering comments, reactions, pull requests, branches, git objects, reviews, and check runs. With more actions planned to be added as we port more use cases. - **Actions RPC** - outbound SCM commands exposed over the network. The `SourceCodeManager` is fully available over the network. This enables usage of the SCM for services outside the monolith. -- **Git relay** - `git` fetches and pushes for services that hold no provider credentials. Seer signs a grant naming the organization, operation and repositories with `scm.git.grant.sign_grant` and an Ed25519 private key only Seer holds. `scm.git.server.GitServer` verifies the grant against the matching public keys, so whoever verifies grants cannot mint one, and checks each git smart-HTTP request against it before relaying the request to the provider with a token, streaming both bodies. A missing or unusable key on either side raises `scm.errors.GitGrantKeyError`, the server's misconfiguration rather than a refused grant. `require_private_key` and `require_public_keys` check the keys at startup, and `generate_private_key` makes a key for tests and local tools. `key_id` gives a public key's `kid`, the id each grant names its signing key by, so a log can name the key that signed a grant without quoting it. +- **Git relay** - `git` fetches and pushes for services that hold no provider credentials. Seer signs a grant with `scm.git.grant.sign_grant` and an Ed25519 private key only Seer holds. A `Grant` names the organization and a list of rules, each a repository, an operation (`FetchRule` or `PushRule`) and the branches and tags it may touch, or `any_object` for a fetch that may read any object. Both kinds of rule name branches by `branches` or full-match `branch_patterns`, and tags by `tags` or `tag_patterns`, and a push rule's `create_only` limits it to new refs. A request passes if any rule allows it. `scm.git.server.GitServer` verifies the grant against the matching public keys, so whoever verifies grants cannot mint one, and checks each git smart-HTTP request against it before relaying the request to the provider with a token, streaming both bodies. For a push under a push rule it first reads the ref updates, and answers a push that updates any other ref, deletes one or carries push options with a status report git shows as rejected refs. A missing or unusable key on either side raises `scm.errors.GitGrantKeyError`, the server's misconfiguration rather than a refused grant. `require_private_key` and `require_public_keys` check the keys at startup, and `generate_private_key` makes a key for tests and local tools. `key_id` gives a public key's `kid`, the id each grant names its signing key by, so a log can name the key that signed a grant without quoting it. - **Event Stream** — inbound webhook processing. SCM service-providers push events which are deserialized into typed, provider-neutral dataclasses (`CheckRunEvent`, `CommentEvent`, `PullRequestEvent`) which are then dispatched to registered listener functions. ### Why diff --git a/bin/git-server b/bin/git-server index 6096ef24..280278da 100755 --- a/bin/git-server +++ b/bin/git-server @@ -50,14 +50,15 @@ Configuration: GITLAB_BASE_URL Instance URL (default: https://gitlab.com) Serving: - The server prints a fetch grant and a push grant for the repository, valid - for an hour, with commands that pass them to git: + The server prints a grant to fetch any object of the repository and one to + push its branch my-branch, valid for an hour, with commands that pass them + to git: git -c http.extraHeader="X-Scm-Git-Grant: $FETCH_GRANT" \\ clone http://127.0.0.1:8080/OWNER/REPO.git The URL path is the repository's path, which the server uses as its external - id, so a grant names the repository as (provider, path). + id, so a grant's rule names the repository as (provider, path). Checks: --check starts the server on a free port and runs git against it: @@ -68,20 +69,27 @@ Checks: for credentials - a repository the token cannot see fails with the provider's own message, which the relay passes on - With --push it also commits a 256 KiB file on a new branch, pushes it with - a push grant, sending the pack chunked as git does for a large push, and - then deletes the branch. This writes to the repository, so point it at a - test repository. + With --push it also commits a 256 KiB file, pushes it with a push grant for + a new branch, sending the pack chunked as git does for a large push, then + tags the commit under the same grant. It checks that the grant can neither + push another branch or tag nor delete its own, that the provider refuses a + forged create of the existing branch and tag under a create_only rule, and + that it keeps each apart from a name that differs only in case. It then + deletes the branch, the tag and their case variants with the provider + token. This writes to the repository, so point it at a test repository. git runs without the user's or the system's git config, so credential helpers and URL rewrites cannot hide a failure. """ import argparse +import base64 +import hashlib import importlib.machinery import importlib.util import logging import os +import re import secrets import subprocess import sys @@ -95,15 +103,17 @@ from pathlib import Path from types import ModuleType from urllib.parse import urlsplit +import requests + from scm import Secret from scm.errors import GitGrantKeyError from scm.git.grant import generate_private_key, public_key_of, sign_grant from scm.git.relay import GitRelay from scm.git.server import GitServer -from scm.git.types import GRANT_HEADER, GetToken, GitOperation, Grant, Readable +from scm.git.types import GRANT_HEADER, FetchRule, GetToken, Grant, PushRule, Readable from scm.providers.github.provider import GitHubProvider from scm.providers.gitlab.provider import GitLabProvider -from scm.types import Provider, ProviderName, Repository, RepositoryId +from scm.types import GetGitRemoteProtocol, Provider, ProviderName, Repository, RepositoryId logger = logging.getLogger(__name__) @@ -278,17 +288,20 @@ def make_git_server( ) -def grant_for(private_key: Secret, operation: GitOperation, repository_id: RepositoryId) -> str: +def grant_for(private_key: Secret, rule: FetchRule | PushRule) -> str: grant = Grant( organization_id=ORGANIZATION_ID, referrer=REFERRER, - operation=operation, - repositories=[repository_id], + rules=[rule], expires_at=int(time.time()) + GRANT_TTL, ) return sign_grant(private_key, grant) +def fetch_grant_for(private_key: Secret, repository_id: RepositoryId) -> str: + return grant_for(private_key, FetchRule(repository=repository_id, any_object=True)) + + class GitHandler(BaseHTTPRequestHandler): """Serves /.git/, as Sentry's endpoint serves a repository's git routes. @@ -317,8 +330,7 @@ class GitHandler(BaseHTTPRequestHandler): elif self.headers.get("Transfer-Encoding", "").lower() == "chunked": body = ChunkedBody(self.rfile) else: - # The relay reads no more than Content-Length from it. - body = self.rfile + body = SizedBody(self.rfile, self.headers.get("Content-Length")) try: response = self.server.git.handle( method=self.command, @@ -365,6 +377,29 @@ class GitHTTPServer(ThreadingHTTPServer): return f"http://{host!s}:{port}" +class SizedBody: + """A request body of known length, ending where the body does, as GitServer reads it. + + Read on, the connection's stream would wait for a next request. A Content-Length that isn't a length reads as an + empty body, and the relay refuses the header itself. + """ + + def __init__(self, rfile, content_length: str | None) -> None: + self.rfile = rfile + try: + self.remaining = max(int(content_length or 0), 0) + except ValueError: + self.remaining = 0 + + def read(self, size: int = -1, /) -> bytes: + size = self.remaining if size < 0 else min(size, self.remaining) + if size <= 0: + return b"" + data = self.rfile.read(size) + self.remaining -= len(data) + return data + + class ChunkedBody: """A chunked request body, decoded as the relay reads it. @@ -408,8 +443,8 @@ def serve(backend: Backend, args: argparse.Namespace) -> None: (args.host, args.port), make_git_server(backend, args.public_key, args.ca_bundle), backend.provider_name ) url = f"{server.url}/{backend.repository['name']}.git" - fetch_grant = grant_for(args.private_key, "fetch", backend.repository_id) - push_grant = grant_for(args.private_key, "push", backend.repository_id) + fetch_grant = fetch_grant_for(args.private_key, backend.repository_id) + push_grant = grant_for(args.private_key, PushRule(repository=backend.repository_id, branches=["my-branch"])) logger.info( f"Serving {backend.provider_name} repository {backend.repository['name']} at {url}\n\n" f"Grants, valid for {GRANT_TTL // 60} minutes:\n" @@ -474,11 +509,19 @@ class Checks: # git shows the text of a refused first request as "remote: ". return self.check(name, result, result.returncode != 0 and f"remote: {message}" in result.stderr) + def expect_rejection(self, name: str, result: subprocess.CompletedProcess[str], reason: str) -> bool: + # git shows a ref the server's status report refuses as "! [remote rejected] ()". + rejected = "[remote rejected]" in result.stderr and f"({reason})" in result.stderr + return self.check(name, result, result.returncode != 0 and rejected) + def run(self) -> int: repository_id = self.backend.repository_id other_repository_id = (self.backend.provider_name, f"{self.backend.repository['name']}-other") - fetch_grant = grant_for(self.args.private_key, "fetch", repository_id) - push_grant = grant_for(self.args.private_key, "push", repository_id) + branch = f"scm-git-server-check/{int(time.time())}" + fetch_grant = fetch_grant_for(self.args.private_key, repository_id) + push_grant = grant_for( + self.args.private_key, PushRule(repository=repository_id, branches=[branch], tags=[f"{branch}-tag"]) + ) server = self.start() url = f"{server.url}/{self.backend.repository['name']}.git" @@ -490,16 +533,15 @@ class Checks: cloned = self.expect_success( "clone with a fetch grant", self.git(fetch_grant, "clone", "--depth=1", url, str(clone)) ) - branch = f"scm-git-server-check/{int(time.time())}" self.expect_refusal( "push with a fetch grant is refused", # Refused before the provider sees it, so any local repository will do. self.git(fetch_grant, "push", url, f"HEAD:refs/heads/{branch}", cwd=self.local_repository()), - "The git grant allows fetch, not push.", + "The git grant allows fetch on this repository, not push.", ) self.expect_refusal( "a grant for another repository is refused", - self.git(grant_for(self.args.private_key, "fetch", other_repository_id), "ls-remote", url), + self.git(fetch_grant_for(self.args.private_key, other_repository_id), "ls-remote", url), "The git grant does not cover this repository.", ) server.shutdown() @@ -519,7 +561,7 @@ class Checks: missing = self.backend.repository_at(f"{self.backend.repository['name']}-does-not-exist") server = self.start(repository=missing) result = self.git( - grant_for(self.args.private_key, "fetch", repository_id_of(self.backend.provider_name, missing)), + fetch_grant_for(self.args.private_key, repository_id_of(self.backend.provider_name, missing)), "ls-remote", f"{server.url}/{missing['name']}.git", ) @@ -557,17 +599,127 @@ class Checks: (clone / "scm-git-server-check.bin").write_bytes(secrets.token_bytes(256 * 1024)) self.git(push_grant, "add", "scm-git-server-check.bin", cwd=clone).check_returncode() self.git(push_grant, "commit", "-m", "scm-platform git-server check", cwd=clone).check_returncode() + self.expect_rejection( + "chunked push to a branch the grant does not list is rejected", + self.git( + push_grant, "-c", "http.postBuffer=65536", "push", url, f"HEAD:refs/heads/{branch}-unlisted", cwd=clone + ), + "the git grant does not cover this branch", + ) pushed = self.expect_success( "chunked push with a push grant", self.git(push_grant, "-c", "http.postBuffer=65536", "push", url, f"HEAD:refs/heads/{branch}", cwd=clone), ) if pushed: - self.expect_success( - f"delete the pushed branch {branch}", - self.git(push_grant, "push", url, f":refs/heads/{branch}", cwd=clone), + tip = self.git(push_grant, "rev-parse", "HEAD", cwd=clone).stdout.strip() + # The commit the clone started from, which the provider has on the default branch. + base = self.git(push_grant, "rev-parse", "HEAD~1", cwd=clone).stdout.strip() + self.check_pushed_ref(url, push_grant, clone, f"refs/heads/{branch}", tip, base) + tag = f"refs/tags/{branch}-tag" + self.expect_rejection( + "push to a tag the grant does not list is rejected", + self.git(push_grant, "push", url, f"HEAD:{tag}-unlisted", cwd=clone), + "the git grant does not cover this tag", ) + if self.expect_success( + "push a tag with the push grant", self.git(push_grant, "push", url, f"HEAD:{tag}", cwd=clone) + ): + self.check_pushed_ref(url, push_grant, clone, tag, tip, base) server.shutdown() + def check_pushed_ref(self, url: str, push_grant: str, clone: Path, ref: str, tip: str, base: str) -> None: + """Check that ``ref``, which the push grant created at ``tip``, can be neither forged over nor deleted, then + delete it with the token. ``base`` is another commit the provider has.""" + kind = "branch" if ref.startswith("refs/heads/") else "tag" + response = self.forge_create(url, ref, tip) + self.check( + f"the provider refuses a forged create of an existing {kind} under a create_only rule", + subprocess.CompletedProcess([], response.status_code, response.text, ""), + response.status_code == 200 and f"ng {ref}".encode() in response.content, + ) + self.check_case_variant(url, push_grant, clone, ref, tip, base) + self.expect_rejection( + f"deleting the pushed {kind} with the push grant is rejected", + self.git(push_grant, "push", url, f":{ref}", cwd=clone), + f"the git grant does not allow deleting a {kind}", + ) + self.expect_success(f"delete the pushed {kind} {ref} with the token", self.delete_directly(ref, clone)) + + def check_case_variant(self, url: str, push_grant: str, clone: Path, ref: str, tip: str, base: str) -> None: + """Check that the provider keeps ``ref`` apart from a name that differs from it only in case. + + A provider that takes the two for one ref would let a rule covering the variant move ``ref``, and a + create_only rule create the variant over it. A listing alone can't show that: git on a case-insensitive + filesystem lists both names but may resolve ``ref`` to the variant, and then refuses to move ``ref`` from + ``tip``. So the check moves ``ref`` to ``base`` after creating the variant there. + """ + kind, prefix = ("branch", "refs/heads/") if ref.startswith("refs/heads/") else ("tag", "refs/tags/") + variant = prefix + ref.removeprefix(prefix).upper() + assert variant != ref, f"{ref} has no letters to change the case of" + response = self.forge_create(url, variant, base) + listed = self.git(fetch_grant_for(self.args.private_key, self.backend.repository_id), "ls-remote", url, ref) + moved = self.git(push_grant, "push", "--force", url, f"{base}:{ref}", cwd=clone) + self.check( + f"the provider keeps a {kind} apart from a name that differs only in case", + subprocess.CompletedProcess([], moved.returncode, f"{response.text}\n{listed.stdout}", moved.stderr), + listed.stdout.strip() == f"{tip}\t{ref}" and moved.returncode == 0, + ) + if response.status_code == 200 and f"ok {variant}".encode() in response.content: + self.expect_success( + f"delete the case variant {variant} with the token", self.delete_directly(variant, clone) + ) + + def forge_create(self, url: str, ref: str, tip: str) -> requests.Response: + """Push ``ref`` to ``tip`` as a new ref under a create_only rule, though it exists already. + + GitServer passes it as a create, so the provider's own refusal of a create over an existing ref is all that + keeps a create_only rule from moving one. git would send the ref's current id, so the request is built by + hand. + """ + prefix, _, name = ref.partition("/")[2].partition("/") + patterns = {"branch_patterns" if prefix == "heads" else "tag_patterns": [re.escape(name)]} + rule = PushRule(repository=self.backend.repository_id, create_only=True, **patterns) + return requests.post( + f"{url}/git-receive-pack", + data=forged_create_body(ref, tip), + headers={ + GRANT_HEADER: grant_for(self.args.private_key, rule), + "Content-Type": "application/x-git-receive-pack-request", + # As git sends it. requests would otherwise send its own, which GitServer refuses on a push. + "Accept": "application/x-git-receive-pack-result", + }, + timeout=60, + ) + + def delete_directly(self, ref: str, clone: Path) -> subprocess.CompletedProcess[str]: + """Delete ``ref`` on the provider with the token Sentry would use, since a push grant cannot.""" + provider = self.backend.fetch_provider(ORGANIZATION_ID, self.backend.repository) + assert isinstance(provider, GetGitRemoteProtocol) + remote = provider.get_git_remote() + token = self.backend.get_token(self.backend.repository, "push") + assert token is not None + credentials = base64.b64encode(f"{remote['basic_auth_username']}:{token.reveal()}".encode()).decode() + # Through the environment rather than the command line, which other processes can read. + env = { + **self.env, + "GIT_CONFIG_COUNT": "1", + "GIT_CONFIG_KEY_0": "http.extraHeader", + "GIT_CONFIG_VALUE_0": f"Authorization: Basic {credentials}", + } + command = ["git", "-c", "credential.helper=", "push", remote["url"], f":{ref}"] + return subprocess.run(command, cwd=clone, env=env, capture_output=True, text=True, timeout=300) + + +def forged_create_body(ref: str, tip: str) -> bytes: + """A receive-pack request that creates ``ref`` at ``tip``, an object the provider has, even if it exists.""" + update = f"{'0' * len(tip)} {tip} {ref}\0report-status".encode() + if len(tip) == 64: + update += b" object-format=sha256" + # An empty pack, since the provider has the commit already: a header for zero objects and its hash. + header = b"PACK" + (2).to_bytes(4, "big") + (0).to_bytes(4, "big") + pack = header + (hashlib.sha1 if len(tip) == 40 else hashlib.sha256)(header).digest() + return b"%04x" % (len(update) + 4) + update + b"0000" + pack + def _indent(text: str) -> str: return "".join(f" {line}\n" for line in text.splitlines()) diff --git a/src/scm/errors.py b/src/scm/errors.py index bfd674a9..71df8e7d 100644 --- a/src/scm/errors.py +++ b/src/scm/errors.py @@ -3,6 +3,7 @@ type ErrorCode = Literal[ "git_grant_expired", "git_invalid_grant", + "git_malformed_request_body", "git_operation_not_granted", "git_provider_not_supported", "git_remote_not_https", @@ -51,6 +52,7 @@ ERROR_CODES: dict[ErrorCode, str] = { "git_grant_expired": "The git grant has expired.", "git_invalid_grant": "The git grant was missing, malformed or forged.", + "git_malformed_request_body": "The request body is not one the git endpoint can read.", "git_operation_not_granted": "The git grant does not allow this operation.", "git_provider_not_supported": "The repository's provider has no git remote.", "git_remote_not_https": "The repository's git remote is not an https URL.", @@ -170,6 +172,10 @@ class GitInvalidGrant(GitSCMCodedError): code = "git_invalid_grant" +class GitMalformedRequestBody(GitSCMCodedError): + code = "git_malformed_request_body" + + class GitOperationNotGranted(GitSCMCodedError): code = "git_operation_not_granted" diff --git a/src/scm/git/grant.py b/src/scm/git/grant.py index 7763458b..fe35c681 100644 --- a/src/scm/git/grant.py +++ b/src/scm/git/grant.py @@ -44,7 +44,7 @@ # The only algorithm the verifier accepts, with parsed Ed25519 keys. A token that names HS256 and uses a public key, # which is no secret, as its HMAC key is refused, as is one that names ``none``. _ALGORITHM = "EdDSA" -# Claims the JWT layer owns. Every other claim must be a Grant field. +# Claims the JWT layer owns. Every other claim must be a field of the grant. _JWT_CLAIMS = frozenset({"aud", "exp", "iat"}) @@ -114,6 +114,8 @@ def require_private_key(private_key: Secret) -> Secret: def verify_grant(public_keys: Sequence[str], token: str, *, now: float | None = None) -> Grant: """Return the grant if the private key of one of ``public_keys`` signed it and it has not expired. + Only version 2 grants verify. A version 1 grant, which lets a push update any ref, is refused. + Verification takes a list of Ed25519 public keys in PEM, so a new key can be added before the signer switches to it and the old one removed after its grants expire. Raises GitGrantKeyError, rather than refusing the grant, if the list is empty or a key is not one, since no grant could verify against it. Every other failure is a @@ -155,17 +157,19 @@ def verify_grant(public_keys: Sequence[str], token: str, *, now: float | None = raise GitInvalidGrant(detail=f"The git grant is invalid: {e}") from None fields = {name: value for name, value in claims.items() if name not in _JWT_CLAIMS} + payload = {**fields, "expires_at": claims["exp"]} + # Checked first, so a version 1 grant is told its version is the problem rather than its first unknown field. + if fields.get("version") != 2: + raise GitInvalidGrant(detail="The git grant is not version 2, the only version the git endpoint accepts.") try: - grant = msgspec.convert({**fields, "expires_at": claims["exp"]}, type=Grant, strict=True) + grant = msgspec.convert(payload, type=Grant, strict=True) except msgspec.ValidationError as e: - # The signature matched, so the holder of the private key wrote these claims; telling them why they are + # The signature matched, so the holder of the private key wrote these claims, and telling them why they are # invalid gives nothing away. raise GitInvalidGrant(detail=f"The git grant's payload is invalid: {e}") from None if grant.expires_at <= now: raise GitGrantExpired(detail="The git grant has expired.") - if not grant.repositories: - raise GitInvalidGrant(detail="The git grant names no repositories.") return grant diff --git a/src/scm/git/push.py b/src/scm/git/push.py new file mode 100644 index 00000000..fc679b2a --- /dev/null +++ b/src/scm/git/push.py @@ -0,0 +1,389 @@ +"""Checking a push's ref updates, so GitServer can keep a push to its grant's push rules. + +A push's request body lists the ref updates as pkt-lines, then holds the pack. ``check_push`` reads the updates +before GitServer looks anything up, refuses the push unless a rule allows every update, and otherwise returns a body +that sends the updates it read followed by the rest, which it never reads. Anything it doesn't recognise refuses the +push. gitprotocol-pack(5) describes the format, which receive-pack speaks in protocol versions 0 and 1 alike, the +only ones it has. +""" + +import contextlib +import re +from collections.abc import Mapping, Sequence +from typing import NamedTuple + +from scm.errors import GitMalformedRequestBody, GitOperationNotGranted, RpcMalformedRequestHeaders, SCMCodedError +from scm.git.relay import DEFAULT_CHUNK_SIZE, _printable, _read +from scm.git.types import PushRule, Readable, _follows_ref_format, _is_short_name + +MAX_UPDATES_LENGTH = 64 * 1024 +"""The most bytes a push's ref updates may take. An update takes under 100 bytes plus its ref's name.""" + +MAX_SHALLOW_LENGTH = 4 * 1024 * 1024 +"""The most bytes a push may take to list its shallow commits, which git sends before the updates of every push from +a shallow clone: about 79,000 SHA-1 commits at 53 bytes each. A clone of many branches at a small depth has many.""" + +# The capabilities git asks for in a push, apart from push options, which ask the provider to act on the push, such +# as GitLab opening a merge request or skipping CI. Any other is refused, since it could change what a push does. +_CAPABILITIES = frozenset({"atomic", "quiet", "report-status", "report-status-v2", "side-band-64k"}) +_CAPABILITIES_WITH_VALUES = frozenset({"agent", "object-format", "session-id"}) +# The object formats git names in object-format=, and the length of an object id in each, in hex. +_OBJECT_ID_LENGTHS = {"sha1": 40, "sha256": 64} +# What git puts in a capability list: printable ASCII, the capabilities split by spaces. +_CAPABILITY_LIST = re.compile(rb"[\x20-\x7e]*") +# The headers the relay passes on, with the only values git sends with each request of a push. git asks for no +# protocol version on a push, or for version 1, since a push has no version 2, and never compresses one. +_HEADER_VALUES = { + "GET": { + "accept": frozenset({"*/*"}), + "content-encoding": frozenset(), + "content-type": frozenset(), + "git-protocol": frozenset({"version=1"}), + }, + "POST": { + "accept": frozenset({"application/x-git-receive-pack-result"}), + "content-encoding": frozenset(), + "content-type": frozenset({"application/x-git-receive-pack-request"}), + "git-protocol": frozenset({"version=1"}), + }, +} +# The other headers the relay passes on, whose values vary with git's version and build. They may hold printable +# ASCII without surrounding space, which is what the relay sends. +_FREE_HEADERS = frozenset({"accept-encoding", "user-agent"}) +_FREE_HEADER_VALUE = re.compile(r"[\x21-\x7e](?:[\x20-\x7e]*[\x21-\x7e])?") +# The longest branch or tag name a push may update, in bytes. A rule's pattern runs on the name the client chose, and +# one that backtracks takes time that grows with the name's length. +_MAX_NAME_LENGTH = 255 +# The refs a push rule can cover, by the prefix of their full names. +_KINDS = {"refs/heads/": "branch", "refs/tags/": "tag"} + +_FLUSH = b"0000" +# A pkt-line's length, in 4 lowercase hex digits as git writes them, counts the 4 digits too. +_PKT_LENGTH = re.compile(rb"[0-9a-f]{4}") +_MAX_PKT_LENGTH = 65520 +_OBJECT_ID = re.compile(rb"[0-9a-f]{40}|[0-9a-f]{64}") +# The most of a ref's name a refusal's detail shows. The pushing client chooses the name, and the detail reaches +# error reports and logs. +_SHOWN_REF_LENGTH = 200 +_SHOWN_CAPABILITY_LENGTH = 40 +# The most of a refused push's body read to its end. A provider takes no push this large, so a client that sends more +# only holds a worker, and git then sees the connection close instead of the reason. +_MAX_DRAIN_LENGTH = 256 * 1024 * 1024 + + +class PushRefused(GitOperationNotGranted): + """A push the grant does not allow, with the response that tells git why, if the client can read one.""" + + def __init__(self, *, detail: str, report: bytes | None) -> None: + super().__init__(detail=detail) + self.report = report + + +class RefUpdate(NamedTuple): + old_id: bytes + new_id: bytes + ref: bytes + + @property + def creates(self) -> bool: + return not self.old_id.strip(b"0") + + @property + def deletes(self) -> bool: + return not self.new_id.strip(b"0") + + +class PushRequest(NamedTuple): + updates: list[RefUpdate] + capabilities: frozenset[str] + """What the client asked for, from its first update.""" + head: bytes + """The bytes read from the body, to send on before the rest of it.""" + + +def check_push(rules: Sequence[PushRule], headers: Mapping[str, str], body: Readable | None) -> Readable: + """Return the body to relay for a push that ``rules`` allow, and refuse any other. + + ``body`` must end where the request body ends. A refused push's body is read to the end first, because git reads + the answer only once it has sent the whole push, and a connection closed before then shows git only a broken + pipe. Raises PushRefused when no rule allows an update, or for a signed push, and GitMalformedRequestBody for a + body that is missing or not a list of ref updates. A push with no rules is refused like any other. + """ + if body is None: + raise GitMalformedRequestBody(detail="The push has no body.") + try: + request = read_push_request(headers, body) + if not request.updates and _read(body, 1): + # git's probe before a large push is a lone flush, and a provider reads nothing after a flush that + # updates no ref. + raise GitMalformedRequestBody(detail="The push updates no ref but goes on after its ref updates.") + refused = refused_updates(rules, request) + if refused: + ref, reason = next(iter(refused.items())) + raise PushRefused( + detail=f"The git grant does not allow this push: {_shown(ref)} ({reason}).", + report=refusal_report(request, refused), + ) + except (GitOperationNotGranted, GitMalformedRequestBody, RpcMalformedRequestHeaders): + _drain(body) + raise + return ReplayedBody(request.head, body) + + +def read_push_request(headers: Mapping[str, str], body: Readable) -> PushRequest: + """Read a push's ref updates from ``body``, leaving the pack unread. + + Raises GitMalformedRequestBody for a body that does not start with a list of ref updates, + RpcMalformedRequestHeaders for a header value git doesn't send with a push, such as any Content-Encoding, + PushRefused for a signed push, whose updates come inside a certificate, and ClientClosedRequest if reading the + body fails. + """ + check_push_headers("POST", headers) + reader = _PktReader(body, limit=MAX_SHALLOW_LENGTH) + shallow: list[bytes] = [] + updates: list[RefUpdate] = [] + capabilities: frozenset[str] = frozenset() + # Each line must be what git sends, in the order it sends them. receive-pack reads more than that, such as + # capabilities on any update and shallow commits between updates, and anything this lets through it relays. + while (line := reader.read_line()) is not None: + if updates: + if b"\0" in line: + raise GitMalformedRequestBody(detail="Only the push's first ref update may carry capabilities.") + updates.append(_ref_update(line)) + elif line.startswith(b"shallow "): + # A push from a shallow clone lists its shallow commits first, each on a line of its own, and they pass + # on unchanged. + if not line.endswith(b"\n"): + raise GitMalformedRequestBody(detail="The push lists a shallow commit that is not an object id.") + shallow.append(line.removeprefix(b"shallow ").removesuffix(b"\n")) + elif line.startswith(b"push-cert\0"): + message = "The git endpoint does not relay signed pushes." + requested = _capabilities(line.partition(b"\0")[2]) + # Band 3 carries a message git prints as "remote: " before it stops. + raise PushRefused(detail=message, report=_on_side_band(requested, 3, f"{message}\n".encode())) + else: + # The updates' own budget starts here, apart from the shallow commits'. + reader.limit_to(reader.line_start + MAX_UPDATES_LENGTH) + update, _, requested_bytes = line.partition(b"\0") + capabilities = _capabilities(requested_bytes) + updates.append(_ref_update(update)) + + formats = [name.partition("=")[2] for name in capabilities if name.startswith("object-format=")] + if len(formats) > 1: + raise GitMalformedRequestBody(detail="The push names more than one object format.") + # Without object-format=, git means SHA-1. + id_length = _OBJECT_ID_LENGTHS.get(formats[0] if formats else "sha1") + if id_length is None: + raise GitMalformedRequestBody(detail="The push names an object format git doesn't have.") + if any(not _OBJECT_ID.fullmatch(commit) or len(commit) != id_length for commit in shallow): + raise GitMalformedRequestBody(detail="The push lists a shallow commit that is not an object id.") + if any(len(update.old_id) != id_length for update in updates): + raise GitMalformedRequestBody(detail="The push's object ids don't match its object format.") + if len({update.ref for update in updates}) < len(updates): + raise GitMalformedRequestBody(detail="The push updates a ref more than once.") + return PushRequest(updates=updates, capabilities=capabilities, head=reader.read_so_far()) + + +def check_push_headers(method: str, headers: Mapping[str, str]) -> None: + """Refuse a push's request unless each header the relay passes on holds a value git sends with it. + + Every value counts, since a mapping can hold one header under two spellings and the relay passes on one of them. + Raises RpcMalformedRequestHeaders. + """ + allowed_values = _HEADER_VALUES[method] + for name, value in headers.items(): + lowered = name.lower() + allowed = allowed_values.get(lowered) + if (allowed is not None and value not in allowed) or ( + lowered in _FREE_HEADERS and not _FREE_HEADER_VALUE.fullmatch(value) + ): + raise RpcMalformedRequestHeaders(detail=f"The push's {lowered} header holds a value git doesn't send.") + + +def refused_updates(rules: Sequence[PushRule], request: PushRequest) -> dict[bytes, str]: + """Return why ``rules`` refuse each ref of ``request``, or nothing if every update has a rule that allows it. + + A push is relayed whole or not at all, so once one update is refused, every other ref gets a reason too. With no + rules, every update is refused. + """ + unsupported = min((name for name in request.capabilities if not _supported(name)), default=None) + if unsupported is not None: + reason = f"the git endpoint does not relay {_capability_name(unsupported)}" + return {update.ref: reason for update in request.updates} + refused: dict[bytes, str] = {} + for update in request.updates: + refusals = [_refusal(rule, update) for rule in rules] if rules else [_refusal(None, update)] + reasons = [refusal for refusal in refusals if refusal is not None] + if len(reasons) == len(refusals): + # The reason of the rule the update came closest to passing says the most. + refused[update.ref] = max(reasons)[1] + if refused: + for update in request.updates: + refused.setdefault(update.ref, "refused with the rest of the push") + return refused + + +def _refusal(rule: PushRule | None, update: RefUpdate) -> tuple[int, str] | None: + """Return why ``rule``, or a grant with no rule, refuses ``update``, ranked by how far the update got, or None if + the rule allows it.""" + # Reading the push checked that the ref's name is one git takes, so it decodes. + ref = update.ref.decode() + kind = next((kind for prefix, kind in _KINDS.items() if ref.startswith(prefix)), None) + if kind is None: + return (0, "the git grant allows pushing branches and tags only") + if rule is None or not rule.covers(ref): + return (1, f"the git grant does not cover this {kind}") + if update.deletes: + return (3, f"the git grant does not allow deleting a {kind}") + if rule.create_only and not update.creates: + return (2, f"the git grant allows only creating this {kind}") + return None + + +def refusal_report(request: PushRequest, refused: Mapping[bytes, str]) -> bytes | None: + """Return the response body that tells git why each ref was refused, or None if the client asked for none. + + git shows each ref as ``! [remote rejected] ()``, as for a provider's own refusal. A refused + HTTP request would show only its status code. + """ + if not {"report-status", "report-status-v2"} & request.capabilities: + return None + # "unpack ok" makes git show each ref's reason rather than an unpack failure. Each "ng" line is shorter than + # the update it answers, so it fits in a pkt-line. + report = ( + _pkt(b"unpack ok\n") + + b"".join(_pkt(b"ng %s %s\n" % (update.ref, refused[update.ref].encode())) for update in request.updates) + + _FLUSH + ) + return _on_side_band(request.capabilities, 1, report) or report + + +class ReplayedBody: + """A request body whose first bytes were read already, sending them before the rest.""" + + def __init__(self, head: bytes, rest: Readable) -> None: + self.head = head + self.rest = rest + + def read(self, size: int = -1, /) -> bytes: + if not self.head: + return self.rest.read(size) + if size < 0: + data, self.head = self.head + self.rest.read(), b"" + return data + data, self.head = self.head[:size], self.head[size:] + return data + + +class _PktReader: + def __init__(self, body: Readable, *, limit: int) -> None: + self.body = body + self.limit = limit + self.read_bytes = bytearray() + self.line_start = 0 + """Where the line read last starts, counted from the start of the body.""" + + def read_line(self) -> bytes | None: + """Return the next pkt-line's payload as sent, or None for a flush.""" + self.line_start = len(self.read_bytes) + header = self._read(4) + if not _PKT_LENGTH.fullmatch(header): + raise GitMalformedRequestBody(detail="The push's ref updates are not pkt-lines.") + length = int(header, 16) + if length == 0: + return None + # 1 to 3 are protocol v2's special packets, which a push never sends. + if not 4 <= length <= _MAX_PKT_LENGTH: + raise GitMalformedRequestBody(detail="The push's ref updates are not pkt-lines.") + return self._read(length - 4) + + def limit_to(self, limit: int) -> None: + """Lower how far into the body the reader may read, refusing a body already read past it.""" + self.limit = min(self.limit, limit) + self._check(0) + + def read_so_far(self) -> bytes: + return bytes(self.read_bytes) + + def _check(self, size: int) -> None: + if len(self.read_bytes) + size > self.limit: + raise GitMalformedRequestBody(detail="The push's ref updates or shallow commits are too long.") + + def _read(self, size: int) -> bytes: + self._check(size) + data = bytearray() + while len(data) < size: + chunk = _read(self.body, size - len(data)) + if not chunk: + raise GitMalformedRequestBody(detail="The push ended before its ref updates did.") + data += chunk + self.read_bytes += data + return bytes(data) + + +def _drain(body: Readable) -> None: + drained = 0 + with contextlib.suppress(OSError, SCMCodedError): + while drained < _MAX_DRAIN_LENGTH and (chunk := body.read(DEFAULT_CHUNK_SIZE)): + drained += len(chunk) + + +def _supported(capability: str) -> bool: + name, has_value, _ = capability.partition("=") + return name in _CAPABILITIES_WITH_VALUES if has_value else name in _CAPABILITIES + + +def _capability_name(capability: str) -> str: + if capability == "push-options": + return "push options" + # The client chose the name, and the reason goes in every refused ref's line of the report. + return f"the {_printable(capability.partition('=')[0], _SHOWN_CAPABILITY_LENGTH)} capability" + + +def _on_side_band(capabilities: frozenset[str], band: int, data: bytes) -> bytes | None: + """Return ``data`` on side band ``band``, ending with a flush, or None if the client asked for no side band.""" + if "side-band-64k" in capabilities: + band_size = _MAX_PKT_LENGTH - 5 + elif "side-band" in capabilities: + band_size = 1000 - 5 + else: + return None + return b"".join(_pkt(bytes([band]) + data[i : i + band_size]) for i in range(0, len(data), band_size)) + _FLUSH + + +def _capabilities(requested: bytes) -> frozenset[str]: + if not _CAPABILITY_LIST.fullmatch(requested): + raise GitMalformedRequestBody(detail="The push's capabilities hold characters git doesn't send.") + return frozenset(requested.decode("ascii").split(" ")) - {""} + + +def _ref_update(line: bytes) -> RefUpdate: + old_id, _, rest = line.partition(b" ") + new_id, _, ref = rest.partition(b" ") + if not (_OBJECT_ID.fullmatch(old_id) and _OBJECT_ID.fullmatch(new_id) and len(old_id) == len(new_id) and ref): + raise GitMalformedRequestBody(detail="The push's ref updates are malformed.") + # A rule checks the ref the push names, and a name git wouldn't take might not be the ref the provider updates. + # The name also reaches logs, where invisible and right-to-left characters can make one ref look like another. + try: + name = ref.decode() + except UnicodeDecodeError: + name = "" + if not (name.startswith("refs/") and name.isprintable() and _follows_ref_format(name)): + raise GitMalformedRequestBody(detail="The push updates a ref whose name git wouldn't take.") + for prefix, kind in _KINDS.items(): + if name.startswith(prefix): + if not _is_short_name(name.removeprefix(prefix)): + raise GitMalformedRequestBody(detail=f"The push updates a {kind} whose name git wouldn't take.") + if len(ref) - len(prefix) > _MAX_NAME_LENGTH: + raise GitMalformedRequestBody( + detail=f"The push updates a {kind} whose name is longer than {_MAX_NAME_LENGTH} bytes." + ) + return RefUpdate(old_id=old_id, new_id=new_id, ref=ref) + + +def _shown(ref: bytes) -> str: + return _printable(ref.decode(errors="replace"), _SHOWN_REF_LENGTH) + + +def _pkt(payload: bytes) -> bytes: + return b"%04x" % (len(payload) + 4) + payload diff --git a/src/scm/git/relay.py b/src/scm/git/relay.py index b65848ae..ff64e615 100644 --- a/src/scm/git/relay.py +++ b/src/scm/git/relay.py @@ -9,9 +9,7 @@ from scm.errors import ( ClientClosedRequest, - GitOperationNotGranted, GitRemoteNotHttps, - GitRepositoryNotGranted, GitRouteNotSupported, ResourceBadGateway, ResourceCertificateInvalid, @@ -20,10 +18,10 @@ RpcMalformedRequestHeaders, SCMCodedError, ) -from scm.git.types import GitOperation, Grant, Readable +from scm.git.types import GitOperation, Readable from scm.rpc.types import StreamResponse from scm.secret import Secret -from scm.types import GitRemote, RepositoryId +from scm.types import GitRemote # The most bytes the relay reads at once, from the client's body or from the provider's response, so also the # most it holds per request in each direction. Smaller chunks cost more Python per byte of a multi-gigabyte pack; @@ -64,23 +62,21 @@ ) -def authorize_request(grant: Grant, repository_id: RepositoryId, method: str, route: str, query: str) -> GitOperation: - """Return the git operation a request belongs to, refusing the request unless the grant allows it. - - A fetch grant reaches only upload-pack and a push grant only receive-pack. The repository must be listed in - the grant in the same form the request uses: a database id does not match the same repository's - ``(provider, external id)`` pair. - """ +def _route_operation(method: str, route: str, query: str) -> GitOperation: operation = _ROUTES.get((method, route, query)) if operation is None: - raise GitRouteNotSupported(detail=f"{method} {route} is not a git fetch or push route.") - if operation != grant.operation: - raise GitOperationNotGranted(detail=f"The git grant allows {grant.operation}, not {operation}.") - if repository_id not in grant.repositories: - raise GitRepositoryNotGranted(detail="The git grant does not cover this repository.") + raise GitRouteNotSupported( + detail=f"{_printable(method, 20)} {_printable(route, 200)} is not a git fetch or push route." + ) return operation +def _printable(text: str, length: int) -> str: + """Return ``text`` for an error's detail, which reaches logs: escaped where it isn't printable, and cut short.""" + escaped = "".join(character if character.isprintable() else ascii(character)[1:-1] for character in text) + return escaped if len(escaped) <= length else f"{escaped[:length]}..." + + class ResponseBody(Iterator[bytes], Protocol): """The provider's response body. Closing it before it ends drops the upstream connection.""" @@ -157,7 +153,7 @@ def forward( ``route`` is the path below ``.git/``. The relay enforces what keeps the token safe: an https remote and one of the four git routes. It knows nothing of grants, so whether the request may run at all is the - caller's to decide; GitServer calls ``authorize_request`` before it looks up the repository or a token. + caller's to decide, as GitServer does before it looks up the repository or a token. Errors carry no URL, because the client has no use for the provider's address. """ @@ -165,8 +161,7 @@ def forward( # The token would cross the network in the clear, e.g. to a self-managed instance served over http. raise GitRemoteNotHttps() # The token never goes to anything but a git route, whatever the caller checked. - if (method, route, query) not in _ROUTES: - raise GitRouteNotSupported(detail=f"{method} {route} is not a git fetch or push route.") + _route_operation(method, route, query) if not f"{remote['basic_auth_username']}:{token.reveal()}".isascii(): # requests would fail to encode the credentials with a UnicodeEncodeError that holds them. No provider # issues such a token, so the caller has a bug; the message leaves the token out. diff --git a/src/scm/git/server.py b/src/scm/git/server.py index e49bedf1..e63614f7 100644 --- a/src/scm/git/server.py +++ b/src/scm/git/server.py @@ -1,10 +1,19 @@ from collections.abc import Callable, Mapping, Sequence from urllib.parse import urlsplit -from scm.errors import ErrorCode, GitProviderNotSupported, GitTokenRejected, GitTokenUnavailable, SCMCodedError +from scm.errors import ( + ErrorCode, + GitOperationNotGranted, + GitProviderNotSupported, + GitRepositoryNotGranted, + GitTokenRejected, + GitTokenUnavailable, + SCMCodedError, +) from scm.git.grant import require_public_keys, verify_grant -from scm.git.relay import GitRelay, ResponseBody, authorize_request -from scm.git.types import GRANT_HEADER, GetToken, Readable +from scm.git.push import PushRefused, check_push, check_push_headers +from scm.git.relay import GitRelay, ResponseBody, _route_operation +from scm.git.types import GRANT_HEADER, GetToken, GitOperation, Grant, Readable from scm.helpers import initialize_provider from scm.rpc.types import StreamResponse from scm.secret import Secret @@ -15,6 +24,7 @@ _STATUS: dict[ErrorCode, int] = { # The client's own request is malformed, or is not a git request this endpoint serves. "rpc_malformed_request_headers": 400, + "git_malformed_request_body": 400, "git_route_not_supported": 404, # The grant does not allow the request. "git_invalid_grant": 403, @@ -58,6 +68,12 @@ class GitServer: does not allow never reaches the database, the token supplier or the provider. Give it a relay whose adapter refuses internal addresses; see ``GitRelay``. + A push is kept to the grant's push rules by ``scm.git.push.check_push``, which reads the push's ref updates + before anything is looked up and refuses the whole push unless a rule allows every update. Anything it doesn't + recognise, such as push options, a signed push or a capability git doesn't send, refuses the push too. A client + that asked for a status report gets one, a 200 that git shows as a rejected ref with its reason. A push the + rules allow is relayed with the updates the server read followed by the rest of the body, unread. + The repository's provider says where its git endpoint is, through ``GetGitRemoteProtocol``. A repository whose provider has no git remote is refused. @@ -118,10 +134,10 @@ def handle( """Answer one git request. ``route`` is the path below ``.git/`` and ``query`` the raw query string. ``body`` is read only - while the request is sent upstream, so pass the unread request stream. It must yield the body already - decoded from its transfer encoding, ending where the body ends. A Django request under WSGI reads a chunked - body as empty, and git sends a push over about 1 MiB chunked, so a Django view needs to pass a stream that - has the whole body. + while the request is sent upstream, apart from a push's ref updates, so pass the unread request stream. It + must yield the body already decoded from its transfer encoding, ending where the body ends. A Django request + under WSGI reads a chunked body as empty, and git sends a push over about 1 MiB chunked, so a Django view + needs to pass a stream that has the whole body. """ # Rebound without the grant, so it shows as [Filtered] in the frames of a reported error rather than in # ``headers``, here and in the relay, which never sends it upstream anyway. @@ -133,8 +149,15 @@ def handle( grant = verify_grant(self.public_keys, grant_token.reveal()) context["organization_id"] = str(grant.organization_id) referrer_tag["referrer"] = context["referrer"] = grant.referrer - operation = authorize_request(grant, repository_id, method, route, query) + operation = _authorize(grant, repository_id, method, route, query) context["operation"] = operation + relayed_body: Readable | None = counted_body + if operation == "push": + if method == "POST": + relayed_body = check_push(grant.push_rules(repository_id), headers, counted_body) + else: + # The push's first request, which lists the refs, goes to the provider with the same headers. + check_push_headers(method, headers) provider = initialize_provider( grant.organization_id, repository_id, @@ -163,7 +186,7 @@ def handle( "read it." ) response = self.relay.forward( - remote, token, method=method, route=route, query=query, headers=headers, body=counted_body + remote, token, method=method, route=route, query=query, headers=headers, body=relayed_body ) if response.status_code == 401: # The provider rejected the token Sentry supplied. Passed on, the 401 would make git ask the client @@ -174,6 +197,8 @@ def handle( self.record_count("sentry.scm.git.request.failed", 1, {"code": e.code} | referrer_tag) if e.code != "client_closed_request": self.emit_error(e, context) + if isinstance(e, PushRefused) and e.report is not None: + return _report_response(e.report) return _error_response(e) except Exception: self.record_count("sentry.scm.git.request.failed", 1, {"code": "unhandled_exception"} | referrer_tag) @@ -207,6 +232,30 @@ def report(error: SCMCodedError) -> None: ) +def _authorize(grant: Grant, repository_id: RepositoryId, method: str, route: str, query: str) -> GitOperation: + """Return the git operation a request belongs to, refusing the request unless the grant allows it. + + Fetches reach only upload-pack and pushes only receive-pack. A request runs only if the grant has a rule for its + repository and operation, and the rule must name the repository in the same form the request uses: a database id + does not match the same repository's ``(provider, external id)`` pair. A fetch needs a fetch rule for any object, + since nothing reads a fetch's body to keep it to a rule's branches and tags. + """ + operation = _route_operation(method, route, query) + fetch_rules, push_rules = grant.fetch_rules(repository_id), grant.push_rules(repository_id) + if not fetch_rules and not push_rules: + raise GitRepositoryNotGranted(detail="The git grant does not cover this repository.") + if operation == "fetch" and not fetch_rules: + raise GitOperationNotGranted(detail="The git grant allows push on this repository, not fetch.") + if operation == "push" and not push_rules: + raise GitOperationNotGranted(detail="The git grant allows fetch on this repository, not push.") + if operation == "fetch" and not any(rule.any_object for rule in fetch_rules): + raise GitOperationNotGranted( + detail="The git endpoint cannot keep a fetch to a rule's branches and tags, so a fetch rule must set " + "any_object." + ) + return operation + + def _take_grant(headers: Mapping[str, str]) -> tuple[Secret, dict[str, str]]: """Split the grant, empty if there is none, from the other headers. Header names are case-insensitive.""" grant = Secret("") @@ -279,6 +328,18 @@ def _end(self, failure: str | None) -> None: self.finish(failure, self.bytes_sent) +def _report_response(report: bytes) -> StreamResponse: + return StreamResponse( + status_code=200, + headers={ + "Content-Type": "application/x-git-receive-pack-result", + "Content-Length": str(len(report)), + "Cache-Control": "no-cache", + }, + content=iter([report]), + ) + + def _error_response(error: SCMCodedError) -> StreamResponse: # A code missing from the map comes from a failure nobody planned for, which is the server's fault. status = _STATUS.get(error.code, 500) diff --git a/src/scm/git/types.py b/src/scm/git/types.py index 66239d0d..d3f829c3 100644 --- a/src/scm/git/types.py +++ b/src/scm/git/types.py @@ -1,15 +1,14 @@ +import re from collections.abc import Callable from typing import Literal, Protocol import msgspec from scm.secret import Secret -from scm.types import Referrer, Repository, RepositoryId +from scm.types import BranchName, Referrer, Repository, RepositoryId type GitOperation = Literal["fetch", "push"] -"""What a grant lets its holder do. ``scm.git.relay`` maps each to its git routes.""" - -GRANT_VERSION: Literal[1] = 1 +"""Whether a git request fetches or pushes. ``scm.git.relay`` maps each to its git routes.""" GRANT_HEADER = "X-Scm-Git-Grant" """The request header that carries a signed grant to the git endpoint.""" @@ -17,23 +16,134 @@ GRANT_MAX_LENGTH = 4096 """The longest signed grant, in characters. The grant travels in a request header, and sandbox-router's ``PUT /grant/{sandbox_id}`` refuses a longer one, so a change here must change the router too. A grant grows with -its repositories: a grant for one is under 450 characters, and the claims are base64-encoded, so each repository -named as ``(provider, path)`` adds about 4/3 of its path's length plus 20, e.g. 52 for a 25-character path. The -limit is a conservative pick rather than a measured ceiling: header limits further along the path are typically -around 8 KiB.""" +its rules: a grant with one is under 500 characters, and the claims are base64-encoded, so a fetch rule for any +object of a repository named as ``(provider, path)`` adds about 4/3 of its path's length plus 87, e.g. 120 for a +25-character path, and each branch or tag a rule lists adds about 4/3 of its name's length plus 4. The limit is a +conservative pick rather than a measured ceiling: header limits further along the path are typically around +8 KiB.""" -class Grant(msgspec.Struct, frozen=True, kw_only=True, forbid_unknown_fields=True): - """Permission to run one git operation against a fixed set of an organization's repositories. +class _RefRule(msgspec.Struct, frozen=True, kw_only=True, forbid_unknown_fields=True, omit_defaults=True): + """What fetch and push rules share: the repository, and the branches and tags of it the rule covers. + + Each kind of ref is set by a list of names or by patterns, and not both. A pattern covers only names of its own + kind, so a pattern too broad for branches still can't reach a tag or any other ref. + """ + + repository: RepositoryId + branches: list[BranchName] | None = None + """Branch names as ``git branch`` takes them, without ``refs/heads/``.""" + branch_patterns: list[str] | None = None + """Python regular expressions, each matched against a branch's whole name without ``refs/heads/``, such as + ``seer/.+``. They run with ``re.ASCII``, so ``\\d`` and ``\\w`` don't match lookalikes such as fullwidth digits. The + pushing client chooses the names they are matched against, so a pattern must not be one that backtracks + catastrophically.""" + tags: list[str] | None = None + """Tag names as ``git tag`` takes them, without ``refs/tags/``.""" + tag_patterns: list[str] | None = None + """Like ``branch_patterns``, each matched against a tag's whole name without ``refs/tags/``.""" + + def __post_init__(self) -> None: + for names_field, names, patterns_field, patterns, kind in ( + ("branches", self.branches, "branch_patterns", self.branch_patterns, "branch"), + ("tags", self.tags, "tag_patterns", self.tag_patterns, "tag"), + ): + if names is not None and patterns is not None: + raise ValueError(f"A rule must not set both {names_field} and {patterns_field}.") + if names is not None: + _check_names(names_field, names, kind) + if patterns is not None: + _check_patterns(patterns_field, patterns) + + @property + def refs(self) -> list[str]: + """The full names of the refs the rule lists, such as ``refs/heads/main``. Its patterns cover more.""" + return [f"refs/heads/{name}" for name in self.branches or ()] + [ + f"refs/tags/{name}" for name in self.tags or () + ] + + def covers(self, ref: str) -> bool: + """Return whether the rule covers ``ref``, a full ref name such as ``refs/heads/main``.""" + if ref in self.refs: + return True + if ref.startswith("refs/heads/"): + name, patterns = ref.removeprefix("refs/heads/"), self.branch_patterns + elif ref.startswith("refs/tags/"): + name, patterns = ref.removeprefix("refs/tags/"), self.tag_patterns + else: + return False + return any(re.fullmatch(pattern, name, re.ASCII) for pattern in patterns or ()) + + def _names_refs(self) -> bool: + return any(field is not None for field in (self.branches, self.branch_patterns, self.tags, self.tag_patterns)) + + +class FetchRule(_RefRule, frozen=True, tag="fetch", tag_field="operation"): + """Lets the holder fetch from ``repository``: any of its objects, or only what the branches and tags it covers + reach. + + A rule sets ``any_object`` or names refs, and not both, so a rule that names no refs is refused rather than read + as one that reads everything. ``any_object`` reaches every object the provider serves, including objects no ref + reaches and, on GitHub, objects of the repository's forks, so give it only to code that runs no agent, such as a + checkout before the agent starts. + """ + + any_object: bool = False + + def __post_init__(self) -> None: + super().__post_init__() + if self.any_object and self._names_refs(): + raise ValueError( + "A fetch rule that sets any_object must not set branches, branch_patterns, tags or tag_patterns." + ) + if not self.any_object and not self._names_refs(): + raise ValueError("A fetch rule must set branches, branch_patterns, tags, tag_patterns or any_object.") + + def covers(self, ref: str) -> bool: + return self.any_object or super().covers(ref) - Seer signs a grant per coding job and Sentry's git endpoint verifies it on every request. Each repository is - a ``RepositoryId`` under the grant's organization, as in the RPC endpoint: Sentry's repository id or a - ``(provider, external id)`` pair. A fetch grant holds one repository today. The list leaves room for - submodules and git-based dependencies, but only from repositories Sentry has connected for the organization: - naming any other repository, such as a public dependency, would need a new kind of entry and a new version. - A grant with a field this version does not know is refused rather than read without it. If a later version - adds a field that restricts a grant, a verifier that ignored the field would allow more than the grant does. +class PushRule(_RefRule, frozen=True, tag="push", tag_field="operation"): + """Lets the holder create or update the branches and tags of ``repository`` the rule covers, and no other ref, + and delete none. + + With ``create_only`` it allows only creating a covered branch or tag that doesn't exist yet, so a pattern can let + the holder start new branches or tag new releases without moving any that exist. Without it, a push may move a + covered branch or tag to any commit, so a force push passes. + + Don't cover the repository's default branch, by name or by pattern. The git endpoint doesn't know which branch + that is, so keeping it out is the signer's job, with the provider's branch protection behind it. Keep tag patterns + to a release shape such as ``v[0-9]+\\.[0-9]+\\.[0-9]+``: git resolves a short name to a tag before a branch, so a + tag named ``main`` redirects whoever fetches or resolves ``main``, and a new tag often starts a release pipeline. + """ + + create_only: bool = False + """Allow only creating a covered branch or tag. git sends a new ref with an old object id of all zeros, and the + provider refuses such an update if the ref already exists.""" + + def __post_init__(self) -> None: + super().__post_init__() + if not self._names_refs(): + raise ValueError("A push rule must set branches, branch_patterns, tags or tag_patterns.") + + +class Grant(msgspec.Struct, frozen=True, kw_only=True, forbid_unknown_fields=True): + """Permission to run git operations on an organization's repositories, as a list of rules. + + Seer signs a grant for each sandbox, and for its own git requests, and Sentry's git endpoint verifies it on + every request. A sandbox holds one grant at a time, so a grant that lets a pod both fetch and push holds a rule + for each. A request runs only if a rule for its repository and operation allows it. Several rules may name the + same repository and operation, and a request runs if any of them allows it, so their order doesn't matter: a + ``create_only`` rule with a pattern can sit next to a rule listing a branch the holder created and may update. + + Each rule names its repository as a ``RepositoryId`` under the grant's organization, as in the RPC endpoint: + Sentry's repository id or a ``(provider, external id)`` pair. Several repositories leave room for submodules and + git-based dependencies, but only from repositories Sentry has connected for the organization: naming any other + repository, such as a public dependency, would need a new kind of rule. + + A grant or rule with a field this version does not know is refused rather than read without it, because a + verifier that ignored a field restricting the grant would allow more than the grant does. So a new field needs + only the verifiers to know it before the signer uses it, and a new version only when a field changes meaning. """ organization_id: int @@ -41,12 +151,68 @@ class Grant(msgspec.Struct, frozen=True, kw_only=True, forbid_unknown_fields=Tru """Who the grant is for, e.g. a coding job or Seer itself, as ``referrer`` is elsewhere in this library. Sentry tags the git endpoint's metrics with it. Seer sets it, because git requests carry nothing else that tells callers apart.""" - operation: GitOperation - repositories: list[RepositoryId] + rules: list[FetchRule | PushRule] expires_at: int """Unix time in seconds. Every grant expires.""" - version: Literal[1] = GRANT_VERSION - """The format version. A verifier refuses a version it does not know.""" + version: Literal[2] = 2 + """The format version. A verifier refuses a version it does not know, so one that knows only version 1 refuses + this one rather than reading its rules as broader than they are.""" + + def __post_init__(self) -> None: + if not self.rules: + raise ValueError("A git grant must hold at least one rule.") + + def fetch_rules(self, repository: RepositoryId) -> list[FetchRule]: + """Return the fetch rules for ``repository``, named as the rules name it.""" + return [rule for rule in self.rules if isinstance(rule, FetchRule) and rule.repository == repository] + + def push_rules(self, repository: RepositoryId) -> list[PushRule]: + """Return the push rules for ``repository``, named as the rules name it.""" + return [rule for rule in self.rules if isinstance(rule, PushRule) and rule.repository == repository] + + +# What git check-ref-format refuses in a ref name: control characters, space, ~ ^ : ? * [ \, "..", "@{" and "//". +_NOT_IN_A_REF = re.compile(r"[\x00-\x20\x7f~^:?*\[\\]|\.\.|@\{|//") + + +def _check_patterns(field: str, patterns: list[str]) -> None: + if not patterns: + raise ValueError(f"A rule's {field} must not be empty.") + for pattern in patterns: + if not pattern: + raise ValueError(f"A pattern in a rule's {field} must not be empty.") + try: + re.compile(pattern, re.ASCII) + except (re.error, ValueError) as e: + raise ValueError(f"{pattern!r} is not a regular expression: {e}") from None + + +def _check_names(field: str, names: list[str], kind: str) -> None: + if not names: + raise ValueError(f"A rule's {field} must not be empty.") + for name in names: + if not _is_short_name(name): + prefix = "refs/heads/" if kind == "branch" else "refs/tags/" + raise ValueError(f"{name!r} is not a {kind} name. Name a {kind} as git {kind} takes it, without {prefix}.") + + +def _is_short_name(name: str) -> bool: + """Return whether ``name`` is a branch or tag name, without its refs/heads/ or refs/tags/.""" + # What git branch and git tag refuse on top of git check-ref-format's rules: a leading "-" and "HEAD". A name + # starting with refs/ is a valid one, but it's far more likely a full ref name by mistake. + return _follows_ref_format(name) and name != "HEAD" and not name.startswith(("-", "refs/")) + + +def _follows_ref_format(name: str) -> bool: + """Return whether git check-ref-format takes ``name``, a ref name or the part of one below refs/heads/.""" + return not ( + not name + or name == "@" + or name.startswith("/") + or name.endswith(("/", ".")) + or _NOT_IN_A_REF.search(name) + or any(part.startswith(".") or part.endswith(".lock") for part in name.split("/")) + ) type GetToken = Callable[[Repository, GitOperation], Secret | None] diff --git a/tests/unit/git_builders.py b/tests/unit/git_builders.py index fe1418c1..7eb8fc14 100644 --- a/tests/unit/git_builders.py +++ b/tests/unit/git_builders.py @@ -6,8 +6,8 @@ from cryptography.hazmat.primitives.asymmetric.ed25519 import Ed25519PrivateKey from cryptography.hazmat.primitives.serialization import Encoding, PublicFormat, load_pem_private_key -from scm.git.grant import generate_private_key -from scm.git.types import Grant +from scm.git.grant import GRANT_AUDIENCE, generate_private_key +from scm.git.types import FetchRule, Grant from scm.providers.cursor_origin.provider import CursorOriginProvider from scm.providers.github.provider import GitHubProvider from scm.providers.gitlab.provider import GitLabProvider @@ -48,18 +48,33 @@ def make_provider(organization_id: int, repository: Repository) -> Provider | No def make_grant(**overrides) -> Grant: - """A fetch grant for the default repository, named both ways a grant can name it.""" + """A grant to fetch any object of the default repository, named both ways a rule can name it.""" fields = { "organization_id": 1, "referrer": "seer.coding_job", - "operation": "fetch", - "repositories": [5, ("github", "123")], + "rules": [ + FetchRule(repository=5, any_object=True), + FetchRule(repository=("github", "123"), any_object=True), + ], "expires_at": FAR_FUTURE, } fields.update(overrides) return Grant(**fields) # type: ignore[arg-type] +VERSION_1_PUSH_CLAIMS = { + "organization_id": 1, + "referrer": "seer.coding_job", + "operation": "push", + "repositories": [5], + "version": 1, + "exp": FAR_FUTURE, + "iat": 1_700_000_000, + "aud": GRANT_AUDIENCE, +} +"""A push grant for the default repository as earlier releases sign one, which lets a push update any ref.""" + + class KeyPair(NamedTuple): key: Ed25519PrivateKey """The private key, to sign tokens sign_grant would not.""" @@ -80,3 +95,30 @@ def make_key_pair() -> KeyPair: # Serialized here rather than by public_key_of, so the tests check public_key_of against cryptography. public=key.public_key().public_bytes(Encoding.PEM, PublicFormat.SubjectPublicKeyInfo).decode(), ) + + +ZERO_ID = "0" * 40 +OLD_ID = "1" * 40 +NEW_ID = "2" * 40 +PACK = b"PACK\x00\x00\x00\x02" + b"pack contents" * 16 + + +def pkt(payload: bytes) -> bytes: + return b"%04x" % (len(payload) + 4) + payload + + +def make_push_body( + *updates: tuple[str, str, str], + capabilities: str = "report-status side-band-64k quiet object-format=sha1 agent=git/2.54.0", + shallow: tuple[str, ...] = (), + pack: bytes = PACK, +) -> bytes: + """A receive-pack request body as git sends it: shallow commits, ref updates (old id, new id, ref) and the pack. + + git ends no update with a newline, and puts its capabilities after a NUL on the first. + """ + lines = [pkt(f"shallow {commit}\n".encode()) for commit in shallow] + for index, (old_id, new_id, ref) in enumerate(updates): + line = f"{old_id} {new_id} {ref}".encode() + lines.append(pkt(line + b"\0 " + capabilities.encode() if index == 0 else line)) + return b"".join(lines) + b"0000" + pack diff --git a/tests/unit/test_git_grant.py b/tests/unit/test_git_grant.py index a8d07ff8..b0d5a159 100644 --- a/tests/unit/test_git_grant.py +++ b/tests/unit/test_git_grant.py @@ -31,10 +31,10 @@ sign_grant, verify_grant, ) -from scm.git.types import GRANT_MAX_LENGTH, Grant +from scm.git.types import GRANT_MAX_LENGTH, FetchRule, Grant, PushRule from scm.secret import Secret from tests.unit.error_reports import assert_not_in_frames -from tests.unit.git_builders import FAR_FUTURE, KeyPair, make_grant, make_key_pair +from tests.unit.git_builders import FAR_FUTURE, VERSION_1_PUSH_CLAIMS, KeyPair, make_grant, make_key_pair # Before the real time: PyJWT checks iat against the real clock, so a grant must not be issued after it. NOW = 1_700_000_000 @@ -117,9 +117,17 @@ def test_public_key_of_the_signing_key(self): # make_key_pair serializes the public key with cryptography directly, not through public_key_of. assert public_key_of(KEY.private) == KEY.public - def test_push_grant(self): - grant = make_grant(operation="push") - assert verify_grant([KEY.public], sign_grant(KEY.private, grant, now=NOW), now=NOW).operation == "push" + def test_rules_of_each_kind(self): + grant = make_grant( + rules=[ + FetchRule(repository=5, branches=["main", "seer/fix"]), + PushRule(repository=5, branches=["seer/fix"]), + FetchRule(repository=("gitlab", "gitlab.com:7"), any_object=True), + ] + ) + verified = verify_grant([KEY.public], sign_grant(KEY.private, grant, now=NOW), now=NOW) + assert verified == grant + assert [type(rule) for rule in verified.rules] == [FetchRule, PushRule, FetchRule] def test_token_is_a_jwt_with_the_grants_claims(self): token = sign_grant(KEY.private, make_grant(), now=NOW) @@ -127,9 +135,11 @@ def test_token_is_a_jwt_with_the_grants_claims(self): assert jwt.decode(token, options={"verify_signature": False}) == { "organization_id": 1, "referrer": "seer.coding_job", - "operation": "fetch", - "repositories": [5, ["github", "123"]], - "version": 1, + "rules": [ + {"operation": "fetch", "repository": 5, "any_object": True}, + {"operation": "fetch", "repository": ["github", "123"], "any_object": True}, + ], + "version": 2, "exp": FAR_FUTURE, "iat": NOW, "aud": GRANT_AUDIENCE, @@ -266,8 +276,10 @@ def test_other_asymmetric_algorithm(self): def grant_with_repositories(count: int) -> Grant: - # Names of one length, so each repository adds the same to the grant. - return make_grant(repositories=[("github", f"getsentry/repository-{i:04d}") for i in range(count)]) + # Names of one length, so each rule adds the same to the grant. + return make_grant( + rules=[FetchRule(repository=("github", f"getsentry/repository-{i:04d}"), any_object=True) for i in range(count)] + ) class TestLength: @@ -291,14 +303,14 @@ def test_longest_grant_that_fits_signs(self): except ValueError: break token, count = longer, count + 1 - # One more repository adds about 50 characters. - assert GRANT_MAX_LENGTH - 100 < len(token) <= GRANT_MAX_LENGTH + # One more rule adds about 120 characters. + assert GRANT_MAX_LENGTH - 150 < len(token) <= GRANT_MAX_LENGTH assert verify_grant([KEY.public], token, now=NOW) == grant_with_repositories(count) def test_grant_for_one_repository(self): # GRANT_MAX_LENGTH's docstring sizes grants from this. - grant = make_grant(repositories=[("github", "getsentry/scm-platform")]) - assert len(sign_grant(KEY.private, grant, now=NOW)) < 450 + grant = make_grant(rules=[FetchRule(repository=("github", "getsentry/scm-platform"), any_object=True)]) + assert len(sign_grant(KEY.private, grant, now=NOW)) < 500 def other_key_types() -> list: @@ -505,10 +517,9 @@ def test_missing(self): {"organization_id": True}, {"organization_id": "1"}, {"referrer": None}, - {"operation": "delete"}, - {"version": 2}, - {"repositories": [["github"]]}, - {"repositories": [["unknown_provider", "123"]]}, + {"rules": None}, + {"rules": [{"operation": "fetch", "repository": ["github"], "any_object": True}]}, + {"rules": [{"operation": "fetch", "repository": ["unknown_provider", "123"], "any_object": True}]}, {"exp": NOW + 600.5}, {"narrower_than_you_think": True}, ], @@ -522,5 +533,320 @@ def test_referrer_is_required(self): del claims["referrer"] assert_invalid(sign_claims(claims), "referrer") - def test_no_repositories(self): - assert_invalid(sign_grant(KEY.private, make_grant(repositories=[]), now=NOW), "names no repositories") + @pytest.mark.parametrize("version", [1, 3, "2", None]) + def test_version_other_than_2(self, version): + assert_invalid(sign_claims(claims_of(make_grant()) | {"version": version}), "not version 2") + + def test_without_a_version(self): + claims = claims_of(make_grant()) + del claims["version"] + assert_invalid(sign_claims(claims), "not version 2") + + def test_version_1_grant(self): + assert_invalid(sign_claims(VERSION_1_PUSH_CLAIMS), "not version 2") + + def test_version_1_claims_labelled_version_2(self): + assert_invalid(sign_claims(VERSION_1_PUSH_CLAIMS | {"version": 2}), "payload is invalid") + + +FETCH_NAMES_NOTHING = "A fetch rule must set branches, branch_patterns, tags, tag_patterns or any_object" +PUSH_NAMES_NOTHING = "A push rule must set branches, branch_patterns, tags or tag_patterns" + + +def rule_claims(*rules: dict) -> dict: + return claims_of(make_grant()) | {"rules": list(rules)} + + +class TestRules: + """A grant's rules, checked both where a grant is built and where it is verified, whoever signed it.""" + + def test_fetch_and_push_rules_for_one_repository(self): + grant = make_grant( + rules=[ + FetchRule(repository=5, branches=["main"]), + PushRule(repository=5, branches=["seer/fix"]), + ] + ) + assert verify_grant([KEY.public], sign_grant(KEY.private, grant, now=NOW), now=NOW) == grant + + def test_claims_hold_only_what_a_rule_sets(self): + # Shorter, and a reader sees which fetch rules reach any object. + grant = make_grant( + rules=[ + FetchRule(repository=5, branches=["main"], tags=["v1"]), + FetchRule(repository=6, any_object=True), + PushRule(repository=5, branches=["seer/fix"]), + PushRule(repository=5, branch_patterns=["seer/.+"], create_only=True), + PushRule(repository=5, tag_patterns=["v[0-9]+"], create_only=True), + ] + ) + claims = jwt.decode(sign_grant(KEY.private, grant, now=NOW), options={"verify_signature": False}) + assert claims["rules"] == [ + {"operation": "fetch", "repository": 5, "branches": ["main"], "tags": ["v1"]}, + {"operation": "fetch", "repository": 6, "any_object": True}, + {"operation": "push", "repository": 5, "branches": ["seer/fix"]}, + {"operation": "push", "repository": 5, "branch_patterns": ["seer/.+"], "create_only": True}, + {"operation": "push", "repository": 5, "tag_patterns": ["v[0-9]+"], "create_only": True}, + ] + + @pytest.mark.parametrize("operation", ["fetch", "push"]) + def test_same_ref_fields_for_fetch_and_push(self, operation): + rule = { + "operation": operation, + "repository": 5, + "branches": ["main"], + "tag_patterns": ["v[0-9]+"], + } + [verified] = verify_grant([KEY.public], sign_claims(rule_claims(rule)), now=NOW).rules + assert verified.covers("refs/heads/main") + assert verified.covers("refs/tags/v2") + + @pytest.mark.parametrize( + "rule,error", + [ + ({"operation": "fetch", "repository": 5}, FETCH_NAMES_NOTHING), + ({"operation": "fetch", "repository": 5, "any_object": False}, FETCH_NAMES_NOTHING), + ({"operation": "fetch", "repository": 5, "branches": None}, FETCH_NAMES_NOTHING), + ( + {"operation": "fetch", "repository": 5, "branches": ["main"], "any_object": True}, + "that sets any_object must not set branches, branch_patterns, tags or tag_patterns", + ), + ( + {"operation": "fetch", "repository": 5, "tag_patterns": ["v.+"], "any_object": True}, + "that sets any_object must not set branches, branch_patterns, tags or tag_patterns", + ), + ({"operation": "fetch", "repository": 5, "branches": []}, "must not be empty"), + ({"operation": "fetch", "repository": 5, "tags": []}, "must not be empty"), + ({"operation": "push", "repository": 5, "branches": []}, "must not be empty"), + ({"operation": "push", "repository": 5, "tags": []}, "must not be empty"), + ({"operation": "push", "repository": 5}, PUSH_NAMES_NOTHING), + ({"operation": "push", "repository": 5, "branches": None}, PUSH_NAMES_NOTHING), + ({"operation": "push", "repository": 5, "create_only": True}, PUSH_NAMES_NOTHING), + ( + {"operation": "push", "repository": 5, "branches": ["a"], "branch_patterns": ["a"]}, + "must not set both branches and branch_patterns", + ), + ( + {"operation": "fetch", "repository": 5, "tags": ["a"], "tag_patterns": ["a"]}, + "must not set both tags and tag_patterns", + ), + ({"operation": "push", "repository": 5, "branch_patterns": []}, "must not be empty"), + ({"operation": "push", "repository": 5, "tag_patterns": []}, "must not be empty"), + ({"operation": "push", "repository": 5, "branch_patterns": [""]}, "must not be empty"), + ({"operation": "fetch", "repository": 5, "tag_patterns": [""]}, "must not be empty"), + ({"operation": "push", "repository": 5, "branch_patterns": ["seer/("]}, "is not a regular expression"), + ({"operation": "fetch", "repository": 5, "tag_patterns": ["v("]}, "is not a regular expression"), + ({"operation": "push", "repository": 5, "branches": ["a"], "create_only": 1}, "Expected `bool`"), + ( + {"operation": "push", "repository": 5, "branches": ["a"], "any_object": True}, + "unknown field `any_object`", + ), + ({"operation": "fetch", "repository": 5, "any_object": True, "create_only": True}, "unknown field"), + ({"operation": "fetch", "repository": 5, "branches": ["main"], "deepen": 50}, "unknown field `deepen`"), + ({"operation": "push", "repository": 5, "refs": ["refs/heads/main"]}, "unknown field `refs`"), + ({"operation": "delete", "repository": 5, "branches": ["main"]}, "Invalid value 'delete'"), + ({"repository": 5, "any_object": True}, "missing required field `operation`"), + ({"operation": "fetch", "any_object": True}, "missing required field `repository`"), + ], + ) + def test_invalid_rule(self, rule, error): + assert_invalid(sign_claims(rule_claims(rule)), error) + + def test_rule_without_refs_or_any_object_cannot_be_built(self): + with pytest.raises(ValueError, match=FETCH_NAMES_NOTHING): + FetchRule(repository=5) + with pytest.raises(ValueError, match=PUSH_NAMES_NOTHING): + PushRule(repository=5, create_only=True) + + def test_rule_with_refs_and_any_object_cannot_be_built(self): + with pytest.raises(ValueError, match="that sets any_object must not set"): + FetchRule(repository=5, tags=["v1"], any_object=True) + + @pytest.mark.parametrize("make_rule", [FetchRule, PushRule], ids=["fetch", "push"]) + @pytest.mark.parametrize("field", ["branches", "branch_patterns", "tags", "tag_patterns"]) + def test_rule_with_an_empty_list_cannot_be_built(self, make_rule, field): + with pytest.raises(ValueError, match="must not be empty"): + make_rule(repository=5, **{field: []}) + + def test_no_rules(self): + with pytest.raises(ValueError, match="at least one rule"): + make_grant(rules=[]) + assert_invalid(sign_claims(rule_claims()), "at least one rule") + + def test_several_rules_for_one_repository_and_operation(self): + # A request passes if any of them allows it, so their order doesn't matter. + grant = make_grant( + rules=[ + FetchRule(repository=5, any_object=True), + FetchRule(repository=5, branches=["main"]), + PushRule(repository=5, branch_patterns=["seer/.+"], create_only=True), + PushRule(repository=5, branches=["seer/fix"]), + ] + ) + assert verify_grant([KEY.public], sign_grant(KEY.private, grant, now=NOW), now=NOW) == grant + assert grant.fetch_rules(5) == grant.rules[:2] + assert grant.push_rules(5) == grant.rules[2:] + assert grant.push_rules(("github", "123")) == [] + + def test_one_repository_named_both_ways(self): + # Nothing matches a database id to a provider id, so the two are separate repositories here, as in requests. + rules = [ + FetchRule(repository=5, any_object=True), + FetchRule(repository=("github", "123"), branches=["main"]), + ] + assert make_grant(rules=rules).rules == rules + + +# Invalid as a branch name and as a tag name. +INVALID_REF_NAMES = [ + "", + "refs/heads/main", + "refs/tags/v1", + "-main", + "HEAD", + "@", + "a..b", + "a b", + "a\tb", + "a\x00b", + "a\x7fb", + "a~1", + "a^", + "a:b", + "a?", + "a*", + "a[b", + "a\\b", + "/main", + "main/", + "a//b", + ".main", + "a/.b", + "main.lock", + "a/b.lock/c", + "main.", + "a@{1}", +] + + +class TestCovers: + """A rule covers full ref names: its branches under refs/heads/ and its tags under refs/tags/.""" + + @pytest.mark.parametrize( + "patterns,ref,covered", + [ + (["seer/.+"], "refs/heads/seer/fix", True), + (["seer/.+"], "refs/heads/seer/fix/deeper", True), + (["seer/.+"], "refs/heads/seer/", False), + # Matched against the whole name, not searched for in it. + (["seer/.+"], "refs/heads/main-seer/fix", False), + (["seer/[a-z-]+"], "refs/heads/seer/fix/deeper", False), + (["seer"], "refs/heads/seer/fix", False), + (["seer/[a-z]+", "cursor/.+"], "refs/heads/cursor/x", True), + (["seer/.+"], "refs/heads/Seer/fix", False), + # A branch pattern covers only branches. + (["seer/.+"], "refs/tags/seer/fix", False), + (["seer/.+"], "seer/fix", False), + ], + ) + def test_branch_patterns(self, patterns, ref, covered): + assert PushRule(repository=5, branch_patterns=patterns).covers(ref) is covered + + @pytest.mark.parametrize( + "ref,covered", + [ + ("refs/tags/v1.2", True), + ("refs/tags/v1", False), + ("refs/tags/x/v1.2", False), + ("refs/heads/v1.2", False), + ], + ) + def test_tag_patterns(self, ref, covered): + assert PushRule(repository=5, tag_patterns=[r"v[0-9]+\.[0-9]+"]).covers(ref) is covered + + def test_listed_refs(self): + rule = PushRule(repository=5, branches=["seer/fix"], tags=["v1"]) + assert rule.refs == ["refs/heads/seer/fix", "refs/tags/v1"] + assert rule.covers("refs/heads/seer/fix") + assert rule.covers("refs/tags/v1") + assert not rule.covers("refs/heads/seer/fix2") + assert not rule.covers("refs/heads/v1") + assert not rule.covers("refs/tags/seer/fix") + + @pytest.mark.parametrize("ref", ["refs/pull/1/head", "refs/notes/commits", "refs/heads", "refs/tags", "HEAD"]) + def test_patterns_cover_only_branches_and_tags(self, ref): + rule = PushRule(repository=5, branch_patterns=[".*"], tag_patterns=[".*"]) + assert rule.refs == [] + assert not rule.covers(ref) + + @pytest.mark.parametrize("make_rule", [FetchRule, PushRule], ids=["fetch", "push"]) + def test_same_for_fetch_and_push(self, make_rule): + rule = make_rule(repository=5, branches=["main"], tag_patterns=["v.+"]) + assert rule.refs == ["refs/heads/main"] + assert rule.covers("refs/heads/main") + assert rule.covers("refs/tags/v1") + assert not rule.covers("refs/heads/dev") + + def test_any_object_covers_every_ref(self): + rule = FetchRule(repository=5, any_object=True) + assert rule.refs == [] + assert rule.covers("refs/heads/main") + assert rule.covers("refs/pull/1/head") + + @pytest.mark.parametrize( + "pattern,ref,covered", + [ + (r"v\d+", "refs/tags/v12", True), + (r"v\d+", "refs/tags/v１２", False), + (r"seer/\w+", "refs/heads/seer/fix_1", True), + (r"seer/\w+", "refs/heads/seer/fïx", False), + # A character the pattern names itself still matches. + ("ünicode/.+", "refs/heads/ünicode/x", True), + (r"(?i)seer/.+", "refs/heads/SEER/x", True), + ], + ) + def test_patterns_run_with_ascii_classes(self, pattern, ref, covered): + field = "tag_patterns" if ref.startswith("refs/tags/") else "branch_patterns" + assert PushRule(repository=5, **{field: [pattern]}).covers(ref) is covered + + def test_pattern_that_asks_for_unicode_classes_cannot_be_built(self): + with pytest.raises(ValueError, match="is not a regular expression"): + PushRule(repository=5, tag_patterns=[r"(?u)v\d+"]) + + @pytest.mark.parametrize("field", ["branch_patterns", "tag_patterns"]) + def test_cannot_be_built_with_an_invalid_pattern(self, field): + with pytest.raises(ValueError, match="'seer/\\(' is not a regular expression"): + PushRule(repository=5, **{field: ["seer/("]}) + + +NAME_FIELDS = pytest.mark.parametrize( + "field,error", [("branches", "is not a branch name"), ("tags", "is not a tag name")], ids=["branch", "tag"] +) + + +class TestRefNames: + @NAME_FIELDS + @pytest.mark.parametrize("name", INVALID_REF_NAMES) + def test_invalid(self, field, error, name): + with pytest.raises(ValueError, match=error): + PushRule(repository=5, **{field: ["main", name]}) + assert_invalid( + sign_claims(rule_claims({"operation": "push", "repository": 5, field: ["main", name]})), + error, + ) + + @NAME_FIELDS + def test_checked_in_fetch_rules_too(self, field, error): + with pytest.raises(ValueError, match=error): + FetchRule(repository=5, **{field: ["refs/heads/main"]}) + + def test_tag_named_without_refs_tags(self): + with pytest.raises(ValueError, match="Name a tag as git tag takes it, without refs/tags/"): + PushRule(repository=5, tags=["refs/tags/v1"]) + + @pytest.mark.parametrize("field", ["branches", "tags"]) + @pytest.mark.parametrize( + "name", ["main", "seer/fix-123", "feature/a.b", "a@b", "release-1.0", "a.lock.b", "x/HEAD", "ünicode", "v1.0.0"] + ) + def test_valid(self, field, name): + assert getattr(PushRule(repository=5, **{field: [name]}), field) == [name] diff --git a/tests/unit/test_git_push.py b/tests/unit/test_git_push.py new file mode 100644 index 00000000..e78b9594 --- /dev/null +++ b/tests/unit/test_git_push.py @@ -0,0 +1,577 @@ +import io + +import pytest + +from scm.errors import ClientClosedRequest, GitMalformedRequestBody, GitOperationNotGranted, RpcMalformedRequestHeaders +from scm.git.push import ( + MAX_UPDATES_LENGTH, + PushRefused, + PushRequest, + RefUpdate, + ReplayedBody, + check_push, + check_push_headers, + read_push_request, + refusal_report, + refused_updates, +) +from scm.git.types import PushRule +from tests.unit.git_builders import NEW_ID, OLD_ID, PACK, ZERO_ID, make_push_body, pkt + +RULES = [PushRule(repository=5, branches=["seer/fix", "seer/other"])] + + +def read(body: bytes, headers: dict[str, str] | None = None) -> tuple[PushRequest, io.BytesIO]: + stream = io.BytesIO(body) + return read_push_request(headers or {}, stream), stream + + +class Trickle(io.BytesIO): + """A body that returns at most one byte per read, as a socket may.""" + + def read(self, size: int | None = -1, /) -> bytes: + return super().read(1 if size is None or size < 0 else min(size, 1)) + + +class Failing: + def read(self, size: int = -1, /) -> bytes: + raise ConnectionResetError() + + +class TestRead: + def test_updates_and_capabilities(self): + body = make_push_body((OLD_ID, NEW_ID, "refs/heads/seer/fix"), (ZERO_ID, NEW_ID, "refs/heads/seer/other")) + request, stream = read(body) + assert request.updates == [ + RefUpdate(old_id=OLD_ID.encode(), new_id=NEW_ID.encode(), ref=b"refs/heads/seer/fix"), + RefUpdate(old_id=ZERO_ID.encode(), new_id=NEW_ID.encode(), ref=b"refs/heads/seer/other"), + ] + assert request.capabilities == { + "report-status", + "side-band-64k", + "quiet", + "object-format=sha1", + "agent=git/2.54.0", + } + + def test_leaves_the_pack_unread(self): + body = make_push_body((OLD_ID, NEW_ID, "refs/heads/seer/fix")) + request, stream = read(body) + assert request.head + stream.read() == body + assert stream.getvalue()[len(request.head) :] == PACK + + def test_probe(self): + # git sends a lone flush before a push too large to buffer, to check its credentials before it streams. + request, stream = read(b"0000") + assert request == PushRequest(updates=[], capabilities=frozenset(), head=b"0000") + + def test_shallow_commits(self): + body = make_push_body((OLD_ID, NEW_ID, "refs/heads/seer/fix"), shallow=("3" * 40, "4" * 40)) + request, stream = read(body) + assert [update.ref for update in request.updates] == [b"refs/heads/seer/fix"] + assert request.head + stream.read() == body + + def test_sha256_object_ids(self): + old_id, new_id = "a" * 64, "b" * 64 + body = make_push_body( + (old_id, new_id, "refs/heads/seer/fix"), capabilities="report-status object-format=sha256" + ) + request, _ = read(body) + assert request.updates[0].new_id == new_id.encode() + + @pytest.mark.parametrize( + "capabilities,ids,shallow", + [ + ("report-status object-format=sha256", ("1" * 40, "2" * 40), ()), + ("report-status object-format=sha1", ("a" * 64, "b" * 64), ()), + ("report-status", ("a" * 64, "b" * 64), ()), + ("report-status", (OLD_ID, NEW_ID), ("3" * 64,)), + ("report-status object-format=md5", (OLD_ID, NEW_ID), ()), + ("report-status object-format=sha1 object-format=sha256", (OLD_ID, NEW_ID), ()), + ], + ids=["sha256-with-sha1-ids", "sha1-with-sha256-ids", "default-with-sha256-ids", "shallow-length", "md5", "two"], + ) + def test_object_ids_that_dont_match_the_object_format(self, capabilities, ids, shallow): + body = make_push_body((*ids, "refs/heads/seer/fix"), capabilities=capabilities, shallow=shallow) + with pytest.raises(GitMalformedRequestBody): + read(body) + + def test_capabilities_on_a_later_update(self): + # receive-pack reads capabilities after a NUL on any update, so one there would slip past the allowlist. + first = f"{OLD_ID} {NEW_ID} refs/heads/seer/fix".encode() + later = f"{OLD_ID} {NEW_ID} refs/heads/seer/other".encode() + body = pkt(first + b"\0report-status") + pkt(later + b"\0push-options") + b"0000" + with pytest.raises(GitMalformedRequestBody, match="first ref update may carry capabilities"): + read(body) + + @pytest.mark.parametrize( + "ref", + [ + b"refs/heads/", + b"refs/heads/a b", + b"refs/heads/a..b", + b"refs/heads/a\x01", + b"refs/heads/x.lock", + b"refs/heads/\xff", + ], + ) + def test_branch_name_git_would_not_take(self, ref): + line = f"{OLD_ID} {NEW_ID} ".encode() + ref + with pytest.raises(GitMalformedRequestBody, match="name git wouldn't take"): + read(pkt(line + b"\0report-status") + b"0000") + + def test_same_ref_twice(self): + body = make_push_body((OLD_ID, NEW_ID, "refs/heads/seer/fix"), ("2" * 40, "3" * 40, "refs/heads/seer/fix")) + with pytest.raises(GitMalformedRequestBody, match="more than once"): + read(body) + + def test_short_reads(self): + body = make_push_body((OLD_ID, NEW_ID, "refs/heads/seer/fix"), shallow=("3" * 40,)) + request = read_push_request({}, Trickle(body)) + assert request.updates[0].ref == b"refs/heads/seer/fix" + + @pytest.mark.parametrize( + "lines", + [ + [b"\0report-status\n", b""], + [b"\0report-status", b"\n"], + ], + ids=["first", "later"], + ) + def test_update_ending_in_a_newline(self, lines): + # git ends no update with a newline, though receive-pack would drop one. + first = f"{OLD_ID} {NEW_ID} refs/heads/seer/fix".encode() + later = f"{OLD_ID} {NEW_ID} refs/heads/seer/other".encode() + with pytest.raises(GitMalformedRequestBody): + read(pkt(first + lines[0]) + pkt(later + lines[1]) + b"0000") + + def test_shallow_commit_without_its_newline(self): + body = pkt(f"shallow {'3' * 40}".encode()) + make_push_body((OLD_ID, NEW_ID, "refs/heads/seer/fix")) + with pytest.raises(GitMalformedRequestBody, match="shallow commit"): + read(body) + + def test_uppercase_pkt_length(self): + # git writes the length in lowercase hex. 0x00ea is a length with a letter in it. + line = f"{OLD_ID} {NEW_ID} refs/heads/{'x' * (0xEA - 4 - 93)}".encode() + assert len(line) + 4 == 0xEA + with pytest.raises(GitMalformedRequestBody, match="not pkt-lines"): + read(b"%04X" % (len(line) + 4) + line + b"0000") + + @pytest.mark.parametrize("encoding", ["identity", "", "gzip", "deflate"]) + def test_content_encoding(self, encoding): + # git sends no Content-Encoding with a push, and compresses only fetches. + with pytest.raises(RpcMalformedRequestHeaders, match="content-encoding"): + read(make_push_body((OLD_ID, NEW_ID, "refs/heads/seer/fix")), {"content-encoding": encoding}) + + @pytest.mark.parametrize( + "method,headers", + [ + ("POST", {"Accept": "application/x-git-receive-pack-result", "Git-Protocol": "version=1"}), + ("POST", {"Content-Type": "application/x-git-receive-pack-request"}), + ("POST", {"User-Agent": "git/2.54.0", "Accept-Encoding": "deflate, gzip"}), + ("GET", {"Accept": "*/*", "Git-Protocol": "version=1", "Pragma": "no-cache"}), + ], + ) + def test_headers_git_sends(self, method, headers): + check_push_headers(method, headers) + + @pytest.mark.parametrize( + "method,headers", + [ + ("POST", {"Git-Protocol": "version=0"}), + ("POST", {"Git-Protocol": "version=2"}), + ("POST", {"Accept": "*/*"}), + ("GET", {"Accept": "application/x-git-receive-pack-result"}), + ("GET", {"Content-Type": "application/x-git-receive-pack-request"}), + ("POST", {"User-Agent": " git/2.54.0"}), + ("POST", {"Accept-Encoding": "gzip\x7f"}), + ("POST", {"User-Agent": ""}), + ], + ) + def test_headers_git_does_not_send(self, method, headers): + with pytest.raises(RpcMalformedRequestHeaders): + check_push_headers(method, headers) + + def test_signed_push(self): + body = pkt(b"push-cert\0report-status") + pkt(b"certificate version 0.1\n") + b"0000" + with pytest.raises(GitOperationNotGranted, match="signed pushes"): + read(body) + + @pytest.mark.parametrize( + "body", + [ + pytest.param(b"", id="empty"), + pytest.param(b"00", id="short-length"), + pytest.param(b"zzzz", id="length-not-hex"), + pytest.param(b"+004", id="length-with-sign"), + pytest.param(b" 0a\n", id="length-with-spaces"), + pytest.param(b"0001", id="delimiter"), + pytest.param(b"0003", id="v2-special-packet"), + pytest.param(b"fff1" + b"x" * 65517, id="pkt-line-too-long"), + pytest.param(pkt(f"{OLD_ID} {NEW_ID} refs/heads/a".encode()), id="no-flush"), + pytest.param(pkt(f"{OLD_ID} {NEW_ID}".encode()) + b"0000", id="no-ref"), + pytest.param(pkt(f"{OLD_ID} {NEW_ID} ".encode()) + b"0000", id="empty-ref"), + pytest.param(pkt(f"{OLD_ID[:-1]} {NEW_ID} refs/heads/a".encode()) + b"0000", id="short-id"), + pytest.param(pkt(f"{'A' * 40} {NEW_ID} refs/heads/a".encode()) + b"0000", id="uppercase-id"), + pytest.param(pkt(f"{OLD_ID} {'b' * 64} refs/heads/a".encode()) + b"0000", id="mixed-id-lengths"), + pytest.param(pkt(b"shallow 123\n") + b"0000", id="bad-shallow"), + pytest.param(pkt(f"{OLD_ID} {NEW_ID} refs/heads/a\0caps \xff".encode("latin-1")) + b"0000", id="caps"), + ], + ) + def test_malformed(self, body): + with pytest.raises(GitMalformedRequestBody): + read(body) + + def test_shallow_line_after_an_update(self): + body = make_push_body((OLD_ID, NEW_ID, "refs/heads/a")) + flush = body.index(b"0000") + with pytest.raises(GitMalformedRequestBody): + read(body[:flush] + pkt(f"shallow {'3' * 40}".encode()) + body[flush:]) + + def test_too_long_before_the_pack(self): + updates = [(OLD_ID, NEW_ID, f"refs/heads/branch-{i:05d}") for i in range(MAX_UPDATES_LENGTH // 90)] + with pytest.raises(GitMalformedRequestBody, match="too long"): + read(make_push_body(*updates)) + + def test_connection_fails(self): + with pytest.raises(ClientClosedRequest): + read_push_request({}, Failing()) + + +def request_for(*updates: tuple[str, str, str], capabilities: str = "report-status side-band-64k") -> PushRequest: + return read(make_push_body(*updates, capabilities=capabilities))[0] + + +class TestRefusedUpdates: + def test_listed_branches(self): + request = request_for((OLD_ID, NEW_ID, "refs/heads/seer/fix"), (ZERO_ID, NEW_ID, "refs/heads/seer/other")) + assert refused_updates(RULES, request) == {} + + def test_force_push_to_a_listed_branch(self): + # Nothing here knows whether the new commit descends from the old one, so the provider's protection decides. + assert refused_updates(RULES, request_for(("3" * 40, NEW_ID, "refs/heads/seer/fix"))) == {} + + def test_probe(self): + assert refused_updates(RULES, read(b"0000")[0]) == {} + + @pytest.mark.parametrize( + "ref,reason", + [ + ("refs/heads/main", "the git grant does not cover this branch"), + ("refs/heads/seer/fix/deeper", "the git grant does not cover this branch"), + ("refs/heads/seer", "the git grant does not cover this branch"), + ("refs/heads/Seer/fix", "the git grant does not cover this branch"), + ("refs/tags/seer/fix", "the git grant does not cover this tag"), + ("refs/pull/1/head", "the git grant allows pushing branches and tags only"), + ("refs/notes/commits", "the git grant allows pushing branches and tags only"), + ], + ) + def test_refs_the_rule_does_not_list(self, ref, reason): + assert refused_updates(RULES, request_for((OLD_ID, NEW_ID, ref))) == {ref.encode(): reason} + + def test_deleting_a_listed_branch(self): + refused = refused_updates(RULES, request_for((OLD_ID, ZERO_ID, "refs/heads/seer/fix"))) + assert refused == {b"refs/heads/seer/fix": "the git grant does not allow deleting a branch"} + + def test_one_refused_update_refuses_the_push(self): + request = request_for((OLD_ID, NEW_ID, "refs/heads/seer/fix"), (OLD_ID, NEW_ID, "refs/heads/main")) + assert refused_updates(RULES, request) == { + b"refs/heads/main": "the git grant does not cover this branch", + b"refs/heads/seer/fix": "refused with the rest of the push", + } + + @pytest.mark.parametrize( + "ref,reason", + [ + ("refs/heads/seer/fix", "the git grant does not cover this branch"), + ("refs/tags/v1", "the git grant does not cover this tag"), + ("refs/notes/commits", "the git grant allows pushing branches and tags only"), + ], + ) + def test_no_rules(self, ref, reason): + assert refused_updates([], request_for((OLD_ID, NEW_ID, ref))) == {ref.encode(): reason} + + @pytest.mark.parametrize( + "old_id,ref,reason", + [ + (ZERO_ID, "refs/heads/seer/new", None), + (OLD_ID, "refs/heads/seer/new", "the git grant allows only creating this branch"), + (ZERO_ID, "refs/heads/main", "the git grant does not cover this branch"), + (ZERO_ID, "refs/tags/seer/new", "the git grant does not cover this tag"), + ], + ) + def test_create_only_pattern(self, old_id, ref, reason): + rules = [PushRule(repository=5, branch_patterns=["seer/.+"], create_only=True)] + refused = refused_updates(rules, request_for((old_id, NEW_ID, ref))) + assert refused == ({} if reason is None else {ref.encode(): reason}) + + def test_create_only_listed_branch(self): + rules = [PushRule(repository=5, branches=["seer/fix"], create_only=True)] + assert refused_updates(rules, request_for((ZERO_ID, NEW_ID, "refs/heads/seer/fix"))) == {} + assert refused_updates(rules, request_for((OLD_ID, NEW_ID, "refs/heads/seer/fix"))) == { + b"refs/heads/seer/fix": "the git grant allows only creating this branch" + } + + @pytest.mark.parametrize( + "old_id,new_id,ref,reason", + [ + (ZERO_ID, NEW_ID, "refs/tags/v1.0", None), + (OLD_ID, NEW_ID, "refs/tags/v1.0", "the git grant allows only creating this tag"), + (OLD_ID, ZERO_ID, "refs/tags/v1.0", "the git grant does not allow deleting a tag"), + (ZERO_ID, NEW_ID, "refs/tags/seer-1.0", "the git grant does not cover this tag"), + (ZERO_ID, NEW_ID, "refs/heads/v1.0", "the git grant does not cover this branch"), + ], + ) + def test_create_only_tag_pattern(self, old_id, new_id, ref, reason): + rules = [PushRule(repository=5, tag_patterns=[r"v[0-9]+\.[0-9]+"], create_only=True)] + refused = refused_updates(rules, request_for((old_id, new_id, ref))) + assert refused == ({} if reason is None else {ref.encode(): reason}) + + def test_listed_tag_moves(self): + # Without create_only a covered tag may move, as a covered branch may. + rules = [PushRule(repository=5, tags=["latest"])] + assert refused_updates(rules, request_for((OLD_ID, NEW_ID, "refs/tags/latest"))) == {} + + def test_one_rule_for_branches_and_tags(self): + rules = [PushRule(repository=5, branch_patterns=["seer/.+"], tags=["seer-1"], create_only=True)] + request = request_for((ZERO_ID, NEW_ID, "refs/heads/seer/fix"), (ZERO_ID, NEW_ID, "refs/tags/seer-1")) + assert refused_updates(rules, request) == {} + + def test_any_rule_allows_an_update(self): + # The agent may start branches under seer/ and keep pushing to the one Seer added after its first push. + rules = [ + PushRule(repository=5, branch_patterns=["seer/.+"], create_only=True), + PushRule(repository=5, branches=["seer/fix"]), + ] + request = request_for((OLD_ID, NEW_ID, "refs/heads/seer/fix"), (ZERO_ID, NEW_ID, "refs/heads/seer/next")) + assert refused_updates(rules, request) == {} + assert refused_updates(list(reversed(rules)), request) == {} + + def test_reason_of_the_rule_the_update_came_closest_to_passing(self): + rules = [ + PushRule(repository=5, branches=["other"]), + PushRule(repository=5, branch_patterns=["seer/.+"], create_only=True), + ] + assert refused_updates(rules, request_for((OLD_ID, NEW_ID, "refs/heads/seer/old"))) == { + b"refs/heads/seer/old": "the git grant allows only creating this branch" + } + assert refused_updates(rules, request_for((OLD_ID, ZERO_ID, "refs/heads/seer/old"))) == { + b"refs/heads/seer/old": "the git grant does not allow deleting a branch" + } + + @pytest.mark.parametrize( + "capabilities", + [ + "report-status side-band-64k quiet object-format=sha1 agent=git/2.54.0", + "report-status-v2 side-band-64k atomic session-id=abc", + "", + ], + ) + def test_capabilities_git_sends(self, capabilities): + assert ( + refused_updates(RULES, request_for((OLD_ID, NEW_ID, "refs/heads/seer/fix"), capabilities=capabilities)) + == {} + ) + + @pytest.mark.parametrize( + "capability,reason", + [ + ("future-capability", "the git endpoint does not relay the future-capability capability"), + ("quiet=1", "the git endpoint does not relay the quiet capability"), + # git asks for neither in a push. + ("side-band", "the git endpoint does not relay the side-band capability"), + ("ofs-delta", "the git endpoint does not relay the ofs-delta capability"), + ("agent", "the git endpoint does not relay the agent capability"), + ], + ) + def test_capability_git_does_not_send(self, capability, reason): + # Anything unknown could change what the push does, so it refuses the push. + request = request_for((OLD_ID, NEW_ID, "refs/heads/seer/fix"), capabilities=f"report-status {capability}") + assert refused_updates(RULES, request) == {b"refs/heads/seer/fix": reason} + + def test_push_options(self): + request = request_for((OLD_ID, NEW_ID, "refs/heads/seer/fix"), capabilities="report-status push-options") + assert refused_updates(RULES, request) == { + b"refs/heads/seer/fix": "the git endpoint does not relay push options" + } + + +def unband(response: bytes) -> bytes: + """The band 1 data of a side band response, checking that it ends with a flush.""" + data, position = b"", 0 + while response[position : position + 4] != b"0000": + length = int(response[position : position + 4], 16) + assert response[position + 4] == 1 + data += response[position + 5 : position + length] + position += length + assert position + 4 == len(response) + return data + + +class TestRefusalReport: + REFUSED = {b"refs/heads/main": "the git grant does not cover this branch"} + REPORT = pkt(b"unpack ok\n") + pkt(b"ng refs/heads/main the git grant does not cover this branch\n") + b"0000" + + def test_report_status(self): + request = request_for((OLD_ID, NEW_ID, "refs/heads/main"), capabilities="report-status") + assert refusal_report(request, self.REFUSED) == self.REPORT + + def test_report_status_v2(self): + request = request_for((OLD_ID, NEW_ID, "refs/heads/main"), capabilities="report-status-v2") + assert refusal_report(request, self.REFUSED) == self.REPORT + + def test_side_band_64k(self): + request = request_for((OLD_ID, NEW_ID, "refs/heads/main"), capabilities="report-status side-band-64k") + response = refusal_report(request, self.REFUSED) + assert response == pkt(b"\1" + self.REPORT) + b"0000" + + def test_side_band_splits_at_1000_bytes(self): + updates = [(OLD_ID, NEW_ID, f"refs/heads/branch-{i:03d}") for i in range(40)] + request = request_for(*updates, capabilities="report-status side-band") + refused = {update.ref: "the git grant does not cover this branch" for update in request.updates} + response = refusal_report(request, refused) + assert response is not None + assert max(int(response[i : i + 4], 16) for i in range(0, len(response) - 4, 1000)) <= 1000 + assert unband(response).startswith(pkt(b"unpack ok\n")) + assert unband(response).count(b"ng refs/heads/branch-") == 40 + + def test_every_update_in_order(self): + request = request_for((OLD_ID, NEW_ID, "refs/heads/seer/fix"), (OLD_ID, NEW_ID, "refs/heads/main")) + response = refusal_report(request, refused_updates(RULES, request)) + assert response is not None + assert unband(response) == ( + pkt(b"unpack ok\n") + + pkt(b"ng refs/heads/seer/fix refused with the rest of the push\n") + + pkt(b"ng refs/heads/main the git grant does not cover this branch\n") + + b"0000" + ) + + def test_client_asked_for_no_report(self): + request = request_for((OLD_ID, NEW_ID, "refs/heads/main"), capabilities="side-band-64k quiet") + assert refusal_report(request, self.REFUSED) is None + + +class TestReplayedBody: + @pytest.mark.parametrize("size", [1, 3, 5, 64, 1024]) + def test_head_then_rest(self, size): + body = ReplayedBody(b"head", io.BytesIO(b"rest of the body")) + chunks = [] + while chunk := body.read(size): + chunks.append(chunk) + assert b"".join(chunks) == b"headrest of the body" + assert all(len(chunk) <= size for chunk in chunks) + + def test_read_all(self): + assert ReplayedBody(b"head", io.BytesIO(b"rest")).read() == b"headrest" + + def test_empty_head(self): + assert ReplayedBody(b"", io.BytesIO(b"rest")).read(64) == b"rest" + + +class Recording(io.BytesIO): + """A body that tells how far it was read.""" + + +class TestCheckPush: + def check(self, body: bytes, rules=RULES, headers: dict[str, str] | None = None) -> tuple[bytes, Recording]: + stream = Recording(body) + relayed = check_push(rules, headers or {}, stream) + return relayed.read(), stream + + def test_allowed_push_relays_the_whole_body(self): + body = make_push_body((OLD_ID, NEW_ID, "refs/heads/seer/fix")) + relayed, _ = self.check(body) + assert relayed == body + + def test_probe(self): + relayed, _ = self.check(b"0000") + assert relayed == b"0000" + + def test_no_updates_with_more_after_them(self): + # A provider reads nothing after a flush that updates no ref, so whatever follows has no business there. + with pytest.raises(GitMalformedRequestBody, match="updates no ref but goes on"): + self.check(b"0000" + PACK) + + def test_refused_push_is_read_to_the_end(self): + stream = Recording(make_push_body((OLD_ID, NEW_ID, "refs/heads/main"), capabilities="report-status")) + with pytest.raises(PushRefused) as exc_info: + check_push(RULES, {}, stream) + assert stream.read() == b"" + assert exc_info.value.code == "git_operation_not_granted" + assert exc_info.value.detail == ( + "The git grant does not allow this push: refs/heads/main (the git grant does not cover this branch)." + ) + assert exc_info.value.report == ( + pkt(b"unpack ok\n") + pkt(b"ng refs/heads/main the git grant does not cover this branch\n") + b"0000" + ) + + def test_no_rules_refuse_every_update(self): + with pytest.raises(PushRefused, match="does not cover this branch"): + self.check(make_push_body((OLD_ID, NEW_ID, "refs/heads/seer/fix")), rules=[]) + + def test_no_body(self): + with pytest.raises(GitMalformedRequestBody, match="has no body"): + check_push(RULES, {}, None) + + @pytest.mark.parametrize( + "body,headers,error", + [ + (b"garbage and then some", {}, GitMalformedRequestBody), + ( + make_push_body((OLD_ID, NEW_ID, "refs/heads/seer/fix")), + {"Content-Encoding": "gzip"}, + RpcMalformedRequestHeaders, + ), + (pkt(b"push-cert\0report-status") + b"0000" + PACK, {}, PushRefused), + ], + ids=["malformed", "compressed", "signed"], + ) + def test_other_refusals_are_read_to_the_end(self, body, headers, error): + stream = Recording(body) + with pytest.raises(error): + check_push(RULES, headers, stream) + assert stream.read() == b"" + + def test_signed_push_on_a_side_band(self): + # git prints band 3 as "remote: " before it stops, where a 403 would show only its status. + body = pkt(b"push-cert\0report-status side-band-64k") + pkt(b"certificate version 0.1\n") + b"0000" + with pytest.raises(PushRefused) as exc_info: + self.check(body) + assert exc_info.value.report == pkt(b"\3The git endpoint does not relay signed pushes.\n") + b"0000" + + def test_signed_push_without_a_side_band(self): + with pytest.raises(PushRefused) as exc_info: + self.check(pkt(b"push-cert\0report-status") + b"0000") + assert exc_info.value.report is None + + def test_body_that_ends_early(self): + body = make_push_body((OLD_ID, NEW_ID, "refs/heads/seer/fix")) + with pytest.raises(GitMalformedRequestBody, match="ended before its ref updates did"): + self.check(body[:20]) + + def test_long_ref_is_cut_short_in_the_detail(self): + ref = "refs/notes/" + "x" * 1000 + with pytest.raises(PushRefused) as exc_info: + self.check(make_push_body((OLD_ID, NEW_ID, ref), capabilities="report-status")) + assert exc_info.value.detail == ( + f"The git grant does not allow this push: {ref[:200]}... " + "(the git grant allows pushing branches and tags only)." + ) + # The report names the whole ref, since git matches it to the ref it pushed. + assert ref.encode() in (exc_info.value.report or b"") + + @pytest.mark.parametrize( + "ref", + ["HEAD", "seer/fix", "refs/heads/" + "x" * 256, "refs/tags/" + "x" * 256, "refs/tags/-v1", "refs/tags/HEAD"], + ids=["head", "bare", "long-branch", "long-tag", "dash-tag", "head-tag"], + ) + def test_ref_git_would_not_send(self, ref): + # git sends refs under refs/, and a branch or tag name past the cap would make a rule's pattern slow. + with pytest.raises(GitMalformedRequestBody): + self.check(make_push_body((OLD_ID, NEW_ID, ref))) + + def test_longest_branch_name(self): + ref = "refs/heads/seer/" + "x" * 250 + rules = [PushRule(repository=5, branch_patterns=["seer/x+"])] + relayed, _ = self.check(make_push_body((OLD_ID, NEW_ID, ref)), rules=rules) + assert ref.encode() in relayed diff --git a/tests/unit/test_git_push_adversarial.py b/tests/unit/test_git_push_adversarial.py new file mode 100644 index 00000000..887fd69a --- /dev/null +++ b/tests/unit/test_git_push_adversarial.py @@ -0,0 +1,135 @@ +"""Pushes built to slip past the push check, each of which it must refuse.""" + +import io +import time + +import pytest + +from scm.errors import GitMalformedRequestBody, GitOperationNotGranted +from scm.git.push import MAX_UPDATES_LENGTH, PushRefused, check_push +from scm.git.relay import GitRelay +from scm.git.types import PushRule +from tests.unit.git_builders import NEW_ID, OLD_ID, make_push_body, make_repository, pkt +from tests.unit.git_upstream import running_upstream +from tests.unit.test_git_server import PUSH_GRANT, Recorder, make_server, request + +RULES = [PushRule(repository=5, branches=["seer/fix"])] + + +def first_update(capabilities: bytes, ref: bytes = b"refs/heads/seer/fix") -> bytes: + return pkt(f"{OLD_ID} {NEW_ID} ".encode() + ref + b"\0" + capabilities) + + +class TestCapabilityValues: + @pytest.mark.parametrize( + "capabilities", + [ + b"report-status agent=git/2.54.0\0push-options", + b"report-status session-id=x\0push-options", + b"report-status agent=git/2.54.0\x01push-options", + b"report-status session-id=\x1b[2Jx", + ], + ids=["nul-in-agent", "nul-in-session-id", "control-in-agent", "escape-in-session-id"], + ) + def test_value_git_would_not_send_is_refused(self, capabilities): + # git sends capability values as printable ASCII. A NUL or control byte in an allowed one can hide a + # capability such as push-options from the check, while a receive-pack that reads the whole line sees it. + body = first_update(capabilities) + b"0000PACK" + with pytest.raises((GitOperationNotGranted, GitMalformedRequestBody)): + check_push(RULES, {}, io.BytesIO(body)) + + +def long_capability_push() -> bytes: + """A push whose one unsupported capability takes half the update budget, followed by as many updates as fit.""" + lines = [first_update(b"report-status " + b"x" * (MAX_UPDATES_LENGTH // 2), ref=b"refs/heads/r0")] + index = 1 + while ( + sum(map(len, lines)) + len(line := pkt(f"{OLD_ID} {NEW_ID} refs/heads/r{index}".encode())) + 4 + <= MAX_UPDATES_LENGTH + ): + lines.append(line) + index += 1 + return b"".join(lines) + b"0000" + + +class TestRefusalSize: + def test_report_does_not_repeat_a_long_capability_name_per_update(self): + # Every refused ref's "ng" line repeats the reason, so a reason that quoted a long capability whole would + # answer a small push with megabytes. + body = long_capability_push() + with pytest.raises(PushRefused) as exc_info: + check_push(RULES, {}, io.BytesIO(body)) + assert exc_info.value.report is not None + assert len(exc_info.value.report) <= 2 * len(body) + + def test_detail_cuts_a_long_capability_name_short(self): + # The client chooses the capability's name, and the detail reaches logs and Sentry issues. + with pytest.raises(PushRefused) as exc_info: + check_push(RULES, {}, io.BytesIO(long_capability_push())) + assert len(exc_info.value.detail) <= 1000 + + +class TestRefusalDetail: + @pytest.mark.parametrize( + "ref", + [b"refs/tags/x\nERROR forged log line", b"refs/tags/x\x1b[2J", b"refs/pull/1\rhead"], + ids=["newline", "escape", "carriage-return"], + ) + def test_detail_holds_no_control_characters(self, ref): + # git refuses such a name as a "funny refname" and never sends one, and the detail reaches logs. + body = first_update(b"report-status", ref=ref) + b"0000" + with pytest.raises((GitOperationNotGranted, GitMalformedRequestBody)) as exc_info: + check_push(RULES, {}, io.BytesIO(body)) + detail = exc_info.value.detail + assert not any(ord(character) < 0x20 or ord(character) == 0x7F for character in detail), detail + + +class TestPatternCost: + def test_long_branch_name_does_not_pin_the_worker(self): + # An ordinary-looking pattern with three wildcards is cubic in the name's length: 1,500 characters take + # seconds, and 60,000 would take days. + rules = [PushRule(repository=5, branch_patterns=["seer/.+-.+-.+-[0-9]+"])] + body = make_push_body((OLD_ID, NEW_ID, "refs/heads/seer/" + "-" * 1500 + "x")) + started = time.perf_counter() + with pytest.raises((GitOperationNotGranted, GitMalformedRequestBody)): + check_push(rules, {}, io.BytesIO(body)) + assert time.perf_counter() - started < 0.25 + + +class TestHeaders: + def test_second_content_encoding_header_is_not_sent_upstream_unchecked(self): + # A mapping can hold both spellings and the relay passes on only one, so a body checked as plain could go + # upstream marked gzip. + body = make_push_body((OLD_ID, NEW_ID, "refs/heads/seer/fix")) + with running_upstream() as upstream: + recorder = Recorder( + repository=make_repository(provider_name="github_enterprise", web_base_url=upstream.url) + ) + request( + make_server(recorder, GitRelay(ca_bundle=upstream.ca_bundle)), + PUSH_GRANT, + method="POST", + route="git-receive-pack", + query="", + headers={"Content-Encoding": "identity", "content-encoding": "gzip"}, + body=io.BytesIO(body), + ) + assert [r.headers.get("Content-Encoding") for r in upstream.requests] in ([], ["identity"]) + + def test_git_protocol_git_does_not_send_on_a_push(self): + # git sends no Git-Protocol header on a push, or version=1, since a push has no protocol v2. + body = make_push_body((OLD_ID, NEW_ID, "refs/heads/seer/fix")) + with running_upstream() as upstream: + recorder = Recorder( + repository=make_repository(provider_name="github_enterprise", web_base_url=upstream.url) + ) + request( + make_server(recorder, GitRelay(ca_bundle=upstream.ca_bundle)), + PUSH_GRANT, + method="POST", + route="git-receive-pack", + query="", + headers={"Git-Protocol": "version=2:future-key=value"}, + body=io.BytesIO(body), + ) + assert all(r.headers.get("Git-Protocol") in (None, "version=0", "version=1") for r in upstream.requests) diff --git a/tests/unit/test_git_push_adversarial_2.py b/tests/unit/test_git_push_adversarial_2.py new file mode 100644 index 00000000..ad987d21 --- /dev/null +++ b/tests/unit/test_git_push_adversarial_2.py @@ -0,0 +1,178 @@ +"""More pushes built to slip past the push check, each of which it must refuse.""" + +import io + +import pytest + +from scm.errors import GitMalformedRequestBody, GitOperationNotGranted, RpcMalformedRequestHeaders +from scm.git.push import check_push +from scm.git.relay import DEFAULT_CHUNK_SIZE, GitRelay +from scm.git.types import PushRule +from tests.unit.git_builders import NEW_ID, OLD_ID, make_push_body, make_repository, pkt +from tests.unit.git_upstream import running_upstream +from tests.unit.test_git_server import PUSH_GRANT, Recorder, make_server, request + +RULES = [PushRule(repository=5, branches=["seer/fix"])] +ALLOWED_PUSH = make_push_body((OLD_ID, NEW_ID, "refs/heads/seer/fix")) +GIT_PROTOCOL_VALUES = (None, "version=0", "version=1") + + +def send_push(upstream, headers: dict[str, str]) -> tuple[Recorder, int]: + recorder = Recorder(repository=make_repository(provider_name="github_enterprise", web_base_url=upstream.url)) + response = request( + make_server(recorder, GitRelay(ca_bundle=upstream.ca_bundle)), + PUSH_GRANT, + method="POST", + route="git-receive-pack", + query="", + headers=headers, + body=io.BytesIO(ALLOWED_PUSH), + ) + b"".join(response.content) + return recorder, response.status_code + + +class TestHeaderValuesAsRelayed: + """Header values are checked as the relay sends them on, not stripped. + + str.strip() removes more than the spaces and tabs an HTTP server trims: U+00A0 and U+0085, which reach a WSGI + application as the header bytes 0xA0 and 0x85, and the controls 0x0B, 0x0C and 0x1C to 0x1F. So a check of + stripped values would pass values git never sends. + """ + + @pytest.mark.parametrize( + "name,value", + [ + ("Git-Protocol", "version=1\xa0"), + ("Git-Protocol", "version=1\x85"), + ("Git-Protocol", "version=1\x0c"), + ("Git-Protocol", "version=1\x1f"), + ("Accept", "application/x-git-receive-pack-result\xa0"), + ("Content-Type", "application/x-git-receive-pack-request\x85"), + ("Content-Encoding", "identity\xa0"), + ("Content-Encoding", "\x85"), + ], + ) + def test_value_padded_with_what_only_strip_removes_is_refused(self, name, value): + with pytest.raises((RpcMalformedRequestHeaders, GitOperationNotGranted)): + check_push(RULES, {name: value}, io.BytesIO(ALLOWED_PUSH)) + + @pytest.mark.parametrize("value", ["version=1\xa0", "version=1\x85", "version=1\x0c"]) + def test_padded_git_protocol_never_reaches_the_provider(self, value): + with running_upstream() as upstream: + send_push(upstream, {"Git-Protocol": value}) + assert all(r.headers.get("Git-Protocol") in GIT_PROTOCOL_VALUES for r in upstream.requests) + + @pytest.mark.parametrize( + "headers", + [{"Git-Protocol": "\xa0version=1"}, {"Accept": "\x85application/x-git-receive-pack-result"}], + ids=["git-protocol", "accept"], + ) + def test_value_padded_in_front_is_refused_as_the_clients_fault(self, headers): + # requests refuses to send a value that starts with whitespace, so letting it past the check would answer the + # client's mistake with a 502 that blames the provider, after fetching a token. + with running_upstream() as upstream: + recorder, status = send_push(upstream, headers) + assert recorder.error_codes == ["rpc_malformed_request_headers"] + assert status == 400 + assert recorder.token_lookups == [] + + +class TestRefusalDetailUnicode: + @pytest.mark.parametrize( + "ref", + [ + "refs/tags/x\u2028ERROR-forged-log-line", + "refs/tags/x\u2029y", + "refs/tags/x\u0085ERROR-forged-log-line", + "refs/tags/x\u009b2Jy", + "refs/heads/main\u202egnp.exe", + "refs/heads/seer/fix\u200b", + ], + ids=["line-separator", "paragraph-separator", "next-line", "c1-csi", "right-to-left-override", "zero-width"], + ) + def test_detail_holds_only_printable_characters(self, ref): + # git takes any byte from 0x80 up in a ref, so a name can decode to a line separator, a C1 control or a + # bidirectional override, and the detail goes to logs and to a Sentry issue's title. + body = pkt(f"{OLD_ID} {NEW_ID} {ref}".encode() + b"\0report-status") + b"0000" + with pytest.raises((GitOperationNotGranted, GitMalformedRequestBody)) as exc_info: + check_push(RULES, {}, io.BytesIO(body)) + assert exc_info.value.detail.isprintable(), ascii(exc_info.value.detail) + + def test_route_in_the_detail_is_printable_and_cut_short(self): + # The caller's web server percent-decodes the route, so it can hold escapes, carriage returns and as many + # bytes as a URL holds, and GitRouteNotSupported's detail reaches the same logs as a ref's name. + recorder = Recorder() + route = "git-receive-pack\x1b[2J\rERROR forged log line " + "x" * 5000 + response = request(make_server(recorder), PUSH_GRANT, method="POST", route=route, query="") + assert response.status_code == 404 + detail = recorder.errors[0][0].detail # type: ignore[attr-defined] + assert detail.isprintable(), ascii(detail[:80]) + assert len(detail) <= 1000 + + +class TestShallowClone: + def test_push_from_a_clone_with_many_shallow_commits(self): + # git lists every shallow commit ahead of a push's updates, so a depth-1 clone of 1,400 branches sends about + # 74 KB of them, more than the updates' own budget. + shallow = tuple(f"{index:040x}" for index in range(1, 1401)) + body = make_push_body((OLD_ID, NEW_ID, "refs/heads/seer/fix"), shallow=shallow) + assert check_push(RULES, {}, io.BytesIO(body)).read() == body + + +class TestRefAdvertisementOfAPush: + @pytest.mark.parametrize( + "name,value,sent_by_git", + [ + ("Git-Protocol", "version=2:future-key=value", GIT_PROTOCOL_VALUES), + ("Content-Encoding", "gzip", (None,)), + ], + ids=["git-protocol", "content-encoding"], + ) + def test_header_value_git_does_not_send_stays_behind(self, name, value, sent_by_git): + # A push's first request, GET info/refs?service=git-receive-pack, goes to the provider with the same headers. + # git asks for no protocol version there, or for version 1, and sends no Content-Encoding. + with running_upstream() as upstream: + recorder = Recorder( + repository=make_repository(provider_name="github_enterprise", web_base_url=upstream.url) + ) + response = request( + make_server(recorder, GitRelay(ca_bundle=upstream.ca_bundle)), + PUSH_GRANT, + query="service=git-receive-pack", + headers={name: value}, + ) + b"".join(response.content) + assert all(r.headers.get(name) in sent_by_git for r in upstream.requests) + + +class Endless: + """A push whose body never ends, as a chunked request body a caller streams can be.""" + + CHUNK = bytes(DEFAULT_CHUNK_SIZE) + + def __init__(self, head: bytes, give_up_after: int) -> None: + self.head = head + self.give_up_after = give_up_after + self.supplied = 0 + + def read(self, size: int = -1, /) -> bytes: + if self.head: + data, self.head = self.head[:size], self.head[size:] + return data + if self.supplied >= self.give_up_after: + return b"" + data = self.CHUNK[:size] + self.supplied += len(data) + return data + + +class TestDrain: + def test_refused_push_is_not_read_without_bound(self): + # A refused push is read to its end so that git reads the answer, and a client that never stops would hold + # the worker. 8 GiB is past any provider's push limit, so nothing a provider would take needs reading that far. + give_up_after = 8 * 1024**3 + body = Endless(b"zzzz", give_up_after) + with pytest.raises(GitMalformedRequestBody): + check_push(RULES, {}, body) + assert body.supplied < give_up_after, "the refused push was read for 8 GiB and would have been read on" diff --git a/tests/unit/test_git_push_adversarial_3.py b/tests/unit/test_git_push_adversarial_3.py new file mode 100644 index 00000000..9a27b162 --- /dev/null +++ b/tests/unit/test_git_push_adversarial_3.py @@ -0,0 +1,27 @@ +"""Tag pushes built to slip past the push check, each of which it must refuse.""" + +import io + +import pytest + +from scm.git.push import PushRefused, check_push +from scm.git.types import PushRule +from tests.unit.git_builders import NEW_ID, ZERO_ID, make_push_body + + +class TestPatternDigits: + @pytest.mark.parametrize( + ("field", "ref"), + [ + ("tag_patterns", "refs/tags/v１.０"), + ("tag_patterns", "refs/tags/v١.٠"), + ("branch_patterns", "refs/heads/v１.０"), + ], + ids=["fullwidth-tag", "arabic-indic-tag", "fullwidth-branch"], + ) + def test_digit_class_does_not_cover_a_lookalike_of_a_release(self, field, ref): + # Without re.ASCII, \d matches every Unicode decimal digit, so a rule written for v1.0 would let its holder + # create a tag that renders like v1.0 and is a different ref. + rules = [PushRule(repository=5, create_only=True, **{field: [r"v\d+\.\d+"]})] + with pytest.raises(PushRefused): + check_push(rules, {}, io.BytesIO(make_push_body((ZERO_ID, NEW_ID, ref)))) diff --git a/tests/unit/test_git_relay.py b/tests/unit/test_git_relay.py index fe304ed5..29a1bbf3 100644 --- a/tests/unit/test_git_relay.py +++ b/tests/unit/test_git_relay.py @@ -14,10 +14,8 @@ from requests.adapters import HTTPAdapter from scm.errors import ( - GitOperationNotGranted, GitRemoteNotHttps, GitRemoteRefused, - GitRepositoryNotGranted, GitRouteNotSupported, ResourceBadGateway, ResourceCertificateInvalid, @@ -25,11 +23,10 @@ ResourceServiceUnavailable, SCMCodedError, ) -from scm.git.relay import GitRelay, _UpstreamStream, authorize_request +from scm.git.relay import GitRelay, _UpstreamStream from scm.secret import Secret from scm.types import GitRemote from tests.unit.error_reports import assert_not_in_frames, assert_not_reported, reported_local -from tests.unit.git_builders import make_grant from tests.unit.git_upstream import ( ClosingAdapter, HostRefused, @@ -737,65 +734,3 @@ def handler(h: UpstreamHandler) -> None: response = fetch_refs(relay, upstream) response.content.close() # type: ignore[attr-defined] assert upstream_saw_close.wait(WAIT) - - -FETCH_ROUTES = [("GET", "info/refs", "service=git-upload-pack"), ("POST", "git-upload-pack", "")] -PUSH_ROUTES = [("GET", "info/refs", "service=git-receive-pack"), ("POST", "git-receive-pack", "")] - - -class TestAuthorizeRequest: - @pytest.mark.parametrize( - "operation,route", - [("fetch", route) for route in FETCH_ROUTES] + [("push", route) for route in PUSH_ROUTES], - ) - def test_grant_reaches_its_operations_routes(self, operation, route): - assert authorize_request(make_grant(operation=operation), 5, *route) == operation - - @pytest.mark.parametrize( - "operation,route", - [("fetch", route) for route in PUSH_ROUTES] + [("push", route) for route in FETCH_ROUTES], - ) - def test_grant_cannot_reach_the_other_operations_routes(self, operation, route): - other = "push" if operation == "fetch" else "fetch" - with pytest.raises(GitOperationNotGranted, match=f"allows {operation}, not {other}"): - authorize_request(make_grant(operation=operation), 5, *route) - - @pytest.mark.parametrize( - "method,route,query", - [ - ("GET", "info/refs", ""), - ("GET", "info/refs", "service=git-upload-pack&x=1"), - ("GET", "info/refs", "service=git-upload-archive"), - ("POST", "info/refs", "service=git-upload-pack"), - ("GET", "git-upload-pack", ""), - ("POST", "git-upload-pack", "service=git-upload-pack"), - ("post", "git-upload-pack", ""), - ("POST", "git-upload-archive", ""), - ("POST", "info/lfs/objects/batch", ""), - ("GET", "HEAD", ""), - ("GET", "objects/info/packs", ""), - ("GET", "../info/refs", "service=git-upload-pack"), - ("POST", "/git-upload-pack", ""), - ], - ) - def test_routes_that_are_not_git(self, method, route, query): - for operation in ("fetch", "push"): - with pytest.raises(GitRouteNotSupported, match="not a git fetch or push route"): - authorize_request(make_grant(operation=operation), 5, method, route, query) - - def test_repository_not_in_grant(self): - with pytest.raises(GitRepositoryNotGranted, match="does not cover this repository"): - authorize_request(make_grant(), 6, "POST", "git-upload-pack", "") - - def test_repository_named_another_way_is_not_in_grant(self): - # The grant and the request name repositories the same way; matching a database id to a provider id - # would mean resolving every repository the grant lists. - grant = make_grant(repositories=[("github", "123")]) - with pytest.raises(GitRepositoryNotGranted): - authorize_request(grant, 5, "POST", "git-upload-pack", "") - assert authorize_request(grant, ("github", "123"), "POST", "git-upload-pack", "") == "fetch" - - def test_any_listed_repository(self): - grant = make_grant(repositories=[5, 6, ("gitlab", "gitlab.com:7")]) - assert authorize_request(grant, 6, "POST", "git-upload-pack", "") == "fetch" - assert authorize_request(grant, ("gitlab", "gitlab.com:7"), "POST", "git-upload-pack", "") == "fetch" diff --git a/tests/unit/test_git_server.py b/tests/unit/test_git_server.py index b6dc082e..8b2cef02 100644 --- a/tests/unit/test_git_server.py +++ b/tests/unit/test_git_server.py @@ -7,25 +7,39 @@ from unittest.mock import MagicMock from urllib.parse import urlsplit +import jwt import pytest from scm.errors import ( ClientClosedRequest, GitGrantKeyError, + GitOperationNotGranted, GitRemoteRefused, + GitRepositoryNotGranted, + GitRouteNotSupported, RateLimitExceeded, ResourceGatewayTimeout, ResourceServiceUnavailable, ) -from scm.git.grant import sign_grant +from scm.git.grant import key_id, sign_grant from scm.git.relay import GitRelay, RelayResponse, ResponseBody -from scm.git.server import _STATUS, GitServer -from scm.git.types import GRANT_HEADER, GitOperation, Readable +from scm.git.server import _STATUS, GitServer, _authorize +from scm.git.types import GRANT_HEADER, FetchRule, GitOperation, PushRule, Readable from scm.rpc.types import StreamResponse from scm.secret import Secret from scm.types import GitRemote, Provider, Repository, RepositoryId from tests.unit.error_reports import assert_not_in_frames, assert_not_reported, reported_local -from tests.unit.git_builders import make_grant, make_key_pair, make_provider, make_repository +from tests.unit.git_builders import ( + NEW_ID, + OLD_ID, + VERSION_1_PUSH_CLAIMS, + make_grant, + make_key_pair, + make_provider, + make_push_body, + make_repository, + pkt, +) from tests.unit.git_upstream import ( ClosingAdapter, HostRefused, @@ -226,7 +240,7 @@ def test_route_that_is_not_git(self): def test_operation_the_grant_does_not_allow(self): recorder = Recorder() - assert_refused(push(make_server(recorder), signed_grant()), 403, "allows fetch, not push") + assert_refused(push(make_server(recorder), signed_grant()), 403, "allows fetch on this repository, not push") assert recorder.repository_lookups == [] assert recorder.token_lookups == [] assert recorder.counts == [ @@ -234,6 +248,13 @@ def test_operation_the_grant_does_not_allow(self): ] assert recorder.errors[0][1] == {"repository_id": "5", "organization_id": "1", "referrer": REFERRER} + def test_fetch_rule_that_lists_branches(self): + recorder = Recorder() + grant = signed_grant(rules=[FetchRule(repository=5, branches=["main"])]) + assert_refused(request(make_server(recorder), grant), 403, "cannot keep a fetch to a rule's branches") + assert recorder.repository_lookups == [] + assert recorder.token_lookups == [] + def test_repository_the_grant_does_not_list(self): recorder = Recorder() assert_refused(request(make_server(recorder), signed_grant(), repository_id=6), 403, "does not cover") @@ -296,10 +317,10 @@ def test_no_token(self): ("sentry.scm.git.request.failed", {"referrer": REFERRER, "code": "git_token_unavailable"}) ] - def test_token_asked_for_the_grants_operation(self): + def test_token_asked_for_the_requests_operation(self): recorder = Recorder() - push(make_server(recorder), signed_grant(operation="push")) - assert recorder.token_lookups == [(make_repository(), "push")] + request(make_server(recorder), signed_grant()) + assert recorder.token_lookups == [(make_repository(), "fetch")] def test_forwards_with_remote_and_token(self): recorder = Recorder() @@ -618,17 +639,17 @@ def test_counts_bytes_each_way_end_to_end(self): relay = GitRelay(ca_bundle=upstream.ca_bundle) response = request( make_server(recorder, relay), - signed_grant(operation="push"), + signed_grant(), method="POST", - route="git-receive-pack", + route="git-upload-pack", query="", headers={"Content-Length": "8"}, body=io.BytesIO(b"00000000"), ) assert b"".join(response.content) == b"00000000" # The fake upstream echoes the body. assert recorder.bytes == { - ("sentry.scm.git.request_size", "push"): 8, - ("sentry.scm.git.response_size", "push"): 8, + ("sentry.scm.git.request_size", "fetch"): 8, + ("sentry.scm.git.response_size", "fetch"): 8, } def test_relays_end_to_end(self): @@ -721,3 +742,272 @@ def test_token_that_is_not_a_secret_is_refused_and_left_out(self): assert recorder.counts == [ ("sentry.scm.git.request.failed", {"referrer": REFERRER, "code": "unhandled_exception"}) ] + + +PUSH_GRANT = sign_grant(KEY.private, make_grant(rules=[PushRule(repository=5, branches=["seer/fix"])])) + + +def push_body(ref: str, capabilities: str = "report-status side-band-64k") -> bytes: + return make_push_body((OLD_ID, NEW_ID, ref), capabilities=capabilities) + + +class Unreliable(io.BytesIO): + def read(self, size: int | None = -1, /) -> bytes: + raise ConnectionResetError() + + +class TestPushRules: + """A push under a push rule reaches the provider only if the rule allows every ref it updates.""" + + def send(self, server: GitServer, body: Readable, headers: dict[str, str] | None = None) -> StreamResponse: + return request( + server, PUSH_GRANT, method="POST", route="git-receive-pack", query="", headers=headers, body=body + ) + + @pytest.mark.parametrize("sized", [True, False], ids=["content-length", "chunked"]) + def test_listed_branch_end_to_end(self, sized): + body = push_body("refs/heads/seer/fix") + with running_upstream() as upstream: + recorder = Recorder( + repository=make_repository(provider_name="github_enterprise", web_base_url=upstream.url) + ) + response = self.send( + make_server(recorder, GitRelay(ca_bundle=upstream.ca_bundle)), + io.BytesIO(body), + headers={"Content-Length": str(len(body))} if sized else None, + ) + # The fake upstream echoes the body, so the provider got the updates the server read and the pack after. + assert b"".join(response.content) == body + assert recorder.token_lookups == [(recorder.repository, "push")] + assert recorder.bytes[("sentry.scm.git.request_size", "push")] == len(body) + + def test_probe(self): + recorder = Recorder() + server = make_server(recorder) + assert self.send(server, io.BytesIO(b"0000")).status_code == 200 + assert server.relay.forward.call_args.kwargs["body"].read() == b"0000" # type: ignore[attr-defined] + + def test_info_refs(self): + recorder = Recorder() + response = request(make_server(recorder), PUSH_GRANT, query="service=git-receive-pack") + assert b"".join(response.content) == b"refs" + + def test_unlisted_branch_gets_a_report(self): + recorder = Recorder() + server = make_server(recorder) + body = io.BytesIO(push_body("refs/heads/main")) + response = self.send(server, body) + assert response.status_code == 200 + assert response.headers["Content-Type"] == "application/x-git-receive-pack-result" + report = b"".join(response.content) + assert int(response.headers["Content-Length"]) == len(report) + assert report == ( + pkt( + b"\1" + + pkt(b"unpack ok\n") + + pkt(b"ng refs/heads/main the git grant does not cover this branch\n") + + b"0000" + ) + + b"0000" + ) + # The pack is read to the end, so git gets to read the report rather than a broken pipe. + assert body.read() == b"" + assert recorder.repository_lookups == [] + assert recorder.token_lookups == [] + server.relay.forward.assert_not_called() # type: ignore[attr-defined] + assert recorder.counts == [ + ("sentry.scm.git.request.failed", {"referrer": REFERRER, "code": "git_operation_not_granted"}) + ] + [(error, context)] = recorder.errors + assert error.detail == ( # type: ignore[attr-defined] + "The git grant does not allow this push: refs/heads/main (the git grant does not cover this branch)." + ) + assert context == {"repository_id": "5", "organization_id": "1", "referrer": REFERRER, "operation": "push"} + + @pytest.mark.parametrize( + "ref,old_id,new_id,reason", + [ + ("refs/tags/v1", OLD_ID, NEW_ID, "does not cover this tag"), + ("refs/notes/commits", OLD_ID, NEW_ID, "allows pushing branches and tags only"), + ("refs/heads/seer/fix", OLD_ID, "0" * 40, "does not allow deleting a branch"), + ], + ) + def test_other_refused_updates(self, ref, old_id, new_id, reason): + body = make_push_body((old_id, new_id, ref), capabilities="report-status") + assert reason.encode() in b"".join(self.send(make_server(Recorder()), io.BytesIO(body)).content) + + def test_no_body(self): + # Refused rather than relayed unchecked. + recorder = Recorder() + server = make_server(recorder) + response = request(server, PUSH_GRANT, method="POST", route="git-receive-pack", query="", body=None) + assert_refused(response, 400, "The push has no body.") + assert recorder.repository_lookups == [] + server.relay.forward.assert_not_called() # type: ignore[attr-defined] + + def test_signed_push_on_a_side_band(self): + body = pkt(b"push-cert\0report-status side-band-64k") + pkt(b"certificate version 0.1\n") + b"0000" + response = self.send(make_server(Recorder()), io.BytesIO(body)) + assert response.status_code == 200 + assert b"".join(response.content) == pkt(b"\3The git endpoint does not relay signed pushes.\n") + b"0000" + + def test_create_only_pattern_and_listed_branch(self): + grant = sign_grant( + KEY.private, + make_grant( + rules=[ + PushRule(repository=5, branch_patterns=["seer/.+"], create_only=True), + PushRule(repository=5, branches=["seer/fix"]), + ] + ), + ) + server = make_server(Recorder()) + for body in ( + make_push_body(("0" * 40, NEW_ID, "refs/heads/seer/next")), + make_push_body((OLD_ID, NEW_ID, "refs/heads/seer/fix")), + ): + response = request(server, grant, method="POST", route="git-receive-pack", query="", body=io.BytesIO(body)) + assert response.status_code == 200 + assert server.relay.forward.call_count == 2 # type: ignore[attr-defined] + refused = make_push_body((OLD_ID, NEW_ID, "refs/heads/seer/next"), capabilities="report-status") + response = request(server, grant, method="POST", route="git-receive-pack", query="", body=io.BytesIO(refused)) + assert b"the git grant allows only creating this branch" in b"".join(response.content) + + def test_client_that_asked_for_no_report(self): + recorder = Recorder() + body = io.BytesIO(push_body("refs/heads/main", capabilities="side-band-64k")) + assert_refused(self.send(make_server(recorder), body), 403, "does not allow this push: refs/heads/main") + assert body.read() == b"" + + def test_malformed(self): + recorder = Recorder() + assert_refused(self.send(make_server(recorder), io.BytesIO(b"garbage")), 400, "not pkt-lines") + assert recorder.repository_lookups == [] + assert recorder.counts == [ + ("sentry.scm.git.request.failed", {"referrer": REFERRER, "code": "git_malformed_request_body"}) + ] + + def test_compressed(self): + response = self.send( + make_server(Recorder()), io.BytesIO(push_body("refs/heads/seer/fix")), headers={"Content-Encoding": "gzip"} + ) + assert_refused(response, 400, "content-encoding header holds a value git doesn't send") + + def test_client_closes_while_sending_its_updates(self): + recorder = Recorder() + response = self.send(make_server(recorder), Unreliable()) + assert response.status_code == 499 + assert recorder.errors == [] + + def test_version_1_push_grant_is_refused(self): + recorder = Recorder() + server = make_server(recorder) + grant = jwt.encode(VERSION_1_PUSH_CLAIMS, KEY.key, algorithm="EdDSA", headers={"kid": key_id(KEY.public)}) + body = io.BytesIO(push_body("refs/heads/main")) + response = request(server, grant, method="POST", route="git-receive-pack", query="", body=body) + assert_refused(response, 403, "not version 2") + assert recorder.token_lookups == [] + server.relay.forward.assert_not_called() # type: ignore[attr-defined] + + +FETCH_ROUTES = [("GET", "info/refs", "service=git-upload-pack"), ("POST", "git-upload-pack", "")] +PUSH_ROUTES = [("GET", "info/refs", "service=git-receive-pack"), ("POST", "git-receive-pack", "")] +FETCH_ANY = FetchRule(repository=5, any_object=True) +PUSH_BRANCH = PushRule(repository=5, branches=["seer/fix"]) + + +class TestAuthorize: + @pytest.mark.parametrize("route", FETCH_ROUTES) + def test_fetch_rule_reaches_fetch_routes(self, route): + assert _authorize(make_grant(rules=[FETCH_ANY]), 5, *route) == "fetch" + + @pytest.mark.parametrize("route", PUSH_ROUTES) + def test_push_rule_reaches_push_routes(self, route): + # check_push then keeps the push to the rule's branches. + assert _authorize(make_grant(rules=[PUSH_BRANCH]), 5, *route) == "push" + + @pytest.mark.parametrize("route", PUSH_ROUTES) + def test_fetch_rule_cannot_reach_push_routes(self, route): + with pytest.raises(GitOperationNotGranted, match="allows fetch on this repository, not push"): + _authorize(make_grant(rules=[FETCH_ANY]), 5, *route) + + @pytest.mark.parametrize("route", FETCH_ROUTES) + def test_push_rule_cannot_reach_fetch_routes(self, route): + with pytest.raises(GitOperationNotGranted, match="allows push on this repository, not fetch"): + _authorize(make_grant(rules=[PUSH_BRANCH]), 5, *route) + + def test_rule_for_another_repository_does_not_apply(self): + grant = make_grant(rules=[FETCH_ANY, PushRule(repository=6, branches=["seer/fix"])]) + with pytest.raises(GitOperationNotGranted, match="allows fetch on this repository, not push"): + _authorize(grant, 5, "POST", "git-receive-pack", "") + + def test_any_of_several_rules(self): + grant = make_grant( + rules=[ + FetchRule(repository=5, branches=["main"]), + FETCH_ANY, + PUSH_BRANCH, + PushRule(repository=5, branch_patterns=["seer/.+"], create_only=True), + ] + ) + assert _authorize(grant, 5, "POST", "git-upload-pack", "") == "fetch" + assert _authorize(grant, 5, "POST", "git-receive-pack", "") == "push" + + def test_fetch_rules_that_all_list_branches_are_refused(self): + grant = make_grant(rules=[FetchRule(repository=5, branches=["main"]), FetchRule(repository=5, branches=["a"])]) + with pytest.raises(GitOperationNotGranted, match="cannot keep a fetch to a rule's branches"): + _authorize(grant, 5, "POST", "git-upload-pack", "") + + @pytest.mark.parametrize("route", FETCH_ROUTES) + @pytest.mark.parametrize( + "refs", + [{"branches": ["main"]}, {"tags": ["v1"]}, {"tag_patterns": ["v.+"]}], + ids=["branches", "tags", "tag-patterns"], + ) + def test_fetch_rule_that_names_refs_is_refused(self, route, refs): + # Nothing reads a fetch's body, so nothing would keep the fetch to the rule's branches and tags. + with pytest.raises(GitOperationNotGranted, match="cannot keep a fetch to a rule's branches and tags"): + _authorize(make_grant(rules=[FetchRule(repository=5, **refs)]), 5, *route) + + @pytest.mark.parametrize( + "method,route,query", + [ + ("GET", "info/refs", ""), + ("GET", "info/refs", "service=git-upload-pack&x=1"), + ("GET", "info/refs", "service=git-upload-archive"), + ("POST", "info/refs", "service=git-upload-pack"), + ("GET", "git-upload-pack", ""), + ("POST", "git-upload-pack", "service=git-upload-pack"), + ("post", "git-upload-pack", ""), + ("POST", "git-upload-archive", ""), + ("POST", "info/lfs/objects/batch", ""), + ("GET", "HEAD", ""), + ("GET", "objects/info/packs", ""), + ("GET", "../info/refs", "service=git-upload-pack"), + ("POST", "/git-upload-pack", ""), + ], + ) + def test_routes_that_are_not_git(self, method, route, query): + with pytest.raises(GitRouteNotSupported, match="not a git fetch or push route"): + _authorize(make_grant(rules=[FETCH_ANY, PUSH_BRANCH]), 5, method, route, query) + + def test_repository_not_in_grant(self): + with pytest.raises(GitRepositoryNotGranted, match="does not cover this repository"): + _authorize(make_grant(), 6, "POST", "git-upload-pack", "") + + def test_repository_named_another_way_is_not_in_grant(self): + # The rule and the request must name a repository the same way, since matching a database id to a provider + # id would mean resolving every repository the grant names. + grant = make_grant(rules=[FetchRule(repository=("github", "123"), any_object=True)]) + with pytest.raises(GitRepositoryNotGranted): + _authorize(grant, 5, "POST", "git-upload-pack", "") + assert _authorize(grant, ("github", "123"), "POST", "git-upload-pack", "") == "fetch" + + def test_any_repository_with_a_rule(self): + grant = make_grant( + rules=[ + FetchRule(repository=repository, any_object=True) for repository in (5, 6, ("gitlab", "gitlab.com:7")) + ] + ) + assert _authorize(grant, 6, "POST", "git-upload-pack", "") == "fetch" + assert _authorize(grant, ("gitlab", "gitlab.com:7"), "POST", "git-upload-pack", "") == "fetch"