Nonexclusive checkout never removes executable permissions

[EliahKagan]

Current behavior 😯

When a checkout is performed with destination_is_initially_empty set to its default value of false, i.e. a nonexclusive checkout, on a Unix-like system where the filesystem supports executable permissions, such permissions are set in a separate step from file creation for any regular file that is to be checked out as executable. Considering only nonexclusive checkout of a regular file where no files of any other type are involved in any way, so that an existing file would be reused, and disregarding unusual situations involving smudge filters^a or race conditions, this can be broken down into four cases:

1. The file is tracked as executable, and it did not already exist. Then it is created without executable permissions, and they are added in a subsequent step. (This differs from an exclusive checkout, where the file is created with executable permissions, and they are not changed in a subsequent step.) The file ends up correctly executable.
2. The file is not tracked as executable, and it did not already exist. Then it is created without executable permissions, and they are not added in a separate step. They are also not removed, but that is no problem, because the file was created without them. The file ends up correctly non-executable.
3. The file is tracked as executable, and it did already exist. The existing file is overwritten if overwrite_existing is true. Then executable permissions are added in a separate step. The file ends up correctly executable whether it was before or not.
4. The file is not tracked executable, and it did already exist. The existing file is overwritten if overwrite_existing is true. Because the file is not tracked as executable, executable permissions are not added in a separate step. But they are not removed, either. The file thus ends up correctly non-executable if it wasn't executable before, but ends up wrongly executable if it was executable before.

^a To clarify, an unusual situation with smudge filters in a nonexclusive checkout would be different from #1783, which is about a specific (but not necessarily unusual) situation with smudge filters in an exclusive checkout.

Relationship to CVE-2025-22620

This was discovered while working on GHSA-fqmf-w4xh-33rh (RUSTSEC-2025-0001), and it is mentioned briefly in the Impact section of the advisory, to cover the effect of its interaction with the vulnerability. But it is a separate bug from that vulnerability.

Furthermore, while the outcome is one in which permissions are broader than they might otherwise be, this is much less severe, to the point that I think it is not a vulnerability (though a fix for it might be considered a minor security enhancement, depending on the expected behavior). After bringing this up--though not in the level of detail it is presented here--in (private) comments in GHSA-fqmf-w4xh-33rh, that seems to be the consensus. So I'm opening this issue for it.

However, some of my thinking about the issue of how significant the bug is, with respect to security, is below, in case it turns out to be relevant.

There are two cases… (click to expand)

1. The file is executable already for reasons other than having been set executable in a previous call to gix_worktree_state::checkout. Then there are a few positions one could take:

   1. It is the responsibility of whoever/whatever set it executable to anticipate that the file may be reused in a checkout (as opposed to being deleted and replaced with a new file), or
   2. It is the responsibility of gix-worktree-state in a nonexclusive checkout to ensure the mode matches the index, to the point that setting unwanted permissions and checking out a file should be safe even if those permissions are not safe as applied, unmodified, to the file with its modified contents, or
   3. The reuse itself is not reasonable and is itself a separate bug, and the file should be deleted and replaced rather than reused, either always or in various cases including when the permissions don't match. (See #1783, "Expected behavior, 5(ii) for more on this possibility.)

   It seems to me that (1) or (3) is correct, though I acknowledge this is subjective. (The reuse itself--which has other effects such as when the reused file is accessed through another hard link or by a program that already has the file open--may be a bug, but it is not the bug being reported here.)

2. The file is executable already in a way that gix_worktree_state is responsible for, because a previous call to gix_worktreee_state::checkout set it executable. This is a situation that presumably does occur. In in, we fail to remove executable bits we added in a previous checkout that corresponded to an index different from the index being checked out now. However:

   • When adding executable permissions, the setuid, setgid, and sticky bits are removed. The setuid and setgid bits could be dangerous to keep while adding executable permissions for anyone, because they make the ability to execute a file decisively more powerful than the ability to read it: the set-user-ID bit causes the file to run with the effective user ID of the owner, and the set-group-ID causes the file to run with the effective group ID of the group-owner. The setgid bit in particular must be removed because, on some systems, setting it when no executable bits are set has a different meaning related to mandatory file locking, and therefore it may intentionally be set in the absence of an intention for anyone to be able to run the executable with an elevated/changed EGID.
   • Executable permissions are not added for users who cannot also read the file. (Unless users cannot read the file for reasons due neither to an inability to traverse directories to it nor due to its mode bits, such as an ACL on the file itself that strangely restricts reading but does not restrict execution.) A user who can traverse directories to a file and read it can usually make a copy that is executable, and can often effectively run it in place with an interpreter if it is a script, or a dynamic loader if it is a binary.

   These are both the case deliberately since #1764. But they were also true in the old vulnerable behavior of replacing the preexisting mode with 0777 (though the reason executable permissions were not added in the absence of read permissions is that read permissions were also added; more concerningly, write permissions were added).

Expected behavior 🤔

