fix(publish): prompt on sensitive-looking env literals

Replace the "flag any literal env var" check with a key-name heuristic
backed by the upstream DefangLabs keyword detector (password, secret,
token, api_key, …), and convert the hard error into a prompt matching
the existing checkForBindMount / checkForSensitiveData UX. --with-env
silences the env prompt; literal config.content gets its own prompt.

The previous check flagged benign vars like LOG_LEVEL=info, blocking
the 99% case, while still missing low-entropy real secrets the
existing secret-detector skips (MYSQL_ROOT_PASSWORD=toto slips through
on entropy ~1.5).

Refs: #13394

Signed-off-by: Guillaume Lours <glours@users.noreply.github.com>

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Guillaume Lours <glours@users.noreply.github.com>

Author: glours

pkg/compose/publish.go

@@ -25,8 +25,10 @@ import (
 	"fmt"
 	"io"
 	"os"
+	"slices"
 	"strings"
 
+	"github.com/DefangLabs/secret-detector/pkg/detectors/keyword"
 	"github.com/DefangLabs/secret-detector/pkg/scanner"
 	"github.com/DefangLabs/secret-detector/pkg/secrets"
 	"github.com/compose-spec/compose-go/v2/loader"
@@ -341,37 +343,285 @@ func (s *composeService) preChecks(ctx context.Context, project *types.Project,
 			return false, err
 		}
 	}
-	err = s.checkEnvironmentVariables(project, options)
+	err = s.checkEnvironmentVariables(ctx, project, options)
 	if err != nil {
 		return false, err
 	}
 	return true, nil
 }
 
