Repository: gogs/gogs
File: internal/database/repo_editor.go (Go)

Code before:

		// Otherwise, delete branch from local copy in case out of sync
		if git.RepoHasBranch(localPath, opts.NewBranch) {
			if err = git.DeleteBranch(localPath, opts.NewBranch, git.DeleteBranchOptions{
				Force: true,
			}); err != nil {
				return fmt.Errorf("delete branch %q: %v", opts.NewBranch, err)
			}
		}

		if err := r.CheckoutNewBranch(opts.OldBranch, opts.NewBranch); err != nil {
			return fmt.Errorf("checkout new branch[%s] from old branch[%s]: %v", opts.NewBranch, opts.OldBranch, err)
		}
	}

	oldFilePath := path.Join(localPath, opts.OldTreeName)
	filePath := path.Join(localPath, opts.NewTreeName)
	if err = os.MkdirAll(path.Dir(filePath), os.ModePerm); err != nil {
		return err
	}

	// If it's meant to be a new file, make sure it doesn't exist.
	if opts.IsNewFile {
		if com.IsExist(filePath) {
			return ErrRepoFileAlreadyExist{filePath}
		}
	} else {
		// 🚨 SECURITY: Prevent updating files in surprising place, check if the file is
		// a symlink.
		if osutil.IsSymlink(filePath) {
			return fmt.Errorf("cannot update symbolic link: %s", opts.NewTreeName)
		}
	}

	// Ignore move step if it's a new file under a directory.
	// Otherwise, move the file when name changed.
	if osutil.IsFile(oldFilePath) && opts.OldTreeName != opts.NewTreeName {
		// 🚨 SECURITY: Prevent updating files in surprising place, check if the file is
		// a symlink.
		if osutil.IsSymlink(oldFilePath) {
			return fmt.Errorf("cannot move symbolic link: %s", opts.OldTreeName)
		}

		if err = git.Move(localPath, opts.OldTreeName, opts.NewTreeName); err != nil {
			return fmt.Errorf("git mv %q %q: %v", opts.OldTreeName, opts.NewTreeName, err)
		}
	}

	if err = os.WriteFile(filePath, []byte(opts.Content), 0600); err != nil {
		return fmt.Errorf("write file: %v", err)
	}

Review comment (bug):
Reorder the symlink check to occur before the existence check in UpdateRepoFile, since Stat follows symlinks and will mark them as existing files. This prevents misclassifying a symlink as an existing file and returning the wrong error.

Code after:

		// Otherwise, delete branch from local copy in case out of sync
		if git.RepoHasBranch(localPath, opts.NewBranch) {
			if err = git.DeleteBranch(localPath, opts.NewBranch, git.DeleteBranchOptions{
				Force: true,
			}); err != nil {
				return fmt.Errorf("delete branch %q: %v", opts.NewBranch, err)
			}
		}

		if err := r.CheckoutNewBranch(opts.OldBranch, opts.NewBranch); err != nil {
			return fmt.Errorf("checkout new branch[%s] from old branch[%s]: %v", opts.NewBranch, opts.OldBranch, err)
		}
	}

	oldFilePath := path.Join(localPath, opts.OldTreeName)
	filePath := path.Join(localPath, opts.NewTreeName)
	if err = os.MkdirAll(path.Dir(filePath), os.ModePerm); err != nil {
		return err
	}

	// If it's meant to be a new file, make sure it doesn't exist.
	if opts.IsNewFile {
		// 🚨 SECURITY: Prevent updating files in surprising place, check if the file is
		// a symlink.
		if osutil.IsSymlink(filePath) {
			return fmt.Errorf("cannot update symbolic link: %s", opts.NewTreeName)
		}
		if osutil.IsExist(filePath) {
			return ErrRepoFileAlreadyExist{filePath}
		}
	}

	// Ignore move step if it's a new file under a directory.
	// Otherwise, move the file when name changed.
	if osutil.IsFile(oldFilePath) && opts.OldTreeName != opts.NewTreeName {
		// 🚨 SECURITY: Prevent updating files in surprising place, check if the file is
		// a symlink.
		if osutil.IsSymlink(oldFilePath) {
			return fmt.Errorf("cannot move symbolic link: %s", opts.OldTreeName)
		}

		if err = git.Move(localPath, opts.OldTreeName, opts.NewTreeName); err != nil {
			return fmt.Errorf("git mv %q %q: %v", opts.OldTreeName, opts.NewTreeName, err)
		}
	}

	if err = os.WriteFile(filePath, []byte(opts.Content), 0600); err != nil {
		return fmt.Errorf("write file: %v", err)
	}