Skip to content

Commit

Permalink
git.c: make sure we do not leak GIT_* to alias scripts
Browse files Browse the repository at this point in the history
The unfortunate commit d95138e (setup: set env $GIT_WORK_TREE when
work tree is set, like $GIT_DIR - 2015-06-26) exposes another problem,
besides git-clone that's described in the previous commit. If
GIT_WORK_TREE (or even GIT_DIR) is exported to an alias script, it may
mislead git commands in the script where the repo is. Granted, most
scripts work on the repo where the alias is summoned from. But nowhere
do we forbid the script to visit another repository.

The revert of d95138e in the previous commit is sufficient as a
fix. However, to protect us from accidentally leaking GIT_*
environment variables again, we restore certain sensitive env before
calling the external script.

GIT_PREFIX is let through because there's another setup side effect
that we simply accepted so far: current working directory is
moved. Maybe in future we can introduce a new alias format that
guarantees no cwd move, then we can unexport GIT_PREFIX.

Reported-by: Gabriel Ganne <gabriel.ganne@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
  • Loading branch information
Nguyễn Thái Ngọc Duy authored and Junio C Hamano committed Dec 22, 2015
1 parent 86d26f2 commit 57ea712
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 3 deletions.
10 changes: 7 additions & 3 deletions git.c
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,16 @@ static void save_env_before_alias(void)
}
}

static void restore_env(void)
static void restore_env(int external_alias)
{
int i;
if (orig_cwd && chdir(orig_cwd))
if (!external_alias && orig_cwd && chdir(orig_cwd))
die_errno("could not move to %s", orig_cwd);
free(orig_cwd);
for (i = 0; i < ARRAY_SIZE(env_names); i++) {
if (external_alias &&
!strcmp(env_names[i], GIT_PREFIX_ENVIRONMENT))
continue;
if (orig_env[i])
setenv(env_names[i], orig_env[i], 1);
else
Expand Down Expand Up @@ -243,6 +246,7 @@ static int handle_alias(int *argcp, const char ***argv)
int argc = *argcp, i;

commit_pager_choice();
restore_env(1);

/* build alias_argv */
alias_argv = xmalloc(sizeof(*alias_argv) * (argc + 1));
Expand Down Expand Up @@ -291,7 +295,7 @@ static int handle_alias(int *argcp, const char ***argv)
ret = 1;
}

restore_env();
restore_env(0);

errno = saved_errno;

Expand Down
17 changes: 17 additions & 0 deletions t/t0001-init.sh
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,23 @@ test_expect_success 'plain nested in bare through aliased command' '
check_config bare-ancestor-aliased.git/plain-nested/.git false unset
'

test_expect_success 'No extra GIT_* on alias scripts' '
(
env | sed -ne "/^GIT_/s/=.*//p" &&
echo GIT_PREFIX && # setup.c
echo GIT_TEXTDOMAINDIR # wrapper-for-bin.sh
) | sort | uniq >expected &&
cat <<-\EOF >script &&
#!/bin/sh
env | sed -ne "/^GIT_/s/=.*//p" | sort >actual
exit 0
EOF
chmod 755 script &&
git config alias.script \!./script &&
( mkdir sub && cd sub && git script ) &&
test_cmp expected actual
'

test_expect_success 'plain with GIT_WORK_TREE' '
mkdir plain-wt &&
test_must_fail env GIT_WORK_TREE="$(pwd)/plain-wt" git init plain-wt
Expand Down

0 comments on commit 57ea712

Please sign in to comment.