From 258d46ec88fa3790a5f90b68fcee599fd643d858 Mon Sep 17 00:00:00 2001 From: bilby91 Date: Wed, 24 Jun 2026 18:39:12 -0300 Subject: [PATCH 1/2] compose: apply feature security metadata + entrypoint chaining to services MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Dev Container Features declare security options (privileged, init, capAdd, securityOpt) and entrypoint scripts in their metadata. For image/Dockerfile devcontainers these become `docker run` flags; the reference devcontainers/cli also applies them to docker-compose services via its generated override compose file. Our compose path merged the metadata into ResolvedConfig but never carried it onto the service, so features like docker-in-docker silently failed on compose-source devcontainers: the daemon came up unprivileged and its docker-init.sh entrypoint never ran, so `docker` reported the daemon down. Security options: - Extend compose.Override with Privileged/Init/CapAdd/SecurityOpt; build it once from cfg in createFreshCompose and apply on both backends (ApplyRunOverride for native, WriteRunOverride YAML for shellout). - serviceToRunSpec now forwards Privileged/SecurityOpt to RunSpec (Init/CapAdd were already mapped). Entrypoint chaining: - Aggregate the metadata entrypoint chain into ResolvedConfig.Entrypoints (base-image label entries first, then features; idempotent). - RenderEntrypointWrapper builds the `/bin/sh -c '; exec "$@"'` wrapper that runs feature entrypoints before the original entrypoint+command. Native path emits a single `$`; shellout path doubles to `$$` because `docker compose` re-interpolates the written YAML. - Add ImageDetails.Entrypoint (+ docker InspectImage) so a non-null image ENTRYPOINT is preserved underneath the wrapper. Inspect contract: ContainerDetails gains Privileged/CapAdd/SecurityOpt (read from HostConfig in docker InspectContainer, reused by podman) so the new integration test can verify what landed on the real container. Applecontainer can't surface these (VM isolation) — left at zero values. Tested: unit coverage for the wrapper rendering, override apply/emit, and metadata aggregation; integration test brings up a compose devcontainer whose feature declares privileged/init/capAdd/securityOpt + an entrypoint and asserts all of them land on the real container — green on both native and shellout backends. Image-source (non-compose) entrypoint chaining is left as a follow-up. Co-Authored-By: Claude Opus 4.8 (1M context) --- compose/apply_override.go | 24 +++ compose/apply_override_test.go | 97 +++++++++ compose/orchestrator.go | 14 +- compose/orchestrator_test.go | 26 +++ compose/override.go | 115 ++++++++++- compose/override_test.go | 135 +++++++++++++ config/merge_metadata.go | 13 ++ config/merge_metadata_test.go | 23 +++ config/resolved.go | 8 + .../applecontainer/inspect_darwin_arm64.go | 5 + runtime/docker/inspect.go | 6 + runtime/runtime.go | 17 ++ .../compose_feature_security_test.go | 190 ++++++++++++++++++ up.go | 60 ++++-- 14 files changed, 706 insertions(+), 27 deletions(-) create mode 100644 test/integration/compose_feature_security_test.go diff --git a/compose/apply_override.go b/compose/apply_override.go index 59a76a6..f201d1e 100644 --- a/compose/apply_override.go +++ b/compose/apply_override.go @@ -107,6 +107,30 @@ func ApplyRunOverride(project *composetypes.Project, primaryService string, ov O } } + // Security/entrypoint options from merged feature+image metadata. + // Pointers: nil means "leave the service's own value untouched" so we + // never downgrade a user's `privileged: true` when no feature asked + // for it. Slices: union (dedup) with the service's existing entries. + if ov.Privileged != nil { + svc.Privileged = *ov.Privileged + } + if ov.Init != nil { + svc.Init = ov.Init + } + svc.CapAdd = unionStrings(svc.CapAdd, ov.CapAdd) + svc.SecurityOpt = unionStrings(svc.SecurityOpt, ov.SecurityOpt) + + // Feature-entrypoint chaining. Replace the service entrypoint with a + // wrapper that runs each feature entrypoint then execs the original + // entrypoint+command. Not escaped: the in-memory project is consumed + // directly (no compose re-interpolation). The service's `command` + // (svc.Command) is left untouched. + if len(ov.Entrypoints) > 0 { + svc.Entrypoint = composetypes.ShellCommand( + RenderEntrypointWrapper(ov.Entrypoints, ov.OriginalEntrypoint, false), + ) + } + project.Services[primaryService] = svc return nil } diff --git a/compose/apply_override_test.go b/compose/apply_override_test.go index 9595d86..c8e085e 100644 --- a/compose/apply_override_test.go +++ b/compose/apply_override_test.go @@ -1,6 +1,7 @@ package compose import ( + "strings" "testing" composetypes "github.com/compose-spec/compose-go/v2/types" @@ -86,6 +87,102 @@ func TestApplyRunOverride_AppendsVolumeAndEnvAndLabels(t *testing.T) { } } +func TestApplyRunOverride_AppliesSecurityMetadata(t *testing.T) { + proj := projectForApply() + // Service already declares one cap and a privileged:false implied by zero. + svc := proj.Services["app"] + svc.CapAdd = []string{"NET_ADMIN"} + proj.Services["app"] = svc + + priv := true + initT := true + ov := Override{ + Service: "app", + Privileged: &priv, + Init: &initT, + CapAdd: []string{"SYS_ADMIN", "NET_ADMIN"}, // NET_ADMIN dup + SecurityOpt: []string{"seccomp=unconfined"}, + } + if err := ApplyRunOverride(proj, "app", ov); err != nil { + t.Fatalf("ApplyRunOverride: %v", err) + } + got := proj.Services["app"] + + if !got.Privileged { + t.Error("Privileged not set") + } + if got.Init == nil || !*got.Init { + t.Error("Init not set") + } + // Union: existing NET_ADMIN preserved first, SYS_ADMIN appended, no dup. + if want := []string{"NET_ADMIN", "SYS_ADMIN"}; !equalStrings(got.CapAdd, want) { + t.Errorf("CapAdd = %v, want %v", got.CapAdd, want) + } + if want := []string{"seccomp=unconfined"}; !equalStrings(got.SecurityOpt, want) { + t.Errorf("SecurityOpt = %v, want %v", got.SecurityOpt, want) + } +} + +func TestApplyRunOverride_AppliesEntrypointWrapper(t *testing.T) { + proj := projectForApply() + ov := Override{ + Service: "app", + Entrypoints: []string{"/usr/local/share/docker-init.sh"}, + } + if err := ApplyRunOverride(proj, "app", ov); err != nil { + t.Fatalf("ApplyRunOverride: %v", err) + } + ep := []string(proj.Services["app"].Entrypoint) + if len(ep) < 4 || ep[0] != "/bin/sh" || ep[3] != "-" { + t.Fatalf("entrypoint wrapper not applied: %v", ep) + } + // Native path: single-dollar (no compose re-interpolation). + if !strings.Contains(ep[2], `exec "$@"`) || strings.Contains(ep[2], "$$") { + t.Errorf("native entrypoint should use single-dollar exec: %q", ep[2]) + } + if !strings.Contains(ep[2], "docker-init.sh") { + t.Errorf("feature entrypoint missing from wrapper: %q", ep[2]) + } +} + +func TestApplyRunOverride_NoEntrypointWhenNoneDeclared(t *testing.T) { + proj := projectForApply() + if err := ApplyRunOverride(proj, "app", Override{Service: "app"}); err != nil { + t.Fatalf("ApplyRunOverride: %v", err) + } + if ep := proj.Services["app"].Entrypoint; len(ep) != 0 { + t.Errorf("entrypoint should be untouched when no feature entrypoints; got %v", ep) + } +} + +// TestApplyRunOverride_NilPrivilegedLeavesServiceValue confirms a nil +// Override.Privileged does not downgrade a user's `privileged: true`. +func TestApplyRunOverride_NilPrivilegedLeavesServiceValue(t *testing.T) { + proj := projectForApply() + svc := proj.Services["app"] + svc.Privileged = true + proj.Services["app"] = svc + + if err := ApplyRunOverride(proj, "app", Override{Service: "app"}); err != nil { + t.Fatalf("ApplyRunOverride: %v", err) + } + if !proj.Services["app"].Privileged { + t.Error("nil Override.Privileged clobbered the service's privileged:true") + } +} + +func equalStrings(a, b []string) bool { + if len(a) != len(b) { + return false + } + for i := range a { + if a[i] != b[i] { + return false + } + } + return true +} + func TestApplyRunOverride_HandlesNilMaps(t *testing.T) { proj := &composetypes.Project{ Services: composetypes.Services{ diff --git a/compose/orchestrator.go b/compose/orchestrator.go index 94c82ab..e939813 100644 --- a/compose/orchestrator.go +++ b/compose/orchestrator.go @@ -655,12 +655,12 @@ func (o *Orchestrator) waitFor( } // serviceToRunSpec is the in-memory translation from compose's -// ServiceConfig to runtime.RunSpec. This is intentionally minimal -// for C6 — env / labels / mounts / command / entrypoint / user / -// workdir / init / cap_add. RunArgs / Privileged / SecurityOpt are -// not in compose's typed model (those are docker-cli concepts), so -// they stay at their zero values. Ports / restart / healthcheck -// are RunSpec gaps to fix in a later PR. +// ServiceConfig to runtime.RunSpec — env / labels / mounts / command / +// entrypoint / user / workdir / init / cap_add / privileged / +// security_opt. The security fields are populated by ApplyRunOverride +// from merged feature+image metadata (e.g. docker-in-docker's +// privileged/init/capAdd), mirroring the flags the image path applies. +// RunArgs has no compose-typed equivalent and stays zero. func serviceToRunSpec( plan *Plan, svc composetypes.ServiceConfig, @@ -719,6 +719,8 @@ func serviceToRunSpec( HealthCheck: healthCheckOf(svc.HealthCheck), Init: svc.Init != nil && *svc.Init, CapAdd: svc.CapAdd, + Privileged: svc.Privileged, + SecurityOpt: svc.SecurityOpt, MemoryBytes: memBytes, NanoCPUs: nanoCPUs, } diff --git a/compose/orchestrator_test.go b/compose/orchestrator_test.go index 1593c66..e42f0f3 100644 --- a/compose/orchestrator_test.go +++ b/compose/orchestrator_test.go @@ -299,6 +299,32 @@ func TestUp_SingleService(t *testing.T) { } } +func TestServiceToRunSpec_CarriesSecurityFields(t *testing.T) { + initT := true + svc := composetypes.ServiceConfig{ + Name: "app", + Image: "alpine", + Init: &initT, + Privileged: true, + CapAdd: []string{"SYS_ADMIN"}, + SecurityOpt: []string{"seccomp=unconfined"}, + } + spec := serviceToRunSpec(&Plan{ProjectName: "dc-x"}, svc, nil, "hash", "") + + if !spec.Privileged { + t.Error("Privileged not carried into RunSpec") + } + if !spec.Init { + t.Error("Init not carried into RunSpec") + } + if len(spec.CapAdd) != 1 || spec.CapAdd[0] != "SYS_ADMIN" { + t.Errorf("CapAdd = %v, want [SYS_ADMIN]", spec.CapAdd) + } + if len(spec.SecurityOpt) != 1 || spec.SecurityOpt[0] != "seccomp=unconfined" { + t.Errorf("SecurityOpt = %v, want [seccomp=unconfined]", spec.SecurityOpt) + } +} + func TestUp_DependencyOrder(t *testing.T) { rt := newMockRuntime() orch := NewOrchestrator(rt, "docker") diff --git a/compose/override.go b/compose/override.go index e57674e..a01fc5a 100644 --- a/compose/override.go +++ b/compose/override.go @@ -44,6 +44,65 @@ type Override struct { // Labels written on the primary service so Engine.Attach's // label-based lookup works (`dev.containers.id`, etc.). Labels map[string]string + + // Security/entrypoint options merged from feature + image metadata + // (config.ResolvedConfig.{Privileged,Init,CapAdd,SecurityOpt}). + // These mirror the `docker run --privileged/--init/--cap-add/ + // --security-opt` flags the image/Dockerfile path applies; on the + // compose path upstream devcontainers/cli emits the equivalent + // service fields into its generated override compose file. Without + // them a feature like docker-in-docker (which declares + // privileged/init/capAdd) silently fails on compose-source + // devcontainers. + // + // Privileged / Init are pointers so nil means "no change" — we must + // not clobber a user's `privileged: true` in their compose file when + // no feature requested it. CapAdd / SecurityOpt are unioned with the + // service's existing entries (dedup, user entries preserved). + Privileged *bool + Init *bool + CapAdd []string + SecurityOpt []string + + // Entrypoints is the ordered chain of feature/image-metadata + // entrypoint scripts (config.ResolvedConfig.Entrypoints). When + // non-empty the primary service's entrypoint is replaced with a + // generated wrapper that runs each in sequence then execs the + // original entrypoint+command — mirroring devcontainers/cli's + // generated compose override. Required for features like + // docker-in-docker whose dockerd is launched by docker-init.sh. + Entrypoints []string + + // OriginalEntrypoint is the entrypoint to preserve underneath the + // wrapper: the service's own `entrypoint:` if it declared one, else + // the image's ENTRYPOINT. The service's `command` is left untouched + // (the wrapper execs entrypoint+command together). Only consulted + // when Entrypoints is non-empty. + OriginalEntrypoint []string +} + +// RenderEntrypointWrapper builds the wrapper entrypoint array that runs +// each feature entrypoint in order, then execs the original +// entrypoint+command (`exec "$@"`), then falls back to a keep-alive +// loop. Mirrors devcontainers/cli's generated compose override entrypoint. +// +// escapeDollar controls $-escaping: the shellout path writes a YAML file +// that `docker compose` re-interpolates, so `$` must be doubled to `$$` +// to survive as a literal; the native path mutates the in-memory project +// (no interpolation) and must keep a single `$`. +func RenderEntrypointWrapper(entrypoints, original []string, escapeDollar bool) []string { + script := "echo Container started\n" + + "trap \"exit 0\" 15\n" + + strings.Join(entrypoints, "\n") + "\n" + + "exec \"$@\"\n" + + "while sleep 1 & wait $!; do :; done" + arr := append([]string{"/bin/sh", "-c", script, "-"}, original...) + if escapeDollar { + for i := range arr { + arr[i] = strings.ReplaceAll(arr[i], "$", "$$") + } + } + return arr } // BindMount describes one bind volume in the override. @@ -99,9 +158,20 @@ func WriteRunOverride(dst string, project *composetypes.Project, ov Override) er return err } + // cap_add / security_opt use compose v2's sequence-REPLACE merge, so + // (like volumes) we union with the user's existing service entries + // and re-emit the full list rather than risk dropping them. + emit := ov + if project != nil { + if svc, err := PrimaryService(project, ov.Service); err == nil { + emit.CapAdd = unionStrings(svc.CapAdd, ov.CapAdd) + emit.SecurityOpt = unionStrings(svc.SecurityOpt, ov.SecurityOpt) + } + } + doc := map[string]any{ "services": map[string]any{ - ov.Service: buildServiceOverride(merged, ov), + ov.Service: buildServiceOverride(merged, emit), }, } body, err := yaml.Marshal(doc) @@ -178,9 +248,52 @@ func buildServiceOverride(volumes []any, ov Override) map[string]any { } svc["labels"] = labels } + if ov.Privileged != nil { + svc["privileged"] = *ov.Privileged + } + if ov.Init != nil { + svc["init"] = *ov.Init + } + if len(ov.CapAdd) > 0 { + svc["cap_add"] = append([]string(nil), ov.CapAdd...) + } + if len(ov.SecurityOpt) > 0 { + svc["security_opt"] = append([]string(nil), ov.SecurityOpt...) + } + if len(ov.Entrypoints) > 0 { + // Escaped: this YAML is re-interpolated by `docker compose`. + svc["entrypoint"] = RenderEntrypointWrapper(ov.Entrypoints, ov.OriginalEntrypoint, true) + } return svc } +// unionStrings returns existing followed by any entries in add not +// already present, preserving order and dropping duplicates. Used to +// merge feature-contributed cap_add / security_opt into a service's +// own entries without clobbering either. +func unionStrings(existing, add []string) []string { + if len(add) == 0 { + return existing + } + seen := make(map[string]struct{}, len(existing)+len(add)) + out := make([]string, 0, len(existing)+len(add)) + for _, s := range existing { + if _, ok := seen[s]; ok { + continue + } + seen[s] = struct{}{} + out = append(out, s) + } + for _, s := range add { + if _, ok := seen[s]; ok { + continue + } + seen[s] = struct{}{} + out = append(out, s) + } + return out +} + // yamlScalar quotes a value if it contains characters that would // require quoting in YAML. Conservative: any non-alphanumeric / dot / // dash / underscore / slash / colon / @ character triggers quoting. diff --git a/compose/override_test.go b/compose/override_test.go index e881a1f..cb461cb 100644 --- a/compose/override_test.go +++ b/compose/override_test.go @@ -148,6 +148,141 @@ volumes: } } +func TestWriteRunOverride_EmitsSecurityMetadata(t *testing.T) { + dir := t.TempDir() + composeFile := filepath.Join(dir, "compose.yml") + if err := os.WriteFile(composeFile, []byte(` +services: + app: + image: alpine + cap_add: + - NET_ADMIN +`), 0o644); err != nil { + t.Fatal(err) + } + project, err := Load(context.Background(), LoadOptions{ + Files: []string{composeFile}, WorkingDir: dir, ProjectName: "test", + }) + if err != nil { + t.Fatal(err) + } + + priv := true + initT := true + dst := filepath.Join(dir, "dc-run.yaml") + if err := WriteRunOverride(dst, project, Override{ + Service: "app", + Privileged: &priv, + Init: &initT, + CapAdd: []string{"SYS_ADMIN"}, + SecurityOpt: []string{"seccomp=unconfined"}, + }); err != nil { + t.Fatalf("WriteRunOverride: %v", err) + } + body, _ := os.ReadFile(dst) + str := string(body) + + for _, want := range []string{ + "privileged: true", + "init: true", + "cap_add:", + "- NET_ADMIN", // user's existing cap preserved (union) + "- SYS_ADMIN", // feature cap added + "security_opt:", + "- seccomp=unconfined", + } { + if !strings.Contains(str, want) { + t.Errorf("dc-run.yaml missing %q\n%s", want, str) + } + } +} + +func TestWriteRunOverride_OmitsUnsetSecurityMetadata(t *testing.T) { + dst := filepath.Join(t.TempDir(), "dc-run.yaml") + if err := WriteRunOverride(dst, nil, Override{ + Service: "app", + ExtraEnvironment: map[string]string{"X": "1"}, + }); err != nil { + t.Fatalf("WriteRunOverride: %v", err) + } + body, _ := os.ReadFile(dst) + str := string(body) + for _, unwanted := range []string{"privileged", "init:", "cap_add", "security_opt"} { + if strings.Contains(str, unwanted) { + t.Errorf("dc-run.yaml unexpectedly contains %q\n%s", unwanted, str) + } + } +} + +func TestRenderEntrypointWrapper_NativeVsEscaped(t *testing.T) { + eps := []string{"/usr/local/share/docker-init.sh"} + + // Native (in-memory, no compose interpolation): single-dollar. + native := RenderEntrypointWrapper(eps, nil, false) + if len(native) != 4 { + t.Fatalf("native wrapper = %v, want 4 elements", native) + } + if native[0] != "/bin/sh" || native[1] != "-c" || native[3] != "-" { + t.Errorf("wrapper shape wrong: %v", native) + } + if !strings.Contains(native[2], `exec "$@"`) { + t.Errorf("native script missing single-dollar exec: %q", native[2]) + } + if !strings.Contains(native[2], "/usr/local/share/docker-init.sh") { + t.Errorf("native script missing feature entrypoint: %q", native[2]) + } + if strings.Contains(native[2], "$$") { + t.Errorf("native script must NOT double-escape dollars: %q", native[2]) + } + + // Escaped (shellout YAML, re-interpolated by docker compose): $$. + escaped := RenderEntrypointWrapper(eps, nil, true) + if !strings.Contains(escaped[2], `exec "$$@"`) { + t.Errorf("escaped script missing $$@: %q", escaped[2]) + } + if !strings.Contains(escaped[2], "$$!") { + t.Errorf("escaped script missing $$!: %q", escaped[2]) + } +} + +func TestRenderEntrypointWrapper_PreservesOriginalEntrypoint(t *testing.T) { + w := RenderEntrypointWrapper( + []string{"/init.sh"}, + []string{"/my/orig-entrypoint", "--flag"}, + false, + ) + // Original tokens appended after the "-" sentinel so `exec "$@"` runs them. + if w[3] != "-" || w[4] != "/my/orig-entrypoint" || w[5] != "--flag" { + t.Errorf("original entrypoint not appended after sentinel: %v", w) + } +} + +func TestWriteRunOverride_EmitsEntrypointWrapper(t *testing.T) { + dst := filepath.Join(t.TempDir(), "dc-run.yaml") + if err := WriteRunOverride(dst, nil, Override{ + Service: "app", + Entrypoints: []string{"/usr/local/share/docker-init.sh"}, + }); err != nil { + t.Fatalf("WriteRunOverride: %v", err) + } + body, _ := os.ReadFile(dst) + str := string(body) + for _, want := range []string{ + "entrypoint:", + "/bin/sh", + "docker-init.sh", + "$$@", // doubled so it survives docker compose interpolation as $@ + } { + if !strings.Contains(str, want) { + t.Errorf("dc-run.yaml missing %q\n%s", want, str) + } + } + // Must NOT contain a bare single-dollar $@ that compose would mangle. + if strings.Contains(str, `"$@"`) { + t.Errorf("dc-run.yaml has un-escaped $@ that compose would interpolate:\n%s", str) + } +} + func TestWriteRunOverride_RequiresService(t *testing.T) { dst := filepath.Join(t.TempDir(), "x.yaml") if err := WriteRunOverride(dst, nil, Override{}); err == nil { diff --git a/config/merge_metadata.go b/config/merge_metadata.go index e3954e5..e6c0e71 100644 --- a/config/merge_metadata.go +++ b/config/merge_metadata.go @@ -76,6 +76,19 @@ func MergeMetadata(cfg *ResolvedConfig, subCtx SubstitutionContext, layers []Fea cfg.CapAdd = unionDedup(layerStrings(layers, func(l FeatureMetadata) []string { return l.CapAdd }), cfg.CapAdd) cfg.SecurityOpt = unionDedup(layerStrings(layers, func(l FeatureMetadata) []string { return l.SecurityOpt }), cfg.SecurityOpt) + // Entrypoints: accumulate every non-empty layer entrypoint in chain + // order (base-image label entries first, features last). Unlike the + // scalar fields these do NOT follow last-wins — every feature that + // declares an entrypoint must run. Assigned fresh (not appended) so + // MergeMetadata stays idempotent; devcontainer.json contributes none. + var entrypoints []string + for _, l := range layers { + if l.Entrypoint != "" { + entrypoints = append(entrypoints, l.Entrypoint) + } + } + cfg.Entrypoints = entrypoints + // Maps: layered union with user overlay last. cfg.ContainerEnv = mergeStringMaps(layerMaps(layers, func(l FeatureMetadata) map[string]string { return l.ContainerEnv }), cfg.ContainerEnv) cfg.RemoteEnv = mergeStringMaps(layerMaps(layers, func(l FeatureMetadata) map[string]string { return l.RemoteEnv }), cfg.RemoteEnv) diff --git a/config/merge_metadata_test.go b/config/merge_metadata_test.go index 858bf51..aaa2e17 100644 --- a/config/merge_metadata_test.go +++ b/config/merge_metadata_test.go @@ -75,6 +75,29 @@ func TestMergeMetadata_CapAdd_UnionDedup(t *testing.T) { } } +func TestMergeMetadata_Entrypoints_OrderedChainSkipEmpty(t *testing.T) { + cfg := &ResolvedConfig{} + MergeMetadata(cfg, SubstitutionContext{}, []FeatureMetadata{ + {ID: "base", Entrypoint: "/base-init.sh"}, + {ID: "no-ep"}, // contributes nothing + {ID: "dind", Entrypoint: "/usr/local/share/docker-init.sh"}, + }) + want := []string{"/base-init.sh", "/usr/local/share/docker-init.sh"} + if !reflect.DeepEqual(cfg.Entrypoints, want) { + t.Errorf("got %v, want %v", cfg.Entrypoints, want) + } +} + +func TestMergeMetadata_Entrypoints_Idempotent(t *testing.T) { + cfg := &ResolvedConfig{} + layers := []FeatureMetadata{{ID: "dind", Entrypoint: "/init.sh"}} + MergeMetadata(cfg, SubstitutionContext{}, layers) + MergeMetadata(cfg, SubstitutionContext{}, layers) + if want := []string{"/init.sh"}; !reflect.DeepEqual(cfg.Entrypoints, want) { + t.Errorf("merge not idempotent: got %v, want %v", cfg.Entrypoints, want) + } +} + func TestMergeMetadata_LifecycleHooks_OrderedAndUserLast(t *testing.T) { cfg := &ResolvedConfig{ Lifecycle: LifecycleCommands{ diff --git a/config/resolved.go b/config/resolved.go index 29cba50..a5f91e1 100644 --- a/config/resolved.go +++ b/config/resolved.go @@ -35,6 +35,14 @@ type ResolvedConfig struct { OverrideCommand *bool ShutdownAction ShutdownAction + // Entrypoints is the ordered chain of feature/image-metadata + // `entrypoint` scripts (e.g. docker-in-docker's docker-init.sh). + // At container start each runs in sequence before the original + // entrypoint/command, via a generated wrapper. Sourced only from + // metadata layers (base-image label entries first, then features) — + // devcontainer.json has no top-level entrypoint. Empty = no chaining. + Entrypoints []string + Features []ResolvedFeature Lifecycle LifecycleCommands diff --git a/runtime/applecontainer/inspect_darwin_arm64.go b/runtime/applecontainer/inspect_darwin_arm64.go index 2116470..902e1dd 100644 --- a/runtime/applecontainer/inspect_darwin_arm64.go +++ b/runtime/applecontainer/inspect_darwin_arm64.go @@ -239,6 +239,11 @@ func snapshotToDetails(s containerSnapshot) *runtime.ContainerDetails { // Created / FinishedAt / ExitCode are not in Apple's // ContainerSnapshot. Left as zero values; later PRs can // surface them via an additional XPC call if exposed. + // + // Privileged / CapAdd / SecurityOpt likewise have no + // equivalent in Apple's snapshot (the VM-isolation model + // doesn't expose docker-style HostConfig security flags), so + // they stay at zero values. } } diff --git a/runtime/docker/inspect.go b/runtime/docker/inspect.go index a5b8dc7..d05ce7c 100644 --- a/runtime/docker/inspect.go +++ b/runtime/docker/inspect.go @@ -29,6 +29,7 @@ func (r *Runtime) InspectImage(ctx context.Context, ref string) (*runtime.ImageD out.Labels = copyLabels(res.Config.Labels) out.Env = append([]string(nil), res.Config.Env...) out.User = res.Config.User + out.Entrypoint = append([]string(nil), res.Config.Entrypoint...) } return out, nil } @@ -62,6 +63,11 @@ func (r *Runtime) InspectContainer(ctx context.Context, id string) (*runtime.Con out.Env = append([]string(nil), c.Config.Env...) out.Labels = copyLabels(c.Config.Labels) } + if c.HostConfig != nil { + out.Privileged = c.HostConfig.Privileged + out.CapAdd = append([]string(nil), c.HostConfig.CapAdd...) + out.SecurityOpt = append([]string(nil), c.HostConfig.SecurityOpt...) + } return out, nil } diff --git a/runtime/runtime.go b/runtime/runtime.go index 0e47673..a0fff22 100644 --- a/runtime/runtime.go +++ b/runtime/runtime.go @@ -310,6 +310,16 @@ type ContainerDetails struct { Mounts []MountInspect Labels map[string]string + // Security options as actually applied to the container, read back + // from the backend's inspect (docker/podman HostConfig). Lets callers + // verify that RunSpec.Privileged/CapAdd/SecurityOpt — including the + // values merged from feature metadata onto compose services — landed + // on the real container. Backends that don't surface these (e.g. + // applecontainer) leave them at zero values. + Privileged bool + CapAdd []string + SecurityOpt []string + // ExitCode is the container's last exit code. Zero is ambiguous: // either "process is still running" or "process exited cleanly". // Use State to disambiguate (running vs exited). @@ -368,6 +378,13 @@ type ImageDetails struct { // reconciliation when devcontainer.json's remoteUser/containerUser // are also empty. User string + + // Entrypoint is the image's ENTRYPOINT (Config.Entrypoint), nil if + // unset. Used to preserve the image's own entrypoint underneath the + // feature-entrypoint wrapper when a feature declares an entrypoint + // (e.g. docker-in-docker) and the consumer (compose service) declares + // none of its own. + Entrypoint []string } // MountInspect describes a mount as reported by the runtime, not what diff --git a/test/integration/compose_feature_security_test.go b/test/integration/compose_feature_security_test.go new file mode 100644 index 0000000..fa6050d --- /dev/null +++ b/test/integration/compose_feature_security_test.go @@ -0,0 +1,190 @@ +//go:build integration + +package integration + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" + "time" + + devcontainer "github.com/crunchloop/devcontainer" +) + +// writeComposeSecurityWorkspace creates a compose-source workspace whose +// primary service declares NO security options itself, plus a local +// feature that declares privileged / init / capAdd / securityOpt in its +// metadata. It exercises the parity fix: feature-declared security +// metadata must be applied to the compose service (mirroring what the +// image/Dockerfile path does and what upstream devcontainers/cli does via +// its generated override compose file). Without the fix the container +// comes up unprivileged and a feature like docker-in-docker silently +// fails. +func writeComposeSecurityWorkspace(t *testing.T) string { + t.Helper() + dir := t.TempDir() + + // Note: the user's compose service does NOT set privileged/cap_add — + // the values must come purely from the feature metadata. + mustWrite(t, filepath.Join(dir, "docker-compose.yml"), ` +services: + app: + image: `+testImage+` + command: ["sh", "-c", "while sleep 1000; do :; done"] +`) + + mustWrite(t, filepath.Join(dir, ".devcontainer", "devcontainer.json"), `{ + "dockerComposeFile": "../docker-compose.yml", + "service": "app", + "workspaceFolder": "/workspaces/proj", + "features": { "./sec-feature": {} } + }`) + + featureDir := filepath.Join(dir, ".devcontainer", "sec-feature") + mustWrite(t, filepath.Join(featureDir, "devcontainer-feature.json"), `{ + "id": "sec-feature", + "version": "1.0.0", + "privileged": true, + "init": true, + "capAdd": ["SYS_ADMIN"], + "securityOpt": ["seccomp=unconfined"], + "entrypoint": "/usr/local/share/sec-entrypoint.sh" + }`) + // install.sh: drop the marker AND install an entrypoint script that + // runs at container start (proving feature-entrypoint chaining). The + // entrypoint must NOT exec/block — it records that it ran and returns + // so the wrapper goes on to exec the service command. + mustWrite(t, filepath.Join(featureDir, "install.sh"), `#!/bin/sh +set -e +echo sec-feature-ran > /etc/sec-feature-marker +cat > /usr/local/share/sec-entrypoint.sh <<'EOF' +#!/bin/sh +echo sec-entrypoint-ran > /tmp/sec-entrypoint-marker +EOF +chmod +x /usr/local/share/sec-entrypoint.sh +`) + if err := os.Chmod(filepath.Join(featureDir, "install.sh"), 0o755); err != nil { + t.Fatal(err) + } + return dir +} + +// TestComposeSource_FeatureSecurityOptionsApplied confirms that a +// feature's privileged/init/capAdd/securityOpt land on the real primary +// container for BOTH compose backends (native = in-memory project mutate; +// shellout = generated YAML override). Asserts against the backend's +// inspect (HostConfig), which is the source of truth for what was +// actually applied. +func TestComposeSource_FeatureSecurityOptionsApplied(t *testing.T) { + if testing.Short() { + t.Skip() + } + + backends := []struct { + name string + backend devcontainer.ComposeBackend + }{ + {"native", devcontainer.ComposeBackendNative}, + {"shellout", devcontainer.ComposeBackendShellout}, + } + + for _, bk := range backends { + t.Run(bk.name, func(t *testing.T) { + eng, rt := newEngineWith(t, devcontainer.EngineOptions{ + ComposeBackend: bk.backend, + }) + defer rt.Close() + + ws := writeComposeSecurityWorkspace(t) + + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute) + defer cancel() + + wsObj, err := eng.Up(ctx, devcontainer.UpOptions{ + LocalWorkspaceFolder: ws, + Recreate: true, + SkipLifecycle: true, + }) + if err != nil { + t.Fatalf("Up: %v", err) + } + defer func() { + _ = eng.Down(context.Background(), wsObj, devcontainer.DownOptions{ + Remove: true, + RemoveVolumes: true, + }) + }() + + if wsObj.Container == nil { + t.Fatal("Workspace.Container is nil") + } + + details, err := rt.InspectContainer(ctx, wsObj.Container.ID) + if err != nil { + t.Fatalf("InspectContainer: %v", err) + } + + if !details.Privileged { + t.Error("container is not privileged; feature privileged:true was not applied to the compose service") + } + // Docker/podman normalize capability names to the CAP_-prefixed + // form in inspect output (SYS_ADMIN -> CAP_SYS_ADMIN), so compare + // with the prefix stripped. + if !hasCapability(details.CapAdd, "SYS_ADMIN") { + t.Errorf("CapAdd = %v, want it to contain SYS_ADMIN", details.CapAdd) + } + if !containsStr(details.SecurityOpt, "seccomp=unconfined") { + t.Errorf("SecurityOpt = %v, want it to contain seccomp=unconfined", details.SecurityOpt) + } + + // Sanity: the feature actually installed (proves we inspected + // the right container and the feature layer ran). + res, err := eng.Exec(ctx, wsObj, devcontainer.ExecOptions{ + Cmd: []string{"cat", "/etc/sec-feature-marker"}, + }) + if err != nil { + t.Fatalf("Exec marker: %v", err) + } + if res.ExitCode != 0 { + t.Errorf("sec-feature-marker missing: stderr=%q", res.Stderr) + } + + // Feature-entrypoint chaining: the feature's entrypoint script + // must have run at container start (before the service command, + // which is still running — proving the wrapper exec'd it). + res, err = eng.Exec(ctx, wsObj, devcontainer.ExecOptions{ + Cmd: []string{"cat", "/tmp/sec-entrypoint-marker"}, + }) + if err != nil { + t.Fatalf("Exec entrypoint marker: %v", err) + } + if res.ExitCode != 0 || !strings.Contains(res.Stdout, "sec-entrypoint-ran") { + t.Errorf("feature entrypoint did not run: exit=%d stdout=%q stderr=%q", + res.ExitCode, res.Stdout, res.Stderr) + } + }) + } +} + +func containsStr(haystack []string, needle string) bool { + for _, s := range haystack { + if s == needle { + return true + } + } + return false +} + +// hasCapability reports whether caps contains the given capability, +// ignoring the optional CAP_ prefix the daemon adds in inspect output. +func hasCapability(caps []string, want string) bool { + want = strings.TrimPrefix(want, "CAP_") + for _, c := range caps { + if strings.TrimPrefix(c, "CAP_") == want { + return true + } + } + return false +} diff --git a/up.go b/up.go index ab3e517..518f169 100644 --- a/up.go +++ b/up.go @@ -559,13 +559,47 @@ func (e *Engine) createFreshCompose(ctx context.Context, cfg *config.ResolvedCon } overrideEnv := mergeEnv(cfg.ContainerEnv, extraEnv) + // When features/image metadata declare entrypoint scripts (e.g. + // docker-in-docker's docker-init.sh), they must run before the + // service's command — see RenderEntrypointWrapper. The wrapper + // preserves the "original" entrypoint underneath: the service's own + // `entrypoint:` if it declared one, else the image's ENTRYPOINT. + var origEntrypoint []string + if len(cfg.Entrypoints) > 0 { + if svc, ok := project.Services[src.Service]; ok && len(svc.Entrypoint) > 0 { + origEntrypoint = []string(svc.Entrypoint) + } else if details, err := e.runtime.InspectImage(ctx, finalImage); err == nil && details != nil { + origEntrypoint = details.Entrypoint + } + } + + // Run-time override layered onto the primary service. Mirrors the + // flags the image path applies in newRunSpec: workspace/cfg mounts, + // container env, convergence labels, the security options merged from + // feature+image metadata (Privileged/Init/CapAdd/SecurityOpt), and the + // feature-entrypoint chain — without these, features like + // docker-in-docker that declare privileged/init/entrypoint silently + // fail on compose-source devcontainers. + runOverride := compose.Override{ + Service: src.Service, + ExtraBindMounts: bindMounts, + ExtraEnvironment: overrideEnv, + Labels: overrideLabels, + Privileged: cfg.Privileged, + Init: cfg.Init, + CapAdd: cfg.CapAdd, + SecurityOpt: cfg.SecurityOpt, + Entrypoints: cfg.Entrypoints, + OriginalEntrypoint: origEntrypoint, + } + switch e.opts.ComposeBackend { case ComposeBackendNative: return e.upComposeNative(ctx, cfg, opts, project, src, projectName, - finalImage, bindMounts, overrideEnv, overrideLabels) + finalImage, runOverride) case ComposeBackendShellout: return e.upComposeShellout(ctx, cfg, opts, project, src, projectName, - workingDir, finalImage, bindMounts, overrideEnv, overrideLabels, existingContainer) + workingDir, finalImage, runOverride, existingContainer) default: // Reject unknown values explicitly so a typo in // EngineOptions.ComposeBackend doesn't silently route to @@ -583,9 +617,7 @@ func (e *Engine) upComposeShellout( project *composetypes.Project, src *config.ComposeSource, projectName, workingDir, finalImage string, - bindMounts []compose.BindMount, - overrideEnv map[string]string, - overrideLabels map[string]string, + runOverride compose.Override, existingContainer bool, ) (*Workspace, error) { cr, ok := e.runtime.(runtime.ComposeRuntime) @@ -609,12 +641,7 @@ func (e *Engine) upComposeShellout( return nil, err } - if err := compose.WriteRunOverride(runOverridePath, project, compose.Override{ - Service: src.Service, - ExtraBindMounts: bindMounts, - ExtraEnvironment: overrideEnv, - Labels: overrideLabels, - }); err != nil { + if err := compose.WriteRunOverride(runOverridePath, project, runOverride); err != nil { return nil, err } @@ -663,19 +690,12 @@ func (e *Engine) upComposeNative( project *composetypes.Project, src *config.ComposeSource, projectName, finalImage string, - bindMounts []compose.BindMount, - overrideEnv map[string]string, - overrideLabels map[string]string, + runOverride compose.Override, ) (*Workspace, error) { if err := compose.ApplyBuildOverride(project, src.Service, finalImage); err != nil { return nil, err } - if err := compose.ApplyRunOverride(project, src.Service, compose.Override{ - Service: src.Service, - ExtraBindMounts: bindMounts, - ExtraEnvironment: overrideEnv, - Labels: overrideLabels, - }); err != nil { + if err := compose.ApplyRunOverride(project, src.Service, runOverride); err != nil { return nil, err } From 828cf594dcbe2f18efdba7febb733c2c5e2a0151 Mon Sep 17 00:00:00 2001 From: bilby91 Date: Wed, 24 Jun 2026 18:57:23 -0300 Subject: [PATCH 2/2] compose: surface InspectImage failure in entrypoint-wrapper fallback When a feature declares an entrypoint and the compose service declares none, we inspect the final image to preserve its ENTRYPOINT under the wrapper. A failed inspect left origEntrypoint nil and silently dropped the image ENTRYPOINT from `exec "$@"`. Emit a WarnEvent so the fallback is observable. Addresses CodeRabbit review on #103. Co-Authored-By: Claude Opus 4.8 (1M context) --- up.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/up.go b/up.go index 518f169..cc4f40e 100644 --- a/up.go +++ b/up.go @@ -570,6 +570,15 @@ func (e *Engine) createFreshCompose(ctx context.Context, cfg *config.ResolvedCon origEntrypoint = []string(svc.Entrypoint) } else if details, err := e.runtime.InspectImage(ctx, finalImage); err == nil && details != nil { origEntrypoint = details.Entrypoint + } else if err != nil { + // Non-fatal: we still apply the entrypoint wrapper, but the + // image's own ENTRYPOINT can't be preserved underneath it. + // Surface it so this silent fallback is diagnosable. + opts.bus.Emit(events.WarnEvent{ + Code: "compose_entrypoint_image_inspect_failed", + Message: fmt.Sprintf("could not inspect %s to preserve its ENTRYPOINT under the feature-entrypoint wrapper for service %q: %v", + finalImage, src.Service, err), + }) } }