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

coding-conventions.chapter.md: update to account for #89885 #191378

Merged
merged 4 commits into from Oct 12, 2022
Merged

coding-conventions.chapter.md: update to account for #89885 #191378

merged 4 commits into from Oct 12, 2022

Conversation

ghost
Copy link

@ghost ghost commented Sep 15, 2022

Description of changes

This PR: #89885 ensures that fetches are done securely (i.e. without --insecure) when the hash parameter is one of the four special "fake" hashes. However the manual was not updated in that PR. This commit updates the manual to account for the already-merged changes from that PR.

ping @matthewbauer

Future-proofing

It would probably be a good idea if Nix didn't print out the complete unexpected hash of insecure fetches on the console, in order to deter people from copy-and-pasting it. For example, by default the error message printed to the console could deliberately garble a few characters in the middle of the hash by replacing them with an invalid character like . unless --option dammit-give-me-the-footgun is set.

The question is how to communicate to Nix that the fetch was insecure. I see two possibilities:

  1. Nix adopts the nixpkgs convention that if expected-hash is "", lib.fakeHash, lib.fakeSha256, or lib.fakeSha512 then the unexpected-hash was computed over something which was fetched securely. This could be considered a layering violation.

  2. A new derivation attribute __insecure for FODs is added, which tells Nix that the fetch was performed insecurely. Since this would appear only on FODs it shouldn't affect any outpath addresses. It should probably also be allowed on __impure derivations, and be inherited by FODs which depend on __insecure __impure derivations.

@edolstra if either of these approaches sounds acceptable to you I will implement it.

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 22.11 Release Notes (or backporting 22.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

This PR: #89885 ensures that fetches are
done securely (i.e. without `--insecure`) when the `hash` parameter is one of
the four special "fake" hashes.  However the manual was not updated in that PR.
This commit updates the manual to account for the already-merged changes from
that PR.
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild 10.rebuild-linux: 0 This PR does not cause any packages to rebuild labels Sep 15, 2022
Adam Joseph and others added 2 commits September 20, 2022 08:31
Co-authored-by: Valentin Gagarin <valentin.gagarin@tweag.io>
Co-authored-by: Valentin Gagarin <valentin.gagarin@tweag.io>
@ghost ghost requested a review from fricklerhandwerk September 20, 2022 08:36
@fricklerhandwerk fricklerhandwerk merged commit 0d0c03f into NixOS:master Oct 12, 2022
@ghost ghost deleted the pr/manual/hashes branch January 23, 2024 06:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.has: documentation 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild 10.rebuild-linux: 0 This PR does not cause any packages to rebuild
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant