Skip to content

Working directory path validation - #5823

Merged
ethomson merged 22 commits into
mainfrom
ethomson/path_validation
May 2, 2021
Merged

Working directory path validation#5823
ethomson merged 22 commits into
mainfrom
ethomson/path_validation

Conversation

@ethomson

Copy link
Copy Markdown
Member

Add git_path_validate_ondisk which validates... well, on-disk paths. These paths could be working directory paths or things inside the .git directory.

The goal is to limit Windows path lengths to MAX_PATH in general. For working directory paths when core.longpaths is set, we will allow arbitrary length paths.

We do not consult core.longpaths for things outside the working directory and will restrict any paths inside .git to be less than MAX_PATH. Note that this is not in keeping with Git for Windows, which does no validation and will fail silently when trying to construct a directory where a critical component of the .git directory would be longer than MAX_PATH. 🤷

This converts several path writers to use this, but not all. In particular, submodules and worktrees will require some more careful thought. In addition, I'll perform a more extensive audit before converting this from a draft to a real PR. 🙃

@ethomson
ethomson marked this pull request as draft March 20, 2021 17:19
@ethomson

Copy link
Copy Markdown
Member Author

Thinking through this, I think that a rough validation plan is:

  1. Anything that reads/writes in the working directory (eg, checkout, staging, patch application, etc.) There are several of these - and there are some weird cases around worktrees and submodules that need to be dealt with - but this is largely pretty straightforward since we have a repository and can look up its configuration.

  2. Anything that deals with references. We should validate that loose references or reflogs will not end up getting written with > MAX_PATH characters. There's no prior art here in Git for Windows that we are required to emulate: although Git for Windows did not really define whether core.longpaths applies to references or just things in the working directory, in practice it seems to ignore the setting and just blindly try writing very long reference names and ignoring the failure. 🤷

  3. Everything else inside the .git folder: we can limit the gitpath to MAX_PATH - length_of_longest_filename_inside. (I think that this is the length of a loose object.) This means that we don't need to validate _every since call to CreateFile, we can just validate the length of the git directory when we open a repository. 😅

@ethomson
ethomson force-pushed the ethomson/path_validation branch 5 times, most recently from 1729a42 to 6c5217b Compare April 5, 2021 09:59
@ethomson ethomson mentioned this pull request Apr 14, 2021
4 tasks
@ethomson
ethomson force-pushed the ethomson/path_validation branch from 805e9ca to 92d8e69 Compare April 14, 2021 21:53
Move the utf8 functions into a proper namespace `git_utf8` instead of
being in the namespaceless `git__` function group.  Update them to
have out-params first and use `char *` instead of `uint8_t *` to match
our API treating strings as `char *` (even if they truly contain `uchar`s
inside).
Introduce a function to determine the number of Unicode characters in a
given UTF-8 string.
If we want to validate more and different types of paths, the name
`git_path_validate` makes that easier and more expressive.  We can add,
for example, `git_path_validate_foo` while the current name makes that
less ergonomic.
@ethomson
ethomson force-pushed the ethomson/path_validation branch 2 times, most recently from f271ee5 to fc47848 Compare April 14, 2021 22:31
@ethomson
ethomson dismissed a stale review via ac38750 April 24, 2021 08:47
@ethomson
ethomson force-pushed the ethomson/path_validation branch 2 times, most recently from 37a7b94 to 5f516ee Compare April 28, 2021 10:45
@ethomson
ethomson marked this pull request as ready for review April 28, 2021 12:02
ethomson added 11 commits April 28, 2021 13:03
Introduce `git_path_validate_filesystem` which validates (absolute) on-disk
paths and `git_path_validate_workdir` to perform validations on (absolute)
working directory paths.  These functions are useful as there may be system
limitations on on-disk paths, particularly on Windows (for example,
enforcing MAX_PATH).

For working directory paths, these limitations may be per-repository, based
on the `core.longpaths` configuration setting.
There was no test ensuring that we validate `.git` paths.  We do, but
let's add a test to make sure that we never regress this.
Add a simple accessor for workdir paths to get an absolute on-disk path
given a repository and a relative path within it.  This is useful to
avoid copy-pasta `git_buf_joinpath` and to ensure that we validate
working directory paths while honoring `core.longpaths` settings.
We're not necessarily checking out into the working directory.  We could
be checking out into an arbitrary location.  Ensure that when we are
writing conflict data that we do it in the checkout target.
Ensure that we are validating working directory paths before we try to
write to them.
Use `git_repository_workdir_path` to generate workdir paths since it
will validate the length.
The new git_repository_workdir_path function does error checking on
working directory inputs on Windows; use it to construct paths within
working directories.
We should allow attribute files - inside working directories - to have
names longer than MAX_PATH when core.longpaths is set.
`git_attr_path__init` takes a repository to validate the path with.
Supply the repository for the filesystem and workdir iterators - for
workdir iterators, this is non-null and we can lookup the core.longpaths
configuration option.  (For regular filesystem iterators, this is NULL,
so core.longpaths does not apply.)
Validate that working directory paths honor `core.longpaths` where
appropriate.  Paths to the submodule gitdirs must always honor the
operating system length restrictions; `core.longpaths` does not affect
gitdir paths.
Worktree paths need to fix within MAX_PATH always, regardless of
`core.longpaths` setting.
Let `git_path_find_dir` simply take a `git_buf` that contains a
directory or a file, instead of trying to both join a path AND then deal
with prettifying it or its basename.  This allows consumers to join
paths themselves (and apply any necessary rules - like fitting within
MAX_PATH).
On Windows, we need to enforce MAX_PATH for loose references and their
reflogs.  Ensure that any path - including the lock file - would fit
within the 260 character maximum.

We do not honor core.longpaths for loose reference files or reflogs.
core.longpaths only applies to paths in the working directory.
Ensure that a repository's path (at initialization or open time) is
valid.  On Windows systems, this means that the longest known path
beneath the repository will fit within MAX_PATH: this is a lock file for
a loose object within the repository itself.

Other paths, like a very long loose reference, may fail to be opened
after the repository is opened.  These variable length paths will be
checked when they are accessed themselves.  This new functionality is
done at open to prevent needlessly checking every file in the gitdir
(eg, `MERGE_HEAD`) for its length when we could instead check once at
repository open time.
@ethomson
ethomson force-pushed the ethomson/path_validation branch from 5f516ee to c15ed35 Compare April 28, 2021 12:03
@ethomson

ethomson commented May 2, 2021

Copy link
Copy Markdown
Member Author

Going to go ahead and merge this to land #5347

@ethomson
ethomson merged commit c44f568 into main May 2, 2021
@ethomson
ethomson deleted the ethomson/path_validation branch May 13, 2021 22:08
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

Successfully merging this pull request may close these issues.

1 participant