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

[pvc] run git commands as gitpod user when using pvc #13929

Merged
merged 1 commit into from
Oct 24, 2022
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
10 changes: 9 additions & 1 deletion components/content-service/pkg/git/git.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,9 @@ type Client struct {

// UpstreamCloneURI is the fork upstream of a repository
UpstreamRemoteURI string

// if true will run git command as gitpod user (should be executed as root that has access to sudo in this case)
RunAsGitpodUser bool
}

// Status describes the status of a Git repo/working copy akin to "git status"
Expand Down Expand Up @@ -197,7 +200,12 @@ func (c *Client) GitWithOutput(ctx context.Context, ignoreErr *string, subcomman

span.LogKV("args", fullArgs)

cmd := exec.Command("git", fullArgs...)
cmdName := "git"
if c.RunAsGitpodUser {
cmdName = "sudo"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sudo is required?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah. that is the whole point here, and assumes that this runs as root (which it is from ring0).
so it runs sudo -u gitpod git ...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At this code phase, is the permission root?

If yes, I am thinking using runuser is more suitable or not. 🤔
https://prasadlinuxblog.wordpress.com/2012/09/04/392/

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it safe from a security viewpoint?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jenting I am not sure if there is any meaningful difference in our case for sudo vs runuser. WDYT?

@utam0k I don't think so. That code was already running as root (supervisor) prior to this changes.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree with you.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just want to confirm with you about it. 💯

fullArgs = append([]string{"-u", "gitpod", "git"}, fullArgs...)
}
cmd := exec.Command(cmdName, fullArgs...)
cmd.Dir = c.Location
cmd.Env = env

Expand Down
17 changes: 17 additions & 0 deletions components/content-service/pkg/initializer/git.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,23 @@ func (ws *GitInitializer) Run(ctx context.Context, mappings []archive.IDMapping)
return err
}

// make sure that folder itself is owned by gitpod user prior to doing git clone
// this is needed as otherwise git clone will fail if the folder is owned by root
if ws.RunAsGitpodUser {
args := []string{"gitpod", ws.Location}
cmd := exec.Command("chown", args...)
res, cerr := cmd.CombinedOutput()
if cerr != nil && !process.IsNotChildProcess(cerr) {
err = git.OpFailedError{
Args: args,
ExecErr: cerr,
Output: string(res),
Subcommand: "chown",
}
return err
}
}

log.WithField("stage", "init").WithField("location", ws.Location).Debug("Running git clone on workspace")
err = ws.Clone(ctx)
if err != nil {
Expand Down
3 changes: 2 additions & 1 deletion components/content-service/pkg/initializer/initializer.go
Original file line number Diff line number Diff line change
Expand Up @@ -261,10 +261,11 @@ func newGitInitializer(ctx context.Context, loc string, req *csapi.GitInitialize
Config: req.Config.CustomConfig,
AuthMethod: authMethod,
AuthProvider: authProvider,
RunAsGitpodUser: forceGitpodUser,
},
TargetMode: targetMode,
CloneTarget: req.CloneTaget,
Chown: forceGitpodUser,
Chown: false,
}, nil
}

Expand Down