Skip to content

Conversation

aciceri
Copy link
Member

@aciceri aciceri commented May 1, 2025

Things done

  • Done
    • doesn't require performing manual operations the first time it was started
    • free form settings option
    • (partially) included hardening options from nixos/amuled: add systemd hardening options #354517
    • complete refactor and improved ergonomics
    • use file options for passwords
    • section in NixOS manual
    • add myself as maintainer for both packages (amule, amule-gui, amule-web and amule-daemon) and the NixOS module
    • add mainProgram to all the packages above

This PR should supersede both #226502 and #354517

  • 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/)
  • 25.05 Release Notes (or backporting 24.11 and 25.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
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` labels May 1, 2025
@aciceri aciceri force-pushed the fix-amule-module branch from 7e414f9 to fa3fa81 Compare May 1, 2025 13:27
@github-actions github-actions bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels May 1, 2025
@aciceri aciceri force-pushed the fix-amule-module branch 2 times, most recently from 47daf5f to 4d1aff2 Compare May 1, 2025 14:19
@github-actions github-actions bot added the 8.has: documentation This PR adds or changes documentation label May 1, 2025
@aciceri aciceri force-pushed the fix-amule-module branch from 4d1aff2 to 5785b08 Compare May 1, 2025 14:52
@aciceri aciceri marked this pull request as ready for review May 1, 2025 14:55
@aciceri aciceri force-pushed the fix-amule-module branch 3 times, most recently from ba23761 to fe31927 Compare May 5, 2025 13:11
@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/prs-ready-for-review-december/1711/29

@aciceri
Copy link
Member Author

aciceri commented May 5, 2025

Requesting a review from @ymarkus since he created the module currently present in nixpkgs.
Also requesting reviews from @snicket2100 and @someone-stole-my-name since they are the authors of the two PRs mentioned above (which should be superseded by this PR).

Sorry for the noise but since there are no maintainers I don't know who to ping.

EDIT: apparently I cannot request a review from @snicket2100 and @someone-stole-my-name, probably it's because they are not in the organization

@aciceri aciceri requested a review from ymarkus May 5, 2025 14:19
@aciceri aciceri force-pushed the fix-amule-module branch from fe31927 to cb8066d Compare May 5, 2025 14:24
@someone-stole-my-name
Copy link
Contributor

Requesting a review from @ymarkus since he created the module currently present in nixpkgs. Also requesting reviews from @snicket2100 and @someone-stole-my-name since they are the authors of the two PRs mentioned above (which should be superseded by this PR).

Sorry for the noise but since there are no maintainers I don't know who to ping.

EDIT: apparently I cannot request a review from @snicket2100 and @someone-stole-my-name, probably it's because they are not in the organization

Hey, I forgot #354517 never made its way in. Thanks for stepping up and taking care of this :) There was another pending batch of changes focusing mostly on config (like this PR), perhaps you could borrow some ideas from it (eg the enableWeb flag):

full amuled
{
  config,
  lib,
  pkgs,
  utils,
  ...
}:
with lib;
let
  cfg = config.services.amule;
  amuledConfigFile = builtins.toString (
    pkgs.writeTextFile {
      name = "amule.conf";
      text = lib.generators.toINI { } amuledMergedConfig;
    }
  );
  amuledMergedConfig = attrsets.recursiveUpdate amuledDefaultConfig cfg.config;
  amuledDefaultConfig = {
    eMule = {
      AppVersion = "GIT";
      Nick = "http://www.aMule.org";
      QueueSizePref = 50;
      MaxUpload = 0;
      MaxDownload = 0;
      SlotAllocation = 2;
      Port = 4662;
      UDPPort = 4672;
      UDPEnable = 1;
      Address = "";
      Autoconnect = 1;
      MaxSourcesPerFile = 300;
      MaxConnections = 500;
      MaxConnectionsPerFiveSeconds = 20;
      RemoveDeadServer = 1;
      DeadServerRetry = 3;
      ServerKeepAliveTimeout = 0;
      Reconnect = 1;
      Scoresystem = 1;
      Serverlist = 0;
      AddServerListFromServer = 0;
      AddServerListFromClient = 0;
      SafeServerConnect = 0;
      AutoConnectStaticOnly = 0;
      UPnPEnabled = 0;
      UPnPTCPPort = 50000;
      SmartIdCheck = 1;
      ConnectToKad = 1;
      ConnectToED2K = 1;
      TempDir = "${cfg.dataDir}/temp";
      IncomingDir = "${cfg.dataDir}/incoming";
      ICH = 1;
      AICHTrust = 0;
      CheckDiskspace = 1;
      MinFreeDiskSpace = 1;
      AddNewFilesPaused = 0;
      PreviewPrio = 0;
      ManualHighPrio = 0;
      StartNextFile = 0;
      StartNextFileSameCat = 0;
      StartNextFileAlpha = 0;
      FileBufferSizePref = 16;
      DAPPref = 1;
      UAPPref = 1;
      AllocateFullFile = 0;
      OSDirectory = cfg.dataDir;
      OnlineSignature = 0;
      OnlineSignatureUpdate = 5;
      EnableTrayIcon = 0;
      MinToTray = 0;
      Notifications = 0;
      ConfirmExit = 1;
      StartupMinimized = 0;
      "3DDepth" = 10;
      ToolTipDelay = 1;
      ShowOverhead = 0;
      ShowInfoOnCatTabs = 1;
      VerticalToolbar = 0;
      GeoIPEnabled = 1;
      ShowVersionOnTitle = 0;
      VideoPlayer = "";
      StatGraphsInterval = 3;
      statsInterval = 30;
      DownloadCapacity = 300;
      UploadCapacity = 100;
      StatsAverageMinutes = 5;
      VariousStatisticsMaxValue = 100;
      SeeShare = 2;
      FilterLanIPs = 1;
      ParanoidFiltering = 1;
      IPFilterAutoLoad = 1;
      IPFilterURL = "";
      FilterLevel = 127;
      IPFilterSystem = 0;
      FilterMessages = 1;
      FilterAllMessages = 0;
      MessagesFromFriendsOnly = 0;
      MessageFromValidSourcesOnly = 1;
      FilterWordMessages = 0;
      MessageFilter = "";
      ShowMessagesInLog = 1;
      FilterComments = 0;
      CommentFilter = "";
      ShareHiddenFiles = 0;
      AutoSortDownloads = 0;
      NewVersionCheck = 1;
      AdvancedSpamFilter = 1;
      MessageUseCaptchas = 1;
      Language = "";
      SplitterbarPosition = 75;
      YourHostname = "";
      DateTimeFormat = "%A, %x, %X";
      AllcatType = 0;
      ShowAllNotCats = 0;
      SmartIdState = 0;
      DropSlowSources = 0;
      KadNodesUrl = "http://upd.emule-security.org/nodes.dat";
      Ed2kServersUrl = "http://upd.emule-security.org/server.met";
      ShowRatesOnTitle = 0;
      GeoLiteCountryUpdateUrl = "http://geolite.maxmind.com/download/geoip/database/GeoLiteCountry/GeoIP.dat.gz";
      StatsServerName = "Shorty's ED2K stats";
      StatsServerURL = "http://ed2k.shortypower.dyndns.org/?hash=";
      CreateSparseFiles = 1;
    };

    Browser = {
      OpenPageInTab = 1;
      CustomBrowserString = "";
    };

    Proxy = {
      ProxyEnableProxy = 0;
      ProxyType = 0;
      ProxyName = "";
      ProxyPort = 1080;
      ProxyEnablePassword = 0;
      ProxyUser = "";
      ProxyPassword = "";
    };

    ExternalConnect = {
      UseSrcSeeds = 0;
      AcceptExternalConnections = 1;
      ECAddress = "127.0.0.1";
      ECPort = 4712;
      ECPassword = null;
      UPnPECEnabled = 0;
      ShowProgressBar = 1;
      ShowPercent = 1;
      UseSecIdent = 1;
      IpFilterClients = 1;
      IpFilterServers = 1;
      TransmitOnlyUploadingClients = 0;
    };

    WebServer = {
      Enabled = if cfg.enableWeb then 1 else 0;
      Password = null;
      PasswordLow = "";
      Port = 4711;
      WebUPnPTCPPort = 50001;
      UPnPWebServerEnabled = 0;
      UseGzip = 1;
      UseLowRightsUser = 0;
      PageRefreshTime = 60;
      Template = "default";
      Path = "amuleweb";
    };

    GUI = {
      HideOnClose = 0;
    };

    Razor_Preferences = {
      FastED2KLinksHandler = 1;
    };

    SkinGUIOptions = {
      Skin = "";
    };

    Statistics = {
      MaxClientVersions = 0;
    };

    Obfuscation = {
      IsClientCryptLayerSupported = 1;
      IsCryptLayerRequested = 1;
      IsClientCryptLayerRequired = 0;
      CryptoPaddingLenght = 254;
      CryptoKadUDPKey = null;
    };

    PowerManagement = {
      PreventSleepWhileDownloading = 0;
    };

    UserEvents = { };

    "UserEvents/DownloadCompleted" = {
      CoreEnabled = 0;
      CoreCommand = "";
      GUIEnabled = 0;
      GUICommand = "";
    };

    "UserEvents/NewChatSession" = {
      CoreEnabled = 0;
      CoreCommand = "";
      GUIEnabled = 0;
      GUICommand = "";
    };

    "UserEvents/OutOfDiskSpace" = {
      CoreEnabled = 0;
      CoreCommand = "";
      GUIEnabled = 0;
      GUICommand = "";
    };

    "UserEvents/ErrorOnCompletion" = {
      CoreEnabled = 0;
      CoreCommand = "";
      GUIEnabled = 0;
      GUICommand = "";
    };

    HTTPDownload = {
      URL_1 = "https://shortypower.org/server.met";
      URL_2 = "http://upd.emule-security.org/server.met";
    };
  };
in

{
  options = {
    services.amule = {
      enable = mkOption {
        type = types.bool;
        default = false;
        description = ''
          Whether to run the AMule daemon.
        '';
      };

      enableWeb = mkOption {
        type = types.bool;
        default = false;
        description = ''
          Whether to run the AMule web interface.
        '';
      };

      config = mkOption {
        type = types.attrs;
        default = amuledDefaultConfig;
        description = ''
          The configuration for the AMule daemon. At a minimum, you need to
          provide: ExternalConnect.ECPassword, and Obfuscation.CryptoKadUDPKey.
        '';
      };

      dataDir = mkOption {
        type = types.str;
        default = "/var/lib/amule/";
        description = ''
          The directory holding the configuration, and by default incoming and
          temporary files.
        '';
      };

      user = mkOption {
        type = types.str;
        default = "amule";
        description = ''
          The user the AMule daemon should run as.
        '';
      };

      group = mkOption {
        type = types.str;
        default = "amule";
        description = ''
          The group the AMule daemon should run as.
        '';
      };

      package = mkPackageOption pkgs "amule-daemon" { };

    };
  };

  config = mkIf cfg.enable {
    assertions = [
      {
        assertion = amuledMergedConfig.ExternalConnect.ECPassword != null;
        message = ''
          <option>services.amule.config.ExternalConnect.ECPassword</option> needs to be set.
        '';
      }
      {
        assertion = amuledMergedConfig.Obfuscation.CryptoKadUDPKey != null;
        message = ''
          <option>services.amule.config.Obfuscation.CryptoKadUDPKey</option> needs to be set.
          You can generate a random unsigned 32bit integer using "od -An -N4 -tu4 < /dev/urandom | tr -d ' '"
        '';
      }
      {
        assertion = cfg.enableWeb -> amuledMergedConfig.WebServer.Password != null;
        message = ''
          <option>services.amule.config.WebServer.Password</option> needs to be set if the web interface is enabled.
        '';
      }
    ];

    systemd.tmpfiles.settings."10-amuled".${cfg.dataDir}.d = {
      inherit (cfg) user group;
      mode = "0700";
    };

    users.users = mkIf (cfg.user == "amule") {
      amule = {
        inherit (cfg) group;
        home = cfg.dataDir;
        isNormalUser = false;
        isSystemUser = true;
        uid = config.ids.uids.amule;
      };
    };

    users.groups = mkIf (cfg.group == "amule") {
      amule = {
        gid = config.ids.gids.amule;
      };
    };

    systemd.services.amuled = {
      description = "AMule daemon";
      wantedBy = [ "multi-user.target" ];
      after = [ "network.target" ];
      environment.HOME = cfg.dataDir;
      restartTriggers = [ amuledConfigFile ];

      # aMule requires write permissions to the config. A link to the store,
      # bind mount or the likes causes a crash, and if it's possible aMule
      # recreating the config as a normal file.
      preStart = ''
        rm -f ${cfg.dataDir}/amule.conf
        cat ${amuledConfigFile} > ${cfg.dataDir}/amule.conf
      '';

      serviceConfig = {
        User = "${cfg.user}";
        Group = "${cfg.group}";
        WorkingDirectory = cfg.dataDir;
        ExecStart = utils.escapeSystemdExecArgs (
          [
            (lib.getExe' cfg.package "amuled")
            "--config-dir=${config.services.amule.dataDir}"
          ]
          ++ optionals cfg.enableWeb [ "--use-amuleweb=${pkgs.amule-web}/bin/amuleweb" ]
        );

        # TODO: this failed
        Restart = "on-failure";
        RestartSec = "5s";

        # Hardening
        NoNewPrivileges = true;
        PrivateTmp = true;
        PrivateDevices = true;
        DevicePolicy = "closed";
        ProtectSystem = "strict";
        ReadWritePaths = cfg.dataDir;
        ProtectHome = "tmpfs";
        ProtectControlGroups = true;
        ProtectKernelModules = true;
        ProtectKernelTunables = true;
        RestrictAddressFamilies = "AF_INET";
        RestrictNamespaces = true;
        RestrictRealtime = true;
        RestrictSUIDSGID = true;
        LockPersonality = true;
      };
    };
  };
}

@aciceri aciceri force-pushed the fix-amule-module branch from cb8066d to b7d5545 Compare May 5, 2025 17:41
@aciceri
Copy link
Member Author

aciceri commented May 5, 2025

@someone-stole-my-name Cool! Thanks for the quick answer, I had no idea you've already worked on that, I will definitely steal from your module.
Let me know if you want to be added to maintainers.

However there is one thing that I can't understand in your implementation i.e. why you force the user into setting CryptoKadUDPKey. According to what I understood that value is a public key corresponding to a private key contained in cryptkey.dat (inside the configuration directory). It wasn't entirely clear to me before, I've just checked it because of your comment.

I've just edited my module, now there is an assertion that forces the user into not setting that key. Then, when aMule is first started I noticed that it edits the configuration file populating that value and creates the cryptkey.dat.

Does this make sense to you?

@someone-stole-my-name
Copy link
Contributor

However there is one thing that I can't understand in your implementation i.e. why you force the user into setting CryptoKadUDPKey. According to what I understood that value is a public key corresponding to a private key contained in cryptkey.dat (inside the configuration directory). It wasn't entirely clear to me before, I've just checked it because of your comment.

iirc the idea was to force the user to do what amule would do for him if the value was unset (src/Preferences.cpp#L1225). No idea what's the relationship between CryptoKadUDPKey and cryptkey.dat tbh. From what I can see they're generating a randomly seeded RSA in that file, and there's no relationship between the two values.

@aciceri
Copy link
Member Author

aciceri commented May 5, 2025

Probably you are right, they are not related. However what's the point of forcing the user into generating that value manually and then set the options when aMule already dynamically generates it?

Honestly I prefer leaving aMule generate it freeing the user from the burden of manually set it.

EDIT: I believe I was wrong, probably I didn't really delete the configuration file and restarted the service, I've just tried again and that value in the configuration file is empty. However I still think that that value may be automatically generated in the pre-start script.

@someone-stole-my-name
Copy link
Contributor

Probably you are right, they are not related. However what's the point of forcing the user into generating that value manually and then set the options when aMule already dynamically generates it?

Honestly I prefer leaving aMule generate it freeing the user from the burden of manually set it.

It's just a matter of taste I suppose 😅 I wanted config file to look exactly as specified by the user without hidden magic underneath. Your change forces a new CryptoKadUDPKey every time the config changes, right? If that works, that's fine too :)

@aciceri aciceri force-pushed the fix-amule-module branch 3 times, most recently from 14288f0 to b38b954 Compare May 6, 2025 09:21
@aciceri aciceri force-pushed the fix-amule-module branch 11 times, most recently from 966199f to c6cf19b Compare May 6, 2025 10:37
@ymarkus
Copy link
Contributor

ymarkus commented May 6, 2025

Sorry, I'm a bit surprised as I can't remember ever having anything to do with this, so I can't help.
Good luck, looks like a big change. 😀

@ymarkus ymarkus removed their request for review May 6, 2025 10:50
@aciceri
Copy link
Member Author

aciceri commented May 6, 2025

It's just a matter of taste I suppose 😅 I wanted config file to look exactly as specified by the user without hidden magic underneath. Your change forces a new CryptoKadUDPKey every time the config changes, right? If that works, that's fine too :)

I changed my mind several times, I did as you but then I've realized that I'm not very confident that CryptoKadUDPKey is meant to be public so I've tried to leave it empty and aMule still works perfectly. It doesn't populate that value but works, I've briefly tried looking at the code but honestly I didn't understand.

Sorry, I'm a bit surprised as I can't remember ever having anything to do with this, so I can't help.

Don't worry about that and thank you for answering!

@aciceri aciceri force-pushed the fix-amule-module branch from c6cf19b to 93141df Compare May 6, 2025 12:25
@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/prs-ready-for-review/3032/5455

@h7x4 h7x4 added the 8.has: module (new) This PR adds a module in `nixos/` label Jun 1, 2025
@aciceri aciceri force-pushed the fix-amule-module branch 2 times, most recently from ccce0ec to 2eafe4b Compare June 9, 2025 16:12
@aciceri aciceri force-pushed the fix-amule-module branch from 2eafe4b to 0d23c42 Compare June 9, 2025 16:12
@aciceri aciceri requested a review from drupol June 12, 2025 07:57
@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Jul 28, 2025
Copy link
Contributor

@SigmaSquadron SigmaSquadron left a comment

Choose a reason for hiding this comment

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

This has slipped through the cracks; forgive me. Please rebase to address the merge conflicts.

Please also give the option descriptions one final pass to fix missing punctuation if needed.

Comment on lines +28 to +223
SmartIdState = 1;
DropSlowSources = 0;
KadNodesUrl = "http://upd.emule-security.org/nodes.dat";
Ed2kServersUrl = "http://upd.emule-security.org/server.met";
ShowRatesOnTitle = 0;
GeoLiteCountryUpdateUrl = "http://geolite.maxmind.com/download/geoip/database/GeoLiteCountry/GeoIP.dat.gz";
StatsServerName = "Shorty's ED2K stats";
StatsServerURL = "http://ed2k.shortypower.dyndns.org/?hash=";
CreateSparseFiles = 1;
};
Browser = {
OpenPageInTab = 1;
CustomBrowserString = "";
};
Proxy = {
ProxyEnableProxy = 0;
ProxyType = 0;
ProxyName = "";
ProxyPort = 1080;
ProxyEnablePassword = 0;
ProxyUser = "";
ProxyPassword = "";
};
ExternalConnect = {
UseSrcSeeds = 0;
AcceptExternalConnections = 1;
ECAddress = "";
ECPort = null;
ECPassword = "";
UPnPECEnabled = 0;
ShowProgressBar = 1;
ShowPercent = 1;
UseSecIdent = 1;
IpFilterClients = 1;
IpFilterServers = 1;
TransmitOnlyUploadingClients = 0;
};
WebServer = {
Enabled = null;
Password = "";
PasswordLow = "";
Port = null;
WebUPnPTCPPort = 50001;
UPnPWebServerEnabled = 0;
UseGzip = 1;
UseLowRightsUser = 0;
PageRefreshTime = 120;
Template = "default";
Path = "amuleweb";
};
GUI = {
HideOnClose = 0;
};
Razor_Preferences = {
FastED2KLinksHandler = 1;
};
SkinGUIOptions = {
Skin = "";
};
Statistics = {
MaxClientVersions = 0;
};
Obfuscation = {
IsClientCryptLayerSupported = 1;
IsCryptLayerRequested = 1;
IsClientCryptLayerRequired = 0;
CryptoPaddingLenght = 254;
CryptoKadUDPKey = "";
};
PowerManagement = {
PreventSleepWhileDownloading = 0;
};
"UserEvents/DownloadCompleted" = {
CoreEnabled = 0;
CoreCommand = "";
GUIEnabled = 0;
GUICommand = "";
};
"UserEvents/NewChatSession" = {
CoreEnabled = 0;
CoreCommand = "";
GUIEnabled = 0;
GUICommand = "";
};
"UserEvents/OutOfDiskSpace" = {
CoreEnabled = 0;
CoreCommand = "";
GUIEnabled = 0;
GUICommand = "";
};
"UserEvents/ErrorOnCompletion" = {
CoreEnabled = 0;
CoreCommand = "";
GUIEnabled = 0;
GUICommand = "";
};
HTTPDownload = {
URL_1 = "";
};
};
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we actually need to access the value of every single one of these options? Does the final settings file need everything here?

The user the AMule daemon should run as.
Directory where aMule moves completed downloads.
Files in this directory are automatically shared.
Ensure the aMule service has write permissions
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Ensure the aMule service has write permissions

The module is already making sure of this, so no need to bother the user.

Comment on lines +262 to +267
OSDirectory = mkOption {
type = types.path;
default = cfg.dataDir;
defaultText = literalExpression "\${config.services.amule.dataDir}";
description = "On-disk state directory, probably you don't want to change this";
};
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
OSDirectory = mkOption {
type = types.path;
default = cfg.dataDir;
defaultText = literalExpression "\${config.services.amule.dataDir}";
description = "On-disk state directory, probably you don't want to change this";
};
OSDirectory = mkOption {
type = types.path;
default = cfg.dataDir;
defaultText = literalExpression "\${config.services.amule.dataDir}";
description = "On-disk state directory, probably you don't want to change this.";
internal = true;
};

Options that shouldn't be changed should be marked as internal.

Comment on lines +284 to +291
Enabled = lib.mkOption {
type = types.enum [
0
1
];
default = 0;
description = "Set to 1 to enable the web server";
};
Copy link
Contributor

Choose a reason for hiding this comment

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

If aMule makes heavy use of integer booleans, then it might be worth it to create a helper function that converted our true/false booleans into integer booleans, for ergonomics.

Comment on lines +343 to +347
openPeerPorts = mkEnableOption "open the peer port(s) in the firewall";

openExternalConnectPort = mkEnableOption "open the external connect port";

openWebServerPort = mkEnableOption "open the web server port";
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
openPeerPorts = mkEnableOption "open the peer port(s) in the firewall";
openExternalConnectPort = mkEnableOption "open the external connect port";
openWebServerPort = mkEnableOption "open the web server port";
openFirewall = mkEnableOption "opening the aMule ports in the firewall";

Any reason why this can't be combined into one openFirewall setting? Enabling it will open the aMule ports, and if the webserver is also enabled, then it'll open the webserver port as well.

users.groups = lib.mkIf (cfg.user == null) [
{
name = "amule";
gid = config.ids.gids.amule;
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we no longer using the amule GID?

@SigmaSquadron SigmaSquadron added the 0.kind: package adoption Requests or PRs for adopting packages that have no maintainers label Aug 14, 2025
Comment on lines +438 to +439
User = cfg.user;
Group = cfg.group;
Copy link
Contributor

Choose a reason for hiding this comment

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

Since there's only one service, can we use systemd's DynamicUser feature here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0.kind: package adoption Requests or PRs for adopting packages that have no maintainers 2.status: merge conflict This PR has merge conflicts with the target branch 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: documentation This PR adds or changes documentation 8.has: module (new) This PR adds a module in `nixos/` 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants