Skip to content

Commit 056aaac

Browse files
gloursclaude
andcommitted
fix(publish): prompt on sensitive-looking env literals
Replace the previous "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 an interactive prompt — matching the existing checkForBindMount / checkForSensitiveData UX. --with-env continues to silence the env prompt; literal config.content gets its own independent prompt (decoupled from --with-env, which is documented to cover env vars only). The previous strict check produced false positives on common benign vars like LOG_LEVEL=info or DB_USER=postgres, blocking the 99% case while still missing low-entropy real secrets that the existing secret-detector skips (MYSQL_ROOT_PASSWORD=toto slips through because "toto" has Shannon entropy ~1.5, well below the keyword detector's default threshold of 4). Scoping the new check to suspicious key names with no entropy floor: - catches literal weak passwords on sensitive keys (MYSQL_ROOT_PASSWORD=toto, DB_PASSWORD=changeme) - leaves benign config silent (LOG_LEVEL=info) - leaves interpolation silent (${VAR}, $VAR) - flags compose-spec $$ escapes that the upstream value regex would otherwise skip (pa$$word resolves to pa$word) Demo authors publishing examples with changeme-style placeholders see one prompt and confirm — they are no longer blocked behind --with-env. 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>
1 parent 60584e7 commit 056aaac

6 files changed

Lines changed: 745 additions & 54 deletions

File tree

pkg/compose/publish.go

Lines changed: 267 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,10 @@ import (
2525
"fmt"
2626
"io"
2727
"os"
28+
"slices"
2829
"strings"
2930

31+
"github.com/DefangLabs/secret-detector/pkg/detectors/keyword"
3032
"github.com/DefangLabs/secret-detector/pkg/scanner"
3133
"github.com/DefangLabs/secret-detector/pkg/secrets"
3234
"github.com/compose-spec/compose-go/v2/loader"
@@ -341,37 +343,285 @@ func (s *composeService) preChecks(ctx context.Context, project *types.Project,
341343
return false, err
342344
}
343345
}
344-
err = s.checkEnvironmentVariables(project, options)
346+
err = s.checkEnvironmentVariables(ctx, project, options)
345347
if err != nil {
346348
return false, err
347349
}
348350
return true, nil
349351
}
350352

