WIP: new check: 'out-of-tree-commit' #96
Conversation
…ence a sha that does not belong to the repo it is being fetched from.
| locations=[], | ||
| ) | ||
| ) | ||
| return {"<unknown>": reports} |
There was a problem hiding this comment.
Here's an example of what it looks like:
nix run -f ~/projects/nixpkgs-hammering -c nixpkgs-hammer -f . openexr -e attribute-ordering -e no-build-output -e unused-argument -e maintainers-missing
When evaluating attribute ‘openexr’:
No issues found.
When evaluating attribute ‘<unknown>’:
warning: out-of-repo-commit
The dependency chain for these expressions includes https://github.com/AcademySoftwareFoundation/openexr/commit/6442fb71a86c09fb0a8118b6dbd93bcec4883a3c.patch,
which references a commit which does not belong to any
branch on this repository, and is in fact associated with
a fork of this repository.
See: https://github.com/jtojnar/nixpkgs-hammering/blob/master/explanations/out-of-repo-commit.md
Currently, a big technical limitation is that I don't know how to associate the fetchurl call with the attr -- that is, if you're simultaneously running nixpkgs-hammering on two or more attrs and the dependency chain for one of the attrs triggers a report of this type, I don't know how to determine which attr triggered the report.
There was a problem hiding this comment.
Perhaps we could print the outPath of the fetchurl call, get a drv file from that and then check which of the attributes has that in closure.
There was a problem hiding this comment.
Yes, that seems reasonable. As best I can tell, we can't compute the closure from within the nix language, and instead that needs to be done from the cmdline (nix why-depends, nix show-derivation -r or nix-store --query --graphml, or similar). Is that also your understanding?
There was a problem hiding this comment.
With IFD, you might run those commands using runCommand or write a new store-querying builting. But yeah, I am not aware of any current facility to query Nix store within Nix language. (Well, you could compute the closure by literally reading the *.drv store paths but letting Nix get the values from the SQLite database will probably be much faster.)
We will probably want to print the why-depends result in the check output anyway, to make it clear from which derivation it comes from.
| yield (url,) + m1.groups() | ||
|
|
||
| def _fetch_data(url: str, owner: str, repo: str, sha: str) -> Tuple[str, str]: | ||
| branch_commit_url = f"https://github.com/{owner}/{repo}/branch_commits/{sha}" |
There was a problem hiding this comment.
That is an undocumented path which returns html. It could change at any time and I think info should be added as a comment.
There was a problem hiding this comment.
Do you know if there's another endpoint that's documented? I suppose a graphql or something could work too.
There was a problem hiding this comment.
When I researched that a while ago I didn't find anything else.
Flag patches and fetchFromGithub calls that reference a sha that does not belong to the repo it is being fetched from. (Note: since this is a security-related check, it's actually looking not just at the attr that you're running hammering on, but the entire dependency chain for that attr).
Discussed in #65