28 ms·
I have also been working on an alternative written in Rust, but in my version the hooks are WASI programs. They run on a virtual filesystem backed by the Git re
by timhh 8mo ago
I have also been working on an alternative written in Rust, but in my version the hooks are WASI programs. They run on a virtual filesystem backed by the Git repo. That means a) there are no security issues (they have no network access, and no file access outside the repo), b) you can run them in parallel, c) you can choose whether to apply fixes or not without needing explicit support from the plugin, and most importantly d) they work reliably.
I'm sure this is more reliably than pre-commit, but you still have hooks building Python wheels and whatnot, which fails annoyingly often.
https://github.com/timmmm/nit https://github.com/timmmm/nit
The VFS stuff is not quite finished yet though (it's really complicated). If anyone wants to help me with that it would be welcome!
- jdxcode 8mo agothe second the hooks modify the code they've broken your sandbox I think wasi is a cool way to handle this problem. I don't think security is a reason though.
- timhh 8mo ago> the second the hooks modify the code they've broken your sandbox Changes to code would obviously need to be reviewed before they are committed. That's still much better than with pre-commit, where e.g. to do simple things like banning tabs you pretty much give some guy you don't know full access to your machine. Even worse - almost everyone that uses pre-commit also uses tags instead of commit hashes so the hook can be modified retroactively. One interesting attack would be for a hook to modify e.g. `.vscode/settings.json`... I should probably make the default config exclude those files. Is that what you meant? Even without that it's a lot more secure than pre-commit.
- accelbred 8mo agoI wouldn't want hooks modifying the code. They should be only approve/reject. Ideally landlock rules would give them only ro access to repo dir
- sgarland 8mo agoIt depends. I wrote a pre-commit hook (in shell, not precommit the tool) at a previous job that ran terraform fmt on any staged files (and add the changes to the commit) because I was really tired of having people push commits that would then fail for trivial things. It was overrideable with an env var. IMO if there’s a formatting issue, and the tool knows how it should look, it should fix it for you.
- pxc 8mo agoThe standard way for this with current tools is to have the formatter/linter make the changes but exit with a non-zero status, failing the hook. Then the person reviews the changes, stages, and commits. (That's what our setup currently has `tofu fmt` do.) But if you don't want to have hooks modify code, in a case like this you can also just use `tofu validate`. Our setup does `tflint` and `tofu validate` for this purpose, neither of which modifies the code. This is also, of course, a reasonable place to have people use `tofu plan`. It you want bad code to fail as quickly as possible, you can do: tflint -> tfsec -> tofu validate -> tofu plan That'll catch everything Terraform will let you catch before deploy time— most of it very quickly— without modifying any code.
- sgarland 8mo ago> make the changes but exit with a non-zero status That's reasonable. My personal (and that of my team at the time) take was that I was willing to let formatting - and only formatting - be auto-merged into the commit, since that isn't going to impact logic. For anything else, though, I would definitely want to let submitter review the changes.
- jdxcode 8mo agook but I was replying to a comment about a tool which advertises precisely that feature