From bcb81f0b84990f07ee531b001e6f2fcecdf80f88 Mon Sep 17 00:00:00 2001 From: Hooman Date: Tue, 8 Sep 2026 17:27:43 +0330 Subject: [PATCH] fix(publish): support unresolved variables during pre-checks When publishing a Compose project containing unresolved variables in typed fields (e.g. short-form port syntax `${PORT:-3000}:3000`, `mem_limit: ${MEM}`, or `deploy.replicas: ${REPLICAS}`), publish previously failed with type decode errors because `loadUnresolvedFile` strictly transformed the raw model dictionary into a full `types.Project` with `SkipInterpolation = true`. Neither consumer requires strict decoding of the entire `types.Project`: 1. `collectEnvCheckFindings` only inspects service environment, env_files, extends, and project configs. It now decodes only these fields into a dedicated lightweight struct via `loader.Transform`, ignoring unrelated typed fields like ports, mem_limit, and deploy. 2. `composeFileAsByteReader` keeps `loader.LoadModelWithContext`, normalizes any list-form environment declarations into map form using `types.NewMappingWithEquals`, and marshals the raw dictionary directly to YAML. This preserves full-file scanning and prevents secret detection regressions on list-form environment entries. 3. Temporarily pin compose-go#928 via `replace` in go.mod to fix unresolved variables in build.ssh and ulimits during canonicalization. 4. Added tests verifying `loadUnresolvedFile`, `checkForSensitiveData`, and `collectEnvCheckFindings` with unresolved variables in ports, mem_limit, and deploy.replicas, as well as secret detection in list-form environments. Fixes #13672 Signed-off-by: Hooman --- go.mod | 2 + go.sum | 4 +- pkg/compose/publish.go | 91 +++++++++++++++++++++++++---- pkg/compose/publish_test.go | 110 ++++++++++++++++++++++++++++++++++++ 4 files changed, 195 insertions(+), 12 deletions(-) diff --git a/go.mod b/go.mod index 65b5f96bed..59d6fc02d8 100644 --- a/go.mod +++ b/go.mod @@ -144,3 +144,5 @@ exclude ( github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 ) + +replace github.com/compose-spec/compose-go/v2 => github.com/compose-spec/compose-go/v2 v2.15.1-0.20260910130034-0e17c6437ebb diff --git a/go.sum b/go.sum index 7812739409..29dcc0df5c 100644 --- a/go.sum +++ b/go.sum @@ -32,8 +32,8 @@ github.com/cloudflare/circl v1.6.3 h1:9GPOhQGF9MCYUeXyMYlqTR6a5gTrgR/fBLXvUgtVcg github.com/cloudflare/circl v1.6.3/go.mod h1:2eXP6Qfat4O/Yhh8BznvKnJ+uzEoTQ6jVKJRn81BiS4= github.com/codahale/rfc6979 v0.0.0-20141003034818-6a90f24967eb h1:EDmT6Q9Zs+SbUoc7Ik9EfrFqcylYqgPZ9ANSbTAntnE= github.com/codahale/rfc6979 v0.0.0-20141003034818-6a90f24967eb/go.mod h1:ZjrT6AXHbDs86ZSdt/osfBi5qfexBrKUdONk989Wnk4= -github.com/compose-spec/compose-go/v2 v2.15.1-0.20260908103050-cda18529aca7 h1:D9jScaeUAk7DjyB71SD9zvOFPR6klw0FxPcR2bgu72c= -github.com/compose-spec/compose-go/v2 v2.15.1-0.20260908103050-cda18529aca7/go.mod h1:Q1+qtN4vhzEjGrnqRtzx1xa8raDZQlMUe3WJxndYNiQ= +github.com/compose-spec/compose-go/v2 v2.15.1-0.20260910130034-0e17c6437ebb h1:2r/BSgm9NJLvTF84ULhXkq94woFgD2A5/p9PjY6Vmug= +github.com/compose-spec/compose-go/v2 v2.15.1-0.20260910130034-0e17c6437ebb/go.mod h1:Q1+qtN4vhzEjGrnqRtzx1xa8raDZQlMUe3WJxndYNiQ= github.com/containerd/cgroups/v3 v3.1.3 h1:eUNflyMddm18+yrDmZPn3jI7C5hJ9ahABE5q6dyLYXQ= github.com/containerd/cgroups/v3 v3.1.3/go.mod h1:PKZ2AcWmSBsY/tJUVhtS/rluX0b1uq1GmPO1ElCmbOw= github.com/containerd/console v1.0.5 h1:R0ymNeydRqH2DmakFNdmjR2k0t7UPuiOV/N/27/qqsc= diff --git a/pkg/compose/publish.go b/pkg/compose/publish.go index 4f362bef19..5d40618eb1 100644 --- a/pkg/compose/publish.go +++ b/pkg/compose/publish.go @@ -40,6 +40,7 @@ import ( "github.com/opencontainers/image-spec/specs-go" v1 "github.com/opencontainers/image-spec/specs-go/v1" "github.com/sirupsen/logrus" + "go.yaml.in/yaml/v4" "github.com/docker/compose/v5/internal/desktop" "github.com/docker/compose/v5/internal/oci" @@ -508,9 +509,15 @@ func collectEnvCheckFindings(ctx context.Context, project *types.Project) (*envC return nil, fmt.Errorf("failed to load compose file %s: %w", file, err) } - for _, service := range unresolved.Services { - recordServiceEnvFindings(findings.services, keywordDetector, service) - if parent := localExtendsParent(service); parent != "" { + for name, service := range unresolved.Services { + svc := types.ServiceConfig{ + Name: name, + Environment: service.Environment, + EnvFiles: service.EnvFiles, + Extends: service.Extends, + } + recordServiceEnvFindings(findings.services, keywordDetector, svc) + if parent := localExtendsParent(svc); parent != "" { queue = append(queue, parent) } } @@ -631,11 +638,27 @@ func buildConfigContentPromptMessage(configs []string) string { return b.String() } -// loadUnresolvedFile loads a single compose file with interpolation and -// environment resolution skipped, so callers can inspect raw user-provided -// values. Used by both checkEnvironmentVariables and composeFileAsByteReader. -func loadUnresolvedFile(ctx context.Context, project *types.Project, filePath string) (*types.Project, error) { - return loader.LoadWithContext(ctx, types.ConfigDetails{ +type unresolvedFile struct { + Services map[string]unresolvedService `yaml:"services"` + Configs map[string]unresolvedConfig `yaml:"configs"` +} + +type unresolvedService struct { + Environment types.MappingWithEquals `yaml:"environment"` + EnvFiles []types.EnvFile `yaml:"env_file"` + Extends *types.ExtendsConfig `yaml:"extends"` +} + +type unresolvedConfig struct { + Content string `yaml:"content"` +} + +// loadUnresolvedModel loads a single compose file with interpolation and +// environment resolution skipped, returning the raw model dictionary. +// Callers can inspect raw user-provided values without strict decoding of +// typed fields that fail on un-interpolated variable syntax. +func loadUnresolvedModel(ctx context.Context, project *types.Project, filePath string) (map[string]any, error) { + return loader.LoadModelWithContext(ctx, types.ConfigDetails{ WorkingDir: project.WorkingDir, Environment: project.Environment, ConfigFiles: []types.ConfigFile{{Filename: filePath}}, @@ -654,6 +677,24 @@ func loadUnresolvedFile(ctx context.Context, project *types.Project, filePath st }) } +// loadUnresolvedFile loads a single compose file with interpolation and +// environment resolution skipped, decoding only the fields inspected by +// collectEnvCheckFindings (service environment, env_files, extends, and +// config content). Decoding only used fields avoids failures on un-interpolated +// variable syntax in typed fields (e.g. ports, mem_limit, deploy.replicas). +func loadUnresolvedFile(ctx context.Context, project *types.Project, filePath string) (*unresolvedFile, error) { + dict, err := loadUnresolvedModel(ctx, project, filePath) + if err != nil { + return nil, err + } + + var file unresolvedFile + if err := loader.Transform(dict, &file); err != nil { + return nil, err + } + return &file, nil +} + func envFileLayers(files map[string]string) []v1.Descriptor { var layers []v1.Descriptor for file, hash := range files { @@ -794,12 +835,42 @@ func scanFiles(scan secrets.Scanner, kind string, paths []string) ([]secrets.Det return allFindings, nil } +func normalizeServicesEnvironment(dict map[string]any) { + services, ok := dict["services"].(map[string]any) + if !ok { + return + } + for serviceName, cfg := range services { + serviceConfig, ok := cfg.(map[string]any) + if !ok { + continue + } + switch env := serviceConfig["environment"].(type) { + case []any: + list := make([]string, 0, len(env)) + for _, item := range env { + if s, ok := item.(string); ok { + list = append(list, s) + } + } + serviceConfig["environment"] = types.NewMappingWithEquals(list) + services[serviceName] = serviceConfig + case []string: + serviceConfig["environment"] = types.NewMappingWithEquals(env) + services[serviceName] = serviceConfig + } + } +} + func composeFileAsByteReader(ctx context.Context, filePath string, project *types.Project) (io.Reader, error) { - base, err := loadUnresolvedFile(ctx, project, filePath) + dict, err := loadUnresolvedModel(ctx, project, filePath) if err != nil { return nil, fmt.Errorf("failed to load compose file %s: %w", filePath, err) } - in, err := base.MarshalYAML() + + normalizeServicesEnvironment(dict) + + in, err := yaml.Marshal(dict) if err != nil { return nil, err } diff --git a/pkg/compose/publish_test.go b/pkg/compose/publish_test.go index c78808ac25..9c7bc4e19b 100644 --- a/pkg/compose/publish_test.go +++ b/pkg/compose/publish_test.go @@ -209,6 +209,95 @@ services: assert.Equal(t, len(envFiles), 1, "present optional env file should be added") } +func Test_loadUnresolvedFile_short_port_mapping(t *testing.T) { + dir := t.TempDir() + composePath := filepath.Join(dir, "compose.yaml") + composeContent := `name: test +services: + whoami: + image: docker.io/traefik/whoami:v1.11 + ports: + - ${DASHBOARD_PORT:-3000}:3000 + - $PORT:80 + - 8080:${TARGET_PORT:-8080} + mem_limit: ${MEM} + deploy: + replicas: ${REPLICAS} + healthcheck: + retries: ${RETRIES} + environment: + API_KEY: "$ENV_KEY" + worker: + image: alpine + environment: + - LIST_KEY=list_val +` + assert.NilError(t, os.WriteFile(composePath, []byte(composeContent), 0o600)) + + project := &types.Project{ + WorkingDir: dir, + ComposeFiles: []string{composePath}, + } + + unresolved, err := loadUnresolvedFile(t.Context(), project, composePath) + assert.NilError(t, err) + assert.Assert(t, unresolved.Services["whoami"].Environment != nil) + assert.Equal(t, *unresolved.Services["whoami"].Environment["API_KEY"], "$ENV_KEY") + assert.Assert(t, unresolved.Services["worker"].Environment != nil) + assert.Equal(t, *unresolved.Services["worker"].Environment["LIST_KEY"], "list_val") +} + +func Test_checkForSensitiveData_short_port_mapping(t *testing.T) { + dir := t.TempDir() + composePath := filepath.Join(dir, "compose.yaml") + composeContent := `name: test +services: + whoami: + image: docker.io/traefik/whoami:v1.11 + ports: + - ${DASHBOARD_PORT:-3000}:3000 + mem_limit: ${MEM} + deploy: + replicas: ${REPLICAS} + healthcheck: + retries: ${RETRIES} +` + assert.NilError(t, os.WriteFile(composePath, []byte(composeContent), 0o600)) + + project := &types.Project{ + WorkingDir: dir, + ComposeFiles: []string{composePath}, + } + + svc := &composeService{} + findings, err := svc.checkForSensitiveData(t.Context(), project) + assert.NilError(t, err) + assert.Equal(t, len(findings), 0) +} + +func Test_checkForSensitiveData_list_form_secret(t *testing.T) { + dir := t.TempDir() + composePath := filepath.Join(dir, "compose.yaml") + composeContent := `name: test +services: + web: + image: nginx + environment: + - AWS_SECRET_ACCESS_KEY="wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY" +` + assert.NilError(t, os.WriteFile(composePath, []byte(composeContent), 0o600)) + + project := &types.Project{ + WorkingDir: dir, + ComposeFiles: []string{composePath}, + } + + svc := &composeService{} + findings, err := svc.checkForSensitiveData(t.Context(), project) + assert.NilError(t, err) + assert.Assert(t, len(findings) > 0, "secret scanner must detect secrets in list-form environment entries") +} + func Test_checkForSensitiveData_optional_env_file_missing(t *testing.T) { dir := t.TempDir() project := &types.Project{ @@ -326,6 +415,27 @@ services: environment: DB_PASSWORD: "${DB_PASSWORD}" API_KEY: "$API_KEY" +`, + }, + }, + { + name: "unresolved variables in ports, mem_limit, and replicas do not fail env check", + files: map[string]string{ + "compose.yaml": `name: test +services: + whoami: + image: traefik/whoami:v1.11 + ports: + - ${DASHBOARD_PORT:-3000}:3000 + - $PORT:80 + - 8080:${TARGET_PORT:-8080} + mem_limit: ${MEM:-512m} + deploy: + replicas: ${REPLICAS:-2} + healthcheck: + retries: ${RETRIES:-3} + environment: + API_KEY: "$ENV_KEY" `, }, },