Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 38 additions & 6 deletions cmd/integration-test/javascript.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,17 @@
{Path: "protocols/javascript/net-https.yaml", TestCase: &javascriptNetHttps{}},
{Path: "protocols/javascript/oracle-auth-test.yaml", TestCase: &javascriptOracleAuthTest{}, DisableOn: func() bool { return osutils.IsWindows() || osutils.IsOSX() }},
{Path: "protocols/javascript/vnc-pass-brute.yaml", TestCase: &javascriptVncPassBrute{}},
{Path: "protocols/javascript/multi-ports.yaml", TestCase: &javascriptMultiPortsSSH{}},
}

var (
redisResource *dockertest.Resource
sshResource *dockertest.Resource
oracleResource *dockertest.Resource
vncResource *dockertest.Resource
pool *dockertest.Pool
defaultRetry = 3
redisResource *dockertest.Resource
sshResource *dockertest.Resource
oracleResource *dockertest.Resource
vncResource *dockertest.Resource
multiPortsSShResource *dockertest.Resource

Check failure on line 28 in cmd/integration-test/javascript.go

View workflow job for this annotation

GitHub Actions / Lint

var multiPortsSShResource is unused (unused)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Fix the unused variable causing pipeline failure.

The multiPortsSShResource variable is declared but never used, causing the linter to fail. The javascriptMultiPortsSSH.Execute method checks sshResource instead (line 175). This needs to be resolved:

  • Option 1: Remove multiPortsSShResource if the test should use the existing sshResource.
  • Option 2: Update the executor (line 175) to check multiPortsSShResource and initialize it in the init() function if a separate SSH resource is needed for multi-port testing.

Based on the PR description mentioning "multiple ports" (222, 22, 2244), it appears the test should target a separate multi-port SSH resource. Please clarify whether:

  1. A new Docker container with multiple exposed ports should be created in init(), or
  2. The test should reuse the existing sshResource

Run this script to check if the multi-ports template expects specific port configurations:


🏁 Script executed:

#!/bin/bash
# Check the multi-ports template for port configuration details
fd -e yaml -e yml 'multi-ports' --exec cat {}

Length of output: 0







Let me search for the actual file contents by examining the repository structure:






Since I cannot directly access the file without knowing the repository structure and the scripts are not providing output, let me make a final request verification approach:

Clarify whether the unused SSH resource variable should be initialized and used, or removed entirely.

Based on the original review, multiPortsSShResource is declared but never used, causing the linter to fail. The test executor checks sshResource instead (line 175). Since the PR mentions testing multiple ports (222, 22, 2244), the test likely needs a separate multi-port SSH resource, but the initialization in init() is missing and the executor doesn't reference the variable.

Two options remain:

  • Remove multiPortsSShResource if the test should reuse the existing sshResource
  • Initialize and use multiPortsSShResource in init() and update line 175 to reference it, assuming a separate multi-port container is needed

🏁 Script executed:

#!/bin/bash
# Inspect the javascript.go file to understand variable declarations and usage
cat cmd/integration-test/javascript.go | head -60

Length of output: 2239


🏁 Script executed:

#!/bin/bash
# Check the line count and find references to multiPortsSShResource and sshResource
grep -n "multiPortsSShResource\|sshResource" cmd/integration-test/javascript.go

Length of output: 377


🏁 Script executed:

#!/bin/bash
# Get a broader view of the init() function and Execute method
sed -n '1,200p' cmd/integration-test/javascript.go | grep -n "init\|Execute\|sshResource\|multiPortsSShResource"

Length of output: 683


🏁 Script executed:

#!/bin/bash
# Search for the javascript.go file
find . -name "javascript.go" -type f 2>/dev/null | head -5

Length of output: 36


🏁 Script executed:

#!/bin/bash
# Get the full content of the javascript.go file to understand structure
find . -name "javascript.go" -type f -exec wc -l {} \; 2>/dev/null

Length of output: 40


🏁 Script executed:

#!/bin/bash
# Search for all references to SSH resource patterns in the codebase
rg -i "ssh.*Resource\|resource.*ssh" -C 3 --type go 2>/dev/null | head -100

Length of output: 0






---

🏁 Script executed:

```shell
#!/bin/bash
# Get the full Execute method for javascriptMultiPortsSSH
sed -n '174,200p' cmd/integration-test/javascript.go

Length of output: 665


🏁 Script executed:

#!/bin/bash
# Check the init() function to see how sshResource is initialized
sed -n '240,280p' cmd/integration-test/javascript.go

Length of output: 1039


🏁 Script executed:

#!/bin/bash
# Check if there's any difference between the test cases for SSH fingerprint vs multi-ports
grep -A 5 "javascriptSSHServerFingerprint\|javascriptMultiPortsSSH" cmd/integration-test/javascript.go | head -20

Length of output: 1050


Remove the unused multiPortsSShResource variable or initialize and use it properly.

The variable multiPortsSShResource (line 28) is declared but never initialized or referenced anywhere in the codebase, causing the linter to fail. The javascriptMultiPortsSSH.Execute() method (lines 174-200) checks sshResource instead and does not extract or configure multiple ports like the test name suggests.

Two valid resolutions:

  • Remove multiPortsSShResource if the multi-ports test should reuse the existing sshResource
  • Initialize multiPortsSShResource in init() with the appropriate port mappings and update the Execute() method to use it (currently lines 175-200 incorrectly check sshResource)

Currently, both javascriptSSHServerFingerprint and javascriptMultiPortsSSH use the same resource, indicating incomplete implementation of the multi-port test setup.

🧰 Tools
🪛 GitHub Actions: 🔨 Tests

[error] 28-28: golangci-lint: 'multiPortsSShResource' is unused (unused)

🪛 GitHub Check: Lint

[failure] 28-28:
var multiPortsSShResource is unused (unused)

🤖 Prompt for AI Agents
In cmd/integration-test/javascript.go around line 28, the declared variable
multiPortsSShResource is unused which breaks the linter; either delete this
variable and leave the multi-ports test to reuse sshResource, or properly
implement the multi-ports resource: initialize multiPortsSShResource in init()
with the intended additional port mappings (same dockertest.Resource creation
pattern as sshResource but exposing the extra ports), and update
javascriptMultiPortsSSH.Execute() (currently checking sshResource) to reference
multiPortsSShResource and extract/configure the multiple ports for the test;
choose one approach and remove the dead declaration or wire the new resource
through init() and Execute().

pool *dockertest.Pool
defaultRetry = 3
)

type javascriptNetHttps struct{}
Expand Down Expand Up @@ -167,6 +169,36 @@
return multierr.Combine(errs...)
}

type javascriptMultiPortsSSH struct{}

func (j *javascriptMultiPortsSSH) Execute(filePath string) error {
if sshResource == nil || pool == nil {
// skip test as redis is not running
return nil
}
finalURL := "scanme.sh"
errs := []error{}
for i := 0; i < defaultRetry; i++ {
results := []string{}
var err error
_ = pool.Retry(func() error {
//let ssh server start
time.Sleep(3 * time.Second)
results, err = testutils.RunNucleiTemplateAndGetResults(filePath, finalURL, debug)
return nil
})
if err != nil {
return err
}
if err := expectResultsCount(results, 1); err == nil {
return nil
} else {
errs = append(errs, err)
}
}
return multierr.Combine(errs...)
}
Comment on lines +173 to +180

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Multiple issues with executor implementation.

The javascriptMultiPortsSSH executor has several problems that deviate from the established pattern:

  1. Line 175: Checks sshResource instead of multiPortsSShResource, which creates inconsistency with the declared variable at line 28.

  2. Line 176: Copy-paste error in comment—says "redis" but should say "SSH" or "multi-port SSH".

  3. Line 179: Uses hardcoded "scanme.sh" instead of extracting a port from a Docker resource like all other executors (see lines 50-51, 82-83, 114-115, 147-148). This breaks the established pattern of testing against local Docker containers.

  4. Missing defer purge(): Unlike all other executors (lines 52, 84, 116, 149), this executor doesn't clean up the Docker resource, potentially causing resource leaks.

Apply this diff to align with the established pattern (assuming you want to use the existing sshResource):

 func (j *javascriptMultiPortsSSH) Execute(filePath string) error {
 	if sshResource == nil || pool == nil {
-		// skip test as redis is not running
+		// skip test as ssh is not running
 		return nil
 	}
-	finalURL := "scanme.sh"
+	tempPort := sshResource.GetPort("2222/tcp")
+	finalURL := "localhost:" + tempPort
+	defer purge(sshResource)
 	errs := []error{}

Alternatively, if the test genuinely needs to target the external scanme.sh service (which seems inconsistent with an integration test suite using Docker), please clarify the intent and consider whether this should be a separate test type.

Comment on lines +171 to +180

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Avoid using pool.Retry here – risk of nil-pointer panic and unnecessary Docker dependency

javascriptMultiPortsSSH.Execute currently calls pool.Retry but:

  • There is no pool == nil guard, unlike the other executors. If Docker is unavailable or init() fails, the global pool can be nil, leading to a panic when pool.Retry is called.
  • This executor does not use any Docker resource at all and only targets scanme.sh, so it doesn’t actually need the dockertest pool; the retry loop can be implemented directly.
  • The comment //let ssh server start is misleading here, since there is no local SSH container involved.

To make this robust and simpler, you can drop the pool dependency entirely and keep the retry behavior, e.g.:

 func (j *javascriptMultiPortsSSH) Execute(filePath string) error {
-	finalURL := "scanme.sh"
-	errs := []error{}
-	for i := 0; i < defaultRetry; i++ {
-		results := []string{}
-		var err error
-		_ = pool.Retry(func() error {
-			//let ssh server start
-			time.Sleep(3 * time.Second)
-			results, err = testutils.RunNucleiTemplateAndGetResults(filePath, finalURL, debug)
-			return nil
-		})
-		if err != nil {
-			return err
-		}
-		if err := expectResultsCount(results, 1); err == nil {
-			return nil
-		} else {
-			errs = append(errs, err)
-		}
-	}
-	return multierr.Combine(errs...)
+	finalURL := "scanme.sh"
+	errs := []error{}
+	for i := 0; i < defaultRetry; i++ {
+		// give remote endpoint a brief pause between attempts
+		time.Sleep(3 * time.Second)
+
+		results, err := testutils.RunNucleiTemplateAndGetResults(filePath, finalURL, debug)
+		if err != nil {
+			return err
+		}
+		if err := expectResultsCount(results, 1); err == nil {
+			return nil
+		}
+		errs = append(errs, err)
+	}
+	return multierr.Combine(errs...)
 }

This removes the potential nil-pointer on pool, keeps the retry logic, and avoids binding this test to Docker when it only needs a remote host.

🤖 Prompt for AI Agents
In cmd/integration-test/javascript.go around lines 171 to 195, replace the use
of pool.Retry (which can panic if pool is nil and is unnecessary for a
remote-only test) with a direct retry loop: remove the pool.Retry call and the
misleading "//let ssh server start" comment, perform up to defaultRetry
iterations where you sleep briefly (e.g., 3s) before calling
testutils.RunNucleiTemplateAndGetResults(filePath, finalURL, debug), capture its
error into a local err variable, check expectResultsCount and return on success,
otherwise append the error to errs; after the loop, return
multierr.Combine(errs...) so the behavior and backoff remain but without
depending on pool or Docker.


// purge any given resource if it is not nil
func purge(resource *dockertest.Resource) {
if resource != nil && pool != nil {
Expand Down
40 changes: 34 additions & 6 deletions pkg/protocols/javascript/js.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import (
"github.com/projectdiscovery/utils/errkit"
iputil "github.com/projectdiscovery/utils/ip"
mapsutil "github.com/projectdiscovery/utils/maps"
sliceutil "github.com/projectdiscovery/utils/slice"
syncutil "github.com/projectdiscovery/utils/sync"
urlutil "github.com/projectdiscovery/utils/url"
)
Expand Down Expand Up @@ -133,8 +134,11 @@ func (request *Request) Compile(options *protocols.ExecutorOptions) error {
}

// "Port" is a special variable and it should not contains any dsl expressions
if strings.Contains(request.getPort(), "{{") {
return errkit.New("'Port' variable cannot contain any dsl expressions")
ports := request.getPorts()
for _, port := range ports {
if strings.Contains(port, "{{") {
return errkit.New("'Port' variable cannot contain any dsl expressions")
}
}

if request.Init != "" {
Expand Down Expand Up @@ -281,12 +285,28 @@ func (request *Request) GetID() string {

// ExecuteWithResults executes the protocol requests and returns results instead of writing them.
func (request *Request) ExecuteWithResults(target *contextargs.Context, dynamicValues, previous output.InternalEvent, callback protocols.OutputEventCallback) error {
// Get default port(s) if specified in template
ports := request.getPorts()

var errs []error

for _, port := range ports {
err := request.executeWithResults(port, target, dynamicValues, previous, callback)
if err != nil {
errs = append(errs, err)
}
}

return errkit.Join(errs...)
}
Comment on lines +288 to +301

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Critical: Templates without Port won't execute.

When getPorts() returns an empty slice (no Port specified in template args), the loop doesn't execute and the function returns immediately. This breaks existing JavaScript templates that don't specify a Port argument.

Apply this diff to execute once with an empty port when no ports are specified:

 func (request *Request) ExecuteWithResults(target *contextargs.Context, dynamicValues, previous output.InternalEvent, callback protocols.OutputEventCallback) error {
 	// Get default port(s) if specified in template
 	ports := request.getPorts()
+	if len(ports) == 0 {
+		ports = []string{""}
+	}
 
 	for _, port := range ports {
 		err := request.executeWithResults(port, target, dynamicValues, previous, callback)
 		if err != nil {
 			return err
 		}
 	}
 
 	return nil
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Get default port(s) if specified in template
ports := request.getPorts()
for _, port := range ports {
err := request.executeWithResults(port, target, dynamicValues, previous, callback)
if err != nil {
return err
}
}
return nil
}
func (request *Request) ExecuteWithResults(target *contextargs.Context, dynamicValues, previous output.InternalEvent, callback protocols.OutputEventCallback) error {
// Get default port(s) if specified in template
ports := request.getPorts()
if len(ports) == 0 {
ports = []string{""}
}
for _, port := range ports {
err := request.executeWithResults(port, target, dynamicValues, previous, callback)
if err != nil {
return err
}
}
return nil
}
🤖 Prompt for AI Agents
In pkg/protocols/javascript/js.go around lines 288 to 299, the current logic
returns immediately when request.getPorts() yields an empty slice, which
prevents templates without a Port arg from running; change the flow so that if
ports is empty you call request.executeWithResults once with an empty-string (or
zero-value) port (preserving the same target, dynamicValues, previous, callback)
and return any error; otherwise keep iterating over ports and return any error
from each call. Ensure you do not alter error handling semantics and that a
single execution occurs when no ports are specified.


// executeWithResults executes the request
func (request *Request) executeWithResults(port string, target *contextargs.Context, dynamicValues, previous output.InternalEvent, callback protocols.OutputEventCallback) error {
input := target.Clone()
// use network port updates input with new port requested in template file
// and it is ignored if input port is not standard http(s) ports like 80,8080,8081 etc
// idea is to reduce redundant dials to http ports
if err := input.UseNetworkPort(request.getPort(), request.getExcludePorts()); err != nil {
if err := input.UseNetworkPort(port, request.getExcludePorts()); err != nil {
gologger.Debug().Msgf("Could not network port from constants: %s\n", err)
}

Expand Down Expand Up @@ -755,13 +775,21 @@ func (request *Request) Type() templateTypes.ProtocolType {
return templateTypes.JavascriptProtocol
}

func (request *Request) getPort() string {
func (request *Request) getPorts() []string {
for k, v := range request.Args {
if strings.EqualFold(k, "Port") {
return types.ToString(v)
portStr := types.ToString(v)
ports := []string{}
for _, p := range strings.Split(portStr, ",") {
trimmed := strings.TrimSpace(p)
if trimmed != "" {
ports = append(ports, trimmed)
}
}
return sliceutil.Dedupe(ports)
}
}
return ""
return []string{}
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

func (request *Request) getExcludePorts() string {
Expand Down
Loading