feat(sdk): mirror -config, -report-config, -dashboard for SDK callers - #7393
Conversation
SDK consumers had to rebuild types.Options by hand or shell out to the CLI to get config-file parity. This shares BindOptionFlags, LoadReportingOptionsFromBytes, ApplyExporterOptionsFromTypes, and SetupPDCPUpload between cmd/nuclei and lib/ so both paths produce identical engine state. WithConfigFile/Bytes apply only YAML-set fields (reflection diff against goflags defaults) so prior With* and DefaultOptions() values survive YAML that omits them. Per-scan ExecuteNucleiWithOpts rejects these options via a new threadSafePerScan mode, matching the pattern for options that only make sense at construction.
WalkthroughThe pull request adds SDK option helpers and wiring for YAML-based configuration and reporting, paired with PDCP cloud upload support. It introduces reporting options YAML parsing, integrates reporting configuration into the engine, defines a RuntimeConfig schema for overlaying scanner settings, and provides helper constructors to load these configurations from files or bytes. ChangesConfiguration and Cloud Upload Infrastructure
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
- Inline (*Runner).setupPDCPUpload — wrapper just delegated to the package-level SetupPDCPUpload. Call site inlines the three lines. - Dedupe reporting-config file load: WithReportingConfigFile and loadImplicitReportingConfig share loadReportingConfigFromPath. - Move config-overlay/load helpers (newConfigFlagSet, overlayConfigFromFile, applyOverlay, loadReportingConfigFromPath, loadImplicitReportingConfig) out of lib/config.go into lib/config_load.go. - Drop threadSafePerScan engine mode and isThreadSafe helper. The four new With* options no longer gate per-scan use. pd-agent doesn't enter thread-safe mode; reintroducing the gate ships as a follow-up PR with its own scope.
- L1: sdk_private.go references SetupPDCPUpload (was lowercase). - L2: config_test.go test comment describes how tmpEngine is constructed rather than incorrectly claiming "inherits from parent". - Trim godocs and inline comments across the PR — drop restated WHAT, keep WHY and limitations.
Extracted into flags.go from cmd/nuclei/main.go but the trailing //nolint:all was dropped — golangci-lint trips on SA1019 in the SDK-shared path. Restoring matches the dev branch line.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/runner/options.go (1)
377-379:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPreserve reporting YAML
omit-rawunless explicitly overriddenLine 377 always overwrites
reportingOptions.OmitRawwithoptions.OmitRawRequests(defaultfalse), which can silently dropomit-raw: trueloaded from-report-config.Suggested fix
- reportingOptions.OmitRaw = options.OmitRawRequests + if options.OmitRawRequests { + reportingOptions.OmitRaw = true + } reportingOptions.ExecutionId = options.ExecutionId🤖 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/options.go` around lines 377 - 379, The code unconditionally assigns reportingOptions.OmitRaw = options.OmitRawRequests which overwrites any omit-raw value loaded from the report YAML; change this to only overwrite reportingOptions.OmitRaw when the CLI flag was explicitly provided (e.g., check a corresponding "was set" indicator for options.OmitRawRequests or change the option to a pointer/tristate and test for nil), otherwise leave reportingOptions.OmitRaw untouched; update the assignment near reportingOptions.OmitRaw / options.OmitRawRequests so that existing YAML-loaded omit-raw is preserved unless the user explicitly set the flag.
🤖 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`:
- Around line 481-483: The branch that checks opts.EnableCloudUpload and global
EnableCloudUpload is using pdcpUploadErrMsg to return a user-visible message for
the normal default path; change that to return an empty string when both are
false (i.e., when the user did not request cloud upload) so
displayExecutionInfo() won’t print "Scan results upload to cloud is disabled."
instead of the usual dashboard hint; ensure pdcpUploadErrMsg is only
set/returned in real error paths where upload was requested but setup failed
(the code around opts.EnableCloudUpload, EnableCloudUpload and the current
return statement should be updated accordingly).
In `@lib/config.go`:
- Around line 560-649: These option constructors (WithPDCPUpload,
WithConfigFile, WithConfigBytes, WithReportingConfigFile,
WithReportingConfigBytes) must refuse to run when the engine is in per‑scan
threadSafe mode; update each returned function to check e.mode == threadSafe at
the top and immediately return ErrOptionsNotSupported instead of mutating
e.opts, e.reportingOpts, calling overlayConfigFromFile or
loadImplicitReportingConfig so these changes cannot be applied after
initialization (preserve existing error wrapping where applicable).
---
Outside diff comments:
In `@internal/runner/options.go`:
- Around line 377-379: The code unconditionally assigns reportingOptions.OmitRaw
= options.OmitRawRequests which overwrites any omit-raw value loaded from the
report YAML; change this to only overwrite reportingOptions.OmitRaw when the CLI
flag was explicitly provided (e.g., check a corresponding "was set" indicator
for options.OmitRawRequests or change the option to a pointer/tristate and test
for nil), otherwise leave reportingOptions.OmitRaw untouched; update the
assignment near reportingOptions.OmitRaw / options.OmitRawRequests so that
existing YAML-loaded omit-raw is preserved unless the user explicitly set the
flag.
🪄 Autofix (Beta)
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
Run ID: de318506-eba0-4b0e-9b42-eb2e0e87ef78
📒 Files selected for processing (11)
cmd/nuclei/main.gointernal/runner/flags.gointernal/runner/options.gointernal/runner/runner.golib/config.golib/config_load.golib/config_test.golib/example_test.golib/internal_test.golib/sdk.golib/sdk_private.go
DisableUpdateCheck() mutates DefaultConfig process-globally. config_test.go files run alphabetically before sdk_test.go, so my tests were disabling the update-check globally and TestContextCancelNucleiEngine (which depends on template auto-install) was hitting an empty template store on first run. Drop the option from the seven test sites. First test in the binary now triggers template install (sync.Once gated; subsequent tests free).
…scans SetupPDCPUpload returned a non-empty status string in the upload-disabled branch, which displayExecutionInfo then logs as a Warning on every nuclei run that doesn't request -dashboard. That also suppresses the friendlier dashboard hint Info line. Return empty for the default-disabled case so the Info hint fires. The latent issue predates this PR; CodeRabbit surfaced it on the refactor.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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`:
- Around line 497-500: The code silently discards the error returned by
uploadWriter.SetScanID when opts.ScanID is provided; instead, check the returned
error from uploadWriter.SetScanID(opts.ScanID) and handle it (return the error
up the call stack or log and return a wrapped error) so callers are informed
when an invalid ScanID is supplied; update the surrounding function (the caller
of uploadWriter.SetScanID in runner.go) to propagate a meaningful error message
including opts.ScanID and the underlying error rather than ignoring it.
🪄 Autofix (Beta)
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
Run ID: 09c9becd-b315-4a51-8592-7279a926ec56
📒 Files selected for processing (2)
internal/runner/runner.golib/config_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/config_test.go
dogancanbakir
left a comment
There was a problem hiding this comment.
-h outputs twice, also we should aim minimal change to expose those funcs in lib. discussed internally as well.
The BindOptionFlags refactor of readConfig() caused duplicate group headers in `nuclei -h` (CLI-only flags appended new same-named groups after the shared ones). Suppressing the disabled-message on the CLI was also a behavior change. Both are scope creep. This PR is for SDK additions, not CLI refactor. - Revert cmd/nuclei/main.go to dev. `nuclei -h` byte-identical. - Restore "Scan results upload to cloud is disabled." in SetupPDCPUpload. - flags.go stays for SDK-only use via lib/config_load.go.
WithConfigFile/Bytes now decode YAML directly into a typed RuntimeConfig struct and apply it via MergeOptions(opts). The goflags-backed reflection overlay had too much surface area (266-line flag inventory duplicate plus a flag-default vs DefaultOptions diff) for the actual SDK use case, which is accepting a known set of scan knobs from cloud-shipped YAML. RuntimeConfig schema covers tags/severity filtering, headers/vars, interactsh, socks5-proxy, plus rate-limit/bulk-size/concurrency/timeout/ retries/rate-limit-host. Scalar knobs use *int so omitted keys preserve the engine's existing value. Drops internal/runner/flags.go (266 lines) and lib/config_load.go (86 lines). Reverts internal/runner/runner.go to dev (no SetupPDCPUpload export; SDK inlines its own PDCP wrap). Keeps the LoadReportingOptionsFromBytes extract in internal/runner/options.go for WithReportingConfigFile/Bytes.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@lib/config.go`:
- Around line 616-668: The MergeOptions method currently swallows errors from
validation calls (calls to opts.Severities.Set, opts.ExcludeSeverities.Set,
opts.Protocols.Set, opts.ExcludeProtocols.Set, opts.Vars.Set) which lets invalid
runtime config silently pass; change RuntimeConfig.MergeOptions to return error,
propagate and return any Set() errors (either the first encountered or an
aggregated error) instead of assigning to _, and update its caller
applyRuntimeConfigFromBytes to handle/return that error so invalid values fail
fast; keep all other merging logic the same but ensure every place that
previously used `_ = ...Set(...)` now checks and returns the error.
🪄 Autofix (Beta)
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
Run ID: 5a9fbbdb-d6ac-4dd1-b0b8-c2d713264426
📒 Files selected for processing (6)
internal/runner/options.gointernal/runner/runner.golib/config.golib/config_test.golib/example_test.golib/sdk_private.go
✅ Files skipped from review due to trivial changes (1)
- internal/runner/runner.go
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/example_test.go
Summary
Closes the SDK gap where embedding nuclei via
lib/couldn't ingest the same-config/-report-configYAMLs or push results to the PDCP dashboard the way the CLI does. SDK consumers like pd-agent had to either rebuild*types.Optionsby hand, hand-roll the dashboard import API, or shell out to the CLI binary. This PR exposes:WithConfigFile(path string)/WithConfigBytes(data []byte)— equivalent to-config.WithReportingConfigFile(path string)/WithReportingConfigBytes(data []byte)— equivalent to-report-config.WithPDCPUpload(scanID, teamID string)— equivalent to-dashboard -scan-id $ID -team-id $T.So pd-agent now does:
Shared helpers (cmd/nuclei + lib/)
To keep one source of truth, four pieces were lifted from
cmd/nuclei/main.go/internal/runner/and shared with the SDK:runner.BindOptionFlags(fs, opts)— every*types.Options-bound flag (CLI-only callbacks-version,-update,-auth,-config,-profile, etc. stay inline inmain.go).runner.LoadReportingOptionsFromBytes(data)— YAML parse + env-var expansion for reporting config.runner.ApplyExporterOptionsFromTypes(rOpts, opts)— wiresmarkdown-export,sarif-export,json-export,jsonl-export,pdf-export,report-dbfrom*types.Optionsinto*reporting.Options(CLI did this; SDK didn't).runner.SetupPDCPUpload(ctx, logger, opts, writer)— upload-writer wrap, returns the original writer + a human-readable status string on failure.Semantics worth knowing
WithConfigFile/Bytesfield merge. A naïve goflags bind would overwritetypes.DefaultOptions()and any priorWith*value because flag registration writes defaults into the bound struct. Instead we build abaseline(flag-defaults only) and anoverlay(flag-defaults + YAML), then reflect-diff the two and copy only fields the YAML actually touched.Timeout=5,ResponseReadSize=10MB, etc. survive YAML that doesn't mention them.Known limitation (documented in godoc): if YAML sets a key to a value equal to the goflags default, the diff cannot distinguish it from "not set" and the value is silently dropped. Workaround: use the explicit
With*instead.ThreadSafe per-scan rejection. Per-scan
ExecuteNucleiWithOptsrejects the four new options via a newthreadSafePerScanengine mode, matching the existingErrOptionsNotSupportedpattern for options that only make sense at engine construction. Added anisThreadSafe()helper so all 9 existing gates broadened to cover the new mode without duplicating the check.Reporting wire-up.
lib/sdk_private.gopreviously hardcodedreporting.New(&reporting.Options{}, "", false)— exporters andreport-dbwere silently dead. NowApplyExporterOptionsFromTypesruns at init and the second arg ise.opts.ReportingDB. Strictly additive for callers who didn't set these (everyone got the empty/""default before).Compatibility
Strictly additive. No CLI flag rename / default change / shortname change (verified by flag-by-flag diff of
nuclei -h; group placement of CLI-only flags is duplicated but every flag is reachable with the same name/shortname/default).Test plan
go build ./...cleango vet ./...cleango test -count=1 -short ./lib/... ./internal/runner/...passesTestWithConfigFile_PreservesDefaults— DefaultOptions values survive empty YAMLTestWithConfigFile_DoesNotClobberPriorOptions— priorWith*survivesWithConfigFileTestPerScanOptions_RejectIncompatibleOptions—WithVerbosityfrom per-scan path rejected withErrOptionsNotSupportedTestWithReportingConfigBytes_InvalidYAML— useful error on bad YAMLTestWithReportingConfigFile/TestWithReportingConfigBytes— GitHub tracker fields land correctly one.reportingOptsnuclei -hflag inventory unchanged (194 flags both sides)Files touched
cmd/nuclei/main.go—readConfigdelegates torunner.BindOptionFlagsinternal/runner/flags.go(new),internal/runner/options.go,internal/runner/runner.go— shared helperslib/config.go— five newWith*options +applyOverlay/overlayConfigFromFile/loadImplicitReportingConfiglib/sdk.go—reportingOptsfield,threadSafePerScanmode,isThreadSafe()helperlib/sdk_private.go—ApplyExporterOptionsFromTypes+ReportingDBwired into init, PDCP wraplib/multi.go— tmpEngine mode =threadSafePerScanlib/example_test.go—ExampleWithPDCPUpload,ExampleWithConfigFilelib/config_test.go(new),lib/internal_test.go(new) — regression + parity testsSummary by CodeRabbit
New Features
Tests