fix(variable): global variable not same between two request in flow mode

[iuliu8899]

fix #6337

Proposed changes

reproduce code:
main.go

package main

import (
	"context"
	"fmt"
	nuclei "github.com/projectdiscovery/nuclei/v3/lib"
	"github.com/projectdiscovery/nuclei/v3/pkg/output"
	"net/http"
)

func main() {
	// Start HTTP server
	go startHTTPServer()

	ne, err := nuclei.NewNucleiEngineCtx(context.Background(),
		nuclei.WithTemplatesOrWorkflows(
			nuclei.TemplateSources{Templates: []string{`C:\Users\xxx\GolandProjects\nuclei\examples\simple\test.yaml`}},
		),
	)
	if err != nil {
		panic(err)
	}
	// load targets and optionally probe non http/https targets
	ne.LoadTargets([]string{"http://127.0.0.1:12345"}, false)
	err = ne.ExecuteWithCallback(func(event *output.ResultEvent) {
		fmt.Println(event)
	})
	if err != nil {
		panic(err)
	}
	defer ne.Close()
}

// HTTP server handler function
func startHTTPServer() {
	http.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
		fmt.Printf("Received request: %s %s\n", r.Method, r.URL.Path)
		w.WriteHeader(http.StatusOK)
		w.Write([]byte(r.URL.Path))
	})

	fmt.Println("HTTP server started on port :12345")
	if err := http.ListenAndServe(":12345", nil); err != nil {
		panic(err)
	}
}

test.yaml

id: dahua-smart-park-bitmap-uploadfile

info:
  name: xxxxx
  author: xxxx
  severity: critical
  description: |
    xxxxxxx
  tags: uploadfile
variables:
  filename: '{{to_lower(rand_base(5))}}'
  str: 'ovybpdeynlqqajl4ca5lyebrqlxrws'
  string: <%out.print("{{str}}");new java.io.File(application.getRealPath(request.getServletPath())).delete();%>
flow: http(1) && http(2)
http:
  - raw:
      - |
        POST /{{filename}} HTTP/1.1
        Host: {{Hostname}}

    matchers:
      - type: status
        status:
          - 200
        internal: true
  - raw:
      - |
        POST /{{filename}} HTTP/1.1
        Host: {{Hostname}}
        

    matchers:
      - type: dsl
        dsl:
          - "http_1_body == http_2_body"

Since the variable string contains Path keyword, which would be marked as LazyEval. In LazyEval mode, variables would be evaluated everytime the request build.

The root cause is the false positive in this function.

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

• Bug Fixes

  • Runtime/dynamic variables are now included when evaluating external interaction points, improving request accuracy and reducing missed callbacks.
  • Interaction callback URLs are more reliably collected and tracked, improving stability of network interaction handling.

• Chores

  • Local settings for developer tooling are now ignored to reduce accidental commits.

[coderabbitai]

Walkthrough

Merge dynamicValues into the variables passed to EvaluateWithInteractsh during HTTP request build; derive variablesMap and interactURLs from that merged map and append any returned interact URLs to r.interactshURLs. No exported/public signatures changed.

Changes

Cohort / File(s)	Summary
HTTP request build & Interactsh evaluation pkg/protocols/http/build_request.go	Merge defaultReqVars, optionVars, and dynamicValues before calling EvaluateWithInteractsh; obtain variablesMap and interactURLs from that merged map; append returned interactURLs to r.interactshURLs.
Repository metadata .gitignore	Add .claude/settings.local.json to the ignore list.

Sequence Diagram(s)

sequenceDiagram
  participant Make as HTTP.Make
  participant Eval as EvaluateWithInteractsh
  participant Interact as Interactsh

  Note over Make: Prepare variables
  Make->>Make: Merge defaultReqVars + optionVars + dynamicValues
  Make->>Eval: EvaluateWithInteractsh(mergedVars)
  Eval->>Interact: Use merged variables to evaluate/register
  Interact-->>Eval: interactURLs
  Eval-->>Make: variablesMap, interactURLs
  Make->>Make: Append interactURLs to r.interactshURLs

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Assessment against linked issues

