Skip to content
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

flakes: git-crypt is now broken #5260

Open
alarsyo opened this issue Sep 16, 2021 · 18 comments
Open

flakes: git-crypt is now broken #5260

alarsyo opened this issue Sep 16, 2021 · 18 comments
Labels
bug fetching Networking with the outside (non-Nix) world, input locking

Comments

@alarsyo
Copy link
Contributor

alarsyo commented Sep 16, 2021

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

  • Use git-crypt to encrypt some secrets in a repo.
  • Use fileContents to load one of these secrets in a Nix string
  • Run nix build
  • get error about the file's content not being representable as a nix string, because it's binary garbage since it's still encrypted

Expected behavior

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

Additional context

I've bisected the regression up to PR #4922

@alarsyo alarsyo added the bug label Sep 16, 2021
@alarsyo alarsyo changed the title flakes: git filters broken flakes: git-crypt is now broken Sep 16, 2021
@nrdxp
Copy link
Contributor

nrdxp commented Sep 16, 2021

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

@plabadens
Copy link

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

@nrdxp
Copy link
Contributor

nrdxp commented Sep 16, 2021

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
Copy link

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
Copy link
Contributor Author

alarsyo commented Sep 17, 2021

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
Copy link

miuirussia commented Sep 19, 2021

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
Copy link

nuance commented Sep 27, 2021

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
Copy link
Contributor

nrdxp commented Sep 27, 2021

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

@roberth
Copy link
Member

roberth commented Sep 29, 2021

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 #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
Copy link

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
Copy link
Member

roberth commented Oct 17, 2021

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.

jtojnar added a commit to jtojnar/nixfiles that referenced this issue Nov 6, 2021
git-crypt no longer works: NixOS/nix#5260
jtojnar added a commit to jtojnar/nixfiles that referenced this issue Nov 6, 2021
git-crypt no longer works: NixOS/nix#5260
@stale
Copy link

stale bot commented Apr 16, 2022

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

@stale stale bot added the stale label Apr 16, 2022
@lunik1
Copy link

lunik1 commented Apr 29, 2022

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
Copy link
Member

roberth commented May 23, 2023

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

@stale stale bot removed the stale label May 23, 2023
@chayleaf
Copy link

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
Copy link
Member

roberth commented May 25, 2023

@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
Copy link

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
Copy link
Member

roberth commented May 25, 2023

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.

@roberth roberth added the fetching Networking with the outside (non-Nix) world, input locking label Jun 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug fetching Networking with the outside (non-Nix) world, input locking
Projects
None yet
Development

No branches or pull requests

9 participants