NixOS / nix

Nix, the purely functional package manager
https://nixos.org/
GNU Lesser General Public License v2.1
12.17k stars 1.47k forks source link

flakes: git-crypt is now broken #5260

Open alarsyo opened 3 years ago

alarsyo commented 3 years ago

Describe the bug

I'm using git-crypt to manage my config, and since this nixpkgs commit I can't build it anymore using flakes:

It fails when trying to load some of the secrets of the repo, because they're still encrypted (it looks like the flake build re-clones my local repo elsewhere, ending up in a state where nothing is decrypted yet)

Steps To Reproduce

Expected behavior

building a flake that uses git-crypt should still work?

Additional context

I've bisected the regression up to PR #4922

nrdxp commented 3 years ago

why would fetching submodules break git-crypt? Do you have your secrets in a submodule?

plabadens commented 3 years ago

I'm able to reproduce the issue. Somehow this has changed the way the git repo gets moved to the nix store.

nrdxp commented 3 years ago

oh I guess I can see that, since it probably copies the repo from the git history (which has the secrets encrypted by default), instead of just a plain copy. For the sake of investigation, is this broken even when your file tree is dirty? I would imagine the behavior would be different in this case since it can't simply rely on the commit history as the dirty changes don't exist there yet.

plabadens commented 3 years ago

I'm currently doing a bit of testing, but it seems to evaluate perfectly fine when the worktree is dirty, as you expected. Using git-crypt was always a bit of a hack and I should probably be moving to something like sops-nix anyway.

alarsyo commented 3 years ago

Yes, I should also move to sops-nix or age-nix, but having a dirty worktree is indeed a great workaround for the time being, thanks for the tip!

miuirussia commented 3 years ago

I also encountered this bug, rolled back the changes that led to an error using this patch: https://github.com/miuirussia/nix-flake-env/blob/master/overlays/nixUnstable/revert-769ca4e.patch

nuance commented 2 years ago

I was using git-lfs to manage some binaries in my nix config, which this also appears to break. Reverting the commit fixes the issue.

nrdxp commented 2 years ago

the commit is reverted in master so we can close this I think.

roberth commented 2 years ago

Git-crypt should not work, at least not with the way flakes are currently implemented. Nix Flakes will write your secrets unconditionally to the store, which is not a secure place. Also, like arbitrary smudge filters, it is not reproducible. See https://github.com/NixOS/nix/pull/4635 for a more detailed argumentation.

I would like for git-crypt to work somehow, but it would take special treatment of local flakes, which I think it is too early to consider that at this point.

nixos-discourse commented 2 years ago

This issue has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/using-git-crypt-with-flakes/15372/5

roberth commented 2 years ago

Perhaps features like git-crypt, lfs and submodules can get their own top-level attribute in flake.nix (next to inputs, outputs) to describe how the flake should be fetched.

When evaluating a local flake, this means parsing the flake.nix file, before the current process of fetching and then evaluating the local flake. This can use restrictions similar to those of inputs. The overhead of parsing a single file (without adding to the store) is negligible.

When evaluating a remote flake via the a lock file, these attributes can be retrieved from the lock file.

When evaluating a remote flake from a mutable flake reference, it can be fetched twice as a general solution, or use the GitHub Contents API to retrieve flake.nix efficiently. When the flake uses the defaults, the first fetch can be reused immediately.

stale[bot] commented 2 years ago

I marked this as stale due to inactivity. → More info

lunik1 commented 2 years ago

Something of a workaround for the git-crypt case is to ensure the encrypted file is dirty before you evaluate the flake; this way the unencrypted file is evaluated. Of course, your secrets will still be globally readable in the nix store.

roberth commented 1 year ago

A proper git-crypt integration both allows access to the encrypted file for delayed decryption, and integrates with the string context to avoid adding decrypted files to the store

chayleaf commented 1 year ago

Disagreed, adding decrypted files to store is fine for some use cases - i.e. where you're fine with the files being in your locally readable store, but not in your online git repo.

roberth commented 1 year ago

@chayleaf would you be ok with a function that turns a secret string into a regular string? While this does remove an absolute guarantee from the system, it does still prevent almost all accidental leaking, assuming that libraries will not apply this function for you, which is reasonable. I would not reuse builtins.unsafeDiscardStringContext for this, because code has been written without this consideration in mind. It should be explicit about secrets; for example: builtins.exposeSecret.

chayleaf commented 1 year ago

I think the best option is to have the secret files marked as encrypted, but have any processing of the file (e.g. import, fromJSON) lose that context, it would be hard to do otherwise anyway

roberth commented 1 year ago

Right. #8388 only solves the problem for strings. To connect that up with git-crypt, we'd need something like lazy trees (#6530) to carry this "context" at the file level anyway. readFile would carry that context right over to a string, but import on the file or fromJSON on the string will expose things that aren't strings, such as attribute names, key numbers in a JWK, etc. We could have Nixpkgs library function that converts the JSON into a representation that exposes the attribute names but encodes the other attributes as secret strings. It's not ideal, but still covers a range of use cases quite safely.