Skip to content

Commit d5367bf

Browse files
authored
Harden updater container isolation (#667)
Run updater containers as the existing non-root user while dropping all Linux capabilities, enabling no-new-privileges, and explicitly selecting Docker's built-in seccomp profile. Reject Docker hosts that do not advertise seccomp support, including correct handling of modern and legacy API security-option formats. Keep local staging capability-free by assigning the updater UID and GID through moby/go-archive, and run required certificate, repository, and case-insensitive setup commands with checked exit handling. Preserve existing repository configuration, CIFS behavior, internal-only updater networking, proxy credential separation, and main updater exit semantics. Roll back case-insensitive storage containers and volumes on initialization failures with bounded cleanup and explicit ownership transfer to Updater on success. Cover container requests, minimum Docker API compatibility, setup failures, cleanup paths, archive ownership, and effective runtime controls for normal, local, case-insensitive, and combined flows. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 090f48f7-de21-4eb1-b076-8c5852c16f14
1 parent a944603 commit d5367bf

8 files changed

Lines changed: 1284 additions & 44 deletions

File tree

‎internal/infra/run.go‎

Lines changed: 97 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,9 @@ import (
1212
"net/http"
1313
"os"
1414
"os/signal"
15+
"path/filepath"
1516
"regexp"
17+
"strconv"
1618
"strings"
1719
"syscall"
1820
"time"
@@ -462,20 +464,24 @@ func runContainers(ctx context.Context, params RunParams) (err error) {
462464
}
463465
}()
464466

467+
if err = configureCaseInsensitiveRepo(ctx, updater, params.Job); err != nil {
468+
return err
469+
}
470+
465471
// put the clone dir in the updater container to be used by during the update
466472
if params.LocalDir != "" {
467473
containerDir := guestRepoDir
468474
if params.Job.UseCaseInsensitiveFileSystem() {
469475
// since the updater is using the storage container, we need to populate the repo on that device because that's the directory that will be used for the update
470476
containerDir = caseSensitiveRepoContentsPath
471477
}
472-
if err = putCloneDir(ctx, cli, updater, params.LocalDir, containerDir); err != nil {
478+
if err = putCloneDir(ctx, cli, updater, updater.containerID, params.LocalDir, containerDir); err != nil {
473479
return err
474480
}
475481
}
476482

477483
// update CA certificates as root prior to start debug shell or running dependabot commands
478-
if err := updater.RunCmd(ctx, "update-ca-certificates", root); err != nil {
484+
if err := updateUpdaterCertificates(ctx, updater); err != nil {
479485
return err
480486
}
481487

@@ -504,6 +510,35 @@ func runContainers(ctx context.Context, params RunParams) (err error) {
504510
return nil
505511
}
506512

513+
func caseInsensitiveRepoSetupCommand(job *model.Job) string {
514+
if !job.UseCaseInsensitiveFileSystem() {
515+
return ""
516+
}
517+
return "git config --global --add safe.directory " + caseSensitiveRepoContentsPath
518+
}
519+
520+
type checkedCommandRunner interface {
521+
runCmdChecked(context.Context, string, string, ...string) error
522+
}
523+
524+
func configureCaseInsensitiveRepo(ctx context.Context, updater checkedCommandRunner, job *model.Job) error {
525+
cmd := caseInsensitiveRepoSetupCommand(job)
526+
if cmd == "" {
527+
return nil
528+
}
529+
if err := updater.runCmdChecked(ctx, cmd, dependabot); err != nil {
530+
return fmt.Errorf("failed to configure case-insensitive repository: %w", err)
531+
}
532+
return nil
533+
}
534+
535+
func updateUpdaterCertificates(ctx context.Context, updater checkedCommandRunner) error {
536+
if err := updater.runCmdChecked(ctx, "update-ca-certificates", root); err != nil {
537+
return fmt.Errorf("failed to update updater certificates: %w", err)
538+
}
539+
return nil
540+
}
541+
507542
func getFromContainer(ctx context.Context, cli *client.Client, containerID, srcPath string) {
508543
reader, _, err := cli.CopyFromContainer(ctx, containerID, srcPath)
509544
if err != nil {
@@ -525,45 +560,88 @@ func getFromContainer(ctx context.Context, cli *client.Client, containerID, srcP
525560
}
526561
}
527562

528-
func putCloneDir(ctx context.Context, cli *client.Client, updater *Updater, localDir, containerDir string) error {
529-
// Docker won't create the directory, so we have to do it first.
530-
cmd := fmt.Sprintf("mkdir -p %s", containerDir)
531-
err := updater.RunCmd(ctx, cmd, dependabot)
563+
type localSetupRunner interface {
564+
checkedCommandRunner
565+
runCmdOutput(context.Context, string, string) (string, error)
566+
}
567+
568+
func putCloneDir(ctx context.Context, cli *client.Client, updater localSetupRunner, containerID, localDir, containerDir string) error {
569+
if err := createLocalStagingDir(ctx, updater, containerDir); err != nil {
570+
return err
571+
}
572+
573+
identity, err := updater.runCmdOutput(ctx, "id -u && id -g", dependabot)
574+
if err != nil {
575+
return fmt.Errorf("failed to resolve updater user identity: %w", err)
576+
}
577+
fields := strings.Fields(identity)
578+
if len(fields) != 2 {
579+
return fmt.Errorf("failed to resolve updater user identity: unexpected output %q", strings.TrimSpace(identity))
580+
}
581+
uid, err := strconv.Atoi(fields[0])
532582
if err != nil {
583+
return fmt.Errorf("failed to resolve updater user UID %q: %w", fields[0], err)
584+
}
585+
gid, err := strconv.Atoi(fields[1])
586+
if err != nil {
587+
return fmt.Errorf("failed to resolve updater user GID %q: %w", fields[1], err)
588+
}
589+
590+
if err := copyLocalDir(ctx, cli, containerID, localDir, containerDir, uid, gid); err != nil {
591+
return err
592+
}
593+
594+
if err := initializeLocalRepository(ctx, updater, containerDir); err != nil {
595+
return err
596+
}
597+
598+
return nil
599+
}
600+
601+
func createLocalStagingDir(ctx context.Context, updater checkedCommandRunner, containerDir string) error {
602+
// Docker won't create the directory, so create it as the updater user first.
603+
if err := updater.runCmdChecked(ctx, "mkdir -p "+containerDir, dependabot); err != nil {
533604
return fmt.Errorf("failed to create clone dir: %w", err)
534605
}
606+
return nil
607+
}
535608

536-
r, err := archive.TarWithOptions(localDir, &archive.TarOptions{})
609+
func copyLocalDir(ctx context.Context, cli *client.Client, containerID, localDir, containerDir string, uid, gid int) error {
610+
localDir, err := filepath.Abs(localDir)
611+
if err != nil {
612+
return fmt.Errorf("failed to resolve local clone dir: %w", err)
613+
}
614+
r, err := archive.TarWithOptions(localDir, &archive.TarOptions{
615+
ChownOpts: &archive.ChownOpts{UID: uid, GID: gid},
616+
})
537617
if err != nil {
538618
return fmt.Errorf("failed to tar clone dir: %w", err)
539619
}
620+
defer r.Close()
540621

541622
opt := container.CopyToContainerOptions{}
542-
err = cli.CopyToContainer(ctx, updater.containerID, containerDir, r, opt)
623+
err = cli.CopyToContainer(ctx, containerID, containerDir, r, opt)
543624
if err != nil {
544625
return fmt.Errorf("failed to copy clone dir to container: %w", err)
545626
}
627+
return nil
628+
}
546629

547-
err = updater.RunCmd(ctx, "chown -R dependabot "+containerDir, root)
548-
if err != nil {
549-
return fmt.Errorf("failed to initialize clone dir: %w", err)
550-
}
551-
552-
// The directory needs to be a git repo, so we need to initialize it.
630+
func initializeLocalRepository(ctx context.Context, updater checkedCommandRunner, containerDir string) error {
553631
commands := []string{
554632
"cd " + containerDir,
555633
"git config --global init.defaultBranch main",
556-
"git init",
634+
// Preserve existing repository settings such as core.filemode, which is
635+
// needed when staging a Windows worktree in a Linux container.
636+
"(git rev-parse --git-dir >/dev/null 2>&1 || git init)",
557637
"git config user.email 'dependabot@github.com'",
558638
"git config user.name 'dependabot'",
559639
"git add .",
560-
"git commit --quiet -m 'Dependabot CLI automated commit'",
640+
"(status=0; git diff --cached --quiet || status=$?; if [ \"$status\" -eq 1 ]; then git commit --quiet -m 'Dependabot CLI automated commit'; elif [ \"$status\" -ne 0 ]; then exit \"$status\"; fi)",
561641
}
562-
err = updater.RunCmd(ctx, strings.Join(commands, " && "), dependabot)
563-
if err != nil {
642+
if err := updater.runCmdChecked(ctx, strings.Join(commands, " && "), dependabot); err != nil {
564643
return fmt.Errorf("failed to initialize clone dir: %w", err)
565644
}
566-
567645
return nil
568646
}
569647

0 commit comments

Comments
 (0)