Skip to content

Commit

Permalink
feat: add file size limit (default 2 MB)
Browse files Browse the repository at this point in the history
Add a file size check to enforce a 2 MB default limit.
Include tests to validate the functionality.
  • Loading branch information
DominicWrege authored and Scriptkiddi committed Dec 28, 2024
1 parent e80928f commit dac9f73
Show file tree
Hide file tree
Showing 7 changed files with 88 additions and 5 deletions.
3 changes: 2 additions & 1 deletion nixpkgs_merge_bot/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,10 @@ def parse_args() -> Settings:
restricted_authors=args.restricted_authors.split(" "),
database_path=args.database_folder,
repo_path=args.repo_path,
max_file_size_mb=args.max_file_size_mb
max_file_size_mb=args.max_file_size_mb,
)


def main() -> None:
settings = parse_args()
con = sqlite3.connect(f"{settings.database_path}/nixpkgs_merge_bot.db")
Expand Down
6 changes: 6 additions & 0 deletions nixpkgs_merge_bot/github/GitHubClient.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ def _request(
"Content-Type": "application/json",
"X-GitHub-Api-Version": "2022-11-28",
}

if self.api_token:
headers["Authorization"] = f"Bearer {self.api_token}"
headers["User-Agent"] = "nixpkgs-merge-bot"
Expand Down Expand Up @@ -159,6 +160,11 @@ def get_comment(self, owner: str, repo: str, comment_id: int) -> HttpResponse:
def pull_request_files(self, owner: str, repo: str, pr_number: int) -> HttpResponse:
return self.get(f"/repos/{owner}/{repo}/pulls/{pr_number}/files")

def get_request_file_content(
self, owner: str, repo: str, filepath: str
) -> HttpResponse:
return self.get(f"/repos/{owner}/{repo}/contents/{filepath}")

def get_issue(self, owner: str, repo: str, issue_number: int) -> HttpResponse:
return self.get(f"/repos/{owner}/{repo}/issues/{issue_number}")

