Skip to content

Commit b59e6de

Browse files
ChristopherHXRcplee
authored
feat: non root user container (#1202)
* feat: non root user container * Also chown WorkingDir * . * . * Update docker_run.go * Add Test * Update runner_test.go * Update docker_run.go * Apply suggestions from code review Co-authored-by: R <[email protected]> * remove cruft from master merge Co-authored-by: R <[email protected]> Co-authored-by: Casey Lee <[email protected]> Co-authored-by: Casey Lee <[email protected]>
1 parent c30bc82 commit b59e6de

File tree

4 files changed

+71
-6
lines changed

4 files changed

+71
-6
lines changed

pkg/container/docker_run.go

+62-2
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"path/filepath"
1313
"regexp"
1414
"runtime"
15+
"strconv"
1516
"strings"
1617

1718
"github.com/go-git/go-billy/v5/helper/polyfill"
@@ -125,6 +126,13 @@ func (cr *containerReference) Start(attach bool) common.Executor {
125126
cr.attach().IfBool(attach),
126127
cr.start(),
127128
cr.wait().IfBool(attach),
129+
cr.tryReadUID(),
130+
cr.tryReadGID(),
131+
func(ctx context.Context) error {
132+
// If this fails, then folders have wrong permissions on non root container
133+
_ = cr.Exec([]string{"chown", "-R", fmt.Sprintf("%d:%d", cr.UID, cr.GID), cr.input.WorkingDir}, nil, "0", "")(ctx)
134+
return nil
135+
},
128136
).IfNot(common.Dryrun),
129137
)
130138
}
@@ -154,8 +162,12 @@ func (cr *containerReference) Copy(destPath string, files ...*FileEntry) common.
154162
func (cr *containerReference) CopyDir(destPath string, srcPath string, useGitIgnore bool) common.Executor {
155163
return common.NewPipelineExecutor(
156164
common.NewInfoExecutor("%sdocker cp src=%s dst=%s", logPrefix, srcPath, destPath),
157-
cr.Exec([]string{"mkdir", "-p", destPath}, nil, "", ""),
158165
cr.copyDir(destPath, srcPath, useGitIgnore),
166+
func(ctx context.Context) error {
167+
// If this fails, then folders have wrong permissions on non root container
168+
_ = cr.Exec([]string{"chown", "-R", fmt.Sprintf("%d:%d", cr.UID, cr.GID), destPath}, nil, "0", "")(ctx)
169+
return nil
170+
},
159171
).IfNot(common.Dryrun)
160172
}
161173

@@ -211,6 +223,8 @@ type containerReference struct {
211223
cli client.APIClient
212224
id string
213225
input *NewContainerInput
226+
UID int
227+
GID int
214228
}
215229

216230
func GetDockerClient(ctx context.Context) (cli client.APIClient, err error) {
@@ -581,6 +595,47 @@ func (cr *containerReference) exec(cmd []string, env map[string]string, user, wo
581595
}
582596
}
583597

