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

fetchgit: Remove incomplete/unneeded isURI check. #1601

Merged
merged 1 commit into from Oct 16, 2017

Conversation

shlevy
Copy link
Member

@shlevy shlevy commented Oct 16, 2017

This check spuriously fails for e.g. git@github.com:NixOS/nixpkgs.git,
and even for ssh://git@github.com/NixOS/nixpkgs.git, and is made
redundant by the checks git itself will do when fetching the repo.

@shlevy shlevy requested a review from edolstra October 16, 2017 16:59
@edolstra
Copy link
Member

Well, some sort of validation is in order to prevent a command line injection...

@shlevy
Copy link
Member Author

shlevy commented Oct 16, 2017

Why? Does runProgram use a shell?

@edolstra
Copy link
Member

E.g. if the URI begins with a dash, git will interpret it as a flag.

@shlevy
Copy link
Member Author

shlevy commented Oct 16, 2017

Can't we just use a -- before the repo/rev?

@edolstra
Copy link
Member

Yeah that's probably good enough.

@shlevy
Copy link
Member Author

shlevy commented Oct 16, 2017

OK incoming shortly

This check spuriously fails for e.g. git@github.com:NixOS/nixpkgs.git,
and even for ssh://git@github.com/NixOS/nixpkgs.git, and is made
redundant by the checks git itself will do when fetching the repo. We
instead pass a -- before passing the URI to git to avoid injection.
@shlevy
Copy link
Member Author

shlevy commented Oct 16, 2017

@edolstra good now?

@edolstra edolstra merged commit be59f07 into NixOS:master Oct 16, 2017
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

Successfully merging this pull request may close these issues.

None yet

2 participants