Subject: bug#77402: 31.0.50; Tramp: file-writable-p broken

From: Michael Albinus <[email-masked]>
Date: Mon, 31 Mar 2025 18:53:55 +0200

Gerd Möllmann <[email-masked]> writes:

Hi Gerd,

>>> I'm afraid that's also not it. I can now boil it down to the following
>>> test with emacs -Q (I'm always building without window system FWIW):
>>>
>>>   (file-writable-p "/ssh:mini:/tmp")
>>>    => nil
>>>   (file-writable-p (file-truename "/ssh:mini:/tmp"))
>>>    => t
>>>
>>> where
>>>
>>>   (file-truename "/ssh:mini:/tmp")
>>>    => "/ssh:mini:/private/tmp"
>>
>> Yes, this is the area I'm looking at. "/ssh:mini:/tmp" has file
>> permissions lrwxr-xr-x, and "/ssh:mini:/private/tmp" has file
>> permissions drwxrwxrwt. (file-writable-p "/ssh:mini:/tmp") should look
>> at the permissions of the target, but it doesn't. It looks at the
>> permissions of the symlink itself.
>
> That's what I suspected..
>
>> And everything is mixed happily with cached values, for performance
>> speedup ...
>
> ...and there I got stuck :-)

Finally., I could reproduce the problem. There was the weirdness, that on
GNU/Linux I couldn't give a symlink the permissions 555 (your "/tmp"
symlink has it), and so I couldn't test. Fortunately, I remembered my
FreeBSD 14 VM, and there I could reproduce the problem.

The appended patch ought to fix it. Could you, pls, test?

>> OK, I'll continue to understand what happens. In the meantime, you might
>> be better with setting tramp-use-file-attributes to nil, globally. At
>> least it doesn't use the caches then.
>
> Yeah, (setq tramp-use-file-attributes nil) makes it work. Thanks!

Don't forget to revert this, testing may patch.

Best regards, Michael.

diff --git a/lisp/net/tramp-sh.el b/lisp/net/tramp-sh.el
index 5e79d4be41e..047d93a51b8 100644
--- a/lisp/net/tramp-sh.el
+++ b/lisp/net/tramp-sh.el
@@ -1787,10 +1787,10 @@ tramp-sh-handle-file-executable-p
     (with-tramp-file-property v localname "file-executable-p"
       ;; Examine `file-attributes' cache to see if request can be
       ;; satisfied without remote operation.
-      (if (tramp-use-file-attributes v)
-	  (or (tramp-check-cached-permissions v ?x)
-	      (tramp-check-cached-permissions v ?s))
-	(tramp-run-test v "-x" localname)))))
+      (or (tramp-check-cached-permissions v ?x)
+	  (tramp-check-cached-permissions v ?s)
+	  (tramp-check-cached-permissions v ?t)
+	  (tramp-run-test v "-x" localname)))))

 (defun tramp-sh-handle-file-readable-p (filename)
   "Like `file-readable-p' for Tramp files."
@@ -1798,9 +1798,8 @@ tramp-sh-handle-file-readable-p
     (with-tramp-file-property v localname "file-readable-p"
       ;; Examine `file-attributes' cache to see if request can be
       ;; satisfied without remote operation.
-      (if (tramp-use-file-attributes v)
-	  (tramp-handle-file-readable-p filename)
-	(tramp-run-test v "-r" localname)))))
+      (or (tramp-handle-file-readable-p filename)
+	  (tramp-run-test v "-r" localname)))))

 ;; Functions implemented using the basic functions above.

@@ -1830,13 +1829,11 @@ tramp-sh-handle-file-writable-p
       (if (file-exists-p filename)
 	  ;; Examine `file-attributes' cache to see if request can be
 	  ;; satisfied without remote operation.
-	  (if (tramp-use-file-attributes v)
-	      (tramp-check-cached-permissions v ?w)
-	    (tramp-run-test v "-w" localname))
+          (or (tramp-check-cached-permissions v ?w)
+	      (tramp-run-test v "-w" localname))
 	;; If file doesn't exist, check if directory is writable.
-	(and
-	 (file-directory-p (file-name-directory filename))
-	 (file-writable-p (file-name-directory filename)))))))
+	(and (file-directory-p (file-name-directory filename))
+	     (file-writable-p (file-name-directory filename)))))))

 (defun tramp-sh-handle-file-ownership-preserved-p (filename &optional group)
   "Like `file-ownership-preserved-p' for Tramp files."
diff --git a/lisp/net/tramp.el b/lisp/net/tramp.el
index 91ba71510e1..082a385e56b 100644
--- a/lisp/net/tramp.el
+++ b/lisp/net/tramp.el
@@ -4373,8 +4373,7 @@ tramp-handle-file-readable-p
 	  ;; links.
 	  (and-let* ((symlink (file-symlink-p filename))
 		     ((stringp symlink))
-		     ((file-readable-p
-		       (concat (file-remote-p filename) symlink)))))))))
+		     ((file-readable-p (file-truename filename)))))))))

 (defun tramp-handle-file-regular-p (filename)
   "Like `file-regular-p' for Tramp files."
@@ -4468,7 +4467,12 @@ tramp-handle-file-writable-p
   (with-parsed-tramp-file-name (expand-file-name filename) nil
     (with-tramp-file-property v localname "file-writable-p"
       (if (file-exists-p filename)
-	  (tramp-check-cached-permissions v ?w)
+          (or (tramp-check-cached-permissions v ?w)
+	      ;; `tramp-check-cached-permissions' doesn't handle
+	      ;; symbolic links.
+	      (and-let* ((symlink (file-symlink-p filename))
+		         ((stringp symlink))
+		         ((file-writable-p (file-truename filename))))))
 	;; If file doesn't exist, check if directory is writable.
 	(and (file-directory-p (file-name-directory filename))
 	     (file-writable-p (file-name-directory filename)))))))
@@ -6480,14 +6484,14 @@ tramp-check-cached-permissions
                        ((eq ?r access) 1)
                        ((eq ?w access) 2)
                        ((eq ?x access) 3)
-                       ((eq ?s access) 3)))
+                       ((eq ?s access) 3)
+                       ((eq ?t access) 3)))
+              ((tramp-use-file-attributes vec))
 	      (file-attr (file-attributes (tramp-make-tramp-file-name vec)))
+              ;; Not a symlink.
+              ((not (stringp (file-attribute-type file-attr))))
 	      (remote-uid (tramp-get-remote-uid vec 'integer))
 	      (remote-gid (tramp-get-remote-gid vec 'integer)))
-    (or
-     ;; Not a symlink.
-     (eq t (file-attribute-type file-attr))
-     (null (file-attribute-type file-attr)))
     (or
      ;; World accessible.
      (eq access (aref (file-attribute-modes file-attr) (+ offset 6)))