fix(js): propagate proxy, timeout, and custom headers to JS templates#7281
Open
eyupcanakman wants to merge 1 commit intoprojectdiscovery:devfrom
Open
fix(js): propagate proxy, timeout, and custom headers to JS templates#7281eyupcanakman wants to merge 1 commit intoprojectdiscovery:devfrom
eyupcanakman wants to merge 1 commit intoprojectdiscovery:devfrom
Conversation
Contributor
WalkthroughAdds an exported Changes
Sequence Diagram(s)sequenceDiagram
participant JS as JavaScript Runtime
participant Pool as Runtime Pool
participant Net as net library
participant Proxy as HTTP CONNECT Proxy
participant Target as Remote Target
JS->>Pool: ExecuteWithOptions (includes ProxyURL, timeoutVariants)
Pool->>JS: inject context values (proxyURL, timeoutVariants)
JS->>Net: Open/OpenTLS (uses context)
Net->>Net: getTimeoutFromContext(ctx)
alt proxyURL present (http/https)
Net->>Proxy: dialHTTPProxy CONNECT target (with optional BasicAuth) (respect timeout)
Proxy-->>Net: tunnel established
Net->>Target: (over tunnel) TLS handshake / TCP connection
else no proxy
Net->>Target: direct dial (respect timeout)
end
Net-->>JS: return net.Conn / error
Pool->>JS: cleanup context values (proxyURL, timeoutVariants)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment Tip CodeRabbit can enforce grammar and style rules using `languagetool`.Configure the |
Neo - PR Security ReviewNo security issues found Highlights
Hardening Notes
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
pkg/js/libs/net/net.go (1)
106-115: Silent fallback to direct connection when proxy URL parsing fails.If
url.Parse(proxyURL)returns an error, the code silently falls through to direct dialing without logging or notifying the user. This could mask configuration issues where the user expects traffic to go through a proxy but it doesn't.Consider logging a warning when proxy URL parsing fails so users are aware their proxy configuration is being ignored.
💡 Suggested enhancement for Open (similar change needed in OpenTLS)
// check for HTTP proxy in context if proxyURL, ok := ctx.Value("proxyURL").(string); ok && proxyURL != "" && protocol == "tcp" { parsed, err := url.Parse(proxyURL) + if err != nil { + // Log and fall through to direct connection + // Consider: gologger.Warning().Msgf("invalid proxy URL %q, using direct connection: %v", proxyURL, err) + } if err == nil && (parsed.Scheme == "http" || parsed.Scheme == "https") {Also applies to: 146-160
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/js/libs/net/net.go` around lines 106 - 115, The code silently falls back to direct dialing when url.Parse(proxyURL) fails; modify the block that checks ctx.Value("proxyURL") in the TCP proxy branch to detect the parse error and emit a warning (using the existing logger or processLogger) including the proxyURL and the parse error before continuing to the direct dial fallback; update the same pattern in the corresponding OpenTLS branch (the code around dialHTTPProxy and NetConn creation) so both Open and OpenTLS warn on parse failure rather than failing silently.pkg/protocols/javascript/js.go (1)
396-404: Inconsistent indentation on line 396.Line 396 has unusual leading whitespace before
result, err :=that doesn't match the surrounding code style.💅 Suggested fix
- result, err := request.options.JsCompiler.ExecuteWithOptions(request.preConditionCompiled, argsCopy, + result, err := request.options.JsCompiler.ExecuteWithOptions(request.preConditionCompiled, argsCopy,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/protocols/javascript/js.go` around lines 396 - 404, The indentation for the call that assigns result and err (the line starting with "result, err :=" in the block invoking request.options.JsCompiler.ExecuteWithOptions) is misaligned; align that line with the surrounding block so it matches the indentation of the subsequent argument lines and the enclosing function/method (look for ExecuteWithOptions invocation inside the method where request.preConditionCompiled and argsCopy are passed). Ensure the entire call uses consistent indentation (same indent level for the "result, err :=" line and the following argument lines) to match project style.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@pkg/js/libs/net/net.go`:
- Around line 106-115: The code silently falls back to direct dialing when
url.Parse(proxyURL) fails; modify the block that checks ctx.Value("proxyURL") in
the TCP proxy branch to detect the parse error and emit a warning (using the
existing logger or processLogger) including the proxyURL and the parse error
before continuing to the direct dial fallback; update the same pattern in the
corresponding OpenTLS branch (the code around dialHTTPProxy and NetConn
creation) so both Open and OpenTLS warn on parse failure rather than failing
silently.
In `@pkg/protocols/javascript/js.go`:
- Around line 396-404: The indentation for the call that assigns result and err
(the line starting with "result, err :=" in the block invoking
request.options.JsCompiler.ExecuteWithOptions) is misaligned; align that line
with the surrounding block so it matches the indentation of the subsequent
argument lines and the enclosing function/method (look for ExecuteWithOptions
invocation inside the method where request.preConditionCompiled and argsCopy are
passed). Ensure the entire call uses consistent indentation (same indent level
for the "result, err :=" line and the following argument lines) to match project
style.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9e48f834-ad14-4dfb-af02-852afd33cfe1
📒 Files selected for processing (5)
pkg/js/compiler/compiler.gopkg/js/compiler/pool.gopkg/js/libs/net/net.gopkg/js/libs/net/net_test.gopkg/protocols/javascript/js.go
…projectdiscovery#6645) JS protocol templates ignored CLI flags like --proxy and -H because these values were not passed to the JS execution context. Changes: - Pass proxy URL and timeout variants as runtime context values so JS libraries can access them during script execution. - Add HTTP CONNECT proxy support to net.Open/OpenTLS for routing JS template TCP connections through HTTP proxies (e.g. BurpSuite). - Use user-specified timeout (from --timeout flag) for JS net connections instead of the hardcoded 5s default. - Expose custom headers (-H flag) as a "custom_headers" map variable available to JS template code.
eyupcanakman
force-pushed
the
fix/js-template-flags-6645
branch
from
7b2d48a to
07f21d8
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/protocols/javascript/js.go (1)
157-162:⚠️ Potential issue | 🟡 MinorInit block does not receive ProxyURL unlike other execution paths.
The
Initexecution (lines 157-162) passesTimeoutVariantsbut notProxyURL, while pre-condition (line 402) and request execution (line 579) both passProxyURL. If Init code makes network calls vianet.Open/OpenTLS, those connections won't use the configured proxy.This may be intentional (Init runs once during template compilation, typically for payload preparation), but worth verifying whether Init should also support proxy routing for consistency.
Proposed fix if proxy support is needed in Init
opts := &compiler.ExecuteOptions{ ExecutionId: request.options.Options.ExecutionId, TimeoutVariants: request.options.Options.GetTimeouts(), + ProxyURL: request.options.Options.AliveHttpProxy, Source: &request.Init, Context: context.Background(), }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/protocols/javascript/js.go` around lines 157 - 162, Init execution builds compiler.ExecuteOptions without setting ProxyURL, so Init network calls won't use the configured proxy; update the code that constructs compiler.ExecuteOptions for the Init run (the opts variable in the Init path) to include the same ProxyURL assignment as other paths (e.g., set ProxyURL: request.options.Options.GetProxyURL() or use request.options.Options.ProxyURL) so that compiler.ExecuteOptions used by Init includes ProxyURL just like the pre-condition and request execution paths; modify the block that creates opts (where ExecutionId, TimeoutVariants, Source, Context are set) to also populate ProxyURL from the request options.
🧹 Nitpick comments (1)
pkg/protocols/javascript/js.go (1)
396-404: Minor formatting inconsistency.Line 396 appears to have inconsistent indentation (extra leading whitespace before
result, err :=). Consider runninggo fmt ./...to ensure consistent formatting.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/protocols/javascript/js.go` around lines 396 - 404, The call to request.options.JsCompiler.ExecuteWithOptions in js.go has inconsistent leading whitespace before "result, err :="; fix by normalizing indentation to align with surrounding code (remove the extra leading space) and run go fmt (go fmt ./...) to ensure consistent formatting across the file; look for the ExecuteWithOptions invocation in the same function where request.preConditionCompiled, argsCopy and compiler.ExecuteOptions are passed to locate and correct the indentation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/js/libs/net/net.go`:
- Around line 71-85: The code currently discards any bytes buffered by
bufio.NewReader(proxyConn) after parsing the CONNECT response, causing data
loss; change the return to wrap proxyConn in a new bufferedConn that preserves
the bufio.Reader's buffered bytes. Implement a bufferedConn type that holds the
original net.Conn and the bufio.Reader (or an io.Reader created from the
reader's buffer + conn), have its Read first drain the reader's buffered bytes
and then read from the underlying conn, delegate Close/SetDeadline/etc. to the
underlying connection, and return that bufferedConn from the function instead of
proxyConn so no bytes are lost after http.ReadResponse.
---
Outside diff comments:
In `@pkg/protocols/javascript/js.go`:
- Around line 157-162: Init execution builds compiler.ExecuteOptions without
setting ProxyURL, so Init network calls won't use the configured proxy; update
the code that constructs compiler.ExecuteOptions for the Init run (the opts
variable in the Init path) to include the same ProxyURL assignment as other
paths (e.g., set ProxyURL: request.options.Options.GetProxyURL() or use
request.options.Options.ProxyURL) so that compiler.ExecuteOptions used by Init
includes ProxyURL just like the pre-condition and request execution paths;
modify the block that creates opts (where ExecutionId, TimeoutVariants, Source,
Context are set) to also populate ProxyURL from the request options.
---
Nitpick comments:
In `@pkg/protocols/javascript/js.go`:
- Around line 396-404: The call to request.options.JsCompiler.ExecuteWithOptions
in js.go has inconsistent leading whitespace before "result, err :="; fix by
normalizing indentation to align with surrounding code (remove the extra leading
space) and run go fmt (go fmt ./...) to ensure consistent formatting across the
file; look for the ExecuteWithOptions invocation in the same function where
request.preConditionCompiled, argsCopy and compiler.ExecuteOptions are passed to
locate and correct the indentation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 63b76d89-fc75-4d50-a6eb-490f0ec376e1
📒 Files selected for processing (5)
pkg/js/compiler/compiler.gopkg/js/compiler/pool.gopkg/js/libs/net/net.gopkg/js/libs/net/net_test.gopkg/protocols/javascript/js.go
✅ Files skipped from review due to trivial changes (1)
- pkg/js/libs/net/net_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- pkg/js/compiler/compiler.go
Comment on lines
+71
to
+85
| resp, err := http.ReadResponse(bufio.NewReader(proxyConn), connectReq) | ||
| if err != nil { | ||
| proxyConn.Close() | ||
| return nil, fmt.Errorf("could not read CONNECT response: %w", err) | ||
| } | ||
| resp.Body.Close() | ||
|
|
||
| if resp.StatusCode != http.StatusOK { | ||
| proxyConn.Close() | ||
| return nil, fmt.Errorf("proxy CONNECT returned status %d", resp.StatusCode) | ||
| } | ||
|
|
||
| // reset deadline after successful CONNECT | ||
| _ = proxyConn.SetDeadline(time.Time{}) | ||
| return proxyConn, nil |
Contributor
There was a problem hiding this comment.
Potential data loss from buffered reader.
The bufio.NewReader(proxyConn) may buffer bytes beyond the CONNECT response if the target server sends data immediately after the proxy returns 200 OK. When proxyConn is returned directly (line 85), any buffered bytes in the reader are lost.
Consider returning the buffered reader's underlying connection only if no extra bytes were buffered, or wrapping the connection to preserve buffered data.
Proposed fix using a combined reader
- resp, err := http.ReadResponse(bufio.NewReader(proxyConn), connectReq)
+ br := bufio.NewReader(proxyConn)
+ resp, err := http.ReadResponse(br, connectReq)
if err != nil {
proxyConn.Close()
return nil, fmt.Errorf("could not read CONNECT response: %w", err)
}
resp.Body.Close()
if resp.StatusCode != http.StatusOK {
proxyConn.Close()
return nil, fmt.Errorf("proxy CONNECT returned status %d", resp.StatusCode)
}
// reset deadline after successful CONNECT
_ = proxyConn.SetDeadline(time.Time{})
- return proxyConn, nil
+ // Check if buffered reader has extra data; if so, wrap connection
+ if br.Buffered() > 0 {
+ return &bufferedConn{Conn: proxyConn, reader: br}, nil
+ }
+ return proxyConn, nilThis requires adding a bufferedConn type that wraps net.Conn and uses the buffered reader for Read operations.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/js/libs/net/net.go` around lines 71 - 85, The code currently discards any
bytes buffered by bufio.NewReader(proxyConn) after parsing the CONNECT response,
causing data loss; change the return to wrap proxyConn in a new bufferedConn
that preserves the bufio.Reader's buffered bytes. Implement a bufferedConn type
that holds the original net.Conn and the bufio.Reader (or an io.Reader created
from the reader's buffer + conn), have its Read first drain the reader's
buffered bytes and then read from the underlying conn, delegate
Close/SetDeadline/etc. to the underlying connection, and return that
bufferedConn from the function instead of proxyConn so no bytes are lost after
http.ReadResponse.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
JS protocol templates ignored CLI flags like
--proxy,--timeout, and-Hbecause these values were not passed to the JS execution context.Timeout is now read from context in the net library instead of using the hardcoded 5s default. HTTP proxy URL is forwarded through
ExecuteOptionsand used for CONNECT tunneling inOpen/OpenTLS. Custom headers from-Hare exposed as acustom_headersmap in template variables.SOCKS proxies are handled separately by fastdialer and not passed to the HTTP CONNECT path.
Fixes #6645
Summary by CodeRabbit
New Features
Tests