From 047bcff19d3ac5795d9ad7f86143b8476729c0ff Mon Sep 17 00:00:00 2001 From: Dan Luhring Date: Tue, 12 Nov 2024 19:52:11 -0500 Subject: [PATCH] fix(adv): unchecked package repo URL During distro auto-detection. Also a general improvement to error handling for distro param values. Signed-off-by: Dan Luhring --- pkg/cli/advisory_create.go | 42 +++++++++++++++++++++++++++++++++----- pkg/cli/advisory_update.go | 42 +++++++++++++++++++++++++++++++++----- 2 files changed, 74 insertions(+), 10 deletions(-) diff --git a/pkg/cli/advisory_create.go b/pkg/cli/advisory_create.go index 681f7911..f6c20705 100644 --- a/pkg/cli/advisory_create.go +++ b/pkg/cli/advisory_create.go @@ -5,6 +5,7 @@ import ( "fmt" "net/http" "os" + "strings" "chainguard.dev/apko/pkg/apk/apk" "chainguard.dev/apko/pkg/apk/client" @@ -48,20 +49,45 @@ newly created advisory and any other advisories for the same package.`, SilenceErrors: true, Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, _ []string) error { + // Get initial values for distro-related parameters. archs := p.archs packageRepositoryURL := p.packageRepositoryURL distroRepoDir := resolveDistroDir(p.distroRepoDir) advisoriesRepoDir := resolveAdvisoriesDirInput(p.advisoriesRepoDir) - if distroRepoDir == "" || advisoriesRepoDir == "" { - if p.doNotDetectDistro { - return fmt.Errorf("distro repo dir and/or advisories repo dir was left unspecified") + + // If we're not using auto-detection, we need to fail fast on any missing + // parameter values. + if p.doNotDetectDistro { + var missingParamWarnings []string + + if distroRepoDir == "" { + missingParamWarnings = append(missingParamWarnings, fmt.Sprintf(" - distro repository directory (specify using --%s)", flagNameDistroRepoDir)) } + if advisoriesRepoDir == "" { + missingParamWarnings = append(missingParamWarnings, fmt.Sprintf(" - advisories repository directory (specify using --%s)", flagNameAdvisoriesRepoDir)) + } + if packageRepositoryURL == "" { + missingParamWarnings = append(missingParamWarnings, fmt.Sprintf(" - package repository URL (specify using --%s)", flagNamePackageRepoURL)) + } + if len(missingParamWarnings) > 0 { + return fmt.Errorf( + "one or more distro configuration values was left unspecified and couldn't be automatically resolved because distro auto-detection was disabled by user:\n%v", + strings.Join(missingParamWarnings, "\n"), + ) + } + } + // If we made it this far, we either have all the values, or we'll be able to + // use auto-detection to resolve the rest. + + if distroRepoDir == "" || advisoriesRepoDir == "" || packageRepositoryURL == "" { d, err := distro.Detect() if err != nil { return fmt.Errorf("distro repo dir and/or advisories repo dir was left unspecified, and distro auto-detection failed: %w", err) } + // Replace only the values that are still empty. + if len(archs) == 0 { archs = d.Absolute.SupportedArchitectures } @@ -70,8 +96,14 @@ newly created advisory and any other advisories for the same package.`, packageRepositoryURL = d.Absolute.APKRepositoryURL } - distroRepoDir = d.Local.PackagesRepo.Dir - advisoriesRepoDir = d.Local.AdvisoriesRepo.Dir + if distroRepoDir == "" { + distroRepoDir = d.Local.PackagesRepo.Dir + } + + if advisoriesRepoDir == "" { + advisoriesRepoDir = d.Local.AdvisoriesRepo.Dir + } + _, _ = fmt.Fprint(os.Stderr, renderDetectedDistro(d)) } diff --git a/pkg/cli/advisory_update.go b/pkg/cli/advisory_update.go index c35df70e..95d35db9 100644 --- a/pkg/cli/advisory_update.go +++ b/pkg/cli/advisory_update.go @@ -5,6 +5,7 @@ import ( "net/http" "os" "sort" + "strings" "chainguard.dev/apko/pkg/apk/apk" "chainguard.dev/apko/pkg/apk/client" @@ -44,20 +45,45 @@ required fields are missing.`, SilenceErrors: true, Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, _ []string) error { + // Get initial values for distro-related parameters. archs := p.archs packageRepositoryURL := p.packageRepositoryURL distroRepoDir := resolveDistroDir(p.distroRepoDir) advisoriesRepoDir := resolveAdvisoriesDirInput(p.advisoriesRepoDir) - if distroRepoDir == "" || advisoriesRepoDir == "" { - if p.doNotDetectDistro { - return fmt.Errorf("distro repo dir and/or advisories repo dir was left unspecified") + + // If we're not using auto-detection, we need to fail fast on any missing + // parameter values. + if p.doNotDetectDistro { + var missingParamWarnings []string + + if distroRepoDir == "" { + missingParamWarnings = append(missingParamWarnings, fmt.Sprintf(" - distro repository directory (specify using --%s)", flagNameDistroRepoDir)) } + if advisoriesRepoDir == "" { + missingParamWarnings = append(missingParamWarnings, fmt.Sprintf(" - advisories repository directory (specify using --%s)", flagNameAdvisoriesRepoDir)) + } + if packageRepositoryURL == "" { + missingParamWarnings = append(missingParamWarnings, fmt.Sprintf(" - package repository URL (specify using --%s)", flagNamePackageRepoURL)) + } + if len(missingParamWarnings) > 0 { + return fmt.Errorf( + "one or more distro configuration values was left unspecified and couldn't be automatically resolved because distro auto-detection was disabled by user:\n%v", + strings.Join(missingParamWarnings, "\n"), + ) + } + } + // If we made it this far, we either have all the values, or we'll be able to + // use auto-detection to resolve the rest. + + if distroRepoDir == "" || advisoriesRepoDir == "" || packageRepositoryURL == "" { d, err := distro.Detect() if err != nil { return fmt.Errorf("distro repo dir and/or advisories repo dir was left unspecified, and distro auto-detection failed: %w", err) } + // Replace only the values that are still empty. + if len(archs) == 0 { archs = d.Absolute.SupportedArchitectures } @@ -66,8 +92,14 @@ required fields are missing.`, packageRepositoryURL = d.Absolute.APKRepositoryURL } - distroRepoDir = d.Local.PackagesRepo.Dir - advisoriesRepoDir = d.Local.AdvisoriesRepo.Dir + if distroRepoDir == "" { + distroRepoDir = d.Local.PackagesRepo.Dir + } + + if advisoriesRepoDir == "" { + advisoriesRepoDir = d.Local.AdvisoriesRepo.Dir + } + _, _ = fmt.Fprint(os.Stderr, renderDetectedDistro(d)) }