fix: file executable permissions check error on macos in docker conta…

[eeslook]

On macOS Sonoma v14.5, it was discovered that when using avocado in a Docker container to run scripts without execute permissions, they are considered to have executable permissions.

By directly reading the file's permission bits, the stat method can provide more accurate permission check results, especially in cases where user context and file system characteristics might affect the behavior of os.access. This method is closer to the underlying implementation of the file system, thus providing consistent results across different environments (such as inside and outside Docker containers). After entering the container using docker exec -it container bash, use the stat command to check the file permission bits.

Reference: #5945
Signed-off-by: Kui Li [email protected]

[richtja]

Hi @eeslook, your changes LGTM, just squash them into one commit please, there is no need to have commits to fix your initial changes.

Unfortunately, I don't have access to any macOS machines, therefore I can't reproduce your issue, but I am quite curious why os.access() is not working in such environment. Can you please provide deeper explanation or link documentation about how user context and file system characteristics might affect the behaviour of os.access()? It would be really helpful, thank you.

[eeslook]

Hi @richtja,

Regarding why it is recommended to use os.stat over os.access for checking file permissions, here are some relevant documentation and resource links:

1. Python Official Documentation:

   • os.stat()
   • os.access()

2. Security Considerations in Python Official Documentation:

   • Security considerations mention the security considerations of os.access:

     Note I/O operations may fail even when access() indicates that they would succeed, particularly for operations on network filesystems which may have permissions semantics beyond the usual POSIX permission-bit model.
     

3. Linux Manual Pages:

   • access(2) - Linux manual page

     access() checks whether the calling process can access the file pathname. If pathname is a symbolic link, it is dereferenced.
     
     The check is done using the calling process's real UID and GID, rather than the effective IDs as is done when actually attempting an operation (e.g., open(2)) on the file.
     
     This allows set-user-ID programs and capability-endowed programs to easily determine the invoking user's authority.  In other words, access() does not answer the "can I read/write/execute this file?" question.  It answers a slightly different question:"(assuming I'm a setuid binary) can the user who invoked me read/write/execute this file?", which gives set-user-ID programs the possibility to prevent malicious users from causing them to read files which users shouldn't be able to read.
     
     If the calling process is privileged (i.e., its real UID is zero), then an X_OK check is successful for a regular file if execute permission is enabled for any of the file owner, group, or other.
     

4. Secure Programming Practices:

   • Secure Programming for Linux and Unix HOWTO
     • This document discusses many best practices for secure programming, including avoiding the use of access() for permission checks to prevent race conditions and privilege escalation attacks.

These resources can help you understand why it is recommended to use os.stat over os.access for permission checks.

[richtja]

Hi @eeslook, thank you very much for provided resources. I have learned a lot today and the os.access is not that bulletproof as I thought.
The changes LGTM, there is only one problem with the commit signature. The commit is authored as likui <[email protected]> but in the commit messages you wrote Signed-off-by: Kui Li <[email protected]>. Can you please fix that before we will merge it? Thanks

[eeslook]

Hi @eeslook , thank you very much for provided resources. I have learned a lot today and the os.access is not that bulletproof as I thought. The changes LGTM, there is only one problem with the commit signature. The commit is authored as likui <[email protected]> but in the commit messages you wrote Signed-off-by: Kui Li <[email protected]>. Can you please fix that before we will merge it? Thanks

Hi @richtja, thank you for pointing that out. I have corrected the commit signature to ensure it matches the author information. The commit now includes the correct Signed-off-by line.
Please review the changes again, and let me know if there are any further issues.
Thanks!

[richtja]

Hi @eeslook , thank you very much for provided resources. I have learned a lot today and the os.access is not that bulletproof as I thought. The changes LGTM, there is only one problem with the commit signature. The commit is authored as likui <[email protected]> but in the commit messages you wrote Signed-off-by: Kui Li <[email protected]>. Can you please fix that before we will merge it? Thanks

Hi @richtja, thank you for pointing that out. I have corrected the commit signature to ensure it matches the author information. The commit now includes the correct Signed-off-by line. Please review the changes again, and let me know if there are any further issues. Thanks!

Hi @eeslook, thank you for the update, but it looks like your signature has changed. You have correctly changed the commit message to Signed-off-by: likui <[email protected]>, but with that change, your git signature has changed to Kui Li <[email protected]>. This is what I can see when I run git log:

commit bc7dfd242685f1efa2daa2cb97c399e45184f711 (HEAD -> eeslook_fix_os_access_err)
Author: Kui Li <[email protected]>
Date:   Wed Jun 19 09:08:39 2024 +0800

    fix file executable permissions check error on macos in docker container

    On macOS Sonoma v14.5, it was discovered that when using avocado in a Docker container
    to run scripts without execute permissions, they are considered to have executable permissions.

    By directly reading the file's permission bits, the stat method can provide more accurate
    permission check results, especially in cases where user context and file system
    characteristics might affect the behavior of os.access. This method is closer to the
    underlying implementation of the file system, thus providing consistent results across
    different environments (such as inside and outside Docker containers). After entering the
    container using docker exec -it container bash, use the stat command to check the
    file permission bits.

    Reference: #5945
    Signed-off-by: likui <[email protected]>

You can see that the Author and Signed-off-by differ, and that causes the check to fail. Haven't you updated your git configuration lately? Because that might cause this issue. Please do the update once more and make sure that Author and Signed-off-by lines has the same signature. Thank you.

[eeslook]

Hi @richtja, fixed.

[richtja]

Hi @eeslook, thank you very much, it LGTM.