23 Oct 2024 |
aloisw | No, readFile on everything the static analyzer doesn't recognize. | 18:10:26 |
aloisw | Like literals or very simple expressions it can resolve. | 18:10:37 |
emily | right. | 18:10:57 |
emily | I'm probably just going to do the workaround for now since enumerating the possibilities explicitly would be a pain and I think something like this should be morally allowed… | 18:11:43 |
aloisw | But the check is not really supposed to be adversarial anyway I guess, builtins.readFile (builtins.readFile ./foo)) is a giant smell that will most likely be caught in the review. | 18:11:53 |
emily | like really we just want an import in Nix that applies chroot-style restrictions. | 18:11:54 |
emily | because, I mean, you can also do someOtherPackage.somePassthruFunctionThatWillReadAFileForMe . | 18:12:17 |
emily | you would have to globally statically analyse Nixpkgs to truly enforce this. | 18:12:28 |
aloisw | I'm not sure we want yet another buggy userspace "chroot"… | 18:12:29 |
infinisil | In reply to @emilazy:matrix.org I'm not sure why ./${foo}/x is banned when as far as I can tell ./. + "/../x" is allowed (though I haven't actually tried it yet, just skimmed the code). Yeah nixpkgs-vet only does a simple syntactic check, while catching ./. + "/.." would need to be done with evaluation | 18:30:09 |
emily | aloisw's proposal to avoid eval seems plausible, but it doesn't solve all the ways that random packages can expose things you could use to acheive the same. | 18:30:59 |
emily | it's like the return-oriented programming of Nixpkgs. | 18:31:05 |
| aktaboot changed their profile picture. | 19:54:05 |
Randy Eckenrode | In reply to @emilazy:matrix.org because, I mean, you can also do someOtherPackage.somePassthruFunctionThatWillReadAFileForMe . That seems like it should be fine. You’re not directly access the implementation details of the package. It can get moved around or changed (implementation-wise) without affecting users of it. | 20:03:53 |
emily | In reply to @reckenrode:matrix.org That seems like it should be fine. You’re not directly access the implementation details of the package. It can get moved around or changed (implementation-wise) without affecting users of it. what I mean is, it can give you a way to access files outside your directory | 20:04:12 |
emily | it's almost certainly the case that you can construct a gadget like that out of existing Nixpkgs stuff in a way that isn't practically statically lintable | 20:04:36 |
emily | even if you heavily restrict directly file-reading functions as aloisw proposed | 20:04:53 |
emily | I don't mean that accessing another package itself constitutes a violation | 20:05:18 |
Randy Eckenrode | Does this hypothetical function allow the caller to control which file is read? | 20:05:44 |
emily | I mean that all you need is something somewhere that can read a path you input | 20:05:46 |
emily | there's lots of things that read files. e.g. you can construct a bad path and treat it as a Cargo lock file or whatever | 20:06:22 |
emily | I'm just saying that I don't think restricting the arguments you can pass to a set of known functions like readFile , importJSON , etc. is enough to truly statically lint the relevant property | 20:06:54 |
| Drewry Pope joined the room. | 20:58:55 |
| Leonardo Santiago set a profile picture. | 21:07:47 |
ReecerTV | I have a question, I have an open pr in which there is also a commit which adds me as maintainer. Now I want to open another pr, in which I am also listed as package maintainer, is this possible without problems? | 21:08:58 |
ReecerTV | I mean, should I just remove the old commit and move it to the new pr, because the open commit will not be mergeable at first? | 21:14:27 |
emily | I believe that if you have a duplicate commit in both PRs they can merge successfulyl | 21:19:00 |
emily | so it would be okay to do it like that | 21:19:03 |
frontear | and if by some chance it doesnt, you can rebase after the other has been merged and it should be fine | 22:21:45 |
frontear | currently thats what ive done, holding a commit until a sibling pr is merged, then rebase should take care of it | 22:22:04 |