@matheusbsbs / matheus-bagliano / commits / 4e33535ea9

clone: error specifically with --local and symlinked objects

6f054f9fb3 (builtin/clone.c: disallow --local clones with symlinks, 2022-07-28) gives a good error message when "git clone --local" fails when the repo to clone has symlinks in "$GIT_DIR/objects". In bffc762f87 (dir-iterator: prevent top-level symlinks without FOLLOW_SYMLINKS, 2023-01-24), we later extended this restriction to the case where "$GIT_DIR/objects" is itself a symlink, but we didn't update the error message then - bffc762f87's tests show that we print a generic "failed to start iterator over" message. This is exacerbated by the fact that Documentation/git-clone.txt mentions neither restriction, so users are left wondering if this is intentional behavior or not. Fix this by adding a check to builtin/clone.c: when doing a local clone, perform an extra check to see if "$GIT_DIR/objects" is a symlink, and if so, assume that that was the reason for the failure and report the relevant information. Ideally, dir_iterator_begin() would tell us that the real failure reason is the presence of the symlink, but (as far as I can tell) there isn't an appropriate errno value for that. Also, update Documentation/git-clone.txt to reflect that this restriction exists. Signed-off-by: Glen Choo <chooglen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Glen Choo committed Apr 10, 2023 at 22:18 UTC 4e33535ea98ac16d2163e8e9fcbba5e015881e65

3 files changed +17 -2

Documentation/git-clone.txt

@@ -58,6 +58,11 @@ never use the local optimizations). Specifying `--no-local` will

58 override the default when `/path/to/repo` is given, using the regular

59 Git transport instead.

60 +

61 +If the repository's `$GIT_DIR/objects` has symbolic links or is a

62 +symbolic link, the clone will fail. This is a security measure to

63 +prevent the unintentional copying of files by dereferencing the symbolic

64 +links.

65 ++

66 *NOTE*: this operation can race with concurrent modification to the

67 source repository, similar to running `cp -r src dst` while modifying

68 `src`.

builtin/clone.c

+11 -1

@@ -331,8 +331,18 @@ static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest,

331

332 iter = dir_iterator_begin(src->buf, DIR_ITERATOR_PEDANTIC);

333

334 - if (!iter)

334 + if (!iter) {

335 + if (errno == ENOTDIR) {

336 + int saved_errno = errno;

337 + struct stat st;

338 +

339 + if (!lstat(src->buf, &st) && S_ISLNK(st.st_mode))

340 + die(_("'%s' is a symlink, refusing to clone with --local"),

341 + src->buf);

342 + errno = saved_errno;

343 + }

344 die_errno(_("failed to start iterator over '%s'"), src->buf);

345 + }

346

347 strbuf_addch(src, '/');

348 src_len = src->len;

t/t5604-clone-reference.sh

+1 -1

@@ -358,7 +358,7 @@ test_expect_success SYMLINKS 'clone repo with symlinked objects directory' '

358 test_must_fail git clone --local malicious clone 2>err &&

359

360 test_path_is_missing clone &&

361 - grep "failed to start iterator over" err

361 + grep "is a symlink, refusing to clone with --local" err

362 '

363

364 test_done