populate file: check the .rsync-filter files that really apply

rsync reads .rsync-filter files in the directories above a source as
well, and their rules apply to the transfer.  The check for files that
rsync 3.5.0 and later can't read only starts if the source itself
contains one, so a subdirectory of a repository with a filter file at
its top gets synced without it, silently.  The check also starts for
filter files above the source.

The check's dry run is the transfer it stands for, as far as filtering
goes:

- It gets the source's exclude and filters, so it doesn't enter
  directories the transfer skips.
- It doesn't run for transfers that apply no .rsync-filter files, i.e.
  uploads of flakes and patches.
- A filter file the user may not read anyway doesn't count: rsync fails
  on its own when it gets to transfer that file, and --insecure-links
  changes nothing about it.
- rsync prints bytes it takes for unprintable as \#ooo, which depends on
  the locale.  File names get decoded before checking that they exist, so
  files below, e.g., directories with umlauts in their names don't pass
  for missing in the C locale.
- If rsync doesn't tell which filter files it reads at all, populate
  warns that it can't tell either.

The README mentions that --insecure-links also affects the way to the
target path on local targets, and that only rsync 3.5.0 ignores filter
files below directories that may be entered but not read.

Phases of scenarios can set environment variables for populate.  New
file scenarios sync a subdirectory whose filter file is above it, and a
source with a filter file below a directory with an umlaut in its name,
in the C locale.

5 files changed, 153 insertions, 44 deletions

diff --git a/README.md b/README.md
index 9f1b760..7fdd8eb 100644
--- a/README.md
+++ b/README.md
@@ -356,15 +356,17 @@ Supported attributes:
   of rsync. Defaults to `true`.
 
 * `insecureLinks` (optional) -
-  what to do if rsync can't read `.rsync-filter` files in `path`.  rsync 3.5.0
+  what to do if rsync can't read `.rsync-filter` files that apply to `path`:
+  the ones therein, and the ones in the directories above it.  rsync 3.5.0
   and later open them more strictly than the files they transfer, and ignore
-  the ones they can't open, e.g. because a directory on the way may be
-  entered but not read, like a home directory with mode 711, or because a
-  symlink on the way belongs to neither root nor the user deploying.  By
-  default, with `null`, populating fails with an error naming the file.  With
-  `true`, rsync reads such files anyway (`--insecure-links`); with `false`,
-  rsync syncs without them.  Both print a warning.  Older versions of rsync
-  aren't affected.
+  the ones they can't open, e.g. because a symlink on the way belongs to
+  neither root nor the user deploying, or, with rsync 3.5.0, because a
+  directory on the way may be entered but not read, like a home directory
+  with mode 711.  By default, with `null`, populating fails with an error
+  naming the file.  With `true`, rsync reads such files anyway
+  (`--insecure-links`), which also lets it pass such symlinks on the way to
+  the target path of a local target; with `false`, rsync syncs without them.
+  Both print a warning.  Older versions of rsync aren't affected.
 
 * `inputs` (optional) -
   inputs of the flake in this source to draw from other sources, see
diff --git a/pkgs/populate/default.nix b/pkgs/populate/default.nix
index 2811e27..c8229fb 100644
--- a/pkgs/populate/default.nix
+++ b/pkgs/populate/default.nix
@@ -1140,24 +1140,21 @@ let
     if test -d "$source_path"; then
       source_path=$source_path/
     fi
-    ${insecureLinks config.insecureLinks}
+    # Without .rsync-filter files applying, rsync reads none.
+    ${if config.filterFiles or true
+      then insecureLinks config
+      else "insecure_links="}
     # Each item gets logged with what changed about it, and its size, as
     # transferred and in total.
     ${rsync}/bin/rsync \
         ${optionalString config.useChecksum /* sh */ "--checksum"} \
         ${optionalString target.sudo /* sh */ "--rsync-path=\"sudo rsync\""} \
