Skip to content

fix: add nil check for opts in executeWithRuntime (#7248)#7285

Open
DrGalio wants to merge 1 commit intoprojectdiscovery:devfrom
DrGalio:fix-nil-check-pool-executeWithRuntime
Open

fix: add nil check for opts in executeWithRuntime (#7248)#7285
DrGalio wants to merge 1 commit intoprojectdiscovery:devfrom
DrGalio:fix-nil-check-pool-executeWithRuntime

Conversation

@DrGalio
Copy link

@DrGalio DrGalio commented Mar 22, 2026

Summary

Adds a nil check before accessing opts.ExecutionId in executeWithRuntime in pkg/js/compiler/pool.go.

Problem

Line 115 accesses opts.ExecutionId without checking if opts is nil first:

runtime.SetContextValue("executionId", opts.ExecutionId)

However, other accesses to opts in the same function already perform nil checks:

  • Line 86: if opts != nil && opts.Cleanup != nil
  • Line 109: if opts != nil && opts.Callback != nil

This inconsistency means calling executeWithRuntime with a nil opts will panic.

Fix

Wrap the runtime.SetContextValue call in an if opts != nil guard, consistent with the existing nil checks in the function.

Changes

  • pkg/js/compiler/pool.go: Added nil check before opts.ExecutionId access

Closes #7248

Summary by CodeRabbit

  • Bug Fixes
    • Fixed a potential runtime error that could occur when processing execution requests.

)

Closes projectdiscovery#7248

Adds nil check before accessing opts.ExecutionId in executeWithRuntime,
consistent with existing nil checks for opts.Callback and opts.Cleanup
in the same function.
@auto-assign auto-assign bot requested a review from Mzack9999 March 22, 2026 07:48
@neo-by-projectdiscovery-dev
Copy link

neo-by-projectdiscovery-dev bot commented Mar 22, 2026

Neo - PR Security Review

No security issues found

Highlights

  • Adds nil check before accessing opts.ExecutionId, preventing a potential panic
  • Makes nil handling consistent across the executeWithRuntime function
  • No security-impacting changes to JavaScript execution logic

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

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 22, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 4ca6e055-e079-4ab3-bc6d-f4a227992231

📥 Commits

Reviewing files that changed from the base of the PR and between 6eda56f and 03da46c.

📒 Files selected for processing (1)
  • pkg/js/compiler/pool.go

Walkthrough

Fixed a missing nil check in executeWithRuntime function that unconditionally dereferenced opts when setting the executionId context value, adding defensive programming to prevent potential panic when opts is nil.

Changes

Cohort / File(s) Summary
Nil Check Fix
pkg/js/compiler/pool.go
Added defensive nil check before dereferencing opts for opts.ExecutionId in executeWithRuntime function to prevent panic when opts argument is nil.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

A rabbit hops through code with care,
Where nil pointers once lurked there,
A simple check, a guard so tight,
Makes execution safe and right! 🐰✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. 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 specifically describes the main change: adding a nil check for opts in executeWithRuntime function, with direct reference to the related issue.
Linked Issues check ✅ Passed The PR directly addresses issue #7248 by adding the required nil check for opts before accessing ExecutionId, matching the expected behavior outlined in the issue.
Out of Scope Changes check ✅ Passed All changes are scoped to the identified issue; only pkg/js/compiler/pool.go is modified to add the nil check as required by #7248.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 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 and usage tips.

Tip

You can disable poems in the walkthrough.

Disable the reviews.poem setting to disable the poems in the walkthrough.

Copy link

@themavik themavik left a comment

Choose a reason for hiding this comment

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

test review from automation - please ignore

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.

[BUG] Missing nil check for opts in js/compiler/pool/executeWithRuntime

2 participants