598+
func (cr *containerReference) tryReadID(opt string, cbk func(id int)) common.Executor {
599+
return func(ctx context.Context) error {
600+
idResp, err := cr.cli.ContainerExecCreate(ctx, cr.id, types.ExecConfig{
601+
Cmd: []string{"id", opt},
602+
AttachStdout: true,
603+
AttachStderr: true,
604+
})
605+
if err != nil {
606+
return nil
607+
}
608+
609+
resp, err := cr.cli.ContainerExecAttach(ctx, idResp.ID, types.ExecStartCheck{})
610+
if err != nil {
611+
return nil
612+
}
613+
defer resp.Close()
614+
615+
sid, err := resp.Reader.ReadString('\n')
616+
if err != nil {
617+
return nil
618+
}
619+
exp := regexp.MustCompile(`\d+\n`)
620+
found := exp.FindString(sid)
621+
id, err := strconv.ParseInt(found[:len(found)-1], 10, 32)
622+
if err != nil {
623+
return nil
624+
}
625+
cbk(int(id))
626+
627+
return nil
628+
}
629+
}
630+
631+
func (cr *containerReference) tryReadUID() common.Executor {
632+
return cr.tryReadID("-u", func(id int) { cr.UID = id })
633+
}
634+
635+
func (cr *containerReference) tryReadGID() common.Executor {
636+
return cr.tryReadID("-g", func(id int) { cr.GID = id })
637+
}
638+
584639
func (cr *containerReference) waitForCommand(ctx context.Context, isTerminal bool, resp types.HijackedResponse, idResp types.IDResponse, user string, workdir string) error {
585640
logger := common.Logger(ctx)
586641

@@ -670,6 +725,9 @@ func (cr *containerReference) copyDir(dstPath string, srcPath string, useGitIgno
670725
SrcPrefix: srcPrefix,
671726
Handler: &tarCollector{
672727
TarWriter: tw,
728+
UID: cr.UID,
729+
GID: cr.GID,
730+
DstDir: dstPath[1:],
673731
},
674732
}
675733

@@ -686,7 +744,7 @@ func (cr *containerReference) copyDir(dstPath string, srcPath string, useGitIgno
686744
if err != nil {
687745
return fmt.Errorf("failed to seek tar archive: %w", err)
688746
}
689-
err = cr.cli.CopyToContainer(ctx, cr.id, dstPath, tarFile, types.CopyToContainerOptions{})
747+
err = cr.cli.CopyToContainer(ctx, cr.id, "/", tarFile, types.CopyToContainerOptions{})
690748
if err != nil {
691749
return fmt.Errorf("failed to copy content to container: %w", err)
692750
}
@@ -705,6 +763,8 @@ func (cr *containerReference) copyContent(dstPath string, files ...*FileEntry) c
705763
Name: file.Name,
706764
Mode: file.Mode,
707765
Size: int64(len(file.Body)),
766+
Uid: cr.UID,
767+
Gid: cr.GID,
708768
}
709769
if err := tw.WriteHeader(hdr); err != nil {
710770
return err

pkg/container/file_collector.go

+8-3
Original file line numberDiff line numberDiff line change
@@ -23,19 +23,24 @@ type fileCollectorHandler interface {
2323

2424
type tarCollector struct {
2525
TarWriter *tar.Writer
26+
UID int
27+
GID int
28+
DstDir string
2629
}
2730

28-
func (tc tarCollector) WriteFile(path string, fi fs.FileInfo, linkName string, f io.Reader) error {
31+
func (tc tarCollector) WriteFile(fpath string, fi fs.FileInfo, linkName string, f io.Reader) error {
2932
// create a new dir/file header
3033
header, err := tar.FileInfoHeader(fi, linkName)
3134
if err != nil {
3235
return err
3336
}
3437

3538
// update the name to correctly reflect the desired destination when untaring
36-
header.Name = path
39+
header.Name = path.Join(tc.DstDir, fpath)
3740
header.Mode = int64(fi.Mode())
3841
header.ModTime = fi.ModTime()
42+
header.Uid = tc.UID
43+
header.Gid = tc.GID
3944

4045
// write the header
4146
if err := tc.TarWriter.WriteHeader(header); err != nil {
@@ -138,7 +143,7 @@ func (fc *fileCollector) collectFiles(ctx context.Context, submodulePath []strin
138143
}
139144
}
140145
if err == nil && entry.Mode == filemode.Submodule {
141-
err = filepath.Walk(fi.Name(), fc.collectFiles(ctx, split))
146+
err = fc.Fs.Walk(fi.Name(), fc.collectFiles(ctx, split))
142147
if err != nil {
143148
return err
144149
}

pkg/runner/run_context.go

-1
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,6 @@ func (rc *RunContext) startJobContainer() common.Executor {
180180
rc.JobContainer.Start(false),
181181
rc.JobContainer.UpdateFromImageEnv(&rc.Env),
182182
rc.JobContainer.UpdateFromEnv("/etc/environment", &rc.Env),
183-
rc.JobContainer.Exec([]string{"mkdir", "-m", "0777", "-p", ActPath}, rc.Env, "root", ""),
184183
rc.JobContainer.Copy(ActPath+"/", &container.FileEntry{
185184
Name: "workflow/event.json",
186185
Mode: 0644,

pkg/runner/runner_test.go

+1
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,7 @@ func TestRunEvent(t *testing.T) {
158158
{workdir, "container-hostname", "push", "", platforms},
159159
{workdir, "remote-action-docker", "push", "", platforms},
160160
{workdir, "remote-action-js", "push", "", platforms},
161+
{workdir, "remote-action-js", "push", "", map[string]string{"ubuntu-latest": "ghcr.io/catthehacker/ubuntu:runner-latest"}}, // Test if this works with non root container
161162
{workdir, "matrix", "push", "", platforms},
162163
{workdir, "matrix-include-exclude", "push", "", platforms},
163164
{workdir, "commands", "push", "", platforms},

0 commit comments

Comments
 (0)