findEnabled can cause infinite recursion

[scottbot95]

Description

findEnabled will attempt to recurse through all attributes in the provided tree until it finds an attribute set that contains { enabled = true; } in each of the branches. This works in the "normal" case when it it used on a configuration like:

services.ethereum.geth.sepolia = {
    enable = true;
    ...
  };

  services.ethereum.geth.goerli = {
    enable = true;
    ...
  };

However if you disable any of the "top level" services, things start to break down. Since all services contain a package option, findEnabled will attempt to recurse into the derivation itself which will result in an infinite recursion as derivations always contain a reference to themselves (out). Additionally, since findEnabled is now searching deeper than it was actually intended for, it may now come across submodules that are enabled (such as geth.<name>.args.http.enable or even geth.<name>.backup.enable) despite the fact that they won't actually do anything since the outer module is disabled. This can result in additional evaluation errors as bad data is being fed into the backup/restore modules.

Steps to reproduce

1. Attempt to evaluate a module containing:

services.ethereum.geth.sepolia = {
    enable = false;
    backup = {
      enable = false; # not respected since findEnabled gets confused by get service being disabled
      restic.passwordFile = ""; # needed since backup/restore modules think backup/restore is enabled
    };
    restore = {
      enable = false; # not respected since findEnabled gets confused by get service being disabled
      restic.passwordFile = ""; # needed since backup/restore modules think backup/restore is enabled
      snapshot = ""; # needed since backup/restore modules think backup/restore is enabled
    };
};

2. Observe infinite recursion

Proposed solution

Modify findEnabled so that it only recurses if the current attribute set does not contain the enabled attribute regardless of its value. We still only add to sum if enabled == true, however for the case when enabled is present but false, we will consider that branch done and stop recursing. This will avoid us searching deeper than was intended which resulted in the bugs highlighted above.

[aldoborrero]

@scottbot95 good find. Also, we could add additional safeguards and only recurse if is not a derivation to avoid issues like you described before (with builtins.isDerivation).