git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v3 1/3] t: document regression git safe.directory when using sudo

From
Johannes Schindelin <johannes.schindelin@gmx.de>
Date
May 5, 2022, 13:44 UTC
Message-ID
<nycvar.QRO.7.76.6.2205051439290.355@tvgsbejvaqbjf.bet>
In-Reply-To
<20220503065442.95699-2-carenas@gmail.com>
Hi Carlo,
On Mon, 2 May 2022, Carlo Marcelo Arenas Belón wrote:
Show 44 quoted lines
> Originally reported after release of v2.35.2 (and other maint branches)
> for CVE-2022-24765 and blocking otherwise harmless commands that were
> done using sudo in a repository that was owned by the user.
>
> Add a new test script with very basic support to allow running git
> commands through sudo, so a reproduction could be implemented and that
> uses only `git status` as a proxy of the issue reported.
>
> Note that because of the way sudo interacts with the system, a much
> more complete integration with the test framework will require a lot
> more work and that was therefore intentionally punted for now.
>
> The current implementation requires the execution of a special cleanup
> function which should always be kept as the last "test" or otherwise
> the standard cleanup functions will fail because they can't remove
> the root owned directories that are used.  This also means that if
> failures are found while running the specifics of the failure might
> not be kept for further debugging and if the test was interrupted, it
> will be necessary to clean the working directory manually before
> restarting by running:
>
>   $ sudo rm -rf trash\ directory.t0034-root-safe-directory/
>
> The test file also uses at least one initial "setup" test that creates
> a parallel execution directory, while ignoring the repository created
> by the test framework, and special care should be taken when invoking
> commands through sudo, since the environment is otherwise independent
> from what the test framework expects.  Indeed `git status` was used
> as a proxy because it doesn't even require commits in the repository
> to work.
>
> A new SUDO prerequisite is provided that does some sanity checking
> to make sure the sudo command that will be used allows for passwordless
> execution as root and doesn't mess with git execution paths, but
> otherwise additional work will be required to ensure additional
> commands behave as expected and that will be addressed in a later patch.
>
> Most of those characteristics make this test mostly suitable only for
> CI, but it could be executed locally if special care is taken to provide
> for some of them in the local configuration and maybe making use of the
> sudo credential cache by first invoking sudo, entering your password if
> needed, and then invoking the test by doing:
>
>   $ IKNOWWHATIAMDOING=YES ./t0034-root-safe-directory.sh

Hmm. I would like to suggest that we can side-step all of these issues (and the ones I outline below) by considering a similar approach to the one Stolee took in t0033: use one or more `GIT_TEST_*` environment variables to pretend the exact scenario we want to test for.

Show 35 quoted lines
>
> Reported-by: SZEDER Gábor <szeder.dev@gmail.com>
> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
> ---
>  t/t0034-root-safe-directory.sh | 49 ++++++++++++++++++++++++++++++++++
>  1 file changed, 49 insertions(+)
>  create mode 100755 t/t0034-root-safe-directory.sh
>
> diff --git a/t/t0034-root-safe-directory.sh b/t/t0034-root-safe-directory.sh
> new file mode 100755
> index 00000000000..6dac7a05cfd
> --- /dev/null
> +++ b/t/t0034-root-safe-directory.sh
> @@ -0,0 +1,49 @@
> +#!/bin/sh
> +
> +test_description='verify safe.directory checks while running as root'
> +
> +. ./test-lib.sh
> +
> +if [ "$IKNOWWHATIAMDOING" != "YES" ]
> +then
> +	skip_all="You must set env var IKNOWWHATIAMDOING=YES in order to run this test"
> +	test_done
> +fi
> +
> +# this prerequisite should be added to all the tests, it not only prevents
> +# the test from failing but also warms up any authentication cache sudo
> +# might need to avoid asking for a password
> +test_lazy_prereq SUDO '
> +	sudo -n id -u >u &&
> +	id -u root >r &&
> +	test_cmp u r &&
> +	command -v git >u &&
> +	sudo command -v git >r &&

In my Ubuntu setup, `/bin/sh` is a symbolic link to `/bin/dash`, which does not understand the `command`. It might make more sense to use `type` here, but it is quite possible that `type git` uses a different output format than `sudo type git` if they use different shells.

Another complication is that the `/etc/sudoers` I have over here specifies a `secure_path`, which prevents the directory with the just-built `git` executable from being left in `PATH`. I had to edit `/etc/sudoers` _and_ change the script to using `sudo -sE` to fix these woes.