351-
func (s *composeService) checkEnvironmentVariables(project *types.Project, options api.PublishOptions) error {
352-
errorList := map[string][]string{}
353+
// envCheckFindings groups everything checkEnvironmentVariables surfaces to
354+
// the user during publish pre-checks for env-related leak risks.
355+
type envCheckFindings struct {
356+
// services maps service name -> findings for that service. Only services
357+
// with at least one finding are present.
358+
services map[string]*serviceEnvFindings
359+
// configsLiteralContent lists configs whose inline `content:` is a literal
360+
// (not interpolation). Sorted alphabetically. config.content is decoupled
361+
// from --with-env because the flag is documented as controlling environment
362+
// variable publishing only.
363+
configsLiteralContent []string
364+
}
353365

354-
for _, service := range project.Services {
355-
if len(service.EnvFiles) > 0 {
356-
errorList[service.Name] = append(errorList[service.Name], fmt.Sprintf("service %q has env_file declared.", service.Name))
366+
type serviceEnvFindings struct {
367+
hasEnvFile bool
368+
// suspiciousKeys is the set of environment variable names whose literal
369+
// values look sensitive, as classified by the upstream DefangLabs keyword
370+
// detector (password, secret, token, api_key, …). A set is used because
371+
// the same service may be visited across multiple compose files during
372+
// the extends walk; callers convert to a sorted slice via sortedKeys
373+
// when surfacing to the user.
374+
suspiciousKeys map[string]struct{}
375+
}
376+
377+
// sortedSuspiciousKeys returns the suspicious env var names alphabetically
378+
// sorted for stable output.
379+
func (f *serviceEnvFindings) sortedSuspiciousKeys() []string {
380+
return sortedMapKeys(f.suspiciousKeys)
381+
}
382+
383+
func sortedMapKeys[V any](m map[string]V) []string {
384+
keys := make([]string, 0, len(m))
385+
for k := range m {
386+
keys = append(keys, k)
387+
}
388+
slices.Sort(keys)
389+
return keys
390+
}
391+
392+
func (f *envCheckFindings) hasEnvFinding() bool {
393+
for _, svc := range f.services {
394+
if svc.hasEnvFile || len(svc.suspiciousKeys) > 0 {
395+
return true
357396
}
358397
}
398+
return false
399+
}
359400

360-
if !options.WithEnvironment && len(errorList) > 0 {
361-
errorMsgSuffix := "To avoid leaking sensitive data, you must either explicitly allow the sending of environment variables by using the --with-env flag,\n" +
362-
"or remove sensitive data from your Compose configuration"
363-
var errorMsg strings.Builder
364-
for _, errors := range errorList {
365-
for _, err := range errors {
366-
fmt.Fprintf(&errorMsg, "%s\n", err)
367-
}
401+
// checkEnvironmentVariables walks every compose file that will be serialized
402+
// into the OCI artifact (the top-level files plus any local extends parents)
403+
// and prompts the user to confirm before publishing:
404+
//
405+
// 1. service env_file declarations and literal environment values whose key
406+
// name looks sensitive (password, secret, token, api_key, …) — silenced
407+
// by --with-env;
408+
// 2. literal inline config.content — always prompts (decoupled from
409+
// --with-env, which is documented to cover env vars only).
410+
//
411+
// Interpolated values like "${SECRET}" or "$VAR" are preserved as placeholders
412+
// in the published YAML and don't leak the resolved value; the keyword
413+
// detector's value regex skips them automatically.
414+
func (s *composeService) checkEnvironmentVariables(ctx context.Context, project *types.Project, options api.PublishOptions) error {
415+
if len(project.ComposeFiles) == 0 {
416+
return nil
417+
}
418+
419+
findings, err := collectEnvCheckFindings(ctx, project)
420+
if err != nil {
421+
return err
422+
}
423+
424+
if !options.WithEnvironment && findings.hasEnvFinding() {
425+
if err := s.confirmOrCancel(buildEnvPromptMessage(findings.services)); err != nil {
426+
return err
368427
}
369-
return fmt.Errorf("%s%s", errorMsg.String(), errorMsgSuffix)
428+
}
370429

430+
if len(findings.configsLiteralContent) > 0 {
431+
if err := s.confirmOrCancel(buildConfigContentPromptMessage(findings.configsLiteralContent)); err != nil {
432+
return err
433+
}
434+
}
435+
436+
return nil
437+
}
438+
439+
// confirmOrCancel runs an interactive yes/no prompt and returns:
440+
// - the prompt's error verbatim, if it failed;
441+
// - api.ErrCanceled if the user declined;
442+
// - nil if the user accepted.
443+
func (s *composeService) confirmOrCancel(message string) error {
444+
confirm, err := s.prompt(message, false)
445+
if err != nil {
446+
return err
447+
}
448+
if !confirm {
449+
return api.ErrCanceled
371450
}
372451
return nil
373452
}
374453

454+
// collectEnvCheckFindings walks every compose file scheduled for publication
455+
// (top-level files plus any local extends parents discovered along the way)
456+
// and aggregates per-service and per-config findings. The walk mirrors
457+
// processExtends so coverage matches what is actually serialized into the OCI
458+
// artifact.
459+
func collectEnvCheckFindings(ctx context.Context, project *types.Project) (*envCheckFindings, error) {
460+
findings := &envCheckFindings{services: map[string]*serviceEnvFindings{}}
461+
literalCfgs := map[string]struct{}{}
462+
keywordDetector := keyword.NewDetector("0")
463+
464+
seen := map[string]struct{}{}
465+
queue := slices.Clone(project.ComposeFiles)
466+
for len(queue) > 0 {
467+
file := queue[0]
468+
queue = queue[1:]
469+
if _, ok := seen[file]; ok {
470+
continue
471+
}
472+
seen[file] = struct{}{}
473+
474+
unresolved, err := loadUnresolvedFile(ctx, project, file)
475+
if err != nil {
476+
return nil, fmt.Errorf("failed to load compose file %s: %w", file, err)
477+
}
478+
479+
for _, service := range unresolved.Services {
480+
recordServiceEnvFindings(findings.services, keywordDetector, service)
481+
if parent := localExtendsParent(service); parent != "" {
482+
queue = append(queue, parent)
483+
}
484+
}
485+
for name, config := range unresolved.Configs {
486+
// config.Environment is a variable *name* (only the name is
487+
// published, not its resolved value) so it is not a leak. Inline
488+
// config.Content is what ends up in the artifact. compose-go
489+
// enforces that file, environment, and content are mutually
490+
// exclusive. The map key is the name as written in the compose
491+
// file; config.Name is the project-namespaced version, which is
492+
// less helpful when surfaced to the user.
493+
if config.Content != "" && configContentLooksLiteral(config.Content, keywordDetector) {
494+
literalCfgs[name] = struct{}{}
495+
}
496+
}
497+
}
498+
499+
if len(literalCfgs) > 0 {
500+
findings.configsLiteralContent = sortedMapKeys(literalCfgs)
501+
}
502+
return findings, nil
503+
}
504+
505+
func recordServiceEnvFindings(services map[string]*serviceEnvFindings, detector secrets.Detector, service types.ServiceConfig) {
506+
envValues := map[string]string{}
507+
for key, value := range service.Environment {
508+
if value == nil {
509+
continue
510+
}
511+
envValues[key] = replaceDollarEscape(*value)
512+
}
513+
514+
hits, _ := detector.ScanMap(envValues)
515+
if len(hits) == 0 && len(service.EnvFiles) == 0 {
516+
return
517+
}
518+
519+
f := services[service.Name]
520+
if f == nil {
521+
f = &serviceEnvFindings{suspiciousKeys: map[string]struct{}{}}
522+
services[service.Name] = f
523+
}
524+
if len(service.EnvFiles) > 0 {
525+
f.hasEnvFile = true
526+
}
527+
for _, hit := range hits {
528+
f.suspiciousKeys[hit.Key] = struct{}{}
529+
}
530+
}
531+
532+
// configContentLooksLiteral returns true when the inline config.content has
533+
// a literal portion that would be published as-is, leaking the value to
534+
// consumers of the OCI artifact.
535+
//
536+
// We piggyback on the keyword detector's value regex (`[^${\s].+[^}\s]`) by
537+
// passing a fake "password" key to ScanMap — the regex isn't exported
538+
// directly, only via the key+value match path. The regex excludes values
539+
// starting with `$` (`${VAR}`/`$VAR` interpolation), ending with `}`
540+
// (templates like `key=${SECRET}`), or shorter than 3 chars, which neatly
541+
// matches our notion of "looks like a template, not a literal".
542+
func configContentLooksLiteral(content string, detector secrets.Detector) bool {
543+
hits, _ := detector.ScanMap(map[string]string{"password": replaceDollarEscape(content)})
544+
return len(hits) > 0
545+
}
546+
547+
// replaceDollarEscape substitutes the compose-spec `$$` escape (which
548+
// represents a literal `$` in the resolved value) with a placeholder. The
549+
// placeholder is `X` rather than `$` because the keyword detector's value
550+
// regex excludes any value beginning with `$`; using `$` would mask the
551+
// literal we're trying to flag. Any non-special char would do — we picked
552+
// `X` for readability.
553+
func replaceDollarEscape(value string) string {
554+
return strings.ReplaceAll(value, "$$", "X")
555+
}
556+
557+
// localExtendsParent returns the path of an extends parent file that exists on
558+
// disk, or "" when the service does not extend or extends a remote resource.
559+
func localExtendsParent(service types.ServiceConfig) string {
560+
if service.Extends == nil || service.Extends.File == "" {
561+
return ""
562+
}
563+
if _, err := os.Stat(service.Extends.File); err != nil {
564+
return ""
565+
}
566+
return service.Extends.File
567+
}
568+
569+
func buildEnvPromptMessage(services map[string]*serviceEnvFindings) string {
570+
var b strings.Builder
571+
b.WriteString("you are about to publish env-related declarations within your OCI artifact.\n")
572+
b.WriteString("env_file paths and literal values for sensitive-looking keys are embedded as-is in the published YAML;\n")
573+
b.WriteString("interpolated values like \"${VAR}\" are kept symbolic and have already been excluded.\n")
574+
for _, name := range sortedMapKeys(services) {
575+
f := services[name]
576+
if f.hasEnvFile {
577+
fmt.Fprintf(&b, " service %q: env_file declared\n", name)
578+
}
579+
if keys := f.sortedSuspiciousKeys(); len(keys) > 0 {
580+
quoted := make([]string, len(keys))
581+
for i, k := range keys {
582+
quoted[i] = fmt.Sprintf("%q", k)
583+
}
584+
fmt.Fprintf(&b, " service %q: literal value for %s\n", name, strings.Join(quoted, ", "))
585+
}
586+
}
587+
b.WriteString("Use --with-env to silence this prompt and always publish env declarations.\n")
588+
b.WriteString("Are you ok to publish these env declarations?")
589+
return b.String()
590+
}
591+
592+
func buildConfigContentPromptMessage(configs []string) string {
593+
var b strings.Builder
594+
b.WriteString("you are about to publish literal inline config content within your OCI artifact.\n")
595+
for _, name := range configs {
596+
fmt.Fprintf(&b, " config %q\n", name)
597+
}
598+
b.WriteString("Are you ok to publish these config contents?")
599+
return b.String()
600+
}
601+
602+
// loadUnresolvedFile loads a single compose file with interpolation and
603+
// environment resolution skipped, so callers can inspect raw user-provided
604+
// values. Used by both checkEnvironmentVariables and composeFileAsByteReader.
605+
func loadUnresolvedFile(ctx context.Context, project *types.Project, filePath string) (*types.Project, error) {
606+
return loader.LoadWithContext(ctx, types.ConfigDetails{
607+
WorkingDir: project.WorkingDir,
608+
Environment: project.Environment,
609+
ConfigFiles: []types.ConfigFile{{Filename: filePath}},
610+
}, func(options *loader.Options) {
611+
options.SkipValidation = true
612+
options.SkipExtends = true
613+
options.SkipConsistencyCheck = true
614+
options.ResolvePaths = true
615+
// SkipInclude mirrors processFile: include directives stay symbolic in
616+
// the published artifact, so included content must not be inspected
617+
// here either (otherwise we'd flag literals that never ship).
618+
options.SkipInclude = true
619+
options.SkipInterpolation = true
620+
options.SkipResolveEnvironment = true
621+
options.Profiles = project.Profiles
622+
})
623+
}
624+
375625
func envFileLayers(files map[string]string) []v1.Descriptor {
376626
var layers []v1.Descriptor
377627
for file, hash := range files {
@@ -473,31 +723,10 @@ func (s *composeService) checkForSensitiveData(ctx context.Context, project *typ
473723
}
474724

475725
func composeFileAsByteReader(ctx context.Context, filePath string, project *types.Project) (io.Reader, error) {
476-
composeFile, err := os.ReadFile(filePath)
726+
base, err := loadUnresolvedFile(ctx, project, filePath)
477727
if err != nil {
478-
return nil, fmt.Errorf("failed to open compose file %s: %w", filePath, err)
728+
return nil, fmt.Errorf("failed to load compose file %s: %w", filePath, err)
479729
}
480-
base, err := loader.LoadWithContext(ctx, types.ConfigDetails{
481-
WorkingDir: project.WorkingDir,
482-
Environment: project.Environment,
483-
ConfigFiles: []types.ConfigFile{
484-
{
485-
Filename: filePath,
486-
Content: composeFile,
487-
},
488-
},
489-
}, func(options *loader.Options) {
490-
options.SkipValidation = true
491-
options.SkipExtends = true
492-
options.SkipConsistencyCheck = true
493-
options.ResolvePaths = true
494-
options.SkipInterpolation = true
495-
options.SkipResolveEnvironment = true
496-
})
497-
if err != nil {
498-
return nil, err
499-
}
500-
501730
in, err := base.MarshalYAML()
502731
if err != nil {
503732
return nil, err

0 commit comments

Comments
 (0)