@matheusbsbs / matheus-bagliano / commits / e35f202b45

setup: trace bare repository setups

safe.bareRepository=explicit is a safer default mode of operation, since it guards against the embedded bare repository attack [1]. Most end users don't use bare repositories directly, so they should be able to set safe.bareRepository=explicit, with the expectation that they can reenable bare repositories by specifying GIT_DIR or --git-dir. However, the user might use a tool that invokes Git on bare repositories without setting GIT_DIR (e.g. "go mod" will clone bare repositories [2]), so even if a user wanted to use safe.bareRepository=explicit, it wouldn't be feasible until their tools learned to set GIT_DIR. To make this transition easier, add a trace message to note when we attempt to set up a bare repository without setting GIT_DIR. This allows users and tool developers to audit which of their tools are problematic and report/fix the issue. When they are sufficiently confident, they would switch over to "safe.bareRepository=explicit". Note that this uses trace2_data_string(), which isn't supported by the "normal" GIT_TRACE2 target, only _EVENT or _PERF. [1] https://lore.kernel.org/git/kl6lsfqpygsj.fsf@chooglen-macbookpro.roam.corp.google.com/ [2] https://go.dev/ref/mod Signed-off-by: Glen Choo <chooglen@google.com> Signed-off-by: Josh Steadmon <steadmon@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Glen Choo committed May 1, 2023 at 10:30 UTC e35f202b4503256db148ad61487fe13aa75960f2

2 files changed +26 -7

setup.c

@@ -1352,6 +1352,7 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir,

1352 }

1353

1354 if (is_git_directory(dir->buf)) {

1355 + trace2_data_string("setup", NULL, "implicit-bare-repository", dir->buf);

1356 if (get_allowed_bare_repo() == ALLOWED_BARE_REPO_EXPLICIT)

1357 return GIT_DIR_DISALLOWED_BARE;

1358 if (!ensure_valid_ownership(NULL, NULL, dir->buf, report))

t/t0035-safe-bare-repository.sh

+25 -7

@@ -7,13 +7,26 @@ TEST_PASSES_SANITIZE_LEAK=true

8 pwd="$(pwd)"

10 -expect_accepted () {

11 - git "$@" rev-parse --git-dir

10 +expect_accepted_implicit () {

11 + test_when_finished 'rm "$pwd/trace.perf"' &&

12 + GIT_TRACE2_PERF="$pwd/trace.perf" git "$@" rev-parse --git-dir &&

13 + # Note: we're intentionally only checking that the bare repo has a

14 + # directory *prefix* of $pwd

15 + grep -F "implicit-bare-repository:$pwd" "$pwd/trace.perf"

16 +}

17 +

18 +expect_accepted_explicit () {

19 + test_when_finished 'rm "$pwd/trace.perf"' &&

20 + GIT_DIR="$1" GIT_TRACE2_PERF="$pwd/trace.perf" git rev-parse --git-dir &&

21 + ! grep -F "implicit-bare-repository:$pwd" "$pwd/trace.perf"

22 }

24 expect_rejected () {

15 - test_must_fail git "$@" rev-parse --git-dir 2>err &&

16 - grep -F "cannot use bare repository" err

25 + test_when_finished 'rm "$pwd/trace.perf"' &&

26 + test_env GIT_TRACE2_PERF="$pwd/trace.perf" \

27 + test_must_fail git "$@" rev-parse --git-dir 2>err &&

28 + grep -F "cannot use bare repository" err &&

29 + grep -F "implicit-bare-repository:$pwd" "$pwd/trace.perf"

30 }

32 test_expect_success 'setup bare repo in worktree' '

@@ -22,12 +35,13 @@ test_expect_success 'setup bare repo in worktree' '

35 '

37 test_expect_success 'safe.bareRepository unset' '

25 - expect_accepted -C outer-repo/bare-repo

38 + test_unconfig --global safe.bareRepository &&

39 + expect_accepted_implicit -C outer-repo/bare-repo

40 '

42 test_expect_success 'safe.bareRepository=all' '

43 test_config_global safe.bareRepository all &&

30 - expect_accepted -C outer-repo/bare-repo

44 + expect_accepted_implicit -C outer-repo/bare-repo

45 '

47 test_expect_success 'safe.bareRepository=explicit' '

@@ -47,7 +61,7 @@ test_expect_success 'safe.bareRepository in the repository' '

62 test_expect_success 'safe.bareRepository on the command line' '

63 test_config_global safe.bareRepository explicit &&

50 - expect_accepted -C outer-repo/bare-repo \

64 + expect_accepted_implicit -C outer-repo/bare-repo \

65 -c safe.bareRepository=all

66 '

@@ -60,4 +74,8 @@ test_expect_success 'safe.bareRepository in included file' '

74 expect_rejected -C outer-repo/bare-repo

75 '

77 +test_expect_success 'no trace when GIT_DIR is explicitly provided' '

78 + expect_accepted_explicit "$pwd/outer-repo/bare-repo"

79 +'

80 +

81 test_done