Objective	Addressed	Explanation
Fix variable handling in flow so variables are not re-evaluated unexpectedly and remain consistent across multiple HTTP raws (#6337)	❓	The change ensures dynamicValues are included when evaluating Interactsh, but the diff does not show broader alterations to template evaluation order or lifecycle that would demonstrably prevent repeated re-evaluation across flow runs.

Assessment against linked issues: Out-of-scope changes

Code Change	Explanation
Added .claude/settings.local.json to .gitignore	Editor/IDE settings ignore entry unrelated to variable-evaluation behavior.

Suggested reviewers

• dwisiswant0

Poem

I'm a rabbit who merged with care,
Defaults, options, dynamic — stitched in a pair.
Interact URLs hop into the list,
Variables map now won't be missed.
🐇

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Linked Issues Check	⚠️ Warning	The changes only modify variable merging for interactsh evaluation in build_request.go and update .gitignore, but they do not address the core requirement from issue #6337 to evaluate template variables once and maintain consistent values across flow requests by fixing the variable evaluation logic in pkg/protocols/common/variables/variables.go.	Adjust the implementation to ensure that template variables are evaluated only once per execution and remain stable across flow mode, targeting the variable evaluation logic in variables.go and include corresponding tests for issue #6337.
Out of Scope Changes Check	⚠️ Warning	The addition to .gitignore and the modifications for interactsh variable merging fall outside the scope of issue #6337, which specifically requires fixing the variable evaluation mechanism to preserve global template variables across flow requests rather than adjusting ignore rules or interactsh behavior.	Remove or separate unrelated .gitignore and interactsh changes and focus the PR on updating the variable evaluation code in variables.go to meet the linked issue’s objectives.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The provided title clearly describes the primary fix addressing inconsistent global variable values across two requests in flow mode by highlighting “global variable not same between two request in flow mode,” making the main change immediately understandable from the title.
Docstring Coverage	✅ Passed	No functions found in the changes. Docstring coverage check skipped.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

📜 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 d3f99a1 and 817ed86.

📒 Files selected for processing (1)

• pkg/protocols/http/build_request.go (1 hunks)

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

• pkg/protocols/http/build_request.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). (1)

• GitHub Check: Lint

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

pkg/protocols/http/build_request.go (1)

184-184: Consider including constants during variables evaluation and verify precedence so dynamic values remain stable across flow.

Rationale:

• MergeMaps uses “last map wins.” Today, in allVars (Line 189), variablesMap overrides dynamicValues for duplicate keys. If a variable is pre-populated in dynamicValues (from http(1)) but also exists in the template variables section, variablesMap may overwrite it for http(2), which could reintroduce drift.
• EvaluateWithInteractsh (Line 184) does not include r.options.Constants. If variables definitions reference constants, including them here would enable full evaluation and proper interactsh URL extraction at variables stage.

Suggestions (optional; validate against existing tests/templates):

• Include constants in the variables-evaluation context (safe, improves completeness for variable expressions referencing constants):

- var iablesMap, interactURLs := r.options.Variables.EvaluateWithInteractsh(generators.MergeMaps(defaultReqVars, optionVars, dynamicValues), r.options.Interactsh)
+ variablesMap, interactURLs := r.options.Variables.EvaluateWithInteractsh(
+   generators.MergeMaps(defaultReqVars, optionVars, dynamicValues, r.options.Constants),
+   r.options.Interactsh,
+ )

• If the intent is “dynamic values win” across a flow for duplicate keys, consider reordering allVars so dynamicValues come after variablesMap (last map wins):

- allVars := generators.MergeMaps(dynamicValues, defaultReqVars, optionVars, variablesMap, r.options.Constants)
+ allVars := generators.MergeMaps(defaultReqVars, optionVars, variablesMap, dynamicValues, r.options.Constants)

