diff --git a/cmd/automate.go b/cmd/automate.go index 9d5b687..44f34b5 100644 --- a/cmd/automate.go +++ b/cmd/automate.go @@ -1,6 +1,7 @@ package cmd import ( + "fmt" "io" "os" "path/filepath" @@ -22,10 +23,10 @@ var automateCmd = &cobra.Command{ Long: `The automate command sends a request to each discovered endpoint and returns the status code of the result. This enables the user to get a quick look at which endpoints require authentication and which ones do not. If a request responds in an abnormal way, manual testing should be conducted (prepare manual tests using the "prepare" command).`, - Run: func(cmd *cobra.Command, args []string) { + RunE: func(cmd *cobra.Command, args []string) error { if outfile != "" && strings.ToLower(outputFormat) != "" { if !strings.HasSuffix(strings.ToLower(outfile), "json") && strings.ToLower(outputFormat) != "json" { - die("Only the JSON output format is supported at the moment.") + return fmt.Errorf("only the JSON output format is supported at the moment") } else if strings.HasSuffix(strings.ToLower(outfile), "json") && strings.ToLower(outputFormat) == "console" { outputFormat = "json" } @@ -45,12 +46,15 @@ responds in an abnormal way, manual testing should be conducted (prepare manual _, err := time.Parse("2006-01-02", customDate) if err != nil { - die("An invalid date was supplied. Please supply a date in '2006-01-02' format.") + return fmt.Errorf("an invalid date was supplied. Please supply a date in '2006-01-02' format") } var bodyBytes []byte - client, replayClient := CheckAndConfigureProxy() + client, replayClient, err := CheckAndConfigureProxy() + if err != nil { + return err + } if strings.ToLower(outputFormat) != "json" { printInfo("\n") @@ -58,11 +62,14 @@ responds in an abnormal way, manual testing should be conducted (prepare manual } if swaggerURL != "" { - bodyBytes, _, _ = MakeRequest(client, "GET", swaggerURL, timeout, nil) + bodyBytes, _, _, err = MakeRequest(client, "GET", swaggerURL, timeout, nil) + if err != nil { + return err + } } else { specFile, err := os.Open(localFile) if err != nil { - die("Error opening file: %v", err) + return fmt.Errorf("error opening file: %v", err) } // Set the base directory for resolving external refs specBaseDir = filepath.Dir(localFile) @@ -79,7 +86,7 @@ responds in an abnormal way, manual testing should be conducted (prepare manual log.Info("Sending requests at a rate of ", rateLimit, " requests per second.") } */ - GenerateRequests(bodyBytes, client, replayClient) + return GenerateRequests(bodyBytes, client, replayClient) }, } diff --git a/cmd/brute.go b/cmd/brute.go index a14e70c..ddf2a5e 100644 --- a/cmd/brute.go +++ b/cmd/brute.go @@ -44,7 +44,7 @@ var bruteCmd = &cobra.Command{ Use: "brute", Short: "Sends a series of automated requests to discover hidden API operation definitions.", Long: `The brute command sends requests to the target to find operation definitions based on commonly used file locations.`, - Run: func(cmd *cobra.Command, args []string) { + RunE: func(cmd *cobra.Command, args []string) error { /* // NEED TO RE-IMPLEMENT RATE LIMIT if rateLimit <= 0 { @@ -58,7 +58,10 @@ var bruteCmd = &cobra.Command{ } } - client, _ := CheckAndConfigureProxy() + client, _, err := CheckAndConfigureProxy() + if err != nil { + return err + } var allURLs []string u, err := url.Parse(swaggerURL) @@ -80,7 +83,7 @@ var bruteCmd = &cobra.Command{ } else { endpointList, err := os.Open(endpointWordlist) if err != nil { - die("failed to open file: %s", err) + return fmt.Errorf("failed to open file: %s", err) } defer endpointList.Close() @@ -92,7 +95,7 @@ var bruteCmd = &cobra.Command{ } if err := scanner.Err(); err != nil { - die("failed to read words from file: %s", err) + return fmt.Errorf("failed to read words from file: %s", err) } } if rateLimit > 0 && strings.ToLower(outputFormat) != "json" { @@ -101,7 +104,10 @@ var bruteCmd = &cobra.Command{ printInfo("Sending %d requests. This could take a while...\n", len(allURLs)) } - specFound, definitionFile := findDefinitionFile(allURLs, client) + specFound, definitionFile, err := findDefinitionFile(allURLs, client) + if err != nil { + return err + } if specFound { definedOperations, err := json.Marshal(definitionFile) if err != nil { @@ -126,7 +132,7 @@ var bruteCmd = &cobra.Command{ } } else { if endpointOnly { - return + return nil } else { fmt.Println(string(definedOperations)) } @@ -135,6 +141,7 @@ var bruteCmd = &cobra.Command{ } else { printErr("\nNo definition file found for:\t%s", swaggerURL) } + return nil }, } @@ -166,45 +173,52 @@ func makeURLs(target string, basePath string, endpoints []string, fileExtension // maxFollowDepth bounds how far brute chases discovered spec references. const maxFollowDepth = 3 -func findDefinitionFile(urls []string, client http.Client) (bool, *openapi3.T) { +func findDefinitionFile(urls []string, client http.Client) (bool, *openapi3.T, error) { seen := make(map[string]bool) for i, url := range urls { - if found, checkSpec := checkURLForSpec(url, client, seen, 0); found { - return true, checkSpec + found, checkSpec, err := checkURLForSpec(url, client, seen, 0) + if err != nil { + return false, nil, err + } + if found { + return true, checkSpec, nil } writeProgress("Request: %d", i+1) } - return false, nil + return false, nil, nil } // checkURLForSpec fetches one URL and validates it as an OpenAPI/Swagger // definition. For a Swagger UI page, initializer script, or swagger-config JSON, // it follows the referenced URLs (bounded by maxFollowDepth and a shared visited // set) so specs on non-standard paths are still found. -func checkURLForSpec(target string, client http.Client, seen map[string]bool, depth int) (bool, *openapi3.T) { +func checkURLForSpec(target string, client http.Client, seen map[string]bool, depth int) (bool, *openapi3.T, error) { if depth > maxFollowDepth || seen[target] || isExternalDemoSpecURL(target) { - return false, nil + return false, nil, nil } seen[target] = true // One request so body and Content-Type come from the same response (two // separate requests can desync behind a WAF/CDN). - bodyBytes, bodyString, _, ctRaw, _ := MakeRequestFull(client, "GET", target, timeout, nil) + bodyBytes, bodyString, _, ctRaw, _, reqErr := MakeRequestFull(client, "GET", target, timeout, nil) + if reqErr != nil { + return false, nil, reqErr + } if bodyBytes == nil { - return false, nil + return false, nil, nil } ct := strings.ToLower(ctRaw) // Skip content types that cannot be a spec or reference one. for _, skip := range []string{"image/", "video/", "audio/", "font/", "text/css", "application/zip", "application/pdf"} { if strings.Contains(ct, skip) { - return false, nil + return false, nil, nil } } if looksLikeChallengeResponse(bodyString) { warnWAFChallengeOnce() - return false, nil + return false, nil, nil } // Swagger UI HTML page: follow the references it declares. @@ -216,14 +230,14 @@ func checkURLForSpec(target string, client http.Client, seen map[string]bool, de // Library bundles are excluded to avoid their internal url: strings. if strings.Contains(ct, "javascript") || strings.HasSuffix(strings.ToLower(urlPath(target)), ".js") { if !looksLikeSwaggerInitCandidate(target) { - return false, nil + return false, nil, nil } if bodyHasEmbeddedSpec(bodyString) { if jsonContent, ok := ExtractJSONFromJSSpec(bodyBytes); ok { - checkSpec := UnmarshalSpec(jsonContent) + checkSpec, _ := UnmarshalSpec(jsonContent) if strings.HasPrefix(checkSpec.OpenAPI, "2") || strings.HasPrefix(checkSpec.OpenAPI, "3") { printInfo("Found operation definitions embedded in JavaScript file at %s\n", target) - return true, checkSpec + return true, checkSpec, nil } } } @@ -240,10 +254,10 @@ func checkURLForSpec(target string, client http.Client, seen map[string]bool, de } // Otherwise treat the body as a candidate spec and validate by parsing. - checkSpec := UnmarshalSpec(bodyBytes) + checkSpec, _ := UnmarshalSpec(bodyBytes) if (strings.HasPrefix(checkSpec.OpenAPI, "2") || strings.HasPrefix(checkSpec.OpenAPI, "3")) && checkSpec.Paths != nil { printInfo("Definition file found: %s\n", target) - return true, checkSpec + return true, checkSpec, nil } // Not a spec: may be a swagger-config document that references one. @@ -251,13 +265,17 @@ func checkURLForSpec(target string, client http.Client, seen map[string]bool, de } // followReferences recurses into a set of discovered spec/reference URLs. -func followReferences(refs []string, client http.Client, seen map[string]bool, depth int) (bool, *openapi3.T) { +func followReferences(refs []string, client http.Client, seen map[string]bool, depth int) (bool, *openapi3.T, error) { for _, ref := range refs { - if found, checkSpec := checkURLForSpec(ref, client, seen, depth+1); found { - return true, checkSpec + found, checkSpec, err := checkURLForSpec(ref, client, seen, depth+1) + if err != nil { + return false, nil, err + } + if found { + return true, checkSpec, nil } } - return false, nil + return false, nil, nil } func init() { @@ -357,7 +375,7 @@ func ExtractSpecFromJS(bodyBytes []byte) []byte { return bodyBytes } -func UnmarshalSpec(bodyBytes []byte) (newDoc *openapi3.T) { +func UnmarshalSpec(bodyBytes []byte) (*openapi3.T, error) { var doc openapi2.T var doc3 openapi3.T @@ -373,19 +391,19 @@ func UnmarshalSpec(bodyBytes []byte) (newDoc *openapi3.T) { _ = json.Unmarshal(bodyBytes, &doc3) if strings.HasPrefix(doc3.OpenAPI, "3") { - newDoc := &doc3 - return newDoc + return &doc3, nil } else if strings.HasPrefix(doc.Swagger, "2") { newDoc, err := openapi2conv.ToV3(&doc) if err != nil { printErr("Error converting v2 document to v3: %s", err) } - return newDoc + return newDoc, nil } else if Mode == "brute" { var noDoc openapi3.T - return &noDoc + return &noDoc, nil } else { - die("Error parsing definition file.") - return nil + // Return a non-nil doc alongside the error so callers that ignore the + // error don't dereference nil. + return &openapi3.T{}, fmt.Errorf("error parsing definition file") } } diff --git a/cmd/convert.go b/cmd/convert.go index c764baa..6797744 100644 --- a/cmd/convert.go +++ b/cmd/convert.go @@ -19,7 +19,7 @@ var convertCmd = &cobra.Command{ Use: "convert", Short: "Converts a Swagger definition file to an OpenAPI v3 definition file.", Long: `The convert command converts a provided definition file from the Swagger specification (v2) to the OpenAPI specification (v3) and stores it into an output file.`, - Run: func(cmd *cobra.Command, args []string) { + RunE: func(cmd *cobra.Command, args []string) error { var bodyBytes []byte @@ -29,7 +29,10 @@ var convertCmd = &cobra.Command{ } } - client, _ := CheckAndConfigureProxy() + client, _, err := CheckAndConfigureProxy() + if err != nil { + return err + } if strings.ToLower(outputFormat) != "json" { printInfo("\n") @@ -37,11 +40,14 @@ var convertCmd = &cobra.Command{ } if swaggerURL != "" { - bodyBytes, _, _ = MakeRequest(client, "GET", swaggerURL, timeout, nil) + bodyBytes, _, _, err = MakeRequest(client, "GET", swaggerURL, timeout, nil) + if err != nil { + return err + } } else { specFile, err := os.Open(localFile) if err != nil { - die("Error opening definition file: %v", err) + return fmt.Errorf("error opening definition file: %v", err) } bodyBytes, _ = io.ReadAll(specFile) @@ -75,7 +81,7 @@ var convertCmd = &cobra.Command{ } converted, err := json.Marshal(newDoc) if err != nil { - die("Error converting definition file to v3: %v", err) + return fmt.Errorf("error converting definition file to v3: %v", err) } if !strings.HasSuffix(string(converted), "}") { if outfile == "" { @@ -94,7 +100,7 @@ var convertCmd = &cobra.Command{ case "yaml", "yml": converted, err := yaml.Marshal(newDoc) if err != nil { - die("Error converting definition file to v3: %v", err) + return fmt.Errorf("error converting definition file to v3: %v", err) } if outfile == "" { fmt.Println(string(converted)) @@ -104,8 +110,9 @@ var convertCmd = &cobra.Command{ } } else { - die("Error parsing definition file.") + return fmt.Errorf("error parsing definition file") } + return nil }, } diff --git a/cmd/endpoints.go b/cmd/endpoints.go index d69a21f..7187428 100644 --- a/cmd/endpoints.go +++ b/cmd/endpoints.go @@ -1,6 +1,7 @@ package cmd import ( + "fmt" "io" "os" "path/filepath" @@ -13,14 +14,17 @@ var endpointsCmd = &cobra.Command{ Short: "Prints a list of endpoints from the target.", Long: `The endpoints command allows you to pull a list of endpoints out of a Swagger definition file. This list contains the raw endpoints (parameter values will not be appended or modified).`, - Run: func(cmd *cobra.Command, args []string) { + RunE: func(cmd *cobra.Command, args []string) error { if randomUserAgent { if UserAgent != "Swagger Jacker (github.com/BishopFox/sj)" { printWarn("A supplied User Agent was detected (%s) while supplying the 'random-user-agent' flag.", UserAgent) } } - client, _ := CheckAndConfigureProxy() + client, _, err := CheckAndConfigureProxy() + if err != nil { + return err + } var bodyBytes []byte @@ -28,22 +32,25 @@ This list contains the raw endpoints (parameter values will not be appended or m printInfo("Gathering endpoints.\n\n") if swaggerURL != "" { - bodyBytes, _, _ = MakeRequest(client, "GET", swaggerURL, timeout, nil) - GenerateRequests(bodyBytes, client, nil) - } else { - specFile, err := os.Open(localFile) + bodyBytes, _, _, err = MakeRequest(client, "GET", swaggerURL, timeout, nil) if err != nil { - die("Error opening file: %v", err) + return err } - // Set the base directory for resolving external refs - specBaseDir = filepath.Dir(localFile) - if specBaseDir == "." { - if absPath, err := filepath.Abs(localFile); err == nil { - specBaseDir = filepath.Dir(absPath) - } - } - bodyBytes, _ = io.ReadAll(specFile) - GenerateRequests(bodyBytes, client, nil) + return GenerateRequests(bodyBytes, client, nil) } + + specFile, err := os.Open(localFile) + if err != nil { + return fmt.Errorf("error opening file: %v", err) + } + // Set the base directory for resolving external refs + specBaseDir = filepath.Dir(localFile) + if specBaseDir == "." { + if absPath, err := filepath.Abs(localFile); err == nil { + specBaseDir = filepath.Dir(absPath) + } + } + bodyBytes, _ = io.ReadAll(specFile) + return GenerateRequests(bodyBytes, client, nil) }, } diff --git a/cmd/log.go b/cmd/log.go index 8b76778..f130898 100644 --- a/cmd/log.go +++ b/cmd/log.go @@ -80,11 +80,6 @@ func printErr(format string, args ...interface{}) { fmt.Fprintf(os.Stderr, "%s %s\n", red("[✗]"), msg) } -func die(format string, args ...interface{}) { - printErr(format, args...) - os.Exit(1) -} - // writeLog is the main dispatch function for endpoint results. func writeLog(sc int, target, method, errorMsg, response, security string, missingAuth bool) { var out io.Writer = os.Stdout @@ -97,14 +92,16 @@ func writeLog(sc int, target, method, errorMsg, response, security string, missi if outfile != "" { file, err := os.OpenFile(outfile, os.O_RDWR|os.O_APPEND|os.O_CREATE, 0644) if err != nil { - fmt.Fprintln(os.Stderr, "Output file does not exist or cannot be created") - os.Exit(1) + // Package code must not exit the process; fall back to stdout so the + // results are still emitted. + printErr("Output file '%s' cannot be opened; writing to stdout instead.", outfile) + } else { + defer file.Close() + out = file + // Disable color when writing to a file. + color.NoColor = true + defer func() { color.NoColor = false }() } - defer file.Close() - out = file - // Disable color when writing to a file. - color.NoColor = true - defer func() { color.NoColor = false }() } preview := "" diff --git a/cmd/prepare.go b/cmd/prepare.go index 7ba6e44..444d894 100644 --- a/cmd/prepare.go +++ b/cmd/prepare.go @@ -1,6 +1,7 @@ package cmd import ( + "fmt" "io" "os" "path/filepath" @@ -14,7 +15,7 @@ var prepareCmd = &cobra.Command{ Short: "Prepares a set of commands for manual testing of each endpoint.", Long: `The prepare command prepares a set of commands for manual testing of each endpoint. This enables you to test specific API functions for common vulnerabilities or misconfigurations.`, - Run: func(cmd *cobra.Command, args []string) { + RunE: func(cmd *cobra.Command, args []string) error { if randomUserAgent { if UserAgent != "Swagger Jacker (github.com/BishopFox/sj)" { @@ -24,32 +25,38 @@ This enables you to test specific API functions for common vulnerabilities or mi _, err := time.Parse("2006-01-02", customDate) if err != nil { - die("An invalid date was supplied. Please supply a date in '2006-01-02' format.") + return fmt.Errorf("an invalid date was supplied. Please supply a date in '2006-01-02' format") } - client, _ := CheckAndConfigureProxy() + client, _, err := CheckAndConfigureProxy() + if err != nil { + return err + } printInfo("\n") printInfo("Gathering API details.\n\n") if swaggerURL != "" { - bodyBytes, _, _ := MakeRequest(client, "GET", swaggerURL, timeout, nil) - GenerateRequests(bodyBytes, client, nil) - } else { - specFile, err := os.Open(localFile) + bodyBytes, _, _, err := MakeRequest(client, "GET", swaggerURL, timeout, nil) if err != nil { - die("Error opening file: %v", err) + return err } - // Set the base directory for resolving external refs - specBaseDir = filepath.Dir(localFile) - if specBaseDir == "." { - if absPath, err := filepath.Abs(localFile); err == nil { - specBaseDir = filepath.Dir(absPath) - } - } - - specBytes, _ := io.ReadAll(specFile) - GenerateRequests(specBytes, client, nil) + return GenerateRequests(bodyBytes, client, nil) } + + specFile, err := os.Open(localFile) + if err != nil { + return fmt.Errorf("error opening file: %v", err) + } + // Set the base directory for resolving external refs + specBaseDir = filepath.Dir(localFile) + if specBaseDir == "." { + if absPath, err := filepath.Abs(localFile); err == nil { + specBaseDir = filepath.Dir(absPath) + } + } + + specBytes, _ := io.ReadAll(specFile) + return GenerateRequests(specBytes, client, nil) }, } var prepareFor string diff --git a/cmd/requests.go b/cmd/requests.go index 7ff3295..4a10d1f 100644 --- a/cmd/requests.go +++ b/cmd/requests.go @@ -157,9 +157,9 @@ func dangerousRequestDeclined(target string) bool { // MakeRequest returns the body bytes, body string, and status code. It wraps // MakeRequestFull for callers that do not need the Content-Type. -func MakeRequest(client http.Client, method, target string, timeout int64, reqData io.Reader) ([]byte, string, int) { - bodyBytes, bodyString, status, _, _ := MakeRequestFull(client, method, target, timeout, reqData) - return bodyBytes, bodyString, status +func MakeRequest(client http.Client, method, target string, timeout int64, reqData io.Reader) ([]byte, string, int, error) { + bodyBytes, bodyString, status, _, _, err := MakeRequestFull(client, method, target, timeout, reqData) + return bodyBytes, bodyString, status, err } // splitWords lowercases s and splits it into words on non-alphanumeric @@ -217,7 +217,7 @@ func requestWords(u *url.URL) map[string]bool { // MakeRequestFull also returns the response Content-Type and the User-Agent sent. // Taking body and Content-Type from one response avoids a second request per URL. -func MakeRequestFull(client http.Client, method, target string, timeout int64, reqData io.Reader) ([]byte, string, int, string, string) { +func MakeRequestFull(client http.Client, method, target string, timeout int64, reqData io.Reader) ([]byte, string, int, string, string, error) { if quiet { avoidDangerousRequests = "y" } @@ -226,10 +226,10 @@ func MakeRequestFull(client http.Client, method, target string, timeout int64, r u, err := url.Parse(target) if err != nil || u == nil { printWarn("Error parsing URL '%s': %v - skipping request.", target, err) - return nil, "", 0, "", "" + return nil, "", 0, "", "", nil } if Mode == "automate" && target != approvedTarget && dangerousRequestDeclined(target) { - return nil, "", 0, "", "" + return nil, "", 0, "", "", nil } ctx, cancel := context.WithTimeout(context.Background(), time.Duration(timeout)*time.Second) @@ -247,9 +247,9 @@ func MakeRequestFull(client http.Client, method, target string, timeout int64, r req, err := http.NewRequest(method, target, body) if err != nil { if err != context.Canceled && err != io.EOF { - die("Error: could not create HTTP request - %v", err) + return nil, "", 0, "", "", fmt.Errorf("could not create HTTP request - %v", err) } - return nil, "", 0, "", "" + return nil, "", 0, "", "", nil } sentUserAgent := applyHeaders(req, "") @@ -257,18 +257,18 @@ func MakeRequestFull(client http.Client, method, target string, timeout int64, r resp, err := client.Do(req.WithContext(ctx)) if err == context.DeadlineExceeded { printWarn("Error: %s - skipping request.", err) - return nil, "", 0, "", "" + return nil, "", 0, "", "", nil } else if err != nil && err != context.Canceled && err != io.EOF { if (strings.Contains(fmt.Sprint(err), "tls") || strings.Contains(fmt.Sprint(err), "x509")) && !strings.Contains(fmt.Sprint(err), "user canceled") { - die("Try supplying the --insecure flag.") + return nil, "", 0, "", "", fmt.Errorf("Try supplying the --insecure flag.") } else if strings.Contains(fmt.Sprint(err), "tcp") && strings.Contains(fmt.Sprint(err), "no such host") { - die("The target '%s' is not reachable. Check the declared host(s) and supply a target manually using -T if needed.", u.Scheme+"://"+u.Host) + return nil, "", 0, "", "", fmt.Errorf("The target '%s' is not reachable. Check the declared host(s) and supply a target manually using -T if needed.", u.Scheme+"://"+u.Host) } else if strings.Contains(fmt.Sprint(err), "user canceled") { - return nil, "skipped", 1, "", "" + return nil, "skipped", 1, "", "", nil } else { printErr("Error: response not received.\n%v", err) } - return nil, "", 0, "", "" + return nil, "", 0, "", "", nil } bodyBytes, _ := io.ReadAll(resp.Body) @@ -281,23 +281,24 @@ func MakeRequestFull(client http.Client, method, target string, timeout int64, r if locErr != nil || redirect == nil { requestStatus = resp.StatusCode depth = 0 - return bodyBytes, bodyString, requestStatus, contentTypeHeader, sentUserAgent + return bodyBytes, bodyString, requestStatus, contentTypeHeader, sentUserAgent, nil } depth += 1 var ct, ua string + var rerr error // redirect.String() keeps the query string, and the buffered body is // replayed through a fresh reader. - bodyBytes, bodyString, requestStatus, ct, ua = MakeRequestFull(client, method, redirect.String(), timeout, bytes.NewReader(reqBody)) - return bodyBytes, bodyString, requestStatus, ct, ua + bodyBytes, bodyString, requestStatus, ct, ua, rerr = MakeRequestFull(client, method, redirect.String(), timeout, bytes.NewReader(reqBody)) + return bodyBytes, bodyString, requestStatus, ct, ua, rerr } requestStatus = resp.StatusCode depth = 0 - return bodyBytes, bodyString, requestStatus, contentTypeHeader, sentUserAgent + return bodyBytes, bodyString, requestStatus, contentTypeHeader, sentUserAgent, nil } -func CheckAndConfigureProxy() (client http.Client, replayClient *http.Client) { +func CheckAndConfigureProxy() (client http.Client, replayClient *http.Client, err error) { var proxyUrl *url.URL transport := &http.Transport{} @@ -323,9 +324,9 @@ func CheckAndConfigureProxy() (client http.Client, replayClient *http.Client) { if insecure { replayTransport.TLSClientConfig = &tls.Config{InsecureSkipVerify: true} } - replayProxyUrl, err := url.Parse(replayProxy) - if err != nil { - die("Error parsing replay proxy URL: %v", err) + replayProxyUrl, parseErr := url.Parse(replayProxy) + if parseErr != nil { + return client, replayClient, fmt.Errorf("error parsing replay proxy URL: %v", parseErr) } replayTransport.Proxy = http.ProxyURL(replayProxyUrl) replayClient = &http.Client{ @@ -336,7 +337,7 @@ func CheckAndConfigureProxy() (client http.Client, replayClient *http.Client) { } } - return client, replayClient + return client, replayClient, nil } // userAgent is the one the replayed request carried, so the copy reproduces it. diff --git a/cmd/requests_test.go b/cmd/requests_test.go index 7bc8b7a..d79ea57 100644 --- a/cmd/requests_test.go +++ b/cmd/requests_test.go @@ -112,7 +112,7 @@ func TestRedirectPreservesBodyAndQuery(t *testing.T) { }} want := `{"name":"bishopfox"}` - _, _, sc, _, _ := MakeRequestFull(client, "PUT", redirector.URL+"/first", 30, bytes.NewReader([]byte(want))) + _, _, sc, _, _, _ := MakeRequestFull(client, "PUT", redirector.URL+"/first", 30, bytes.NewReader([]byte(want))) if sc != http.StatusOK { t.Errorf("redirect not followed: status %d, want %d", sc, http.StatusOK) diff --git a/cmd/root.go b/cmd/root.go index e81717a..161d51d 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -1,6 +1,9 @@ package cmd import ( + "fmt" + "os" + "github.com/spf13/cobra" ) @@ -58,16 +61,27 @@ $ sj convert -u https://petstore.swagger.io/v2/swagger.json -o openapi.json`, PersistentPreRun: func(cmd *cobra.Command, args []string) { Mode = cmd.Name() }, - Run: func(cmd *cobra.Command, args []string) { + RunE: func(cmd *cobra.Command, args []string) error { if len(args) < 1 { - printErr("Command not specified. See the --help flag for usage.") + return fmt.Errorf("command not specified. See the --help flag for usage") } + return nil }, Version: "2.8.2", + // Errors and usage are surfaced once by Execute() with our own styling, so + // cobra should not also print them on a RunE failure. + SilenceErrors: true, + SilenceUsage: true, } +// Execute runs the root command. It is the single place in the package that +// terminates the process: command and library code return errors up to here +// instead of calling os.Exit, which keeps that logic testable and reusable. func Execute() { - cobra.CheckErr(rootCmd.Execute()) + if err := rootCmd.Execute(); err != nil { + printErr("%v", err) + os.Exit(1) + } } func init() { diff --git a/cmd/utils.go b/cmd/utils.go index f94c7f5..aa23394 100644 --- a/cmd/utils.go +++ b/cmd/utils.go @@ -40,10 +40,10 @@ type SchemaNode struct { AdditionalProperties *SchemaNode } -func BuildRequestsFromPaths(spec map[string]interface{}, client http.Client, replayClient *http.Client) { +func BuildRequestsFromPaths(spec map[string]interface{}, client http.Client, replayClient *http.Client) error { paths, ok := spec["paths"].(map[string]interface{}) if !ok || paths == nil { - die("Could not find any defined operations. Review the file manually.") + return fmt.Errorf("could not find any defined operations. Review the file manually") } pathKeys := make([]string, 0, len(paths)) @@ -364,7 +364,10 @@ func BuildRequestsFromPaths(spec map[string]interface{}, client http.Client, rep // the wire cannot disagree. sentBody := []byte(v.body) - _, resp, sc, _, sentUA := MakeRequestFull(client, strings.ToUpper(method), targetURL, timeout, bytes.NewReader(sentBody)) + _, resp, sc, _, sentUA, reqErr := MakeRequestFull(client, strings.ToUpper(method), targetURL, timeout, bytes.NewReader(sentBody)) + if reqErr != nil { + return reqErr + } tempResponsePreviewLength := responsePreviewLength if len(resp) <= responsePreviewLength { @@ -456,19 +459,20 @@ func BuildRequestsFromPaths(spec map[string]interface{}, client http.Client, rep if verbose { err := json.Unmarshal([]byte(strings.TrimPrefix(jsonResultsStringArray[r], ",")), &verboseResult) if err != nil { - die("Error marshalling JSON: %v", err) + return fmt.Errorf("error unmarshalling JSON: %v", err) } jsonVerboseResultArray = append(jsonVerboseResultArray, verboseResult) } else { err := json.Unmarshal([]byte(strings.TrimPrefix(jsonResultsStringArray[r], ",")), &result) if err != nil { - die("Error marshalling JSON: %v", err) + return fmt.Errorf("error unmarshalling JSON: %v", err) } jsonResultArray = append(jsonResultArray, result) } } writeLog(8899, "", "", "", "", "", false) } + return nil } func EnforceSingleContentType(newContentType string) { @@ -679,7 +683,7 @@ func GenerateExample(node *SchemaNode) interface{} { } } -func GenerateRequests(bodyBytes []byte, client http.Client, replayClient *http.Client) { +func GenerateRequests(bodyBytes []byte, client http.Client, replayClient *http.Client) error { // Swagger UI bundles serve the spec wrapped in JavaScript (e.g. // swagger-ui-init.js). Detect that and unwrap before parsing so the // downstream YAML/JSON unmarshal succeeds. @@ -690,7 +694,10 @@ func GenerateRequests(bodyBytes []byte, client http.Client, replayClient *http.C } // Ingests the specification file - spec := SafelyUnmarshalSpec(bodyBytes) + spec, err := SafelyUnmarshalSpec(bodyBytes) + if err != nil { + return err + } // Checks defined security schemes and prompts for authentication CheckSecuritySchemes(spec) @@ -741,44 +748,42 @@ func GenerateRequests(bodyBytes []byte, client http.Client, replayClient *http.C } else if v, ok := spec["openapi"].(string); ok && strings.HasPrefix(v, "3") { // OpenAPI (v3) if servers, ok := spec["servers"].([]interface{}); ok && len(servers) > 0 { - if len(servers) > 1 { - if !quiet && (Mode != "endpoints") && apiTarget == "" { - printWarn("Multiple servers detected in documentation. You can manually set a server to test with the -T flag.\nThe detected servers are as follows:") - for i := range servers { - if srv, ok := servers[i].(map[string]interface{}); ok { - if serverURL, ok := srv["url"].(string); ok { - printInfo("%s\n", serverURL) - } + // When several servers are declared and the user hasn't pinned one + // with -T, warn and fall through to the first declared server rather + // than aborting the run. + if len(servers) > 1 && !quiet && Mode != "endpoints" && apiTarget == "" { + printWarn("Multiple servers detected in documentation. Defaulting to the first; set a specific server with the -T flag.\nThe detected servers are as follows:") + for i := range servers { + if srv, ok := servers[i].(map[string]interface{}); ok { + if serverURL, ok := srv["url"].(string); ok { + printInfo("%s\n", serverURL) } } } - } else { - if srv, ok := servers[0].(map[string]interface{}); ok { - if serverURL, ok := srv["url"].(string); ok { - if strings.Contains(serverURL, "://") { - // Full URL in server - if parsedServerURL, err := url.Parse(serverURL); err == nil { - basePath = normalizeBasePath(parsedServerURL.Path) - if apiTarget == "" { - apiTarget = parsedServerURL.Scheme + "://" + parsedServerURL.Host - } - } - } else if serverURL == "/" { - basePath = "" - } else { - // Relative URL - this becomes the basePath - basePath = normalizeBasePath(serverURL) - // Only try to construct apiTarget if -T wasn't used + } + if srv, ok := servers[0].(map[string]interface{}); ok { + if serverURL, ok := srv["url"].(string); ok { + if strings.Contains(serverURL, "://") { + // Full URL in server + if parsedServerURL, err := url.Parse(serverURL); err == nil { + basePath = normalizeBasePath(parsedServerURL.Path) if apiTarget == "" { - if u.Scheme != "" && u.Host != "" { - apiTarget = u.Scheme + "://" + u.Host - } else { - // Local file with relative server URL and no -T flag - // Only fail for commands that need full URLs - if Mode != "endpoints" { - die("Spec has relative server URL '%s' but no base URL available. Use -T to specify target server.", serverURL) - } - } + apiTarget = parsedServerURL.Scheme + "://" + parsedServerURL.Host + } + } + } else if serverURL == "/" { + basePath = "" + } else { + // Relative URL - this becomes the basePath + basePath = normalizeBasePath(serverURL) + // Only try to construct apiTarget if -T wasn't used + if apiTarget == "" { + if u.Scheme != "" && u.Host != "" { + apiTarget = u.Scheme + "://" + u.Host + } else if Mode != "endpoints" { + // Local file with relative server URL and no -T flag: + // there is no host to resolve, so this can't proceed. + return fmt.Errorf("spec has relative server URL '%s' but no base URL available. Use -T to specify target server", serverURL) } } } @@ -791,12 +796,10 @@ func GenerateRequests(bodyBytes []byte, client http.Client, replayClient *http.C if apiTarget == "" { if u.Scheme != "" && u.Host != "" { apiTarget = u.Scheme + "://" + u.Host - } else { - // No server info and no URL to parse - require user to specify target - // Only fail for commands that need full URLs - if Mode != "endpoints" { - die("No server information found in spec and no URL provided. Use -T to specify target server.") - } + } else if Mode != "endpoints" { + // No server info and no URL to parse - require user to specify target. + // Only fail for commands that need full URLs. + return fmt.Errorf("no server information found in spec and no URL provided. Use -T to specify target server") } } @@ -806,7 +809,7 @@ func GenerateRequests(bodyBytes []byte, client http.Client, replayClient *http.C } // Reviews all defined API routes and builds requests as defined - BuildRequestsFromPaths(spec, client, replayClient) + return BuildRequestsFromPaths(spec, client, replayClient) } func ResolveRef(spec map[string]interface{}, ref string) map[string]interface{} { @@ -975,8 +978,8 @@ func ResolveExternalRef(ref string, baseDir string) map[string]interface{} { return nil } - externalSpec := SafelyUnmarshalSpec(fileData) - if externalSpec == nil { + externalSpec, err := SafelyUnmarshalSpec(fileData) + if err != nil || externalSpec == nil { return nil } @@ -1052,14 +1055,14 @@ func looksLikeJSSpec(b []byte) bool { return false } -func SafelyUnmarshalSpec(data []byte) map[string]interface{} { +func SafelyUnmarshalSpec(data []byte) (map[string]interface{}, error) { var doc map[string]interface{} if err := yaml.Unmarshal(data, &doc); err != nil { - die("Failed to unmarshal API documentation: %v", err) + return nil, fmt.Errorf("failed to unmarshal API documentation: %v", err) } - return doc + return doc, nil } /* diff --git a/cmd/utils_test.go b/cmd/utils_test.go index 4ffa2e2..007a623 100644 --- a/cmd/utils_test.go +++ b/cmd/utils_test.go @@ -34,7 +34,7 @@ func TestSwaggerV2SchemeHandling(t *testing.T) { return } - spec := SafelyUnmarshalSpec(data) + spec, _ := SafelyUnmarshalSpec(data) if spec == nil { t.Fatal("Failed to unmarshal spec") } @@ -75,7 +75,7 @@ func TestExternalReferenceResolution(t *testing.T) { return } - spec := SafelyUnmarshalSpec(data) + spec, _ := SafelyUnmarshalSpec(data) if spec == nil { t.Fatal("Failed to unmarshal spec") } @@ -122,7 +122,7 @@ func TestNestedExternalReferences(t *testing.T) { return } - externalSpec := SafelyUnmarshalSpec(data) + externalSpec, _ := SafelyUnmarshalSpec(data) if externalSpec == nil { t.Fatal("Failed to unmarshal external schema") } @@ -180,7 +180,7 @@ func TestQueryObjectParameterHandling(t *testing.T) { return } - spec := SafelyUnmarshalSpec(data) + spec, _ := SafelyUnmarshalSpec(data) if spec == nil { t.Fatal("Failed to unmarshal spec") } @@ -243,7 +243,7 @@ func TestRequestBodyContextPreservation(t *testing.T) { return } - spec := SafelyUnmarshalSpec(data) + spec, _ := SafelyUnmarshalSpec(data) if spec == nil { t.Fatal("Failed to unmarshal spec") } @@ -298,7 +298,7 @@ func TestDefaultValueHandling(t *testing.T) { return } - spec := SafelyUnmarshalSpec(data) + spec, _ := SafelyUnmarshalSpec(data) if spec == nil { t.Fatal("Failed to unmarshal spec") } @@ -366,7 +366,7 @@ func TestJSONCurlQuoting(t *testing.T) { return } - spec := SafelyUnmarshalSpec(data) + spec, _ := SafelyUnmarshalSpec(data) if spec == nil { t.Fatal("Failed to unmarshal spec") } @@ -467,7 +467,7 @@ func TestRelativeServerURLHandling(t *testing.T) { return } - spec := SafelyUnmarshalSpec(data) + spec, _ := SafelyUnmarshalSpec(data) if spec == nil { t.Fatal("Failed to unmarshal spec") } @@ -546,7 +546,7 @@ func TestTargetFlagPreservesSpecBasePath(t *testing.T) { return } - spec := SafelyUnmarshalSpec(data) + spec, _ := SafelyUnmarshalSpec(data) if spec == nil { t.Fatal("Failed to unmarshal spec") } @@ -599,7 +599,7 @@ func TestOpenAPIv3AbsoluteServerURLWithTargetFlag(t *testing.T) { return } - spec := SafelyUnmarshalSpec(data) + spec, _ := SafelyUnmarshalSpec(data) if spec == nil { t.Fatal("Failed to unmarshal spec") } @@ -669,7 +669,7 @@ func TestOpenAPIv3AbsoluteServerURLWithoutTargetFlag(t *testing.T) { return } - spec := SafelyUnmarshalSpec(data) + spec, _ := SafelyUnmarshalSpec(data) if spec == nil { t.Fatal("Failed to unmarshal spec") } @@ -704,6 +704,87 @@ func TestOpenAPIv3AbsoluteServerURLWithoutTargetFlag(t *testing.T) { } } +// withResolveGlobals saves and restores the globals GenerateRequests reads while +// resolving a target, so these tests don't leak state into one another. +func withResolveGlobals(t *testing.T, mode string, fn func()) { + t.Helper() + oldMode, oldSwaggerURL, oldAPITarget, oldBasePath, oldQuiet := Mode, swaggerURL, apiTarget, basePath, quiet + defer func() { + Mode, swaggerURL, apiTarget, basePath, quiet = oldMode, oldSwaggerURL, oldAPITarget, oldBasePath, oldQuiet + }() + Mode = mode + swaggerURL = "" + apiTarget = "" + basePath = "" + quiet = true + fn() +} + +// TestGenerateRequestsMultipleServersDefaultsToFirst verifies that a spec +// declaring several servers no longer aborts the run (previously a fatal die()): +// GenerateRequests resolves against the first declared server instead. +func TestGenerateRequestsMultipleServersDefaultsToFirst(t *testing.T) { + spec := []byte(`{"openapi":"3.0.0","info":{"title":"t","version":"1"},"servers":[{"url":"https://api1.example.com/v1"},{"url":"https://api2.example.com/v2"}],"paths":{"/users":{"get":{"responses":{"200":{"description":"ok"}}}}}}`) + + withResolveGlobals(t, "endpoints", func() { + var err error + _ = captureStdout(t, func() { err = GenerateRequests(spec, http.Client{}, nil) }) + if err != nil { + t.Fatalf("expected no error for multiple servers, got: %v", err) + } + if apiTarget != "https://api1.example.com" { + t.Errorf("expected apiTarget to default to first server host, got '%s'", apiTarget) + } + if basePath != "/v1" { + t.Errorf("expected basePath from first server, got '%s'", basePath) + } + }) +} + +// TestGenerateRequestsRelativeServerURLWithoutTargetReturnsError verifies the +// relative-server-URL-with-no-base case now returns an error (it used to die() +// and exit the process), so callers can surface it gracefully. +func TestGenerateRequestsRelativeServerURLWithoutTargetReturnsError(t *testing.T) { + spec := []byte(`{"openapi":"3.0.0","info":{"title":"t","version":"1"},"servers":[{"url":"/api"}],"paths":{"/users":{"get":{"responses":{"200":{"description":"ok"}}}}}}`) + + withResolveGlobals(t, "automate", func() { + var err error + _ = captureStdout(t, func() { err = GenerateRequests(spec, http.Client{}, nil) }) + if err == nil { + t.Fatal("expected an error for relative server URL with no base and no -T, got nil") + } + }) +} + +// TestGenerateRequestsRelativeServerURLEndpointsModeNoError confirms the +// endpoints-mode exemption is preserved: listing endpoints needs no host, so the +// same ambiguous spec resolves without error. +func TestGenerateRequestsRelativeServerURLEndpointsModeNoError(t *testing.T) { + spec := []byte(`{"openapi":"3.0.0","info":{"title":"t","version":"1"},"servers":[{"url":"/api"}],"paths":{"/users":{"get":{"responses":{"200":{"description":"ok"}}}}}}`) + + withResolveGlobals(t, "endpoints", func() { + var err error + _ = captureStdout(t, func() { err = GenerateRequests(spec, http.Client{}, nil) }) + if err != nil { + t.Fatalf("expected no error in endpoints mode, got: %v", err) + } + }) +} + +// TestGenerateRequestsNoServerNoURLReturnsError verifies the no-server-info and +// no-URL case now returns an error rather than exiting the process. +func TestGenerateRequestsNoServerNoURLReturnsError(t *testing.T) { + spec := []byte(`{"openapi":"3.0.0","info":{"title":"t","version":"1"},"paths":{"/users":{"get":{"responses":{"200":{"description":"ok"}}}}}}`) + + withResolveGlobals(t, "automate", func() { + var err error + _ = captureStdout(t, func() { err = GenerateRequests(spec, http.Client{}, nil) }) + if err == nil { + t.Fatal("expected an error when no server info and no URL are available, got nil") + } + }) +} + func TestNormalizeBasePath(t *testing.T) { cases := []struct { in string