Expand Down
4 changes: 2 additions & 2 deletions nixpkgs_merge_bot/merging_strategies/merging_strategy.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ def run_technical_limits_check(
log.info(
f"{pull_request.number}: Checking mergeability of {pull_request.number} with sha {sha}"
)

if pull_request.state != "open":
result = False
message = f"pr is not open, state is {pull_request.state}"
Expand Down Expand Up @@ -54,7 +54,7 @@ def run_technical_limits_check(
return result, decline_reasons

def get_file_size_bytes(self, pull_request, filename) -> int:
response = self.github_client.pull_request_file_content(
response = self.github_client.get_request_file_content(
pull_request.repo_owner, pull_request.repo_name, filename
)
return response.json()["size"]
Expand Down
4 changes: 2 additions & 2 deletions nixpkgs_merge_bot/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ class Settings:
repo: str = "https://github.com/nixos/nixpkgs"
repo_path: Path = Path("nixpkgs")
database_path: str = "/tmp/"
max_file_size_mb: int = 2;
max_file_size_mb: int = 2

@property
def max_file_size_bytes(self) -> int:
"""Return the maximum file size in bytes."""
return self.max_file_size_mb * 1024 * 1024
return self.max_file_size_mb * 1024 * 1024
18 changes: 18 additions & 0 deletions tests/data/pull_request_file_content.large.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
{
"name": "package.nix",
"path": "pkgs/by-name/ni/large-file.nix",
"sha": "2e6d3136cf9272090c4a4babda8946674c74a863",
"size": 1348576,
"url": "https://api.github.com/repos/nixpkgs-merge/nixpkgs/contents/pkgs/by-name/ni/large-file.nix?ref=c272a8a05b5776ef59a4c60b3b8f7c23a61defd0",
"html_url": "https://github.com/nixpkgs-merge/nixpkgs/blob/c272a8a05b5776ef59a4c60b3b8f7c23a61defd0/pkgs/by-name/ni/large-file.nix",
"git_url": "https://api.github.com/repos/nixpkgs-merge/nixpkgs/git/blobs/2e6d3136cf9272090c4a4babda8946674c74a863",
"download_url": "https://raw.githubusercontent.com/nixpkgs-merge/nixpkgs/c272a8a05b5776ef59a4c60b3b8f7c23a61defd0/pkgs/by-name/ni/large-file.nix",
"type": "file",
"content": "eyBzdGRlbnYKLCBmZXRjaEZyb21HaXRIdWIKLCBvcGVuc3NoCiwgZ2l0TWlu\naW1hbAosIHJzeW5jCiwgbml4CiwgY29yZXV0aWxzCiwgY3VybAosIGdudWdy\nZXAKLCBnYXdrCiwgZmluZHV0aWxzCiwgZ251c2VkCiwgbGliCiwgbWFrZVdy\nYXBwZXIKfToKbGV0CiAgcnVudGltZURlcHMgPSBbCiAgICBnaXRNaW5pbWFs\nICMgZm9yIGdpdCBmbGFrZXMKICAgIHJzeW5jCiAgICBuaXgKICAgIGNvcmV1\ndGlscwogICAgY3VybCAjIHdoZW4gdXBsb2FkaW5nIHRhcmJhbGxzCiAgICBn\nbnVncmVwCiAgICBnYXdrCiAgICBmaW5kdXRpbHMKICAgIGdudXNlZCAjIG5l\nZWRlZCBieSBzc2gtY29weS1pZAogIF07CmluCnN0ZGVudi5ta0Rlcml2YXRp\nb24gKGZpbmFsQXR0cnM6IHsKICBwbmFtZSA9ICJuaXhvcy1hbnl3aGVyZSI7\nCiAgdmVyc2lvbiA9ICIxLjAuMCI7CiAgc3JjID0gZmV0Y2hGcm9tR2l0SHVi\nIHsKICAgIG93bmVyID0gIm51bXRpZGUiOwogICAgcmVwbyA9ICJuaXhvcy1h\nbnl3aGVyZSI7CiAgICByZXYgPSBmaW5hbEF0dHJzLnZlcnNpb247CiAgICBo\nYXNoID0gInNoYTI1Ni16TStONytYRFIzNER1VHJWTEpkN0dncTFKUGxVUmRk\nc3FOT2pYWS9yY1FNPSI7CiAgfTsKICBuYXRpdmVCdWlsZElucHV0cyA9IFsg\nbWFrZVdyYXBwZXIgXTsKICBpbnN0YWxsUGhhc2UgPSAnJwogICAgaW5zdGFs\nbCAtRCAtbSAwNzU1IHNyYy9uaXhvcy1hbnl3aGVyZS5zaCAkb3V0L2Jpbi9u\naXhvcy1hbnl3aGVyZQoKICAgICMgV2UgcHJlZmVyIHRoZSBzeXN0ZW0ncyBv\ncGVuc3NoIG92ZXIgb3VyIG93biwgc2luY2UgaXQgbWlnaHQgY29tZSB3aXRo\nIGZlYXR1cmVzIG5vdCBwcmVzZW50IGluIG91cnM6CiAgICAjIGh0dHBzOi8v\nZ2l0aHViLmNvbS9udW10aWRlL25peG9zLWFueXdoZXJlL2lzc3Vlcy82Mgog\nICAgd3JhcFByb2dyYW0gJG91dC9iaW4vbml4b3MtYW55d2hlcmUgXAogICAg\nICAtLXByZWZpeCBQQVRIIDogJHtsaWIubWFrZUJpblBhdGggcnVudGltZURl\ncHN9IC0tc3VmZml4IFBBVEggOiAke2xpYi5tYWtlQmluUGF0aCBbIG9wZW5z\nc2ggXX0KICAnJzsKCiAgbWV0YSA9IHdpdGggbGliOyB7CiAgICBkZXNjcmlw\ndGlvbiA9ICJJbnN0YWxsIG5peG9zIGV2ZXJ5d2hlcmUgdmlhIHNzaCI7CiAg\nICBob21lcGFnZSA9ICJodHRwczovL2dpdGh1Yi5jb20vbnVtdGlkZS9uaXhv\ncy1hbnl3aGVyZSI7CiAgICBtYWluUHJvZ3JhbSA9ICJuaXhvcy1hbnl3aGVy\nZSI7CiAgICBsaWNlbnNlID0gbGljZW5zZXMubWl0OwogICAgcGxhdGZvcm1z\nID0gcGxhdGZvcm1zLmFsbDsKICAgIG1haW50YWluZXJzID0gWyBtYWludGFp\nbmVycy5taWM5MiBtYWludGFpbmVycy5sYXNzdWx1cyBtYWludGFpbmVycy5w\naGFlciBdOwogICAgIyBtaWMgaXMgdGhlIG9yaWdpbmFsIGF1dGhvciBvZiBu\naXhvcy1hbnl3aGVyZQogIH07Cn0pCg==\n",
"encoding": "base64",
"_links": {
"self": "https://api.github.com/repos/nixpkgs-merge/nixpkgs/contents/pkgs/by-name/ni/large-file.nix?ref=c272a8a05b5776ef59a4c60b3b8f7c23a61defd0",
"git": "https://api.github.com/repos/nixpkgs-merge/nixpkgs/git/blobs/2e6d3136cf9272090c4a4babda8946674c74a863",
"html": "https://github.com/nixpkgs-merge/nixpkgs/blob/c272a8a05b5776ef59a4c60b3b8f7c23a61defd0/pkgs/by-name/ni/large-file.nix"
}
}
18 changes: 18 additions & 0 deletions tests/data/pull_request_file_content.package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
{
"name": "package.nix",
"path": "pkgs/by-name/ni/nixos-anywhere/package.nix",
"sha": "2e6d3136cf9272090c4a4babda8946674c74a863",
"size": 1417,
"url": "https://api.github.com/repos/nixpkgs-merge/nixpkgs/contents/pkgs/by-name/ni/nixos-anywhere/package.nix?ref=c272a8a05b5776ef59a4c60b3b8f7c23a61defd0",
"html_url": "https://github.com/nixpkgs-merge/nixpkgs/blob/c272a8a05b5776ef59a4c60b3b8f7c23a61defd0/pkgs/by-name/ni/nixos-anywhere/package.nix",
"git_url": "https://api.github.com/repos/nixpkgs-merge/nixpkgs/git/blobs/2e6d3136cf9272090c4a4babda8946674c74a863",
"download_url": "https://raw.githubusercontent.com/nixpkgs-merge/nixpkgs/c272a8a05b5776ef59a4c60b3b8f7c23a61defd0/pkgs/by-name/ni/nixos-anywhere/package.nix",
"type": "file",
"content": "eyBzdGRlbnYKLCBmZXRjaEZyb21HaXRIdWIKLCBvcGVuc3NoCiwgZ2l0TWlu\naW1hbAosIHJzeW5jCiwgbml4CiwgY29yZXV0aWxzCiwgY3VybAosIGdudWdy\nZXAKLCBnYXdrCiwgZmluZHV0aWxzCiwgZ251c2VkCiwgbGliCiwgbWFrZVdy\nYXBwZXIKfToKbGV0CiAgcnVudGltZURlcHMgPSBbCiAgICBnaXRNaW5pbWFs\nICMgZm9yIGdpdCBmbGFrZXMKICAgIHJzeW5jCiAgICBuaXgKICAgIGNvcmV1\ndGlscwogICAgY3VybCAjIHdoZW4gdXBsb2FkaW5nIHRhcmJhbGxzCiAgICBn\nbnVncmVwCiAgICBnYXdrCiAgICBmaW5kdXRpbHMKICAgIGdudXNlZCAjIG5l\nZWRlZCBieSBzc2gtY29weS1pZAogIF07CmluCnN0ZGVudi5ta0Rlcml2YXRp\nb24gKGZpbmFsQXR0cnM6IHsKICBwbmFtZSA9ICJuaXhvcy1hbnl3aGVyZSI7\nCiAgdmVyc2lvbiA9ICIxLjAuMCI7CiAgc3JjID0gZmV0Y2hGcm9tR2l0SHVi\nIHsKICAgIG93bmVyID0gIm51bXRpZGUiOwogICAgcmVwbyA9ICJuaXhvcy1h\nbnl3aGVyZSI7CiAgICByZXYgPSBmaW5hbEF0dHJzLnZlcnNpb247CiAgICBo\nYXNoID0gInNoYTI1Ni16TStONytYRFIzNER1VHJWTEpkN0dncTFKUGxVUmRk\nc3FOT2pYWS9yY1FNPSI7CiAgfTsKICBuYXRpdmVCdWlsZElucHV0cyA9IFsg\nbWFrZVdyYXBwZXIgXTsKICBpbnN0YWxsUGhhc2UgPSAnJwogICAgaW5zdGFs\nbCAtRCAtbSAwNzU1IHNyYy9uaXhvcy1hbnl3aGVyZS5zaCAkb3V0L2Jpbi9u\naXhvcy1hbnl3aGVyZQoKICAgICMgV2UgcHJlZmVyIHRoZSBzeXN0ZW0ncyBv\ncGVuc3NoIG92ZXIgb3VyIG93biwgc2luY2UgaXQgbWlnaHQgY29tZSB3aXRo\nIGZlYXR1cmVzIG5vdCBwcmVzZW50IGluIG91cnM6CiAgICAjIGh0dHBzOi8v\nZ2l0aHViLmNvbS9udW10aWRlL25peG9zLWFueXdoZXJlL2lzc3Vlcy82Mgog\nICAgd3JhcFByb2dyYW0gJG91dC9iaW4vbml4b3MtYW55d2hlcmUgXAogICAg\nICAtLXByZWZpeCBQQVRIIDogJHtsaWIubWFrZUJpblBhdGggcnVudGltZURl\ncHN9IC0tc3VmZml4IFBBVEggOiAke2xpYi5tYWtlQmluUGF0aCBbIG9wZW5z\nc2ggXX0KICAnJzsKCiAgbWV0YSA9IHdpdGggbGliOyB7CiAgICBkZXNjcmlw\ndGlvbiA9ICJJbnN0YWxsIG5peG9zIGV2ZXJ5d2hlcmUgdmlhIHNzaCI7CiAg\nICBob21lcGFnZSA9ICJodHRwczovL2dpdGh1Yi5jb20vbnVtdGlkZS9uaXhv\ncy1hbnl3aGVyZSI7CiAgICBtYWluUHJvZ3JhbSA9ICJuaXhvcy1hbnl3aGVy\nZSI7CiAgICBsaWNlbnNlID0gbGljZW5zZXMubWl0OwogICAgcGxhdGZvcm1z\nID0gcGxhdGZvcm1zLmFsbDsKICAgIG1haW50YWluZXJzID0gWyBtYWludGFp\nbmVycy5taWM5MiBtYWludGFpbmVycy5sYXNzdWx1cyBtYWludGFpbmVycy5w\naGFlciBdOwogICAgIyBtaWMgaXMgdGhlIG9yaWdpbmFsIGF1dGhvciBvZiBu\naXhvcy1hbnl3aGVyZQogIH07Cn0pCg==\n",
"encoding": "base64",
"_links": {
"self": "https://api.github.com/repos/nixpkgs-merge/nixpkgs/contents/pkgs/by-name/ni/nixos-anywhere/package.nix?ref=c272a8a05b5776ef59a4c60b3b8f7c23a61defd0",
"git": "https://api.github.com/repos/nixpkgs-merge/nixpkgs/git/blobs/2e6d3136cf9272090c4a4babda8946674c74a863",
"html": "https://github.com/nixpkgs-merge/nixpkgs/blob/c272a8a05b5776ef59a4c60b3b8f7c23a61defd0/pkgs/by-name/ni/nixos-anywhere/package.nix"
}
}
40 changes: 40 additions & 0 deletions tests/test_webhook.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,9 @@ def default_mocks(mocker: MockerFixture) -> dict[str, Any]:
"nixpkgs_merge_bot.github.GitHubClient.GithubClient.create_issue_reaction": FakeHttpResponse(
TEST_DATA / "issue_comment.merge.json"
), # unused
"nixpkgs_merge_bot.github.GitHubClient.GithubClient.get_request_file_content": FakeHttpResponse(
TEST_DATA / "pull_request_file_content.package.json"
), #
}


Expand Down Expand Up @@ -222,3 +225,40 @@ def test_post_merge_maintainer_multiline(
response_body = json.loads(response.read().decode("utf-8"))
assert response.status == 200, f"Response: {response.status}, {response_body}"
assert response_body["action"] == "not-permitted"


@pytest.mark.parametrize(
"mock_overrides",
[
{
"nixpkgs_merge_bot.github.GitHubClient.GithubClient.get_request_file_content": FakeHttpResponse(
TEST_DATA / "pull_request_file_content.large.json"
)
},
],
)
def test_post_merge_too_large_file(
webhook_client: WebhookClient, mocker: MockerFixture, mock_overrides: dict[str, Any]
) -> None:
mocks = default_mocks(mocker)
mocks.update(mock_overrides)
for name, return_value in mocks.items():
mocker.patch(name, return_value=return_value)

client = webhook_client.http_connect()
create_event = (TEST_DATA / "issue_comment_multiline.merge.json").read_bytes()
headers = {
"Content-Type": "application/json",
"X-GitHub-Event": "issue_comment",
"X-Hub-Signature": "sha1=eff1fea4ebf0cc443d53a499e2466904da8c3062",
"X-Hub-Signature-256": "sha256=53e04dda8e5d322028f7111eb9b92dc0056c8bc0bcb084edec7c9b87d594a4bb",
}
client.request("POST", "/", body=create_event, headers=headers)

SETTINGS.max_file_size_mb = 1
GithubWebHook(webhook_client.server_sock, webhook_client.addr, SETTINGS)

response = client.getresponse()
response_body = json.loads(response.read().decode("utf-8"))
assert response.status == 200, f"Response: {response.status}, {response_body}"
assert response_body["action"] == "not-permitted"

0 comments on commit dac9f73

Please sign in to comment.