Skip to content

fix: handle urlscan pagination and source errors safely - #136

Open
MyCode83 wants to merge 6 commits into
projectdiscovery:devfrom
MyCode83:fix/urlscan-incomplete-sort-panic
Open

fix: handle urlscan pagination and source errors safely#136
MyCode83 wants to merge 6 commits into
projectdiscovery:devfrom
MyCode83:fix/urlscan-incomplete-sort-panic

Conversation

@MyCode83

@MyCode83 MyCode83 commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Description

This PR prevents urlscan from panicking when the API returns incomplete or malformed pagination sort values.

Previously, urlscan accessed the second sort value after only checking that the first one existed. It also used an unsafe type assertion for the first value, which could cause another panic if the API returned an unexpected type.

Pagination sort values are now validated before use and are only extracted when another page is available.

The PR also adds the missing source.Error type to error results in the AlienVault, Common Crawl, and urlscan sources. Without this type, the runner interpreted those errors as URL results and silently discarded them.

Changes

  • Reproduce the panic caused by an incomplete urlscan sort array.
  • Validate the length and types of urlscan search-after values.
  • Avoid extracting a pagination cursor when has_more is false.
  • Return an error instead of panicking on malformed pagination data.
  • Add unit tests for valid and invalid search-after values.
  • Add missing source.Error types to AlienVault, Common Crawl, and urlscan error results.

Testing

go test ./...

<!-- This is an auto-generated comment: release notes by coderabbit.ai -->
## Summary by CodeRabbit

* **Bug Fixes**
  * Improved error reporting across AlienVault, Common Crawl, and urlscan data sources.
  * Fixed urlscan pagination to stop correctly when no additional results are available.
  * Added validation for malformed pagination data and incomplete responses.
  * Prevented invalid sorting information from causing unexpected failures.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

@coderabbitai

coderabbitai Bot commented Aug 1, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 8b20c2a9-0762-4d15-8de8-128b5a5d6264

📥 Commits

Reviewing files that changed from the base of the PR and between 094764b and 89175b0.

📒 Files selected for processing (2)
  • pkg/source/urlscan/urlscan.go
  • pkg/source/urlscan/urlscan_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
  • pkg/source/urlscan/urlscan_test.go

Walkthrough

AlienVault and Common Crawl now classify failure results as source.Error. urlscan adds validated pagination cursor construction, explicit error typing, stricter pagination termination, and table-driven tests.

Changes

Source handling

Layer / File(s) Summary
Explicit source error results
pkg/source/alienvault/alienvault.go, pkg/source/commoncrawl/commoncrawl.go, pkg/source/urlscan/urlscan.go
Failure paths now emit results with Type: source.Error.
Validated urlscan pagination
pkg/source/urlscan/urlscan.go
Pagination checks has_more, requires results, and validates sort data before creating the next cursor.
Pagination cursor tests
pkg/source/urlscan/urlscan_test.go
Tests cover valid cursors and missing, invalid, empty, fractional, and non-finite sort values.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Run
  participant urlscan_API
  participant buildSearchAfter
  Run->>urlscan_API: Request search page
  urlscan_API-->>Run: Return results, has_more, and sort
  Run->>buildSearchAfter: Validate sort data
  buildSearchAfter-->>Run: Return next-page token
  Run->>urlscan_API: Request next page with token
Loading

Possibly related PRs

Poem

I’m a rabbit reviewing cursors in the sun,
Errors wear clear badges when each task is done.
has_more now guides the hop,
Bad sort values make us stop.
Tests guard every leap we run.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main changes to urlscan pagination and source error handling.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@pkg/source/urlscan/urlscan.go`:
- Around line 70-84: The buildSearchAfter function must reject non-finite and
fractional first sort values before formatting, while preserving valid integral
values. Validate the float64 with an integral-safe check, format it without
converting through int, and update the related tests in
pkg/source/urlscan/urlscan_test.go lines 275-309 to cover fractional and
non-finite rejection; the implementation change belongs in
pkg/source/urlscan/urlscan.go lines 70-84.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 70e07d85-b2f1-4f3b-b460-47e7bcf4a28a

📥 Commits

Reviewing files that changed from the base of the PR and between 3548dc2 and 094764b.

📒 Files selected for processing (4)
  • pkg/source/alienvault/alienvault.go
  • pkg/source/commoncrawl/commoncrawl.go
  • pkg/source/urlscan/urlscan.go
  • pkg/source/urlscan/urlscan_test.go

Comment thread pkg/source/urlscan/urlscan.go Outdated
@MyCode83

MyCode83 commented Aug 1, 2026

Copy link
Copy Markdown
Contributor Author

Fixed by rejecting fractional and non-finite cursor values and formatting the validated number without converting it to int.

@MyCode83

MyCode83 commented Aug 1, 2026

Copy link
Copy Markdown
Contributor Author

The lint check is failing before analyzing this PR because the golangci-lint binary provided by projectdiscovery/actions/golangci-lint@v1 was built with Go 1.24, while urlfinder targets Go 1.25.0.

All build jobs pass on Linux, macOS, and Windows, and CodeQL reports no new alerts. This appears to be an issue with the shared lint action rather than the changes in this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant