Exercise the git "dubious ownership" path

To do this, we run the e2e test as a different user.  To do that, we
need git-sync to make sure that everything is group accessible.  To
clean up after the test, we need everything to be group writable.  To do
that, we add a new flag: `--group-write`.

cmd/git-sync/main.go

@@ -93,6 +93,10 @@ var flSyncOnSignal = pflag.String("sync-on-signal",
 var flMaxFailures = pflag.Int("max-failures",
 	envInt(0, "GITSYNC_MAX_FAILURES", "GIT_SYNC_MAX_FAILURES"),
 	"the number of consecutive failures allowed before aborting (the first sync must succeed, -1 will retry forever")
+
+var flGroupWrite = pflag.Bool("group-write",
+	envBool(false, "GITSYNC_GROUP_WRITE", "GIT_SYNC_GROUP_WRITE"),
+	"ensure that all data (repo, worktrees, etc.) is group writable")
 var flChmod = pflag.Int("change-permissions",
 	envInt(0, "GITSYNC_PERMISSIONS", "GIT_SYNC_PERMISSIONS"),
 	"optionally change permissions on the checked-out files to the specified mode")
@@ -261,6 +265,8 @@ const (
 	gcOff        = "off"
 )
 
+const defaultDirMode = os.FileMode(0775) // subject to umask
+
 func init() {
 	prometheus.MustRegister(syncDuration)
 	prometheus.MustRegister(syncCount)
@@ -730,10 +736,18 @@ func main() {
 		os.Exit(1)
 	}
 
-	// Make sure the root exists.  0755 ensures that this is usable as a volume
+	// If the user asked for group-writable data, make sure the umask allows it.
-	// when the consumer isn't running as the same UID.  We do this very early
+	if *flGroupWrite {
-	// so that we can normalize the path even when there are symlinks in play.
+		syscall.Umask(0002)
-	if err := os.MkdirAll(absRoot.String(), 0755); err != nil {
+	} else {
+		syscall.Umask(0022)
+	}
+
+	// Make sure the root exists.  defaultDirMode ensures that this is usable
+	// as a volume when the consumer isn't running as the same UID.  We do this
+	// very early so that we can normalize the path even when there are
+	// symlinks in play.
+	if err := os.MkdirAll(absRoot.String(), defaultDirMode); err != nil {
 		log.Error(err, "ERROR: can't make root dir", "path", absRoot)
 		os.Exit(1)
 	}
@@ -1049,7 +1063,7 @@ func makeAbsPath(path string, root absPath) absPath {
 // its timestamps are updated.
 func touch(path absPath) error {
 	dir := path.Dir()
-	if err := os.MkdirAll(dir, 0755); err != nil {
+	if err := os.MkdirAll(dir, defaultDirMode); err != nil {
 		return err
 	}
 	file, err := os.Create(path.String())
@@ -1218,10 +1232,10 @@ func (git *repoSync) initRepo(ctx context.Context) error {
 	_, err := os.Stat(git.root.String())
 	switch {
 	case os.IsNotExist(err):
-		// Probably the first sync.  0755 ensures that this is usable as a
+		// Probably the first sync.  defaultDirMode ensures that this is usable
-		// volume when the consumer isn't running as the same UID.
+		// as a volume when the consumer isn't running as the same UID.
 		git.log.V(2).Info("repo directory does not exist, creating it", "path", git.root)
-		if err := os.MkdirAll(git.root.String(), 0755); err != nil {
+		if err := os.MkdirAll(git.root.String(), defaultDirMode); err != nil {
 			return err
 		}
 	case err != nil:
@@ -1366,7 +1380,7 @@ func (git *repoSync) publishSymlink(ctx context.Context, worktree worktree) erro
 	linkDir, linkFile := git.link.Split()
 
 	// Make sure the link directory exists.
-	if err := os.MkdirAll(linkDir, os.FileMode(int(0755))); err != nil {
+	if err := os.MkdirAll(linkDir, defaultDirMode); err != nil {
 		return fmt.Errorf("error making symlink dir: %v", err)
 	}
 
@@ -1473,8 +1487,7 @@ func (git *repoSync) configureWorktree(ctx context.Context, worktree worktree) e
 		defer source.Close()
 
 		if _, err := os.Stat(gitInfoPath); os.IsNotExist(err) {
-			fileMode := os.FileMode(int(0755))
+			err := os.Mkdir(gitInfoPath, defaultDirMode)
-			err := os.Mkdir(gitInfoPath, fileMode)
 			if err != nil {
 				return err
 			}
@@ -1972,6 +1985,7 @@ func (git *repoSync) SetupDefaultGitConfigs(ctx context.Context) error {
 		key: "safe.directory",
 		val: "*",
 	}}
+
 	for _, kv := range configs {
 		if _, err := git.Run(ctx, "", "config", "--global", kv.key, kv.val); err != nil {
 			return fmt.Errorf("error configuring git %q %q: %v", kv.key, kv.val, err)
@@ -2293,6 +2307,13 @@ OPTIONS
             - off: Disable explicit git garbage collection, which may be a good
               fit when also using --one-time.
 
+    --group-write, $GITSYNC_GROUP_WRITE
+            Ensure that data written to disk (including the git repo metadata,
+            checked out files, worktrees, and symlink) are all group writable.
+            This corresponds to git's notion of a "shared repository".  This is
+            useful in cases where data produced by git-sync is used by a
+            different UID.
+
     -h, --help
             Print help text and exit.
 

pkg/logging/logging.go

@@ -104,7 +104,7 @@ func (l *Logger) DeleteErrorFile() {
 // writeContent writes the error content to the error file.
 func (l *Logger) writeContent(content []byte) {
 	if _, err := os.Stat(l.root); os.IsNotExist(err) {
-		fileMode := os.FileMode(0755)
+		fileMode := os.FileMode(0775) // umask applies
 		if err := os.Mkdir(l.root, fileMode); err != nil {
 			l.Logger.Error(err, "can't create the root directory", "root", l.root)
 			return

test_e2e.sh

@@ -188,6 +188,7 @@ ROOT="$DIR/root"
 function clean_root() {
     rm -rf "$ROOT"
     mkdir -p "$ROOT"
+    chmod g+rwx "$ROOT"
 }
 
 # How long we wait for sync operations to happen between test steps, in seconds
@@ -210,6 +211,7 @@ DOT_SSH="$DIR/dot_ssh"
 mkdir -p "$DOT_SSH"
 ssh-keygen -f "$DOT_SSH/id_test" -P "" >/dev/null
 cat "$DOT_SSH/id_test.pub" > "$DOT_SSH/authorized_keys"
+chmod -R g+r "$DOT_SSH"
 
 TEST_TOOLS="_test_tools"
 SLOW_GIT_FETCH="$TEST_TOOLS/git_slow_fetch.sh"
@@ -221,8 +223,9 @@ EXECHOOK_COMMAND_FAIL_SLEEPY="$TEST_TOOLS/exechook_command_fail_with_sleep.sh"
 EXECHOOK_ENVKEY=ENVKEY
 EXECHOOK_ENVVAL=envval
 RUNLOG="$DIR/runlog"
-rm -f $RUNLOG
+rm -f "$RUNLOG"
-touch $RUNLOG
+touch "$RUNLOG"
+chmod g+rw "$RUNLOG"
 HTTP_PORT=9376
 METRIC_GOOD_SYNC_COUNT='git_sync_count_total{status="success"}'
 METRIC_FETCH_COUNT='git_fetch_count_total'
@@ -238,7 +241,7 @@ function GIT_SYNC() {
         ${RM} \
         --label git-sync-e2e="$RUNID" \
         --network="host" \
-        -u $(id -u):$(id -g) \
+        -u git-sync:$(id -g) `# rely on GID, triggering "dubious ownership"` \
         -v "$ROOT":"$ROOT":rw \
         -v "$REPO":"$REPO":ro \
         -v "$REPO2":"$REPO2":ro \
@@ -250,6 +253,7 @@ function GIT_SYNC() {
         e2e/git-sync:"${E2E_TAG}"__$(go env GOOS)_$(go env GOARCH) \
             -v=6 \
             --add-user \
+            --group-write \
             --touch-file="$INTERLOCK" \
             --git-config='protocol.file.allow:always' \
             --http-bind=":$HTTP_PORT" \
@@ -565,6 +569,7 @@ function e2e::worktree_cleanup() {
     # make a worktree to collide with git-sync
     SHA=$(git -C "$REPO" rev-list -n1 HEAD)
     git -C "$REPO" worktree add -q "$ROOT/.worktrees/$SHA" -b e2e --no-checkout
+    chmod g+w "$ROOT/.worktrees/$SHA"
 
     # add some garbage
     mkdir -p "$ROOT/.worktrees/not_a_hash/subdir"
@@ -2828,9 +2833,15 @@ function run_test() {
 }
 
 # Override local configs for predictability in this test.
-export GIT_CONFIG_GLOBAL=/dev/null
+export GIT_CONFIG_GLOBAL="$DIR/gitconfig"
 export GIT_CONFIG_SYSTEM=/dev/null
 
+# Make sure files we create can be group writable.
+umask 0002
+
+# Mark all repos as safe, to avoid "dubious ownership".
+git config --global --add safe.directory '*'
+
 # Iterate over the chosen tests and run them.
 FAILS=()
 FINAL_RET=0