Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a file size limit #66

Open
Janik-Haag opened this issue Mar 3, 2024 · 7 comments
Open

Add a file size limit #66

Janik-Haag opened this issue Mar 3, 2024 · 7 comments

Comments

@Janik-Haag
Copy link
Member

Do not allow merges if a file larger $size is in the pr to avoid people increasing the git history with pictures or similar.
Currently this isn't really relevant as long as only prs by r-ryantm are allowed but we need something like this if we rollout it out to more contributors.

@Mic92
Copy link
Member

Mic92 commented Mar 3, 2024

Is this an actual problem? I have not seen anyone trying to do this so far in a all the pull reuqests I have seen so far.

@Mic92
Copy link
Member

Mic92 commented Mar 3, 2024

I think when we roll this out to PRs beyond r-ryantm, we probably have a "trusted" maintainers set as far as I remember, who can be hopefully trusted enough to not do this.

@Janik-Haag
Copy link
Member Author

Is this an actual problem? I have not seen anyone trying to do this so far in a all the pull reuqests I have seen so far.

I know of at least one case were it happened by accident. And the more people we give access the more likely something like this might happen again and it also increases the risk of someone with malicious intended might try to bloat the git history.

we probably have a "trusted" maintainers set as far as I remember, who can be hopefully trusted enough to not do this.

oh maybe I misunderstood, but my understanding was that at some point every package maintainer should be allowed to merge update pr's by themselves.

@Mic92
Copy link
Member

Mic92 commented Mar 4, 2024

Since historically, we didn't had maintainers with extended privileges, we need some transition phase from the old system to the new system.

@Scriptkiddi
Copy link
Collaborator

I guess what Janik is trying to say we should enforce some type of checking for file size limits so people do not add a binary blob with a size of 5gb without review.
I would suggest we add this to the rfc draft and we will discuss this there as soon as it is submitted.

@Mic92
Copy link
Member

Mic92 commented Mar 6, 2024

I guess no one should be allowed to do that.

@mweinelt
Copy link
Member

mweinelt commented Mar 6, 2024

Two situations come to mind:

  • You never wanted to have large package-lock.json files in nixpkgs
  • Ma27 recently committed a large .svg file into nixpkgs without noticing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants