test(tools/git): a skip that exits 0 is a pass -- refuse instead
ci/woodpecker/pr/ci Pipeline was canceled
ci/woodpecker/pr/ci Pipeline was canceled
be-coder-07 (#1089 review 131) showed the precondition guard exited 0 when the scratch dir turned out to be inside a git worktree: ( cd "$TMP" && git rev-parse --git-dir ) && { echo "SKIP"; exit 0; } so pointing TMPDIR beneath a git worktree made this test PASS against unchanged main. A skip that exits 0 is indistinguishable from a pass -- the same defect this suite exists to catch, in the suite itself. Now: set GIT_CEILING_DIRECTORIES to the PHYSICAL path (it is matched physically, and /tmp is commonly a symlink, so a logical path silently disables the ceiling), and if the outside-a-repo precondition still cannot be established, FAIL with an explicit message rather than skipping. Replaying be-coder-07's attack (TMPDIR beneath a worktree): before: fix rc=0, main rc=0 <- both "pass", the vulnerability after: fix rc=1, main rc=1 <- both refuse; no false pass either way Normal conditions are unchanged and still discriminate: fix rc=0, main rc=1. The test refuses to report a result it cannot establish. That is weaker than running under a hostile TMPDIR and strictly better than lying about it. Reported-by: be-coder-07
This commit is contained in:
@@ -27,8 +27,21 @@ run_outside() { # $1=function name -> "rc:sawmessage"
|
||||
}
|
||||
check() { if [ "$2" = "$3" ]; then echo " PASS $1 ($2)"; else echo " FAIL $1: got $2, want $3"; fail=1; fi; }
|
||||
|
||||
# $TMP is deliberately not a git repo, and must not be inside one.
|
||||
( cd "$TMP" && git rev-parse --git-dir >/dev/null 2>&1 ) && { echo " SKIP scratch dir is inside a repo"; exit 0; }
|
||||
# $TMP must not be inside a git repo. Do not SKIP on failure: be-coder-07 showed the
|
||||
# original SKIP exited 0, so pointing TMPDIR beneath a git worktree made this test PASS
|
||||
# against unchanged main. A skip that exits 0 is indistinguishable from a pass.
|
||||
# GIT_CEILING_DIRECTORIES stops git walking above $TMP, making the condition hold
|
||||
# regardless of where TMPDIR lives, rather than merely detecting when it does not.
|
||||
# GIT_CEILING_DIRECTORIES is matched against the PHYSICAL path -- a symlinked TMPDIR
|
||||
# (/tmp is commonly one) makes the logical path never match, and the ceiling silently
|
||||
# does nothing. Resolve it before exporting.
|
||||
TMP="$(cd "$TMP" && pwd -P)"
|
||||
export GIT_CEILING_DIRECTORIES="$TMP"
|
||||
if ( cd "$TMP" && git rev-parse --git-dir >/dev/null 2>&1 ); then
|
||||
echo " FAIL scratch dir is inside a git repo even with GIT_CEILING_DIRECTORIES set;"
|
||||
echo " the outside-a-repo precondition cannot be established -- refusing to report a result"
|
||||
exit 1
|
||||
fi
|
||||
|
||||
echo "== outside a git repo: rc=1 AND the diagnostic is emitted =="
|
||||
check "detect_platform" "$(run_outside detect_platform)" "1:yes"
|
||||
|
||||
Reference in New Issue
Block a user