fix(guest): run steps with umask 022, as GitHub's runners do (#38)
- commit
- 122e784c35227cd36fcf9c16f401a95a7c03ed34
- author
- brice · · 2026-10-17 18:47 UTC
- committer
- bnema · · 2026-10-17 18:47 UTC
- parent
- 76a5504
- signature
- SSH Unverified
SHA256:VOCA5lYARHcbKDZ7GhTV45VmcjWnJ6ZQIBazj17LivAThe key is unknown - refs
- tag: v0.4.1
3 files · +26 -0
M internal/guest/exec.go +3 -0
| @@ -95,6 +95,9 @@ func (c *ExecConfig) defaults() { | |||
| 95 | 95 | // group with escalating signals (see ExecConfig). | |
| 96 | 96 | func Exec(ctx context.Context, cfg ExecConfig) (int, error) { | |
| 97 | 97 | cfg.defaults() | |
| 98 | + | // The steps inherit the executor's umask: GitHub's 022, whatever started | |
| 99 | + | // the executor (Init sets it too; a container's entrypoint may not). | |
| 100 | + | syscall.Umask(StepUmask) | |
| 98 | 101 | raw, err := cfg.Target.Dial(ctx, cfg.ConnectTimeout) | |
| 99 | 102 | if err != nil { | |
| 100 | 103 | return 1, err | |
| @@ -95,6 +95,9 @@ func (c *ExecConfig) defaults() { | |||||
| 95 | // group with escalating signals (see ExecConfig). | 95 | // group with escalating signals (see ExecConfig). | ||
| 96 | func Exec(ctx context.Context, cfg ExecConfig) (int, error) { | 96 | func Exec(ctx context.Context, cfg ExecConfig) (int, error) { | ||
| 97 | cfg.defaults() | 97 | cfg.defaults() | ||
| 98 | + | // The steps inherit the executor's umask: GitHub's 022, whatever started | |||
| 99 | + | // the executor (Init sets it too; a container's entrypoint may not). | |||
| 100 | + | syscall.Umask(StepUmask) | |||
| 98 | raw, err := cfg.Target.Dial(ctx, cfg.ConnectTimeout) | 101 | raw, err := cfg.Target.Dial(ctx, cfg.ConnectTimeout) | ||
| 99 | if err != nil { | 102 | if err != nil { | ||
| 100 | return 1, err | 103 | return 1, err | ||
M internal/guest/guest_test.go +15 -0
| @@ -675,3 +675,18 @@ func TestStopDockerdKillsAndReapsATermIgnoringDaemon(t *testing.T) { | |||
| 675 | 675 | t.Fatal("dockerd was not reaped (zombie or alive)") | |
| 676 | 676 | } | |
| 677 | 677 | } | |
| 678 | + | ||
| 679 | + | // A step runs with GitHub's umask 022 even when the executor was started with | |
| 680 | + | // 0, as the kernel starts a VM's init: its files and directories are not | |
| 681 | + | // world-writable. | |
| 682 | + | func TestExecStepsGetUmask022(t *testing.T) { | |
| 683 | + | old := syscall.Umask(0) | |
| 684 | + | defer syscall.Umask(old) | |
| 685 | + | out, _, _, _, code, err := host(t, `umask`, 0, nil) | |
| 686 | + | if err != nil || code != 0 { | |
| 687 | + | t.Fatalf("code=%d err=%v", code, err) | |
| 688 | + | } | |
| 689 | + | if strings.TrimSpace(out) != "0022" { | |
| 690 | + | t.Fatalf("umask = %q, want 0022", out) | |
| 691 | + | } | |
| 692 | + | } | |
| @@ -675,3 +675,18 @@ func TestStopDockerdKillsAndReapsATermIgnoringDaemon(t *testing.T) { | |||||
| 675 | t.Fatal("dockerd was not reaped (zombie or alive)") | 675 | t.Fatal("dockerd was not reaped (zombie or alive)") | ||
| 676 | } | 676 | } | ||
| 677 | } | 677 | } | ||
| 678 | + | ||||
| 679 | + | // A step runs with GitHub's umask 022 even when the executor was started with | |||
| 680 | + | // 0, as the kernel starts a VM's init: its files and directories are not | |||
| 681 | + | // world-writable. | |||
| 682 | + | func TestExecStepsGetUmask022(t *testing.T) { | |||
| 683 | + | old := syscall.Umask(0) | |||
| 684 | + | defer syscall.Umask(old) | |||
| 685 | + | out, _, _, _, code, err := host(t, `umask`, 0, nil) | |||
| 686 | + | if err != nil || code != 0 { | |||
| 687 | + | t.Fatalf("code=%d err=%v", code, err) | |||
| 688 | + | } | |||
| 689 | + | if strings.TrimSpace(out) != "0022" { | |||
| 690 | + | t.Fatalf("umask = %q, want 0022", out) | |||
| 691 | + | } | |||
| 692 | + | } | |||
M internal/guest/init.go +8 -0
| @@ -63,6 +63,10 @@ type InitConfig struct { | |||
| 63 | 63 | // powering the VM off, or exiting with the executor's code in a container. | |
| 64 | 64 | func Init(ctx context.Context, cfg InitConfig) int { | |
| 65 | 65 | log := zerowrap.FromCtx(ctx) | |
| 66 | + | // The kernel starts a VM's init with umask 0, which dockerd, the executor | |
| 67 | + | // and every step would inherit: files and t.TempDir() directories would be | |
| 68 | + | // world-writable. GitHub's runners have 022, and so do the jobs here. | |
| 69 | + | syscall.Umask(StepUmask) | |
| 66 | 70 | if cfg.Getpid == nil { | |
| 67 | 71 | cfg.Getpid = os.Getpid | |
| 68 | 72 | } | |
| @@ -102,6 +106,10 @@ func Init(ctx context.Context, cfg InitConfig) int { | |||
| 102 | 106 | return code | |
| 103 | 107 | } | |
| 104 | 108 | ||
| 109 | + | // StepUmask is the umask of everything the init starts, steps included: the | |
| 110 | + | // one of GitHub's hosted runners. | |
| 111 | + | const StepUmask = 0o022 | |
| 112 | + | ||
| 105 | 113 | // defaultPath is the PATH of a sandbox whose init was started by the kernel | |
| 106 | 114 | // with an empty environment. | |
| 107 | 115 | const defaultPath = "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin" | |
| @@ -63,6 +63,10 @@ type InitConfig struct { | |||||
| 63 | // powering the VM off, or exiting with the executor's code in a container. | 63 | // powering the VM off, or exiting with the executor's code in a container. | ||
| 64 | func Init(ctx context.Context, cfg InitConfig) int { | 64 | func Init(ctx context.Context, cfg InitConfig) int { | ||
| 65 | log := zerowrap.FromCtx(ctx) | 65 | log := zerowrap.FromCtx(ctx) | ||
| 66 | + | // The kernel starts a VM's init with umask 0, which dockerd, the executor | |||
| 67 | + | // and every step would inherit: files and t.TempDir() directories would be | |||
| 68 | + | // world-writable. GitHub's runners have 022, and so do the jobs here. | |||
| 69 | + | syscall.Umask(StepUmask) | |||
| 66 | if cfg.Getpid == nil { | 70 | if cfg.Getpid == nil { | ||
| 67 | cfg.Getpid = os.Getpid | 71 | cfg.Getpid = os.Getpid | ||
| 68 | } | 72 | } | ||
| @@ -102,6 +106,10 @@ func Init(ctx context.Context, cfg InitConfig) int { | |||||
| 102 | return code | 106 | return code | ||
| 103 | } | 107 | } | ||
| 104 | 108 | ||||
| 109 | + | // StepUmask is the umask of everything the init starts, steps included: the | |||
| 110 | + | // one of GitHub's hosted runners. | |||
| 111 | + | const StepUmask = 0o022 | |||
| 112 | + | ||||
| 105 | // defaultPath is the PATH of a sandbox whose init was started by the kernel | 113 | // defaultPath is the PATH of a sandbox whose init was started by the kernel | ||
| 106 | // with an empty environment. | 114 | // with an empty environment. | ||
| 107 | const defaultPath = "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin" | 115 | const defaultPath = "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin" | ||