Skip to content

fix(headless): respect system-chrome option in headless mode - #1882

Open
AdamMagued wants to merge 3 commits into
projectdiscovery:devfrom
AdamMagued:fix-issue-1878
Open

AdamMagued wants to merge 3 commits into
projectdiscovery:devfrom
AdamMagued:fix-issue-1878

Conversation

@AdamMagued

@AdamMagued AdamMagued commented Oct 10, 2026 •

Copy link
Copy Markdown

Fixes #1878

Summary

  • Honored system-chrome configuration when launching headless browser instances
  • Added unit tests verifying chrome binary resolution flags

Verified in container sandbox.

Summary by CodeRabbit

  • New Features
    • Headless and hybrid modes can now launch an installed Chrome browser. Chrome is detected automatically, with an option to specify its path.
  • Bug Fixes
    • Selecting installed Chrome without headless or hybrid mode now returns a clear mode-required error.

@neo-by-projectdiscovery-dev

neo-by-projectdiscovery-dev Bot commented Oct 10, 2026 •

Copy link
Copy Markdown

Neo - PR Security Review

No exploitable security vulnerabilities in the delta. The change correctly propagates the UseInstalledChrome flag through the option chain, gates it behind the existing headless/hybrid-mode validation, and resolves the Chrome binary via go-rod's LookPath against well-known OS paths rather than attacker-influenced input.

Hardening Notes
  • lookPath is an unexported package-level var wrapping launcher.LookPath — a go-rod helper that scans hardcoded OS Chrome install locations, not the ambient PATH. The testability hook (var lookPath = launcher.LookPath) is unexported and cannot be swapped from outside the package.
  • ChromiumPath/SystemChromePath is validated with fileutil.FileExists in options.go before reaching the launcher, unchanged by this PR.
  • UserArguments and headlessFlags processing was moved into createChromeLauncher unchanged from the prior implementation — no new injection surface.
What Neo reviewed

pkg/engine/headless/browser/browser.go, internal/runner/options.go, internal/runner/options_test.go, pkg/engine/headless/crawler/crawler.go, pkg/engine/headless/crawler/crawler_test.go, pkg/engine/headless/headless.go

Comment @pdneo help for available commands. · Open in Neo

@coderabbitai

coderabbitai Bot commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: f6c5bc35-f168-4fdd-b803-71c0d9281354

📥 Commits

Reviewing files that changed from the base of the PR and between 3e65064 and 8844dde.


📒 Files selected for processing (7)
  • internal/runner/options.go
  • internal/runner/options_test.go
  • pkg/engine/headless/browser/browser.go
  • pkg/engine/headless/browser/browser_test.go
  • pkg/engine/headless/crawler/crawler.go
  • pkg/engine/headless/crawler/crawler_test.go
  • pkg/engine/headless/headless.go

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.



Walkthrough

The installed-Chrome option now requires headless or hybrid mode. The headless engine passes the option to the browser launcher, which can resolve installed Chrome when no explicit Chromium path is set.

Changes

Headless installed Chrome

Layer / File(s) Summary
Option validation and propagation
internal/runner/options.go, internal/runner/options_test.go, pkg/engine/headless/headless.go, pkg/engine/headless/crawler/crawler.go, pkg/engine/headless/crawler/crawler_test.go
Runner validation requires headless or hybrid mode when installed Chrome is enabled. The headless engine passes the option through crawler options to browser launcher options. Tests cover validation and crawler option propagation.
Browser executable selection
pkg/engine/headless/browser/browser.go, pkg/engine/headless/browser/browser_test.go
The launcher uses an explicit Chromium path or resolves installed Chrome when requested. Without the option, existing explicit-path and supported headless-shell selection remains. Tests cover executable resolution, missing Chrome, and explicit-path selection.

Priority: ➖ Normal

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

Change: Bug fix · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant Headless.Crawl
  participant crawler.New
  participant createChromeLauncher
  participant launcher.LookPath
  Headless.Crawl->>crawler.New: Pass UseInstalledChrome
  crawler.New->>createChromeLauncher: Pass launcher options
  createChromeLauncher->>launcher.LookPath: Resolve Chrome when no path is set
  launcher.LookPath-->>createChromeLauncher: Return executable path or lookup error
Loading

Suggested reviewers: mzack9999

Merge Risk

Merge Risk: ⚪ Minimal · up to 8844d

The documented headless command passes the installed-Chrome setting through to browser launch. No issue remains that should prevent merging after normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 8844d

The change is opt-in and preserves existing profile handling and launch controls. No introduced security defect was established, but executable discovery now depends on the local browser installation and environment. Deployment-specific sandbox behavior remains unverified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The changed execution path affects opted-in local headless browser launches. Trust in the selected executable depends on an explicit operator path or the host installation and PATH. Controlling discovery requires influence over that local environment; no remote page-content input to executable selection was established.

Trust Boundaries and Controls

  • observed — Absent operator profile or flag overrides, local installed-browser launches receive a generated temporary profile and use the existing default incognito setting. Selecting installed Chrome does not itself select the operator's normal browser profile. Explicitly configured profiles remain a preexisting session-sharing choice.

Resilience and Maintainability Implications

  • inferred — Existing initialization failure paths still leave cleanup dependent on library or process behavior: a connection failure does not explicitly close the launched browser, and initial page-creation failure returns before deferred cleanup is installed. These caller-level gaps were present at the base and were not established as introduced or worsened by this PR.

Hardening Proposals

  • proposed — For privileged unattended deployments, configure a maintained browser through an explicit absolute executable path and keep the discovery environment trusted. Verify effective sandbox enforcement for the deployed browser and container rather than relying only on the application flag.



Pre-merge checks | Passed 4 | Failed 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 7 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 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 change: honoring the system-Chrome option in headless mode.
Linked Issues check Passed Issue #1878 requires headless mode to honor -system-chrome and use installed Chrome or Chromium instead of chrome-headless-shell. The change propagates UseInstalledChrome from Headless.Crawl t…
Out of Scope Changes check Passed The changes stay within issue #1878. The runner validation, option propagation, launcher refactor, and unit tests directly support correct -headless -system-chrome behavior. No unrelated engine beha…


  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR


  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

A rabbit checks the Chrome path,
Then hops through headless mode.
A found browser joins the crawl,
No shell path takes its place,
The rabbit leaves fresh tracks.

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

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.

-system-chrome is ignored with -headless, so headless crawls in the Docker image find nothing

2 participants