-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Handle DisableUnsignedTemplates option #6524
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Conversation
WalkthroughRestricts skipping of unverified Code-protocol templates so they are skipped only when Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Loader as CatalogLoader
participant Template as TemplateMeta
participant Opts as Options
Loader->>Template: read metadata (RequestsCode, Verified, Workflows)
Loader->>Opts: read DisableUnsignedTemplates
alt RequestsCode && !Verified && no Workflows && DisableUnsignedTemplates
Loader-->>Template: skip template
else
Loader-->>Template: load template
end
sequenceDiagram
autonumber
participant Test as IntegrationTest
participant Runner as RunNucleiArgsWithEnv...
Test->>Runner: build args (insert "-dut", then "-t", ...)
Runner-->>Test: execute nuclei with args
Note right of Runner: previously args lacked "-dut"
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 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 |
Option DisableUnsignedTemplates (CLI -dut) disables "running unsigned templates or templates with mismatched signature" according to the documentation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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/catalog/loader/loader.go (1)
560-564
: Critical: Modify this check to allow Code templates with workflows to pass through.The general unsigned template check here returns early for ALL unverified templates when
DisableUnsignedTemplates
is enabled. However, based on the PR objectives and the change at line 608, the intent is to allow unverified Code templates that have workflows to load.The current implementation has a logic flaw: line 608's new condition
len(parsed.Workflows) == 0
will never be evaluated for unverified Code templates because this check (lines 560-564) returns early, preventing execution from reaching line 608.Apply this diff to allow Code templates with workflows to bypass this check:
if !parsed.Verified && store.config.ExecutorOptions.Options.DisableUnsignedTemplates { - // skip unverified templates when prompted to - stats.Increment(templates.SkippedUnsignedStats) - return + // skip unverified templates when prompted to, except Code templates with workflows + if len(parsed.RequestsCode) > 0 && len(parsed.Workflows) > 0 { + // Allow unsigned code templates with workflows to continue + } else { + stats.Increment(templates.SkippedUnsignedStats) + return + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
cmd/integration-test/code.go
(2 hunks)pkg/catalog/loader/loader.go
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go
: Format Go code using go fmt
Run static analysis with go vet
Files:
pkg/catalog/loader/loader.go
cmd/integration-test/code.go
🧬 Code graph analysis (2)
pkg/catalog/loader/loader.go (1)
pkg/types/types.go (1)
Options
(34-472)
cmd/integration-test/code.go (1)
pkg/testutils/integration.go (1)
RunNucleiArgsWithEnvAndGetResults
(166-202)
⏰ 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). (3)
- GitHub Check: Tests (ubuntu-latest)
- GitHub Check: Tests (macOS-latest)
- GitHub Check: Tests (windows-latest)
🔇 Additional comments (1)
cmd/integration-test/code.go (1)
140-140
: LGTM! Test changes correctly validate unsigned template rejection.The addition of
-dut
flag to bothunsignedCode
andcodePyNoSig
test cases properly validates that theDisableUnsignedTemplates
option rejects unsigned templates as expected. The tests correctly expect an error when unsigned templates are executed with this flag enabled.Also applies to: 155-155
store.logger.Print().Msgf("[%v] Code flag is required for code protocol template '%s'.\n", aurora.Yellow("WRN").String(), templatePath) | ||
} | ||
} else if len(parsed.RequestsCode) > 0 && !parsed.Verified && len(parsed.Workflows) == 0 { | ||
} else if len(parsed.RequestsCode) > 0 && !parsed.Verified && len(parsed.Workflows) == 0 && store.config.ExecutorOptions.Options.DisableUnsignedTemplates { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Verify the intended behavior for Code templates with workflows.
The added condition len(parsed.Workflows) == 0
aims to allow Code templates with workflows to load even when unsigned. However, due to the early return at line 560-564, this condition is currently unreachable for unverified templates.
After fixing line 560-564 as suggested, verify this logic works as intended:
🏁 Script executed:
#!/bin/bash
# Description: Search for templates with both RequestsCode and Workflows to understand the use case
# Find templates that might have both code requests and workflows
rg -nP -A 5 -B 5 --type=go 'RequestsCode.*Workflows|Workflows.*RequestsCode'
# Look for workflow-related logic in template structures
ast-grep --pattern $'type Template struct {
$$$
RequestsCode $_
$$$
Workflows $_
$$$
}'
Length of output: 1165
🏁 Script executed:
#!/bin/bash
# Inspect early return in loader.go around line 560
sed -n '550,580p' pkg/catalog/loader/loader.go
# Find Template struct definitions across pkg/catalog
rg -nP --type=go 'type\s+Template\s+' -n pkg/catalog
Length of output: 1469
Revise early-return to allow unsigned templates with workflows
The if !parsed.Verified && DisableUnsignedTemplates { … return }
at lines 560–564 skips all unverified templates, making the later else if len(parsed.RequestsCode)>0 && !parsed.Verified && len(parsed.Workflows)==0 && DisableUnsignedTemplates
branch unreachable. Change the early-return to only skip when both len(parsed.Workflows)==0
and len(parsed.RequestsCode)==0
, so unsigned templates with workflows can load.
🤖 Prompt for AI Agents
In pkg/catalog/loader/loader.go around line 608 (and the early-return at lines
~560–564), the current early-return skips all unverified templates when
DisableUnsignedTemplates is true, making the later branch at line 608
unreachable; change the early-return condition so it only returns when the
template is unverified, DisableUnsignedTemplates is true, AND both
len(parsed.Workflows) == 0 AND len(parsed.RequestsCode) == 0, allowing
unverified templates that contain workflows to continue loading while still
skipping completely empty unsigned templates.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR! I think this case should be already covered as mentioned by @coderabbitai at line 560-564 as the conditions should cause an early return, making the changed branch unreacheable
Proposed changes
Option
DisableUnsignedTemplates
(CLI-dut
) should "disable running unsigned templates or templates with mismatched signature" according to the documentation.Checklist
Summary by CodeRabbit