-func (s *composeService) checkEnvironmentVariables(project *types.Project, options api.PublishOptions) error {
-	errorList := map[string][]string{}
+// envCheckFindings groups everything checkEnvironmentVariables surfaces to
+// the user during publish pre-checks for env-related leak risks.
+type envCheckFindings struct {
+	// services maps service name -> findings for that service. Only services
+	// with at least one finding are present.
+	services map[string]*serviceEnvFindings
+	// configsLiteralContent lists configs whose inline `content:` is a literal
+	// (not interpolation). Sorted alphabetically. config.content is decoupled
+	// from --with-env because the flag is documented as controlling environment
+	// variable publishing only.
+	configsLiteralContent []string
+}
 
-	for _, service := range project.Services {
-		if len(service.EnvFiles) > 0 {
-			errorList[service.Name] = append(errorList[service.Name], fmt.Sprintf("service %q has env_file declared.", service.Name))
+type serviceEnvFindings struct {
+	hasEnvFile bool
+	// suspiciousKeys is the set of environment variable names whose literal
+	// values look sensitive, as classified by the upstream DefangLabs keyword
+	// detector (password, secret, token, api_key, …). A set is used because
+	// the same service may be visited across multiple compose files during
+	// the extends walk; callers convert to a sorted slice via sortedKeys
+	// when surfacing to the user.
+	suspiciousKeys map[string]struct{}
+}
+
+// sortedSuspiciousKeys returns the suspicious env var names alphabetically
+// sorted for stable output.
+func (f *serviceEnvFindings) sortedSuspiciousKeys() []string {
+	return sortedMapKeys(f.suspiciousKeys)
+}
+
+func sortedMapKeys[V any](m map[string]V) []string {
+	keys := make([]string, 0, len(m))
+	for k := range m {
+		keys = append(keys, k)
+	}
+	slices.Sort(keys)
+	return keys
+}
+
+func (f *envCheckFindings) hasEnvFinding() bool {
+	for _, svc := range f.services {
+		if svc.hasEnvFile || len(svc.suspiciousKeys) > 0 {
+			return true
 		}
 	}
+	return false
+}
 
-	if !options.WithEnvironment && len(errorList) > 0 {
-		errorMsgSuffix := "To avoid leaking sensitive data, you must either explicitly allow the sending of environment variables by using the --with-env flag,\n" +
-			"or remove sensitive data from your Compose configuration"
-		var errorMsg strings.Builder
-		for _, errors := range errorList {
-			for _, err := range errors {
-				fmt.Fprintf(&errorMsg, "%s\n", err)
-			}
+// checkEnvironmentVariables walks every compose file that will be serialized
+// into the OCI artifact (the top-level files plus any local extends parents)
+// and prompts the user to confirm before publishing:
+//
+//  1. service env_file declarations and literal environment values whose key
+//     name looks sensitive (password, secret, token, api_key, …) — silenced
+//     by --with-env;
+//  2. literal inline config.content — always prompts (decoupled from
+//     --with-env, which is documented to cover env vars only).
+//
+// Interpolated values like "${SECRET}" or "$VAR" are preserved as placeholders
+// in the published YAML and don't leak the resolved value; the keyword
+// detector's value regex skips them automatically.
+func (s *composeService) checkEnvironmentVariables(ctx context.Context, project *types.Project, options api.PublishOptions) error {
+	if len(project.ComposeFiles) == 0 {
+		return nil
+	}
+
+	findings, err := collectEnvCheckFindings(ctx, project)
+	if err != nil {
+		return err
+	}
+
+	if !options.WithEnvironment && findings.hasEnvFinding() {
+		if err := s.confirmOrCancel(buildEnvPromptMessage(findings.services)); err != nil {
+			return err
 		}
-		return fmt.Errorf("%s%s", errorMsg.String(), errorMsgSuffix)
+	}
 
+	if len(findings.configsLiteralContent) > 0 {
+		if err := s.confirmOrCancel(buildConfigContentPromptMessage(findings.configsLiteralContent)); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
+// confirmOrCancel runs an interactive yes/no prompt and returns:
+//   - the prompt's error verbatim, if it failed;
+//   - api.ErrCanceled if the user declined;
+//   - nil if the user accepted.
+func (s *composeService) confirmOrCancel(message string) error {
+	confirm, err := s.prompt(message, false)
+	if err != nil {
+		return err
+	}
+	if !confirm {
+		return api.ErrCanceled
 	}
 	return nil
 }
 
+// collectEnvCheckFindings walks every compose file scheduled for publication
+// (top-level files plus any local extends parents discovered along the way)
+// and aggregates per-service and per-config findings. The walk mirrors
+// processExtends so coverage matches what is actually serialized into the OCI
+// artifact.
+func collectEnvCheckFindings(ctx context.Context, project *types.Project) (*envCheckFindings, error) {
+	findings := &envCheckFindings{services: map[string]*serviceEnvFindings{}}
+	literalCfgs := map[string]struct{}{}
+	keywordDetector := keyword.NewDetector("0")
+
+	seen := map[string]struct{}{}
+	queue := slices.Clone(project.ComposeFiles)
+	for len(queue) > 0 {
+		file := queue[0]
+		queue = queue[1:]
+		if _, ok := seen[file]; ok {
+			continue
+		}
+		seen[file] = struct{}{}
+
+		unresolved, err := loadUnresolvedFile(ctx, project, file)
+		if err != nil {
+			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 != "" {
+				queue = append(queue, parent)
+			}
+		}
+		for name, config := range unresolved.Configs {
+			// config.Environment is a variable *name* (only the name is
+			// published, not its resolved value) so it is not a leak. Inline
+			// config.Content is what ends up in the artifact. compose-go
+			// enforces that file, environment, and content are mutually
+			// exclusive. The map key is the name as written in the compose
+			// file; config.Name is the project-namespaced version, which is
+			// less helpful when surfaced to the user.
+			if config.Content != "" && configContentLooksLiteral(config.Content, keywordDetector) {
+				literalCfgs[name] = struct{}{}
+			}
+		}
+	}
+
+	if len(literalCfgs) > 0 {
+		findings.configsLiteralContent = sortedMapKeys(literalCfgs)
+	}
+	return findings, nil
+}
+
+func recordServiceEnvFindings(services map[string]*serviceEnvFindings, detector secrets.Detector, service types.ServiceConfig) {
+	envValues := map[string]string{}
+	for key, value := range service.Environment {
+		if value == nil {
+			continue
+		}
+		envValues[key] = replaceDollarEscape(*value)
+	}
+
+	hits, _ := detector.ScanMap(envValues)
+	if len(hits) == 0 && len(service.EnvFiles) == 0 {
+		return
+	}
+
+	f := services[service.Name]
+	if f == nil {
+		f = &serviceEnvFindings{suspiciousKeys: map[string]struct{}{}}
+		services[service.Name] = f
+	}
+	if len(service.EnvFiles) > 0 {
+		f.hasEnvFile = true
+	}
+	for _, hit := range hits {
+		f.suspiciousKeys[hit.Key] = struct{}{}
+	}
+}
+
+// configContentLooksLiteral returns true when the inline config.content has
+// a literal portion that would be published as-is, leaking the value to
+// consumers of the OCI artifact.
+//
+// We piggyback on the keyword detector's value regex (`[^${\s].+[^}\s]`) by
+// passing a fake "password" key to ScanMap — the regex isn't exported
+// directly, only via the key+value match path. The regex excludes values
+// starting with `$` (`${VAR}`/`$VAR` interpolation), ending with `}`
+// (templates like `key=${SECRET}`), or shorter than 3 chars, which neatly
+// matches our notion of "looks like a template, not a literal".
+func configContentLooksLiteral(content string, detector secrets.Detector) bool {
+	hits, _ := detector.ScanMap(map[string]string{"password": replaceDollarEscape(content)})
+	return len(hits) > 0
+}
+
+// replaceDollarEscape substitutes the compose-spec `$$` escape (which
+// represents a literal `$` in the resolved value) with a placeholder. The
+// placeholder is `X` rather than `$` because the keyword detector's value
+// regex excludes any value beginning with `$`; using `$` would mask the
+// literal we're trying to flag. Any non-special char would do — we picked
+// `X` for readability.
+func replaceDollarEscape(value string) string {
+	return strings.ReplaceAll(value, "$$", "X")
+}
+
+// localExtendsParent returns the path of an extends parent file that exists on
+// disk, or "" when the service does not extend or extends a remote resource.
+func localExtendsParent(service types.ServiceConfig) string {
+	if service.Extends == nil || service.Extends.File == "" {
+		return ""
+	}
+	if _, err := os.Stat(service.Extends.File); err != nil {
+		return ""
+	}
+	return service.Extends.File
+}
+
+func buildEnvPromptMessage(services map[string]*serviceEnvFindings) string {
+	var b strings.Builder
+	b.WriteString("you are about to publish env-related declarations within your OCI artifact.\n")
+	b.WriteString("env_file paths and literal values for sensitive-looking keys are embedded as-is in the published YAML;\n")
+	b.WriteString("interpolated values like \"${VAR}\" are kept symbolic and have already been excluded.\n")
+	for _, name := range sortedMapKeys(services) {
+		f := services[name]
+		if f.hasEnvFile {
+			fmt.Fprintf(&b, "  service %q: env_file declared\n", name)
+		}
+		if keys := f.sortedSuspiciousKeys(); len(keys) > 0 {
+			quoted := make([]string, len(keys))
+			for i, k := range keys {
+				quoted[i] = fmt.Sprintf("%q", k)
+			}
+			fmt.Fprintf(&b, "  service %q: literal value for %s\n", name, strings.Join(quoted, ", "))
+		}
+	}
+	b.WriteString("Use --with-env to silence this prompt and always publish env declarations.\n")
+	b.WriteString("Are you ok to publish these env declarations?")
+	return b.String()
+}
+
+func buildConfigContentPromptMessage(configs []string) string {
+	var b strings.Builder
+	b.WriteString("you are about to publish literal inline config content within your OCI artifact.\n")
+	for _, name := range configs {
+		fmt.Fprintf(&b, "  config %q\n", name)
+	}
+	b.WriteString("Are you ok to publish these config contents?")
+	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{
+		WorkingDir:  project.WorkingDir,
+		Environment: project.Environment,
+		ConfigFiles: []types.ConfigFile{{Filename: filePath}},
+	}, func(options *loader.Options) {
+		options.SkipValidation = true
+		options.SkipExtends = true
+		options.SkipConsistencyCheck = true
+		options.ResolvePaths = true
+		// SkipInclude mirrors processFile: include directives stay symbolic in
+		// the published artifact, so included content must not be inspected
+		// here either (otherwise we'd flag literals that never ship).
+		options.SkipInclude = true
+		options.SkipInterpolation = true
+		options.SkipResolveEnvironment = true
+		options.Profiles = project.Profiles
+	})
+}
+
 func envFileLayers(files map[string]string) []v1.Descriptor {
 	var layers []v1.Descriptor
 	for file, hash := range files {
@@ -473,31 +723,10 @@ func (s *composeService) checkForSensitiveData(ctx context.Context, project *typ
 }
 
 func composeFileAsByteReader(ctx context.Context, filePath string, project *types.Project) (io.Reader, error) {
-	composeFile, err := os.ReadFile(filePath)
+	base, err := loadUnresolvedFile(ctx, project, filePath)
 	if err != nil {
-		return nil, fmt.Errorf("failed to open compose file %s: %w", filePath, err)
+		return nil, fmt.Errorf("failed to load compose file %s: %w", filePath, err)
 	}
-	base, err := loader.LoadWithContext(ctx, types.ConfigDetails{
-		WorkingDir:  project.WorkingDir,
-		Environment: project.Environment,
-		ConfigFiles: []types.ConfigFile{
-			{
-				Filename: filePath,
-				Content:  composeFile,
-			},
-		},
-	}, func(options *loader.Options) {
-		options.SkipValidation = true
-		options.SkipExtends = true
-		options.SkipConsistencyCheck = true
-		options.ResolvePaths = true
-		options.SkipInterpolation = true
-		options.SkipResolveEnvironment = true
-	})
-	if err != nil {
-		return nil, err
-	}
-
 	in, err := base.MarshalYAML()
 	if err != nil {
 		return nil, err