Please verify which precedence is intended across the codebase (dynamic vs. variablesMap) and adjust accordingly to avoid regressions.

Also applies to: 189-189

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 44eeb5a and 4f6d3d8.

📒 Files selected for processing (1)

• pkg/protocols/http/build_request.go (1 hunks)

🧰 Additional context used

🧬 Code Graph Analysis (1)

pkg/protocols/http/build_request.go (1)

pkg/protocols/common/generators/maps.go (1)

• MergeMaps (48-54)

🔇 Additional comments (1)

pkg/protocols/http/build_request.go (1)

184-187: Good fix: dynamic values now seed variables evaluation (+ interactsh URL tracking).

Passing dynamicValues into EvaluateWithInteractsh (Line 184) aligns with the flow-mode requirement: variables computed once can persist across http(1) && http(2) instead of being re-evaluated. Appending returned interactURLs ensures URLs introduced via variables are also tracked.

[ehsandeep]

id: test-template

info:
  name: xxxxx
  author: xxxx
  severity: critical
  description: |
    xxxxxxx
  tags: uploadfile

variables:
  filename: test

flow: http(1) && http(2)
http:
  - raw:
      - |
        POST /{{filename}} HTTP/1.1
        Host: {{Hostname}}

    matchers:
      - type: status
        status:
          - 403
        internal: true

  - raw:
      - |
        POST /{{filename}} HTTP/1.1
        Host: {{Hostname}}
        

    matchers:
      - type: dsl
        dsl:
          - "http_1_status_code == http_2_status_code"

$ ./bin/nuclei -t test.yaml -u https://example.com -silent
[test-template] [http] [critical] https://example.com/test

[tarunKoyalwar]

lgtm!

[dwisiswant0]

The failing integration tests were likely introduced in this PR, a few of them:

...
[✘] Test "protocols/headless/headless-basic.yaml" failed: unexpected result: expecting an error but got ""
...
[✘] Test "protocols/http/variables.yaml" failed: incorrect number of results: 0 (actual) vs [1] (expected) 
...
[✘] Test "protocols/http/variables.yaml" failed: incorrect number of results: 0 (actual) vs [1] (expected) 
...

Some edge case:

[✘] Test "protocols/ssl/multi-req.yaml" failed: incorrect number of results: 7 (actual) vs [2] (expected) 

@coderabbitai create an issue for that.

[coderabbitai]

Tip

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Create a GitHub issue for failing integration tests in PR #6395

gh issue create \
  --title "Failing integration tests introduced in PR #6395 (variable fix)" \
  --body "## Context

PR #6395 (https://github.com/projectdiscovery/nuclei/pull/6395) introduced failing integration tests.

**Reported by:** @dwisiswant0  
**Comment:** https://github.com/projectdiscovery/nuclei/pull/6395#discussion_r1905820379

## Failing Tests

The following integration tests are failing:

### 1. \`protocols/headless/headless-basic.yaml\`
- **Error:** unexpected result: expecting an error but got \"\"

### 2. \`protocols/http/variables.yaml\` (appears twice)
- **Error:** incorrect number of results: 0 (actual) vs [1] (expected)

### 3. \`protocols/ssl/multi-req.yaml\` (edge case)
- **Error:** incorrect number of results: 7 (actual) vs [2] (expected)

## Next Steps

These test failures need to be investigated and fixed to ensure the PR changes don't break existing functionality." \
  --assignee dwisiswant0

Length of output: 54

---

@dwisiswant0, I've created issue #6537 to track the failing integration tests introduced in this PR. The issue documents all the test failures you identified, including the edge case with protocols/ssl/multi-req.yaml, and has been assigned to you.

You can view the issue here: #6537

🐰💭

  ∧＿∧
 (  ･ω･)  *Task complete!*
 ⊂      ⊃
  しーＪ