From 24c06a5b48b3655cfb0701376760c82b1d3ee879 Mon Sep 17 00:00:00 2001
From: Tim Hockin <thockin@google.com>
Date: Sat, 7 Nov 2020 11:38:48 -0800
Subject: [PATCH] Normalize the root path

This makes sure there's never any confusion about whether it is an
absolute path or has symlinks or whatever.  Add e2e cases to cover.
---
 cmd/git-sync/main.go | 26 +++++++++++++++--
 test_e2e.sh          | 67 ++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 88 insertions(+), 5 deletions(-)

diff --git a/cmd/git-sync/main.go b/cmd/git-sync/main.go
index 0bd8996..c5b37eb 100644
--- a/cmd/git-sync/main.go
+++ b/cmd/git-sync/main.go
@@ -398,6 +398,16 @@ func main() {
 		}
 	}
 
+	if err := os.MkdirAll(*flRoot, 0700); err != nil {
+		fmt.Fprintf(os.Stderr, "ERROR: can't make root dir: %v", err)
+		os.Exit(1)
+	}
+	absRoot, err := normalizePath(*flRoot)
+	if err != nil {
+		fmt.Fprintf(os.Stderr, "ERROR: can't normalize root path: %v", err)
+		os.Exit(1)
+	}
+
 	if *flAddUser {
 		if err := addUser(); err != nil {
 			fmt.Fprintf(os.Stderr, "ERROR: can't write to /etc/passwd: %v\n", err)
@@ -496,7 +506,7 @@ func main() {
 	for {
 		start := time.Now()
 		ctx, cancel := context.WithTimeout(context.Background(), *flSyncTimeout)
-		if changed, hash, err := syncRepo(ctx, *flRepo, *flBranch, *flRev, *flDepth, *flRoot, *flLink, *flAskPassURL, *flSubmodules); err != nil {
+		if changed, hash, err := syncRepo(ctx, *flRepo, *flBranch, *flRev, *flDepth, absRoot, *flLink, *flAskPassURL, *flSubmodules); err != nil {
 			updateSyncMetrics(metricKeyError, start)
 			if *flMaxSyncFailures != -1 && failCount >= *flMaxSyncFailures {
 				// Exit after too many retries, maybe the error is not recoverable.
@@ -523,7 +533,7 @@ func main() {
 			if *flOneTime {
 				os.Exit(0)
 			}
-			if isHash, err := revIsHash(ctx, *flRev, *flRoot); err != nil {
+			if isHash, err := revIsHash(ctx, *flRev, absRoot); err != nil {
 				log.Error(err, "can't tell if rev is a git hash, exiting", "rev", *flRev)
 				os.Exit(1)
 			} else if isHash {
@@ -540,6 +550,18 @@ func main() {
 	}
 }
 
+func normalizePath(path string) (string, error) {
+	delinked, err := filepath.EvalSymlinks(path)
+	if err != nil {
+		return "", err
+	}
+	abs, err := filepath.Abs(delinked)
+	if err != nil {
+		return "", err
+	}
+	return abs, nil
+}
+
 func updateSyncMetrics(key string, start time.Time) {
 	syncDuration.WithLabelValues(key).Observe(time.Since(start).Seconds())
 	syncCount.WithLabelValues(key).Inc()
diff --git a/test_e2e.sh b/test_e2e.sh
index 2ecc8ee..e076db8 100755
--- a/test_e2e.sh
+++ b/test_e2e.sh
@@ -144,10 +144,30 @@ function remove_containers() {
 }
 
 ##############################################
-# Test HEAD one-time
+# Test HEAD one-time when root doesn't exist
 ##############################################
-testcase "head-once"
-# First sync
+testcase "head-once-root-doesnt-exist"
+echo "$TESTCASE" > "$REPO"/file
+git -C "$REPO" commit -qam "$TESTCASE"
+rm -rf "$ROOT" # remove the root to test
+GIT_SYNC \
+    --one-time \
+    --repo="file://$REPO" \
+    --branch=master \
+    --rev=HEAD \
+    --root="$ROOT" \
+    --link="link" \
+    > "$DIR"/log."$TESTCASE" 2>&1
+assert_link_exists "$ROOT"/link
+assert_file_exists "$ROOT"/link/file
+assert_file_eq "$ROOT"/link/file "$TESTCASE"
+# Wrap up
+pass
+
+##############################################
+# Test HEAD one-time when root exists
+##############################################
+testcase "head-once-root-exists"
 echo "$TESTCASE" > "$REPO"/file
 git -C "$REPO" commit -qam "$TESTCASE"
 GIT_SYNC \
@@ -164,6 +184,47 @@ assert_file_eq "$ROOT"/link/file "$TESTCASE"
 # Wrap up
 pass
 
+##############################################
+# Test HEAD one-time with a weird --root flag
+##############################################
+testcase "head-once-root-flag-is-weird"
+echo "$TESTCASE" > "$REPO"/file
+git -C "$REPO" commit -qam "$TESTCASE"
+GIT_SYNC \
+    --one-time \
+    --repo="file://$REPO" \
+    --branch=master \
+    --rev=HEAD \
+    --root="../../../../../$ROOT/../../../../../../$ROOT" \
+    --link="link" \
+    > "$DIR"/log."$TESTCASE" 2>&1
+assert_link_exists "$ROOT"/link
+assert_file_exists "$ROOT"/link/file
+assert_file_eq "$ROOT"/link/file "$TESTCASE"
+# Wrap up
+pass
+
+##############################################
+# Test HEAD one-time with a symlink in --root
+##############################################
+testcase "head-once-root-flag-has-symlink"
+echo "$TESTCASE" > "$REPO"/file
+git -C "$REPO" commit -qam "$TESTCASE"
+ln -s "$ROOT" "$DIR/rootlink" # symlink to test
+GIT_SYNC \
+    --one-time \
+    --repo="file://$REPO" \
+    --branch=master \
+    --rev=HEAD \
+    --root="$DIR/rootlink" \
+    --link="link" \
+    > "$DIR"/log."$TESTCASE" 2>&1
+assert_link_exists "$ROOT"/link
+assert_file_exists "$ROOT"/link/file
+assert_file_eq "$ROOT"/link/file "$TESTCASE"
+# Wrap up
+pass
+
 ##############################################
 # Test default syncing
 ##############################################