-        ${concatMapStringsSep " "
-          (pattern: /* sh */ "--exclude ${quote pattern}")
-          config.exclude} \
-        ${concatMapStringsSep " "
-          (filter: /* sh */ "--${filter.type} ${quote filter.pattern}")
-          config.filters} \
         -e ${quote (ssh' target)} \
         $insecure_links \
         --itemize-changes \
         --out-format=${quote "%i %n%L %'''b/%'''l"} \
         --info=backup,del,copy,misc,mount,name,nonreg,remove,skip,symsafe \
-        ${optionalString (config.filterFiles or true) "-F"} \
+        ${rsyncFilterArgs config} \
         -rlptD \
         ${optionalString config.deleteExcluded /* sh */ "--delete-excluded"} \
         "$source_path" \
@@ -1179,41 +1176,102 @@ let
     insecureLinks = null;
   };
 
+  # rsync's arguments for what a source excludes and filters, followed by
+  # -F, which applies .rsync-filter files, unless the config turns that off.
+  rsyncFilterArgs = config: /* sh */ ''
+    ${concatMapStringsSep " "
+      (pattern: /* sh */ "--exclude ${quote pattern}")
+      config.exclude} \
+    ${concatMapStringsSep " "
+      (filter: /* sh */ "--${filter.type} ${quote filter.pattern}")
+      config.filters} \
+    ${optionalString (config.filterFiles or true) "-F"}'';
+
   # rsync 3.5.0 and later open .rsync-filter files with a stricter walk than
   # the files they transfer, and treat the ones that walk can't open as
   # missing, without saying so.  It fails, e.g., for a directory on the way
   # that the user may enter but not read, like a home directory with mode
   # 711, or a symlink that neither root nor the user owns.  So rsync, when
-  # asked, tells which .rsync-filter files in the source in $source_path it
-  # reads.  If it reports existing ones as missing, setting fails with an
-  # error if it's null, syncs without them if it's false, and makes rsync
-  # open them like the files it transfers with --insecure-links if it's true,
-  # both with a warning.  Sets insecure_links to rsync's extra argument.
-  insecureLinks = setting: /* sh */ ''
+  # asked, tells which .rsync-filter files it reads for the source in
+  # $source_path: the ones therein, and the ones in the directories above
+  # it, which apply as well.  If it reports ones as missing that exist and
+  # that the user may read, the config's insecureLinks decides: populate
+  # fails with an error if it's null, syncs without them if it's false, and
+  # makes rsync open them like the files it transfers with --insecure-links
+  # if it's true, both with a warning.  Sets insecure_links to rsync's extra
+  # argument.
+  insecureLinks = config: let
+    setting = config.insecureLinks;
+  in /* sh */ ''
     insecure_links=
     rsync_version=$(${rsync}/bin/rsync --version | ${coreutils}/bin/head -n 1)
     rsync_version=$(set -- $rsync_version; echo "$3")
+    # Whether a .rsync-filter file applies at all.
+    filtered=$(${findutils}/bin/find "$source_path" -name .rsync-filter -print -quit)
+    above=$(${coreutils}/bin/dirname "$source_path")
+    while test -z "$filtered"; do
+      if test -e "$above"/.rsync-filter; then
+        filtered=$above/.rsync-filter
+      elif test "$above" = / || test "$above" = .; then
+        break
+      fi
+      above=$(${coreutils}/bin/dirname "$above")
+    done
     if test "$(printf '%s\n' 3.5.0 "$rsync_version" | ${coreutils}/bin/sort -V |
                ${coreutils}/bin/head -n 1)" = 3.5.0 &&
-        test -n "$(${findutils}/bin/find "$source_path" -name .rsync-filter -print -quit)"
+        test -n "$filtered"
     then
-      # The first existing .rsync-filter file rsync reports as missing.
-      ignored=$(
-        ${rsync}/bin/rsync --dry-run --debug=FILTER2 -F -rlptD \
-            "$source_path" "$TMPDIR"/rsync-filter-check 2>&1 |
-        while read -r line; do
-          case $line in
-            "[sender] parse_filter_file("*") [not found]")
-              file=''${line#*parse_filter_file(}
-              file=''${file%,*,*}
-              if test -e "$file"; then
-                printf '%s\n' "$file"
-                break
-              fi
-              ;;
-          esac
-        done
-      )
+      # A dry run of the same transfer, as far as filtering goes.
+      ${rsync}/bin/rsync --dry-run --debug=FILTER2 \
+          ${rsyncFilterArgs config} \
+          -rlptD \
+          "$source_path" "$TMPDIR"/rsync-filter-check \
+        > "$TMPDIR"/rsync-filter-check.log 2>&1 || :
+      # The first .rsync-filter file rsync reports as missing although it
+      # exists.  One the user may not read isn't for rsync to read anyway.
+      parsed=
+      ignored=
+      while IFS= read -r line; do
+        case $line in
+          "[sender] parse_filter_file("*)
+            parsed=1
+            ;;
+        esac
+        case $line in
+          "[sender] parse_filter_file("*") [not found]")
+            file=''${line#*parse_filter_file(}
+            file=''${file%,*,*}
+            # rsync prints bytes it takes for unprintable, which depends on
+            # the locale, as \#ooo.
+            escaped=$file
+            file=
+            while :; do
+              case $escaped in
+                *'\#'[0-7][0-7][0-7]*)
+                  literal=''${escaped%%'\#'[0-7][0-7][0-7]*}
+                  escaped=''${escaped#"$literal"'\#'}
+                  byte=$(printf "\\$(printf %s "$escaped" | ${coreutils}/bin/cut -c1-3)x")
+                  file=$file$literal''${byte%x}
+                  escaped=''${escaped#???}
+                  ;;
+                *)
+                  file=$file$escaped
+                  break
+                  ;;
+              esac
+            done
+            if test -e "$file" && test -r "$file"; then
+              ignored=$file
+              break
+            fi
+            ;;
+        esac
+      done < "$TMPDIR"/rsync-filter-check.log
+      ${coreutils}/bin/rm "$TMPDIR"/rsync-filter-check.log
+      if test -z "$parsed"; then
+        >&2 printf 'warning: rsync %s doesn'"'"'t tell which .rsync-filter files it reads, so it may sync %s without some of them unnoticed.\n' \
+            "$rsync_version" "$source_path"
+      fi
       if test -n "$ignored"; then
         ${if setting == null then /* sh */ ''
           >&2 printf 'error: rsync %s can'"'"'t read %s, so it would sync %s without it.  This happens, e.g., if a directory on the way isn'"'"'t readable, or if a symlink on the way belongs to neither root nor the user running rsync.  Set the source'"'"'s insecureLinks to true to make rsync read it anyway, or to false to sync without it.\n' \
diff --git a/tests/fixtures.sh b/tests/fixtures.sh
index d9d8b2b..4e794db 100755
--- a/tests/fixtures.sh
+++ b/tests/fixtures.sh
@@ -461,3 +461,13 @@ echo keep > /fixtures/filtered-parent/sub/keep
 echo garbage > /fixtures/filtered-parent/sub/garbage
 ln -s filtered-parent /fixtures/filtered-parent-link
 chown -h nobody /fixtures/filtered-parent-link
+
+# filtered-name, filtered-name-link: like filtered, but the filter file is in
+# a subdirectory whose name has a byte that rsync prints escaped in the C
+# locale.
+mkdir -p /fixtures/filtered-name/bär
+printf -- '- garbage\n' > /fixtures/filtered-name/bär/.rsync-filter
+echo keep > /fixtures/filtered-name/bär/keep
+echo garbage > /fixtures/filtered-name/bär/garbage
+ln -s filtered-name /fixtures/filtered-name-link
+chown -h nobody /fixtures/filtered-name-link
diff --git a/tests/scenarios.nix b/tests/scenarios.nix
index 7743482..2fb6609 100644
--- a/tests/scenarios.nix
+++ b/tests/scenarios.nix
@@ -96,10 +96,11 @@ let
     # pass for failing for an unrelated reason, e.g. a misspelt option.
     # targetPath deploys to another path than the scenario's target
     # directory, and controlTargetPath is the control for a path that makes
-    # evaluation fail.
+    # evaluation fail.  env sets environment variables for populate, as
+    # arguments of env.
     phase = {
       sources, backup ? false, prune ? false, deploy ? null,
-      targetPath ? path,
+      targetPath ? path, env ? "",
       expect ? "success", control ? null, controlTargetPath ? null,
       check ? "", mutate ? null, checkDeployer ? null, mutateDeployer ? null,
     }: let
@@ -134,6 +135,7 @@ let
           null
       else {
         populate = toString (command sources targetPath);
+        inherit env;
         expect = if expect == "success" then null else expect.fail;
         # No phase gets around checking the canaries.
         check = script (check + "\ncanary_check\n");
@@ -337,7 +339,44 @@ assert !(krops.isLocalTarget (targets.remote "/"));
           ''}
         '';
       }));
-    }) [ null true false ]) targetKinds;
+    }) [ null true false ]) targetKinds
+    ++ concatMap (targetKind: let
+      affected = lib.versionAtLeast rsyncVersion "3.5.0";
+      unreadable = name: { path, file, dir ? "x", env ? "" }: scenario {
+        name = "file-${targetKind}-insecureLinks${name}";
+        inherit targetKind;
+        prepare = ''
+          mkdir -p "$T"/${dir}
+          echo old > "$T"/${dir}/garbage
+        '';
+        phases = twice ({
+          sources.x.file = { inherit path; };
+          inherit env;
+        } // (if affected then {
+          expect.fail = "can't read ${file}, so it would sync ${path}/ without it.";
+        } else {
+          check = ''
+            expect_eq keep "$(cat "$T"/${dir}/keep)" keep
+            test ! -e "$T"/${dir}/garbage || fail "garbage not excluded"
+          '';
+        }));
+      };
+    in [
+      # The filter file is in the directory above what gets synced, where it
+      # applies as well.
+      (unreadable "Parent" {
+        path = "/fixtures/filtered-parent-link/sub";
+        file = "/fixtures/filtered-parent-link/.rsync-filter";
+      })
+      # The filter file is in a directory whose name rsync prints escaped
+      # in the C locale.
+      (unreadable "Name" {
+        path = "/fixtures/filtered-name-link";
+        file = "/fixtures/filtered-name-link/bär/.rsync-filter";
+        dir = "x/bär";
+        env = "LC_ALL=C";
+      })
+    ]) targetKinds;
 
   flake = matrix "flake" {
     kind = [ "git" "path" "subdir" "remote" ];
diff --git a/tests/vm.nix b/tests/vm.nix
index 5eb4dbb..014897a 100644
--- a/tests/vm.nix
+++ b/tests/vm.nix
@@ -152,7 +152,7 @@ in pkgs.testers.runNixOSTest {
                 for i, phase in enumerate(scenario["phases"]):
                     # A hang fails the scenario, not the whole test.
                     status, out = deployer.execute(
-                        f"HOME=/root timeout 600 {phase['populate']} 2>&1",
+                        f"env HOME=/root {phase['env']} timeout 600 {phase['populate']} 2>&1",
                         timeout=900,
                     )
                     if phase["expect"] is None: