Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

V3: prevent git's 'dubious ownership' error #700

Merged
merged 2 commits into from
Mar 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 20 additions & 4 deletions cmd/git-sync/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import (
"strconv"
"strings"
"sync"
"syscall"
"time"

"github.com/prometheus/client_golang/prometheus"
Expand Down Expand Up @@ -74,6 +75,8 @@ var flOneTime = flag.Bool("one-time", envBool("GIT_SYNC_ONE_TIME", false),
"exit after the first sync")
var flMaxSyncFailures = flag.Int("max-sync-failures", envInt("GIT_SYNC_MAX_SYNC_FAILURES", 0),
"the number of consecutive failures allowed before aborting (the first sync must succeed, -1 will retry forever after the initial sync)")
var flGroupWrite = flag.Bool("group-write", envBool("GIT_SYNC_GROUP_WRITE", false),
"ensure that all data (repo, worktrees, etc.) is group writable")
var flChmod = flag.Int("change-permissions", envInt("GIT_SYNC_PERMISSIONS", 0),
"the file permissions to apply to the checked-out files (0 will not change permissions at all)")
var flSparseCheckoutFile = flag.String("sparse-checkout-file", envString("GIT_SYNC_SPARSE_CHECKOUT_FILE", ""),
Expand Down Expand Up @@ -178,6 +181,8 @@ const (
gcAlways = "always"
gcAggressive = "aggressive"
gcOff = "off"

defaultDirMode = os.FileMode(0775) // subject to umask
)

func init() {
Expand Down Expand Up @@ -379,6 +384,13 @@ func main() {
}
}

// If the user asked for group-writable data, make sure the umask allows it.
if *flGroupWrite {
syscall.Umask(0002)
} else {
syscall.Umask(0022)
}

if *flAddUser {
if err := addUser(); err != nil {
handleError(false, "ERROR: can't write to /etc/passwd: %v", err)
Expand Down Expand Up @@ -684,7 +696,7 @@ func updateSymlink(ctx context.Context, gitRoot, link, newDir string) (string, e
// Make sure the link directory exists. We do this here, rather than at
// startup because it might be under --root and that gets wiped in some
// circumstances.
if err := os.MkdirAll(filepath.Dir(linkDir), os.FileMode(int(0755))); err != nil {
if err := os.MkdirAll(filepath.Dir(linkDir), defaultDirMode); err != nil {
return "", fmt.Errorf("error making symlink dir: %v", err)
}

Expand Down Expand Up @@ -818,8 +830,7 @@ func addWorktreeAndSwap(ctx context.Context, repo, gitRoot, dest, branch, rev st
defer source.Close()

if _, err := os.Stat(gitInfoPath); os.IsNotExist(err) {
fileMode := os.FileMode(int(0755))
err := os.Mkdir(gitInfoPath, fileMode)
err := os.Mkdir(gitInfoPath, defaultDirMode)
if err != nil {
return err
}
Expand Down Expand Up @@ -974,7 +985,7 @@ func cloneRepo(ctx context.Context, repo, branch, rev string, depth int, gitRoot
defer source.Close()

if _, err := os.Stat(gitInfoPath); os.IsNotExist(err) {
fileMode := os.FileMode(int(0755))
fileMode := os.FileMode(defaultDirMode)
err := os.Mkdir(gitInfoPath, fileMode)
if err != nil {
return err
Expand Down Expand Up @@ -1285,7 +1296,12 @@ func setupDefaultGitConfigs(ctx context.Context) error {
// How to manage credentials (for those modes that need it).
key: "credential.helper",
val: "cache --timeout 3600",
}, {
// Our working root is safe (avoid a "dubious ownership" error).
key: "safe.directory",
val: "*",
}}

for _, kv := range configs {
if _, err := cmdRunner.Run(ctx, "", nil, *flGitCmd, "config", "--global", kv.key, kv.val); err != nil {
return fmt.Errorf("error configuring git %q %q: %v", kv.key, kv.val, err)
Expand Down
2 changes: 1 addition & 1 deletion pkg/logging/logging.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,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
Expand Down
7 changes: 6 additions & 1 deletion test_e2e.sh
Original file line number Diff line number Diff line change
Expand Up @@ -139,13 +139,15 @@ ROOT="$DIR/root"
function clean_root() {
rm -rf "$ROOT"
mkdir -p "$ROOT"
chmod g+rwx "$ROOT"
}

# Init SSH for test cases.
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"

SLOW_GIT_CLONE=/slow_git_clone.sh
SLOW_GIT_FETCH=/slow_git_fetch.sh
Expand All @@ -159,6 +161,7 @@ EXECHOOK_ENVVAL=envval
RUNLOG="$DIR/runlog.exechook-fail-retry"
rm -f $RUNLOG
touch $RUNLOG
chmod g+rw $RUNLOG

function GIT_SYNC() {
#./bin/linux_amd64/git-sync "$@"
Expand All @@ -171,7 +174,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 \
Expand All @@ -189,6 +192,7 @@ function GIT_SYNC() {
e2e/git-sync:"${E2E_TAG}"__$(go env GOOS)_$(go env GOARCH) \
--v=6 \
--add-user \
--group-write \
--git-config='protocol.file.allow:always' \
"$@"
}
Expand Down Expand Up @@ -392,6 +396,7 @@ function e2e::worktree_cleanup() {
git -C "$REPO" commit -qam "$FUNCNAME new file"
REV=$(git -C "$REPO" rev-list -n1 HEAD)
git -C "$REPO" worktree add -q "$ROOT"/"$REV" -b e2e --no-checkout
chmod g+w "$ROOT"/"$REV"
sleep 10
assert_file_exists "$ROOT"/link/file2
assert_file_eq "$ROOT"/link/file2 "$FUNCNAME-ok"
Expand Down