Merge branch 'no-disable-arti-permission-checks' into 'main'

ci: don't disable arti permission checks, fix resulting issues

See merge request tpo/core/arti!530

.gitlab-ci.yml

@@ -7,6 +7,13 @@ stages:
 variables:
   # We don't need Husky to install the Git hooks for CI.
   CARGO_HUSKY_DONT_INSTALL_HOOKS: "true"
+  # fs-mistrust doesn't like umask 0
+  FF_DISABLE_UMASK_FOR_DOCKER_EXECUTOR: "true"
+
+default:
+  before_script:
+    # gitlab fetch strategy doesn't reset permissions
+    - (while [ "$PWD" != / ]; do chmod go-w . && cd ..; done)
 
 check-editorconfig:
   stage: check
@@ -134,9 +141,6 @@ build-repro:
 integration:
   stage: test
   image: debian:stable-slim
-  variables:
-    # The build environment here runs as root and seems to have umask 000.
-    ARTI_FS_DISABLE_PERMISSION_CHECKS: "true"
   script:
     - apt update
     - apt install -y tor git python3 curl dnsutils
@@ -157,9 +161,6 @@ coverage-aggregated:
   stage: test
   image: rust:latest
   needs: []
-  variables:
-    # The build environment here runs as root and seems to have umask 000.
-    ARTI_FS_DISABLE_PERMISSION_CHECKS: "true"
   script:
     - apt update && apt install -y tor python3 python3-pip python3-setuptools curl
     # install deps for report generation

crates/fs-mistrust/src/imp.rs

@@ -182,6 +182,14 @@ impl<'a> super::Verifier<'a> {
         if uid != 0 && Some(uid) != self.mistrust.trust_user {
             errors.push(Error::BadOwner(path.into(), uid));
         }
+
+        // On Unix-like platforms, symlink permissions are ignored (and usually
+        // not settable). Theoretically, the symlink owner shouldn't matter, but
+        // it's less confusing to consistently require the right owner.
+        if path_type == PathType::Symlink {
+            return;
+        }
+
         let mut forbidden_bits = if !self.readable_okay && path_type == PathType::Final {
             // If this is the target object, and it must not be readable, then
             // we forbid it to be group-rwx and all-rwx.

crates/fs-mistrust/src/lib.rs

@@ -694,7 +694,7 @@ impl<'a> Verifier<'a> {
 mod test {
     #![allow(clippy::unwrap_used)]
     use super::*;
-    use testing::Dir;
+    use testing::{Dir, LinkType};
 
     #[test]
     fn simple_cases() {
@@ -706,6 +706,7 @@ mod test {
         d.chmod("a/b/c", 0o700);
         d.chmod("e", 0o755);
         d.chmod("e/f", 0o777);
+        d.link_rel(LinkType::Dir, "a/b/c", "d");
 
         let m = Mistrust::builder()
             .trust_no_group_id()
@@ -719,6 +720,8 @@ mod test {
         let e = m.check_directory(d.path("e/f/g")).unwrap_err();
         assert!(matches!(e, Error::BadPermission(_, 0o022)));
         assert_eq!(e.path().unwrap(), d.path("e/f").canonicalize().unwrap());
+
+        m.check_directory(d.path("d")).unwrap();
     }
 
     #[cfg(target_family = "unix")]