noBrokenSymlinks: check for unreadable symlinks

pkgs/build-support/setup-hooks/no-broken-symlinks.sh

@@ -13,14 +13,16 @@ postFixupHooks+=(noBrokenSymlinksInAllOutputs)
 
 # A symlink is "dangling" if it points to a non-existent target.
 # A symlink is "reflexive" if it points to itself.
-# A symlink is considered "broken" if it is either dangling or reflexive.
+# A symlink is "unreadable" if the readlink command fails, e.g. because of permission errors.
+# A symlink is considered "broken" if it is either dangling, reflexive or unreadable.
 noBrokenSymlinks() {
   local -r output="${1:?}"
   local path
   local pathParent
   local symlinkTarget
   local -i numDanglingSymlinks=0
   local -i numReflexiveSymlinks=0
+  local -i numUnreadableSymlinks=0
 
   # NOTE(@connorbaker): This hook doesn't check for cycles in symlinks.
 
@@ -33,7 +35,11 @@ noBrokenSymlinks() {
   # NOTE: path is absolute because we're running `find` against an absolute path (`output`).
   while IFS= read -r -d $'\0' path; do
     pathParent="$(dirname "$path")"
-    symlinkTarget="$(readlink "$path")"
+    if ! symlinkTarget="$(readlink "$path")"; then
+      nixErrorLog "the symlink $path is unreadable"
+      numUnreadableSymlinks+=1
+      continue
+    fi
 
     # Canonicalize symlinkTarget to an absolute path.
     if [[ $symlinkTarget == /* ]]; then
@@ -61,8 +67,8 @@ noBrokenSymlinks() {
     fi
   done < <(find "$output" -type l -print0)
 
-  if ((numDanglingSymlinks > 0 || numReflexiveSymlinks > 0)); then
-    nixErrorLog "found $numDanglingSymlinks dangling symlinks and $numReflexiveSymlinks reflexive symlinks"
+  if ((numDanglingSymlinks > 0 || numReflexiveSymlinks > 0 || numUnreadableSymlinks > 0)); then
+    nixErrorLog "found $numDanglingSymlinks dangling symlinks, $numReflexiveSymlinks reflexive symlinks and $numUnreadableSymlinks unreadable symlinks"
     exit 1
   fi
   return 0

pkgs/stdenv/generic/default.nix

@@ -69,6 +69,7 @@ let
       defaultNativeBuildInputs =
         extraNativeBuildInputs
         ++ [
+          ../../build-support/setup-hooks/no-broken-symlinks.sh
           ../../build-support/setup-hooks/audit-tmpdir.sh
           ../../build-support/setup-hooks/compress-man-pages.sh
           ../../build-support/setup-hooks/make-symlinks-relative.sh
@@ -77,7 +78,6 @@ let
           ../../build-support/setup-hooks/move-sbin.sh
           ../../build-support/setup-hooks/move-systemd-user-units.sh
           ../../build-support/setup-hooks/multiple-outputs.sh
-          ../../build-support/setup-hooks/no-broken-symlinks.sh
           ../../build-support/setup-hooks/patch-shebangs.sh
           ../../build-support/setup-hooks/prune-libtool-files.sh
           ../../build-support/setup-hooks/reproducible-builds.sh

pkgs/test/stdenv/hooks.nix

@@ -7,6 +7,7 @@
 # ordering should match defaultNativeBuildInputs
 
 {
+  no-broken-symlinks = import ./no-broken-symlinks.nix { inherit stdenv lib pkgs; };
   # TODO: add audit-tmpdir
   compress-man-pages =
     let
@@ -97,7 +98,6 @@
       [[ -e $out/bin/foo ]]
     '';
   };
-  no-broken-symlinks = import ./no-broken-symlinks.nix { inherit stdenv lib pkgs; };
   # TODO: add multiple-outputs
   patch-shebangs = import ./patch-shebangs.nix { inherit stdenv lib pkgs; };
   prune-libtool-files =

pkgs/test/stdenv/no-broken-symlinks.nix

@@ -17,6 +17,19 @@ let
     ln -s${optionalString (!absolute) "r"} "$out/reflexive-symlink" "$out/reflexive-symlink"
   '';
 
+  # Some platforms implement permissions for symlinks, while others - including Linux - ignore them.
+  # As a result, testing this hook's handling of unreadable symlinks requires careful attention to
+  # which kind of platform we're on. See the comments by `meta.badPlatforms` below for details.
+  platformsWithSymlinkPermissions = with lib.platforms; darwin ++ freebsd ++ netbsd ++ openbsd;
+  platformsWithoutSymlinkPermissions = lib.subtractLists platformsWithSymlinkPermissions lib.platforms.all;
+  mkUnreadableSymlink = absolute: ''
+    touch "$out/unreadable-symlink-target"
+    (
+      umask 777
+      ln -s${optionalString (!absolute) "r"} "$out/unreadable-symlink-target" "$out/unreadable-symlink"
+    )
+  '';
+
   mkValidSymlink = absolute: ''
     touch "$out/valid"
     ln -s${optionalString (!absolute) "r"} "$out/valid" "$out/valid-symlink"
@@ -31,10 +44,11 @@ let
       name,
       commands ? [ ],
       derivationArgs ? { },
+      meta ? { },
     }:
     stdenv.mkDerivation (
       {
-        inherit name;
+        inherit name meta;
         strictDeps = true;
         dontUnpack = true;
         dontPatch = true;
@@ -61,7 +75,7 @@ in
       }
       ''
         (( 1 == "$(cat "$failed/testBuildFailure.exit")" ))
-        grep -F 'found 1 dangling symlinks and 0 reflexive symlinks' "$failed/testBuildFailure.log"
+        grep -F 'found 1 dangling symlinks, 0 reflexive symlinks and 0 unreadable symlinks' "$failed/testBuildFailure.log"
         touch $out
       '';
 
@@ -81,7 +95,7 @@ in
       }
       ''
         (( 1 == "$(cat "$failed/testBuildFailure.exit")" ))
-        grep -F 'found 1 dangling symlinks and 0 reflexive symlinks' "$failed/testBuildFailure.log"
+        grep -F 'found 1 dangling symlinks, 0 reflexive symlinks and 0 unreadable symlinks' "$failed/testBuildFailure.log"
         touch $out
       '';
 
@@ -101,7 +115,7 @@ in
       }
       ''
         (( 1 == "$(cat "$failed/testBuildFailure.exit")" ))
-        grep -F 'found 0 dangling symlinks and 1 reflexive symlinks' "$failed/testBuildFailure.log"
+        grep -F 'found 0 dangling symlinks, 1 reflexive symlinks and 0 unreadable symlinks' "$failed/testBuildFailure.log"
         touch $out
       '';
 
@@ -121,7 +135,7 @@ in
       }
       ''
         (( 1 == "$(cat "$failed/testBuildFailure.exit")" ))
-        grep -F 'found 0 dangling symlinks and 1 reflexive symlinks' "$failed/testBuildFailure.log"
+        grep -F 'found 0 dangling symlinks, 1 reflexive symlinks and 0 unreadable symlinks' "$failed/testBuildFailure.log"
         touch $out
       '';
 
@@ -131,6 +145,61 @@ in
     derivationArgs.dontCheckForBrokenSymlinks = true;
   };
 
+  fail-unreadable-symlink-relative =
+    runCommand "fail-unreadable-symlink-relative"
+      {
+        failed = testBuildFailure (testBuilder {
+          name = "fail-unreadable-symlink-relative-inner";
+          commands = [ (mkUnreadableSymlink false) ];
+        });
+
+        # Skip test if symlink permissions are not supported, since the hook won't have anything to report.
+        meta.badPlatforms = platformsWithoutSymlinkPermissions;
+      }
+      ''
+        (( 1 == "$(cat "$failed/testBuildFailure.exit")" ))
+        grep -F 'found 0 dangling symlinks, 0 reflexive symlinks and 1 unreadable symlinks' "$failed/testBuildFailure.log"
+        touch $out
+      '';
+
+  pass-unreadable-symlink-relative-allowed = testBuilder {
+    name = "pass-unreadable-symlink-relative-allowed";
+    commands = [ (mkUnreadableSymlink false) ];
+    derivationArgs.dontCheckForBrokenSymlinks = true;
+
+    # This test will break on platforms that use symlink permissions, because even though this hook will be okay, later ones will error out.
+    # It should be safe to run on other platforms, just to make sure the hook isn't completely broken. It won't have anything to report, though.
+    meta.badPlatforms = platformsWithSymlinkPermissions;
+  };
+
+  fail-unreadable-symlink-absolute =
+    runCommand "fail-unreadable-symlink-absolute"
+      {
+        failed = testBuildFailure (testBuilder {
+          name = "fail-unreadable-symlink-absolute-inner";
+          commands = [ (mkUnreadableSymlink true) ];
+        });
+
+        # Skip test if symlink permissions are not supported, since the hook won't have anything to report.
+        meta.badPlatforms = platformsWithoutSymlinkPermissions;
+      }
+      ''
+        (( 1 == "$(cat "$failed/testBuildFailure.exit")" ))
+        grep -F 'found 0 dangling symlinks, 0 reflexive symlinks and 1 unreadable symlinks' "$failed/testBuildFailure.log"
+        touch $out
+      '';
+
+  pass-unreadable-symlink-absolute-allowed = testBuilder {
+    name = "pass-unreadable-symlink-absolute-allowed";
+    commands = [ (mkUnreadableSymlink true) ];
+    derivationArgs.dontCheckForBrokenSymlinks = true;
+
+    # This test will break on platforms that use symlink permissions, because even though this hook will be okay, later ones will error out.
+    # It should be safe to run on other platforms, just to make sure the hook isn't completely broken. It won't have anything to report, though.
+    meta.badPlatforms = platformsWithSymlinkPermissions;
+  };
+
+  # Leave the unreadable symlink out of the combined 'broken' test since it doesn't work on all platforms.
   fail-broken-symlinks-relative =
     runCommand "fail-broken-symlinks-relative"
       {
@@ -144,7 +213,7 @@ in
       }
       ''
         (( 1 == "$(cat "$failed/testBuildFailure.exit")" ))
-        grep -F 'found 1 dangling symlinks and 1 reflexive symlinks' "$failed/testBuildFailure.log"
+        grep -F 'found 1 dangling symlinks, 1 reflexive symlinks and 0 unreadable symlinks' "$failed/testBuildFailure.log"
         touch $out
       '';
 
@@ -170,7 +239,7 @@ in
       }
       ''
         (( 1 == "$(cat "$failed/testBuildFailure.exit")" ))
-        grep -F 'found 1 dangling symlinks and 1 reflexive symlinks' "$failed/testBuildFailure.log"
+        grep -F 'found 1 dangling symlinks, 1 reflexive symlinks and 0 unreadable symlinks' "$failed/testBuildFailure.log"
         touch $out
       '';
 
@@ -183,6 +252,84 @@ in
     derivationArgs.dontCheckForBrokenSymlinks = true;
   };
 
+  # The `all-broken` tests include unreadable symlinks along with the other kinds of broken links.
+  # They should be run/skipped on the same sets platforms as the corresponding `unreadable` tests.
+  fail-all-broken-symlinks-relative =
+    runCommand "fail-all-broken-symlinks-relative"
+      {
+        failed = testBuildFailure (testBuilder {
+          name = "fail-all-broken-symlinks-relative-inner";
+          commands = [
+            (mkDanglingSymlink false)
+            (mkReflexiveSymlink false)
+            (mkUnreadableSymlink false)
+          ];
+        });
+
+        # Skip test if symlink permissions are not supported, since the hook won't have anything to report.
+        meta.badPlatforms = platformsWithoutSymlinkPermissions;
+      }
+      ''
+        (( 1 == "$(cat "$failed/testBuildFailure.exit")" ))
+        if ! grep -F 'found 1 dangling symlinks, 1 reflexive symlinks and 1 unreadable symlinks' "$failed/testBuildFailure.log"; then
+          grep -F 'symlink permissions not supported' "$failed/testBuildFailure.log"
+          grep -F 'found 1 dangling symlinks, 1 reflexive symlinks and 0 unreadable symlinks' "$failed/testBuildFailure.log"
+        fi
+        touch $out
+      '';
+
+  pass-all-broken-symlinks-relative-allowed = testBuilder {
+    name = "pass-all-broken-symlinks-relative-allowed";
+    commands = [
+      (mkDanglingSymlink false)
+      (mkReflexiveSymlink false)
+      (mkUnreadableSymlink false)
+    ];
+    derivationArgs.dontCheckForBrokenSymlinks = true;
+
+    # This test will break on platforms that use symlink permissions, because even though this hook will be okay, later ones will error out.
+    # It should be safe to run on other platforms, just to make sure the hook isn't completely broken. It won't have anything to report, though.
+    meta.badPlatforms = platformsWithSymlinkPermissions;
+  };
+
+  fail-all-broken-symlinks-absolute =
+    runCommand "fail-all-broken-symlinks-absolute"
+      {
+        failed = testBuildFailure (testBuilder {
+          name = "fail-all-broken-symlinks-absolute-inner";
+          commands = [
+            (mkDanglingSymlink true)
+            (mkReflexiveSymlink true)
+            (mkUnreadableSymlink true)
+          ];
+        });
+
+        # Skip test if symlink permissions are not supported, since the hook won't have anything to report.
+        meta.badPlatforms = platformsWithoutSymlinkPermissions;
+      }
+      ''
+        (( 1 == "$(cat "$failed/testBuildFailure.exit")" ))
+        if ! grep -F 'found 1 dangling symlinks, 1 reflexive symlinks and 1 unreadable symlinks' "$failed/testBuildFailure.log"; then
+          grep -F 'symlink permissions not supported' "$failed/testBuildFailure.log"
+          grep -F 'found 1 dangling symlinks, 1 reflexive symlinks and 0 unreadable symlinks' "$failed/testBuildFailure.log"
+        fi
+        touch $out
+      '';
+
+  pass-all-broken-symlinks-absolute-allowed = testBuilder {
+    name = "pass-all-broken-symlinks-absolute-allowed";
+    commands = [
+      (mkDanglingSymlink true)
+      (mkReflexiveSymlink true)
+      (mkUnreadableSymlink true)
+    ];
+    derivationArgs.dontCheckForBrokenSymlinks = true;
+
+    # This test will break on platforms that use symlink permissions, because even though this hook will be okay, later ones will error out.
+    # It should be safe to run on other platforms, just to make sure the hook isn't completely broken. It won't have anything to report, though.
+    meta.badPlatforms = platformsWithSymlinkPermissions;
+  };
+
   pass-valid-symlink-relative = testBuilder {
     name = "pass-valid-symlink-relative";
     commands = [ (mkValidSymlink false) ];