diff --git a/cmd/db_export.go b/cmd/db_export.go index 68c1d57..c302e5d 100644 --- a/cmd/db_export.go +++ b/cmd/db_export.go @@ -131,6 +131,7 @@ func exportCSV(entries []storage.Entry) error { "base_category", "disabled", "is_ignored", + "base_in_scope", } if err := writer.Write(header); err != nil { return err @@ -153,6 +154,10 @@ func exportCSV(entries []storage.Entry) error { e.BaseCategory, fmt.Sprintf("%t", e.Disabled), fmt.Sprintf("%t", e.IsIgnored), + "", + } + if e.BaseInScope != nil { + record[len(record)-1] = fmt.Sprintf("%t", *e.BaseInScope) } if err := writer.Write(record); err != nil { return err diff --git a/cmd/db_import.go b/cmd/db_import.go index c66c9cd..432ea7f 100644 --- a/cmd/db_import.go +++ b/cmd/db_import.go @@ -208,11 +208,17 @@ func groupEntriesForImport(entries []storage.Entry) ([]programKey, map[programKe items := grouped[key] pos, exists := index[key][identity] if !exists { + // An "ai" row's InScope is the variant's effective value; the base + // target's own flag travels in BaseInScope (absent in old backups). + baseInScope := e.InScope + if e.BaseInScope != nil { + baseInScope = *e.BaseInScope + } items = append(items, storage.TargetItem{ URI: raw, Category: baseCat, Description: e.Description, - InScope: e.InScope, + InScope: baseInScope, IsBBP: e.IsBBP, }) pos = len(items) - 1 @@ -234,7 +240,8 @@ func groupEntriesForImport(entries []storage.Entry) ([]programKey, map[programKe v.HasCategory = true v.Category = e.Category } - v.HasInScope = true + // Only an effective value that differs from the base is an override. + v.HasInScope = e.BaseInScope == nil || *e.BaseInScope != e.InScope v.InScope = e.InScope item.Variants = append(item.Variants, v) } @@ -381,6 +388,12 @@ func parseimportCSV(r io.Reader) ([]storage.Entry, error) { inScope = parseCSVBool(raw) } + var baseInScope *bool + if raw := field(record, "base_in_scope"); raw != "" { + b := parseCSVBool(raw) + baseInScope = &b + } + entries = append(entries, storage.Entry{ ProgramURL: field(record, "program_url"), Platform: field(record, "platform"), @@ -390,6 +403,7 @@ func parseimportCSV(r io.Reader) ([]storage.Entry, error) { Category: field(record, "category"), BaseCategory: field(record, "base_category"), BaseTargetRaw: field(record, "base_target_raw"), + BaseInScope: baseInScope, Description: field(record, "description"), InScope: inScope, IsBBP: parseCSVBool(field(record, "is_bbp")), diff --git a/cmd/db_roundtrip_test.go b/cmd/db_roundtrip_test.go index ec3653e..29489a8 100644 --- a/cmd/db_roundtrip_test.go +++ b/cmd/db_roundtrip_test.go @@ -372,3 +372,59 @@ func TestIntegration_ImportMergesWithoutDeletingLiveTargets(t *testing.T) { t.Fatalf("replace import should leave only the file contents, got %#v", got) } } + +// An AI row's in_scope is the variant's effective value. Import used it as the +// base target's in_scope, so an in-scope target with an out-of-scope variant +// came back out of scope. +func TestIntegration_ImportKeepsBaseInScopeUnderVariantOverride(t *testing.T) { + db, raw := openRoundtripDB(t) + ctx := context.Background() + platform := "itest_basescope_" + strconv.FormatInt(time.Now().UnixNano(), 36) + programURL := "https://example.com/" + platform + "/a" + cleanup := func() { + _, _ = raw.ExecContext(ctx, `DELETE FROM targets_ai_enhanced WHERE target_id IN ( + SELECT tr.id FROM targets_raw tr JOIN programs p ON tr.program_id = p.id WHERE p.platform = $1)`, platform) + _, _ = raw.ExecContext(ctx, `DELETE FROM targets_raw WHERE program_id IN (SELECT id FROM programs WHERE platform = $1)`, platform) + _, _ = raw.ExecContext(ctx, `DELETE FROM scope_changes WHERE platform = $1`, platform) + _, _ = raw.ExecContext(ctx, `DELETE FROM programs WHERE platform = $1`, platform) + } + cleanup() + t.Cleanup(cleanup) + + built, err := storage.BuildEntries(programURL, platform, "a", []storage.TargetItem{{ + URI: "*.base.example.com", Category: "wildcard", InScope: true, + Variants: []storage.TargetVariant{{Value: "legacy.base.example.com", HasInScope: true, InScope: false}}, + }}) + if err != nil { + t.Fatalf("BuildEntries: %v", err) + } + if _, err := db.UpsertProgramEntriesWithOptions(ctx, programURL, platform, "a", built, + storage.UpsertOptions{SkipChangeLog: true}); err != nil { + t.Fatalf("seeding: %v", err) + } + exported, err := db.ListEntries(ctx, storage.ListOptions{Platform: platform, IncludeOOS: true}) + if err != nil { + t.Fatalf("ListEntries: %v", err) + } + + cleanup() + if _, failed := importEntries(ctx, db, exported, false); failed != 0 { + t.Fatalf("import failed for %d entries", failed) + } + + var baseInScope int + var variantInScope sql.NullInt64 + if err := raw.QueryRowContext(ctx, ` + SELECT tr.in_scope, a.in_scope FROM targets_raw tr + JOIN programs p ON tr.program_id = p.id + JOIN targets_ai_enhanced a ON a.target_id = tr.id + WHERE p.platform = $1`, platform).Scan(&baseInScope, &variantInScope); err != nil { + t.Fatalf("reading restored target: %v", err) + } + if baseInScope != 1 { + t.Errorf("base in_scope = %d after round trip, want 1", baseInScope) + } + if !variantInScope.Valid || variantInScope.Int64 != 0 { + t.Errorf("variant in_scope = %+v after round trip, want 0", variantInScope) + } +} diff --git a/cmd/exit_codes_test.go b/cmd/exit_codes_test.go index 5d90644..7902091 100644 --- a/cmd/exit_codes_test.go +++ b/cmd/exit_codes_test.go @@ -3,17 +3,26 @@ package cmd import ( "context" "errors" + "os" "strings" "testing" "github.com/spf13/cobra" "github.com/spf13/viper" + "github.com/zalando/go-keyring" ) +// TestMain swaps in the in-memory keyring so tests never read a developer's +// real credentials or block on a desktop Secret Service unlock prompt. +func TestMain(m *testing.M) { + keyring.MockInit() + os.Exit(m.Run()) +} + // runPollSubcommand invokes one poll subcommand's RunE directly with no // credentials available. Going through Execute would traverse up to the root // command and print its help instead of running this subcommand. Credentials -// resolve through viper here because the test environment has no OS keychain. +// resolve through viper here because TestMain mocks the OS keychain empty. func runPollSubcommand(t *testing.T, cmd *cobra.Command) error { t.Helper() viper.Reset() diff --git a/cmd/poll.go b/cmd/poll.go index 69cf3c2..877030d 100644 --- a/cmd/poll.go +++ b/cmd/poll.go @@ -41,18 +41,27 @@ var pollCmd = &cobra.Command{ proxyURL, _ := cmd.Flags().GetString("proxy") // Parent poll includes all configured platforms (including Immunefi). pollers, err := buildPollersFromConfig(cmd.Context(), proxyURL, nil) - if err != nil { - return err - } - if len(pollers) == 0 { - cmd.SilenceUsage = true - return fmt.Errorf("no platforms to poll; configure credentials or use a platform subcommand") - } - - return runPollWithPollers(cmd, pollers) + return pollAvailable(cmd, pollers, err) }, } +// pollAvailable polls every poller that was built even when others failed to +// authenticate, so one banned or expired login cannot skip the remaining +// platforms. The build error is still returned to keep the exit code non-zero. +func pollAvailable(cmd *cobra.Command, pollers []platforms.PlatformPoller, buildErr error) error { + if len(pollers) == 0 { + if buildErr != nil { + return buildErr + } + cmd.SilenceUsage = true + return fmt.Errorf("no platforms to poll; configure credentials or use a platform subcommand") + } + if buildErr != nil { + utils.Log.Warnf("Polling the platforms that authenticated; others failed: %v", buildErr) + } + return errors.Join(buildErr, runPollWithPollers(cmd, pollers)) +} + func init() { rootCmd.AddCommand(pollCmd) @@ -235,7 +244,9 @@ func runPollWithPollers(cmd *cobra.Command, pollers []platforms.PlatformPoller) continue } - if useDB { + if useDB && pollIsFiltered(opts) { + utils.Log.Infof("Scope filters are active; skipping removal sync for %s so filtered-out scope is kept", p.Name()) + } else if useDB { // After processing all programs for a platform, sync the state. // This will mark any programs that were not in the latest poll as disabled. removedProgramChanges, err := db.SyncPlatformPrograms(ctx, p.Name(), polledProgramURLs) @@ -261,6 +272,14 @@ func runPollWithPollers(cmd *cobra.Command, pollers []platforms.PlatformPoller) return errors.Join(runErrs...) } +// pollIsFiltered reports whether the poll returns a filtered subset of the +// platform's scope. A subset must be merged, never reconciled: treating it as +// the full scope deletes every filtered-out target and disables every +// filtered-out program. +func pollIsFiltered(opts platforms.PollOptions) bool { + return opts.BountyOnly || opts.PrivateOnly || scope.GetAllStringsForCategories(opts.Categories) != nil +} + // emptyListingWouldWipe reports whether syncing an empty handle list would // disable programs already stored for the platform. Any non-zero stored count // is enough: a one-program platform is exactly the case a truncated listing @@ -346,51 +365,52 @@ func processProgramsConcurrently(ctx context.Context, cmd *cobra.Command, p plat var aiCandidates []storage.TargetItem var aiEnhancements map[string][]storage.TargetVariant - if aiNormalizer != nil && len(allItems) > 0 { + // Always reattach stored AI variants, not only under --ai: the + // upsert treats an entry without variants as "none desired" and + // would delete every stored variant on a plain --db poll. + if len(allItems) > 0 { var err error aiEnhancements, err = db.ListAIEnhancements(ctx, pd.Url) if err != nil { - utils.Log.Warnf("Failed to load AI enhancements for %s: %v", pd.Url, err) - aiEnhancements = nil + utils.Log.Warnf("Failed to load AI enhancements for %s: %v; skipping program to keep stored variants", pd.Url, err) + errorMu.Lock() + if firstError == nil { + firstError = err + } + errorMu.Unlock() + continue } } - if aiNormalizer != nil && len(allItems) > 0 { - aiCandidates = make([]storage.TargetItem, 0, len(allItems)) - for _, item := range allItems { - key := storage.BuildTargetCategoryKey(item.URI, item.Category) - if variants, ok := aiEnhancements[key]; ok && len(variants) > 0 { - clone := item - clone.Variants = append([]storage.TargetVariant(nil), variants...) - processedItems = append(processedItems, clone) - continue - } - aiCandidates = append(aiCandidates, item) + for _, item := range allItems { + key := storage.BuildTargetCategoryKey(item.URI, item.Category) + if variants := aiEnhancements[key]; len(variants) > 0 { + clone := item + clone.Variants = append([]storage.TargetVariant(nil), variants...) + processedItems = append(processedItems, clone) + continue } - - if len(aiCandidates) > 0 { - normalized, err := aiNormalizer.NormalizeTargets(ctx, ai.ProgramInfo{ - ProgramURL: pd.Url, - Platform: p.Name(), - Handle: h, - }, aiCandidates) - if err != nil { - utils.Log.Warnf("AI normalization failed for %s: %v", pd.Url, err) - processedItems = append(processedItems, aiCandidates...) - } else if len(normalized) > 0 { - processedItems = append(processedItems, normalized...) - } else { - processedItems = append(processedItems, aiCandidates...) - } + if aiNormalizer == nil { + processedItems = append(processedItems, item) + continue } + aiCandidates = append(aiCandidates, item) + } - // if there were no candidates but also no pre-existing enhancements, - // ensure raw items still get processed - if len(processedItems) == 0 { - processedItems = append(processedItems, allItems...) + if len(aiCandidates) > 0 { + normalized, err := aiNormalizer.NormalizeTargets(ctx, ai.ProgramInfo{ + ProgramURL: pd.Url, + Platform: p.Name(), + Handle: h, + }, aiCandidates) + if err != nil { + utils.Log.Warnf("AI normalization failed for %s: %v", pd.Url, err) + processedItems = append(processedItems, aiCandidates...) + } else if len(normalized) > 0 { + processedItems = append(processedItems, normalized...) + } else { + processedItems = append(processedItems, aiCandidates...) } - } else if len(processedItems) == 0 { - processedItems = append(processedItems, allItems...) } entries, err := storage.BuildEntries(pd.Url, p.Name(), h, processedItems) @@ -412,7 +432,7 @@ func processProgramsConcurrently(ctx context.Context, cmd *cobra.Command, p plat p.Name(), h, entries, - storage.UpsertOptions{SkipChangeLog: isFirstRunForPlatform}, + storage.UpsertOptions{SkipChangeLog: isFirstRunForPlatform, MergeOnly: pollIsFiltered(opts)}, ) if err != nil { @@ -420,6 +440,12 @@ func processProgramsConcurrently(ctx context.Context, cmd *cobra.Command, p plat utils.Log.Warnf("Potential scope wipe detected for program %s. Skipping update. This might be due to a broken poller or a platform API change.", pd.Url) continue // Don't treat this as a fatal error for the whole poll } + if errors.Is(err, storage.ErrProgramURLOwned) { + // e.g. `db add -u` stored this URL as a custom program. Failing the + // platform here would skip its sync on every future run. + utils.Log.Warnf("Skipping program %s: %v", pd.Url, err) + continue + } // For other errors, log but continue processing utils.Log.Warnf("Database error for program %s: %v", pd.Url, err) errorMu.Lock() diff --git a/cmd/poll_db_integration_test.go b/cmd/poll_db_integration_test.go new file mode 100644 index 0000000..8b9735b --- /dev/null +++ b/cmd/poll_db_integration_test.go @@ -0,0 +1,132 @@ +package cmd + +import ( + "context" + "os" + "strconv" + "testing" + "time" + + "github.com/spf13/viper" + + "github.com/cozyGarage/bbscope/v2/pkg/platforms" + "github.com/cozyGarage/bbscope/v2/pkg/scope" + "github.com/cozyGarage/bbscope/v2/pkg/storage" +) + +// seedPollProgram stores one program with a url target carrying an AI variant +// plus a wildcard target, then returns a mock poller that serves only the url +// target (what --category url would return) and a raw target counter. +func seedPollProgram(t *testing.T) (*platforms.MockPoller, func() int, func() int) { + t.Helper() + db, raw := openRoundtripDB(t) + ctx := context.Background() + platform := "itest_poll_" + strconv.FormatInt(time.Now().UnixNano(), 36) + programURL := "https://example.com/" + platform + "/a" + + cleanup := func() { + _, _ = raw.ExecContext(ctx, `DELETE FROM targets_ai_enhanced WHERE target_id IN ( + SELECT tr.id FROM targets_raw tr JOIN programs p ON tr.program_id = p.id WHERE p.platform = $1)`, platform) + _, _ = raw.ExecContext(ctx, `DELETE FROM targets_raw WHERE program_id IN (SELECT id FROM programs WHERE platform = $1)`, platform) + _, _ = raw.ExecContext(ctx, `DELETE FROM scope_changes WHERE platform = $1`, platform) + _, _ = raw.ExecContext(ctx, `DELETE FROM programs WHERE platform = $1`, platform) + } + cleanup() + t.Cleanup(cleanup) + + built, err := storage.BuildEntries(programURL, platform, "a", []storage.TargetItem{ + {URI: "https://app.poll.example.com", Category: "url", InScope: true, + Variants: []storage.TargetVariant{{Value: "app.poll.example.com", HasCategory: true, Category: "url"}}}, + {URI: "*.poll.example.com", Category: "wildcard", InScope: true}, + }) + if err != nil { + t.Fatalf("BuildEntries: %v", err) + } + if _, err := db.UpsertProgramEntriesWithOptions(ctx, programURL, platform, "a", built, + storage.UpsertOptions{SkipChangeLog: true}); err != nil { + t.Fatalf("seeding: %v", err) + } + + viper.Reset() + t.Cleanup(viper.Reset) + viper.Set("db_url", os.Getenv("TEST_DB_URL")) + + mock := platforms.NewMockPoller(platform) + mock.Handles = []string{"a"} + mock.ScopeByHandle = map[string]scope.ProgramData{"a": { + Url: programURL, + InScope: []scope.ScopeElement{{Target: "https://app.poll.example.com", Category: "url"}}, + }} + + count := func(query string) func() int { + return func() int { + var n int + if err := raw.QueryRowContext(ctx, query, platform).Scan(&n); err != nil { + t.Fatalf("count: %v", err) + } + return n + } + } + targets := count(`SELECT count(*) FROM targets_raw tr JOIN programs p ON tr.program_id = p.id WHERE p.platform = $1`) + variants := count(`SELECT count(*) FROM targets_ai_enhanced a JOIN targets_raw tr ON a.target_id = tr.id + JOIN programs p ON tr.program_id = p.id WHERE p.platform = $1`) + return mock, targets, variants +} + +// A --category poll returns a subset of scope; storing it as the full scope +// deleted every filtered-out target. +func TestIntegration_FilteredPollKeepsFilteredOutScope(t *testing.T) { + mock, targets, _ := seedPollProgram(t) + cmd := newOrchestrationTestCmd() + _ = cmd.Flags().Set("db", "true") + _ = cmd.Flags().Set("category", "url") + + if err := runPollWithPollers(cmd, []platforms.PlatformPoller{mock}); err != nil { + t.Fatalf("runPollWithPollers: %v", err) + } + if got := targets(); got != 2 { + t.Fatalf("filtered poll left %d targets, want 2 (wildcard must survive)", got) + } +} + +// A plain --db poll (no --ai) carries no variants; it used to delete every +// stored AI variant for the targets it polled. +func TestIntegration_PollWithoutAIKeepsVariants(t *testing.T) { + mock, _, variants := seedPollProgram(t) + cmd := newOrchestrationTestCmd() + _ = cmd.Flags().Set("db", "true") + + if err := runPollWithPollers(cmd, []platforms.PlatformPoller{mock}); err != nil { + t.Fatalf("runPollWithPollers: %v", err) + } + if got := variants(); got != 1 { + t.Fatalf("poll without --ai left %d AI variants, want 1", got) + } +} + +// A custom program (`db add -u`) that owns a platform's program URL used to +// fail that platform's poll, skipping its sync on every run. +func TestIntegration_PollSkipsProgramOwnedByCustom(t *testing.T) { + mock, _, _ := seedPollProgram(t) + db, raw := openRoundtripDB(t) + ctx := context.Background() + ownedURL := "https://example.com/" + mock.Name() + "/owned" + t.Cleanup(func() { + _, _ = raw.ExecContext(ctx, `DELETE FROM targets_raw WHERE program_id IN (SELECT id FROM programs WHERE url = $1)`, ownedURL) + _, _ = raw.ExecContext(ctx, `DELETE FROM programs WHERE url = $1`, ownedURL) + }) + if _, err := db.AddCustomTarget(ctx, "owned.example.com", "url", ownedURL); err != nil { + t.Fatalf("AddCustomTarget: %v", err) + } + mock.Handles = append(mock.Handles, "owned") + mock.ScopeByHandle["owned"] = scope.ProgramData{ + Url: ownedURL, + InScope: []scope.ScopeElement{{Target: "owned.example.com", Category: "url"}}, + } + + cmd := newOrchestrationTestCmd() + _ = cmd.Flags().Set("db", "true") + if err := runPollWithPollers(cmd, []platforms.PlatformPoller{mock}); err != nil { + t.Fatalf("runPollWithPollers: %v (a custom-owned URL must not fail the platform)", err) + } +} diff --git a/cmd/poll_orchestration_test.go b/cmd/poll_orchestration_test.go index 3abccc9..f69fdb3 100644 --- a/cmd/poll_orchestration_test.go +++ b/cmd/poll_orchestration_test.go @@ -96,3 +96,18 @@ func TestRunPollWithPollers_EmptyHandles(t *testing.T) { t.Fatalf("empty handle list should succeed: %v", err) } } + +// One platform's auth failure used to abort `bbscope poll` before any of the +// platforms that did authenticate were polled. +func TestPollAvailable_AuthFailureStillPollsOthers(t *testing.T) { + mock := platforms.NewMockPoller("mock") + authErr := errors.New("bugcrowd: 403") + + err := pollAvailable(newOrchestrationTestCmd(), []platforms.PlatformPoller{mock}, authErr) + if !errors.Is(err, authErr) { + t.Fatalf("err = %v, want the auth error kept for the exit code", err) + } + if got := len(mock.FetchedHandles()); got != 2 { + t.Fatalf("fetched %d programs, want 2: authenticated platforms must still be polled", got) + } +} diff --git a/pkg/credentials/credentials_test.go b/pkg/credentials/credentials_test.go index 1fc4d31..4aeb4bc 100644 --- a/pkg/credentials/credentials_test.go +++ b/pkg/credentials/credentials_test.go @@ -1,10 +1,20 @@ package credentials import ( + "os" "strings" "testing" + + "github.com/zalando/go-keyring" ) +// TestMain swaps in the in-memory keyring so tests never block on a real +// desktop Secret Service (dbus) unlock prompt. +func TestMain(m *testing.M) { + keyring.MockInit() + os.Exit(m.Run()) +} + func TestKeyConstants(t *testing.T) { // Verify all key constants are properly formatted keys := ListKeys() diff --git a/pkg/platforms/bugcrowd/bugcrowd.go b/pkg/platforms/bugcrowd/bugcrowd.go index 8b90408..e78522f 100644 --- a/pkg/platforms/bugcrowd/bugcrowd.go +++ b/pkg/platforms/bugcrowd/bugcrowd.go @@ -1,6 +1,7 @@ package bugcrowd import ( + "context" "crypto/tls" "errors" "fmt" @@ -71,19 +72,21 @@ func rateLimitedRequestWorker() { } // rateLimitedSendHTTPRequest is a wrapper for whttp.SendHTTPRequest that enforces the 1-req/sec rate limit. -func rateLimitedSendHTTPRequest(req *whttp.WHTTPReq, client *retryablehttp.Client) (*whttp.WHTTPRes, error) { +// ctx cancels both the wait for a rate-limit slot and the request itself. +func rateLimitedSendHTTPRequest(ctx context.Context, req *whttp.WHTTPReq, client *retryablehttp.Client) (*whttp.WHTTPRes, error) { + req.Ctx = ctx resultChan := make(chan rateLimitedResult, 1) - rateLimitRequestChan <- rateLimitedRequest{ - req: req, - client: client, - resultChan: resultChan, + select { + case rateLimitRequestChan <- rateLimitedRequest{req: req, client: client, resultChan: resultChan}: + case <-ctx.Done(): + return nil, ctx.Err() } result := <-resultChan return result.res, result.err } // Automated email + password login. 2FA needs to be disabled -func Login(email, password, otpSecret, proxy string) (string, error) { +func Login(ctx context.Context, email, password, otpSecret, proxy string) (string, error) { // Create a cookie jar jar, err := cookiejar.New(nil) if err != nil { @@ -121,7 +124,7 @@ func Login(email, password, otpSecret, proxy string) (string, error) { } } - firstRes, err := rateLimitedSendHTTPRequest( + firstRes, err := rateLimitedSendHTTPRequest(ctx, &whttp.WHTTPReq{ Method: "GET", URL: "https://identity.bugcrowd.com/login?user_hint=researcher&returnTo=/dashboard", @@ -148,7 +151,7 @@ func Login(email, password, otpSecret, proxy string) (string, error) { } // Step 1: Initial login with username/password (without OTP) - firstLoginRes, err := rateLimitedSendHTTPRequest( + firstLoginRes, err := rateLimitedSendHTTPRequest(ctx, &whttp.WHTTPReq{ Method: "POST", URL: "https://identity.bugcrowd.com/login", @@ -192,7 +195,7 @@ func Login(email, password, otpSecret, proxy string) (string, error) { } // Step 2: Submit OTP - otpRes, err := rateLimitedSendHTTPRequest( + otpRes, err := rateLimitedSendHTTPRequest(ctx, &whttp.WHTTPReq{ Method: "POST", URL: "https://identity.bugcrowd.com/auth/otp-challenge", @@ -229,7 +232,7 @@ func Login(email, password, otpSecret, proxy string) (string, error) { return "", err } - redirectRes, err := rateLimitedSendHTTPRequest( + redirectRes, err := rateLimitedSendHTTPRequest(ctx, &whttp.WHTTPReq{ Method: "GET", URL: redirectUrl, @@ -331,7 +334,7 @@ func resolveBugcrowdAPIURL(pathOrURL string) (string, error) { return resolved.String(), nil } -func GetProgramHandles(sessionToken string, engagementType string, pvtOnly bool) ([]string, error) { +func GetProgramHandles(ctx context.Context, sessionToken string, engagementType string, pvtOnly bool) ([]string, error) { pageIndex := 1 var totalCount int totalCountKnown := false @@ -345,7 +348,7 @@ func GetProgramHandles(sessionToken string, engagementType string, pvtOnly bool) var res *whttp.WHTTPRes var err error - res, err = rateLimitedSendHTTPRequest( + res, err = rateLimitedSendHTTPRequest(ctx, &whttp.WHTTPReq{ Method: "GET", URL: listEndpointURL + strconv.Itoa(pageIndex), @@ -416,7 +419,7 @@ func GetProgramHandles(sessionToken string, engagementType string, pvtOnly bool) return paths, nil } -func GetProgramScope(handle string, categories string, token string) (pData scope.ProgramData, err error) { +func GetProgramScope(ctx context.Context, handle string, categories string, token string) (pData scope.ProgramData, err error) { isEngagement := strings.HasPrefix(handle, "/engagements/") if isEngagement { handle = strings.TrimPrefix(handle, "/engagements/") @@ -426,19 +429,19 @@ func GetProgramScope(handle string, categories string, token string) (pData scop if isEngagement { var getBriefVersionDocument string - getBriefVersionDocument, err = getEngagementBriefVersionDocument("/engagements/"+handle, token) + getBriefVersionDocument, err = getEngagementBriefVersionDocument(ctx, "/engagements/"+handle, token) if err != nil { return pData, err } if getBriefVersionDocument != "" { - err = extractScopeFromEngagement(getBriefVersionDocument, categories, token, &pData) + err = extractScopeFromEngagement(ctx, getBriefVersionDocument, categories, token, &pData) if err != nil { return pData, err } } } else { - err = extractScopeFromTargetGroups(pData.Url, categories, token, &pData) + err = extractScopeFromTargetGroups(ctx, pData.Url, categories, token, &pData) if err != nil { return pData, err } @@ -447,12 +450,12 @@ func GetProgramScope(handle string, categories string, token string) (pData scop return pData, nil } -func getEngagementBriefVersionDocument(handle string, token string) (string, error) { +func getEngagementBriefVersionDocument(ctx context.Context, handle string, token string) (string, error) { pageURL, err := resolveBugcrowdAPIURL(handle) if err != nil { return "", err } - res, err := rateLimitedSendHTTPRequest( + res, err := rateLimitedSendHTTPRequest(ctx, &whttp.WHTTPReq{ Method: "GET", URL: pageURL, @@ -499,7 +502,7 @@ func getEngagementBriefVersionDocument(handle string, token string) (string, err return path + ".json", nil } -func extractScopeFromEngagement(getBriefVersionDocument string, categories string, token string, pData *scope.ProgramData) (err error) { +func extractScopeFromEngagement(ctx context.Context, getBriefVersionDocument string, categories string, token string, pData *scope.ProgramData) (err error) { if getBriefVersionDocument == "" || getBriefVersionDocument == ".json" { // Missing brief endpoint usually means compliance gate / 2FA / HTML change. // Do not invent sentinel targets that would pollute storage and notifications. @@ -510,7 +513,7 @@ func extractScopeFromEngagement(getBriefVersionDocument string, categories strin if err != nil { return err } - res, err := rateLimitedSendHTTPRequest( + res, err := rateLimitedSendHTTPRequest(ctx, &whttp.WHTTPReq{ Method: "GET", URL: briefURL, @@ -582,8 +585,8 @@ func extractScopeFromEngagement(getBriefVersionDocument string, categories strin return nil } -func extractScopeFromTargetGroups(url string, categories string, token string, pData *scope.ProgramData) error { - res, err := rateLimitedSendHTTPRequest( +func extractScopeFromTargetGroups(ctx context.Context, url string, categories string, token string, pData *scope.ProgramData) error { + res, err := rateLimitedSendHTTPRequest(ctx, &whttp.WHTTPReq{ Method: "GET", URL: url + "/target_groups", @@ -612,7 +615,7 @@ func extractScopeFromTargetGroups(url string, categories string, token string, p noScopeTable := true for i, scopeTableURL := range gjson.Get(res.BodyString, "groups.#.targets_url").Array() { inScope := gjson.Get(res.BodyString, fmt.Sprintf("groups.%d.in_scope", i)).Bool() - err = extractScopeFromTargetTable(scopeTableURL.String(), categories, token, pData, inScope) + err = extractScopeFromTargetTable(ctx, scopeTableURL.String(), categories, token, pData, inScope) if err != nil { return err } @@ -626,12 +629,12 @@ func extractScopeFromTargetGroups(url string, categories string, token string, p return nil } -func extractScopeFromTargetTable(scopeTableURL string, categories string, token string, pData *scope.ProgramData, inScope bool) error { +func extractScopeFromTargetTable(ctx context.Context, scopeTableURL string, categories string, token string, pData *scope.ProgramData, inScope bool) error { tableURL, err := resolveBugcrowdAPIURL(scopeTableURL) if err != nil { return err } - res, err := rateLimitedSendHTTPRequest( + res, err := rateLimitedSendHTTPRequest(ctx, &whttp.WHTTPReq{ Method: "GET", URL: tableURL, diff --git a/pkg/platforms/bugcrowd/fetch_test.go b/pkg/platforms/bugcrowd/fetch_test.go index 2dea8a6..428fce6 100644 --- a/pkg/platforms/bugcrowd/fetch_test.go +++ b/pkg/platforms/bugcrowd/fetch_test.go @@ -200,7 +200,7 @@ func TestGetProgramHandles_SkipsEmptyBriefURL(t *testing.T) { defer srv.Close() withBaseURL(t, srv.URL) - got, err := GetProgramHandles("tok", "bug_bounty", false) + got, err := GetProgramHandles(context.Background(), "tok", "bug_bounty", false) if err != nil { t.Fatalf("GetProgramHandles: %v", err) } @@ -225,7 +225,7 @@ func TestGetProgramHandles_CountsEveryEmptyBriefURLRow(t *testing.T) { defer srv.Close() withBaseURL(t, srv.URL) - got, err := GetProgramHandles("tok", "bug_bounty", false) + got, err := GetProgramHandles(context.Background(), "tok", "bug_bounty", false) if err != nil { t.Fatalf("GetProgramHandles: %v", err) } @@ -245,7 +245,7 @@ func TestGetProgramHandles_WAFBanned(t *testing.T) { defer srv.Close() withBaseURL(t, srv.URL) - _, err := GetProgramHandles("tok", "bug_bounty", false) + _, err := GetProgramHandles(context.Background(), "tok", "bug_bounty", false) if err == nil || !strings.Contains(err.Error(), "WAF banned") { t.Fatalf("expected WAF banned error, got %v", err) } @@ -267,7 +267,7 @@ func TestGetProgramHandles_MissingTotalCountKeepsPaging(t *testing.T) { defer srv.Close() withBaseURL(t, srv.URL) - got, err := GetProgramHandles("tok", "bug_bounty", false) + got, err := GetProgramHandles(context.Background(), "tok", "bug_bounty", false) if err != nil { t.Fatalf("GetProgramHandles: %v", err) } @@ -284,7 +284,7 @@ func TestGetProgramHandles_Non2xxAndHTML(t *testing.T) { defer srv.Close() withBaseURL(t, srv.URL) - _, err := GetProgramHandles("tok", "bug_bounty", false) + _, err := GetProgramHandles(context.Background(), "tok", "bug_bounty", false) if err == nil { t.Fatal("expected an error for a non-2xx listing page, not an empty list") } @@ -297,7 +297,7 @@ func TestGetProgramHandles_HTML200(t *testing.T) { defer srv.Close() withBaseURL(t, srv.URL) - _, err := GetProgramHandles("tok", "bug_bounty", false) + _, err := GetProgramHandles(context.Background(), "tok", "bug_bounty", false) if err == nil { t.Fatal("expected an error when the listing body has no engagements array") } @@ -326,7 +326,7 @@ func TestExtractScopeFromTargetTable_RejectsOffOrigin(t *testing.T) { apiBaseURL = "https://bugcrowd.com" t.Cleanup(func() { apiBaseURL = orig }) - err := extractScopeFromTargetTable("https://evil.example/targets.json", "all", "tok", &scope.ProgramData{}, true) + err := extractScopeFromTargetTable(context.Background(), "https://evil.example/targets.json", "all", "tok", &scope.ProgramData{}, true) if err == nil { t.Fatal("expected off-origin targets_url to be rejected") } diff --git a/pkg/platforms/bugcrowd/poller.go b/pkg/platforms/bugcrowd/poller.go index 5fd6d28..3d6fec4 100644 --- a/pkg/platforms/bugcrowd/poller.go +++ b/pkg/platforms/bugcrowd/poller.go @@ -13,15 +13,6 @@ type Poller struct{ token string } // NewPollerFromToken uses an existing _bugcrowd_session token. func NewPollerFromToken(token string) *Poller { return &Poller{token: token} } -// NewPollerWithLogin logs in using email/password and OTP secret to obtain a session token. -func NewPollerWithLogin(email, password, otpSecret, proxy string) (*Poller, error) { - tok, err := Login(email, password, otpSecret, proxy) - if err != nil { - return nil, err - } - return &Poller{token: tok}, nil -} - func (p *Poller) Name() string { return "bc" } func (p *Poller) Authenticate(ctx context.Context, cfg platforms.AuthConfig) error { @@ -33,7 +24,7 @@ func (p *Poller) Authenticate(ctx context.Context, cfg platforms.AuthConfig) err return nil } if cfg.Email != "" && cfg.Password != "" && cfg.OtpSecret != "" { - tok, err := Login(cfg.Email, cfg.Password, cfg.OtpSecret, cfg.Proxy) + tok, err := Login(ctx, cfg.Email, cfg.Password, cfg.OtpSecret, cfg.Proxy) if err != nil { return err } @@ -45,14 +36,14 @@ func (p *Poller) Authenticate(ctx context.Context, cfg platforms.AuthConfig) err func (p *Poller) ListProgramHandles(ctx context.Context, opts platforms.PollOptions) ([]string, error) { // Reuse existing listing: bbpOnly controls category, pvtOnly controls open/private - handles, err := GetProgramHandles(p.token, "bug_bounty", opts.PrivateOnly) + handles, err := GetProgramHandles(ctx, p.token, "bug_bounty", opts.PrivateOnly) if err != nil { return nil, err } // Optionally include VDP if not bbpOnly. A VDP listing failure used to // be swallowed, so a WAF/HTML page looked like "no VDP programs". if !opts.BountyOnly { - vdp, err := GetProgramHandles(p.token, "vdp", opts.PrivateOnly) + vdp, err := GetProgramHandles(ctx, p.token, "vdp", opts.PrivateOnly) if err != nil { return nil, err } @@ -66,7 +57,7 @@ func (p *Poller) FetchProgramScope(ctx context.Context, handle string, opts plat if cats == "" { cats = "all" } - pd, err := GetProgramScope(handle, cats, p.token) + pd, err := GetProgramScope(ctx, handle, cats, p.token) if err != nil { return scope.ProgramData{Url: strings.TrimPrefix(handle, "/")}, err } diff --git a/pkg/platforms/hackerone/poller.go b/pkg/platforms/hackerone/poller.go index 1a9131a..193ffeb 100644 --- a/pkg/platforms/hackerone/poller.go +++ b/pkg/platforms/hackerone/poller.go @@ -55,6 +55,7 @@ func (p *Poller) ListProgramHandles(ctx context.Context, opts platforms.PollOpti var err error for attempt := 1; attempt <= maxListRetries; attempt++ { res, err = whttp.SendHTTPRequest(&whttp.WHTTPReq{ + Ctx: ctx, Method: "GET", URL: currentURL, Headers: []whttp.WHTTPHeader{{Name: "Authorization", Value: "Basic " + p.authB64}}, @@ -141,6 +142,7 @@ func (p *Poller) FetchProgramScope(ctx context.Context, handle string, opts plat for attempt := 1; attempt <= maxScopeRetries; attempt++ { res, err = whttp.SendHTTPRequest(&whttp.WHTTPReq{ + Ctx: ctx, Method: "GET", URL: currentPageURL, Headers: []whttp.WHTTPHeader{{Name: "Authorization", Value: "Basic " + p.authB64}}, diff --git a/pkg/platforms/immunefi/fetch_test.go b/pkg/platforms/immunefi/fetch_test.go index a02f26f..5761f6a 100644 --- a/pkg/platforms/immunefi/fetch_test.go +++ b/pkg/platforms/immunefi/fetch_test.go @@ -22,7 +22,7 @@ func withTestTransport(t *testing.T, url string) { origSleep := sleepFunc PLATFORM_URL = url maxRetries = 2 - sleepFunc = func(time.Duration) {} + sleepFunc = func(context.Context, time.Duration) error { return nil } t.Cleanup(func() { PLATFORM_URL = origURL maxRetries = origRetries @@ -160,6 +160,24 @@ func TestFetchWithRetry_RateLimitThenOK(t *testing.T) { } } +// A delisted program's 404 used to be retried maxRetries times with no pause. +func TestFetchWithRetry_ClientErrorFailsFast(t *testing.T) { + var hits int + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + hits++ + w.WriteHeader(http.StatusNotFound) + })) + defer srv.Close() + withTestTransport(t, srv.URL) + + if _, err := fetchWithRetry(context.Background(), srv.URL+"/"); err == nil { + t.Fatal("expected a 404 to fail") + } + if hits != 1 { + t.Fatalf("404 was requested %d times, want 1", hits) + } +} + func TestFetchWithRetry_HonorsCancel(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) cancel() diff --git a/pkg/platforms/immunefi/poller.go b/pkg/platforms/immunefi/poller.go index 6f0cd1a..e50cf0d 100644 --- a/pkg/platforms/immunefi/poller.go +++ b/pkg/platforms/immunefi/poller.go @@ -15,9 +15,21 @@ import ( ) // maxRetries and sleepFunc are package variables so httptest tests can avoid -// long exponential backoff against a local server. -var maxRetries = 20 -var sleepFunc = time.Sleep +// long exponential backoff against a local server. whttp's client already +// retries 429/5xx/network errors 10 times per call (honoring Retry-After), so +// this outer loop only covers a few longer outages; 20 here meant up to ~220 +// requests for one URL. +var maxRetries = 3 +var sleepFunc = func(ctx context.Context, d time.Duration) error { + t := time.NewTimer(d) + defer t.Stop() + select { + case <-ctx.Done(): + return ctx.Err() + case <-t.C: + return nil + } +} type Poller struct{} @@ -26,8 +38,9 @@ func (p *Poller) Name() string { return "immunefi" } // Authenticate is a no-op for Immunefi (no auth required) func (p *Poller) Authenticate(ctx context.Context, cfg platforms.AuthConfig) error { return nil } -// fetchWithRetry sends an HTTP request with retry logic for 429 rate limits. -// It will retry up to maxRetries times with exponential backoff. +// fetchWithRetry sends an HTTP request, retrying network errors, 429 rate +// limits, and 5xx responses up to maxRetries times with backoff. Other 4xx +// responses fail immediately: a delisted program's 404 never recovers. func fetchWithRetry(ctx context.Context, url string) (*whttp.WHTTPRes, error) { var lastErr error for attempt := 0; attempt < maxRetries; attempt++ { @@ -36,6 +49,7 @@ func fetchWithRetry(ctx context.Context, url string) (*whttp.WHTTPRes, error) { } res, err := whttp.SendHTTPRequest( &whttp.WHTTPReq{ + Ctx: ctx, Method: "GET", URL: url, Headers: []whttp.WHTTPHeader{ @@ -44,36 +58,26 @@ func fetchWithRetry(ctx context.Context, url string) (*whttp.WHTTPRes, error) { }, }, nil) - if err != nil { + backoff := time.Duration(attempt+1) * time.Second + switch { + case err != nil: lastErr = err - // Network error, retry with backoff - sleepFunc(time.Duration(attempt+1) * time.Second) - continue - } - - if res.StatusCode == 429 { - // Rate limited, wait with exponential backoff and retry - backoff := time.Duration(attempt+1) * 2 * time.Second - if backoff > 30*time.Second { - backoff = 30 * time.Second - } - sleepFunc(backoff) - continue - } - - if res.StatusCode >= 200 && res.StatusCode < 300 { + case res.StatusCode >= 200 && res.StatusCode < 300: return res, nil + case res.StatusCode == 429: + lastErr = fmt.Errorf("HTTP 429 for %s", url) + backoff = min(2*backoff, 30*time.Second) + case res.StatusCode >= 500: + // Immunefi occasionally serves transient 5xx during RSC navigations. + lastErr = fmt.Errorf("HTTP %d for %s", res.StatusCode, url) + default: + return nil, fmt.Errorf("HTTP %d for %s", res.StatusCode, url) + } + if err := sleepFunc(ctx, backoff); err != nil { + return nil, err } - - // Other error status — keep retrying (Immunefi occasionally serves - // transient non-2xx responses during RSC navigations). - lastErr = fmt.Errorf("HTTP %d for %s", res.StatusCode, url) - } - - if lastErr != nil { - return nil, fmt.Errorf("failed after %d retries: %w", maxRetries, lastErr) } - return nil, fmt.Errorf("failed after %d retries", maxRetries) + return nil, fmt.Errorf("failed after %d retries: %w", maxRetries, lastErr) } func (p *Poller) ListProgramHandles(ctx context.Context, opts platforms.PollOptions) ([]string, error) { diff --git a/pkg/platforms/intigriti/poller.go b/pkg/platforms/intigriti/poller.go index 57b33c7..60df089 100644 --- a/pkg/platforms/intigriti/poller.go +++ b/pkg/platforms/intigriti/poller.go @@ -58,6 +58,7 @@ func (p *Poller) ListProgramHandles(ctx context.Context, opts platforms.PollOpti return nil, err } res, err := whttp.SendHTTPRequest(&whttp.WHTTPReq{ + Ctx: ctx, Method: "GET", URL: fmt.Sprintf(apiBaseURL+"/programs?statusId=3&limit=%d&offset=%d", limit, offset), Headers: []whttp.WHTTPHeader{{Name: "Authorization", Value: "Bearer " + p.token}}, @@ -158,6 +159,7 @@ func (p *Poller) FetchProgramScope(ctx context.Context, handle string, opts plat for attempt := 1; ; attempt++ { var err error res, err = whttp.SendHTTPRequest(&whttp.WHTTPReq{ + Ctx: ctx, Method: "GET", URL: apiBaseURL + "/programs/" + url.PathEscape(id), Headers: []whttp.WHTTPHeader{{Name: "Authorization", Value: "Bearer " + p.token}}, diff --git a/pkg/platforms/yeswehack/fetch_test.go b/pkg/platforms/yeswehack/fetch_test.go index f9b37f2..dd6f32f 100644 --- a/pkg/platforms/yeswehack/fetch_test.go +++ b/pkg/platforms/yeswehack/fetch_test.go @@ -2,6 +2,7 @@ package yeswehack import ( "context" + "errors" "io" "net/http" "net/http/httptest" @@ -10,6 +11,7 @@ import ( "runtime" "strings" "testing" + "time" "github.com/cozyGarage/bbscope/v2/pkg/platforms" ) @@ -255,3 +257,53 @@ func TestFetchProgramScopeEscapesHandle(t *testing.T) { t.Fatalf("path = %q, want PathEscape of handle", gotPath) } } + +// A later page without pagination read as nb_pages=0 and silently ended the +// listing, returning a partial program list as success. +func TestListProgramHandles_LaterPageWithoutPagination(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Query().Get("page") { + case "1": + _, _ = io.WriteString(w, `{"items":[{"slug":"p1"}],"pagination":{"nb_pages":3}}`) + case "2": + _, _ = io.WriteString(w, `{"items":[{"slug":"p2"}]}`) + default: + _, _ = io.WriteString(w, `{"items":[{"slug":"p3"}]}`) + } + })) + defer srv.Close() + withBaseURL(t, srv.URL) + + got, err := NewPoller("tok").ListProgramHandles(context.Background(), platforms.PollOptions{}) + if err != nil { + t.Fatalf("ListProgramHandles: %v", err) + } + if want := []string{"p1", "p2", "p3"}; !equal(got, want) { + t.Fatalf("handles = %v, want %v", got, want) + } +} + +// Login retried a rejected TOTP with plain time.Sleep, so cancellation waited +// out every remaining attempt. +func TestLogin_HonorsCancelBetweenTOTPAttempts(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/login" { + _, _ = io.WriteString(w, `{"totp_token":"t"}`) + return + } + cancel() // shutdown arrives while TOTP is being rejected + w.WriteHeader(http.StatusBadRequest) + })) + defer srv.Close() + withBaseURL(t, srv.URL) + + start := time.Now() + _, err := login(ctx, "a@b.c", "pw", "JBSWY3DPEHPK3PXP", "") + if !errors.Is(err, context.Canceled) { + t.Fatalf("err = %v, want context.Canceled", err) + } + if elapsed := time.Since(start); elapsed > time.Second { + t.Fatalf("login ignored cancellation for %s", elapsed) + } +} diff --git a/pkg/platforms/yeswehack/poller.go b/pkg/platforms/yeswehack/poller.go index 22025d9..ade5300 100644 --- a/pkg/platforms/yeswehack/poller.go +++ b/pkg/platforms/yeswehack/poller.go @@ -35,7 +35,7 @@ func (p *Poller) Authenticate(ctx context.Context, cfg platforms.AuthConfig) err return nil } if cfg.Email != "" && cfg.Password != "" && cfg.OtpSecret != "" { - tok, err := login(cfg.Email, cfg.Password, cfg.OtpSecret, cfg.Proxy) + tok, err := login(ctx, cfg.Email, cfg.Password, cfg.OtpSecret, cfg.Proxy) if err != nil { return err } @@ -52,6 +52,7 @@ func (p *Poller) ListProgramHandles(ctx context.Context, opts platforms.PollOpti for page <= nb_pages { res, err := whttp.SendHTTPRequest(&whttp.WHTTPReq{ + Ctx: ctx, Method: "GET", URL: apiBaseURL + "/programs" + "?page=" + strconv.Itoa(page), Headers: []whttp.WHTTPHeader{{Name: "Authorization", Value: "Bearer " + p.token}}, @@ -66,8 +67,15 @@ func (p *Poller) ListProgramHandles(ctx context.Context, opts platforms.PollOpti if !gjson.Get(res.BodyString, "items").Exists() { return nil, fmt.Errorf("yeswehack: listing response missing items") } - if page == 1 && !gjson.Get(res.BodyString, "pagination.nb_pages").Exists() { - return nil, fmt.Errorf("yeswehack: listing response missing pagination.nb_pages") + // The page count comes from page 1 only: a later page without + // pagination would read as 0 and end the listing early, and a short + // listing lets sync disable the programs it missed. + if page == 1 { + pagesField := gjson.Get(res.BodyString, "pagination.nb_pages") + if !pagesField.Exists() { + return nil, fmt.Errorf("yeswehack: listing response missing pagination.nb_pages") + } + nb_pages = int(pagesField.Int()) } // Read each item as an object rather than zipping parallel `items.#.field` @@ -91,7 +99,6 @@ func (p *Poller) ListProgramHandles(ctx context.Context, opts platforms.PollOpti handles = append(handles, slug) } - nb_pages = int(gjson.Get(res.BodyString, "pagination.nb_pages").Int()) page++ } @@ -105,6 +112,7 @@ func (p *Poller) FetchProgramScope(ctx context.Context, handle string, opts plat pData := scope.ProgramData{Url: programWebURL} res, err := whttp.SendHTTPRequest(&whttp.WHTTPReq{ + Ctx: ctx, Method: "GET", URL: programAPIURL, Headers: []whttp.WHTTPHeader{{Name: "Authorization", Value: "Bearer " + p.token}}, @@ -174,7 +182,7 @@ func (p *Poller) FetchProgramScope(ctx context.Context, handle string, opts plat return pData, nil } -func login(email string, password, otpSecret, proxy string) (string, error) { +func login(ctx context.Context, email string, password, otpSecret, proxy string) (string, error) { if proxy != "" { if err := whttp.SetupProxy(proxy); err != nil { return "", fmt.Errorf("failed to setup proxy: %w", err) @@ -188,6 +196,7 @@ func login(email string, password, otpSecret, proxy string) (string, error) { } loginRes, err := whttp.SendHTTPRequest(&whttp.WHTTPReq{ + Ctx: ctx, Method: "POST", URL: loginURL, Headers: []whttp.WHTTPHeader{ @@ -231,6 +240,7 @@ func login(email string, password, otpSecret, proxy string) (string, error) { } totpRes, err := whttp.SendHTTPRequest(&whttp.WHTTPReq{ + Ctx: ctx, Method: "POST", URL: totpURL, Headers: []whttp.WHTTPHeader{ @@ -254,10 +264,14 @@ func login(email string, password, otpSecret, proxy string) (string, error) { return finalToken, nil } - time.Sleep(2 * time.Second) if attempts == OTP_ATTEMPTS { return "", fmt.Errorf("TOTP verification failed after %d attempts", OTP_ATTEMPTS) } + select { + case <-ctx.Done(): + return "", ctx.Err() + case <-time.After(2 * time.Second): + } } return "", fmt.Errorf("unexpected error in TOTP verification") diff --git a/pkg/storage/integration_test.go b/pkg/storage/integration_test.go index db8b8d4..9f2b0fe 100644 --- a/pkg/storage/integration_test.go +++ b/pkg/storage/integration_test.go @@ -314,3 +314,37 @@ func TestIntegrationDatabaseIsConfiguredInCI(t *testing.T) { "Check the postgres service and TEST_DB_URL in .github/workflows/ci.yml.") } } + +// Legacy rows under different platform spellings (h1 vs hackerone) can share +// one canonical URL. The rewrite's unique violation used to be ignored inside +// the migration transaction, which Postgres then refused to continue, so Open +// failed on every startup. +func TestIntegration_MergeDuplicateProgramsSurvivesURLConflict(t *testing.T) { + db := openTestDB(t) + ctx := context.Background() + platform := uniquePlatform(t) + url := "https://example.com/" + platform + t.Cleanup(func() { + _, _ = db.sql.ExecContext(ctx, `DELETE FROM programs WHERE platform IN ($1, $2)`, platform+"_a", platform+"_b") + }) + if _, err := db.sql.ExecContext(ctx, `INSERT INTO programs(platform, handle, url) VALUES ($1, 'x', $2), ($3, 'x', $4)`, + platform+"_a", url, platform+"_b", url+"/"); err != nil { + t.Fatalf("seeding: %v", err) + } + + tx, err := db.sql.BeginTx(ctx, nil) + if err != nil { + t.Fatal(err) + } + defer func() { _ = tx.Rollback() }() + if err := mergeDuplicatePrograms(tx); err != nil { + t.Fatalf("mergeDuplicatePrograms: %v", err) + } + var n int + if err := tx.QueryRowContext(ctx, `SELECT count(*) FROM programs WHERE platform IN ($1, $2)`, platform+"_a", platform+"_b").Scan(&n); err != nil { + t.Fatalf("transaction unusable after the skipped rewrite: %v", err) + } + if n != 2 { + t.Fatalf("programs = %d, want 2 (conflicting rewrite skipped, not merged)", n) + } +} diff --git a/pkg/storage/normalize.go b/pkg/storage/normalize.go index 1389b3c..74fcd8b 100644 --- a/pkg/storage/normalize.go +++ b/pkg/storage/normalize.go @@ -15,8 +15,13 @@ func NormalizeTarget(s string) string { // Wildcard hosts used to drop the path, so https://*.example.com/admin // and https://*.example.com/api collapsed to the same identity key and // one target was deleted on upsert. - if u, err := url.Parse(s); err == nil && u.Host != "" { + if u, ok := parseTargetURL(s); ok { u.Host = strings.ToLower(u.Host) + // Default the scheme before stripping default ports, or "//host:443" + // keeps its port now and loses it on the next pass. + if u.Scheme == "" { + u.Scheme = "https" + } if u.Scheme == "http" && u.Port() == "80" { u.Host = strings.TrimSuffix(u.Host, ":80") } @@ -24,18 +29,35 @@ func NormalizeTarget(s string) string { u.Host = strings.TrimSuffix(u.Host, ":443") } u.Path = strings.TrimRight(u.Path, "/") - if u.Scheme == "" { - u.Scheme = "https" + // Commit only if the result is itself still a URL; otherwise a second + // pass would take the domain branch and rewrite it (e.g. "//::#A"). + if out := u.String(); isTargetURL(out) { + return out } - return u.String() } // Wildcards/domains s = strings.ToLower(s) - s = strings.TrimSuffix(s, ".") - if strings.HasSuffix(s, "/") { - s = strings.TrimRight(s, "/") + // Trim slashes and the root dot together: trimming "." first left + // "example.com./" as "example.com.", which a second pass changed again. + trimmed := strings.TrimRight(s, "/.") + if len(trimmed) < len(s) { + // Trimming can expose a URL shape ("//host:1." -> "//host:1"), so + // renormalize; the input only shrinks, so this terminates. + return NormalizeTarget(trimmed) } - return s + return trimmed +} + +// parseTargetURL parses s as a URL with a real hostname. A port-only +// authority ("//:443") is not a URL target. +func parseTargetURL(s string) (*url.URL, bool) { + u, err := url.Parse(s) + return u, err == nil && u.Hostname() != "" +} + +func isTargetURL(s string) bool { + _, ok := parseTargetURL(s) + return ok } // NormalizeProgramURL ensures consistent program URL identity. diff --git a/pkg/storage/normalize_fuzz_test.go b/pkg/storage/normalize_fuzz_test.go index 827af0b..d67cd21 100644 --- a/pkg/storage/normalize_fuzz_test.go +++ b/pkg/storage/normalize_fuzz_test.go @@ -1,6 +1,9 @@ package storage -import "testing" +import ( + "testing" + "unicode/utf8" +) func FuzzNormalizeTarget(f *testing.F) { f.Add("*.example.com") @@ -8,8 +11,14 @@ func FuzzNormalizeTarget(f *testing.F) { f.Add("HTTPS://EXAMPLE.COM:443/a/") f.Add("") f.Add("not a url") + f.Add("example.com./") f.Fuzz(func(t *testing.T, input string) { - _ = NormalizeTarget(input) + // Stored identities are NormalizeTarget output; normalizing one again + // must not move it, or the same target gets two identity keys. Targets + // arrive via JSON, so invalid UTF-8 (which net/url re-escapes) can't occur. + if once := NormalizeTarget(input); utf8.ValidString(input) && NormalizeTarget(once) != once { + t.Fatalf("NormalizeTarget not idempotent: %q -> %q -> %q", input, once, NormalizeTarget(once)) + } _ = NormalizeProgramURL(input) _ = identityKey(input, "url") }) diff --git a/pkg/storage/normalize_test.go b/pkg/storage/normalize_test.go index 2ba7c60..8fd9244 100644 --- a/pkg/storage/normalize_test.go +++ b/pkg/storage/normalize_test.go @@ -12,6 +12,8 @@ func TestNormalizeTarget(t *testing.T) { {"https://*.example.com/api", "https://*.example.com/api"}, {"*.Example.COM.", "*.example.com"}, {"Example.COM/", "example.com"}, + {"example.com./", "example.com"}, + {"//example.com:443/a", "https://example.com/a"}, {"", ""}, } for _, tc := range tests { diff --git a/pkg/storage/queries.go b/pkg/storage/queries.go index 74459b7..6d7eb5f 100644 --- a/pkg/storage/queries.go +++ b/pkg/storage/queries.go @@ -238,6 +238,8 @@ func (d *DB) ListEntries(ctx context.Context, opts ListOptions) ([]Entry, error) if aiIDNS.Valid { entry.Source = "ai" + base := baseInScope == 1 + entry.BaseInScope = &base if aiTargetNS.Valid && aiTargetNS.String != "" { entry.TargetNormalized = aiTargetNS.String } else { diff --git a/pkg/storage/schema.go b/pkg/storage/schema.go index b0849a7..2ed86a6 100644 --- a/pkg/storage/schema.go +++ b/pkg/storage/schema.go @@ -2,8 +2,11 @@ package storage import ( "database/sql" + "errors" "fmt" + "github.com/jackc/pgx/v5/pgconn" + "github.com/cozyGarage/bbscope/v2/pkg/scope" ) @@ -152,6 +155,27 @@ func applyMigrations(db *sql.DB) error { return nil } +// execSkippingConflict runs an optional rewrite inside a savepoint. In Postgres +// any failed statement aborts the whole transaction, so ignoring the error +// directly made every later migration statement fail and Open fail forever. +// Unique violations are skipped; any other error is returned. +func execSkippingConflict(tx *sql.Tx, query string, args ...any) error { + if _, err := tx.Exec(`SAVEPOINT optional_rewrite`); err != nil { + return err + } + if _, err := tx.Exec(query, args...); err != nil { + var pgErr *pgconn.PgError + if !errors.As(err, &pgErr) || pgErr.Code != "23505" { // unique_violation + return err + } + if _, err := tx.Exec(`ROLLBACK TO SAVEPOINT optional_rewrite`); err != nil { + return err + } + } + _, err := tx.Exec(`RELEASE SAVEPOINT optional_rewrite`) + return err +} + // canonicalizeProgramURLsAndTargets merges legacy duplicate programs/targets that // only differ by URL/target canonicalization (trailing slash, host case, etc.). func canonicalizeProgramURLsAndTargets(tx *sql.Tx) error { @@ -194,9 +218,9 @@ func mergeDuplicatePrograms(tx *sql.Tx) error { // Still rewrite single rows to canonical URL when needed. canonical := NormalizeProgramURL(group[0].url) if canonical != "" && canonical != group[0].url { - if _, err := tx.Exec(`UPDATE programs SET url = $1 WHERE id = $2`, canonical, group[0].id); err != nil { - // Unique conflict means another row already owns it; ignore rewrite. - _ = err + // A unique conflict means another row already owns the URL; skip the rewrite. + if err := execSkippingConflict(tx, `UPDATE programs SET url = $1 WHERE id = $2`, canonical, group[0].id); err != nil { + return err } } continue @@ -359,10 +383,10 @@ func collapseDuplicateTargets(tx *sql.Tx, rewriteKeeper bool) error { // still finishes. v3 must not rewrite target; the backfill sets category // and target_identity without touching the stored spelling. if rewriteKeeper && (canonicalTarget != keeper.target || canonicalCat != keeper.cat) { - if _, err := tx.Exec(` + if err := execSkippingConflict(tx, ` UPDATE targets_raw SET target = $1, category = $2 WHERE id = $3 `, canonicalTarget, canonicalCat, keeper.id); err != nil { - _ = err + return err } } } diff --git a/pkg/storage/types.go b/pkg/storage/types.go index eec5218..fb8b88b 100644 --- a/pkg/storage/types.go +++ b/pkg/storage/types.go @@ -24,6 +24,9 @@ type Entry struct { Category string `json:"category"` BaseCategory string `json:"base_category,omitempty"` + // BaseInScope is the raw target's own in_scope on "ai" rows, whose InScope + // is the variant's effective value. Nil for raw rows and older exports. + BaseInScope *bool `json:"base_in_scope,omitempty"` Description string `json:"description,omitempty"` InScope bool `json:"in_scope"` IsBBP bool `json:"is_bbp"` diff --git a/pkg/whttp/whttp.go b/pkg/whttp/whttp.go index 9070572..ca50f98 100644 --- a/pkg/whttp/whttp.go +++ b/pkg/whttp/whttp.go @@ -1,6 +1,7 @@ package whttp import ( + "context" "crypto/tls" "fmt" "io" @@ -29,6 +30,8 @@ type WHTTPReq struct { CustomHost string Headers []WHTTPHeader Debug bool + // Ctx cancels the request and its retry backoff; nil means Background. + Ctx context.Context } type WHTTPRes struct { @@ -125,11 +128,15 @@ func SendHTTPRequest(wReq *WHTTPReq, customClient *retryablehttp.Client) (wRes * client = GetDefaultClient() } + ctx := wReq.Ctx + if ctx == nil { + ctx = context.Background() + } var req *retryablehttp.Request if wReq.Body != "" { - req, err = retryablehttp.NewRequest(wReq.Method, wReq.URL, strings.NewReader(wReq.Body)) + req, err = retryablehttp.NewRequestWithContext(ctx, wReq.Method, wReq.URL, strings.NewReader(wReq.Body)) } else { - req, err = retryablehttp.NewRequest(wReq.Method, wReq.URL, nil) + req, err = retryablehttp.NewRequestWithContext(ctx, wReq.Method, wReq.URL, nil) } if err != nil { diff --git a/pkg/whttp/whttp_test.go b/pkg/whttp/whttp_test.go index 20ad72d..6585fce 100644 --- a/pkg/whttp/whttp_test.go +++ b/pkg/whttp/whttp_test.go @@ -1,6 +1,8 @@ package whttp import ( + "context" + "errors" "io" "log" "net/http" @@ -262,3 +264,25 @@ func TestWHTTPReq_DefaultMethod(t *testing.T) { t.Fatalf("SendHTTPRequest() error = %v", err) } } + +// Without a request context, Ctrl-C or daemon shutdown could not interrupt a +// request stuck in retry backoff. +func TestSendHTTPRequest_HonorsContext(t *testing.T) { + block := make(chan struct{}) + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + <-block + })) + defer srv.Close() + defer close(block) + + ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond) + defer cancel() + start := time.Now() + _, err := SendHTTPRequest(&WHTTPReq{Ctx: ctx, Method: "GET", URL: srv.URL}, nil) + if !errors.Is(err, context.DeadlineExceeded) { + t.Fatalf("err = %v, want context.DeadlineExceeded", err) + } + if elapsed := time.Since(start); elapsed > 5*time.Second { + t.Fatalf("request ignored cancellation for %s", elapsed) + } +}