feat(input): add per-target JSONL filters - #7597
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughJSONL target records now define per-target tags, exclusions, severities, and templates. The runner prepares these filters, providers retain target metadata, executors skip non-matching templates, progress totals account for skipped work, and preflight keys preserve filtered-input identity. ChangesPer-target filtering
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant JSONL
participant InputProvider
participant Runner
participant TargetFilter
participant Executor
participant Progress
JSONL->>InputProvider: parse target records
InputProvider->>Runner: provide MetaInput values
Runner->>TargetFilter: prepare inherited and per-target criteria
Runner->>Executor: configure and execute templates
Executor->>TargetFilter: evaluate template matches
TargetFilter-->>Executor: return match decision
Executor->>Progress: adjust totals for skipped requests
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
|
Marked ready for review. The branch is synced with current dev, and the affected packages (internal/runner, pkg/input, pkg/protocols/common/contextargs, pkg/templates, pkg/core) pass their tests locally. |
Neo - PR Security ReviewThe delta adds a correct pre-resolution containment guard for JSONL-supplied template selectors with no new exploitable vulnerabilities. What Neo reviewed
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (13)
internal/runner/preflight_portscan_test.go (1)
122-135: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe
Count()assertion does not test the production path.
allowCntis set to2in the literal on line 125, and line 134 asserts thatCount()returns2. The assertion cannot fail. The behavior worth locking down is thatpreflightResolveAndPortScanderivesallowCntfromcountAllowedPreflightInputsrather than fromlen(allowed.GetAll()). Line 117 already covers the helper. Either drop theCount()assertion or setallowCntfromcountAllowedPreflightInputsso the two stay linked.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/runner/preflight_portscan_test.go` around lines 122 - 135, Update the filteringInputProvider test setup so allowCnt is derived via countAllowedPreflightInputs rather than hard-coded as 2, allowing the Count() assertion to exercise the production-derived value. Keep the existing Iterate behavior and assertions unchanged.pkg/core/executors_test.go (2)
247-269: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTwo test files hardcode the
"TargetInputProvider"string. The shared root cause is that neither file uses theprovider.TargetInputProviderconstant frompkg/input/provider/interface.go. A rename of the constant would leave both tests compiling while silently skipping the target-filter branch they intend to cover.
pkg/core/executors_test.go#L247-L269: importpkg/input/providerand replace the threeinputType: "TargetInputProvider"literals with the constant.internal/runner/target_filters_test.go#L40-L40: replace the returned literal inInputTypewithprovider.TargetInputProvider; the package is already imported.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/core/executors_test.go` around lines 247 - 269, The tests hardcode the target provider type instead of using the shared constant. In pkg/core/executors_test.go lines 247-269, import pkg/input/provider and replace all three "TargetInputProvider" literals with provider.TargetInputProvider; in internal/runner/target_filters_test.go line 40, return provider.TargetInputProvider from InputType, using the existing import.
247-249: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse the
provider.TargetInputProviderconstant instead of the literal.The literal
"TargetInputProvider"must stay in sync withpkg/input/provider/interface.go. Importproviderand use the constant so a rename breaks the build instead of silently disabling the tested branch.Also applies to: 253-255, 268-269
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/core/executors_test.go` around lines 247 - 249, Replace the literal "TargetInputProvider" with provider.TargetInputProvider in the fakeTargetProvider initializations for deniedOnly and the additional referenced cases. Import the provider package and use the constant consistently so these tests track the canonical provider name.internal/runner/preflight_portscan.go (1)
55-66: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConsider caching the preflight key to avoid repeated marshaling.
filteringInputProvider.IteratecomputespreflightInputKeyfor every input on every iteration pass. In the template-spray strategyIterateruns once per template, so the total cost istemplates × inputs. For inputs with aTargetFilter, the new code adds a secondMetaInputallocation, a second JSON marshal, and an identity concatenation compared to the previous singleMarshalString.A small
map[*contextargs.MetaInput]stringcache, or a key computed once during preflight collection and reused, would remove the repeated work.Also applies to: 68-89
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/runner/preflight_portscan.go` around lines 55 - 66, The filteringInputProvider.Iterate path repeatedly computes preflightInputKey for the same MetaInput across template passes, causing redundant allocation and marshaling. Add caching keyed by *contextargs.MetaInput, or compute and retain each key during preflight collection, then reuse it in Iterate while preserving the existing allowed-key filtering and callback behavior.internal/tests/integration/target_jsonl_test.go (2)
100-137: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe fixed request count of 2 couples the test to clustering.
The assertion requires exactly one physical request per target, which holds only while the alpha and beta templates cluster. Any change to the clustering heuristics, or an added third template, produces a failure message about "physical request count" that does not explain the cause. Consider deriving the expected count from the scenario, or extending the failure message to state that clustering is the assumed precondition.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/tests/integration/target_jsonl_test.go` around lines 100 - 137, The request-count assertion in the strategy/scenario integration test is coupled to the current clustering behavior. Update the assertion around requestCount.Load to derive the expected count from each scenario when possible; otherwise explicitly state in the failure message that exactly two requests is the clustering precondition, so changes to clustering or template count are clearly diagnosed.
26-31: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a scenario for explicitly empty arrays.
Templatesis declared as*[]stringprecisely so an explicit"templates": []can be distinguished from an omitted field, and the PR documents that an empty array clears the inherited global selection. No scenario exercises that path end to end, and no scenario exercises a per-targetseverityoverride. Both are headline behaviors of this feature.Add one record with
Templatespointing at an empty slice, and one scenario with aseverityfield, then assert the resulting template selection.Also applies to: 81-97
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/tests/integration/target_jsonl_test.go` around lines 26 - 31, Add integration coverage in the target JSONL test scenarios using targetJSONLRecord: include a record whose Templates points to an explicitly empty slice and verify it clears the inherited global templates, then add a scenario with a per-target severity field and assert the resulting template selection. Preserve omitted Templates behavior separately so explicit empty arrays remain distinguishable.pkg/templates/cluster.go (1)
294-297: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winCompute the matching operator set once per execution.
operatorMatchesTargetFilterruns for every operator on every event. Each call performsfilepath.Clean, a binary search over the effective template paths, and two tag loops. A large cluster combined with many events repeats this work. Compute the allowed operators once beforee.requests.ExecuteWithResultsand iterate that slice inside the callback.♻️ Proposed change for `Execute`
filter := ctx.Input.MetaInput.TargetFilter if !e.MatchesTargetFilter(filter) { return false, nil } + activeOperators := e.operators + if filter != nil { + activeOperators = make([]*clusteredOperator, 0, len(e.operators)) + for _, operator := range e.operators { + if operatorMatchesTargetFilter(operator, filter) { + activeOperators = append(activeOperators, operator) + } + } + }Then replace
for _, operator := range e.operatorswithfor _, operator := range activeOperatorsin the event callback and in the matcher-status fallback loop.Also applies to: 382-384
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/templates/cluster.go` around lines 294 - 297, Compute the filtered operator set once in Execute before e.requests.ExecuteWithResults by applying operatorMatchesTargetFilter to e.operators, then reuse that slice in the event callback and matcher-status fallback loop. Replace both per-event iterations over e.operators with activeOperators while preserving existing filtering behavior.internal/runner/target_filters_test.go (1)
156-186: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for
removeIncludedTagsand per-target severity overrides.The suite covers tags, templates, and forced includes. Two behaviors from
prepareTargetFiltersremain untested:
-include-tagsremoving an inherited or per-target exclude tag.filter.HasSeveritiesreplacing the global-severityselection, including the explicitly empty case that clears it.Both paths change which templates run, so a regression there is user visible.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/runner/target_filters_test.go` around lines 156 - 186, Extend TestPrepareTargetFiltersPreservesInheritanceAndForcedIncludes to cover removeIncludedTags by asserting an inherited or per-target exclude tag is removed when included, and add per-target severity cases verifying filter.HasSeverities replaces the global severity selection, including an explicitly empty severity list that clears the global selection. Use MatchesTemplate assertions that demonstrate the resulting template execution behavior.pkg/templates/cluster_test.go (1)
57-96: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtend the cluster filter test to severity and template restriction.
The test covers tag matching only.
operatorMatchesTargetFilterforwards severity and therestrictTemplatestemplate-path check toMatchesTemplate. Add one case that prepares a filter with a severity list and one that preparesrestrictTemplates=truewith a single template path. Both are cheap to add and cover the paths that decide whether the shared clustered request runs.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/templates/cluster_test.go` around lines 57 - 96, Extend TestClusterExecuterMatchesPerTargetFilters with cases for severity filtering and restricted template paths. Prepare a severity filter and assert operatorMatchesTargetFilter accepts the matching apache operator and rejects nginx; prepare a restrictTemplates filter containing the apache template path and assert only the corresponding operator matches, covering the forwarded MatchesTemplate checks.internal/runner/target_filters.go (1)
22-194: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffConsider splitting
prepareTargetFiltersinto smaller units.The function performs validation, loader widening, global path resolution, per-target resolution, and loader assignment in one 170-line body. Extracting three helpers, for example
validateTargetOverrides,resolveGlobalTemplateSets, andprepareInputFilter, would reduce the branch depth and make the template-selection switch easier to test in isolation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/runner/target_filters.go` around lines 22 - 194, Split prepareTargetFilters into focused helpers for override validation, global/default template path resolution, and per-input filter preparation/assignment. Keep loader widening and final template assignment in prepareTargetFilters, while moving the validation and template-selection switch into named helpers such as validateTargetOverrides, resolveGlobalTemplateSets, and prepareInputFilter without changing behavior.pkg/input/provider/http/multiformat.go (1)
43-43: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate provider-type literal across packages.
targetInputProviderTypehere is"TargetInputProvider", andpkg/input/provider/interface.godeclaresTargetInputProvider = "TargetInputProvider".pkg/corecomparestarget.InputType()against theproviderconstant. If either literal changes, the comparison fails silently and per-target filtering stops working.The
providerpackage imports this package, so this package cannot import it back. Add a compile-time assertion inpkg/input/provider/interface.goto bind the two values.♻️ Proposed guard in pkg/input/provider/interface.go
// keep the provider-type constant in sync with the http provider implementation var _ = map[bool]struct{}{true: {}}[TargetInputProvider == (&http.HttpInputProvider{}).InputType() || true]A simpler option is a unit test in
pkg/input/providerthat asserts a JSONL target provider returnsTargetInputProvider.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/input/provider/http/multiformat.go` at line 43, Bind the provider-type values by adding a compile-time assertion in the existing TargetInputProvider declaration area of provider/interface.go, comparing it with the HTTP provider implementation’s InputType() result. Preserve the package dependency direction by having the provider package reference the HTTP implementation without making the HTTP package import provider, and ensure mismatched values fail compilation.pkg/protocols/common/contextargs/target_filter_test.go (1)
12-19: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd a multi-path case with unsorted input.
Every path slice passed to
Preparehere has one element. A one-element slice is always sorted, so these cases cannot detect the binary-search hazard incontainsSortedPath. Add a case that passes several template paths in unsorted order and asserts each one matches.💚 Proposed additional test
func TestTargetFilterMatchesEveryConfiguredTemplatePath(t *testing.T) { filter := &TargetFilter{} filter.Prepare( nil, nil, nil, []string{"/templates/zeta.yaml", "/templates/apache.yaml", "/templates/mid.yaml"}, nil, true, ) for _, path := range []string{"/templates/zeta.yaml", "/templates/apache.yaml", "/templates/mid.yaml"} { require.True(t, filter.MatchesTemplate(path, nil, severity.High, false), path) } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/protocols/common/contextargs/target_filter_test.go` around lines 12 - 19, Add a test near the existing TargetFilter tests that calls TargetFilter.Prepare with multiple unsorted template paths, then verifies MatchesTemplate returns true for every configured path. Keep the case focused on exercising containsSortedPath with unsorted input and use the existing assertion style.pkg/input/formats/json/json_test.go (1)
100-107: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse
t.Chdirinstead of manual working-directory cleanup.The module requires Go 1.26, so
t.Chdiris available. Replace the latererr =assignment witherr :=after removing the initial declaration.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/input/formats/json/json_test.go` around lines 100 - 107, Update TestJSONFormatterTreatsSeverityAsLiteralValue to use t.Chdir(t.TempDir()) instead of manually capturing and restoring the working directory, removing the previousWorkingDirectory cleanup. After removing the initial err declaration, change the later assignment to use err :=.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/runner/runner.go`:
- Line 686: Update the condition in the runner flow around
r.inputProvider.InputType() so TargetInputProvider also enables implicit DAST,
preserving the existing MultiFormatInputProvider and r.options.DAST behavior.
Ensure target JSONL input loads DAST templates by default.
In `@pkg/input/README.md`:
- Around line 46-54: Update the per-target override documentation to state that
built-in ignore-file tags remain enforced despite target-level exclude-tags, and
global -exclude-severity remains enforced despite target-level severity values.
Clarify that explicitly empty arrays do not override these exclusions.
In `@pkg/protocols/common/contextargs/target_filter.go`:
- Around line 45-63: The sorted-path invariant is not guaranteed for template
matching. In pkg/protocols/common/contextargs/target_filter.go lines 45-63,
update TargetFilter.Prepare to defensively copy, filepath.Clean, and sort both
templatePaths and includeTemplatePaths before storing them in
preparedTargetFilter; in pkg/protocols/common/contextargs/target_filter_test.go
lines 12-19, add coverage using multiple unsorted template paths and assert each
path matches.
---
Nitpick comments:
In `@internal/runner/preflight_portscan_test.go`:
- Around line 122-135: Update the filteringInputProvider test setup so allowCnt
is derived via countAllowedPreflightInputs rather than hard-coded as 2, allowing
the Count() assertion to exercise the production-derived value. Keep the
existing Iterate behavior and assertions unchanged.
In `@internal/runner/preflight_portscan.go`:
- Around line 55-66: The filteringInputProvider.Iterate path repeatedly computes
preflightInputKey for the same MetaInput across template passes, causing
redundant allocation and marshaling. Add caching keyed by
*contextargs.MetaInput, or compute and retain each key during preflight
collection, then reuse it in Iterate while preserving the existing allowed-key
filtering and callback behavior.
In `@internal/runner/target_filters_test.go`:
- Around line 156-186: Extend
TestPrepareTargetFiltersPreservesInheritanceAndForcedIncludes to cover
removeIncludedTags by asserting an inherited or per-target exclude tag is
removed when included, and add per-target severity cases verifying
filter.HasSeverities replaces the global severity selection, including an
explicitly empty severity list that clears the global selection. Use
MatchesTemplate assertions that demonstrate the resulting template execution
behavior.
In `@internal/runner/target_filters.go`:
- Around line 22-194: Split prepareTargetFilters into focused helpers for
override validation, global/default template path resolution, and per-input
filter preparation/assignment. Keep loader widening and final template
assignment in prepareTargetFilters, while moving the validation and
template-selection switch into named helpers such as validateTargetOverrides,
resolveGlobalTemplateSets, and prepareInputFilter without changing behavior.
In `@internal/tests/integration/target_jsonl_test.go`:
- Around line 100-137: The request-count assertion in the strategy/scenario
integration test is coupled to the current clustering behavior. Update the
assertion around requestCount.Load to derive the expected count from each
scenario when possible; otherwise explicitly state in the failure message that
exactly two requests is the clustering precondition, so changes to clustering or
template count are clearly diagnosed.
- Around line 26-31: Add integration coverage in the target JSONL test scenarios
using targetJSONLRecord: include a record whose Templates points to an
explicitly empty slice and verify it clears the inherited global templates, then
add a scenario with a per-target severity field and assert the resulting
template selection. Preserve omitted Templates behavior separately so explicit
empty arrays remain distinguishable.
In `@pkg/core/executors_test.go`:
- Around line 247-269: The tests hardcode the target provider type instead of
using the shared constant. In pkg/core/executors_test.go lines 247-269, import
pkg/input/provider and replace all three "TargetInputProvider" literals with
provider.TargetInputProvider; in internal/runner/target_filters_test.go line 40,
return provider.TargetInputProvider from InputType, using the existing import.
- Around line 247-249: Replace the literal "TargetInputProvider" with
provider.TargetInputProvider in the fakeTargetProvider initializations for
deniedOnly and the additional referenced cases. Import the provider package and
use the constant consistently so these tests track the canonical provider name.
In `@pkg/input/formats/json/json_test.go`:
- Around line 100-107: Update TestJSONFormatterTreatsSeverityAsLiteralValue to
use t.Chdir(t.TempDir()) instead of manually capturing and restoring the working
directory, removing the previousWorkingDirectory cleanup. After removing the
initial err declaration, change the later assignment to use err :=.
In `@pkg/input/provider/http/multiformat.go`:
- Line 43: Bind the provider-type values by adding a compile-time assertion in
the existing TargetInputProvider declaration area of provider/interface.go,
comparing it with the HTTP provider implementation’s InputType() result.
Preserve the package dependency direction by having the provider package
reference the HTTP implementation without making the HTTP package import
provider, and ensure mismatched values fail compilation.
In `@pkg/protocols/common/contextargs/target_filter_test.go`:
- Around line 12-19: Add a test near the existing TargetFilter tests that calls
TargetFilter.Prepare with multiple unsorted template paths, then verifies
MatchesTemplate returns true for every configured path. Keep the case focused on
exercising containsSortedPath with unsorted input and use the existing assertion
style.
In `@pkg/templates/cluster_test.go`:
- Around line 57-96: Extend TestClusterExecuterMatchesPerTargetFilters with
cases for severity filtering and restricted template paths. Prepare a severity
filter and assert operatorMatchesTargetFilter accepts the matching apache
operator and rejects nginx; prepare a restrictTemplates filter containing the
apache template path and assert only the corresponding operator matches,
covering the forwarded MatchesTemplate checks.
In `@pkg/templates/cluster.go`:
- Around line 294-297: Compute the filtered operator set once in Execute before
e.requests.ExecuteWithResults by applying operatorMatchesTargetFilter to
e.operators, then reuse that slice in the event callback and matcher-status
fallback loop. Replace both per-event iterations over e.operators with
activeOperators while preserving existing filtering behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 12916cf8-928e-43fc-a2be-655895ba064d
📒 Files selected for processing (22)
internal/runner/preflight_portscan.gointernal/runner/preflight_portscan_test.gointernal/runner/runner.gointernal/runner/target_filters.gointernal/runner/target_filters_test.gointernal/tests/integration/target_jsonl_test.gopkg/core/execute_options.gopkg/core/executors.gopkg/core/executors_test.gopkg/input/README.mdpkg/input/formats/formats.gopkg/input/formats/json/json.gopkg/input/formats/json/json_test.gopkg/input/provider/http/multiformat.gopkg/input/provider/http/multiformat_target_test.gopkg/input/provider/interface.gopkg/protocols/common/contextargs/metainput.gopkg/protocols/common/contextargs/metainput_test.gopkg/protocols/common/contextargs/target_filter.gopkg/protocols/common/contextargs/target_filter_test.gopkg/templates/cluster.gopkg/templates/cluster_test.go
![neo-by-projectdiscovery-dev[bot]](https://avatars.githubusercontent.com/in/2878468?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
pkg/protocols/common/contextargs/target_filter_test.go (1)
95-113: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStrengthen the preparation test assertions.
This test verifies matching outcomes, but it does not directly verify the normalized and sorted
effectiveTemplatesoreffectiveIncludescollections. It also tests forced inclusions without restrictive tags, exclusions, or severities, so it does not prove that inclusions override those filters. Assert the prepared collections directly and add restrictive criteria to the forced-path cases.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/protocols/common/contextargs/target_filter_test.go` around lines 95 - 113, Strengthen TestTargetFilterPrepareNormalizesTemplatePaths by directly asserting the sorted, normalized effectiveTemplates and effectiveIncludes collections after Prepare. Update the forced-path MatchesTemplate cases to use restrictive tags, exclusions, and severities, verifying forced inclusions still match despite those filters, while retaining the non-matching template assertion.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@pkg/protocols/common/contextargs/target_filter_test.go`:
- Around line 95-113: Strengthen TestTargetFilterPrepareNormalizesTemplatePaths
by directly asserting the sorted, normalized effectiveTemplates and
effectiveIncludes collections after Prepare. Update the forced-path
MatchesTemplate cases to use restrictive tags, exclusions, and severities,
verifying forced inclusions still match despite those filters, while retaining
the non-matching template assertion.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 2cf1fd5d-4dc8-49b6-af59-fc1e3ab0ae99
📒 Files selected for processing (3)
pkg/input/README.mdpkg/protocols/common/contextargs/target_filter.gopkg/protocols/common/contextargs/target_filter_test.go
🚧 Files skipped from review as they are similar to previous changes (2)
- pkg/input/README.md
- pkg/protocols/common/contextargs/target_filter.go
|
Addressed the remaining CodeRabbit nitpick in eb1b14c: |
Reject absolute paths and parent-directory traversal in per-target JSONL `templates` selectors so a targets file cannot direct the loader at arbitrary local files (CWE-22). Global -t/-it selectors remain unrestricted trusted CLI input.
1 participant
Proposed changes
Closes #4934.
This adds a target-oriented JSONL mode in which each URL can override
tags,exclude-tags,severity, and localtemplateswhile omitted fields inherit the corresponding global options:{"url":"https://target-a.example","tags":["apache","shiro"],"severity":["critical","high"]} {"url":"https://target-b.example","exclude-tags":["tomcat"],"templates":["http/cves/2026/"]} {"url":"https://target-c.example"}nuclei -l targets.jsonl -input-mode jsonlThe implementation:
-exclude-hostsbefore caching target records;MetaInputIDs, hashes, preflight keys, and multiformat behavior unchanged when no target overrides are present.For combinations whose child execution cannot yet preserve independent target criteria, the runner returns a clear error instead of silently bypassing filters. This currently covers automatic scan, workflows, and global matchers. Per-target template selectors are local-only and cannot be combined with global remote template URLs.
An explicitly empty field clears the inherited CLI selection. For
templates, this is equivalent to clearing-tand therefore falls back to the default template catalog.This is a draft so maintainers can confirm those boundary semantics before it is marked ready.
Proof
go test -race ./internal/runner ./pkg/input/provider/http ./pkg/input/formats/json ./pkg/protocols/common/contextargs ./pkg/core ./pkg/templatesgo test -tags=integration ./internal/tests/integration -run '^TestTargetJSONLTemplateFilters$' -count=1 -vgo vet -tags=integration ./internal/tests/integrationmake vetmake buildgit diff --checkThe complete local
make testrun reached four packages whose Interactsh tests could not register with the public OAST service. The same focused failures reproduce on an untouchedupstream/devworktree atbcf20899; all target-filter packages and integration scenarios above pass with the race detector where applicable.Checklist
Summary by CodeRabbit