A nonexclusive checkout in gitoxide does not currently correspond exactly to any common operation with Git, and it does not have many guarantees. In particular, at least so far it is not intended as a guaranteed way to achieve an effect of switching branches:

• gix-worktree and gix-index (checkout, status, commit) #301
• Create and checkout a branch? #879
• fetch + reset possible today? #726

Nonetheless, I think it is preferable for it to treat file permissions more consistently. In addition, as described below, due to the documentation on a related option, I think developers who use gix_worktree_state may assume old executable permissions are not kept.

Still, that its semantics are not specified exactly, and that they do not yet cover what might one day be the main use of a nonexclusive checkout, suggests that my arguments here about what is expected should taken with a grain of salt. (This is also one of the reasons I do not think of this bug as a security vulnerability: it is not really going against any guarantees that have been articulated or that are likely to be confidently assumed.)

However, in that case, there would still be a bug here, because the current behavior, or at least nonintuitive aspects of what it might not do, should be given in documentation comments.

Greater symmetry between +x and -x

For regular files, Git repositories track a Unix-style executable bit. Where applicable to the operating system and filesystem, and where no configuration prohibits it, I think it is ideal to treat its -x and +x values symmetrically, except where security considerations suggest doing otherwise.

We already do this when destination_is_initially_empty is set to true, where the asymmetry, if any, would be due to executable bits being masked out by the process umask:

• If the file is tracked as executable, we attempt to create the file with initial mode bits of 0777, which is safe because open is one of the calls on a Unix-like system where mode arguments are subject to (i.e. reduced by) the umask:

  gitoxide/gix-worktree-state/src/checkout/entry.rs

  Lines 264 to 267 in 8df0db2

  	use std::os::unix::fs::OpenOptionsExt;
  	// Note that these only work if the file was newly created, but won't if it's already
  	// existing, possibly without the executable bit set. Thus we do this only if the file is new.
  	options.mode(0o777);

• If the file is tracked as non-executable, we do not call mode. In that, as documented for OpenOptionsExt::mode (and also safe because, and only because, it is subject to the umask):

  If no mode is set, the default of 0o666 will be used.

