diff --git a/pkg/container/docker_run.go b/pkg/container/docker_run.go index 682fc39..47d33d7 100644 --- a/pkg/container/docker_run.go +++ b/pkg/container/docker_run.go @@ -1006,27 +1006,78 @@ func (cr *containerReference) wait() common.Executor { } // For Gitea +// volumeRule is one compiled valid_volumes entry. An entry ending in `:ro` +// allows the source but forces the mount read-only; an unmarked entry allows +// the source and preserves the requested mode. +type volumeRule struct { + pattern glob.Glob + readOnly bool +} + +func compileVolumeRules(ctx context.Context, validVolumes []string) []volumeRule { + logger := common.Logger(ctx) + + rules := make([]volumeRule, 0, len(validVolumes)) + for _, v := range validVolumes { + source, readOnly := strings.CutSuffix(v, ":ro") + if g, err := glob.Compile(source); err != nil { + logger.Errorf("create glob from %s error: %v", source, err) + } else { + rules = append(rules, volumeRule{pattern: g, readOnly: readOnly}) + } + } + return rules +} + +// matchVolumeRules reports whether a mount source is allowed and whether it +// must be read-only. Rules are checked in configuration order and the last +// matching rule wins. +func matchVolumeRules(rules []volumeRule, source string) (allowed, readOnly bool) { + for _, r := range rules { + if r.pattern.Match(source) { + allowed = true + readOnly = r.readOnly + } + } + return allowed, readOnly +} + +// forceBindReadOnly rewrites a parsed bind spec's mode to `ro`, keeping other +// options such as SELinux labels and propagation modifiers. +func forceBindReadOnly(bind, source, target string) string { + base := source + ":" + target + if bind == base { + return bind + ":ro" + } + + rawOptions, hasOptions := strings.CutPrefix(bind, base+":") + if !hasOptions { + return bind + ":ro" + } + opts := strings.Split(rawOptions, ",") + sanitized := make([]string, 0, len(opts)+1) + hasMode := false + for _, o := range opts { + if o == "rw" { + o = "ro" + } + if o == "ro" { + hasMode = true + } + sanitized = append(sanitized, o) + } + if !hasMode { + sanitized = append(sanitized, "ro") + } + return base + ":" + strings.Join(sanitized, ",") +} + // sanitizeConfig remove the invalid configurations from `config` and `hostConfig` func (cr *containerReference) sanitizeConfig(ctx context.Context, config *container.Config, hostConfig *container.HostConfig) (*container.Config, *container.HostConfig) { logger := common.Logger(ctx) if len(cr.input.ValidVolumes) > 0 { - globs := make([]glob.Glob, 0, len(cr.input.ValidVolumes)) - for _, v := range cr.input.ValidVolumes { - if g, err := glob.Compile(v); err != nil { - logger.Errorf("create glob from %s error: %v", v, err) - } else { - globs = append(globs, g) - } - } - isValid := func(v string) bool { - for _, g := range globs { - if g.Match(v) { - return true - } - } - return false - } + rules := compileVolumeRules(ctx, cr.input.ValidVolumes) // sanitize binds sanitizedBinds := make([]string, 0, len(hostConfig.Binds)) for _, bind := range hostConfig.Binds { @@ -1040,21 +1091,29 @@ func (cr *containerReference) sanitizeConfig(ctx context.Context, config *contai sanitizedBinds = append(sanitizedBinds, bind) continue } - if isValid(parsed.Source) { - sanitizedBinds = append(sanitizedBinds, bind) - } else { + allowed, readOnly := matchVolumeRules(rules, parsed.Source) + if !allowed { logger.Warnf("[%s] is not a valid volume, will be ignored", parsed.Source) + continue } + if readOnly && !parsed.ReadOnly { + bind = forceBindReadOnly(bind, parsed.Source, parsed.Target) + } + sanitizedBinds = append(sanitizedBinds, bind) } hostConfig.Binds = sanitizedBinds // sanitize mounts sanitizedMounts := make([]mount.Mount, 0, len(hostConfig.Mounts)) for _, mt := range hostConfig.Mounts { - if isValid(mt.Source) { - sanitizedMounts = append(sanitizedMounts, mt) - } else { + allowed, readOnly := matchVolumeRules(rules, mt.Source) + if !allowed { logger.Warnf("[%s] is not a valid volume, will be ignored", mt.Source) + continue + } + if readOnly { + mt.ReadOnly = true } + sanitizedMounts = append(sanitizedMounts, mt) } hostConfig.Mounts = sanitizedMounts } else { diff --git a/pkg/container/docker_run_test.go b/pkg/container/docker_run_test.go index 4b4b700..a449b10 100644 --- a/pkg/container/docker_run_test.go +++ b/pkg/container/docker_run_test.go @@ -258,10 +258,12 @@ var _ ExecutionsEnvironment = &containerReference{} func TestCheckVolumes(t *testing.T) { testCases := []struct { - desc string - validVolumes []string - binds []string - expectedBinds []string + desc string + validVolumes []string + binds []string + mounts []mount.Mount + expectedBinds []string + expectedMounts []mount.Mount }{ { desc: "match all volumes", @@ -313,6 +315,97 @@ func TestCheckVolumes(t *testing.T) { "/etc/conf.d/base.json:/config/base.json", }, }, + { + desc: "read-only entry forces a bind read-only", + validVolumes: []string{"/etc/ssl/certs/ca-certificates.crt:ro"}, + binds: []string{"/etc/ssl/certs/ca-certificates.crt:/etc/ssl/certs/ca-certificates.crt"}, + expectedBinds: []string{"/etc/ssl/certs/ca-certificates.crt:/etc/ssl/certs/ca-certificates.crt:ro"}, + }, + { + desc: "read-only entry rewrites a requested rw mode", + validVolumes: []string{"/host/toolcache:ro"}, + binds: []string{"/host/toolcache:/opt/hostedtoolcache:rw"}, + expectedBinds: []string{"/host/toolcache:/opt/hostedtoolcache:ro"}, + }, + { + desc: "unmarked entry preserves the requested rw mode", + validVolumes: []string{"/home/test/data"}, + binds: []string{"/home/test/data:/test_data:rw"}, + expectedBinds: []string{"/home/test/data:/test_data:rw"}, + }, + { + desc: "already read-only bind stays read-only", + validVolumes: []string{"/home/test/data:ro"}, + binds: []string{"/home/test/data:/test_data:ro"}, + expectedBinds: []string{"/home/test/data:/test_data:ro"}, + }, + { + desc: "read-only entry keeps other bind modifiers", + validVolumes: []string{"/home/test/data:ro"}, + binds: []string{"/home/test/data:/test_data:z"}, + expectedBinds: []string{"/home/test/data:/test_data:z,ro"}, + }, + { + desc: "read-only entry handles Windows drive paths without a mode", + validVolumes: []string{"C:/host/data:ro"}, + binds: []string{"C:/host/data:D:/container/data"}, + expectedBinds: []string{"C:/host/data:D:/container/data:ro"}, + }, + { + desc: "non-matching bind is dropped when read-only rules exist", + validVolumes: []string{"/home/test/data:ro"}, + binds: []string{"/secrets/keys:/keys"}, + expectedBinds: []string{}, + }, + { + desc: "last matching rule wins for overlapping entries", + validVolumes: []string{ + "/data/**", + "/data/cache:ro", + }, + binds: []string{ + "/data/cache:/cache", + "/data/tool:/tool", + }, + expectedBinds: []string{ + "/data/cache:/cache:ro", + "/data/tool:/tool", + }, + }, + { + desc: "internally appended exact entry stays writable", + validVolumes: []string{ + "/**:ro", + "/var/run/docker.sock", + }, + binds: []string{ + "/var/run/docker.sock:/var/run/docker.sock", + "/etc/passwd:/pw", + }, + expectedBinds: []string{ + "/var/run/docker.sock:/var/run/docker.sock", + "/etc/passwd:/pw:ro", + }, + }, + { + desc: "read-only entry forces a long-form mount read-only", + validVolumes: []string{"/etc/ssl/certs/ca-certificates.crt:ro"}, + mounts: []mount.Mount{ + { + Type: mount.TypeBind, + Source: "/etc/ssl/certs/ca-certificates.crt", + Target: "/etc/ssl/certs/ca-certificates.crt", + }, + }, + expectedMounts: []mount.Mount{ + { + Type: mount.TypeBind, + Source: "/etc/ssl/certs/ca-certificates.crt", + Target: "/etc/ssl/certs/ca-certificates.crt", + ReadOnly: true, + }, + }, + }, } for _, tc := range testCases { t.Run(tc.desc, func(t *testing.T) { @@ -323,8 +416,15 @@ func TestCheckVolumes(t *testing.T) { ValidVolumes: tc.validVolumes, }, } - _, hostConf := cr.sanitizeConfig(ctx, &container.Config{}, &container.HostConfig{Binds: tc.binds}) + _, hostConf := cr.sanitizeConfig(ctx, &container.Config{}, &container.HostConfig{Binds: tc.binds, Mounts: tc.mounts}) + if tc.expectedBinds == nil { + tc.expectedBinds = []string{} + } + if tc.expectedMounts == nil { + tc.expectedMounts = []mount.Mount{} + } assert.Equal(t, tc.expectedBinds, hostConf.Binds) + assert.Equal(t, tc.expectedMounts, hostConf.Mounts) }) } }