It took me a good chunk of time to figure out how to run these tests, and I will have to remember to revert the temporary edit of `/etc/sudoers` file. This is definitely not something I plan on doing often, so I wonder how these regression tests can guarantee that no regressions are introduced if they are so hard to run ;-)

Show 25 quoted lines
> +	test_cmp u r
> +'
> +
> +test_expect_success SUDO 'setup' '
> +	sudo rm -rf root &&
> +	mkdir -p root/r &&
> +	sudo chown root root &&
> +	(
> +		cd root/r &&
> +		git init
> +	)
> +'
> +
> +test_expect_failure SUDO 'sudo git status as original owner' '
> +	(
> +		cd root/r &&
> +		git status &&
> +		sudo git status
> +	)
> +'
> +
> +# this MUST be always the last test, if used more than once, the next
> +# test should do a full setup again.
> +test_expect_success SUDO 'cleanup' '
> +	sudo rm -rf root

This would be more canonical as `test_when_finished "sudo rm -rf root"` in the preceding test cases.

But as I said above, I would prefer it if we could figure out a way to pretend a specific scenario via `GIT_TEST_*`. That would ensure not only that those tests are easy to run, but also that they run whenever the test suite runs.

Thank you for working on this! Dscho

Show 7 quoted lines
> +'
> +
> +test_done
> --
> 2.36.0.352.g0cd7feaf86f
>
>
Previous: Phillip WoodNext: Phillip Wood
Message 105 of 170 in “git-compat-util: avoid failing dir ownership checks if running priviledged”
  1. git-compat-util: avoid failing dir ownership checks if running priviledgedCarlo Marcelo Arenas Belón, Apr 26, 2022
  2. Derrick StoleeApr 26, 2022
  3. Junio C HamanoApr 26, 2022
  4. rsbecker@nexbridge.comApr 26, 2022
  5. Carlo ArenasApr 26, 2022
  6. Junio C HamanoApr 26, 2022
  7. Carlo ArenasApr 26, 2022
  8. Carlo ArenasApr 26, 2022
  9. Derrick StoleeApr 29, 2022
  10. git-compat-util: avoid failing dir ownership checks if running privilegedCarlo Marcelo Arenas Belón, Apr 27, 2022
  11. Phillip WoodApr 27, 2022
  12. Phillip WoodApr 27, 2022
  13. rsbecker@nexbridge.comApr 27, 2022
  14. Carlo ArenasApr 27, 2022
  15. Phillip WoodApr 27, 2022
  16. Junio C HamanoApr 27, 2022
  17. Carlo ArenasApr 27, 2022
  18. rsbecker@nexbridge.comApr 27, 2022
  19. Junio C HamanoApr 27, 2022
  20. Phillip WoodApr 28, 2022
  21. Carlo ArenasApr 27, 2022
  22. rsbecker@nexbridge.comApr 27, 2022
  23. Junio C HamanoApr 27, 2022
  24. Carlo ArenasApr 27, 2022
  25. Phillip WoodApr 27, 2022
  26. rsbecker@nexbridge.comApr 27, 2022
  27. Carlo ArenasApr 27, 2022
  28. rsbecker@nexbridge.comApr 27, 2022
  29. Carlo ArenasApr 27, 2022
  30. rsbecker@nexbridge.comApr 27, 2022
  31. Phillip WoodApr 27, 2022
  32. Carlo ArenasApr 27, 2022
  33. Phillip WoodApr 27, 2022
  34. Carlo ArenasApr 27, 2022
  35. git-compat-util: avoid failing dir ownership checks if running privilegedCarlo Marcelo Arenas Belón, Apr 27, 2022
  36. Junio C HamanoApr 27, 2022
  37. 0/2 fix `sudo make install` regression in maintCarlo Marcelo Arenas Belón, Apr 28, 2022
  38. 1/2 Documentation: explain how safe.directory works when running under sudoCarlo Marcelo Arenas Belón, Apr 28, 2022
  39. Junio C HamanoApr 28, 2022
  40. Carlo ArenasApr 28, 2022
  41. Junio C HamanoApr 28, 2022
  42. 2/2 t: add tests for safe.directory when running with sudoCarlo Marcelo Arenas Belón, Apr 28, 2022
  43. Junio C HamanoApr 28, 2022
  44. Junio C HamanoApr 28, 2022
  45. 0/3 fix `sudo make install` regression in maintCarlo Marcelo Arenas Belón, Apr 28, 2022
  46. 1/3 git-compat-util: avoid failing dir ownership checks if running privilegedCarlo Marcelo Arenas Belón, Apr 28, 2022
  47. Phillip WoodApr 28, 2022
  48. Carlo ArenasApr 28, 2022
  49. 2/3 Documentation: explain how safe.directory works when running under sudoCarlo Marcelo Arenas Belón, Apr 28, 2022
  50. Bagas SanjayaApr 30, 2022
  51. Junio C HamanoApr 30, 2022
  52. Carlo Marcelo Arenas BelónApr 30, 2022
  53. 3/3 t: add tests for safe.directory when running with sudoCarlo Marcelo Arenas Belón, Apr 28, 2022
  54. Junio C HamanoApr 28, 2022
  55. Phillip WoodApr 28, 2022
  56. Junio C HamanoApr 28, 2022
  57. Carlo ArenasMay 6, 2022
  58. Junio C HamanoMay 6, 2022
  59. Carlo ArenasMay 6, 2022
  60. Junio C HamanoMay 6, 2022
  61. Carlo Marcelo Arenas BelónMay 7, 2022
  62. rsbecker@nexbridge.comApr 28, 2022
  63. Carlo ArenasApr 28, 2022
  64. rsbecker@nexbridge.comApr 28, 2022
  65. Junio C HamanoApr 28, 2022
  66. Carlo ArenasApr 28, 2022
  67. rsbecker@nexbridge.comApr 28, 2022
  68. Junio C HamanoApr 28, 2022
  69. rsbecker@nexbridge.comApr 28, 2022
  70. Junio C HamanoApr 28, 2022
  71. Junio C HamanoApr 28, 2022
  72. rsbecker@nexbridge.comApr 28, 2022
  73. Carlo ArenasApr 28, 2022
  74. Carlo ArenasApr 28, 2022
  75. Junio C HamanoApr 28, 2022
  76. Carlo Marcelo Arenas BelónApr 29, 2022
  77. Junio C HamanoApr 29, 2022
  78. Carlo Marcelo Arenas BelónApr 29, 2022
  79. 0/3 fix `sudo make install` regression in maintCarlo Marcelo Arenas Belón, May 2, 2022
  80. 1/3 t: document regression git safe.directory when using sudoCarlo Marcelo Arenas Belón, May 2, 2022
  81. Junio C HamanoMay 2, 2022
  82. Carlo ArenasMay 2, 2022
  83. 2/3 git-compat-util: avoid failing dir ownership checks if running privilegedCarlo Marcelo Arenas Belón, May 2, 2022
  84. 3/3 t0034: enhance framework to allow testing more commands under sudoCarlo Marcelo Arenas Belón, May 2, 2022
  85. Junio C HamanoMay 2, 2022
  86. Carlo ArenasMay 3, 2022
  87. 0/3 fix `sudo make install` regression in maintCarlo Marcelo Arenas Belón, May 3, 2022
  88. 2/3 git-compat-util: avoid failing dir ownership checks if running privilegedCarlo Marcelo Arenas Belón, May 3, 2022
  89. Johannes SchindelinMay 5, 2022
  90. Phillip WoodMay 5, 2022
  91. Carlo ArenasMay 6, 2022
  92. Junio C HamanoMay 6, 2022
  93. Carlo ArenasMay 6, 2022
  94. Junio C HamanoMay 6, 2022
  95. Carlo ArenasMay 6, 2022
  96. rsbecker@nexbridge.comMay 6, 2022
  97. Junio C HamanoMay 5, 2022
  98. Carlo ArenasMay 6, 2022
  99. 1/3 t: document regression git safe.directory when using sudoCarlo Marcelo Arenas Belón, May 3, 2022
  100. Phillip WoodMay 3, 2022
  101. Carlo Marcelo Arenas BelónMay 3, 2022
  102. Phillip WoodMay 4, 2022
  103. Carlo ArenasMay 4, 2022
  104. Phillip WoodMay 4, 2022
  105. Johannes SchindelinMay 5, 2022
  106. Phillip WoodMay 5, 2022
  107. Junio C HamanoMay 5, 2022
  108. Junio C HamanoMay 5, 2022
  109. Junio C HamanoMay 5, 2022
  110. Carlo ArenasMay 6, 2022
  111. Phillip WoodMay 9, 2022
  112. Carlo ArenasMay 9, 2022
  113. Phillip WoodMay 9, 2022
  114. Junio C HamanoMay 9, 2022
  115. Carlo ArenasMay 9, 2022
  116. Carlo ArenasMay 6, 2022
  117. 3/3 t0034: enhance framework to allow testing more commands under sudoCarlo Marcelo Arenas Belón, May 3, 2022
  118. Phillip WoodMay 3, 2022
  119. Junio C HamanoMay 3, 2022
  120. Carlo ArenasMay 6, 2022
  121. 0/3 fix `sudo make install` regression in maintCarlo Marcelo Arenas Belón, May 7, 2022
  122. 1/3 t: regression git needs safe.directory when using sudoCarlo Marcelo Arenas Belón, May 7, 2022
  123. 2/3 git-compat-util: avoid failing dir ownership checks if running privilegedCarlo Marcelo Arenas Belón, May 7, 2022
  124. Junio C HamanoMay 7, 2022
  125. Carlo Marcelo Arenas BelónMay 7, 2022
  126. Junio C HamanoMay 9, 2022
  127. rsbecker@nexbridge.comMay 9, 2022
  128. Carlo ArenasMay 9, 2022
  129. rsbecker@nexbridge.comMay 9, 2022
  130. Junio C HamanoMay 9, 2022
  131. 3/3 t0034: add negative tests and allow git init to mostly work under sudoCarlo Marcelo Arenas Belón, May 7, 2022
  132. Phillip WoodMay 10, 2022
  133. Carlo ArenasMay 10, 2022
  134. 0/3 fix `sudo make install` regression in maintCarlo Marcelo Arenas Belón, May 10, 2022
  135. 1/3 t: regression git needs safe.directory when using sudoCarlo Marcelo Arenas Belón, May 10, 2022
  136. Junio C HamanoMay 10, 2022
  137. Carlo ArenasMay 10, 2022
  138. Junio C HamanoMay 10, 2022
  139. Carlo ArenasMay 11, 2022
  140. Junio C HamanoMay 11, 2022
  141. 2/3 git-compat-util: avoid failing dir ownership checks if running privilegedCarlo Marcelo Arenas Belón, May 10, 2022
  142. Junio C HamanoMay 10, 2022
  143. Carlo ArenasMay 11, 2022
  144. Junio C HamanoMay 11, 2022
  145. 3/3 t0034: add negative tests and allow git init to mostly work under sudoCarlo Marcelo Arenas Belón, May 10, 2022
  146. Junio C HamanoMay 10, 2022
  147. Junio C HamanoMay 10, 2022
  148. Carlo ArenasMay 11, 2022
  149. Junio C HamanoMay 11, 2022
  150. 0/4 fix `sudo make install` regression in maintCarlo Marcelo Arenas Belón, May 13, 2022
  151. 1/4 t: regression git needs safe.directory when using sudoCarlo Marcelo Arenas Belón, May 13, 2022
  152. SZEDER GáborJun 3, 2022
  153. 2/4 git-compat-util: avoid failing dir ownership checks if running privilegedCarlo Marcelo Arenas Belón, May 13, 2022
  154. SZEDER GáborJun 3, 2022
  155. Junio C HamanoJun 3, 2022
  156. SZEDER GáborJun 3, 2022
  157. 3/4 t0034: add negative tests and allow git init to mostly work under sudoCarlo Marcelo Arenas Belón, May 13, 2022
  158. Junio C HamanoMay 13, 2022
  159. Carlo ArenasMay 14, 2022
  160. Junio C HamanoMay 15, 2022
  161. Carlo ArenasMay 15, 2022
  162. Junio C HamanoMay 16, 2022
  163. Carlo Marcelo Arenas BelónMay 16, 2022
  164. Junio C HamanoMay 16, 2022
  165. 4/4 git-compat-util: allow root to access both SUDO_UID and root ownedCarlo Marcelo Arenas Belón, May 13, 2022
  166. Johannes SchindelinJun 15, 2022
  167. Carlo ArenasJun 17, 2022
  168. Junio C HamanoJun 17, 2022
  169. git-compat-util: allow root to access both SUDO_UID and root ownedCarlo Marcelo Arenas Belón, Jun 17, 2022
  170. Junio C HamanoJun 17, 2022

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.