-
Notifications
You must be signed in to change notification settings - Fork 99
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 flake.parts module #269
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Sefa Eyeoglu <[email protected]>
Signed-off-by: Sefa Eyeoglu <[email protected]>
Signed-off-by: Sefa Eyeoglu <[email protected]>
Submitting this as a draft to gather feedback first |
Signed-off-by: Sefa Eyeoglu <[email protected]>
It looks like the schema check fails with this, as unset value are exported as |
there is some niche pain points in this strategy I'd like to address. Some pain points
EDIT:
deploy = {
sshOpts = [ "-t" ];
nodes.test-vm.sshOpts = lib.mkForce [ "-T" ];
};
Some upsides
My conclusion to the idea is that -- I generally like it, and think the benefits outweigh the problems. The primary problem in the implementation of this is how we choose to communicate with our users. It is not entirely clear to beginners what a flake-module is, and the possible reasons for their usage, above the pre-existing. In some sense, we have a chicken-egg problem. If there were a large ecosystem of flake-modules, then it'd be self-explanatory, but the current state of affairs is that they're quite rare. I would encourage the author to consider this, and help build the bridge of reason to use the ecosystem by outlining clear benefits. I hope this helps, I'll be watching the thread as its getting worked on. I might be able to invest sometime into it, but I have a full time job, so its only about 0.01% time that I can donate. |
Some after thoughts on this portion. This problem can be solved in the module system, but with some constraints. Instead of directly passing The remaining top level options should be used in conjunction with In respect to
There is likely some unsaid primitive in nix for checksums. Imagine a case like a derivation. {
sshOpts = ["-t"];
# sha2(stringConcatSep " " sshOpts)
sshOptsHash = "b64f681b76d9dc31203b50b41af60f449ed8c657";
} There maybe some use in not using lists, and rather strings directly, so that the type system can be used to say this can only be defined once, but we still have cases of These ideas are more mitigations for a bigger problem, but it is good to have a focus on high impact causalities from possible supply-chain attacks. In this case, a malicious flake-input. Some other ideas might include wrapping deploy-rs to ensure that it can only read a subset of public keys, and any nefarious writes to known_hosts/ssh-key exchange is behind bubblewrap, and won't be applied to the running session. This may be the best option, so far I've thought of. EDIT: Too understand the possible risk of command injection better, I've gone ahead and built a POC against the cwe-78. Fish: fish, version 3.7.0
|
I think this isn't limited to deploy-rs. If you import a malicious flake module, it could modify your devShell to add some malicious commands to the shell hook, or just replace the |
Add a flake-parts module for configuring deploy-rs profiles.
This makes it a lot easier for flake-parts users to use deploy-rs, as there are type checks analogous to NixOS modules.
Feel free to check the flake-parts example.
Future work
If desired, I would be happy to switch the primary
flake.nix
in this repo to flake-parts, to greatly simplify code, like it was proposed in #229 by @drupolIn that case I could additionally add descriptions as @Skarlett seems to have done in https://github.com/the-computer-club/lynx/blob/main/flake-modules/deploy-rs/default.nix and add code for generating module documentation. Of course deploy-rs would still work just as expected without flake-parts.