Unverified Commit 4af22aab authored by Naïm Favier's avatar Naïm Favier
Browse files

stdenv/check-meta: do deep type checks

Use a wrapper around `mergeDefinitions` to type-check values deeply, so
that e.g. `maintainers = [ 42 ];` is an error.
parent 4df10deb
Loading
Loading
Loading
Loading
+4 −2
Original line number Diff line number Diff line
@@ -289,7 +289,9 @@ rec {
       (This means fn is type Val -> String.) */
    allowPrettyValues ? false,
    /* If this option is true, the output is indented with newlines for attribute sets and lists */
    multiline ? true
    multiline ? true,
    /* Initial indentation level */
    indent ? ""
  }:
    let
    go = indent: v: with builtins;
@@ -348,7 +350,7 @@ rec {
                };") v)
        + outroSpace + "}"
    else abort "generators.toPretty: should never happen (v = ${v})";
  in go "";
  in go indent;

  # PLIST handling
  toPlist = {}: v: let
+31 −27
Original line number Diff line number Diff line
@@ -247,16 +247,16 @@ let
      isEnabled = lib.findFirst (x: x == reason) null showWarnings;
    in if isEnabled != null then builtins.trace msg true else true;

  # Deep type-checking. Note that calling `type.check` is not enough: see `lib.mkOptionType`'s documentation.
  # We don't include this in lib for now because this function is flawed: it accepts things like `mkIf true 42`.
  typeCheck = type: value: let
    merged = lib.mergeDefinitions [ ] type [
      { file = lib.unknownModule; inherit value; }
    ];
    eval = builtins.tryEval (builtins.deepSeq merged.mergedValue null);
  in eval.success;

  # A shallow type check. We are using NixOS'
  # option types here, which however have the major drawback
  # of not providing full type checking (part of the type check is
  # done by the module evaluation itself). Therefore, the checks
  # will not recurse into attributes.
  # We still provide the full type for documentation
  # purposes and in the hope that they will be used eventually.
  # See https://github.com/NixOS/nixpkgs/pull/191171 for an attempt
  # to fix this, or mkOptionType in lib/types.nix for more information.
  # TODO make this into a proper module and use the generic option documentation generation?
  metaTypes = with lib.types; rec {
    # These keys are documented
    description = str;
@@ -266,9 +266,11 @@ let
    homepage = either (listOf str) str;
    downloadPage = str;
    changelog = either (listOf str) str;
    license = either (listOf lib.types.attrs) (either lib.types.attrs str);
    sourceProvenance = either (listOf lib.types.attrs) lib.types.attrs;
    maintainers = listOf (attrsOf str);
    license = let
      licenseType = either (attrsOf anything) str; # TODO disallow `str` licenses, use a module
    in either licenseType (listOf licenseType);
    sourceProvenance = either (listOf (attrsOf anything)) (attrsOf anything);
    maintainers = listOf (attrsOf anything); # TODO use the maintainer type from lib/tests/maintainer-module.nix
    priority = int;
    platforms = listOf str;
    hydraPlatforms = listOf str;
@@ -310,16 +312,16 @@ let
    badPlatforms = platforms;
  };

  # WARNING: this does not check inner values of the attribute, like list elements or nested attributes.
  # See metaTypes above and mkOptionType in lib/types.nix for more information
  checkMetaAttr = k: v:
    if metaTypes?${k} then
      if metaTypes.${k}.check v then
      if typeCheck metaTypes.${k} v then
        null
      else
        "key 'meta.${k}' has a value of invalid type ${builtins.typeOf v}; expected ${metaTypes.${k}.description}"
        "key 'meta.${k}' has invalid value; expected ${metaTypes.${k}.description}, got\n    ${
          lib.generators.toPretty { indent = "    "; } v
        }"
    else
      "key 'meta.${k}' is unrecognized; expected one of: \n\t      [${lib.concatMapStringsSep ", " (x: "'${x}'") (lib.attrNames metaTypes)}]";
      "key 'meta.${k}' is unrecognized; expected one of: \n  [${lib.concatMapStringsSep ", " (x: "'${x}'") (lib.attrNames metaTypes)}]";
  checkMeta = meta: if config.checkMeta then lib.remove null (lib.mapAttrsToList checkMetaAttr meta) else [];

  checkOutputsToInstall = attrs: let
@@ -333,25 +335,27 @@ let
  # Check if a derivation is valid, that is whether it passes checks for
  # e.g brokenness or license.
  #
  # Return { valid: Bool } and additionally
  # Return { valid: "yes", "warn" or "no" } and additionally
  # { reason: String; errormsg: String } if it is not valid, where
  # reason is one of "unfree", "blocklisted", "broken", "insecure", ...
  # !!! reason strings are hardcoded into OfBorg, make sure to keep them in sync
  # Along with a boolean flag for each reason
  checkValidity = attrs:
    {
    # Check meta attribute types first, to make sure it is always called even when there are other issues
    # Note that this is not a full type check and functions below still need to by careful about their inputs!
    let res = checkMeta (attrs.meta or {}); in if res != [] then
      { valid = "no"; reason = "unknown-meta"; errormsg = "has an invalid meta attrset:${lib.concatMapStrings (x: "\n  - " + x) res}\n";
        unfree = false; nonSource = false; broken = false; unsupported = false; insecure = false;
      }
    else {
      unfree = hasUnfreeLicense attrs;
      nonSource = hasNonSourceProvenance attrs;
      broken = isMarkedBroken attrs;
      unsupported = hasUnsupportedPlatform attrs;
      insecure = isMarkedInsecure attrs;
    }
    // (
    # Check meta attribute types first, to make sure it is always called even when there are other issues
    # Note that this is not a full type check and functions below still need to by careful about their inputs!
    let res = checkMeta (attrs.meta or {}); in if res != [] then
      { valid = "no"; reason = "unknown-meta"; errormsg = "has an invalid meta attrset:${lib.concatMapStrings (x: "\n\t - " + x) res}"; }
    } // (
    # --- Put checks that cannot be ignored here ---
    else if checkOutputsToInstall attrs then
    if checkOutputsToInstall attrs then
      { valid = "no"; reason = "broken-outputs"; errormsg = "has invalid meta.outputsToInstall"; }

    # --- Put checks that can be ignored here ---