Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

overwrite_existing: false is not documented to allow +x be set #1787

Open
EliahKagan opened this issue Jan 20, 2025 · 0 comments
Open

overwrite_existing: false is not documented to allow +x be set #1787

EliahKagan opened this issue Jan 20, 2025 · 0 comments

Comments

@EliahKagan
Copy link
Member

EliahKagan commented Jan 20, 2025

Current behavior 😯

In gix_worktree_state::checkout::options, destination_is_initially_empty and overwrite_existing are documented as:

/// 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,
/// 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,

This is technically consistent with the behavior (mentioned in the PoC and Impact sections of GHSA-fqmf-w4xh-33rh, but not part of that vulnerability and not changed in #1764) that:

  • When destination_is_initially_empty: false, executable permissions can be added to preexisting files, regardless of the value of overwrite_existing.
  • When destination_is_initially_empty: false and overwrite_existing: false, executable permissions can be added to preexisting files whose contents would also change if the same checkout were done with overwrite_existing: true. That is, a combination of content and permissions that didn't exist in the repository can easily arise, with all options set to the default, in the presence of preexisting files.

However, this seems unintuitive, so I think it should be documented for at least one of those options in the documentation comment, probably for overwrite_existing.

In case this behavior is actually going to change soon, which might imaginably occur depending on what approach is taken for #1783 and #1784, for now I'm opening this issue rather than a PR to change the documentation.

Expected behavior 🤔

See above.

Git behavior

Not directly applicable, because Git doesn't have these specific options. However, overwrite_existing: true is documented to cause a checkout similar in kind to git checkout --force. When git checkout is run without --force under a circumstance where --force would be needed to change contents, it likewise declines to change executable permissions.

Steps to reproduce 🕹

Although this issue is distinct from GHSA-fqmf-w4xh-33rh and from #1784, the procedures to reproduce either of them, with any recent version of gix-worktree-state and other crates, will also show that overwrite_existing can be false and a nonexclusive checkout still changes permissions (+x).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant