diff --git a/pkg/daemon/platform/local_server.go b/pkg/daemon/platform/local_server.go index c0b68cfa2..226935e96 100644 --- a/pkg/daemon/platform/local_server.go +++ b/pkg/daemon/platform/local_server.go @@ -180,20 +180,7 @@ func (l *localServer) watchPlatform(stopChan <-chan struct{}) error { ManagementV1(). Selves(). Create(context.Background(), &managementv1.Self{}, metav1.CreateOptions{}) - l.platformStatus.mu.Lock() - if err != nil { - if IsAccessKeyNotFound(err) { - log.Warnf("client not authenticated: %s", err) - l.platformStatus.authenticated = false - } else { - log.Errorf("failed to create self: %v", err) - } - } else { - // We don't want to be too restrictive in case the error - // is transient and doesn't impact existing connections - l.platformStatus.authenticated = true - } - l.platformStatus.mu.Unlock() + l.updatePlatformAuthStatus(err) } select { @@ -204,6 +191,24 @@ func (l *localServer) watchPlatform(stopChan <-chan struct{}) error { } } +// updatePlatformAuthStatus records the platform authentication status from a +// self-creation attempt's result. +func (l *localServer) updatePlatformAuthStatus(err error) { + l.platformStatus.mu.Lock() + defer l.platformStatus.mu.Unlock() + + if err == nil { + l.platformStatus.authenticated = true + return + } + if IsAccessKeyNotFound(err) { + log.Warnf("client not authenticated: %s", err) + l.platformStatus.authenticated = false + return + } + log.Errorf("failed to create self: %v", err) +} + func (l *localServer) health(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusOK) } diff --git a/pkg/devcontainer/delete.go b/pkg/devcontainer/delete.go index 2b1b61adf..8c5ab9993 100644 --- a/pkg/devcontainer/delete.go +++ b/pkg/devcontainer/delete.go @@ -22,25 +22,22 @@ func (r *runner) Delete(ctx context.Context, options DeleteOptions) error { log.Infof("deleting devcontainer: devcontainerID=%s", containerDetails.ID) if isDockerCompose, projectName := getDockerComposeProject(containerDetails); isDockerCompose { - err = r.deleteDockerCompose(ctx, projectName, options.RemoveVolumes) - if err != nil { - return err - } - } else { - if strings.ToLower(containerDetails.State.Status) == "running" { - err = r.driver.StopDevContainer(ctx, r.id) - if err != nil { - return err - } - } + return r.deleteDockerCompose(ctx, projectName, options.RemoveVolumes) + } + return r.stopAndDeleteContainer(ctx, containerDetails) +} - err = r.driver.DeleteDevContainer(ctx, r.id) - if err != nil { +// stopAndDeleteContainer stops containerDetails' devcontainer if it +// is running, then deletes it. +func (r *runner) stopAndDeleteContainer( + ctx context.Context, containerDetails *config.ContainerDetails, +) error { + if strings.ToLower(containerDetails.State.Status) == "running" { + if err := r.driver.StopDevContainer(ctx, r.id); err != nil { return err } } - - return nil + return r.driver.DeleteDevContainer(ctx, r.id) } func (r *runner) cleanupDeliveryVolume(ctx context.Context) { diff --git a/pkg/provider/workspace.go b/pkg/provider/workspace.go index 11a6d73d5..da3ed2d1f 100644 --- a/pkg/provider/workspace.go +++ b/pkg/provider/workspace.go @@ -471,17 +471,7 @@ func (w WorkspaceSource) gitSourceType() string { func ParseWorkspaceSource(source string) *WorkspaceSource { if after, ok := strings.CutPrefix(source, WorkspaceSourceGit); ok { - info := git.NormalizeRepository(after) - if !isPlausibleGitSource(info.Repository) { - return nil - } - return &WorkspaceSource{ - GitRepository: info.Repository, - GitPRReference: info.PR, - GitBranch: info.Branch, - GitCommit: info.Commit, - GitSubPath: info.SubPath, - } + return parseGitWorkspaceSource(after) } else if after, ok := strings.CutPrefix(source, WorkspaceSourceLocal); ok { after = util.ExpandTilde(after) return &WorkspaceSource{ @@ -504,6 +494,23 @@ func ParseWorkspaceSource(source string) *WorkspaceSource { return nil } +// parseGitWorkspaceSource builds a WorkspaceSource from the portion of a +// source string following the git prefix, or nil if it's not a plausible +// git repository. +func parseGitWorkspaceSource(after string) *WorkspaceSource { + info := git.NormalizeRepository(after) + if !isPlausibleGitSource(info.Repository) { + return nil + } + return &WorkspaceSource{ + GitRepository: info.Repository, + GitPRReference: info.PR, + GitBranch: info.Branch, + GitCommit: info.Commit, + GitSubPath: info.SubPath, + } +} + var gitURLSchemes = map[string]bool{"http": true, "https": true, "ssh": true, "git": true} // isPlausibleGitSource returns true when s looks like a git repository diff --git a/pkg/ssh/server/ssh.go b/pkg/ssh/server/ssh.go index 5e55fe6a1..f7b0eab59 100644 --- a/pkg/ssh/server/ssh.go +++ b/pkg/ssh/server/ssh.go @@ -364,43 +364,12 @@ func (s *server) getCommand(sess ssh.Session, isPty bool) *exec.Cmd { user = "" } - // has user set? if user != "" { - args := []string{} - - // is pty? - if isPty { - args = append(args, "-") - } - - // add user - args = append(args, sess.User()) - - // is there a command? - if len(sess.RawCommand()) > 0 { - args = append(args, "-c", sess.RawCommand()) - } - - cmd = exec.Command( - "su", - args...) // #nosec G204 -- args built from session request in the ssh server + //nolint:gosec // G204: args built from session request in the ssh server + cmd = exec.Command("su", buildSuArgs(sess, isPty)...) } else { - args := []string{} - args = append(args, s.shell[1:]...) - if isPty { - args = append(args, "-l") - } - - if len(sess.RawCommand()) == 0 { - cmd = exec.Command( - s.shell[0], - args...) // #nosec G204 -- shell configured by the ssh server, not user input - } else { - args = append(args, "-c", sess.RawCommand()) - cmd = exec.Command( - s.shell[0], - args...) // #nosec G204 -- shell configured by the ssh server, not user input - } + //nolint:gosec // G204: shell configured by the ssh server, not user input + cmd = exec.Command(s.shell[0], buildShellArgs(s.shell[1:], sess, isPty)...) } cmd.Dir = findWorkdir(s.workdir, user) @@ -409,6 +378,32 @@ func (s *server) getCommand(sess ssh.Session, isPty bool) *exec.Cmd { return cmd } +// buildSuArgs builds the "su" argv for running sess's command as sess.User(). +func buildSuArgs(sess ssh.Session, isPty bool) []string { + args := []string{} + if isPty { + args = append(args, "-") + } + args = append(args, sess.User()) + if len(sess.RawCommand()) > 0 { + args = append(args, "-c", sess.RawCommand()) + } + return args +} + +// buildShellArgs builds the argv for running sess's command via the +// server's configured shell, given its args beyond the executable itself. +func buildShellArgs(shellArgs []string, sess ssh.Session, isPty bool) []string { + args := append([]string{}, shellArgs...) + if isPty { + args = append(args, "-l") + } + if len(sess.RawCommand()) > 0 { + args = append(args, "-c", sess.RawCommand()) + } + return args +} + func (s *server) configureAgent(sess ssh.Session, cmd *exec.Cmd) (func(), bool, error) { if s.reuseSock != "" { // openvscode backhaul / explicit shared-socket mode: keep the diff --git a/pkg/workspace/id.go b/pkg/workspace/id.go index cf3db9eaa..e010a4956 100644 --- a/pkg/workspace/id.go +++ b/pkg/workspace/id.go @@ -20,33 +20,9 @@ func ToID(str string) string { str = strings.ToLower(filepath.ToSlash(str)) splitted := strings.Split(str, "@") if len(splitted) == 2 { - // 1. Check if PR was specified - if prReferenceRegEx.MatchString(str) { - str = prReferenceRegEx.ReplaceAllStringFunc(splitted[1], git.GetIDForPR) - } else { - // 2. Check if a branch name has been specified, if so use this for the ID - str = strings.TrimSuffix(splitted[1], ".git") - // Check if branch name matches expected regex - if !branchRegEx.MatchString(str) { - str = splitted[0] - } - } + str = idFromPROrBranch(str, splitted) } else { - // Ensure we don't have a single trailing slash - str = strings.TrimSuffix(str, "/") - // 3. If not, then parse the repo name as ID - index := strings.LastIndex(str, "/") - if index != -1 { - str = str[index+1:] - - // remove a potential tag / branch name - if len(splitted) == 2 && !branchRegEx.MatchString(splitted[1]) { - str = splitted[0] - } - - // remove .git if there is it - str = strings.TrimSuffix(str, ".git") - } + str = idFromRepoPath(str) } str = workspaceIDRegEx2.ReplaceAllString(workspaceIDRegEx1.ReplaceAllString(str, "-"), "") @@ -56,3 +32,29 @@ func ToID(str string) string { return strings.Trim(str, "-") } + +// idFromPROrBranch derives the ID from the "@..." suffix of a "repo@ref" +// string: a PR reference if str matches one, otherwise a branch name if it +// looks like one, falling back to the repo part. +func idFromPROrBranch(str string, splitted []string) string { + if prReferenceRegEx.MatchString(str) { + return prReferenceRegEx.ReplaceAllStringFunc(splitted[1], git.GetIDForPR) + } + branch := strings.TrimSuffix(splitted[1], ".git") + if !branchRegEx.MatchString(branch) { + return splitted[0] + } + return branch +} + +// idFromRepoPath derives the ID from the final path segment of a repo URL +// with no recognized "@ref" suffix, stripping any trailing ".git". +func idFromRepoPath(str string) string { + str = strings.TrimSuffix(str, "/") + index := strings.LastIndex(str, "/") + if index == -1 { + return str + } + str = str[index+1:] + return strings.TrimSuffix(str, ".git") +}