From 0da471bc04e6f4e1f10887abf7e21db89a0641d7 Mon Sep 17 00:00:00 2001 From: Tim Newsham Date: Wed, 10 Jul 2024 10:51:08 -1000 Subject: [PATCH 01/26] Add flyctl support for scanning images with scantron --- go.mod | 2 +- go.sum | 4 +- internal/buildinfo/buildinfo.go | 3 +- internal/command/root/root.go | 2 + internal/command/scan/auth.go | 92 +++++++++++++++++ internal/command/scan/command.go | 92 +++++++++++++++++ internal/command/scan/sbom.go | 93 +++++++++++++++++ internal/command/scan/scantron.go | 52 ++++++++++ internal/command/scan/vulns.go | 165 ++++++++++++++++++++++++++++++ 9 files changed, 500 insertions(+), 5 deletions(-) create mode 100644 internal/command/scan/auth.go create mode 100644 internal/command/scan/command.go create mode 100644 internal/command/scan/sbom.go create mode 100644 internal/command/scan/scantron.go create mode 100644 internal/command/scan/vulns.go diff --git a/go.mod b/go.mod index e03dce5fc8..29ba3113fa 100644 --- a/go.mod +++ b/go.mod @@ -67,7 +67,7 @@ require ( github.com/superfly/fly-go v0.1.19-0.20240702095246-59db1fe4ffe8 github.com/superfly/graphql v0.2.4 github.com/superfly/lfsc-go v0.1.1 - github.com/superfly/macaroon v0.2.13 + github.com/superfly/macaroon v0.2.14-0.20240702184853-b8ac52a1fc77 github.com/superfly/tokenizer v0.0.2 github.com/vektah/gqlparser v1.3.1 github.com/vektah/gqlparser/v2 v2.5.16 diff --git a/go.sum b/go.sum index 37f9ca3b5f..357bac2d32 100644 --- a/go.sum +++ b/go.sum @@ -615,8 +615,8 @@ github.com/superfly/lfsc-go v0.1.1 h1:dGjLgt81D09cG+aR9lJZIdmonjZSR5zYCi7s54+ZU2 github.com/superfly/lfsc-go v0.1.1/go.mod h1:zVb0VENz/Il8Nmvvd4XAsX2bWhQ+sr0nK8vv9PeezcE= github.com/superfly/ltx v0.3.12 h1:Z7z1sc4g34/jUi3XO84+zBlIsbaoh2RJ3b4zTQpBK/M= github.com/superfly/ltx v0.3.12/go.mod h1:ly+Dq7UVacQVEI5/b0r6j+PSNy9ibwx1yikcWAaSkhE= -github.com/superfly/macaroon v0.2.13 h1:WEZnifapjW5yuCEsdZxqCq4X8xuzIf+AUVPv6lm7GtI= -github.com/superfly/macaroon v0.2.13/go.mod h1:Kt6/EdSYfFjR4GIe+erMwcJgU8iMu1noYVceQ5dNdKo= +github.com/superfly/macaroon v0.2.14-0.20240702184853-b8ac52a1fc77 h1:W5LHJ6jjIB8YxOzsA5ZfdvNHifUKN/mFLiMaCAaWyB8= +github.com/superfly/macaroon v0.2.14-0.20240702184853-b8ac52a1fc77/go.mod h1:Kt6/EdSYfFjR4GIe+erMwcJgU8iMu1noYVceQ5dNdKo= github.com/superfly/tokenizer v0.0.2 h1:gBREpm08sPWUHsqKHKHFy/AIKwrqpg+VBD/wtJ6x5Jk= github.com/superfly/tokenizer v0.0.2/go.mod h1:jO8bIWFsfcBghh7I6cFJHmJb5E3RQr0v4F0Rnev3Yj4= github.com/tj/assert v0.0.0-20171129193455-018094318fb0/go.mod h1:mZ9/Rh9oLWpLLDRpvE+3b7gP/C2YyLFYxNmcLnPTMe0= diff --git a/internal/buildinfo/buildinfo.go b/internal/buildinfo/buildinfo.go index 0a2b8ad257..9665921d6c 100644 --- a/internal/buildinfo/buildinfo.go +++ b/internal/buildinfo/buildinfo.go @@ -6,7 +6,6 @@ import ( "path/filepath" "runtime" "runtime/debug" - "strings" "time" "github.com/pkg/errors" @@ -135,5 +134,5 @@ func Commit() string { } func UserAgent() string { - return strings.TrimSpace(fmt.Sprintf("fly-cli/%s", Version())) + return fmt.Sprintf("flyctl/%s", Version()) } diff --git a/internal/command/root/root.go b/internal/command/root/root.go index 4e71742cf7..8e4002b5df 100644 --- a/internal/command/root/root.go +++ b/internal/command/root/root.go @@ -51,6 +51,7 @@ import ( "github.com/superfly/flyctl/internal/command/regions" "github.com/superfly/flyctl/internal/command/releases" "github.com/superfly/flyctl/internal/command/resume" + "github.com/superfly/flyctl/internal/command/scan" "github.com/superfly/flyctl/internal/command/scale" "github.com/superfly/flyctl/internal/command/secrets" "github.com/superfly/flyctl/internal/command/services" @@ -111,6 +112,7 @@ func New() *cobra.Command { group(proxy.New(), "upkeep"), group(postgres.New(), "dbs_and_extensions"), group(ips.New(), "configuring"), + group(scan.New(), "upkeep"), group(secrets.New(), "configuring"), group(ssh.New(), "upkeep"), group(ssh.NewSFTP(), "upkeep"), diff --git a/internal/command/scan/auth.go b/internal/command/scan/auth.go new file mode 100644 index 0000000000..84eb068d16 --- /dev/null +++ b/internal/command/scan/auth.go @@ -0,0 +1,92 @@ +package scan + +import ( + "context" + "fmt" + "net/http" + + fly "github.com/superfly/fly-go" + "github.com/superfly/macaroon" + "github.com/superfly/macaroon/flyio" + "github.com/superfly/macaroon/resset" + + "github.com/superfly/flyctl/gql" + "github.com/superfly/flyctl/internal/flyutil" +) + +const ( + appFeatureImages = "images" + orgFeatureBuilder = "builder" +) + +func addAuth(ctx context.Context, apiClient flyutil.Client, orgId, appId string, req *http.Request) error { + resp, err := makeToken(ctx, apiClient, "ScantronToken", orgId, "5m", "deploy", &gql.LimitedAccessTokenOptions{ + "app_id": appId, + }) + if err != nil { + return err + } + + token := resp.CreateLimitedAccessToken.LimitedAccessToken.TokenHeader + token, err = attenuateTokens(token, + &resset.IfPresent{ + Ifs: macaroon.NewCaveatSet( + &flyio.FeatureSet{Features: resset.New[string](resset.ActionRead, orgFeatureBuilder)}, + &flyio.AppFeatureSet{Features: resset.New[string](resset.ActionRead, appFeatureImages)}, + ), + Else: resset.ActionNone, + }, + ) + if err != nil { + return err + } + + req.Header.Set("Authorization", fly.AuthorizationHeader(token)) + return nil +} + +func makeToken(ctx context.Context, apiClient flyutil.Client, name, orgID, expiry, profile string, options *gql.LimitedAccessTokenOptions) (*gql.CreateLimitedAccessTokenResponse, error) { + resp, err := gql.CreateLimitedAccessToken( + ctx, + apiClient.GenqClient(), + name, + orgID, + profile, + options, + expiry, + ) + if err != nil { + return nil, fmt.Errorf("failed creating token: %w", err) + } + return resp, nil +} + +func attenuateTokens(tokenHeader string, caveats ...macaroon.Caveat) (string, error) { + toks, err := macaroon.Parse(tokenHeader) + if err != nil { + return "", fmt.Errorf("failed to parse token: %w", err) + } + + perms, _, _, retToks, err := macaroon.FindPermissionAndDischargeTokens(toks, flyio.LocationPermission) + switch { + case err != nil: + return "", fmt.Errorf("failed to find permission tokens: %w", err) + case len(perms) == 0: + return "", fmt.Errorf("no permission tokens found") + } + + for _, perm := range perms { + if err := perm.Add(caveats...); err != nil { + return "", fmt.Errorf("failed to attenuate token: %w", err) + } + + tok, err := perm.Encode() + if err != nil { + return "", fmt.Errorf("failed to encode token: %w", err) + } + + retToks = append(retToks, tok) + } + + return macaroon.ToAuthorizationHeader(retToks...), nil +} diff --git a/internal/command/scan/command.go b/internal/command/scan/command.go new file mode 100644 index 0000000000..4980a1f204 --- /dev/null +++ b/internal/command/scan/command.go @@ -0,0 +1,92 @@ +package scan + +import ( + "context" + "errors" + "fmt" + + "github.com/samber/lo" + "github.com/spf13/cobra" + + fly "github.com/superfly/fly-go" + "github.com/superfly/flyctl/internal/command" + "github.com/superfly/flyctl/internal/flag" + "github.com/superfly/flyctl/internal/flapsutil" + "github.com/superfly/flyctl/internal/prompt" +) + +func New() *cobra.Command { + const ( + usage = "scan" + short = "Scan machine images for vulnerabilities or to get an SBOM" + long = "Scan machine images for vulnerabilities or to get an SBOM." + ) + cmd := command.New(usage, short, long, nil) + + cmd.AddCommand( + newSbom(), + newVulns(), + ) + + return cmd +} + +func selectMachine(ctx context.Context, app *fly.AppCompact) (*fly.Machine, error) { + if flag.IsSpecified(ctx, "machine") { + if flag.IsSpecified(ctx, "select") { + return nil, errors.New("--machine can't be used with -s/--select") + } + return getMachineByID(ctx) + } + return promptForMachine(ctx, app) +} + +func getMachineByID(ctx context.Context) (*fly.Machine, error) { + flapsClient := flapsutil.ClientFromContext(ctx) + machineID := flag.GetString(ctx, "machine") + machine, err := flapsClient.Get(ctx, machineID) + if err != nil { + return nil, err + } + if machine.State != fly.MachineStateStarted { + return nil, fmt.Errorf("machine %s is not started", machineID) + } + + return machine, nil +} + +func promptForMachine(ctx context.Context, app *fly.AppCompact) (*fly.Machine, error) { + anyMachine := !flag.IsSpecified(ctx, "select") + + flapsClient := flapsutil.ClientFromContext(ctx) + machines, err := flapsClient.ListActive(ctx) + if err != nil { + return nil, err + } + + // TODO: Perhaps we should allow selection of non-started machines, + // preferring the started machines over the non-started machines. + machines = lo.Filter(machines, func(machine *fly.Machine, _ int) bool { + return machine.State == fly.MachineStateStarted + }) + + options := []string{} + for _, machine := range machines { + // TODO: show state? show imageref? + options = append(options, fmt.Sprintf("%s: %s %s", machine.Region, machine.ID, machine.Name)) + } + + if len(machines) == 0 { + return nil, fmt.Errorf("there are no running machines") + } + + if anyMachine || len(machines) == 1 { + return machines[0], nil + } + + index := 0 + if err := prompt.Select(ctx, &index, "Select a machine:", "", options...); err != nil { + return nil, fmt.Errorf("failed to prompt for a machine: %w", err) + } + return machines[index], nil +} diff --git a/internal/command/scan/sbom.go b/internal/command/scan/sbom.go new file mode 100644 index 0000000000..627d8faecd --- /dev/null +++ b/internal/command/scan/sbom.go @@ -0,0 +1,93 @@ +package scan + +import ( + "context" + "fmt" + "io" + "net/http" + + "github.com/spf13/cobra" + + "github.com/superfly/fly-go/flaps" + "github.com/superfly/flyctl/internal/appconfig" + "github.com/superfly/flyctl/internal/command" + "github.com/superfly/flyctl/internal/flag" + "github.com/superfly/flyctl/internal/flapsutil" + "github.com/superfly/flyctl/internal/flyutil" + "github.com/superfly/flyctl/iostreams" +) + +func newSbom() *cobra.Command { + const ( + usage = "sbom" + short = "Generate an SBOM for an application's image" + long = "Generate an SBOM for an application's image. If a machine is selected\n" + + "the image from that machine is scanned. Otherwise the image of the first\n" + + "running machine is scanned." + ) + cmd := command.New(usage, short, long, runSbom, + command.RequireSession, + command.RequireAppName, + ) + + cmd.Args = cobra.NoArgs + flag.Add( + cmd, + flag.App(), + flag.String{ + Name: "machine", + Description: "Scan the image of the machine with the specified ID", + }, + flag.Bool{ + Name: "select", + Shorthand: "s", + Description: "Select which machine to scan the image of from a list.", + Default: false, + }, + // TODO: output file + ) + + return cmd +} + +func runSbom(ctx context.Context) error { + var ( + ios = iostreams.FromContext(ctx) + appName = appconfig.NameFromContext(ctx) + apiClient = flyutil.ClientFromContext(ctx) + ) + + app, err := apiClient.GetAppCompact(ctx, appName) + if err != nil { + return fmt.Errorf("failed to get app: %w", err) + } + + flapsClient, err := flapsutil.NewClientWithOptions(ctx, flaps.NewClientOpts{ + AppCompact: app, + AppName: app.Name, + }) + if err != nil { + return fmt.Errorf("failed to create flaps client: %w", err) + } + ctx = flapsutil.NewContextWithClient(ctx, flapsClient) + + machine, err := selectMachine(ctx, app) + if err != nil { + return err + } + + res, err := scantron(ctx, apiClient, app, machine, true) + if err != nil { + return err + } + defer res.Body.Close() + + if res.StatusCode != http.StatusOK { + return fmt.Errorf("failed fetching SBOM (status code %d)", res.StatusCode) + } + + if _, err := io.Copy(ios.Out, res.Body); err != nil { + return fmt.Errorf("failed to read SBOM: %w", err) + } + return nil +} diff --git a/internal/command/scan/scantron.go b/internal/command/scan/scantron.go new file mode 100644 index 0000000000..4fc289fbb2 --- /dev/null +++ b/internal/command/scan/scantron.go @@ -0,0 +1,52 @@ +package scan + +import ( + "context" + "fmt" + "net/http" + "os" + "time" + + fly "github.com/superfly/fly-go" + "github.com/superfly/flyctl/internal/buildinfo" + "github.com/superfly/flyctl/internal/flyutil" +) + +var scantronUrl = "https://scantron.fly.dev" + +var httpClient = &http.Client{ + Timeout: time.Second * 3, +} + +// scantron makes a request to the scantron service for an app's image. +// It returns a vulnerability scan if sbomOnly is false, or an SBOM if sbomOnly is true. +func scantron(ctx context.Context, apiClient flyutil.Client, app *fly.AppCompact, machine *fly.Machine, sbomOnly bool) (*http.Response, error) { + if val := os.Getenv("FLY_SCANTRON"); val != "" { + scantronUrl = val + } + + img := machine.ImageRef + url := fmt.Sprintf("%s/%s/%s@%s", scantronUrl, img.Registry, img.Repository, img.Digest) + + req, err := http.NewRequestWithContext(ctx, "GET", url, http.NoBody) + if err != nil { + return nil, fmt.Errorf("failed to create HTTP request: %w", err) + } + + req.Header.Set("User-Agent", buildinfo.UserAgent()) + if sbomOnly { + req.Header.Set("Accept", "application/spdx+json") + } else { + req.Header.Set("Accept", "application/json") + } + + if err := addAuth(ctx, apiClient, app.Organization.ID, app.ID, req); err != nil { + return nil, fmt.Errorf("failed to create scanner token: %w", err) + } + + res, err := httpClient.Do(req) + if err != nil { + return nil, fmt.Errorf("failed fetching data from scantron: %w", err) + } + return res, nil +} diff --git a/internal/command/scan/vulns.go b/internal/command/scan/vulns.go new file mode 100644 index 0000000000..43f55b14a2 --- /dev/null +++ b/internal/command/scan/vulns.go @@ -0,0 +1,165 @@ +package scan + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + + "github.com/spf13/cobra" + + "github.com/superfly/fly-go/flaps" + "github.com/superfly/flyctl/internal/appconfig" + "github.com/superfly/flyctl/internal/command" + "github.com/superfly/flyctl/internal/flag" + "github.com/superfly/flyctl/internal/flapsutil" + "github.com/superfly/flyctl/internal/flyutil" + "github.com/superfly/flyctl/iostreams" +) + +func newVulns() *cobra.Command { + const ( + usage = "vulns" + short = "Scan an application's image for vulnerabilities" + long = "Generate a text or JSON report of vulnerabilities found in a application's image.\n" + + "If a machine is selected the image from that machine is scanned. Otherwise the image\n" + + "of the first running machine is scanned." + ) + cmd := command.New(usage, short, long, runVulns, + command.RequireSession, + command.RequireAppName, + ) + + cmd.Args = cobra.NoArgs + flag.Add( + cmd, + flag.App(), + flag.String{ + Name: "machine", + Description: "Scan the image of the machine with the specified ID", + }, + flag.Bool{ + Name: "select", + Shorthand: "s", + Description: "Select which machine to scan the image of from a list.", + Default: false, + }, + // TODO: output file + // TODO: format json/text + ) + + return cmd +} + +type Scan struct { + SchemaVersion int + CreatedAt string + // Metadata + Results []ScanResult +} + +type ScanResult struct { + Target string + Type string + Vulnerabilities []ScanVuln +} + +type ScanVuln struct { + VulnerabilityID string + PkgName string + InstalledVersion string + Status string + Title string + Description string + Severity string +} + +func runVulns(ctx context.Context) error { + var ( + appName = appconfig.NameFromContext(ctx) + apiClient = flyutil.ClientFromContext(ctx) + ) + + app, err := apiClient.GetAppCompact(ctx, appName) + if err != nil { + return fmt.Errorf("failed to get app: %w", err) + } + + flapsClient, err := flapsutil.NewClientWithOptions(ctx, flaps.NewClientOpts{ + AppCompact: app, + AppName: app.Name, + }) + if err != nil { + return fmt.Errorf("failed to create flaps client: %w", err) + } + ctx = flapsutil.NewContextWithClient(ctx, flapsClient) + + machine, err := selectMachine(ctx, app) + if err != nil { + return err + } + + res, err := scantron(ctx, apiClient, app, machine, false) + if err != nil { + return err + } + defer res.Body.Close() + + if res.StatusCode != http.StatusOK { + return fmt.Errorf("failed fetching scan data (status code %d)", res.StatusCode) + } + + scan := &Scan{} + if err = json.NewDecoder(res.Body).Decode(scan); err != nil { + return fmt.Errorf("failed to read scan results: %w", err) + } + if scan.SchemaVersion != 2 { + return fmt.Errorf("scan result has the wrong schema") + } + + scan = filterScan(ctx, scan) + return presentScan(ctx, scan) +} + +// filterVuln returns true if the user's command line preferences +// indicate interested in the vulnerability. +func filterVuln(ctx context.Context, vuln *ScanVuln) bool { + // TODO: placeholder + return vuln.VulnerabilityID == "CVE-2024-24791" || + vuln.VulnerabilityID == "CVE-2023-31484" || + vuln.Severity == "CRITICAL" +} + +// filterScan filters each vuln in each result based on the command +// line preferences of the user. Any empty results are discarded. +func filterScan(ctx context.Context, scan *Scan) *Scan { + newRes := []ScanResult{} + for _, res := range scan.Results { + newVulns := []ScanVuln{} + for _, vuln := range res.Vulnerabilities { + if filterVuln(ctx, &vuln) { + newVulns = append(newVulns, vuln) + } + } + if len(newVulns) > 0 { + res.Vulnerabilities = newVulns + newRes = append(newRes, res) + } + } + scan.Results = newRes + return scan +} + +func presentScan(ctx context.Context, scan *Scan) error { + ios := iostreams.FromContext(ctx) + + // TODO: scan.Metadata? + fmt.Fprintf(ios.Out, "Report created at: %s\n", scan.CreatedAt) + for _, res := range scan.Results { + fmt.Fprintf(ios.Out, "Target %s: %s\n", res.Type, res.Target) + for _, vuln := range res.Vulnerabilities { + fmt.Fprintf(ios.Out, " %s %s: %s %s\n", vuln.Severity, vuln.VulnerabilityID, vuln.PkgName, vuln.InstalledVersion) + } + } + return nil +} From 88da1dccf94c205acd555d3513a1290f5a2a9e84 Mon Sep 17 00:00:00 2001 From: Tim Newsham Date: Wed, 10 Jul 2024 11:11:15 -1000 Subject: [PATCH 02/26] go fmt --- internal/command/root/root.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/command/root/root.go b/internal/command/root/root.go index 8e4002b5df..143563a256 100644 --- a/internal/command/root/root.go +++ b/internal/command/root/root.go @@ -51,8 +51,8 @@ import ( "github.com/superfly/flyctl/internal/command/regions" "github.com/superfly/flyctl/internal/command/releases" "github.com/superfly/flyctl/internal/command/resume" - "github.com/superfly/flyctl/internal/command/scan" "github.com/superfly/flyctl/internal/command/scale" + "github.com/superfly/flyctl/internal/command/scan" "github.com/superfly/flyctl/internal/command/secrets" "github.com/superfly/flyctl/internal/command/services" "github.com/superfly/flyctl/internal/command/settings" From d85dd8da3845e48f8ff13a514f26d2673576a687 Mon Sep 17 00:00:00 2001 From: Tim Newsham Date: Wed, 10 Jul 2024 12:05:57 -1000 Subject: [PATCH 03/26] support vulnerability filtering --- internal/command/scan/vulns.go | 59 +++++++++++++++++++++++++++------- 1 file changed, 47 insertions(+), 12 deletions(-) diff --git a/internal/command/scan/vulns.go b/internal/command/scan/vulns.go index 43f55b14a2..71417943f8 100644 --- a/internal/command/scan/vulns.go +++ b/internal/command/scan/vulns.go @@ -6,6 +6,7 @@ import ( "fmt" "net/http" + "github.com/samber/lo" "github.com/spf13/cobra" "github.com/superfly/fly-go/flaps" @@ -17,33 +18,43 @@ import ( "github.com/superfly/flyctl/iostreams" ) +var allowedSeverities = []string{"LOW", "MEDIUM", "HIGH", "CRITICAL"} + func newVulns() *cobra.Command { const ( - usage = "vulns" + usage = "vulns ... [flags]" short = "Scan an application's image for vulnerabilities" long = "Generate a text or JSON report of vulnerabilities found in a application's image.\n" + "If a machine is selected the image from that machine is scanned. Otherwise the image\n" + - "of the first running machine is scanned." + "of the first running machine is scanned. When a severity is specified, any vulnerabilities\n" + + "less than the severity are omitted. When vulnIds are specified, any vulnerability not\n" + + "in the vulnID list is omitted." ) cmd := command.New(usage, short, long, runVulns, command.RequireSession, command.RequireAppName, ) - cmd.Args = cobra.NoArgs + cmd.Args = cobra.ArbitraryArgs flag.Add( cmd, flag.App(), flag.String{ Name: "machine", + Shorthand: "m", Description: "Scan the image of the machine with the specified ID", }, flag.Bool{ Name: "select", Shorthand: "s", - Description: "Select which machine to scan the image of from a list.", + Description: "Select which machine to scan the image of from a list", Default: false, }, + flag.String{ + Name: "severity", + Shorthand: "S", + Description: fmt.Sprintf("Report only issues with a specific severity %v", allowedSeverities), + }, // TODO: output file // TODO: format json/text ) @@ -74,12 +85,24 @@ type ScanVuln struct { Severity string } +type VulnFilter struct { + SeverityLevel int + VulnIds []string +} + func runVulns(ctx context.Context) error { var ( appName = appconfig.NameFromContext(ctx) apiClient = flyutil.ClientFromContext(ctx) ) + vulnIds := flag.Args(ctx) + sev := flag.GetString(ctx, "severity") + if flag.IsSpecified(ctx, "severity") && !lo.Contains(allowedSeverities, sev) { + return fmt.Errorf("severity (%s) must be one of %v", sev, allowedSeverities) + } + filter := &VulnFilter{severityLevel(sev), vulnIds} + app, err := apiClient.GetAppCompact(ctx, appName) if err != nil { return fmt.Errorf("failed to get app: %w", err) @@ -117,27 +140,39 @@ func runVulns(ctx context.Context) error { return fmt.Errorf("scan result has the wrong schema") } - scan = filterScan(ctx, scan) + scan = filterScan(scan, filter) return presentScan(ctx, scan) } +// severityLevel converts an `allowedSeverity` into an integer, +// where larger integers are more severe. Unknown severities +// result in `-1`. +func severityLevel(sev string) int { + return lo.IndexOf(allowedSeverities, sev) +} + // filterVuln returns true if the user's command line preferences // indicate interested in the vulnerability. -func filterVuln(ctx context.Context, vuln *ScanVuln) bool { - // TODO: placeholder - return vuln.VulnerabilityID == "CVE-2024-24791" || - vuln.VulnerabilityID == "CVE-2023-31484" || - vuln.Severity == "CRITICAL" +func filterVuln(vuln *ScanVuln, filter *VulnFilter) bool { + if filter.SeverityLevel > severityLevel(vuln.Severity) { + return false + } + if len(filter.VulnIds) > 0 { + if !lo.Contains(filter.VulnIds, vuln.VulnerabilityID) { + return false + } + } + return true } // filterScan filters each vuln in each result based on the command // line preferences of the user. Any empty results are discarded. -func filterScan(ctx context.Context, scan *Scan) *Scan { +func filterScan(scan *Scan, filter *VulnFilter) *Scan { newRes := []ScanResult{} for _, res := range scan.Results { newVulns := []ScanVuln{} for _, vuln := range res.Vulnerabilities { - if filterVuln(ctx, &vuln) { + if filterVuln(&vuln, filter) { newVulns = append(newVulns, vuln) } } From 5ffdd962b1532aa970af1830df840f14806f7d66 Mon Sep 17 00:00:00 2001 From: Tim Newsham Date: Wed, 10 Jul 2024 15:28:34 -1000 Subject: [PATCH 04/26] sort results before printing --- internal/command/scan/vulns.go | 52 ++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/internal/command/scan/vulns.go b/internal/command/scan/vulns.go index 71417943f8..a064782fca 100644 --- a/internal/command/scan/vulns.go +++ b/internal/command/scan/vulns.go @@ -5,6 +5,9 @@ import ( "encoding/json" "fmt" "net/http" + "slices" + "strconv" + "strings" "github.com/samber/lo" "github.com/spf13/cobra" @@ -165,6 +168,54 @@ func filterVuln(vuln *ScanVuln, filter *VulnFilter) bool { return true } +// cmpVulnId compares a and b component by component. +// Pairs of components are compared numerically if they are both numeric. +func cmpVulnId(a, b string) int { + as := strings.Split(a, "-") + bs := strings.Split(b, "-") + for n, ax := range as { + if n >= len(bs) { + return 1 + } + bx := bs[n] + + an, aerr := strconv.ParseUint(ax, 10, 32) + bn, berr := strconv.ParseUint(bx, 10, 32) + d := 0 + if aerr == nil && berr == nil { + d = int(an) - int(bn) + } else { + d = strings.Compare(ax, bx) + } + if d != 0 { + return d + } + } + + if len(as) < len(bs) { + return 1 + } + return slices.Compare(as, bs) +} + +// revCmpVuln compares vulns for sorting by highest severity and +// most recent vulnID first. +func revCmpVuln(a, b ScanVuln) int { + if a.Severity != b.Severity { + return -(severityLevel(a.Severity) - severityLevel(b.Severity)) + } + if a.VulnerabilityID != b.VulnerabilityID { + return -(cmpVulnId(a.VulnerabilityID, b.VulnerabilityID)) + } + if a.PkgName != b.PkgName { + return -(strings.Compare(a.PkgName, b.PkgName)) + } + if a.InstalledVersion != b.InstalledVersion { + return strings.Compare(a.InstalledVersion, b.InstalledVersion) + } + return 0 +} + // filterScan filters each vuln in each result based on the command // line preferences of the user. Any empty results are discarded. func filterScan(scan *Scan, filter *VulnFilter) *Scan { @@ -177,6 +228,7 @@ func filterScan(scan *Scan, filter *VulnFilter) *Scan { } } if len(newVulns) > 0 { + slices.SortFunc(newVulns, revCmpVuln) res.Vulnerabilities = newVulns newRes = append(newRes, res) } From f067ae145d3d2802ee641af01b9c50a950fb426e Mon Sep 17 00:00:00 2001 From: Tim Newsham Date: Wed, 10 Jul 2024 15:59:58 -1000 Subject: [PATCH 05/26] support reporting raw json scan results --- internal/command/scan/vulns.go | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/internal/command/scan/vulns.go b/internal/command/scan/vulns.go index a064782fca..9679c66c7b 100644 --- a/internal/command/scan/vulns.go +++ b/internal/command/scan/vulns.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "fmt" + "io" "net/http" "slices" "strconv" @@ -42,6 +43,10 @@ func newVulns() *cobra.Command { flag.Add( cmd, flag.App(), + flag.Bool{ + Name: "json", + Description: "Output the scan results in JSON format", + }, flag.String{ Name: "machine", Shorthand: "m", @@ -106,6 +111,14 @@ func runVulns(ctx context.Context) error { } filter := &VulnFilter{severityLevel(sev), vulnIds} + if flag.IsSpecified(ctx, "json") { + if len(vulnIds) > 0 || flag.IsSpecified(ctx, "severity") { + // We could support filtering of JSON results but we would need to + // fully represent the v2 trivy schema structures or import them. + return fmt.Errorf("filtering by severity or CVE is not supported when outputting JSON") + } + } + app, err := apiClient.GetAppCompact(ctx, appName) if err != nil { return fmt.Errorf("failed to get app: %w", err) @@ -135,6 +148,14 @@ func runVulns(ctx context.Context) error { return fmt.Errorf("failed fetching scan data (status code %d)", res.StatusCode) } + if flag.GetBool(ctx, "json") { + ios := iostreams.FromContext(ctx) + if _, err := io.Copy(ios.Out, res.Body); err != nil { + return fmt.Errorf("failed to read scan results: %w", err) + } + return nil + } + scan := &Scan{} if err = json.NewDecoder(res.Body).Decode(scan); err != nil { return fmt.Errorf("failed to read scan results: %w", err) From 5071d449f0a043aaa2487be47e1d67db58577db3 Mon Sep 17 00:00:00 2001 From: Tim Newsham Date: Thu, 11 Jul 2024 16:26:27 -1000 Subject: [PATCH 06/26] explore implementation for a summary scan across apps in an org --- internal/command/scan/auth.go | 32 +--- internal/command/scan/command.go | 15 +- internal/command/scan/filter.go | 123 ++++++++++++ internal/command/scan/sbom.go | 8 +- internal/command/scan/scantron.go | 118 ++++++++++-- internal/command/scan/vulns.go | 162 +++------------- internal/command/scan/vulnsummary.go | 267 +++++++++++++++++++++++++++ 7 files changed, 529 insertions(+), 196 deletions(-) create mode 100644 internal/command/scan/filter.go create mode 100644 internal/command/scan/vulnsummary.go diff --git a/internal/command/scan/auth.go b/internal/command/scan/auth.go index 84eb068d16..d3f7cb25be 100644 --- a/internal/command/scan/auth.go +++ b/internal/command/scan/auth.go @@ -3,12 +3,9 @@ package scan import ( "context" "fmt" - "net/http" - fly "github.com/superfly/fly-go" "github.com/superfly/macaroon" "github.com/superfly/macaroon/flyio" - "github.com/superfly/macaroon/resset" "github.com/superfly/flyctl/gql" "github.com/superfly/flyctl/internal/flyutil" @@ -19,33 +16,8 @@ const ( orgFeatureBuilder = "builder" ) -func addAuth(ctx context.Context, apiClient flyutil.Client, orgId, appId string, req *http.Request) error { - resp, err := makeToken(ctx, apiClient, "ScantronToken", orgId, "5m", "deploy", &gql.LimitedAccessTokenOptions{ - "app_id": appId, - }) - if err != nil { - return err - } - - token := resp.CreateLimitedAccessToken.LimitedAccessToken.TokenHeader - token, err = attenuateTokens(token, - &resset.IfPresent{ - Ifs: macaroon.NewCaveatSet( - &flyio.FeatureSet{Features: resset.New[string](resset.ActionRead, orgFeatureBuilder)}, - &flyio.AppFeatureSet{Features: resset.New[string](resset.ActionRead, appFeatureImages)}, - ), - Else: resset.ActionNone, - }, - ) - if err != nil { - return err - } - - req.Header.Set("Authorization", fly.AuthorizationHeader(token)) - return nil -} - -func makeToken(ctx context.Context, apiClient flyutil.Client, name, orgID, expiry, profile string, options *gql.LimitedAccessTokenOptions) (*gql.CreateLimitedAccessTokenResponse, error) { +func makeToken(ctx context.Context, name, orgID, expiry, profile string, options *gql.LimitedAccessTokenOptions) (*gql.CreateLimitedAccessTokenResponse, error) { + apiClient := flyutil.ClientFromContext(ctx) resp, err := gql.CreateLimitedAccessToken( ctx, apiClient.GenqClient(), diff --git a/internal/command/scan/command.go b/internal/command/scan/command.go index 4980a1f204..62e6c8d943 100644 --- a/internal/command/scan/command.go +++ b/internal/command/scan/command.go @@ -26,6 +26,7 @@ func New() *cobra.Command { cmd.AddCommand( newSbom(), newVulns(), + newVulnSummary(), ) return cmd @@ -64,11 +65,13 @@ func promptForMachine(ctx context.Context, app *fly.AppCompact) (*fly.Machine, e return nil, err } - // TODO: Perhaps we should allow selection of non-started machines, - // preferring the started machines over the non-started machines. - machines = lo.Filter(machines, func(machine *fly.Machine, _ int) bool { - return machine.State == fly.MachineStateStarted - }) + if false { + // TODO: Perhaps we should allow selection of non-started machines, + // preferring the started machines over the non-started machines. + machines = lo.Filter(machines, func(machine *fly.Machine, _ int) bool { + return machine.State == fly.MachineStateStarted + }) + } options := []string{} for _, machine := range machines { @@ -77,7 +80,7 @@ func promptForMachine(ctx context.Context, app *fly.AppCompact) (*fly.Machine, e } if len(machines) == 0 { - return nil, fmt.Errorf("there are no running machines") + return nil, fmt.Errorf("no machines found") } if anyMachine || len(machines) == 1 { diff --git a/internal/command/scan/filter.go b/internal/command/scan/filter.go new file mode 100644 index 0000000000..fa39242767 --- /dev/null +++ b/internal/command/scan/filter.go @@ -0,0 +1,123 @@ +package scan + +import ( + "context" + "fmt" + "slices" + "strconv" + "strings" + + "github.com/samber/lo" + + "github.com/superfly/flyctl/internal/flag" +) + +var allowedSeverities = []string{"LOW", "MEDIUM", "HIGH", "CRITICAL"} + +type VulnFilter struct { + SeverityLevel int + VulnIds []string +} + +func (p *VulnFilter) IsSpecified() bool { + return p.SeverityLevel > -1 && len(p.VulnIds) > 0 +} + +func getVulnFilter(ctx context.Context) (*VulnFilter, error) { + vulnIds := flag.Args(ctx) + sev := flag.GetString(ctx, "severity") + if flag.IsSpecified(ctx, "severity") && !lo.Contains(allowedSeverities, sev) { + return nil, fmt.Errorf("severity (%s) must be one of %v", sev, allowedSeverities) + } + return &VulnFilter{severityLevel(sev), vulnIds}, nil +} + +// severityLevel converts an `allowedSeverity` into an integer, +// where larger integers are more severe. Unknown severities +// result in `-1`. +func severityLevel(sev string) int { + return lo.IndexOf(allowedSeverities, sev) +} + +// filterVuln returns true if the user's command line preferences +// indicate interested in the vulnerability. +func filterVuln(vuln *ScanVuln, filter *VulnFilter) bool { + if filter.SeverityLevel > severityLevel(vuln.Severity) { + return false + } + if len(filter.VulnIds) > 0 { + if !lo.Contains(filter.VulnIds, vuln.VulnerabilityID) { + return false + } + } + return true +} + +// cmpVulnId compares a and b component by component. +// Pairs of components are compared numerically if they are both numeric. +func cmpVulnId(a, b string) int { + as := strings.Split(a, "-") + bs := strings.Split(b, "-") + for n, ax := range as { + if n >= len(bs) { + return 1 + } + bx := bs[n] + + an, aerr := strconv.ParseUint(ax, 10, 32) + bn, berr := strconv.ParseUint(bx, 10, 32) + d := 0 + if aerr == nil && berr == nil { + d = int(an) - int(bn) + } else { + d = strings.Compare(ax, bx) + } + if d != 0 { + return d + } + } + + if len(as) < len(bs) { + return 1 + } + return slices.Compare(as, bs) +} + +// revCmpVuln compares vulns for sorting by highest severity and +// most recent vulnID first. +func revCmpVuln(a, b ScanVuln) int { + if a.Severity != b.Severity { + return -(severityLevel(a.Severity) - severityLevel(b.Severity)) + } + if a.VulnerabilityID != b.VulnerabilityID { + return -(cmpVulnId(a.VulnerabilityID, b.VulnerabilityID)) + } + if a.PkgName != b.PkgName { + return -(strings.Compare(a.PkgName, b.PkgName)) + } + if a.InstalledVersion != b.InstalledVersion { + return strings.Compare(a.InstalledVersion, b.InstalledVersion) + } + return 0 +} + +// filterScan filters each vuln in each result based on the command +// line preferences of the user. Any empty results are discarded. +func filterScan(scan *Scan, filter *VulnFilter) *Scan { + newRes := []ScanResult{} + for _, res := range scan.Results { + newVulns := []ScanVuln{} + for _, vuln := range res.Vulnerabilities { + if filterVuln(&vuln, filter) { + newVulns = append(newVulns, vuln) + } + } + if len(newVulns) > 0 { + slices.SortFunc(newVulns, revCmpVuln) + res.Vulnerabilities = newVulns + newRes = append(newRes, res) + } + } + scan.Results = newRes + return scan +} diff --git a/internal/command/scan/sbom.go b/internal/command/scan/sbom.go index 627d8faecd..2b88ba8c48 100644 --- a/internal/command/scan/sbom.go +++ b/internal/command/scan/sbom.go @@ -76,7 +76,13 @@ func runSbom(ctx context.Context) error { return err } - res, err := scantron(ctx, apiClient, app, machine, true) + imgPath := imageRefPath(&machine.ImageRef) + token, err := makeScantronToken(ctx, app.Organization.ID, app.ID) + if err != nil { + return err + } + + res, err := scantronSbomReq(ctx, imgPath, token) if err != nil { return err } diff --git a/internal/command/scan/scantron.go b/internal/command/scan/scantron.go index 4fc289fbb2..7acb887640 100644 --- a/internal/command/scan/scantron.go +++ b/internal/command/scan/scantron.go @@ -2,51 +2,133 @@ package scan import ( "context" + "encoding/json" "fmt" "net/http" "os" "time" fly "github.com/superfly/fly-go" + "github.com/superfly/flyctl/gql" + "github.com/superfly/macaroon" + "github.com/superfly/macaroon/flyio" + "github.com/superfly/macaroon/resset" + "github.com/superfly/flyctl/internal/buildinfo" - "github.com/superfly/flyctl/internal/flyutil" +) + +const ( + scantronTokenLife = "5m" + scantronTokenName = "ScantronToken" ) var scantronUrl = "https://scantron.fly.dev" var httpClient = &http.Client{ - Timeout: time.Second * 3, + Timeout: time.Second * 15, +} + +func imageRefPath(imgRef *fly.MachineImageRef) string { + return fmt.Sprintf("%s/%s@%s", imgRef.Registry, imgRef.Repository, imgRef.Digest) } -// scantron makes a request to the scantron service for an app's image. -// It returns a vulnerability scan if sbomOnly is false, or an SBOM if sbomOnly is true. -func scantron(ctx context.Context, apiClient flyutil.Client, app *fly.AppCompact, machine *fly.Machine, sbomOnly bool) (*http.Response, error) { +// scantronReq requests information about imgPath from scanTron using token. +// The `accept` parameter is used as a header, which indicates which information +// scantron should serve up. +func scantronReq(ctx context.Context, imgPath, token, accept string) (*http.Response, error) { if val := os.Getenv("FLY_SCANTRON"); val != "" { scantronUrl = val } - img := machine.ImageRef - url := fmt.Sprintf("%s/%s/%s@%s", scantronUrl, img.Registry, img.Repository, img.Digest) - + url := fmt.Sprintf("%s/%s", scantronUrl, imgPath) req, err := http.NewRequestWithContext(ctx, "GET", url, http.NoBody) if err != nil { - return nil, fmt.Errorf("failed to create HTTP request: %w", err) + return nil, fmt.Errorf("failed to create scantron HTTP request: %w", err) } req.Header.Set("User-Agent", buildinfo.UserAgent()) - if sbomOnly { - req.Header.Set("Accept", "application/spdx+json") - } else { - req.Header.Set("Accept", "application/json") + req.Header.Set("Accept", accept) + req.Header.Set("Authorization", fly.AuthorizationHeader(token)) + res, err := httpClient.Do(req) + if err != nil { + return nil, fmt.Errorf("failed fetching data from scantron: %w", err) } + return res, nil +} - if err := addAuth(ctx, apiClient, app.Organization.ID, app.ID, req); err != nil { - return nil, fmt.Errorf("failed to create scanner token: %w", err) +func scantronSbomReq(ctx context.Context, imgPath, token string) (*http.Response, error) { + return scantronReq(ctx, imgPath, token, "application/spdx+json") +} + +func scantronVulnscanReq(ctx context.Context, imgPath, token string) (*http.Response, error) { + return scantronReq(ctx, imgPath, token, "application/json") +} + +type Scan struct { + SchemaVersion int + CreatedAt string + // Metadata + Results []ScanResult +} + +type ScanResult struct { + Target string + Type string + Vulnerabilities []ScanVuln +} + +type ScanVuln struct { + VulnerabilityID string + PkgName string + InstalledVersion string + Status string + Title string + Description string + Severity string +} + +func getVulnScan(ctx context.Context, imgPath, token string) (*Scan, error) { + res, err := scantronVulnscanReq(ctx, imgPath, token) + if err != nil { + return nil, err } + defer res.Body.Close() - res, err := httpClient.Do(req) + if res.StatusCode != http.StatusOK { + return nil, fmt.Errorf("fetching scan results, status code %d", res.StatusCode) + } + + scan := &Scan{} + if err = json.NewDecoder(res.Body).Decode(scan); err != nil { + return nil, fmt.Errorf("reading scan results: %w", err) + } + if scan.SchemaVersion != 2 { + return nil, fmt.Errorf("unknown scan schema %d", scan.SchemaVersion) + } + return scan, nil +} + +func makeScantronToken(ctx context.Context, orgId, appId string) (string, error) { + resp, err := makeToken(ctx, scantronTokenName, orgId, scantronTokenLife, "deploy", &gql.LimitedAccessTokenOptions{ + "app_id": appId, + }) if err != nil { - return nil, fmt.Errorf("failed fetching data from scantron: %w", err) + return "", err } - return res, nil + + token := resp.CreateLimitedAccessToken.LimitedAccessToken.TokenHeader + token, err = attenuateTokens(token, + &resset.IfPresent{ + Ifs: macaroon.NewCaveatSet( + &flyio.FeatureSet{Features: resset.New[string](resset.ActionRead, orgFeatureBuilder)}, + &flyio.AppFeatureSet{Features: resset.New[string](resset.ActionRead, appFeatureImages)}, + ), + Else: resset.ActionNone, + }, + ) + if err != nil { + return "", err + } + + return token, nil } diff --git a/internal/command/scan/vulns.go b/internal/command/scan/vulns.go index 9679c66c7b..fbbe2cc559 100644 --- a/internal/command/scan/vulns.go +++ b/internal/command/scan/vulns.go @@ -6,11 +6,7 @@ import ( "fmt" "io" "net/http" - "slices" - "strconv" - "strings" - "github.com/samber/lo" "github.com/spf13/cobra" "github.com/superfly/fly-go/flaps" @@ -22,8 +18,6 @@ import ( "github.com/superfly/flyctl/iostreams" ) -var allowedSeverities = []string{"LOW", "MEDIUM", "HIGH", "CRITICAL"} - func newVulns() *cobra.Command { const ( usage = "vulns ... [flags]" @@ -58,6 +52,11 @@ func newVulns() *cobra.Command { Description: "Select which machine to scan the image of from a list", Default: false, }, + flag.Bool{ + Name: "running", + Shorthand: "r", + Description: "Only scan images for machines that are running, otherwise scan stopped machines as well.", + }, flag.String{ Name: "severity", Shorthand: "S", @@ -70,55 +69,20 @@ func newVulns() *cobra.Command { return cmd } -type Scan struct { - SchemaVersion int - CreatedAt string - // Metadata - Results []ScanResult -} - -type ScanResult struct { - Target string - Type string - Vulnerabilities []ScanVuln -} - -type ScanVuln struct { - VulnerabilityID string - PkgName string - InstalledVersion string - Status string - Title string - Description string - Severity string -} - -type VulnFilter struct { - SeverityLevel int - VulnIds []string -} - func runVulns(ctx context.Context) error { - var ( - appName = appconfig.NameFromContext(ctx) - apiClient = flyutil.ClientFromContext(ctx) - ) - - vulnIds := flag.Args(ctx) - sev := flag.GetString(ctx, "severity") - if flag.IsSpecified(ctx, "severity") && !lo.Contains(allowedSeverities, sev) { - return fmt.Errorf("severity (%s) must be one of %v", sev, allowedSeverities) + filter, err := getVulnFilter(ctx) + if err != nil { + return err } - filter := &VulnFilter{severityLevel(sev), vulnIds} - if flag.IsSpecified(ctx, "json") { - if len(vulnIds) > 0 || flag.IsSpecified(ctx, "severity") { - // We could support filtering of JSON results but we would need to - // fully represent the v2 trivy schema structures or import them. - return fmt.Errorf("filtering by severity or CVE is not supported when outputting JSON") - } + if flag.IsSpecified(ctx, "json") && filter.IsSpecified() { + // We could support filtering of JSON results but we would need to + // fully represent the v2 trivy schema structures or import them. + return fmt.Errorf("filtering by severity or CVE is not supported when outputting JSON") } + appName := appconfig.NameFromContext(ctx) + apiClient := flyutil.ClientFromContext(ctx) app, err := apiClient.GetAppCompact(ctx, appName) if err != nil { return fmt.Errorf("failed to get app: %w", err) @@ -138,7 +102,13 @@ func runVulns(ctx context.Context) error { return err } - res, err := scantron(ctx, apiClient, app, machine, false) + imgPath := imageRefPath(&machine.ImageRef) + token, err := makeScantronToken(ctx, app.Organization.ID, app.ID) + if err != nil { + return err + } + + res, err := scantronVulnscanReq(ctx, imgPath, token) if err != nil { return err } @@ -168,96 +138,6 @@ func runVulns(ctx context.Context) error { return presentScan(ctx, scan) } -// severityLevel converts an `allowedSeverity` into an integer, -// where larger integers are more severe. Unknown severities -// result in `-1`. -func severityLevel(sev string) int { - return lo.IndexOf(allowedSeverities, sev) -} - -// filterVuln returns true if the user's command line preferences -// indicate interested in the vulnerability. -func filterVuln(vuln *ScanVuln, filter *VulnFilter) bool { - if filter.SeverityLevel > severityLevel(vuln.Severity) { - return false - } - if len(filter.VulnIds) > 0 { - if !lo.Contains(filter.VulnIds, vuln.VulnerabilityID) { - return false - } - } - return true -} - -// cmpVulnId compares a and b component by component. -// Pairs of components are compared numerically if they are both numeric. -func cmpVulnId(a, b string) int { - as := strings.Split(a, "-") - bs := strings.Split(b, "-") - for n, ax := range as { - if n >= len(bs) { - return 1 - } - bx := bs[n] - - an, aerr := strconv.ParseUint(ax, 10, 32) - bn, berr := strconv.ParseUint(bx, 10, 32) - d := 0 - if aerr == nil && berr == nil { - d = int(an) - int(bn) - } else { - d = strings.Compare(ax, bx) - } - if d != 0 { - return d - } - } - - if len(as) < len(bs) { - return 1 - } - return slices.Compare(as, bs) -} - -// revCmpVuln compares vulns for sorting by highest severity and -// most recent vulnID first. -func revCmpVuln(a, b ScanVuln) int { - if a.Severity != b.Severity { - return -(severityLevel(a.Severity) - severityLevel(b.Severity)) - } - if a.VulnerabilityID != b.VulnerabilityID { - return -(cmpVulnId(a.VulnerabilityID, b.VulnerabilityID)) - } - if a.PkgName != b.PkgName { - return -(strings.Compare(a.PkgName, b.PkgName)) - } - if a.InstalledVersion != b.InstalledVersion { - return strings.Compare(a.InstalledVersion, b.InstalledVersion) - } - return 0 -} - -// filterScan filters each vuln in each result based on the command -// line preferences of the user. Any empty results are discarded. -func filterScan(scan *Scan, filter *VulnFilter) *Scan { - newRes := []ScanResult{} - for _, res := range scan.Results { - newVulns := []ScanVuln{} - for _, vuln := range res.Vulnerabilities { - if filterVuln(&vuln, filter) { - newVulns = append(newVulns, vuln) - } - } - if len(newVulns) > 0 { - slices.SortFunc(newVulns, revCmpVuln) - res.Vulnerabilities = newVulns - newRes = append(newRes, res) - } - } - scan.Results = newRes - return scan -} - func presentScan(ctx context.Context, scan *Scan) error { ios := iostreams.FromContext(ctx) diff --git a/internal/command/scan/vulnsummary.go b/internal/command/scan/vulnsummary.go new file mode 100644 index 0000000000..5fd73ebe58 --- /dev/null +++ b/internal/command/scan/vulnsummary.go @@ -0,0 +1,267 @@ +package scan + +import ( + "context" + "fmt" + "slices" + "strings" + + "github.com/samber/lo" + "github.com/spf13/cobra" + + fly "github.com/superfly/fly-go" + "github.com/superfly/fly-go/flaps" + "github.com/superfly/flyctl/internal/appconfig" + "github.com/superfly/flyctl/internal/command" + "github.com/superfly/flyctl/internal/flag" + "github.com/superfly/flyctl/internal/flapsutil" + "github.com/superfly/flyctl/internal/flyutil" + "github.com/superfly/flyctl/internal/render" + "github.com/superfly/flyctl/iostreams" +) + +// TODO: placeholder name.. figure out proper name for this one. +func newVulnSummary() *cobra.Command { + const ( + usage = "vulnsummary ... [flags]" + short = "Show vulnerabilities in machine images" + long = "XXX todo. vulns in images across apps and machines, for running apps, or even stopped apps." + ) + cmd := command.New(usage, short, long, runVulnSummary, + command.RequireSession, + command.LoadAppNameIfPresent, + ) + + cmd.Args = cobra.ArbitraryArgs + flag.Add( + cmd, + flag.App(), + flag.Org(), + flag.Bool{ + Name: "running", + Shorthand: "r", + Description: "Only scan images for machines that are running, otherwise scan stopped machines as well.", + }, + flag.String{ + Name: "severity", + Shorthand: "S", + Description: fmt.Sprintf("Report only issues with a specific severity %v", allowedSeverities), + }, + ) + + return cmd +} + +// ImgInfo carries image information for a machine. +type ImgInfo struct { + Org string + OrgID string + App string + AppID string + Mach string + Path string +} + +func is400(err error) bool { + // Oh gross hack of all gross hacks. + // TODO: do better + return strings.Contains(err.Error(), "status code 400") +} + +func runVulnSummary(ctx context.Context) error { + var err error + filter, err := getVulnFilter(ctx) + if err != nil { + return err + } + + // enumerate all images of interest. + var imgs []ImgInfo + if appName := flag.GetApp(ctx); appName != "" { + imgs, err = getAppImages(ctx, appName) + } else if orgName := flag.GetOrg(ctx); orgName != "" { + imgs, err = getOrgImages(ctx, orgName) + } else if appName := appconfig.NameFromContext(ctx); appName != "" { + imgs, err = getAppImages(ctx, appName) + } else { + err = fmt.Errorf("No org or application specified") + } + if err != nil { + return err + } + + // fetch all image scans. + imageScan := map[string]*Scan{} + token := "" + tokenAppID := "" + for _, img := range imgs { + if _, ok := imageScan[img.Path]; ok { + continue + } + + if img.AppID != tokenAppID { + tokenAppID = img.AppID + token, err = makeScantronToken(ctx, img.OrgID, img.AppID) + if err != nil { + return err + } + } + + scan, err := getVulnScan(ctx, img.Path, token) + if err != nil { + if is400(err) { + // TODO: not fmt.Printf, do better. + fmt.Printf("Skipping %s (%s) from unsupported repository\n", img.App, img.Mach) + continue + } + return fmt.Errorf("Getting vulnerability scan for %s (%s): %w", img.App, img.Mach, err) + } + imageScan[img.Path] = filterScan(scan, filter) + } + + // calculate findings tables + allVids := map[string]bool{} + vidsByApp := map[string]map[string]bool{} + appImgsScanned := map[string]bool{} + for _, img := range imgs { + scan := imageScan[img.Path] + if scan == nil { + continue + } + + k := fmt.Sprintf("%s/%s", img.AppID, img.Path) + if _, ok := appImgsScanned[k]; ok { + continue + } + appImgsScanned[k] = true + + if _, ok := vidsByApp[img.App]; !ok { + vidsByApp[img.App] = map[string]bool{} + } + appVids := vidsByApp[img.App] + + for _, res := range scan.Results { + for _, vuln := range res.Vulnerabilities { + vid := vuln.VulnerabilityID + allVids[vid] = true + appVids[vid] = true + } + } + } + + // Show what is being scanned. + ios := iostreams.FromContext(ctx) + lastOrg := "" + lastApp := "" + fmt.Fprintf(ios.Out, "Scanned images\n") + for _, img := range imgs { + scan := imageScan[img.Path] + if img.Org != lastOrg { + fmt.Fprintf(ios.Out, "Org: %s\n", img.Org) + lastOrg = img.Org + } + if img.App != lastApp { + fmt.Fprintf(ios.Out, " App: %s\n", img.App) + lastApp = img.App + } + if scan != nil { + fmt.Fprintf(ios.Out, " %s\t%s\n", img.Mach, img.Path) + } else { + fmt.Fprintf(ios.Out, " %s\t%s [skipped]\n", img.Mach, img.Path) + } + } + fmt.Fprintf(ios.Out, "\n") + fmt.Fprintf(ios.Out, "To scan an image run: flyctl image scan \n") + fmt.Fprintf(ios.Out, "\n") + + // Report checkmark table with columns of apps and rows of vulns. + // TODO: use flyctl table stuff for pretty pretty + apps := lo.Keys(vidsByApp) + slices.SortFunc(apps, strings.Compare) + vids := lo.Keys(allVids) + slices.SortFunc(vids, cmpVulnId) + + rows := [][]string{} + for _, vid := range vids { + row := []string{vid} + for _, app := range apps { + check := lo.Ternary(vidsByApp[app][vid], "X", "-") + row = append(row, check) + } + rows = append(rows, row) + } + cols := append([]string{""}, apps...) + render.Table(ios.Out, "Vulnerabilities in Apps", rows, cols...) + + return nil +} + +func getOrgImages(ctx context.Context, orgName string) ([]ImgInfo, error) { + client := flyutil.ClientFromContext(ctx) + org, err := client.GetOrganizationBySlug(ctx, orgName) + if err != nil { + return nil, err + } + + apps, err := client.GetAppsForOrganization(ctx, org.ID) + if err != nil { + return nil, err + } + + allImgs := []ImgInfo{} + for _, app := range apps { + imgs, err := getAppImages(ctx, app.Name) + if err != nil { + return nil, fmt.Errorf("could not fetch images for %q app: %w", app.Name, err) + } + allImgs = append(allImgs, imgs...) + } + return allImgs, nil + +} + +func getAppImages(ctx context.Context, appName string) ([]ImgInfo, error) { + apiClient := flyutil.ClientFromContext(ctx) + app, err := apiClient.GetAppCompact(ctx, appName) + if err != nil { + return nil, fmt.Errorf("failed to get app %q: %w", appName, err) + } + + flapsClient, err := flapsutil.NewClientWithOptions(ctx, flaps.NewClientOpts{ + AppCompact: app, + AppName: app.Name, + }) + if err != nil { + return nil, fmt.Errorf("failed to create flaps client for %q: %w", appName, err) + } + org := app.Organization + ctx = flapsutil.NewContextWithClient(ctx, flapsClient) + + machines, err := flapsClient.ListActive(ctx) + if err != nil { + return nil, err + } + + if flag.GetBool(ctx, "running") { + machines = lo.Filter(machines, func(machine *fly.Machine, _ int) bool { + return machine.State == fly.MachineStateStarted + }) + } + + imgs := []ImgInfo{} + for _, machine := range machines { + ir := machine.ImageRef + imgPath := fmt.Sprintf("%s/%s@%s", ir.Registry, ir.Repository, ir.Digest) + + img := ImgInfo{ + Org: org.Name, + OrgID: org.ID, + App: app.Name, + AppID: app.ID, + Mach: machine.Name, + Path: imgPath, + } + imgs = append(imgs, img) + } + return imgs, nil +} From 89d71f78cbc3f0d9be238b81199089651d5f30c4 Mon Sep 17 00:00:00 2001 From: Tim Newsham Date: Fri, 12 Jul 2024 10:57:28 -1000 Subject: [PATCH 07/26] reorg and cleanup, and support for vulns/sbom scan by image path. --- internal/command/scan/args.go | 202 +++++++++++++++++++++++++++ internal/command/scan/command.go | 71 ---------- internal/command/scan/filter.go | 4 +- internal/command/scan/sbom.go | 20 +-- internal/command/scan/scantron.go | 17 ++- internal/command/scan/vulns.go | 32 ++--- internal/command/scan/vulnsummary.go | 119 ++-------------- 7 files changed, 242 insertions(+), 223 deletions(-) create mode 100644 internal/command/scan/args.go diff --git a/internal/command/scan/args.go b/internal/command/scan/args.go new file mode 100644 index 0000000000..6e1882a920 --- /dev/null +++ b/internal/command/scan/args.go @@ -0,0 +1,202 @@ +package scan + +import ( + "context" + "errors" + "fmt" + + "github.com/samber/lo" + + fly "github.com/superfly/fly-go" + "github.com/superfly/fly-go/flaps" + "github.com/superfly/flyctl/internal/appconfig" + "github.com/superfly/flyctl/internal/flag" + "github.com/superfly/flyctl/internal/flapsutil" + "github.com/superfly/flyctl/internal/flyutil" + "github.com/superfly/flyctl/internal/prompt" +) + +// ImgInfo carries image information for a machine. +type ImgInfo struct { + Org string + OrgID string + App string + AppID string + Mach string + Path string +} + +// argsGetMachine returns the selected machine, using `select` and `machine`. +func argsGetMachine(ctx context.Context, app *fly.AppCompact) (*fly.Machine, error) { + if flag.IsSpecified(ctx, "machine") { + if flag.IsSpecified(ctx, "select") { + return nil, errors.New("--machine can't be used with -s/--select") + } + return argsGetMachineByID(ctx, app) + } + return argsSelectMachine(ctx, app) +} + +// argsSelectMachine lets the user select a machine if there are multiple machines and +// the user specified "-s". Otherwise it returns the first machine for an app. +// Using `select`. +func argsSelectMachine(ctx context.Context, app *fly.AppCompact) (*fly.Machine, error) { + anyMachine := !flag.GetBool(ctx, "select") + + flapsClient, err := flapsutil.NewClientWithOptions(ctx, flaps.NewClientOpts{ + AppCompact: app, + AppName: app.Name, + }) + if err != nil { + return nil, fmt.Errorf("failed to create flaps client for app %s: %w", app.Name, err) + } + + machines, err := flapsClient.ListActive(ctx) + if err != nil { + return nil, err + } + + options := []string{} + for _, machine := range machines { + imgPath := imageRefPath(&machine.ImageRef) + options = append(options, fmt.Sprintf("%s: %s %s %s", machine.Region, machine.ID, machine.Name, imgPath)) + } + + if len(machines) == 0 { + return nil, fmt.Errorf("no machines found") + } + + if anyMachine || len(machines) == 1 { + return machines[0], nil + } + + index := 0 + if err := prompt.Select(ctx, &index, "Select a machine:", "", options...); err != nil { + return nil, fmt.Errorf("failed to prompt for a machine: %w", err) + } + return machines[index], nil +} + +// argsGetMachineByID returns an app's machine using the `machine` argument. +func argsGetMachineByID(ctx context.Context, app *fly.AppCompact) (*fly.Machine, error) { + flapsClient, err := flapsutil.NewClientWithOptions(ctx, flaps.NewClientOpts{ + AppCompact: app, + AppName: app.Name, + }) + if err != nil { + return nil, fmt.Errorf("failed to create flaps client for app %s: %w", app.Name, err) + } + + machineID := flag.GetString(ctx, "machine") + machine, err := flapsClient.Get(ctx, machineID) + if err != nil { + return nil, err + } + + return machine, nil +} + +// argsGetImgPath returns an image path from the command line or from a +// selected app machine, using `image`, `select`, and `machine`. +func argsGetImgPath(ctx context.Context, app *fly.AppCompact) (string, error) { + if flag.IsSpecified(ctx, "image") { + if flag.IsSpecified(ctx, "machine") || flag.IsSpecified(ctx, "select") { + return "", fmt.Errorf("image option cannot be used with machien and select options") + } + return flag.GetString(ctx, "image"), nil + } + + machine, err := argsGetMachine(ctx, app) + if err != nil { + return "", err + } + + return imageRefPath(&machine.ImageRef), nil +} + +// argsGetImages returns a list of images in ImgInfo format from +// command line args or the environment, using `org`, `app`, `running`. +func argsGetImages(ctx context.Context) ([]ImgInfo, error) { + if appName := flag.GetApp(ctx); appName != "" { + return argsGetAppImages(ctx, appName) + } else if orgName := flag.GetOrg(ctx); orgName != "" { + return argsGetOrgImages(ctx, orgName) + } else if appName := appconfig.NameFromContext(ctx); appName != "" { + return argsGetAppImages(ctx, appName) + } + return nil, fmt.Errorf("No org or application specified") +} + +// argsGetOrgImages returns a list of images for an org in ImgInfo format +// from `running`. +func argsGetOrgImages(ctx context.Context, orgName string) ([]ImgInfo, error) { + client := flyutil.ClientFromContext(ctx) + org, err := client.GetOrganizationBySlug(ctx, orgName) + if err != nil { + return nil, err + } + + apps, err := client.GetAppsForOrganization(ctx, org.ID) + if err != nil { + return nil, err + } + + allImgs := []ImgInfo{} + for _, app := range apps { + imgs, err := argsGetAppImages(ctx, app.Name) + if err != nil { + return nil, fmt.Errorf("could not fetch images for %q app: %w", app.Name, err) + } + allImgs = append(allImgs, imgs...) + } + return allImgs, nil + +} + +// argsGetAppImages returns a list of images for an app in ImgInfo format +// from `running`. +func argsGetAppImages(ctx context.Context, appName string) ([]ImgInfo, error) { + apiClient := flyutil.ClientFromContext(ctx) + app, err := apiClient.GetAppCompact(ctx, appName) + if err != nil { + return nil, fmt.Errorf("failed to get app %q: %w", appName, err) + } + + flapsClient, err := flapsutil.NewClientWithOptions(ctx, flaps.NewClientOpts{ + AppCompact: app, + AppName: app.Name, + }) + if err != nil { + return nil, fmt.Errorf("failed to create flaps client for %q: %w", appName, err) + } + org := app.Organization + ctx = flapsutil.NewContextWithClient(ctx, flapsClient) + + machines, err := flapsClient.ListActive(ctx) + if err != nil { + return nil, err + } + + if flag.GetBool(ctx, "running") { + machines = lo.Filter(machines, func(machine *fly.Machine, _ int) bool { + return machine.State == fly.MachineStateStarted + }) + } + + imgs := []ImgInfo{} + for _, machine := range machines { + ir := machine.ImageRef + imgPath := fmt.Sprintf("%s/%s@%s", ir.Registry, ir.Repository, ir.Digest) + + img := ImgInfo{ + Org: org.Name, + OrgID: org.ID, + App: app.Name, + AppID: app.ID, + Mach: machine.Name, + Path: imgPath, + } + imgs = append(imgs, img) + } + return imgs, nil +} diff --git a/internal/command/scan/command.go b/internal/command/scan/command.go index 62e6c8d943..617a55f573 100644 --- a/internal/command/scan/command.go +++ b/internal/command/scan/command.go @@ -1,18 +1,9 @@ package scan import ( - "context" - "errors" - "fmt" - - "github.com/samber/lo" "github.com/spf13/cobra" - fly "github.com/superfly/fly-go" "github.com/superfly/flyctl/internal/command" - "github.com/superfly/flyctl/internal/flag" - "github.com/superfly/flyctl/internal/flapsutil" - "github.com/superfly/flyctl/internal/prompt" ) func New() *cobra.Command { @@ -31,65 +22,3 @@ func New() *cobra.Command { return cmd } - -func selectMachine(ctx context.Context, app *fly.AppCompact) (*fly.Machine, error) { - if flag.IsSpecified(ctx, "machine") { - if flag.IsSpecified(ctx, "select") { - return nil, errors.New("--machine can't be used with -s/--select") - } - return getMachineByID(ctx) - } - return promptForMachine(ctx, app) -} - -func getMachineByID(ctx context.Context) (*fly.Machine, error) { - flapsClient := flapsutil.ClientFromContext(ctx) - machineID := flag.GetString(ctx, "machine") - machine, err := flapsClient.Get(ctx, machineID) - if err != nil { - return nil, err - } - if machine.State != fly.MachineStateStarted { - return nil, fmt.Errorf("machine %s is not started", machineID) - } - - return machine, nil -} - -func promptForMachine(ctx context.Context, app *fly.AppCompact) (*fly.Machine, error) { - anyMachine := !flag.IsSpecified(ctx, "select") - - flapsClient := flapsutil.ClientFromContext(ctx) - machines, err := flapsClient.ListActive(ctx) - if err != nil { - return nil, err - } - - if false { - // TODO: Perhaps we should allow selection of non-started machines, - // preferring the started machines over the non-started machines. - machines = lo.Filter(machines, func(machine *fly.Machine, _ int) bool { - return machine.State == fly.MachineStateStarted - }) - } - - options := []string{} - for _, machine := range machines { - // TODO: show state? show imageref? - options = append(options, fmt.Sprintf("%s: %s %s", machine.Region, machine.ID, machine.Name)) - } - - if len(machines) == 0 { - return nil, fmt.Errorf("no machines found") - } - - if anyMachine || len(machines) == 1 { - return machines[0], nil - } - - index := 0 - if err := prompt.Select(ctx, &index, "Select a machine:", "", options...); err != nil { - return nil, fmt.Errorf("failed to prompt for a machine: %w", err) - } - return machines[index], nil -} diff --git a/internal/command/scan/filter.go b/internal/command/scan/filter.go index fa39242767..3d3ba66735 100644 --- a/internal/command/scan/filter.go +++ b/internal/command/scan/filter.go @@ -23,7 +23,9 @@ func (p *VulnFilter) IsSpecified() bool { return p.SeverityLevel > -1 && len(p.VulnIds) > 0 } -func getVulnFilter(ctx context.Context) (*VulnFilter, error) { +// argsGetVulnFilter returns a VulnFilter from command line args +// using `severity` and positional arguments. +func argsGetVulnFilter(ctx context.Context) (*VulnFilter, error) { vulnIds := flag.Args(ctx) sev := flag.GetString(ctx, "severity") if flag.IsSpecified(ctx, "severity") && !lo.Contains(allowedSeverities, sev) { diff --git a/internal/command/scan/sbom.go b/internal/command/scan/sbom.go index 2b88ba8c48..8700488bcb 100644 --- a/internal/command/scan/sbom.go +++ b/internal/command/scan/sbom.go @@ -8,11 +8,9 @@ import ( "github.com/spf13/cobra" - "github.com/superfly/fly-go/flaps" "github.com/superfly/flyctl/internal/appconfig" "github.com/superfly/flyctl/internal/command" "github.com/superfly/flyctl/internal/flag" - "github.com/superfly/flyctl/internal/flapsutil" "github.com/superfly/flyctl/internal/flyutil" "github.com/superfly/flyctl/iostreams" ) @@ -34,6 +32,11 @@ func newSbom() *cobra.Command { flag.Add( cmd, flag.App(), + flag.String{ + Name: "image", + Shorthand: "i", + Description: "Scan the repository image", + }, flag.String{ Name: "machine", Description: "Scan the image of the machine with the specified ID", @@ -44,7 +47,6 @@ func newSbom() *cobra.Command { Description: "Select which machine to scan the image of from a list.", Default: false, }, - // TODO: output file ) return cmd @@ -62,21 +64,11 @@ func runSbom(ctx context.Context) error { return fmt.Errorf("failed to get app: %w", err) } - flapsClient, err := flapsutil.NewClientWithOptions(ctx, flaps.NewClientOpts{ - AppCompact: app, - AppName: app.Name, - }) - if err != nil { - return fmt.Errorf("failed to create flaps client: %w", err) - } - ctx = flapsutil.NewContextWithClient(ctx, flapsClient) - - machine, err := selectMachine(ctx, app) + imgPath, err := argsGetImgPath(ctx, app) if err != nil { return err } - imgPath := imageRefPath(&machine.ImageRef) token, err := makeScantronToken(ctx, app.Organization.ID, app.ID) if err != nil { return err diff --git a/internal/command/scan/scantron.go b/internal/command/scan/scantron.go index 7acb887640..92bb4fb875 100644 --- a/internal/command/scan/scantron.go +++ b/internal/command/scan/scantron.go @@ -18,12 +18,11 @@ import ( ) const ( - scantronTokenLife = "5m" - scantronTokenName = "ScantronToken" + scantronTokenLife = "5m" + scantronTokenName = "ScantronToken" + scantronDefaultUrl = "https://scantron.fly.dev" ) -var scantronUrl = "https://scantron.fly.dev" - var httpClient = &http.Client{ Timeout: time.Second * 15, } @@ -36,6 +35,7 @@ func imageRefPath(imgRef *fly.MachineImageRef) string { // The `accept` parameter is used as a header, which indicates which information // scantron should serve up. func scantronReq(ctx context.Context, imgPath, token, accept string) (*http.Response, error) { + scantronUrl := scantronDefaultUrl if val := os.Getenv("FLY_SCANTRON"); val != "" { scantronUrl = val } @@ -87,6 +87,12 @@ type ScanVuln struct { Severity string } +type ErrUnsupportedPath string + +func (e ErrUnsupportedPath) Error() string { + return fmt.Sprintf("Unsupported image path %q", string(e)) +} + func getVulnScan(ctx context.Context, imgPath, token string) (*Scan, error) { res, err := scantronVulnscanReq(ctx, imgPath, token) if err != nil { @@ -95,6 +101,9 @@ func getVulnScan(ctx context.Context, imgPath, token string) (*Scan, error) { defer res.Body.Close() if res.StatusCode != http.StatusOK { + if res.StatusCode == 422 { + return nil, ErrUnsupportedPath(imgPath) + } return nil, fmt.Errorf("fetching scan results, status code %d", res.StatusCode) } diff --git a/internal/command/scan/vulns.go b/internal/command/scan/vulns.go index fbbe2cc559..9116133cc0 100644 --- a/internal/command/scan/vulns.go +++ b/internal/command/scan/vulns.go @@ -9,11 +9,9 @@ import ( "github.com/spf13/cobra" - "github.com/superfly/fly-go/flaps" "github.com/superfly/flyctl/internal/appconfig" "github.com/superfly/flyctl/internal/command" "github.com/superfly/flyctl/internal/flag" - "github.com/superfly/flyctl/internal/flapsutil" "github.com/superfly/flyctl/internal/flyutil" "github.com/superfly/flyctl/iostreams" ) @@ -41,6 +39,11 @@ func newVulns() *cobra.Command { Name: "json", Description: "Output the scan results in JSON format", }, + flag.String{ + Name: "image", + Shorthand: "i", + Description: "Scan the repository image", + }, flag.String{ Name: "machine", Shorthand: "m", @@ -52,57 +55,38 @@ func newVulns() *cobra.Command { Description: "Select which machine to scan the image of from a list", Default: false, }, - flag.Bool{ - Name: "running", - Shorthand: "r", - Description: "Only scan images for machines that are running, otherwise scan stopped machines as well.", - }, flag.String{ Name: "severity", Shorthand: "S", Description: fmt.Sprintf("Report only issues with a specific severity %v", allowedSeverities), }, - // TODO: output file - // TODO: format json/text ) return cmd } func runVulns(ctx context.Context) error { - filter, err := getVulnFilter(ctx) + filter, err := argsGetVulnFilter(ctx) if err != nil { return err } if flag.IsSpecified(ctx, "json") && filter.IsSpecified() { - // We could support filtering of JSON results but we would need to - // fully represent the v2 trivy schema structures or import them. return fmt.Errorf("filtering by severity or CVE is not supported when outputting JSON") } - appName := appconfig.NameFromContext(ctx) apiClient := flyutil.ClientFromContext(ctx) + appName := appconfig.NameFromContext(ctx) app, err := apiClient.GetAppCompact(ctx, appName) if err != nil { return fmt.Errorf("failed to get app: %w", err) } - flapsClient, err := flapsutil.NewClientWithOptions(ctx, flaps.NewClientOpts{ - AppCompact: app, - AppName: app.Name, - }) - if err != nil { - return fmt.Errorf("failed to create flaps client: %w", err) - } - ctx = flapsutil.NewContextWithClient(ctx, flapsClient) - - machine, err := selectMachine(ctx, app) + imgPath, err := argsGetImgPath(ctx, app) if err != nil { return err } - imgPath := imageRefPath(&machine.ImageRef) token, err := makeScantronToken(ctx, app.Organization.ID, app.ID) if err != nil { return err diff --git a/internal/command/scan/vulnsummary.go b/internal/command/scan/vulnsummary.go index 5fd73ebe58..755897780a 100644 --- a/internal/command/scan/vulnsummary.go +++ b/internal/command/scan/vulnsummary.go @@ -2,6 +2,7 @@ package scan import ( "context" + "errors" "fmt" "slices" "strings" @@ -9,13 +10,8 @@ import ( "github.com/samber/lo" "github.com/spf13/cobra" - fly "github.com/superfly/fly-go" - "github.com/superfly/fly-go/flaps" - "github.com/superfly/flyctl/internal/appconfig" "github.com/superfly/flyctl/internal/command" "github.com/superfly/flyctl/internal/flag" - "github.com/superfly/flyctl/internal/flapsutil" - "github.com/superfly/flyctl/internal/flyutil" "github.com/superfly/flyctl/internal/render" "github.com/superfly/flyctl/iostreams" ) @@ -52,45 +48,20 @@ func newVulnSummary() *cobra.Command { return cmd } -// ImgInfo carries image information for a machine. -type ImgInfo struct { - Org string - OrgID string - App string - AppID string - Mach string - Path string -} - -func is400(err error) bool { - // Oh gross hack of all gross hacks. - // TODO: do better - return strings.Contains(err.Error(), "status code 400") -} - func runVulnSummary(ctx context.Context) error { var err error - filter, err := getVulnFilter(ctx) + filter, err := argsGetVulnFilter(ctx) if err != nil { return err } - // enumerate all images of interest. - var imgs []ImgInfo - if appName := flag.GetApp(ctx); appName != "" { - imgs, err = getAppImages(ctx, appName) - } else if orgName := flag.GetOrg(ctx); orgName != "" { - imgs, err = getOrgImages(ctx, orgName) - } else if appName := appconfig.NameFromContext(ctx); appName != "" { - imgs, err = getAppImages(ctx, appName) - } else { - err = fmt.Errorf("No org or application specified") - } + imgs, err := argsGetImages(ctx) if err != nil { return err } // fetch all image scans. + ios := iostreams.FromContext(ctx) imageScan := map[string]*Scan{} token := "" tokenAppID := "" @@ -109,9 +80,9 @@ func runVulnSummary(ctx context.Context) error { scan, err := getVulnScan(ctx, img.Path, token) if err != nil { - if is400(err) { - // TODO: not fmt.Printf, do better. - fmt.Printf("Skipping %s (%s) from unsupported repository\n", img.App, img.Mach) + errUnsupportedPath := ErrUnsupportedPath("") + if errors.As(err, &errUnsupportedPath) { + fmt.Fprintf(ios.Out, "Skipping %s (%s) from unsupported repository: %s\n", img.App, img.Mach, img.Path) continue } return fmt.Errorf("Getting vulnerability scan for %s (%s): %w", img.App, img.Mach, err) @@ -150,7 +121,6 @@ func runVulnSummary(ctx context.Context) error { } // Show what is being scanned. - ios := iostreams.FromContext(ctx) lastOrg := "" lastApp := "" fmt.Fprintf(ios.Out, "Scanned images\n") @@ -171,15 +141,16 @@ func runVulnSummary(ctx context.Context) error { } } fmt.Fprintf(ios.Out, "\n") - fmt.Fprintf(ios.Out, "To scan an image run: flyctl image scan \n") + fmt.Fprintf(ios.Out, "To scan an image run: flyctl scan vulns -a -i \n") + fmt.Fprintf(ios.Out, "To download an SBOM run: flyctl scan sbom -a -i \n") fmt.Fprintf(ios.Out, "\n") // Report checkmark table with columns of apps and rows of vulns. - // TODO: use flyctl table stuff for pretty pretty apps := lo.Keys(vidsByApp) slices.SortFunc(apps, strings.Compare) vids := lo.Keys(allVids) slices.SortFunc(vids, cmpVulnId) + slices.Reverse(vids) rows := [][]string{} for _, vid := range vids { @@ -195,73 +166,3 @@ func runVulnSummary(ctx context.Context) error { return nil } - -func getOrgImages(ctx context.Context, orgName string) ([]ImgInfo, error) { - client := flyutil.ClientFromContext(ctx) - org, err := client.GetOrganizationBySlug(ctx, orgName) - if err != nil { - return nil, err - } - - apps, err := client.GetAppsForOrganization(ctx, org.ID) - if err != nil { - return nil, err - } - - allImgs := []ImgInfo{} - for _, app := range apps { - imgs, err := getAppImages(ctx, app.Name) - if err != nil { - return nil, fmt.Errorf("could not fetch images for %q app: %w", app.Name, err) - } - allImgs = append(allImgs, imgs...) - } - return allImgs, nil - -} - -func getAppImages(ctx context.Context, appName string) ([]ImgInfo, error) { - apiClient := flyutil.ClientFromContext(ctx) - app, err := apiClient.GetAppCompact(ctx, appName) - if err != nil { - return nil, fmt.Errorf("failed to get app %q: %w", appName, err) - } - - flapsClient, err := flapsutil.NewClientWithOptions(ctx, flaps.NewClientOpts{ - AppCompact: app, - AppName: app.Name, - }) - if err != nil { - return nil, fmt.Errorf("failed to create flaps client for %q: %w", appName, err) - } - org := app.Organization - ctx = flapsutil.NewContextWithClient(ctx, flapsClient) - - machines, err := flapsClient.ListActive(ctx) - if err != nil { - return nil, err - } - - if flag.GetBool(ctx, "running") { - machines = lo.Filter(machines, func(machine *fly.Machine, _ int) bool { - return machine.State == fly.MachineStateStarted - }) - } - - imgs := []ImgInfo{} - for _, machine := range machines { - ir := machine.ImageRef - imgPath := fmt.Sprintf("%s/%s@%s", ir.Registry, ir.Repository, ir.Digest) - - img := ImgInfo{ - Org: org.Name, - OrgID: org.ID, - App: app.Name, - AppID: app.ID, - Mach: machine.Name, - Path: imgPath, - } - imgs = append(imgs, img) - } - return imgs, nil -} From 154ffa1f9e462a8410c45d570890e15cb8dedf79 Mon Sep 17 00:00:00 2001 From: Tim Newsham Date: Fri, 12 Jul 2024 11:18:57 -1000 Subject: [PATCH 08/26] rename scan as registry --- internal/command/{scan => registry}/args.go | 2 +- internal/command/{scan => registry}/auth.go | 2 +- internal/command/{scan => registry}/command.go | 8 ++++---- internal/command/{scan => registry}/filter.go | 2 +- internal/command/{scan => registry}/sbom.go | 2 +- internal/command/{scan => registry}/scantron.go | 2 +- internal/command/{scan => registry}/vulns.go | 2 +- internal/command/{scan => registry}/vulnsummary.go | 2 +- internal/command/root/root.go | 4 ++-- 9 files changed, 13 insertions(+), 13 deletions(-) rename internal/command/{scan => registry}/args.go (99%) rename internal/command/{scan => registry}/auth.go (98%) rename internal/command/{scan => registry}/command.go (59%) rename internal/command/{scan => registry}/filter.go (99%) rename internal/command/{scan => registry}/sbom.go (99%) rename internal/command/{scan => registry}/scantron.go (99%) rename internal/command/{scan => registry}/vulns.go (99%) rename internal/command/{scan => registry}/vulnsummary.go (99%) diff --git a/internal/command/scan/args.go b/internal/command/registry/args.go similarity index 99% rename from internal/command/scan/args.go rename to internal/command/registry/args.go index 6e1882a920..e869ff0ff7 100644 --- a/internal/command/scan/args.go +++ b/internal/command/registry/args.go @@ -1,4 +1,4 @@ -package scan +package registry import ( "context" diff --git a/internal/command/scan/auth.go b/internal/command/registry/auth.go similarity index 98% rename from internal/command/scan/auth.go rename to internal/command/registry/auth.go index d3f7cb25be..a877c95639 100644 --- a/internal/command/scan/auth.go +++ b/internal/command/registry/auth.go @@ -1,4 +1,4 @@ -package scan +package registry import ( "context" diff --git a/internal/command/scan/command.go b/internal/command/registry/command.go similarity index 59% rename from internal/command/scan/command.go rename to internal/command/registry/command.go index 617a55f573..d7fc71fb6a 100644 --- a/internal/command/scan/command.go +++ b/internal/command/registry/command.go @@ -1,4 +1,4 @@ -package scan +package registry import ( "github.com/spf13/cobra" @@ -8,9 +8,9 @@ import ( func New() *cobra.Command { const ( - usage = "scan" - short = "Scan machine images for vulnerabilities or to get an SBOM" - long = "Scan machine images for vulnerabilities or to get an SBOM." + usage = "registry" + short = "Operate on registry images" + long = "Scan registry images for an SBOM or vulnerabilities." ) cmd := command.New(usage, short, long, nil) diff --git a/internal/command/scan/filter.go b/internal/command/registry/filter.go similarity index 99% rename from internal/command/scan/filter.go rename to internal/command/registry/filter.go index 3d3ba66735..0a8254d65f 100644 --- a/internal/command/scan/filter.go +++ b/internal/command/registry/filter.go @@ -1,4 +1,4 @@ -package scan +package registry import ( "context" diff --git a/internal/command/scan/sbom.go b/internal/command/registry/sbom.go similarity index 99% rename from internal/command/scan/sbom.go rename to internal/command/registry/sbom.go index 8700488bcb..ac98c8951a 100644 --- a/internal/command/scan/sbom.go +++ b/internal/command/registry/sbom.go @@ -1,4 +1,4 @@ -package scan +package registry import ( "context" diff --git a/internal/command/scan/scantron.go b/internal/command/registry/scantron.go similarity index 99% rename from internal/command/scan/scantron.go rename to internal/command/registry/scantron.go index 92bb4fb875..e32c663974 100644 --- a/internal/command/scan/scantron.go +++ b/internal/command/registry/scantron.go @@ -1,4 +1,4 @@ -package scan +package registry import ( "context" diff --git a/internal/command/scan/vulns.go b/internal/command/registry/vulns.go similarity index 99% rename from internal/command/scan/vulns.go rename to internal/command/registry/vulns.go index 9116133cc0..82909ff77d 100644 --- a/internal/command/scan/vulns.go +++ b/internal/command/registry/vulns.go @@ -1,4 +1,4 @@ -package scan +package registry import ( "context" diff --git a/internal/command/scan/vulnsummary.go b/internal/command/registry/vulnsummary.go similarity index 99% rename from internal/command/scan/vulnsummary.go rename to internal/command/registry/vulnsummary.go index 755897780a..2a7db002a2 100644 --- a/internal/command/scan/vulnsummary.go +++ b/internal/command/registry/vulnsummary.go @@ -1,4 +1,4 @@ -package scan +package registry import ( "context" diff --git a/internal/command/root/root.go b/internal/command/root/root.go index 143563a256..b393c6c0ed 100644 --- a/internal/command/root/root.go +++ b/internal/command/root/root.go @@ -49,10 +49,10 @@ import ( "github.com/superfly/flyctl/internal/command/proxy" "github.com/superfly/flyctl/internal/command/redis" "github.com/superfly/flyctl/internal/command/regions" + "github.com/superfly/flyctl/internal/command/registry" "github.com/superfly/flyctl/internal/command/releases" "github.com/superfly/flyctl/internal/command/resume" "github.com/superfly/flyctl/internal/command/scale" - "github.com/superfly/flyctl/internal/command/scan" "github.com/superfly/flyctl/internal/command/secrets" "github.com/superfly/flyctl/internal/command/services" "github.com/superfly/flyctl/internal/command/settings" @@ -112,11 +112,11 @@ func New() *cobra.Command { group(proxy.New(), "upkeep"), group(postgres.New(), "dbs_and_extensions"), group(ips.New(), "configuring"), - group(scan.New(), "upkeep"), group(secrets.New(), "configuring"), group(ssh.New(), "upkeep"), group(ssh.NewSFTP(), "upkeep"), group(redis.New(), "dbs_and_extensions"), + group(registry.New(), "upkeep"), group(checks.New(), "upkeep"), group(launch.New(), "deploy"), group(info.New(), "upkeep"), From dad8cfd23fa30a595a0e35ce5e233d0fb0c62c77 Mon Sep 17 00:00:00 2001 From: Tim Newsham Date: Fri, 12 Jul 2024 11:46:38 -1000 Subject: [PATCH 09/26] cleanup command descriptions --- internal/command/registry/sbom.go | 8 ++++---- internal/command/registry/vulns.go | 11 +++++------ internal/command/registry/vulnsummary.go | 11 +++++++---- 3 files changed, 16 insertions(+), 14 deletions(-) diff --git a/internal/command/registry/sbom.go b/internal/command/registry/sbom.go index ac98c8951a..c239cf11e0 100644 --- a/internal/command/registry/sbom.go +++ b/internal/command/registry/sbom.go @@ -18,10 +18,10 @@ import ( func newSbom() *cobra.Command { const ( usage = "sbom" - short = "Generate an SBOM for an application's image" - long = "Generate an SBOM for an application's image. If a machine is selected\n" + - "the image from that machine is scanned. Otherwise the image of the first\n" + - "running machine is scanned." + short = "Generate an SBOM for a registry iamge" + long = "Genearte an SBOM for a registry image.\n" + + "The image is selected by name, or the image of the app's first machine\n" + + "is used unless interactive machine selection or machine ID is specified." ) cmd := command.New(usage, short, long, runSbom, command.RequireSession, diff --git a/internal/command/registry/vulns.go b/internal/command/registry/vulns.go index 82909ff77d..b8c3d383cb 100644 --- a/internal/command/registry/vulns.go +++ b/internal/command/registry/vulns.go @@ -19,12 +19,11 @@ import ( func newVulns() *cobra.Command { const ( usage = "vulns ... [flags]" - short = "Scan an application's image for vulnerabilities" - long = "Generate a text or JSON report of vulnerabilities found in a application's image.\n" + - "If a machine is selected the image from that machine is scanned. Otherwise the image\n" + - "of the first running machine is scanned. When a severity is specified, any vulnerabilities\n" + - "less than the severity are omitted. When vulnIds are specified, any vulnerability not\n" + - "in the vulnID list is omitted." + short = "Report possible vulnerabilities in a registry image" + long = "Report possible vulnerabilities in a registry image in JSON or text.\n" + + "The image is selected by name, or the image of the app's first machine\n" + + "is used unless interactive machine selection or machine ID is specified\n" + + "Limit text reporting to specific vulnerabilitie IDs or severities if specified." ) cmd := command.New(usage, short, long, runVulns, command.RequireSession, diff --git a/internal/command/registry/vulnsummary.go b/internal/command/registry/vulnsummary.go index 2a7db002a2..6870cbfe56 100644 --- a/internal/command/registry/vulnsummary.go +++ b/internal/command/registry/vulnsummary.go @@ -16,12 +16,14 @@ import ( "github.com/superfly/flyctl/iostreams" ) -// TODO: placeholder name.. figure out proper name for this one. func newVulnSummary() *cobra.Command { const ( usage = "vulnsummary ... [flags]" - short = "Show vulnerabilities in machine images" - long = "XXX todo. vulns in images across apps and machines, for running apps, or even stopped apps." + short = "Show a summary of possible vulnerabilities in registry images" + long = "Summarize possible vulnerabilities in registry images in an org, by app.\n" + + "Limit scanning to a single app if specified. Limit scanning to images\n" + + "used by running machines if specified. Limit reporting to\n" + + "specific vulnerability IDs or severities if specified." ) cmd := command.New(usage, short, long, runVulnSummary, command.RequireSession, @@ -36,7 +38,7 @@ func newVulnSummary() *cobra.Command { flag.Bool{ Name: "running", Shorthand: "r", - Description: "Only scan images for machines that are running, otherwise scan stopped machines as well.", + Description: "Only scan images for running machines", }, flag.String{ Name: "severity", @@ -61,6 +63,7 @@ func runVulnSummary(ctx context.Context) error { } // fetch all image scans. + // TODO: spinner for long running fetches. ios := iostreams.FromContext(ctx) imageScan := map[string]*Scan{} token := "" From 1bcb72889b94b74f524d01bbe657f7d6b1223de3 Mon Sep 17 00:00:00 2001 From: Tim Newsham Date: Fri, 12 Jul 2024 12:26:05 -1000 Subject: [PATCH 10/26] cleanup deepsrc recommendations --- internal/command/registry/args.go | 9 +++++---- internal/command/registry/filter.go | 4 ++-- internal/command/registry/sbom.go | 2 +- internal/command/registry/scantron.go | 2 +- internal/command/registry/vulns.go | 2 +- internal/command/registry/vulnsummary.go | 2 +- 6 files changed, 11 insertions(+), 10 deletions(-) diff --git a/internal/command/registry/args.go b/internal/command/registry/args.go index e869ff0ff7..c015b226af 100644 --- a/internal/command/registry/args.go +++ b/internal/command/registry/args.go @@ -56,7 +56,7 @@ func argsSelectMachine(ctx context.Context, app *fly.AppCompact) (*fly.Machine, return nil, err } - options := []string{} + var options []string for _, machine := range machines { imgPath := imageRefPath(&machine.ImageRef) options = append(options, fmt.Sprintf("%s: %s %s %s", machine.Region, machine.ID, machine.Name, imgPath)) @@ -141,8 +141,9 @@ func argsGetOrgImages(ctx context.Context, orgName string) ([]ImgInfo, error) { return nil, err } - allImgs := []ImgInfo{} - for _, app := range apps { + var allImgs []ImgInfo + for n, _ := range apps { + app := &apps[n] imgs, err := argsGetAppImages(ctx, app.Name) if err != nil { return nil, fmt.Errorf("could not fetch images for %q app: %w", app.Name, err) @@ -183,7 +184,7 @@ func argsGetAppImages(ctx context.Context, appName string) ([]ImgInfo, error) { }) } - imgs := []ImgInfo{} + var imgs []ImgInfo for _, machine := range machines { ir := machine.ImageRef imgPath := fmt.Sprintf("%s/%s@%s", ir.Registry, ir.Repository, ir.Digest) diff --git a/internal/command/registry/filter.go b/internal/command/registry/filter.go index 0a8254d65f..f066870945 100644 --- a/internal/command/registry/filter.go +++ b/internal/command/registry/filter.go @@ -106,9 +106,9 @@ func revCmpVuln(a, b ScanVuln) int { // filterScan filters each vuln in each result based on the command // line preferences of the user. Any empty results are discarded. func filterScan(scan *Scan, filter *VulnFilter) *Scan { - newRes := []ScanResult{} + var newRes []ScanResult for _, res := range scan.Results { - newVulns := []ScanVuln{} + var newVulns []ScanVuln for _, vuln := range res.Vulnerabilities { if filterVuln(&vuln, filter) { newVulns = append(newVulns, vuln) diff --git a/internal/command/registry/sbom.go b/internal/command/registry/sbom.go index c239cf11e0..c62ffe6e82 100644 --- a/internal/command/registry/sbom.go +++ b/internal/command/registry/sbom.go @@ -78,7 +78,7 @@ func runSbom(ctx context.Context) error { if err != nil { return err } - defer res.Body.Close() + defer res.Body.Close() // skipcq: GO-S2307 if res.StatusCode != http.StatusOK { return fmt.Errorf("failed fetching SBOM (status code %d)", res.StatusCode) diff --git a/internal/command/registry/scantron.go b/internal/command/registry/scantron.go index e32c663974..fc86d1f0ca 100644 --- a/internal/command/registry/scantron.go +++ b/internal/command/registry/scantron.go @@ -98,7 +98,7 @@ func getVulnScan(ctx context.Context, imgPath, token string) (*Scan, error) { if err != nil { return nil, err } - defer res.Body.Close() + defer res.Body.Close() // skipcq: GO-S2307 if res.StatusCode != http.StatusOK { if res.StatusCode == 422 { diff --git a/internal/command/registry/vulns.go b/internal/command/registry/vulns.go index b8c3d383cb..b81c2831ed 100644 --- a/internal/command/registry/vulns.go +++ b/internal/command/registry/vulns.go @@ -95,7 +95,7 @@ func runVulns(ctx context.Context) error { if err != nil { return err } - defer res.Body.Close() + defer res.Body.Close() // skipcq: GO-S2307 if res.StatusCode != http.StatusOK { return fmt.Errorf("failed fetching scan data (status code %d)", res.StatusCode) diff --git a/internal/command/registry/vulnsummary.go b/internal/command/registry/vulnsummary.go index 6870cbfe56..d3263dc88c 100644 --- a/internal/command/registry/vulnsummary.go +++ b/internal/command/registry/vulnsummary.go @@ -155,7 +155,7 @@ func runVulnSummary(ctx context.Context) error { slices.SortFunc(vids, cmpVulnId) slices.Reverse(vids) - rows := [][]string{} + var rows [][]string for _, vid := range vids { row := []string{vid} for _, app := range apps { From dfc16b496b0f6bbe07e809616a4ce2786bf08527 Mon Sep 17 00:00:00 2001 From: Tim Newsham Date: Fri, 12 Jul 2024 12:41:34 -1000 Subject: [PATCH 11/26] remove unnecessasry placeholder arg --- internal/command/registry/args.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/command/registry/args.go b/internal/command/registry/args.go index c015b226af..9dc86afbf2 100644 --- a/internal/command/registry/args.go +++ b/internal/command/registry/args.go @@ -142,7 +142,7 @@ func argsGetOrgImages(ctx context.Context, orgName string) ([]ImgInfo, error) { } var allImgs []ImgInfo - for n, _ := range apps { + for n := range apps { app := &apps[n] imgs, err := argsGetAppImages(ctx, app.Name) if err != nil { From 9fdd347ddea03a49bcd0ac8ee7924bd86adc0f77 Mon Sep 17 00:00:00 2001 From: Tim Newsham Date: Wed, 17 Jul 2024 12:35:27 -1000 Subject: [PATCH 12/26] address some comments - fetch images concurrently - prefer sets (maps) over lists for performance and clarity --- internal/command/registry/args.go | 55 ++++++++++-- internal/command/registry/filter.go | 19 ++++- internal/command/registry/vulnsummary.go | 104 ++++++++++++++++------- 3 files changed, 136 insertions(+), 42 deletions(-) diff --git a/internal/command/registry/args.go b/internal/command/registry/args.go index 9dc86afbf2..598a8a94aa 100644 --- a/internal/command/registry/args.go +++ b/internal/command/registry/args.go @@ -4,6 +4,8 @@ import ( "context" "errors" "fmt" + "slices" + "strings" "github.com/samber/lo" @@ -16,6 +18,8 @@ import ( "github.com/superfly/flyctl/internal/prompt" ) +type Unit struct{} + // ImgInfo carries image information for a machine. type ImgInfo struct { Org string @@ -26,6 +30,43 @@ type ImgInfo struct { Path string } +func (a ImgInfo) Compare(b ImgInfo) int { + if d := strings.Compare(a.Org, b.Org); d != 0 { + return d + } + if d := strings.Compare(a.OrgID, b.OrgID); d != 0 { + return d + } + if d := strings.Compare(a.App, b.App); d != 0 { + return d + } + if d := strings.Compare(a.AppID, b.AppID); d != 0 { + return d + } + if d := strings.Compare(a.Mach, b.Mach); d != 0 { + return d + } + if d := strings.Compare(a.Path, b.Path); d != 0 { + return d + } + return 0 +} + +// AugmentMap includes all of src into targ. +func AugmentMap[K comparable, V any](targ, src map[K]V) { + for k, v := range src { + targ[k] = v + } +} + +// SortedKeys returns the keys in a map in sorted order. +// Could be made generic. +func SortedKeys(m map[ImgInfo]Unit) []ImgInfo { + keys := lo.Keys(m) + slices.SortFunc(keys, func(a, b ImgInfo) int { return a.Compare(b) }) + return keys +} + // argsGetMachine returns the selected machine, using `select` and `machine`. func argsGetMachine(ctx context.Context, app *fly.AppCompact) (*fly.Machine, error) { if flag.IsSpecified(ctx, "machine") { @@ -116,7 +157,7 @@ func argsGetImgPath(ctx context.Context, app *fly.AppCompact) (string, error) { // argsGetImages returns a list of images in ImgInfo format from // command line args or the environment, using `org`, `app`, `running`. -func argsGetImages(ctx context.Context) ([]ImgInfo, error) { +func argsGetImages(ctx context.Context) (map[ImgInfo]Unit, error) { if appName := flag.GetApp(ctx); appName != "" { return argsGetAppImages(ctx, appName) } else if orgName := flag.GetOrg(ctx); orgName != "" { @@ -129,7 +170,7 @@ func argsGetImages(ctx context.Context) ([]ImgInfo, error) { // argsGetOrgImages returns a list of images for an org in ImgInfo format // from `running`. -func argsGetOrgImages(ctx context.Context, orgName string) ([]ImgInfo, error) { +func argsGetOrgImages(ctx context.Context, orgName string) (map[ImgInfo]Unit, error) { client := flyutil.ClientFromContext(ctx) org, err := client.GetOrganizationBySlug(ctx, orgName) if err != nil { @@ -141,14 +182,14 @@ func argsGetOrgImages(ctx context.Context, orgName string) ([]ImgInfo, error) { return nil, err } - var allImgs []ImgInfo + allImgs := make(map[ImgInfo]Unit) for n := range apps { app := &apps[n] imgs, err := argsGetAppImages(ctx, app.Name) if err != nil { return nil, fmt.Errorf("could not fetch images for %q app: %w", app.Name, err) } - allImgs = append(allImgs, imgs...) + AugmentMap(allImgs, imgs) } return allImgs, nil @@ -156,7 +197,7 @@ func argsGetOrgImages(ctx context.Context, orgName string) ([]ImgInfo, error) { // argsGetAppImages returns a list of images for an app in ImgInfo format // from `running`. -func argsGetAppImages(ctx context.Context, appName string) ([]ImgInfo, error) { +func argsGetAppImages(ctx context.Context, appName string) (map[ImgInfo]Unit, error) { apiClient := flyutil.ClientFromContext(ctx) app, err := apiClient.GetAppCompact(ctx, appName) if err != nil { @@ -184,7 +225,7 @@ func argsGetAppImages(ctx context.Context, appName string) ([]ImgInfo, error) { }) } - var imgs []ImgInfo + imgs := make(map[ImgInfo]Unit) for _, machine := range machines { ir := machine.ImageRef imgPath := fmt.Sprintf("%s/%s@%s", ir.Registry, ir.Repository, ir.Digest) @@ -197,7 +238,7 @@ func argsGetAppImages(ctx context.Context, appName string) ([]ImgInfo, error) { Mach: machine.Name, Path: imgPath, } - imgs = append(imgs, img) + imgs[img] = Unit{} } return imgs, nil } diff --git a/internal/command/registry/filter.go b/internal/command/registry/filter.go index f066870945..6f831b2c53 100644 --- a/internal/command/registry/filter.go +++ b/internal/command/registry/filter.go @@ -16,7 +16,7 @@ var allowedSeverities = []string{"LOW", "MEDIUM", "HIGH", "CRITICAL"} type VulnFilter struct { SeverityLevel int - VulnIds []string + VulnIds map[string]bool } func (p *VulnFilter) IsSpecified() bool { @@ -31,7 +31,18 @@ func argsGetVulnFilter(ctx context.Context) (*VulnFilter, error) { if flag.IsSpecified(ctx, "severity") && !lo.Contains(allowedSeverities, sev) { return nil, fmt.Errorf("severity (%s) must be one of %v", sev, allowedSeverities) } - return &VulnFilter{severityLevel(sev), vulnIds}, nil + + f := &VulnFilter{ + SeverityLevel: severityLevel(sev), + } + + if len(vulnIds) > 0 { + f.VulnIds = make(map[string]bool) + for _, vulnId := range vulnIds { + f.VulnIds[vulnId] = true + } + } + return f, nil } // severityLevel converts an `allowedSeverity` into an integer, @@ -47,8 +58,8 @@ func filterVuln(vuln *ScanVuln, filter *VulnFilter) bool { if filter.SeverityLevel > severityLevel(vuln.Severity) { return false } - if len(filter.VulnIds) > 0 { - if !lo.Contains(filter.VulnIds, vuln.VulnerabilityID) { + if filter.VulnIds != nil { + if !filter.VulnIds[vuln.VulnerabilityID] { return false } } diff --git a/internal/command/registry/vulnsummary.go b/internal/command/registry/vulnsummary.go index d3263dc88c..f0a96ca15f 100644 --- a/internal/command/registry/vulnsummary.go +++ b/internal/command/registry/vulnsummary.go @@ -6,16 +6,23 @@ import ( "fmt" "slices" "strings" + "sync" "github.com/samber/lo" "github.com/spf13/cobra" + "golang.org/x/sync/errgroup" "github.com/superfly/flyctl/internal/command" "github.com/superfly/flyctl/internal/flag" "github.com/superfly/flyctl/internal/render" + "github.com/superfly/flyctl/internal/spinner" "github.com/superfly/flyctl/iostreams" ) +const ( + concurrentScans = 5 // TODO: think about the right value here. +) + func newVulnSummary() *cobra.Command { const ( usage = "vulnsummary ... [flags]" @@ -62,42 +69,16 @@ func runVulnSummary(ctx context.Context) error { return err } - // fetch all image scans. - // TODO: spinner for long running fetches. - ios := iostreams.FromContext(ctx) - imageScan := map[string]*Scan{} - token := "" - tokenAppID := "" - for _, img := range imgs { - if _, ok := imageScan[img.Path]; ok { - continue - } - - if img.AppID != tokenAppID { - tokenAppID = img.AppID - token, err = makeScantronToken(ctx, img.OrgID, img.AppID) - if err != nil { - return err - } - } - - scan, err := getVulnScan(ctx, img.Path, token) - if err != nil { - errUnsupportedPath := ErrUnsupportedPath("") - if errors.As(err, &errUnsupportedPath) { - fmt.Fprintf(ios.Out, "Skipping %s (%s) from unsupported repository: %s\n", img.App, img.Mach, img.Path) - continue - } - return fmt.Errorf("Getting vulnerability scan for %s (%s): %w", img.App, img.Mach, err) - } - imageScan[img.Path] = filterScan(scan, filter) + imageScan, err := fetchImageScans(ctx, imgs, filter) + if err != nil { + return err } // calculate findings tables allVids := map[string]bool{} vidsByApp := map[string]map[string]bool{} appImgsScanned := map[string]bool{} - for _, img := range imgs { + for img, _ := range imgs { scan := imageScan[img.Path] if scan == nil { continue @@ -124,10 +105,11 @@ func runVulnSummary(ctx context.Context) error { } // Show what is being scanned. + ios := iostreams.FromContext(ctx) lastOrg := "" lastApp := "" fmt.Fprintf(ios.Out, "Scanned images\n") - for _, img := range imgs { + for _, img := range SortedKeys(imgs) { scan := imageScan[img.Path] if img.Org != lastOrg { fmt.Fprintf(ios.Out, "Org: %s\n", img.Org) @@ -169,3 +151,63 @@ func runVulnSummary(ctx context.Context) error { return nil } + +// fetchImageScans returns a scan for each image path. +func fetchImageScans(ctx context.Context, imgs map[ImgInfo]Unit, filter *VulnFilter) (map[string]*Scan, error) { + // TODO: spinner for long running fetches. + ios := iostreams.FromContext(ctx) + spin := spinner.Run(ios, "Scanning...") + defer spin.Stop() + + eg, ctx := errgroup.WithContext(ctx) + eg.SetLimit(concurrentScans) + mu := sync.Mutex{} + imageScan := make(map[string]*Scan) + skipped := make(map[ImgInfo]Unit) + for img, _ := range imgs { + img := img + mu.Lock() + _, ok := imageScan[img.Path] + if ok { + mu.Unlock() + continue + } + imageScan[img.Path] = nil + mu.Unlock() + + eg.Go(func() error { + // TODO: fix me. dont make new token each time. make org-wide token. + token, err := makeScantronToken(ctx, img.OrgID, img.AppID) + if err != nil { + return err + } + + scan, err := getVulnScan(ctx, img.Path, token) + if err != nil { + errUnsupportedPath := ErrUnsupportedPath("") + if errors.As(err, &errUnsupportedPath) { + mu.Lock() + skipped[img] = Unit{} + mu.Unlock() + return nil + } + return fmt.Errorf("Getting vulnerability scan for %s (%s): %w", img.App, img.Mach, err) + } + + scan = filterScan(scan, filter) + mu.Lock() + imageScan[img.Path] = scan + mu.Unlock() + return nil + }) + } + if err := eg.Wait(); err != nil { + return nil, err + } + spin.Stop() + + for _, img := range SortedKeys(skipped) { + fmt.Fprintf(ios.Out, "Skipping %s (%s) from unsupported repository: %s\n", img.App, img.Mach, img.Path) + } + return imageScan, nil +} From 106d6ffbde3c028bdf950ebe56e9b1e4eb3b5dbd Mon Sep 17 00:00:00 2001 From: Tim Newsham Date: Wed, 17 Jul 2024 13:11:11 -1000 Subject: [PATCH 13/26] use a single org token and provide spinners --- internal/command/registry/args.go | 6 ++++++ internal/command/registry/sbom.go | 2 +- internal/command/registry/scantron.go | 6 ++---- internal/command/registry/vulns.go | 2 +- internal/command/registry/vulnsummary.go | 20 ++++++++++++++------ 5 files changed, 24 insertions(+), 12 deletions(-) diff --git a/internal/command/registry/args.go b/internal/command/registry/args.go index 598a8a94aa..460c9559db 100644 --- a/internal/command/registry/args.go +++ b/internal/command/registry/args.go @@ -16,6 +16,8 @@ import ( "github.com/superfly/flyctl/internal/flapsutil" "github.com/superfly/flyctl/internal/flyutil" "github.com/superfly/flyctl/internal/prompt" + "github.com/superfly/flyctl/internal/spinner" + "github.com/superfly/flyctl/iostreams" ) type Unit struct{} @@ -158,6 +160,10 @@ func argsGetImgPath(ctx context.Context, app *fly.AppCompact) (string, error) { // argsGetImages returns a list of images in ImgInfo format from // command line args or the environment, using `org`, `app`, `running`. func argsGetImages(ctx context.Context) (map[ImgInfo]Unit, error) { + ios := iostreams.FromContext(ctx) + spin := spinner.Run(ios, "Finding images...") + defer spin.Stop() + if appName := flag.GetApp(ctx); appName != "" { return argsGetAppImages(ctx, appName) } else if orgName := flag.GetOrg(ctx); orgName != "" { diff --git a/internal/command/registry/sbom.go b/internal/command/registry/sbom.go index c62ffe6e82..b777ba218d 100644 --- a/internal/command/registry/sbom.go +++ b/internal/command/registry/sbom.go @@ -69,7 +69,7 @@ func runSbom(ctx context.Context) error { return err } - token, err := makeScantronToken(ctx, app.Organization.ID, app.ID) + token, err := makeScantronToken(ctx, app.Organization.ID) if err != nil { return err } diff --git a/internal/command/registry/scantron.go b/internal/command/registry/scantron.go index fc86d1f0ca..5b6c3cab87 100644 --- a/internal/command/registry/scantron.go +++ b/internal/command/registry/scantron.go @@ -117,10 +117,8 @@ func getVulnScan(ctx context.Context, imgPath, token string) (*Scan, error) { return scan, nil } -func makeScantronToken(ctx context.Context, orgId, appId string) (string, error) { - resp, err := makeToken(ctx, scantronTokenName, orgId, scantronTokenLife, "deploy", &gql.LimitedAccessTokenOptions{ - "app_id": appId, - }) +func makeScantronToken(ctx context.Context, orgId string) (string, error) { + resp, err := makeToken(ctx, scantronTokenName, orgId, scantronTokenLife, "read_organization_apps", &gql.LimitedAccessTokenOptions{}) if err != nil { return "", err } diff --git a/internal/command/registry/vulns.go b/internal/command/registry/vulns.go index b81c2831ed..6d42c44490 100644 --- a/internal/command/registry/vulns.go +++ b/internal/command/registry/vulns.go @@ -86,7 +86,7 @@ func runVulns(ctx context.Context) error { return err } - token, err := makeScantronToken(ctx, app.Organization.ID, app.ID) + token, err := makeScantronToken(ctx, app.Organization.ID) if err != nil { return err } diff --git a/internal/command/registry/vulnsummary.go b/internal/command/registry/vulnsummary.go index f0a96ca15f..2933f86c59 100644 --- a/internal/command/registry/vulnsummary.go +++ b/internal/command/registry/vulnsummary.go @@ -159,6 +159,19 @@ func fetchImageScans(ctx context.Context, imgs map[ImgInfo]Unit, filter *VulnFil spin := spinner.Run(ios, "Scanning...") defer spin.Stop() + // Make all org tokens. Right now there will only be one. + orgToken := make(map[string]string) + for img, _ := range imgs { + if _, ok := orgToken[img.OrgID]; ok { + continue + } + token, err := makeScantronToken(ctx, img.OrgID) + if err != nil { + return nil, err + } + orgToken[img.OrgID] = token + } + eg, ctx := errgroup.WithContext(ctx) eg.SetLimit(concurrentScans) mu := sync.Mutex{} @@ -173,15 +186,10 @@ func fetchImageScans(ctx context.Context, imgs map[ImgInfo]Unit, filter *VulnFil continue } imageScan[img.Path] = nil + token := orgToken[img.OrgID] mu.Unlock() eg.Go(func() error { - // TODO: fix me. dont make new token each time. make org-wide token. - token, err := makeScantronToken(ctx, img.OrgID, img.AppID) - if err != nil { - return err - } - scan, err := getVulnScan(ctx, img.Path, token) if err != nil { errUnsupportedPath := ErrUnsupportedPath("") From 355f899cb332c51bb82b73bfe113c375cb5d2e46 Mon Sep 17 00:00:00 2001 From: Tim Newsham Date: Wed, 17 Jul 2024 14:45:09 -1000 Subject: [PATCH 14/26] concurrent image lookup and refactoring of args processing --- internal/command/registry/args.go | 88 +++++++++++++++++------- internal/command/registry/sbom.go | 18 +---- internal/command/registry/vulns.go | 13 +--- internal/command/registry/vulnsummary.go | 1 - 4 files changed, 68 insertions(+), 52 deletions(-) diff --git a/internal/command/registry/args.go b/internal/command/registry/args.go index 460c9559db..8d4566809d 100644 --- a/internal/command/registry/args.go +++ b/internal/command/registry/args.go @@ -6,8 +6,10 @@ import ( "fmt" "slices" "strings" + "sync" "github.com/samber/lo" + "golang.org/x/sync/errgroup" fly "github.com/superfly/fly-go" "github.com/superfly/fly-go/flaps" @@ -69,7 +71,29 @@ func SortedKeys(m map[ImgInfo]Unit) []ImgInfo { return keys } -// argsGetMachine returns the selected machine, using `select` and `machine`. +// argsGetAppCompact returns the AppCompact for the selected app, using `app`. +func argsGetAppCompact(ctx context.Context) (*fly.AppCompact, error) { + appName := appconfig.NameFromContext(ctx) + apiClient := flyutil.ClientFromContext(ctx) + app, err := apiClient.GetAppCompact(ctx, appName) + if err != nil { + return nil, fmt.Errorf("failed to get app: %w", err) + } + return app, nil +} + +func getFlapsClient(ctx context.Context, app *fly.AppCompact) (*flaps.Client, error) { + flapsClient, err := flapsutil.NewClientWithOptions(ctx, flaps.NewClientOpts{ + AppCompact: app, + AppName: app.Name, + }) + if err != nil { + return nil, fmt.Errorf("failed to create flaps client for app %s: %w", app.Name, err) + } + return flapsClient, nil +} + +// argsGetMachine returns the selected machine, using `app`, `select` and `machine`. func argsGetMachine(ctx context.Context, app *fly.AppCompact) (*fly.Machine, error) { if flag.IsSpecified(ctx, "machine") { if flag.IsSpecified(ctx, "select") { @@ -86,12 +110,9 @@ func argsGetMachine(ctx context.Context, app *fly.AppCompact) (*fly.Machine, err func argsSelectMachine(ctx context.Context, app *fly.AppCompact) (*fly.Machine, error) { anyMachine := !flag.GetBool(ctx, "select") - flapsClient, err := flapsutil.NewClientWithOptions(ctx, flaps.NewClientOpts{ - AppCompact: app, - AppName: app.Name, - }) + flapsClient, err := getFlapsClient(ctx, app) if err != nil { - return nil, fmt.Errorf("failed to create flaps client for app %s: %w", app.Name, err) + return nil, err } machines, err := flapsClient.ListActive(ctx) @@ -122,12 +143,9 @@ func argsSelectMachine(ctx context.Context, app *fly.AppCompact) (*fly.Machine, // argsGetMachineByID returns an app's machine using the `machine` argument. func argsGetMachineByID(ctx context.Context, app *fly.AppCompact) (*fly.Machine, error) { - flapsClient, err := flapsutil.NewClientWithOptions(ctx, flaps.NewClientOpts{ - AppCompact: app, - AppName: app.Name, - }) + flapsClient, err := getFlapsClient(ctx, app) if err != nil { - return nil, fmt.Errorf("failed to create flaps client for app %s: %w", app.Name, err) + return nil, err } machineID := flag.GetString(ctx, "machine") @@ -139,22 +157,29 @@ func argsGetMachineByID(ctx context.Context, app *fly.AppCompact) (*fly.Machine, return machine, nil } -// argsGetImgPath returns an image path from the command line or from a -// selected app machine, using `image`, `select`, and `machine`. -func argsGetImgPath(ctx context.Context, app *fly.AppCompact) (string, error) { +// argsGetImgPath returns an image path and its OrgID from the command line or from a +// selected app machine, using `app`, `image`, `select`, and `machine`. +func argsGetImgPath(ctx context.Context) (string, string, error) { + app, err := argsGetAppCompact(ctx) + if err != nil { + return "", "", err + } + if flag.IsSpecified(ctx, "image") { if flag.IsSpecified(ctx, "machine") || flag.IsSpecified(ctx, "select") { - return "", fmt.Errorf("image option cannot be used with machien and select options") + return "", "", fmt.Errorf("image option cannot be used with machine and select options") } - return flag.GetString(ctx, "image"), nil + + path := flag.GetString(ctx, "image") + return path, app.Organization.ID, nil } machine, err := argsGetMachine(ctx, app) if err != nil { - return "", err + return "", "", err } - return imageRefPath(&machine.ImageRef), nil + return imageRefPath(&machine.ImageRef), app.Organization.ID, nil } // argsGetImages returns a list of images in ImgInfo format from @@ -188,17 +213,30 @@ func argsGetOrgImages(ctx context.Context, orgName string) (map[ImgInfo]Unit, er return nil, err } + eg, ctx := errgroup.WithContext(ctx) + eg.SetLimit(concurrentScans) + mu := sync.Mutex{} allImgs := make(map[ImgInfo]Unit) for n := range apps { - app := &apps[n] - imgs, err := argsGetAppImages(ctx, app.Name) - if err != nil { - return nil, fmt.Errorf("could not fetch images for %q app: %w", app.Name, err) - } - AugmentMap(allImgs, imgs) + n := n + eg.Go(func() error { + app := &apps[n] + imgs, err := argsGetAppImages(ctx, app.Name) + if err != nil { + return fmt.Errorf("could not fetch images for %q app: %w", app.Name, err) + } + + mu.Lock() + AugmentMap(allImgs, imgs) + mu.Unlock() + return nil + }) + } + if err := eg.Wait(); err != nil { + return nil, err } - return allImgs, nil + return allImgs, nil } // argsGetAppImages returns a list of images for an app in ImgInfo format diff --git a/internal/command/registry/sbom.go b/internal/command/registry/sbom.go index b777ba218d..e750c3f0a6 100644 --- a/internal/command/registry/sbom.go +++ b/internal/command/registry/sbom.go @@ -8,10 +8,8 @@ import ( "github.com/spf13/cobra" - "github.com/superfly/flyctl/internal/appconfig" "github.com/superfly/flyctl/internal/command" "github.com/superfly/flyctl/internal/flag" - "github.com/superfly/flyctl/internal/flyutil" "github.com/superfly/flyctl/iostreams" ) @@ -53,23 +51,12 @@ func newSbom() *cobra.Command { } func runSbom(ctx context.Context) error { - var ( - ios = iostreams.FromContext(ctx) - appName = appconfig.NameFromContext(ctx) - apiClient = flyutil.ClientFromContext(ctx) - ) - - app, err := apiClient.GetAppCompact(ctx, appName) - if err != nil { - return fmt.Errorf("failed to get app: %w", err) - } - - imgPath, err := argsGetImgPath(ctx, app) + imgPath, orgId, err := argsGetImgPath(ctx) if err != nil { return err } - token, err := makeScantronToken(ctx, app.Organization.ID) + token, err := makeScantronToken(ctx, orgId) if err != nil { return err } @@ -84,6 +71,7 @@ func runSbom(ctx context.Context) error { return fmt.Errorf("failed fetching SBOM (status code %d)", res.StatusCode) } + ios := iostreams.FromContext(ctx) if _, err := io.Copy(ios.Out, res.Body); err != nil { return fmt.Errorf("failed to read SBOM: %w", err) } diff --git a/internal/command/registry/vulns.go b/internal/command/registry/vulns.go index 6d42c44490..d0f8a4d36d 100644 --- a/internal/command/registry/vulns.go +++ b/internal/command/registry/vulns.go @@ -9,10 +9,8 @@ import ( "github.com/spf13/cobra" - "github.com/superfly/flyctl/internal/appconfig" "github.com/superfly/flyctl/internal/command" "github.com/superfly/flyctl/internal/flag" - "github.com/superfly/flyctl/internal/flyutil" "github.com/superfly/flyctl/iostreams" ) @@ -74,19 +72,12 @@ func runVulns(ctx context.Context) error { return fmt.Errorf("filtering by severity or CVE is not supported when outputting JSON") } - apiClient := flyutil.ClientFromContext(ctx) - appName := appconfig.NameFromContext(ctx) - app, err := apiClient.GetAppCompact(ctx, appName) - if err != nil { - return fmt.Errorf("failed to get app: %w", err) - } - - imgPath, err := argsGetImgPath(ctx, app) + imgPath, orgId, err := argsGetImgPath(ctx) if err != nil { return err } - token, err := makeScantronToken(ctx, app.Organization.ID) + token, err := makeScantronToken(ctx, orgId) if err != nil { return err } diff --git a/internal/command/registry/vulnsummary.go b/internal/command/registry/vulnsummary.go index 2933f86c59..62a5266a77 100644 --- a/internal/command/registry/vulnsummary.go +++ b/internal/command/registry/vulnsummary.go @@ -154,7 +154,6 @@ func runVulnSummary(ctx context.Context) error { // fetchImageScans returns a scan for each image path. func fetchImageScans(ctx context.Context, imgs map[ImgInfo]Unit, filter *VulnFilter) (map[string]*Scan, error) { - // TODO: spinner for long running fetches. ios := iostreams.FromContext(ctx) spin := spinner.Run(ios, "Scanning...") defer spin.Stop() From 7df90fce78da36fe9930433d2a9e3681dda598ed Mon Sep 17 00:00:00 2001 From: Tim Newsham Date: Wed, 17 Jul 2024 15:07:22 -1000 Subject: [PATCH 15/26] remove need for looking up each AppCompact - Allow flaps client without AppCompact by passing OrgSlug. - Refactor code to skip looking up AppCompact and use OrgSlug instead when scanning all apps in an org --- internal/command/registry/args.go | 30 ++++++++++++------------ internal/command/registry/vulnsummary.go | 8 +++++-- 2 files changed, 21 insertions(+), 17 deletions(-) diff --git a/internal/command/registry/args.go b/internal/command/registry/args.go index 8d4566809d..55e631fe43 100644 --- a/internal/command/registry/args.go +++ b/internal/command/registry/args.go @@ -29,7 +29,6 @@ type ImgInfo struct { Org string OrgID string App string - AppID string Mach string Path string } @@ -44,9 +43,6 @@ func (a ImgInfo) Compare(b ImgInfo) int { if d := strings.Compare(a.App, b.App); d != 0 { return d } - if d := strings.Compare(a.AppID, b.AppID); d != 0 { - return d - } if d := strings.Compare(a.Mach, b.Mach); d != 0 { return d } @@ -221,7 +217,7 @@ func argsGetOrgImages(ctx context.Context, orgName string) (map[ImgInfo]Unit, er n := n eg.Go(func() error { app := &apps[n] - imgs, err := argsGetAppImages(ctx, app.Name) + imgs, err := argsGetOrgAppImages(ctx, org.Name, org.ID, app.Name) if err != nil { return fmt.Errorf("could not fetch images for %q app: %w", app.Name, err) } @@ -248,15 +244,20 @@ func argsGetAppImages(ctx context.Context, appName string) (map[ImgInfo]Unit, er return nil, fmt.Errorf("failed to get app %q: %w", appName, err) } + org := app.Organization + return argsGetOrgAppImages(ctx, org.Name, org.ID, app.Name) +} + +// argsGetOrgAppImages returns a list of images for an org/app in ImgInfo format +// from `running`. +func argsGetOrgAppImages(ctx context.Context, orgName, orgId, appName string) (map[ImgInfo]Unit, error) { flapsClient, err := flapsutil.NewClientWithOptions(ctx, flaps.NewClientOpts{ - AppCompact: app, - AppName: app.Name, + OrgSlug: orgId, + AppName: appName, }) if err != nil { return nil, fmt.Errorf("failed to create flaps client for %q: %w", appName, err) } - org := app.Organization - ctx = flapsutil.NewContextWithClient(ctx, flapsClient) machines, err := flapsClient.ListActive(ctx) if err != nil { @@ -275,12 +276,11 @@ func argsGetAppImages(ctx context.Context, appName string) (map[ImgInfo]Unit, er imgPath := fmt.Sprintf("%s/%s@%s", ir.Registry, ir.Repository, ir.Digest) img := ImgInfo{ - Org: org.Name, - OrgID: org.ID, - App: app.Name, - AppID: app.ID, - Mach: machine.Name, - Path: imgPath, + Org: orgName, + OrgID: orgId, + App: appName, + Mach: machine.Name, + Path: imgPath, } imgs[img] = Unit{} } diff --git a/internal/command/registry/vulnsummary.go b/internal/command/registry/vulnsummary.go index 62a5266a77..7cd718117c 100644 --- a/internal/command/registry/vulnsummary.go +++ b/internal/command/registry/vulnsummary.go @@ -75,16 +75,20 @@ func runVulnSummary(ctx context.Context) error { } // calculate findings tables + type AppPath struct { + App string + Path string + } allVids := map[string]bool{} vidsByApp := map[string]map[string]bool{} - appImgsScanned := map[string]bool{} + appImgsScanned := map[AppPath]bool{} for img, _ := range imgs { scan := imageScan[img.Path] if scan == nil { continue } - k := fmt.Sprintf("%s/%s", img.AppID, img.Path) + k := AppPath{img.App, img.Path} if _, ok := appImgsScanned[k]; ok { continue } From 42b832ec409478120f3e3ff00f40d9dd935a6d90 Mon Sep 17 00:00:00 2001 From: Tim Newsham Date: Wed, 17 Jul 2024 15:12:33 -1000 Subject: [PATCH 16/26] gofmt --- internal/command/registry/args.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/command/registry/args.go b/internal/command/registry/args.go index 55e631fe43..7367f3b470 100644 --- a/internal/command/registry/args.go +++ b/internal/command/registry/args.go @@ -279,8 +279,8 @@ func argsGetOrgAppImages(ctx context.Context, orgName, orgId, appName string) (m Org: orgName, OrgID: orgId, App: appName, - Mach: machine.Name, - Path: imgPath, + Mach: machine.Name, + Path: imgPath, } imgs[img] = Unit{} } From 26f92db584d49779490f6406b7f99b7b40a98e57 Mon Sep 17 00:00:00 2001 From: Tim Newsham Date: Wed, 17 Jul 2024 15:22:16 -1000 Subject: [PATCH 17/26] forgot to commit flapsutil changes earlier. --- internal/flapsutil/flapsutil.go | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/internal/flapsutil/flapsutil.go b/internal/flapsutil/flapsutil.go index a44ccaff32..a416dc8b10 100644 --- a/internal/flapsutil/flapsutil.go +++ b/internal/flapsutil/flapsutil.go @@ -21,9 +21,12 @@ import ( func NewClientWithOptions(ctx context.Context, opts flaps.NewClientOpts) (*flaps.Client, error) { // Connect over wireguard depending on FLAPS URL. if strings.TrimSpace(strings.ToLower(os.Getenv("FLY_FLAPS_BASE_URL"))) == "peer" { - orgSlug, err := resolveOrgSlugForApp(ctx, opts.AppCompact, opts.AppName) - if err != nil { - return nil, fmt.Errorf("failed to resolve org for app '%s': %w", opts.AppName, err) + if opts.OrgSlug == "" { + orgSlug, err := resolveOrgSlugForApp(ctx, opts.AppCompact, opts.AppName) + if err != nil { + return nil, fmt.Errorf("failed to resolve org for app '%s': %w", opts.AppName, err) + } + opts.OrgSlug = orgSlug } client := flyutil.ClientFromContext(ctx) @@ -32,12 +35,11 @@ func NewClientWithOptions(ctx context.Context, opts flaps.NewClientOpts) (*flaps return nil, fmt.Errorf("error establishing agent: %w", err) } - dialer, err := agentclient.Dialer(ctx, orgSlug, "") + dialer, err := agentclient.Dialer(ctx, opts.OrgSlug, "") if err != nil { - return nil, fmt.Errorf("flaps: can't build tunnel for %s: %w", orgSlug, err) + return nil, fmt.Errorf("flaps: can't build tunnel for %s: %w", opts.OrgSlug, err) } opts.DialContext = dialer.DialContext - opts.OrgSlug = orgSlug flapsBaseUrlString := fmt.Sprintf("http://[%s]:4280", resolvePeerIP(dialer.State().Peer.Peerip)) if opts.BaseURL, err = url.Parse(flapsBaseUrlString); err != nil { From 893dfa3c9c6de44b97be07316c2072742766286d Mon Sep 17 00:00:00 2001 From: Tim Newsham Date: Wed, 17 Jul 2024 15:33:44 -1000 Subject: [PATCH 18/26] fix lint --- internal/command/registry/vulnsummary.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/command/registry/vulnsummary.go b/internal/command/registry/vulnsummary.go index 7cd718117c..0c8d4f8a48 100644 --- a/internal/command/registry/vulnsummary.go +++ b/internal/command/registry/vulnsummary.go @@ -82,7 +82,7 @@ func runVulnSummary(ctx context.Context) error { allVids := map[string]bool{} vidsByApp := map[string]map[string]bool{} appImgsScanned := map[AppPath]bool{} - for img, _ := range imgs { + for img := range imgs { scan := imageScan[img.Path] if scan == nil { continue @@ -164,7 +164,7 @@ func fetchImageScans(ctx context.Context, imgs map[ImgInfo]Unit, filter *VulnFil // Make all org tokens. Right now there will only be one. orgToken := make(map[string]string) - for img, _ := range imgs { + for img := range imgs { if _, ok := orgToken[img.OrgID]; ok { continue } @@ -180,7 +180,7 @@ func fetchImageScans(ctx context.Context, imgs map[ImgInfo]Unit, filter *VulnFil mu := sync.Mutex{} imageScan := make(map[string]*Scan) skipped := make(map[ImgInfo]Unit) - for img, _ := range imgs { + for img := range imgs { img := img mu.Lock() _, ok := imageScan[img.Path] From e15fac3a98d880d073e3f68402a8d7f3e82e6103 Mon Sep 17 00:00:00 2001 From: Tim Newsham Date: Wed, 17 Jul 2024 16:26:17 -1000 Subject: [PATCH 19/26] use registry_token and throw out attenuation which is no longer needed --- internal/command/registry/auth.go | 38 --------------------------- internal/command/registry/scantron.go | 18 +------------ 2 files changed, 1 insertion(+), 55 deletions(-) diff --git a/internal/command/registry/auth.go b/internal/command/registry/auth.go index a877c95639..0197f8f6b5 100644 --- a/internal/command/registry/auth.go +++ b/internal/command/registry/auth.go @@ -4,18 +4,10 @@ import ( "context" "fmt" - "github.com/superfly/macaroon" - "github.com/superfly/macaroon/flyio" - "github.com/superfly/flyctl/gql" "github.com/superfly/flyctl/internal/flyutil" ) -const ( - appFeatureImages = "images" - orgFeatureBuilder = "builder" -) - func makeToken(ctx context.Context, name, orgID, expiry, profile string, options *gql.LimitedAccessTokenOptions) (*gql.CreateLimitedAccessTokenResponse, error) { apiClient := flyutil.ClientFromContext(ctx) resp, err := gql.CreateLimitedAccessToken( @@ -32,33 +24,3 @@ func makeToken(ctx context.Context, name, orgID, expiry, profile string, options } return resp, nil } - -func attenuateTokens(tokenHeader string, caveats ...macaroon.Caveat) (string, error) { - toks, err := macaroon.Parse(tokenHeader) - if err != nil { - return "", fmt.Errorf("failed to parse token: %w", err) - } - - perms, _, _, retToks, err := macaroon.FindPermissionAndDischargeTokens(toks, flyio.LocationPermission) - switch { - case err != nil: - return "", fmt.Errorf("failed to find permission tokens: %w", err) - case len(perms) == 0: - return "", fmt.Errorf("no permission tokens found") - } - - for _, perm := range perms { - if err := perm.Add(caveats...); err != nil { - return "", fmt.Errorf("failed to attenuate token: %w", err) - } - - tok, err := perm.Encode() - if err != nil { - return "", fmt.Errorf("failed to encode token: %w", err) - } - - retToks = append(retToks, tok) - } - - return macaroon.ToAuthorizationHeader(retToks...), nil -} diff --git a/internal/command/registry/scantron.go b/internal/command/registry/scantron.go index 5b6c3cab87..e233349444 100644 --- a/internal/command/registry/scantron.go +++ b/internal/command/registry/scantron.go @@ -10,9 +10,6 @@ import ( fly "github.com/superfly/fly-go" "github.com/superfly/flyctl/gql" - "github.com/superfly/macaroon" - "github.com/superfly/macaroon/flyio" - "github.com/superfly/macaroon/resset" "github.com/superfly/flyctl/internal/buildinfo" ) @@ -118,24 +115,11 @@ func getVulnScan(ctx context.Context, imgPath, token string) (*Scan, error) { } func makeScantronToken(ctx context.Context, orgId string) (string, error) { - resp, err := makeToken(ctx, scantronTokenName, orgId, scantronTokenLife, "read_organization_apps", &gql.LimitedAccessTokenOptions{}) + resp, err := makeToken(ctx, scantronTokenName, orgId, scantronTokenLife, "registry_token", &gql.LimitedAccessTokenOptions{}) if err != nil { return "", err } token := resp.CreateLimitedAccessToken.LimitedAccessToken.TokenHeader - token, err = attenuateTokens(token, - &resset.IfPresent{ - Ifs: macaroon.NewCaveatSet( - &flyio.FeatureSet{Features: resset.New[string](resset.ActionRead, orgFeatureBuilder)}, - &flyio.AppFeatureSet{Features: resset.New[string](resset.ActionRead, appFeatureImages)}, - ), - Else: resset.ActionNone, - }, - ) - if err != nil { - return "", err - } - return token, nil } From 38561e19742020ae1bf53315098e5661af591c1a Mon Sep 17 00:00:00 2001 From: Tim Newsham Date: Thu, 18 Jul 2024 09:25:54 -1000 Subject: [PATCH 20/26] hide the experimental registry command from help --- internal/command/root/root.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/internal/command/root/root.go b/internal/command/root/root.go index b393c6c0ed..813c9206c7 100644 --- a/internal/command/root/root.go +++ b/internal/command/root/root.go @@ -88,6 +88,10 @@ func New() *cobra.Command { flyctl.InitConfig() + hidden := func(cmd *cobra.Command) *cobra.Command { + cmd.Hidden = true + return cmd + } root.AddCommand( group(apps.New(), "deploy"), group(machine.New(), "deploy"), @@ -116,7 +120,7 @@ func New() *cobra.Command { group(ssh.New(), "upkeep"), group(ssh.NewSFTP(), "upkeep"), group(redis.New(), "dbs_and_extensions"), - group(registry.New(), "upkeep"), + hidden(registry.New()), group(checks.New(), "upkeep"), group(launch.New(), "deploy"), group(info.New(), "upkeep"), From ce07204b765750ddb6065db811cc6f7b8a58369b Mon Sep 17 00:00:00 2001 From: Tim Newsham Date: Thu, 18 Jul 2024 09:28:34 -1000 Subject: [PATCH 21/26] hide the registry command a cleaner way --- internal/command/registry/command.go | 1 + internal/command/root/root.go | 6 +----- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/internal/command/registry/command.go b/internal/command/registry/command.go index d7fc71fb6a..f2ce452bf2 100644 --- a/internal/command/registry/command.go +++ b/internal/command/registry/command.go @@ -13,6 +13,7 @@ func New() *cobra.Command { long = "Scan registry images for an SBOM or vulnerabilities." ) cmd := command.New(usage, short, long, nil) + cmd.Hidden = true cmd.AddCommand( newSbom(), diff --git a/internal/command/root/root.go b/internal/command/root/root.go index 813c9206c7..b393c6c0ed 100644 --- a/internal/command/root/root.go +++ b/internal/command/root/root.go @@ -88,10 +88,6 @@ func New() *cobra.Command { flyctl.InitConfig() - hidden := func(cmd *cobra.Command) *cobra.Command { - cmd.Hidden = true - return cmd - } root.AddCommand( group(apps.New(), "deploy"), group(machine.New(), "deploy"), @@ -120,7 +116,7 @@ func New() *cobra.Command { group(ssh.New(), "upkeep"), group(ssh.NewSFTP(), "upkeep"), group(redis.New(), "dbs_and_extensions"), - hidden(registry.New()), + group(registry.New(), "upkeep"), group(checks.New(), "upkeep"), group(launch.New(), "deploy"), group(info.New(), "upkeep"), From 568e48d360940cc8e3c77499ef293908a1667a6f Mon Sep 17 00:00:00 2001 From: Tim Newsham Date: Thu, 18 Jul 2024 10:14:58 -1000 Subject: [PATCH 22/26] better error handling when fetching scan results. continue summary even if some image scan fetches fail. --- internal/command/registry/scantron.go | 10 +++++++++- internal/command/registry/vulnsummary.go | 19 +++++++++++-------- 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/internal/command/registry/scantron.go b/internal/command/registry/scantron.go index e233349444..f10847b2ab 100644 --- a/internal/command/registry/scantron.go +++ b/internal/command/registry/scantron.go @@ -6,6 +6,7 @@ import ( "fmt" "net/http" "os" + "strings" "time" fly "github.com/superfly/fly-go" @@ -101,7 +102,14 @@ func getVulnScan(ctx context.Context, imgPath, token string) (*Scan, error) { if res.StatusCode == 422 { return nil, ErrUnsupportedPath(imgPath) } - return nil, fmt.Errorf("fetching scan results, status code %d", res.StatusCode) + + buf := make([]byte, 512) + n, _ := res.Body.Read(buf) + msg := strings.TrimSuffix(string(buf[:n]), "\n") + if msg == "" { + msg = "undetermined" + } + return nil, fmt.Errorf("status code %d: %q", res.StatusCode, msg) } scan := &Scan{} diff --git a/internal/command/registry/vulnsummary.go b/internal/command/registry/vulnsummary.go index 0c8d4f8a48..df363a1142 100644 --- a/internal/command/registry/vulnsummary.go +++ b/internal/command/registry/vulnsummary.go @@ -179,7 +179,7 @@ func fetchImageScans(ctx context.Context, imgs map[ImgInfo]Unit, filter *VulnFil eg.SetLimit(concurrentScans) mu := sync.Mutex{} imageScan := make(map[string]*Scan) - skipped := make(map[ImgInfo]Unit) + skipped := make(map[ImgInfo]string) for img := range imgs { img := img mu.Lock() @@ -196,13 +196,16 @@ func fetchImageScans(ctx context.Context, imgs map[ImgInfo]Unit, filter *VulnFil scan, err := getVulnScan(ctx, img.Path, token) if err != nil { errUnsupportedPath := ErrUnsupportedPath("") + var msg string if errors.As(err, &errUnsupportedPath) { - mu.Lock() - skipped[img] = Unit{} - mu.Unlock() - return nil + msg = "from unsupported repository" + } else { + msg = fmt.Sprintf("error getting scan: %v", err) } - return fmt.Errorf("Getting vulnerability scan for %s (%s): %w", img.App, img.Mach, err) + mu.Lock() + skipped[img] = msg + mu.Unlock() + return nil } scan = filterScan(scan, filter) @@ -217,8 +220,8 @@ func fetchImageScans(ctx context.Context, imgs map[ImgInfo]Unit, filter *VulnFil } spin.Stop() - for _, img := range SortedKeys(skipped) { - fmt.Fprintf(ios.Out, "Skipping %s (%s) from unsupported repository: %s\n", img.App, img.Mach, img.Path) + for img, msg := range skipped { + fmt.Fprintf(ios.Out, "Skipping %s (%s) %s\n", img.App, img.Mach, msg) } return imageScan, nil } From d8d25997b038824f9af94763e7f712691e0e29a8 Mon Sep 17 00:00:00 2001 From: Tim Newsham Date: Thu, 18 Jul 2024 10:23:46 -1000 Subject: [PATCH 23/26] fix error message when using a bogus org name --- internal/command/registry/args.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/command/registry/args.go b/internal/command/registry/args.go index 7367f3b470..047e2b4cb1 100644 --- a/internal/command/registry/args.go +++ b/internal/command/registry/args.go @@ -201,12 +201,12 @@ func argsGetOrgImages(ctx context.Context, orgName string) (map[ImgInfo]Unit, er client := flyutil.ClientFromContext(ctx) org, err := client.GetOrganizationBySlug(ctx, orgName) if err != nil { - return nil, err + return nil, fmt.Errorf("failed to get org %q: %w", orgName, err) } apps, err := client.GetAppsForOrganization(ctx, org.ID) if err != nil { - return nil, err + return nil, fmt.Errorf("failed to list apps for %q: %w", orgName, err) } eg, ctx := errgroup.WithContext(ctx) @@ -219,7 +219,7 @@ func argsGetOrgImages(ctx context.Context, orgName string) (map[ImgInfo]Unit, er app := &apps[n] imgs, err := argsGetOrgAppImages(ctx, org.Name, org.ID, app.Name) if err != nil { - return fmt.Errorf("could not fetch images for %q app: %w", app.Name, err) + return fmt.Errorf("failed to fetch images for %q app: %w", app.Name, err) } mu.Lock() From 6855fc0564f41ce410cef9b74d10b13fe172f388 Mon Sep 17 00:00:00 2001 From: Tim Newsham Date: Thu, 18 Jul 2024 10:31:14 -1000 Subject: [PATCH 24/26] mark commands as experimental --- internal/command/registry/command.go | 5 +++-- internal/command/registry/sbom.go | 2 +- internal/command/registry/vulns.go | 2 +- internal/command/registry/vulnsummary.go | 2 +- 4 files changed, 6 insertions(+), 5 deletions(-) diff --git a/internal/command/registry/command.go b/internal/command/registry/command.go index f2ce452bf2..99cb2d2141 100644 --- a/internal/command/registry/command.go +++ b/internal/command/registry/command.go @@ -9,8 +9,9 @@ import ( func New() *cobra.Command { const ( usage = "registry" - short = "Operate on registry images" - long = "Scan registry images for an SBOM or vulnerabilities." + short = "Operate on registry images [experimental]" + long = "Scan registry images for an SBOM or vulnerabilities. These commands\n" + + "are experimental and subject to change." ) cmd := command.New(usage, short, long, nil) cmd.Hidden = true diff --git a/internal/command/registry/sbom.go b/internal/command/registry/sbom.go index e750c3f0a6..d8db7bdc04 100644 --- a/internal/command/registry/sbom.go +++ b/internal/command/registry/sbom.go @@ -16,7 +16,7 @@ import ( func newSbom() *cobra.Command { const ( usage = "sbom" - short = "Generate an SBOM for a registry iamge" + short = "Generate an SBOM for a registry iamge [experimental]" long = "Genearte an SBOM for a registry image.\n" + "The image is selected by name, or the image of the app's first machine\n" + "is used unless interactive machine selection or machine ID is specified." diff --git a/internal/command/registry/vulns.go b/internal/command/registry/vulns.go index d0f8a4d36d..c045a325a0 100644 --- a/internal/command/registry/vulns.go +++ b/internal/command/registry/vulns.go @@ -17,7 +17,7 @@ import ( func newVulns() *cobra.Command { const ( usage = "vulns ... [flags]" - short = "Report possible vulnerabilities in a registry image" + short = "Report possible vulnerabilities in a registry image [experimental]" long = "Report possible vulnerabilities in a registry image in JSON or text.\n" + "The image is selected by name, or the image of the app's first machine\n" + "is used unless interactive machine selection or machine ID is specified\n" + diff --git a/internal/command/registry/vulnsummary.go b/internal/command/registry/vulnsummary.go index df363a1142..06b5a02467 100644 --- a/internal/command/registry/vulnsummary.go +++ b/internal/command/registry/vulnsummary.go @@ -26,7 +26,7 @@ const ( func newVulnSummary() *cobra.Command { const ( usage = "vulnsummary ... [flags]" - short = "Show a summary of possible vulnerabilities in registry images" + short = "Show a summary of possible vulnerabilities in registry images [experimental]" long = "Summarize possible vulnerabilities in registry images in an org, by app.\n" + "Limit scanning to a single app if specified. Limit scanning to images\n" + "used by running machines if specified. Limit reporting to\n" + From fc4af8c20f3c728ea75b99f7a0356892e91cbae6 Mon Sep 17 00:00:00 2001 From: Tim Newsham Date: Thu, 18 Jul 2024 10:53:25 -1000 Subject: [PATCH 25/26] show skipped scans in sorted order --- internal/command/registry/args.go | 2 +- internal/command/registry/vulnsummary.go | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/internal/command/registry/args.go b/internal/command/registry/args.go index 047e2b4cb1..c65dc0344d 100644 --- a/internal/command/registry/args.go +++ b/internal/command/registry/args.go @@ -61,7 +61,7 @@ func AugmentMap[K comparable, V any](targ, src map[K]V) { // SortedKeys returns the keys in a map in sorted order. // Could be made generic. -func SortedKeys(m map[ImgInfo]Unit) []ImgInfo { +func SortedKeys[V any](m map[ImgInfo]V) []ImgInfo { keys := lo.Keys(m) slices.SortFunc(keys, func(a, b ImgInfo) int { return a.Compare(b) }) return keys diff --git a/internal/command/registry/vulnsummary.go b/internal/command/registry/vulnsummary.go index 06b5a02467..c5bebaf30d 100644 --- a/internal/command/registry/vulnsummary.go +++ b/internal/command/registry/vulnsummary.go @@ -220,7 +220,8 @@ func fetchImageScans(ctx context.Context, imgs map[ImgInfo]Unit, filter *VulnFil } spin.Stop() - for img, msg := range skipped { + for _, img := range SortedKeys(skipped) { + msg := skipped[img] fmt.Fprintf(ios.Out, "Skipping %s (%s) %s\n", img.App, img.Mach, msg) } return imageScan, nil From 222590642cc36db0ca6607f592878c22b0a58d2a Mon Sep 17 00:00:00 2001 From: Tim Newsham Date: Fri, 19 Jul 2024 08:45:38 -1000 Subject: [PATCH 26/26] remove TODO comments --- internal/command/registry/vulns.go | 1 - internal/command/registry/vulnsummary.go | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/internal/command/registry/vulns.go b/internal/command/registry/vulns.go index c045a325a0..82cd318d5f 100644 --- a/internal/command/registry/vulns.go +++ b/internal/command/registry/vulns.go @@ -115,7 +115,6 @@ func runVulns(ctx context.Context) error { func presentScan(ctx context.Context, scan *Scan) error { ios := iostreams.FromContext(ctx) - // TODO: scan.Metadata? fmt.Fprintf(ios.Out, "Report created at: %s\n", scan.CreatedAt) for _, res := range scan.Results { fmt.Fprintf(ios.Out, "Target %s: %s\n", res.Type, res.Target) diff --git a/internal/command/registry/vulnsummary.go b/internal/command/registry/vulnsummary.go index c5bebaf30d..5664dc7484 100644 --- a/internal/command/registry/vulnsummary.go +++ b/internal/command/registry/vulnsummary.go @@ -20,7 +20,7 @@ import ( ) const ( - concurrentScans = 5 // TODO: think about the right value here. + concurrentScans = 5 ) func newVulnSummary() *cobra.Command {