(It is not strictly true that the only asymmetry with destination_is_initially_empty: true is as dictated by the process umask, because there is #1783. But that issue describes a bug, not intended behavior.)

In contrast, when destination_is_initially_empty is (explicitly or implicitly) false, we:

• Implicitly request non-executable permissions if we are creating the file by opening it (which we do not assume is what happened).
• Explicitly do something roughly like chmod +x afterwards if the file is tracked as executable, but never do anything at all like chmod -x if the file is not tracked as executable.

This is asymmetric for existing files that may or may not already be executable. But there is no security reason to be less willing to remove an executable bit that to set it. The effect of having fewer executable permissions is generally less, not more, risky than having more (though the gap may be very small in the case we are usually working with here, with files that have read permissions for the same users who might have executable permissions, and that do not have the setuid or setgid bit set).

Greater similarity to git checkout --force

As alluded to above, not many promises are made for when destination_is_initially_empty is kept as false:

gitoxide/gix-worktree-state/src/checkout/mod.rs

Lines 50 to 53 in 8df0db2

	/// If true, we assume no file to exist in the target directory, and want exclusive access to it.
	/// This should be enabled when cloning to avoid checks for freshness of files. This also enables
	/// detection of collisions based on whether or not exclusive file creation succeeds or fails.
	pub destination_is_initially_empty: bool,

But that is likely to be read to mean that, if the destination is not initally empty and it is intended for the checkout to succeed anyway, then that option should not be used with a value of true.

The semantics of the overwrite_existing option--which would typically be used with destination_is_initially_empty kept as false, since the destination may not initially be empty--are more specific and make a comparison to an operation Git supports:

gitoxide/gix-worktree-state/src/checkout/mod.rs

Lines 54 to 58 in 8df0db2

	/// If true, default false, worktree entries on disk will be overwritten with content from the index
	/// even if they appear to be changed. When creating directories that clash with existing worktree entries,
	/// these will try to delete the existing entry.
	/// This is similar in behaviour as `git checkout --force`.
	pub overwrite_existing: bool,

To support being similar to git checkout --force when overwriting is permitted, it seems to me that destination_is_initially_empty: false should not preserve existing executable permissions when checking out a file that is non-executable in the index.

Git behavior

git checkout --force does not preserve executable permissions in the working tree when switching to a branch where colliding files are non-executable. I tested this on Arch Linux with:

git init -b main clobber
cd clobber
git commit --allow-empty -m 'Initial commit'
git switch -c feature
touch a
echo nonempty >b
git add .
git commit -m 'Add some non-executable files'
git switch main
ls -lF
touch a b
chmod +x a b
ls -lF
git checkout --force feature --
ls -lF

A full transcript of that test is:

ek in 🌐 catenary in ~/src
❯ git init -b main clobber
Initialized empty Git repository in /home/ek/src/clobber/.git/

ek in 🌐 catenary in ~/src
❯ cd clobber

ek in 🌐 catenary in clobber on  main
❯ git commit --allow-empty -m 'Initial commit'
[main (root-commit) 6a45365] Initial commit

ek in 🌐 catenary in clobber on  main
❯ git switch -c feature
Switched to a new branch 'feature'

ek in 🌐 catenary in clobber on  feature
❯ touch a

ek in 🌐 catenary in clobber on  feature [?]
❯ echo nonempty >b

ek in 🌐 catenary in clobber on  feature [?]
❯ git add .

ek in 🌐 catenary in clobber on  feature [+]
❯ git commit -m 'Add some non-executable files'
[feature 024e18f] Add some non-executable files
 2 files changed, 1 insertion(+)
 create mode 100644 a
 create mode 100644 b

ek in 🌐 catenary in clobber on  feature
❯ git switch main
Switched to branch 'main'

ek in 🌐 catenary in clobber on  main
❯ ls -lF
total 0

ek in 🌐 catenary in clobber on  main
❯ touch a b

ek in 🌐 catenary in clobber on  main [?]
❯ chmod +x a b

ek in 🌐 catenary in clobber on  main [?]
❯ ls -lF
total 0
-rwxr-xr-x 1 ek ek 0 Jan 20 02:43 a*
-rwxr-xr-x 1 ek ek 0 Jan 20 02:43 b*

ek in 🌐 catenary in clobber on  main [?]
❯ git checkout --force feature --
Switched to branch 'feature'

ek in 🌐 catenary in clobber on  feature
❯ ls -lF
total 4
-rw-r--r-- 1 ek ek 0 Jan 20 02:43 a
-rw-r--r-- 1 ek ek 9 Jan 20 02:43 b

I would say that, in and of itself, this experiment equally supports either (a) unsetting executable permissions, or (b) keeping the behavior the same but clarifying this, and possibly other ways, that a nonexclusive gix_worktree_state::checkout differs from git checkout --force, in documentation comments.

Steps to reproduce 🕹

With the default of overwrite_existing: false

With current versions (to confirm that the bug exists even after the #1764), follow steps 1 to 3 of the PoC instructions in GHSA-fqmf-w4xh-33rh. Two ways to do this are:

• Following them straightforwardly will have that effect, since as written they do not specify any package versions and a new version of gix-worktree-state and dependent crates has been released.
• The checkout-index repository has the PoC in project form. Its main branch deliberately has vulnerable versions (or crates that depend on them) in Cargo.toml and Cargo.lock. But the patched-version branch has current versions that are free of the vulnerability, so it can be used here. The repository has the described src/main.rs already. To get the has-executable repository, run ./make-repo.

Then, rather than leaving the files as they are or removing them as in step 4, flip their permissions:

chmod +x has-executable/a
chmod -x has-executable/b

We should now have:

$ ls -lF has-executable/
total 0
-rwxr-xr-x 1 ek ek 0 Jan 20 02:59 a*
-rw-r--r-- 1 ek ek 0 Jan 20 02:59 b

Now run cargo run. After this, observe that we have:

$ ls -lF has-executable
total 0
-rwxr-xr-x 1 ek ek 0 Jan 20 03:00 a*
-rwxr-xr-x 1 ek ek 0 Jan 20 03:00 b*

Both a and b are executable, even though it just checked out a from an index in which it was not executable, and even though it changed b to be executable because it was executable in the index.

With overwrite_existing: true

To confirm the applicability of the case where overwrite_existing is also set to true--because it is relevant to the arguments in "Expected behavior" above--make a change to src/main.rs similar to what is described in step 7 of the advisory, except set overwrite_existing: true (and not destination_is_initially_empty: true). That is, make it so the entire contents of src/main.rs are:

fn main() -> Result<(), Box<dyn std::error::Error>> {
    let repo = gix::discover("has-executable")?;
    let mut index = repo.open_index()?;
    gix::worktree::state::checkout(
        &mut index,
        repo.work_dir().ok_or("need non-bare repo")?,
        gix_object::find::Never, // Can also use: repo.objects.clone()
        &gix::progress::Discard,
        &gix::progress::Discard,
        &Default::default(),
        gix::worktree::state::checkout::Options {
            overwrite_existing: true,
            ..Default::default()
        },
    )?;
    Ok(())
}

Then either delete and recreate the has-executable test repository, or run git -C has-executable restore . and verify that the files are back to the way they are tracked in the index. Repeat the commands from above that switch the permissions so a is executable and b is non-executable, and run cargo run to build and run the modified program. Observe that, as was the case with implicit overwrite_existing: false, we have:

$ ls -lF has-executable
total 0
-rwxr-xr-x 1 ek ek 0 Jan 20 03:11 a*
-rwxr-xr-x 1 ek ek 0 Jan 20 03:11 b*

[Byron]

Thanks so much for researching this (and in such great detail).

Once again I wasn't able to give it the time it deserves just yet, but hope that all related issues could be solved in one go by overhauling the mode handling of files. Making the related flags, destination_is_initially_empty and overwrite_existing less ambiguous could also be explored then.