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

nist-feed: init at 0-unstable-2024-01-20 #284812

Draft
wants to merge 9 commits into
base: master
Choose a base branch
from
Draft

Conversation

octodi
Copy link
Contributor

@octodi octodi commented Jan 29, 2024

Description of changes

Added NIST-Feed notifies you about the newest published CVEs according your filters

https://github.com/d3vil0p3r/nist-feed

Related to #81418

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • 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/)
  • 24.05 Release Notes (or backporting 23.05 and 23.11 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
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@octodi
Copy link
Contributor Author

octodi commented Jan 29, 2024

Successor of #284782

@D3vil0p3r
Copy link
Contributor

@octodi does it contain also the module, right?

@octodi
Copy link
Contributor Author

octodi commented Jan 29, 2024

@octodi does it contain also the module, right?

Yup

@D3vil0p3r
Copy link
Contributor

@octodi according to the new guidelines, you must change title and commit message from unstable-2024-01-20 to 0-unstable-2024-01-20.

@D3vil0p3r D3vil0p3r self-requested a review January 29, 2024 19:44
Copy link
Member

@h7x4 h7x4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the contribution. Here's a few comments :)

nixos/modules/programs/nist-feed.nix Show resolved Hide resolved
nixos/modules/programs/nist-feed.nix Outdated Show resolved Hide resolved
nixos/modules/programs/nist-feed.nix Outdated Show resolved Hide resolved
nixos/modules/programs/nist-feed.nix Outdated Show resolved Hide resolved
nixos/modules/programs/nist-feed.nix Outdated Show resolved Hide resolved
nixos/modules/programs/nist-feed.nix Outdated Show resolved Hide resolved
pkgs/by-name/ni/nist-feed/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/ni/nist-feed/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/ni/nist-feed/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/ni/nist-feed/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/ni/nist-feed/package.nix Outdated Show resolved Hide resolved
@octodi octodi changed the title nist-feed: init at unstable-2024-01-20 nist-feed: init at 0-unstable-2024-01-20 Jan 30, 2024
Update nixos/modules/programs/nist-feed.nix
Comment on lines +74 to +78
<<<<<<< HEAD
- [NIST-Feed](https://github.com/d3vil0p3r/nist-feed), notifies you about the newest published CVEs from NIST. Available as [programs.nist-feed](#opt-programs.nist-feed.enable).
=======
- [ALVR](https://github.com/alvr-org/alvr), a VR desktop streamer. Available as [programs.alvr](#opt-programs.alvr.enable)
>>>>>>> master
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There seems to be a conflict left over here

nixos/modules/programs/nist-feed.nix Show resolved Hide resolved
nixos/modules/programs/nist-feed.nix Outdated Show resolved Hide resolved
nixos/modules/programs/nist-feed.nix Outdated Show resolved Hide resolved
pkgs/by-name/ni/nist-feed/package.nix Show resolved Hide resolved
};

patches = [
./cron.patch
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe the name of this patch could be a bit more descriptive?

remove-cronjob-instantiation-feature.patch or something along those lines?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should definitely have a comment explaining why it's needed and/or why it couldn't be upstreamed

nixos/modules/programs/nist-feed.nix Outdated Show resolved Hide resolved
nixos/modules/programs/nist-feed.nix Outdated Show resolved Hide resolved
Copy link
Member

@eclairevoyant eclairevoyant left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution! Some feedback below.

@@ -0,0 +1,54 @@
{ config, lib, pkgs, ... }:

with lib;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
with lib;

Bad practice that's now discouraged in nixpkgs.
Tracking issue: #208242

extraArgs = mkOption {
type = with types; listOf str;
default = [ "-l" "-s" "CRITICAL" ];
description = mdDoc ''
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
description = mdDoc ''
description = ''

lib.mdDoc is no longer present in nixpkgs; please remove all usages.

runHook postInstall
'';

meta = with lib; {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
meta = with lib; {
meta = {

Tracking issue: #292468

Comment on lines +46 to +47
license = licenses.gpl3Plus;
maintainers = with maintainers; [ octodi ];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
license = licenses.gpl3Plus;
maintainers = with maintainers; [ octodi ];
license = lib.licenses.gpl3Plus;
maintainers = with lib.maintainers; [ octodi ];

};

patches = [
./cron.patch
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should definitely have a comment explaining why it's needed and/or why it couldn't be upstreamed

description = "A notification daemon for CVEs";
serviceConfig = {
Type = "oneshot";
ExecStart = "${cfg.package}/bin/nist-feed ${escapeShellArgs cfg.extraArgs}";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
ExecStart = "${cfg.package}/bin/nist-feed ${escapeShellArgs cfg.extraArgs}";
ExecStart = "${lib.getExe cfg.package} ${escapeShellArgs cfg.extraArgs}";

@octodi octodi marked this pull request as draft April 25, 2024 04:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants