fix(lib): segfault when init engine with EnableHeadlessWithOpts

[dwisiswant0]

Proposed changes

fix(lib): segfault when init engine with `EnableHeadlessWithOpts`

The panic was caused by attempting to log a
sandbox warning before the logger was initialized.

RCA:
* SDK option funcs were exec'd before logger init.
* `EnableHeadlessWithOpts()` attempted to create
  browser instance & log warnings during the
  config phase.
* `Logger` was only init'd later in `init()`
  phase.
* This caused nil pointer dereference when
  `MustDisableSandbox()` returned true (root on
  Linux/Unix or Windows).

Changes:
* Init `Logger` in `types.DefaultOptions()` to
  ensure it's always available before any option
  functions execute.
* Init `Logger` field in both
  `NewNucleiEngineCtx()` and
  `NewThreadSafeNucleiEngineCtx()` from
  `defaultOptions.Logger`.
* Move browser instance creation from
  `EnableHeadlessWithOpts()` to the `init()` phase
  where `Logger` is guaranteed to be available.
* Simplify logger sync logic in `init()` to only
  update if changed by `WithLogger` option.
* Add test case to verify headless initialization
  works without panic.

The fix maintains backward compatibility while
make sure the logger is always available when
needed by any SDK option function.

Fixes #6601.

Proof

$ go test -v -run TestHeadlessOptionInitialization ./lib/
=== RUN   TestHeadlessOptionInitialization
--- PASS: TestHeadlessOptionInitialization (8.38s)
PASS
ok      github.com/projectdiscovery/nuclei/v3/lib       9.001s

Checklist

• Pull request is created against the dev branch
• All checks passed (lint, unit/integration/regression tests etc.) with my changes
• I have added tests that prove my fix is effective or that my feature works
• I have added necessary documentation (if appropriate)

Summary by CodeRabbit

• Tests

  • Enforced 30-minute test timeout and single-run execution.
  • Extended test waits for navigation and added a headless-option initialization test to surface setup failures.

• New Features

  • Engine now initializes a headless browser when the headless option is enabled.

• Chores

  • Default options now ensure a logger is present and the engine uses the logger from defaults.

[coderabbitai]

Walkthrough

Adds runtime headless browser initialization during engine init, ensures default Logger is non-nil and wired earlier, updates constructors to set Logger/ctx, increases a test timeout, tightens Makefile test flags, and adds a headless-init unit test.

Changes

Cohort / File(s)	Summary
Build config Makefile	Test target GOFLAGS extended with -timeout 30m -count 1.
Headless init (engine init) lib/sdk_private.go, lib/config.go	lib/sdk_private.go: initialize headless browser when opts.Headless (call engine.MustDisableSandbox(), engine.New(e.opts), store e.browserInstance); adjust logger wiring logic during init. lib/config.go: removed runtime browser creation/sandbox logging from EnableHeadlessWithOpts (now only sets options).
Constructors / context / logger wiring lib/multi.go, lib/sdk.go	Constructors now set ctx and initialize Logger from default options (assign Logger: defaultOptions.Logger where applicable).
Default options pkg/types/types.go	DefaultOptions now includes Logger: &gologger.Logger{} to ensure a non-nil default logger.
Tests pkg/protocols/headless/engine/page_actions_test.go, lib/sdk_test.go	page_actions_test.go: increased timeout for testHeadlessSimpleResponse from 20*time.Second to 60*time.Second. lib/sdk_test.go: added TestHeadlessOptionInitialization to exercise headless init and logger wiring.

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant NewEngine as NewNucleiEngineCtx
  participant Engine
  participant Logger
  participant HeadlessEngine as headless/engine

  Caller->>NewEngine: NewNucleiEngineCtx(ctx, options...)
  NewEngine->>Engine: apply options (may set Headless)
  Engine->>Logger: ensure e.opts.Logger != nil (DefaultOptions provides Logger)
  Engine->>Engine: init(ctx)
  alt e.opts.Headless == true
    Engine->>HeadlessEngine: MustDisableSandbox()
    HeadlessEngine-->>Engine: result (warning/info)
    Engine->>HeadlessEngine: New(e.opts)
    HeadlessEngine-->>Engine: browser instance / error
    Engine->>Engine: store e.browserInstance
  else
    Engine->>Engine: skip browser initialization
  end
  Engine-->>NewEngine: init result
  NewEngine-->>Caller: return engine or error

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Inspect lifecycle and cleanup for the new browserInstance field (creation, error paths, Close).
• Verify logger initialization order to prevent nil dereferences when option functions run.
• Review sandbox-disable path and platform/root implications in MustDisableSandbox().
• Confirm new and modified tests correctly exercise error propagation and timing.

Poem

"I hopped through bytes with careful paws,
Wired the logger to soothe the cause.
A headless browser now wakes in line,
Tests wait longer — no panic this time. 🐰✨"

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 57.14% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title accurately describes the main fix: addressing a segmentation fault when initializing the engine with EnableHeadlessWithOpts, which is the core issue resolved.
Linked Issues check	✅ Passed	All requirements from #6601 are met: logger initialized in DefaultOptions, engine Logger field set before options execute, browser creation moved to init() phase, and TestHeadlessOptionInitialization test added to verify no panic.
Out of Scope Changes check	✅ Passed	All changes are directly related to fixing the segfault: logger initialization order, engine field setup, and test addition. The timeout increase in TestActionNavigate is appropriately scoped to address CI flakiness.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch dwisiswant0/fix/lib/segfault-when-init-engine-with-EnableHeadlessWithOpts

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between aa0bfba and 9e38102.

📒 Files selected for processing (1)

• pkg/protocols/headless/engine/page_actions_test.go (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

• pkg/protocols/headless/engine/page_actions_test.go

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)

• GitHub Check: Tests (ubuntu-latest)
• GitHub Check: Tests (macOS-latest)
• GitHub Check: Tests (windows-latest)

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

lib/sdk_private.go (1)

101-111: Guard against dropping back to a nil logger.

If a caller passes WithOptions where opts.Logger is nil (very common for manually constructed options), this block overwrites e.Logger with nil. The next e.Logger.SetMaxLevel(...) then panics, which regresses existing setups that relied on the engine providing a default logger.

Please retain the existing default when opts.Logger is nil, e.g.:

-   if e.opts.Logger != e.Logger {
-       e.Logger = e.opts.Logger
-   }
+   if e.opts.Logger != nil {
+       if e.opts.Logger != e.Logger {
+           e.Logger = e.opts.Logger
+       }
+   } else {
+       if e.Logger == nil {
+           e.Logger = gologger.DefaultLogger
+       }
+       e.opts.Logger = e.Logger
+   }

Remember to import github.com/projectdiscovery/gologger if it is no longer in scope.

🧹 Nitpick comments (1)

lib/sdk_test.go (1)

39-57: LGTM! Test effectively verifies the segfault fix.

The test successfully reproduces the scenario that previously caused a nil pointer panic (initializing the engine with EnableHeadlessWithOpts) and verifies that the logger is now properly initialized before option functions execute. The assertions are appropriate and the cleanup is handled correctly.

Optional enhancement: Consider adding a brief comment explaining the regression this test prevents, for example:

+// TestHeadlessOptionInitialization verifies that initializing the engine with
+// EnableHeadlessWithOpts does not panic due to nil logger (regression test for #6601).
 func TestHeadlessOptionInitialization(t *testing.T) {

You might also consider adding a test case for EnableHeadlessWithOpts(nil) to ensure the nil HeadlessOpts path is also covered:

func TestHeadlessOptionInitializationWithNilOpts(t *testing.T) {
	ne, err := nuclei.NewNucleiEngineCtx(
		context.Background(),
		nuclei.EnableHeadlessWithOpts(nil),
	)
	require.NoError(t, err)
	require.NotNil(t, ne)
	require.NotNil(t, ne.Logger)
	defer ne.Close()
}

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cc0f1e9 and a73e496.

📒 Files selected for processing (6)

• lib/config.go (1 hunks)
• lib/multi.go (1 hunks)
• lib/sdk.go (1 hunks)
• lib/sdk_private.go (3 hunks)
• lib/sdk_test.go (1 hunks)
• pkg/types/types.go (1 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

**/*.go

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.go: Format Go code using go fmt
Run static analysis with go vet

Files:

• pkg/types/types.go
• lib/sdk_test.go
• lib/config.go
• lib/sdk_private.go
• lib/sdk.go
• lib/multi.go

🧠 Learnings (1)

📚 Learning: 2025-09-05T14:23:55.257Z

Learnt from: CR
Repo: projectdiscovery/nuclei PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-05T14:23:55.257Z
Learning: Applies to pkg/protocols/**/*.go : Each protocol implementation must provide a Request interface with methods Compile(), ExecuteWithResults(), Match(), and Extract()

Applied to files:

• lib/config.go
• lib/sdk_private.go

🧬 Code graph analysis (3)

lib/sdk_test.go (2)

lib/sdk.go (1)

• NewNucleiEngineCtx (312-332)

lib/config.go (2)

• EnableHeadlessWithOpts (201-212)
• HeadlessOpts (189-194)

lib/sdk_private.go (3)

pkg/protocols/headless/engine/engine.go (2)

• MustDisableSandbox (132-136)
• New (37-129)

internal/runner/runner.go (1)

• New (109-394)

pkg/core/engine.go (1)

• New (27-34)

lib/multi.go (1)

lib/sdk.go (1)

• NucleiEngine (58-95)

🔇 Additional comments (1)

lib/multi.go (1)

91-95: Nice job preserving the logger on the thread-safe path.

Setting Logger: defaultOptions.Logger keeps SDK options that log during registration from dereferencing nil. Nice catch.

[dwisiswant0]

OK, this is weird. The (flaky?) TestActionNavigate test keeps failing on Linux only, but it "works on my machine" with linux/amd64. What’s up with that, @coderabbitai? :3

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find and examine the TestActionNavigate test
echo "=== Finding TestActionNavigate test ==="
fd -e go -x grep -l "TestActionNavigate" {}

echo -e "\n=== Reading the test implementation ==="
rg -A 50 "func TestActionNavigate" --type go

echo -e "\n=== Checking for any browser/headless configuration in the test ==="
rg -B 5 -A 5 "TestActionNavigate" pkg/protocols/headless/engine/page_actions_test.go

Length of output: 5201

---

🏁 Script executed:

#!/bin/bash
# Check recent CI failures and workflow runs for this PR
echo "=== Checking PR checks and CI status ==="
gh pr checks 6602 --repo projectdiscovery/nuclei

echo -e "\n=== Looking at recent workflow runs for failures ==="
gh run list --repo projectdiscovery/nuclei --branch dwisiswant0/fix/lib/segfault-when-init-engine-with-EnableHeadlessWithOpts --limit 5

echo -e "\n=== Getting details of the most recent failed run if any ==="
FAILED_RUN=$(gh run list --repo projectdiscovery/nuclei --branch dwisiswant0/fix/lib/segfault-when-init-engine-with-EnableHeadlessWithOpts --limit 1 --json databaseId,conclusion --jq '.[] | select(.conclusion=="failure") | .databaseId')
if [ -n "$FAILED_RUN" ]; then
  echo "Found failed run: $FAILED_RUN"
  gh run view $FAILED_RUN --repo projectdiscovery/nuclei --log-failed
fi

Length of output: 1878

---

🏁 Script executed:

#!/bin/bash
# Look for platform-specific code or known issues with headless on Linux
echo "=== Checking for sandbox/Linux-specific code in headless ==="
rg -n "linux|sandbox|MustDisableSandbox" pkg/protocols/headless/engine/ --type go -C 3

echo -e "\n=== Checking for any timing or wait-related issues in page actions ==="
rg -n "timeout|wait|sleep|time\." pkg/protocols/headless/engine/page_actions_test.go --type go

Length of output: 5954

---

🏁 Script executed:

#!/bin/bash
# Get the actual failure logs from the Ubuntu test run
echo "=== Getting detailed logs from the failed Ubuntu test run ==="
gh run view 19298463110 --repo projectdiscovery/nuclei --job 55186444031 --log-failed | head -200

echo -e "\n=== Looking for TestActionNavigate specifically in the logs ==="
gh run view 19298463110 --repo projectdiscovery/nuclei --job 55186444031 --log | grep -A 20 -B 5 "TestActionNavigate"

Length of output: 288

---

🏁 Script executed:

#!/bin/bash
# Examine the test helper functions and initialization
echo "=== Reading testHeadlessSimpleResponse and testHeadless functions ==="
rg -A 60 "^func testHeadless\(" pkg/protocols/headless/engine/page_actions_test.go --type go

echo -e "\n=== Looking at protocolstate.Init call in the test file ==="
rg -B 10 -A 10 "protocolstate.Init" pkg/protocols/headless/engine/page_actions_test.go --type go

Length of output: 3643

---

🏁 Script executed:

#!/bin/bash
# Check if there are any differences in how tests initialize the engine vs the SDK
echo "=== Looking at New() function in engine ==="
rg -A 40 "^func New\(" pkg/protocols/headless/engine/engine.go --type go

echo -e "\n=== Check for any test-specific configuration or setup ==="
rg -B 5 "TestMain|func init\(" pkg/protocols/headless/engine/page_actions_test.go --type go

Length of output: 1404

---

🏁 Script executed:

#!/bin/bash
# Check the previous run that was cancelled to see if there were failures
echo "=== Checking the cancelled run 19297631197 ==="
gh run view 19297631197 --repo projectdiscovery/nuclei --log | grep -i "TestActionNavigate\|FAIL\|panic\|timeout" | head -50

echo -e "\n=== Looking at even earlier runs on this branch ==="
gh run list --repo projectdiscovery/nuclei --branch dwisiswant0/fix/lib/segfault-when-init-engine-with-EnableHeadlessWithOpts --limit 10 --json conclusion,databaseId,createdAt,displayTitle

Length of output: 13715

---

🏁 Script executed:

#!/bin/bash
# Check if there's been a pattern of TestActionNavigate failures in the main branches
echo "=== Searching for recent TestActionNavigate failures in the repository ==="
gh issue list --repo projectdiscovery/nuclei --search "TestActionNavigate" --state all --limit 10

echo -e "\n=== Check if there are any known flaky test issues ==="
gh issue list --repo projectdiscovery/nuclei --search "flaky test headless linux" --state all --limit 10

Length of output: 253

---

🏁 Script executed:

#!/bin/bash
# Look at the Makefile test configuration changes
echo "=== Check the Makefile changes in this PR ==="
cat Makefile | grep -A 5 -B 5 "test:"

echo -e "\n=== Look for any test configuration or environment setup ==="
rg -n "timeout|30m" Makefile

Length of output: 920

---

🏁 Script executed:

#!/bin/bash
# Check if there are race conditions or parallel test issues
echo "=== Looking for parallel test execution patterns ==="
rg -n "t.Parallel\(\)" pkg/protocols/headless/engine/page_actions_test.go --type go

echo -e "\n=== Check testHeadlessSimpleResponse implementation ==="
rg -B 5 -A 10 "^func testHeadlessSimpleResponse" pkg/protocols/headless/engine/page_actions_test.go --type go

Length of output: 751

---

🏁 Script executed:

#!/bin/bash
# Check the failed run from earlier today for actual error messages
echo "=== Checking failed run 19296662492 ==="
gh run view 19296662492 --repo projectdiscovery/nuclei --log | grep -B 10 -A 30 "TestActionNavigate" | head -100

echo -e "\n=== Look for any FAIL messages in that run ==="
gh run view 19296662492 --repo projectdiscovery/nuclei --log | grep -B 5 "FAIL.*headless" | head -50

Length of output: 12009

---

🏁 Script executed:

#!/bin/bash
# Check even earlier failed run for patterns
echo "=== Checking failed run 19295770861 ==="
gh run view 19295770861 --repo projectdiscovery/nuclei --log | grep -B 10 -A 30 "TestActionNavigate" | head -100

Length of output: 11311

---

🏁 Script executed:

#!/bin/bash
# Look at the historical issue about headless tests failing on Ubuntu
echo "=== Getting details of issue #2280 about headless tests on Ubuntu ==="
gh issue view 2280 --repo projectdiscovery/nuclei --comments | head -100

Length of output: 868

---

🏁 Script executed:

#!/bin/bash
# Check if there are environment differences or Chrome/Chromium issues
echo "=== Looking for Chrome/browser installation logic ==="
rg -n "LookPath|chrome|chromium" pkg/protocols/headless/engine/engine.go --type go -C 5

echo -e "\n=== Check for any CI-specific or Docker-specific handling ==="
rg -n "CI|DOCKER|container" pkg/protocols/headless/engine/ --type go

Length of output: 2619

---

🏁 Script executed:

#!/bin/bash
# Look at the CI-specific code in page_actions.go
echo "=== Checking CI-specific logic in page_actions.go ==="
rg -B 10 -A 10 "IsCI\(\)" pkg/protocols/headless/engine/page_actions.go --type go

echo -e "\n=== Look at the exact line 48 and surrounding context in page_actions_test.go ==="
sed -n '40,55p' pkg/protocols/headless/engine/page_actions_test.go

Length of output: 1402

---

🏁 Script executed:

#!/bin/bash
# Check if there are any differences in how the test httptest server is closed
echo "=== Looking at how the test server lifecycle is managed ==="
rg -B 5 -A 15 "httptest.NewServer" pkg/protocols/headless/engine/page_actions_test.go --type go

echo -e "\n=== Check instance.Run implementation signature ==="
rg -n "func.*Run\(" pkg/protocols/headless/engine/ --type go | grep -v test

Length of output: 1376

---

🏁 Script executed:

#!/bin/bash
# Look for any race conditions or timing issues in the Page implementation
echo "=== Check MustInfo implementation or usage patterns ==="
rg -n "MustInfo|Page\(\)" pkg/protocols/headless/engine/page.go --type go -C 3

echo -e "\n=== Check if there's any cleanup or Close logic that might race ==="
rg -n "Close|cleanup" pkg/protocols/headless/engine/page.go --type go -C 2

Length of output: 1266

---

@dwisiswant0, I've dug into this and found the root cause! The failure isn't directly related to your PR's changes. Here's what's happening:

The Problem

The TestActionNavigate test is failing with a network error: &net.OpError{Op:"write", Net:"tcp", ..., Err:poll.errNetClosing{}} — essentially a "use of closed network connection" error.

Why it's flaky on Linux only:

1. Timing at the edge: The test has a 20-second timeout, but on Ubuntu CI it's running for 20.69s and 14.94s (right at the boundary), while on Windows it completes in ~9s and macOS passes consistently.

2. The race:

   • The test navigates to an httptest server with actions: ActionNavigate + ActionWaitLoad
   • On Linux CI, this takes close to (or slightly over) 20 seconds due to slower browser/Chrome performance
   • When the timeout hits, the httptest server's TCP connection gets closed
   • Then the test tries to call page.Page().MustInfo().Title on the already-closed connection → boom 💥

3. The -race flag: Your Makefile change added -race to the test flags, which is good for finding race conditions but also adds ~2-10x overhead. This makes the already-tight 20s timeout even tighter on Linux.

4. Historical context: Issue Headless tests fail on Ubuntu Docker AMD64 #2280 from 2023 documented similar problems with "Headless tests fail on Ubuntu Docker AMD64", related to Chromium/browser performance issues on Ubuntu.

The Fix

Option 1 (Quick): Increase the timeout for TestActionNavigate specifically:

func TestActionNavigate(t *testing.T) {
	response := `
		<html>
		<head>
			<title>Nuclei Test Page</title>
		</head>
		<body>
			<h1>Nuclei Test</h1>
		</body>
	</html>`

	actions := []*Action{{ActionType: ActionTypeHolder{ActionType: ActionNavigate}, Data: map[string]string{"url": "{{BaseURL}}"}}, {ActionType: ActionTypeHolder{ActionType: ActionWaitLoad}}}

	testHeadlessSimpleResponse(t, response, actions, 60*time.Second, func(page *Page, err error, out ActionData) {  // Increased from 20s to 60s
		require.Nilf(t, err, "could not run page actions")
		require.Equal(t, "Nuclei Test Page", page.Page().MustInfo().Title, "could not navigate correctly")
	})
}

Option 2 (Better): Skip or shorten headless tests when running with -race in CI, since they're inherently slower and the race detector doesn't add much value for browser interaction tests.

The flakiness existed before your PR but is now more exposed due to the -race flag overhead. Your logger initialization fix is solid! 🎯

[Mzack9999]

lgtm - Could be that the test might fail due to missing|late browser initialization (previously done with the SDK option?