Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for requesting Curation waivers to the CLI #280

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from
Open

Add support for requesting Curation waivers to the CLI #280

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3f6d423
Add support for requesting Curation waivers to the CLI
igorz-jf Jan 6, 2025
91be60b
Merge branch 'jfrog:dev' into dev
igorz-jf Jan 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
183 changes: 168 additions & 15 deletions commands/curation/curationaudit.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,17 @@ import (
"encoding/json"
"errors"
"fmt"
"github.com/jfrog/jfrog-cli-security/utils/formats"
"net/http"
"os"
"path/filepath"
"regexp"
"sort"
"strconv"
"strings"
"sync"

"golang.org/x/exp/maps"
"golang.org/x/exp/slices"

"github.com/jfrog/gofrog/datastructures"
"github.com/jfrog/gofrog/parallel"
Expand All @@ -24,13 +25,8 @@ import (

"github.com/jfrog/jfrog-cli-core/v2/utils/config"
"github.com/jfrog/jfrog-cli-core/v2/utils/coreutils"
"github.com/jfrog/jfrog-cli-core/v2/utils/ioutils"

"github.com/jfrog/jfrog-cli-security/commands/audit"
"github.com/jfrog/jfrog-cli-security/commands/audit/sca/python"
"github.com/jfrog/jfrog-cli-security/utils"
"github.com/jfrog/jfrog-cli-security/utils/results/output"
"github.com/jfrog/jfrog-cli-security/utils/techutils"
"github.com/jfrog/jfrog-cli-security/utils/xray"
"github.com/jfrog/jfrog-client-go/artifactory"
"github.com/jfrog/jfrog-client-go/auth"
clientutils "github.com/jfrog/jfrog-client-go/utils"
Expand All @@ -40,6 +36,14 @@ import (
xrayClient "github.com/jfrog/jfrog-client-go/xray"
xrayUtils "github.com/jfrog/jfrog-client-go/xray/services/utils"

"github.com/jfrog/jfrog-cli-security/commands/audit"
"github.com/jfrog/jfrog-cli-security/commands/audit/sca/python"
"github.com/jfrog/jfrog-cli-security/utils"
"github.com/jfrog/jfrog-cli-security/utils/formats"
"github.com/jfrog/jfrog-cli-security/utils/results/output"
"github.com/jfrog/jfrog-cli-security/utils/techutils"
"github.com/jfrog/jfrog-cli-security/utils/xray"

"github.com/jfrog/build-info-go/build/utils/dotnet/dependencies"
)

Expand All @@ -62,33 +66,39 @@ const (
errorTemplateUnsupportedTech = "It looks like this project uses '%s' to download its dependencies. " +
"This package manager however isn't supported by this command."

WaiverRequestForbidden = "One or more policies blocking this package do not allow waiver requests."
WaiverRequestApproved = "The waiver request was automatically granted; you can use this package.\nNOTE: The policy owner may review this waiver more thoroughly and contact you if issues are found."
WaiverRequestPending = "A waiver request was opened for review, and the owner was notified.\nYou will receive an email with an update once the status changes."

TotalConcurrentRequests = 10

MinArtiPassThroughSupport = "7.82.0"
MinArtiGolangSupport = "7.87.0"
MinArtiNuGetSupport = "7.93.0"
MinXrayPassThroughSupport = "3.92.0"
MinArtiWaiverRequest = "7.103.0"
MinXrayWaiverRequest = "3.113.0"
)

var CurationOutputFormats = []string{string(outFormat.Table), string(outFormat.Json)}

var supportedTech = map[techutils.Technology]func(ca *CurationAuditCommand) (bool, error){
techutils.Npm: func(ca *CurationAuditCommand) (bool, error) { return true, nil },
techutils.Pip: func(ca *CurationAuditCommand) (bool, error) {
return ca.checkSupportByVersionOrEnv(techutils.Pip, MinArtiPassThroughSupport)
return ca.checkSupportByVersionOrEnv(techutils.Pip, MinXrayPassThroughSupport, MinArtiPassThroughSupport)
},
techutils.Maven: func(ca *CurationAuditCommand) (bool, error) {
return ca.checkSupportByVersionOrEnv(techutils.Maven, MinArtiPassThroughSupport)
return ca.checkSupportByVersionOrEnv(techutils.Maven, MinXrayPassThroughSupport, MinArtiPassThroughSupport)
},
techutils.Go: func(ca *CurationAuditCommand) (bool, error) {
return ca.checkSupportByVersionOrEnv(techutils.Go, MinArtiGolangSupport)
return ca.checkSupportByVersionOrEnv(techutils.Go, MinXrayPassThroughSupport, MinArtiGolangSupport)
},
techutils.Nuget: func(ca *CurationAuditCommand) (bool, error) {
return ca.checkSupportByVersionOrEnv(techutils.Nuget, MinArtiNuGetSupport)
return ca.checkSupportByVersionOrEnv(techutils.Nuget, MinXrayPassThroughSupport, MinArtiNuGetSupport)
},
}

func (ca *CurationAuditCommand) checkSupportByVersionOrEnv(tech techutils.Technology, minArtiVersion string) (bool, error) {
func (ca *CurationAuditCommand) checkSupportByVersionOrEnv(tech techutils.Technology, minXrayVersion, minArtiVersion string) (bool, error) {
if flag, err := clientutils.GetBoolEnvValue(utils.CurationSupportFlag, false); flag {
return true, nil
} else if err != nil {
Expand All @@ -104,7 +114,7 @@ func (ca *CurationAuditCommand) checkSupportByVersionOrEnv(tech techutils.Techno
return false, err
}

xrayVersionErr := clientutils.ValidateMinimumVersion(clientutils.Xray, xrayVersion, MinXrayPassThroughSupport)
xrayVersionErr := clientutils.ValidateMinimumVersion(clientutils.Xray, xrayVersion, minXrayVersion)
rtVersionErr := clientutils.ValidateMinimumVersion(clientutils.Artifactory, artiVersion, minArtiVersion)
if xrayVersionErr != nil || rtVersionErr != nil {
return false, errors.Join(xrayVersionErr, rtVersionErr)
Expand Down Expand Up @@ -170,11 +180,11 @@ type Policy struct {
}

type PackageStatusTable struct {
ID string `col-name:"ID" auto-merge:"true"`
ParentName string `col-name:"Direct\nDependency\nPackage\nName" auto-merge:"true"`
ParentVersion string `col-name:"Direct\nDependency\nPackage\nVersion" auto-merge:"true"`
PackageName string `col-name:"Blocked\nPackage\nName" auto-merge:"true"`
PackageVersion string `col-name:"Blocked\nPackage\nVersion" auto-merge:"true"`
BlockingReason string `col-name:"Blocking Reason" auto-merge:"true"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BlockingReason is deleted from PackageStatusTable but not from PackageStatus. is it still used? why not showing this anymore?

PkgType string `col-name:"Package\nType" auto-merge:"true"`
Policy string `col-name:"Violated\nPolicy\nName"`
Condition string `col-name:"Violated Condition\nName"`
Expand Down Expand Up @@ -208,6 +218,13 @@ type CurationReport struct {
totalNumberOfPackages int
}

type WaiverResponse struct {
PkgName string `col-name:"Package Name"`
Status string `col-name:"Status"`
Explanation string `col-name:"Explanation"`
WaiverID string `col-name:"Waiver ID"`
}

func NewCurationAuditCommand() *CurationAuditCommand {
return &CurationAuditCommand{
extractPoliciesRegex: regexp.MustCompile(extractPoliciesRegexTemplate),
Expand Down Expand Up @@ -267,6 +284,12 @@ func (ca *CurationAuditCommand) Run() (err error) {

for projectPath, packagesStatus := range results {
err = errors.Join(err, printResult(ca.OutputFormat(), projectPath, packagesStatus.packagesStatus))

tech := packagesStatus.packagesStatus[0].PkgType
waiversSupported, _ := ca.checkSupportByVersionOrEnv(techutils.Technology(tech), MinXrayWaiverRequest, MinArtiWaiverRequest)
if len(packagesStatus.packagesStatus) > 0 && waiversSupported {
err = errors.Join(ca.requestWaiver(packagesStatus.packagesStatus))
}
}
err = errors.Join(err, output.RecordSecurityCommandSummary(output.NewCurationSummary(convertResultsToSummary(results))))
return
Expand Down Expand Up @@ -461,6 +484,136 @@ func (ca *CurationAuditCommand) auditTree(tech techutils.Technology, results map
return err
}

func getSelectedPackages(requestedRows string, blockedPackages []*PackageStatus) (selectedPackages []*PackageStatus, ok bool) {
validFormat := regexp.MustCompile(`^(all|(\d+(-\d+)?)(,\d+(-\d+)?)*$)`)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comment what is the valid format that the regex describes to help readability

if !validFormat.MatchString(requestedRows) {
fmt.Print("Invalid request format.\n\n")
Copy link
Contributor

@attiasas attiasas Jan 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
fmt.Print("Invalid request format.\n\n")
log.Output("Invalid request format.\n\n")

use log.Output, also maybe hint the user what is the valid format if it can be controlled (or add debug log if it coming from our platform for debugging when issue occur)

return nil, false
}

if requestedRows == "all" {
return blockedPackages, true
}

var indices []int
parts := strings.Split(requestedRows, ",")
for _, part := range parts {
if strings.Contains(part, "-") {
rangeParts := strings.Split(part, "-")
start, _ := strconv.Atoi(rangeParts[0])
end, _ := strconv.Atoi(rangeParts[1])
for i := start; i <= end; i++ {
if slices.Contains(indices, i) {
continue
}
indices = append(indices, i)
}
} else {
index, err := strconv.Atoi(part)
if err != nil || slices.Contains(indices, index) {
continue
}
indices = append(indices, index)
}
}
Comment on lines +499 to +518
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
parts := strings.Split(requestedRows, ",")
for _, part := range parts {
if strings.Contains(part, "-") {
rangeParts := strings.Split(part, "-")
start, _ := strconv.Atoi(rangeParts[0])
end, _ := strconv.Atoi(rangeParts[1])
for i := start; i <= end; i++ {
if slices.Contains(indices, i) {
continue
}
indices = append(indices, i)
}
} else {
index, err := strconv.Atoi(part)
if err != nil || slices.Contains(indices, index) {
continue
}
indices = append(indices, index)
}
}
rows := strings.Split(requestedRows, ",")
for _, rowStr := range parts {
if strings.Contains(rowStr, "-") {
// Range of rows requested
rangeParts := strings.Split(rowStr, "-")
startRow, _ := strconv.Atoi(rangeParts[0])
endRow, _ := strconv.Atoi(rangeParts[1])
for i := start; i <= end; i++ {
if slices.Contains(indices, i) {
continue
}
indices = append(indices, i)
}
} else {
// Single row requested
index, err := strconv.Atoi(rowStr)
if err != nil || slices.Contains(indices, index) {
continue
}
indices = append(indices, index)
}
}

rename for readability, maybe also refactor this to a separated method
add some comments


for _, index := range indices {
if index > 0 && index <= len(blockedPackages) {
selectedPackages = append(selectedPackages, blockedPackages[index-1])
} else {
fmt.Printf("Row number '%d' does not exist in the table. Please enter a valid row number.\n\n", index)
return nil, false
}
}
return selectedPackages, true
}

func (ca *CurationAuditCommand) sendWaiverRequests(pkgs []*PackageStatus, msg string, serverDetails *config.ServerDetails) (requestStatuses []WaiverResponse, err error) {
fmt.Print("Submitting waiver request...\n\n")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
fmt.Print("Submitting waiver request...\n\n")
log.Info("Submitting waiver request...\n\n")

use the log from "github.com/jfrog/jfrog-client-go/utils/log" (you can also use log.Output)

rtAuth, err := serverDetails.CreateArtAuthConfig()
if err != nil {
return nil, err
}
rtManager, err := rtUtils.CreateServiceManager(serverDetails, 2, 0, false)
if err != nil {
return nil, err
}
clientDetails := rtAuth.CreateHttpClientDetails()
clientDetails.Headers["X-Artifactory-Curation-Request-Waiver"] = msg
for _, pkg := range pkgs {
_, body, _, err := rtManager.Client().SendGet(pkg.BlockedPackageUrl, true, &clientDetails)
if err != nil {
return nil, err
}
Comment on lines +544 to +547
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
_, body, _, err := rtManager.Client().SendGet(pkg.BlockedPackageUrl, true, &clientDetails)
if err != nil {
return nil, err
}
platformResponse, body, _, err := rtManager.Client().SendGet(pkg.BlockedPackageUrl, true, &clientDetails)
if err != nil {
return nil, errors.New("failed while attempting to get waiver:\n" + err.Error())
}
if err = errorutils.CheckResponseStatusWithBody(platformResponse, body, http.StatusOK); err != nil {
return nil, errors.New("got unexpected server response while attempting to get waiver:\n" + err.Error())
}

I would also validate the response status.
Please also wrap error with text to help debugging

var resp struct {
Errors []struct {
Status int `json:"status"`
Message string `json:"message"`
} `json:"errors"`
}
if err := json.Unmarshal(body, &resp); err != nil {
return nil, errors.New("failed decoding waiver request status")
}
Comment on lines +554 to +556
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if err := json.Unmarshal(body, &resp); err != nil {
return nil, errors.New("failed decoding waiver request status")
}
if err := json.Unmarshal(body, &resp); err != nil {
return nil, fmt.Errorf("failed decoding waiver request status: " + err.Error())
}

don't ignore the actual error


var id, status, message string
parts := strings.Split(resp.Errors[0].Message, "|")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why resp.Errors[0] ? what about the other errors? why only one (and is it always exists? could it be empty?)

if len(parts) != 2 {
return nil, errors.New("failed decoding waiver request status")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return nil, errors.New("failed decoding waiver request status")
return nil, fmt.Errorf("failed decoding waiver request status, got unexpected Error msg format while trying to split into parts (%d expected 2)", len(parts))

describe the error so it will be unique

}
id, status = parts[0], parts[1]
switch status {
case "pending":
message = WaiverRequestPending
case "approved":
message = WaiverRequestApproved
case "forbidden":
message = WaiverRequestForbidden
}
requestStatuses = append(requestStatuses, WaiverResponse{
PkgName: pkg.PackageName,
Status: status,
WaiverID: id,
Explanation: message,
})
Comment on lines +544 to +577
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would consider refactoring this into its own method for readability

}
return requestStatuses, nil
}

func getWaiverRequestParams(blockedPackages []*PackageStatus) (selectedPackages []*PackageStatus, requestMsg string) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can only input one reason for rows right? if I want to enter 2 reasons at the same scan? (for 2 groups)

for {
requestedRows := ioutils.AskStringWithDefault("", "Please enter the row number(s) for which you want to request a waiver (comma-separated for multiple, range, or “all”)", "all")
if pkgs, ok := getSelectedPackages(requestedRows, blockedPackages); ok {
selectedPackages = pkgs
break
}
}
for {
requestMsg = ioutils.AskString("", "Please enter the reason for the waiver request:", false, false)
if len(requestMsg) >= 5 && len(requestMsg) <= 300 {
break
}
fmt.Print("The reason must be between 5 and 300 characters.\n\n")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
fmt.Print("The reason must be between 5 and 300 characters.\n\n")
log.Output("The reason must be between 5 and 300 characters.\n\n")

}
return selectedPackages, requestMsg
}

func (ca *CurationAuditCommand) requestWaiver(blockedPackages []*PackageStatus) error {
if !coreutils.AskYesNo("Do you want to request a waiver for any of the listed packages?", false) {
return nil
}
selectedPackages, requestMsg := getWaiverRequestParams(blockedPackages)
if len(selectedPackages) == 0 {
return nil
}
serverDetails, _ := ca.PackageManagerConfig.ServerDetails()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
serverDetails, _ := ca.PackageManagerConfig.ServerDetails()
serverDetails, err := ca.PackageManagerConfig.ServerDetails()

you are ignoring error here, handle this please

pkgStatusTable, err := ca.sendWaiverRequests(selectedPackages, requestMsg, serverDetails)
if err != nil {
return err
}

return coreutils.PrintTable(pkgStatusTable, "Waiver request submitted!", "Requested 0 waivers", true)
}

func printResult(format outFormat.OutputFormat, projectPath string, packagesStatus []*PackageStatus) error {
if format == "" {
format = outFormat.Table
Expand Down Expand Up @@ -495,11 +648,11 @@ func convertToPackageStatusTable(packagesStatus []*PackageStatus) []PackageStatu
uniqLineSep = " "
}
pkgTable := PackageStatusTable{
ID: fmt.Sprintf("%d%s", index+1, uniqLineSep),
ParentName: pkgStatus.ParentName + uniqLineSep,
ParentVersion: pkgStatus.ParentVersion + uniqLineSep,
PackageName: pkgStatus.PackageName + uniqLineSep,
PackageVersion: pkgStatus.PackageVersion + uniqLineSep,
BlockingReason: pkgStatus.BlockingReason + uniqLineSep,
PkgType: pkgStatus.PkgType + uniqLineSep,
}
if len(pkgStatus.Policy) == 0 {
Expand Down
Loading
Loading