Open
Conversation
fix WRN output routing & clean up some message formatting. Use `Warning()` method instead of manually tagged `Print()` method warnings in the runner and template- loading paths, which avoids mixing human-readable warnings into the plain `Print()` output path. Closes: #7314 Also, normalize logger calls at the same time by dropping redundant newline suffixes, trimming trivial `Msgf` wrappers and making user-facing log messages more consistent across the tree. Signed-off-by: Dwi Siswanto <git@dw1.io>
Neo - PR Security ReviewNo security issues found Comment |
Contributor
WalkthroughThis pull request standardizes logging output formatting across the codebase by removing trailing newlines from log messages, replacing colored aurora output with native logger methods, switching from unquoted to quoted format specifiers, and adjusting message capitalization and phrasing for consistency. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/runner/templates.go (1)
21-30:⚠️ Potential issue | 🔴 CriticalHandle parse errors before type assertion to avoid panic
ParseTemplateerrors are checked after assertingt.(*templates.Template). On parse failure, this can panic before logging, which is a hard crash in runner flow.Suggested fix
func (r *Runner) logAvailableTemplate(tplPath string) { t, err := r.parser.ParseTemplate(tplPath, r.catalog) + if err != nil { + r.Logger.Error().Msgf("Could not parse file '%s': %s", tplPath, err) + return + } + tpl, ok := t.(*templates.Template) if !ok { - panic("not a template") + r.Logger.Error().Msgf("Could not parse file '%s': unexpected template type", tplPath) + return } - if err != nil { - r.Logger.Error().Msgf("Could not parse file '%s': %s", tplPath, err) - } else { - r.verboseTemplate(tpl) - } + r.verboseTemplate(tpl) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/runner/templates.go` around lines 21 - 30, The code currently asserts t.(*templates.Template) before checking err from r.parser.ParseTemplate, which can panic on parse failure; move the error check to immediately after calling r.parser.ParseTemplate (check if err != nil and log via r.Logger.Error with tplPath and err, then return or skip further processing), and only perform the type assertion to templates.Template after confirming err is nil and t is non-nil; if the assertion fails, log a clear error instead of panicking and avoid calling r.verboseTemplate unless you have a valid *templates.Template.
🧹 Nitpick comments (7)
pkg/protocols/common/automaticscan/automaticscan.go (2)
177-177: Trailing newline inconsistent with other changes.Other log messages in this PR had trailing
\nremoved, but this one still has it. For consistency, consider removing it.Suggested fix
- gologger.Print().Msgf("Final tags identified for %q: %+v\n", input.Input, finalTags) + gologger.Print().Msgf("Final tags identified for %q: %+v", input.Input, finalTags)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/protocols/common/automaticscan/automaticscan.go` at line 177, The log call in automaticscan.go using gologger.Print().Msgf("Final tags identified for %q: %+v\n", input.Input, finalTags) has a trailing newline inconsistent with other PR changes; update the Msgf format string in that code path (the gologger.Print().Msgf invocation) to remove the trailing "\n" so it matches the other log messages' formatting.
172-175: Consider retaining the target in the warning message.The simplified message loses context about which target had no tags found. When running automatic scan against multiple targets, this could make debugging more difficult.
Suggested improvement
if len(finalTags) == 0 { - gologger.Warning().Msg("Skipping automatic scan since no tags were found") + gologger.Warning().Msgf("Skipping automatic scan for %q since no tags were found", input.Input) return }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/protocols/common/automaticscan/automaticscan.go` around lines 172 - 175, The warning discards which target had no tags; update the log to include the target identifier when finalTags is empty. In the block that checks len(finalTags) == 0 (referencing finalTags and gologger.Warning().Msg), change the log call to include the target variable (e.g., target, host, or req.Target) using a formatted message (Msgf or fmt.Sprintf) so the warning reads like "Skipping automatic scan for %s since no tags were found" and then return unchanged.pkg/protocols/dns/request.go (1)
169-169: Consider using%qfor consistency.Other log messages in this PR use
%qfor domain/question values (lines 126, 140, 144), but this line uses%s. Using%qwould maintain consistency and provide clearer output when the question value contains special characters.♻️ Suggested change
- gologger.Verbose().Msgf("[%s] Sent DNS request to %s", request.options.TemplateID, question) + gologger.Verbose().Msgf("[%s] Sent DNS request to %q", request.options.TemplateID, question)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/protocols/dns/request.go` at line 169, The log call using gologger.Verbose().Msgf currently formats the question with %s; change the format specifier to %q so the message uses quoted/escaped output for request.options.TemplateID and question (i.e., update the call in the DNS request send path where gologger.Verbose().Msgf("[%s] Sent DNS request to %s", request.options.TemplateID, question) is invoked to use %q for the question argument to match other logs and handle special characters consistently).pkg/protocols/code/code.go (1)
353-356: Prefer removing embedded\nfrom logger messages for consistency.These lines still hardcode extra newlines, which works but drifts from the PR’s normalization pattern.
Suggested cleanup
- msg := fmt.Sprintf("[%s] Dumped code execution for %s\n\n", request.options.TemplateID, input.MetaInput.Input) + msg := fmt.Sprintf("[%s] Dumped code execution for %s", request.options.TemplateID, input.MetaInput.Input) if request.options.Options.Debug || request.options.Options.DebugResponse { gologger.Debug().Msg(msg) - gologger.Print().Msgf("%s\n\n", responsehighlighter.Highlight(event.OperatorsResult, dataOutputString, request.options.Options.NoColor, false)) + gologger.Print().Msgf("%s", responsehighlighter.Highlight(event.OperatorsResult, dataOutputString, request.options.Options.NoColor, false)) if gOutput.Stderr.Len() > 0 { gologger.Debug().Msgf("[%s] Code STDERR:\n%s", request.options.TemplateID, gOutput.Stderr.String()) } } @@ - gologger.Debug().Msgf(" [%v] Pre-condition code:\n\n%v", templateId, strings.Join(final, "\n")) + gologger.Debug().Msgf("[%v] Pre-condition code:\n%v", templateId, strings.Join(final, "\n"))Also applies to: 476-476
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/protocols/code/code.go` around lines 353 - 356, The log messages embed trailing "\n" characters (e.g., the formatted msg built with request.options.TemplateID and input.MetaInput.Input and the gologger.Print().Msgf call that wraps responsehighlighter.Highlight(event.OperatorsResult, dataOutputString, request.options.Options.NoColor, false)); remove the hardcoded "\n" characters and rely on the logger to manage line breaks (i.e., stop appending "\n" to the fmt.Sprintf result and to the Msgf format string) and apply the same change to the other occurrence mentioned (the later similar print at the other location).pkg/protocols/network/request.go (1)
401-401: Use quoted address formatting here for consistencyLine 401 still uses
%sforactualAddress, while other updated paths use%q. Switching this one keeps formatting consistent and avoids control-character leakage in logs.Suggested patch
- gologger.Verbose().Msgf("Sent network request to %s", actualAddress) + gologger.Verbose().Msgf("Sent network request to %q", actualAddress)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/protocols/network/request.go` at line 401, Update the gologger call that logs the request address to use quoted formatting: locate the gologger.Verbose().Msgf invocation that passes actualAddress (the logging call in request.go) and change its format verb from %s to %q so the address is logged in quotes (matching other paths) and preventing control-character leakage in logs.pkg/input/formats/openapi/generator.go (1)
62-62: Remove trailing newline from fatal log message for consistency.Line 62 still appends
\nwhile nearby logging has been normalized to omit explicit newlines.Proposed diff
- gologger.Fatal().Msgf("openapi: Missing global auth parameter: %s\n", param.Value.Name) + gologger.Fatal().Msgf("openapi: Missing global auth parameter: %s", param.Value.Name)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/input/formats/openapi/generator.go` at line 62, The fatal log call that prints a missing global auth parameter currently includes an explicit newline; locate the gologger.Fatal().Msgf call that formats "openapi: Missing global auth parameter: %s\n" (used when validating param.Value.Name) and remove the trailing "\n" so the message becomes "openapi: Missing global auth parameter: %s" to match the normalized logging style.pkg/protocols/javascript/js.go (1)
596-596: Consider avoiding multiline debug payload in a single log message.
"\n%v"makes this entry multi-line, which is harder for line-oriented log processors. Prefer structured fields or separate debug calls for header/body.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/protocols/javascript/js.go` at line 596, The log currently builds a multiline message into msg using fmt.Sprintf with "\n%v" and vardump.DumpVariables(results); change this to emit a single-line header message and move the large payload into a separate structured field or a separate debug call: keep the header using requestOptions.TemplateID and input.MetaInput.Input in the primary log, and log vardump.DumpVariables(results) either as a separate logger.Debug call or as a structured field (e.g., "dump": <payload>) so the output is line-oriented and avoids embedded newlines.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/nuclei/main.go`:
- Line 125: The error log calls f.Name() after os.Create failed, which can
dereference nil; update the profile creation error handling in the profile
creation block so you do not access f when err != nil: check if err != nil and
log the intended filename/path (use the existing target path variable or the
string passed to os.Create) instead of f.Name(), or only call f.Name() after
confirming f != nil; ensure the Fatal() call uses the safe name/path and
includes the original err.
In `@cmd/tmc/main.go`:
- Around line 128-130: The current error branch in main (the if err :=
process(opts) { ... } in cmd/tmc/main.go) only logs the failure but leaves the
program exiting with status 0; update that branch to exit with a non-zero status
(e.g., call os.Exit(1)) after logging the error, or alternatively propagate the
error out of main so the runtime returns a non-zero code; ensure you import "os"
if you add os.Exit and reference the process(opts) error path and its logging
(gologger.Error().Msgf) when making the change.
In `@internal/runner/options.go`:
- Around line 102-105: The current code logs filesystem walk errors but always
calls os.Exit(0); change the exit behavior so that when an error occurs during
the template/profile walk you exit with a non-zero status (e.g., os.Exit(1)).
Concretely, in the block using options.Logger.Error().Msg(...) and os.Exit(...)
ensure that the err != nil branch performs the non-zero exit and only successes
return/os.Exit(0); keep the logging via options.Logger.Error().Msg(err.Error())
and adjust the os.Exit call accordingly.
In `@internal/runner/runner.go`:
- Line 491: The pdcpUploadErrMsg string assigned in runner.go includes a
trailing "\n" which causes double line breaks when later emitted via
Logger.Warning().Msg; update the fmt.Sprintf call that sets r.pdcpUploadErrMsg
(using pdcpauth.DashBoardURL and err) to remove the trailing newline so the
message is normalized with the project's other log messages.
In `@pkg/input/provider/list/hmap.go`:
- Line 318: Fix the log line that reads gologger.Info().Msgf("Running uncover
query against for %q", strings.Join(options.UncoverEngine, ",")) by removing the
extra "for" so the message reads either "Running uncover query against %q" (or
"Running uncover query for %q" if preferred); update the string literal in the
gologger.Info().Msgf call that uses strings.Join(options.UncoverEngine, ",") to
reflect the corrected phrase.
In `@pkg/js/global/scripts.go`:
- Around line 83-87: The JS builtin log() currently calls
gologger.DefaultLogger.Info() (in pkg/js/global/scripts.go for the cases
handling plain values and map[string]interface{}) and similarly in
pkg/tmplexec/flow/vm.go; change those Info() calls to Print() so the
debug-oriented log() always emits output regardless of logger level
filtering—update the calls to gologger.DefaultLogger.Print(...) in the log()
implementation(s) that reference Info() to ensure consistent, unfiltered debug
output.
In `@pkg/protocols/offlinehttp/request.go`:
- Around line 104-107: The log lines currently include the full raw payload
`data` in the metadata messages (using `%q`) and duplicate the dump; change
those metadata lines to reference an input identifier instead of `data` (e.g.,
use an available identifier like `request.input.ID` or
`request.options.InputID`) and keep the full dump only in the explicit dump
call; update the gologger.Info/Verbose Msgf calls that use
`request.options.TemplateID` to include the input identifier variable in the
"for/to" context rather than the raw `data`, and remove any redundant payload
duplication in those metadata logs.
---
Outside diff comments:
In `@internal/runner/templates.go`:
- Around line 21-30: The code currently asserts t.(*templates.Template) before
checking err from r.parser.ParseTemplate, which can panic on parse failure; move
the error check to immediately after calling r.parser.ParseTemplate (check if
err != nil and log via r.Logger.Error with tplPath and err, then return or skip
further processing), and only perform the type assertion to templates.Template
after confirming err is nil and t is non-nil; if the assertion fails, log a
clear error instead of panicking and avoid calling r.verboseTemplate unless you
have a valid *templates.Template.
---
Nitpick comments:
In `@pkg/input/formats/openapi/generator.go`:
- Line 62: The fatal log call that prints a missing global auth parameter
currently includes an explicit newline; locate the gologger.Fatal().Msgf call
that formats "openapi: Missing global auth parameter: %s\n" (used when
validating param.Value.Name) and remove the trailing "\n" so the message becomes
"openapi: Missing global auth parameter: %s" to match the normalized logging
style.
In `@pkg/protocols/code/code.go`:
- Around line 353-356: The log messages embed trailing "\n" characters (e.g.,
the formatted msg built with request.options.TemplateID and
input.MetaInput.Input and the gologger.Print().Msgf call that wraps
responsehighlighter.Highlight(event.OperatorsResult, dataOutputString,
request.options.Options.NoColor, false)); remove the hardcoded "\n" characters
and rely on the logger to manage line breaks (i.e., stop appending "\n" to the
fmt.Sprintf result and to the Msgf format string) and apply the same change to
the other occurrence mentioned (the later similar print at the other location).
In `@pkg/protocols/common/automaticscan/automaticscan.go`:
- Line 177: The log call in automaticscan.go using gologger.Print().Msgf("Final
tags identified for %q: %+v\n", input.Input, finalTags) has a trailing newline
inconsistent with other PR changes; update the Msgf format string in that code
path (the gologger.Print().Msgf invocation) to remove the trailing "\n" so it
matches the other log messages' formatting.
- Around line 172-175: The warning discards which target had no tags; update the
log to include the target identifier when finalTags is empty. In the block that
checks len(finalTags) == 0 (referencing finalTags and gologger.Warning().Msg),
change the log call to include the target variable (e.g., target, host, or
req.Target) using a formatted message (Msgf or fmt.Sprintf) so the warning reads
like "Skipping automatic scan for %s since no tags were found" and then return
unchanged.
In `@pkg/protocols/dns/request.go`:
- Line 169: The log call using gologger.Verbose().Msgf currently formats the
question with %s; change the format specifier to %q so the message uses
quoted/escaped output for request.options.TemplateID and question (i.e., update
the call in the DNS request send path where gologger.Verbose().Msgf("[%s] Sent
DNS request to %s", request.options.TemplateID, question) is invoked to use %q
for the question argument to match other logs and handle special characters
consistently).
In `@pkg/protocols/javascript/js.go`:
- Line 596: The log currently builds a multiline message into msg using
fmt.Sprintf with "\n%v" and vardump.DumpVariables(results); change this to emit
a single-line header message and move the large payload into a separate
structured field or a separate debug call: keep the header using
requestOptions.TemplateID and input.MetaInput.Input in the primary log, and log
vardump.DumpVariables(results) either as a separate logger.Debug call or as a
structured field (e.g., "dump": <payload>) so the output is line-oriented and
avoids embedded newlines.
In `@pkg/protocols/network/request.go`:
- Line 401: Update the gologger call that logs the request address to use quoted
formatting: locate the gologger.Verbose().Msgf invocation that passes
actualAddress (the logging call in request.go) and change its format verb from
%s to %q so the address is logged in quotes (matching other paths) and
preventing control-character leakage in logs.
🪄 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: b6975342-0a2e-4de4-bec9-4b99b75a0766
📒 Files selected for processing (58)
cmd/nuclei/main.gocmd/tmc/main.gocmd/tools/signer/main.gointernal/runner/inputs.gointernal/runner/options.gointernal/runner/runner.gointernal/runner/templates.gointernal/server/requests_worker.gopkg/catalog/config/ignorefile.gopkg/catalog/disk/find.gopkg/catalog/loader/loader.gopkg/core/executors.gopkg/core/workpool.gopkg/external/customtemplates/github.gopkg/fuzz/component/body.gopkg/fuzz/component/value.gopkg/fuzz/execute.gopkg/input/formats/json/json.gopkg/input/formats/openapi/generator.gopkg/input/provider/http/multiformat.gopkg/input/provider/interface.gopkg/input/provider/list/hmap.gopkg/installer/template.gopkg/js/compiler/pool.gopkg/js/devtools/tsgen/parser.gopkg/js/global/scripts.gopkg/progress/progress.gopkg/protocols/code/code.gopkg/protocols/common/automaticscan/automaticscan.gopkg/protocols/common/contextargs/contextargs.gopkg/protocols/common/helpers/eventcreator/eventcreator.gopkg/protocols/common/helpers/writer/writer.gopkg/protocols/dns/request.gopkg/protocols/file/find.gopkg/protocols/file/request.gopkg/protocols/headless/engine/page.gopkg/protocols/headless/engine/page_actions.gopkg/protocols/http/build_request.gopkg/protocols/http/request.gopkg/protocols/http/request_fuzz.gopkg/protocols/javascript/js.gopkg/protocols/network/request.gopkg/protocols/offlinehttp/request.gopkg/protocols/ssl/ssl.gopkg/protocols/websocket/websocket.gopkg/protocols/whois/whois.gopkg/reporting/reporting.gopkg/templates/cluster.gopkg/templates/compile.gopkg/templates/log.gopkg/templates/signer/default.gopkg/templates/signer/tmpl_signer.gopkg/templates/tag_filter.gopkg/templates/workflows.gopkg/tmplexec/exec.gopkg/tmplexec/flow/vm.gopkg/tmplexec/generic/exec.gopkg/utils/monitor/monitor.go
| f, err := os.Create(memProfile + ext) | ||
| if err != nil { | ||
| options.Logger.Fatal().Msgf("profile: could not create %s profile %q file: %v", profileType, f.Name(), err) | ||
| options.Logger.Fatal().Msgf("Could not create %s profile %q file: %v", profileType, f.Name(), err) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify createProfileFile uses f.Name() in os.Create error branch.
rg -n -C3 'createProfileFile|os\.Create\(|f\.Name\(\)' cmd/nuclei/main.goRepository: projectdiscovery/nuclei
Length of output: 719
Nil pointer dereference in profile creation error handler.
Line 125: f.Name() is called after a failed os.Create(), but f will be nil and cause a panic before the error is logged.
Suggested fix
createProfileFile := func(ext, profileType string) *os.File {
- f, err := os.Create(memProfile + ext)
+ profilePath := memProfile + ext
+ f, err := os.Create(profilePath)
if err != nil {
- options.Logger.Fatal().Msgf("Could not create %s profile %q file: %v", profileType, f.Name(), err)
+ options.Logger.Fatal().Msgf("Could not create %s profile %q file: %v", profileType, profilePath, err)
}
return f
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| options.Logger.Fatal().Msgf("Could not create %s profile %q file: %v", profileType, f.Name(), err) | |
| createProfileFile := func(ext, profileType string) *os.File { | |
| profilePath := memProfile + ext | |
| f, err := os.Create(profilePath) | |
| if err != nil { | |
| options.Logger.Fatal().Msgf("Could not create %s profile %q file: %v", profileType, profilePath, err) | |
| } | |
| return f | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cmd/nuclei/main.go` at line 125, The error log calls f.Name() after os.Create
failed, which can dereference nil; update the profile creation error handling in
the profile creation block so you do not access f when err != nil: check if err
!= nil and log the intended filename/path (use the existing target path variable
or the string passed to os.Create) instead of f.Name(), or only call f.Name()
after confirming f != nil; ensure the Fatal() call uses the safe name/path and
includes the original err.
Comment on lines
128
to
130
| if err := process(opts); err != nil { | ||
| gologger.Error().Msgf("could not process: %s\n", err) | ||
| gologger.Error().Msgf("could not process: %s", err) | ||
| } |
Contributor
There was a problem hiding this comment.
Return a non-zero exit code on processing failure.
The branch logs an error but still exits with status 0, which makes automation treat failures as success.
Proposed fix
if err := process(opts); err != nil {
gologger.Error().Msgf("could not process: %s", err)
+ os.Exit(1)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if err := process(opts); err != nil { | |
| gologger.Error().Msgf("could not process: %s\n", err) | |
| gologger.Error().Msgf("could not process: %s", err) | |
| } | |
| if err := process(opts); err != nil { | |
| gologger.Error().Msgf("could not process: %s", err) | |
| os.Exit(1) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cmd/tmc/main.go` around lines 128 - 130, The current error branch in main
(the if err := process(opts) { ... } in cmd/tmc/main.go) only logs the failure
but leaves the program exiting with status 0; update that branch to exit with a
non-zero status (e.g., call os.Exit(1)) after logging the error, or
alternatively propagate the error out of main so the runtime returns a non-zero
code; ensure you import "os" if you add os.Exit and reference the process(opts)
error path and its logging (gologger.Error().Msgf) when making the change.
Comment on lines
102
to
105
| if err != nil { | ||
| options.Logger.Error().Msgf("%s\n", err) | ||
| options.Logger.Error().Msg(err.Error()) | ||
| } | ||
| os.Exit(0) |
Contributor
There was a problem hiding this comment.
Template profile listing should fail with non-zero status on walk errors.
After logging the filesystem error, the flow still exits with status 0, which hides failure from callers.
Proposed fix
if err != nil {
options.Logger.Error().Msg(err.Error())
+ os.Exit(1)
}
os.Exit(0)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if err != nil { | |
| options.Logger.Error().Msgf("%s\n", err) | |
| options.Logger.Error().Msg(err.Error()) | |
| } | |
| os.Exit(0) | |
| if err != nil { | |
| options.Logger.Error().Msg(err.Error()) | |
| os.Exit(1) | |
| } | |
| os.Exit(0) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/runner/options.go` around lines 102 - 105, The current code logs
filesystem walk errors but always calls os.Exit(0); change the exit behavior so
that when an error occurs during the template/profile walk you exit with a
non-zero status (e.g., os.Exit(1)). Concretely, in the block using
options.Logger.Error().Msg(...) and os.Exit(...) ensure that the err != nil
branch performs the non-zero exit and only successes return/os.Exit(0); keep the
logging via options.Logger.Error().Msg(err.Error()) and adjust the os.Exit call
accordingly.
| uploadWriter, err := pdcp.NewUploadWriter(context.Background(), r.Logger, creds) | ||
| if err != nil { | ||
| r.pdcpUploadErrMsg = fmt.Sprintf("PDCP (%v) Auto-Save Failed: %s\n", pdcpauth.DashBoardURL, err) | ||
| r.pdcpUploadErrMsg = fmt.Sprintf("PDCP (%s) auto-save failed: %s\n", pdcpauth.DashBoardURL, err) |
Contributor
There was a problem hiding this comment.
Remove trailing newline from pdcpUploadErrMsg to finish log normalization.
This message is later emitted with Logger.Warning().Msg(...); keeping \n here can produce double line breaks and is inconsistent with the rest of this PR’s newline cleanup.
Suggested fix
- r.pdcpUploadErrMsg = fmt.Sprintf("PDCP (%s) auto-save failed: %s\n", pdcpauth.DashBoardURL, err)
+ r.pdcpUploadErrMsg = fmt.Sprintf("PDCP (%s) auto-save failed: %s", pdcpauth.DashBoardURL, err)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| r.pdcpUploadErrMsg = fmt.Sprintf("PDCP (%s) auto-save failed: %s\n", pdcpauth.DashBoardURL, err) | |
| r.pdcpUploadErrMsg = fmt.Sprintf("PDCP (%s) auto-save failed: %s", pdcpauth.DashBoardURL, err) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/runner/runner.go` at line 491, The pdcpUploadErrMsg string assigned
in runner.go includes a trailing "\n" which causes double line breaks when later
emitted via Logger.Warning().Msg; update the fmt.Sprintf call that sets
r.pdcpUploadErrMsg (using pdcpauth.DashBoardURL and err) to remove the trailing
newline so the message is normalized with the project's other log messages.
| } | ||
| if options.Uncover && options.UncoverQuery != nil { | ||
| gologger.Info().Msgf("Running uncover query against: %s", strings.Join(options.UncoverEngine, ",")) | ||
| gologger.Info().Msgf("Running uncover query against for %q", strings.Join(options.UncoverEngine, ",")) |
Contributor
There was a problem hiding this comment.
Fix log message typo in uncover startup line.
"Running uncover query against for %q" has an extra "for".
✏️ Proposed fix
- gologger.Info().Msgf("Running uncover query against for %q", strings.Join(options.UncoverEngine, ","))
+ gologger.Info().Msgf("Running uncover query against %q", strings.Join(options.UncoverEngine, ","))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| gologger.Info().Msgf("Running uncover query against for %q", strings.Join(options.UncoverEngine, ",")) | |
| gologger.Info().Msgf("Running uncover query against %q", strings.Join(options.UncoverEngine, ",")) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/input/provider/list/hmap.go` at line 318, Fix the log line that reads
gologger.Info().Msgf("Running uncover query against for %q",
strings.Join(options.UncoverEngine, ",")) by removing the extra "for" so the
message reads either "Running uncover query against %q" (or "Running uncover
query for %q" if preferred); update the string literal in the
gologger.Info().Msgf call that uses strings.Join(options.UncoverEngine, ",") to
reflect the corrected phrase.
Comment on lines
+83
to
+87
| gologger.DefaultLogger.Info().Msgf("[%v] %v", "JS", value) | ||
| case map[string]interface{}: | ||
| gologger.DefaultLogger.Print().Msgf("[%v] %v", aurora.BrightCyan("JS"), vardump.DumpVariables(value)) | ||
| gologger.DefaultLogger.Info().Msgf("[%v] %v", "JS", vardump.DumpVariables(value)) | ||
| default: | ||
| gologger.DefaultLogger.Print().Msgf("[%v] %v", aurora.BrightCyan("JS"), value) | ||
| gologger.DefaultLogger.Info().Msgf("[%v] %v", "JS", value) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify consistency and intended visibility semantics for JS log builtins.
rg -nP --type=go 'Name:\s*"log"|DefaultLogger\.(Info|Print)\(\)\.Msgf\("\[%v\] %v",\s*"JS"' -C2 pkg/js pkg/tmplexec
rg -nP --type=go 'Verbose|Debug|Silent|SetMaxLevel|SetFormatter|SetWriter|NoColor' -C2Repository: projectdiscovery/nuclei
Length of output: 50380
Fix logging level filtering in JS log() builtins to ensure consistent output.
The log() function in both pkg/js/global/scripts.go (lines 83, 85, 87) and pkg/tmplexec/flow/vm.go (lines 66, 68, 70) uses Info(), which subjects debug output to logger level filtering. Since the function is documented as "for debugging purposes," it should use Print() to guarantee output regardless of logger configuration.
Suggested fix
- gologger.DefaultLogger.Info().Msgf("[%v] %v", "JS", value)
+ gologger.DefaultLogger.Print().Msgf("[%v] %v", "JS", value)
...
- gologger.DefaultLogger.Info().Msgf("[%v] %v", "JS", vardump.DumpVariables(value))
+ gologger.DefaultLogger.Print().Msgf("[%v] %v", "JS", vardump.DumpVariables(value))Apply the same change in both files.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| gologger.DefaultLogger.Info().Msgf("[%v] %v", "JS", value) | |
| case map[string]interface{}: | |
| gologger.DefaultLogger.Print().Msgf("[%v] %v", aurora.BrightCyan("JS"), vardump.DumpVariables(value)) | |
| gologger.DefaultLogger.Info().Msgf("[%v] %v", "JS", vardump.DumpVariables(value)) | |
| default: | |
| gologger.DefaultLogger.Print().Msgf("[%v] %v", aurora.BrightCyan("JS"), value) | |
| gologger.DefaultLogger.Info().Msgf("[%v] %v", "JS", value) | |
| gologger.DefaultLogger.Print().Msgf("[%v] %v", "JS", value) | |
| case map[string]interface{}: | |
| gologger.DefaultLogger.Print().Msgf("[%v] %v", "JS", vardump.DumpVariables(value)) | |
| default: | |
| gologger.DefaultLogger.Print().Msgf("[%v] %v", "JS", value) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/js/global/scripts.go` around lines 83 - 87, The JS builtin log()
currently calls gologger.DefaultLogger.Info() (in pkg/js/global/scripts.go for
the cases handling plain values and map[string]interface{}) and similarly in
pkg/tmplexec/flow/vm.go; change those Info() calls to Print() so the
debug-oriented log() always emits output regardless of logger level
filtering—update the calls to gologger.DefaultLogger.Print(...) in the log()
implementation(s) that reference Info() to ensure consistent, unfiltered debug
output.
Comment on lines
+104
to
+107
| gologger.Info().Msgf("[%s] Dumped offline-http request for %q:", request.options.TemplateID, data) | ||
| gologger.Print().Msgf("%s", data) | ||
| } | ||
| gologger.Verbose().Msgf("[%s] Sent OFFLINE-HTTP request to %s", request.options.TemplateID, data) | ||
| gologger.Verbose().Msgf("[%s] Sent offline-http request to %q", request.options.TemplateID, data) |
Contributor
There was a problem hiding this comment.
Use input identifier in metadata logs, not raw payload content.
data is the full raw HTTP response string, so %q here can emit large/sensitive payload content in the “for/to” metadata lines and duplicates the dump output.
Proposed fix
- if request.options.Options.Debug || request.options.Options.DebugRequests {
- gologger.Info().Msgf("[%s] Dumped offline-http request for %q:", request.options.TemplateID, data)
- gologger.Print().Msgf("%s", data)
- }
- gologger.Verbose().Msgf("[%s] Sent offline-http request to %q", request.options.TemplateID, data)
+ target := inputString
+ if target == "" {
+ target = getURLFromRequest(resp.Request)
+ }
+ if request.options.Options.Debug || request.options.Options.DebugRequests {
+ gologger.Info().Msgf("[%s] Dumped offline-http request for %q:", request.options.TemplateID, target)
+ gologger.Print().Msgf("%s", data)
+ }
+ gologger.Verbose().Msgf("[%s] Sent offline-http request to %q", request.options.TemplateID, target)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/protocols/offlinehttp/request.go` around lines 104 - 107, The log lines
currently include the full raw payload `data` in the metadata messages (using
`%q`) and duplicate the dump; change those metadata lines to reference an input
identifier instead of `data` (e.g., use an available identifier like
`request.input.ID` or `request.options.InputID`) and keep the full dump only in
the explicit dump call; update the gologger.Info/Verbose Msgf calls that use
`request.options.TemplateID` to include the input identifier variable in the
"for/to" context rather than the raw `data`, and remove any redundant payload
duplication in those metadata logs.
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.
Proposed changes
fix: malformed JSON lines output
fix WRN output routing & clean up some message
formatting.
Use
Warning()method instead of manually taggedPrint()method warnings in the runner and template-loading paths, which avoids mixing human-readable
warnings into the plain
Print()output path.Closes: #7314
Also, normalize logger calls at the same time by
dropping redundant newline suffixes, trimming
trivial
Msgfwrappers and making user-facing logmessages more consistent across the tree.
Proof
Checklist
Summary by CodeRabbit
Release Notes