fix(reporting): harden Markdown filenames and remediate Go vulnerabilities - #7633
Open
prithvee07 wants to merge 12 commits into
Open
fix(reporting): harden Markdown filenames and remediate Go vulnerabilities#7633prithvee07 wants to merge 12 commits into
prithvee07 wants to merge 12 commits into
Conversation
Sanitize/truncate markdown report filenames; update GitHub client and dependency versions
Harden Markdown report filenames and update Go dependencies (go1.26.5, go-github v81) # Markdown and security change report This file is a standalone, downloadable summary of the Markdown report filename hardening and Go vulnerability remediation work. ## Executive summary The change set: - prevents Markdown report paths from being influenced by directory traversal sequences; - prevents long template IDs and hosts from removing the per-finding UUID; - includes the selected matcher or extractor name in result filenames; - truncates filenames without producing invalid UTF-8; - updates the Go toolchain and vulnerable dependencies; and - reduces reachable `govulncheck` findings from 23 to one upstream advisory with no fixed dependency version. ## Markdown filename hardening Markdown finding files use this structure: ```text <template-id>-<host>-<uuid>-<matcher-or-extractor>.md ``` The matcher name is selected when both a matcher and extractor name are available. Extractor-only findings use the extractor name. Before a file is written, unsafe filesystem characters, path separators, and parent-directory references are replaced. The resulting filename is limited to 255 bytes and truncated only at a valid UTF-8 boundary. Prefix truncation does not remove the UUID or `.md` extension, so findings cannot collide merely because their long template and host prefixes are identical. ## Dependency remediation | Component | Previous version | Updated version | | --- | --- | --- | | Go toolchain | 1.26.0 | 1.26.5 | | `golang.org/x/text` | v0.38.0 | v0.39.0 | | `github.com/yuin/goldmark` | v1.7.13 | v1.7.17 | | Direct `github.com/google/go-github` use | v30.1.0 | v81.0.0 | The direct GitHub client migration covers the custom-template downloader and GitHub reporting tracker. Module metadata was refreshed after the upgrades. ## Vulnerability scan result The initial source scan reported 23 reachable vulnerabilities. After the updates, the scan reports only GO-2026-5932. GO-2026-5932 remains reachable through the latest available `github.com/projectdiscovery/utils/update` dependency, which transitively uses `github.com/google/go-github/v30` and `golang.org/x/crypto/openpgp`. The advisory does not identify a fixed `golang.org/x/crypto` release. Eliminating this final result requires the upstream update package to migrate away from the older GitHub client or a replacement of that update subsystem. Run the vulnerability scan with: ```console go run golang.org/x/vuln/cmd/govulncheck@latest \ -db=https://storage.googleapis.com/go-vulndb ./... ``` An exit status indicating GO-2026-5932 is expected until the upstream dependency is migrated. ## Validation commands ```console go mod verify go test ./pkg/reporting/exporters/markdown go test ./pkg/external/customtemplates -run '^$' go test ./pkg/reporting/trackers/github go vet ./pkg/external/customtemplates \ ./pkg/reporting/trackers/github \ ./pkg/reporting/exporters/markdown ``` The `-run '^$'` custom-template command checks compilation without running the network-dependent GitHub download tests. ## Detailed documentation - [Markdown report filenames](markdown-report-filenames.md) - [Go vulnerability remediation](security-remediation.md)
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe change adds HTTP API token authentication, restricts generated file and directory permissions, hardens Markdown filename construction, updates Go dependencies, improves raw HTTP request parsing, and documents security remediation. ChangesSecurity hardening and remediation
Raw HTTP request parsing
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant Options
participant HTTPAPI
participant HTTPClient
CLI->>Options: set http-api-token
Options->>HTTPAPI: provide HttpApiToken
HTTPClient->>HTTPAPI: send token query parameter
HTTPAPI->>HTTPClient: return 401 or API response
Possibly related issues
Possibly related PRs
Suggested reviewers: 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 |
![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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/reporting/exporters/markdown/markdown.go (1)
150-158: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winRemove NUL bytes from normalized filename components.
stringsutil.ReplaceAllpreserves\x00, andcreateFileNameincludesevent.Host,event.TemplateID,event.MatcherName, andevent.ExtractorName.Exporter.Exportwrites the report under that NUL-containing path after the index row is written; replace NUL here. Add a test that assertscreateFileNamefor NUL-containing inputs does not contain\x00, and update existing NUL-input assertions to match this expectation.Proposed fix
-return stringsutil.ReplaceAll(filename, "_", "?", "/", "\\", "..", ">", "|", ":", ";", "*", "<", "\"", "'", " ") +return stringsutil.ReplaceAll(filename, "_", "\x00", "?", "/", "\\", "..", ">", "|", ":", ";", "*", "<", "\"", "'", " ")🤖 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/reporting/exporters/markdown/markdown.go` around lines 150 - 158, Update sanitizeFilenamePart to replace NUL bytes alongside the existing unsafe filename characters, ensuring createFileName produces no \x00 for event.Host, event.TemplateID, event.MatcherName, or event.ExtractorName. Add or update tests for createFileName with NUL-containing inputs to assert the result excludes \x00, and adjust existing NUL-input expectations accordingly.
🤖 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 `@go.mod`:
- Line 78: Update the utils/update dependency path so it no longer imports
github.com/google/go-github/v30/github, while preserving cmd/nuclei’s existing
update behavior. Migrate utils/update to the existing
github.com/google/go-github/v81 dependency, or fork/vendor the consumer if
direct migration is unavailable; only retain v30 if the intentional security
exposure is explicitly documented.
In `@pkg/reporting/exporters/markdown/markdown_test.go`:
- Around line 69-74: Update TestSanitizeFilenameTruncatesAtUTF8Boundary to use a
non-replaced four-byte UTF-8 rune, such as an emoji, repeated enough to truncate
mid-rune before calling sanitizeFilename. Keep assertions that the output byte
length is at most maxFilenameLength and that utf8.ValidString(output) is true.
---
Outside diff comments:
In `@pkg/reporting/exporters/markdown/markdown.go`:
- Around line 150-158: Update sanitizeFilenamePart to replace NUL bytes
alongside the existing unsafe filename characters, ensuring createFileName
produces no \x00 for event.Host, event.TemplateID, event.MatcherName, or
event.ExtractorName. Add or update tests for createFileName with NUL-containing
inputs to assert the result excludes \x00, and adjust existing NUL-input
expectations accordingly.
🪄 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 Plus
Run ID: 8ee317bb-8054-475e-9663-307ff335f4d4
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (8)
docs/markdown-report-filenames.mddocs/markdown-security-change-report.mddocs/security-remediation.mdgo.modpkg/external/customtemplates/github.gopkg/reporting/exporters/markdown/markdown.gopkg/reporting/exporters/markdown/markdown_test.gopkg/reporting/trackers/github/github.go
Comment on lines
+69
to
+74
| func TestSanitizeFilenameTruncatesAtUTF8Boundary(t *testing.T) { | ||
| filename := sanitizeFilename(strings.Repeat("界", maxFilenameLength)) | ||
|
|
||
| require.LessOrEqual(t, len(filename), maxFilenameLength) | ||
| require.True(t, utf8.ValidString(filename)) | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Go files under markdown exporter:\n'
git ls-files | rg 'pkg/reporting/exporters/markdown/.*\.go$' || true
printf '\nRelevant test and sanitizer files:\n'
fd '\.go$' pkg/reporting/exporters/markdown | sort | while read -r f; do
echo "--- $f ($(wc -l < "$f") lines) ---"
done
printf '\nSearch sanitizeFilename definitions and tests:\n'
rg -n "sanitizeFilename|TestSanitizeFilename|maxFilenameLength|RunesToUTF8Truncation|InvalidUTF8" pkg/reporting/exporters/markdown || true
printf '\nInspect markdown_test.go around relevant test:\n'
sed -n '1,120p' pkg/reporting/exporters/markdown/markdown_test.go
printf '\nInspect sanitize implementation candidates:\n'
for f in $(git ls-files | rg 'pkg/reporting/exporters/markdown/.*\.go$'); do
if rg -q "func sanitizeFilename|sanitizeFilename" "$f"; then
ast-grep outline "$f" --view compact || true
echo "--- $f ---"
rg -n -C 8 "func sanitizeFilename|var maxFilenameLength|maxFilenameLength|utf8\.Invalid|InvalidUTF8|Runes" "$f" || true
fi
done
printf '\nBehavioral probe: byte lengths and truncation around UTF-8 boundaries:\n'
python3 - <<'PY'
import sys
r = "界"
print("界 len utf8", len(r.encode('utf8')), "rune chars", len(r))
print("interface", 255 % len(r.encode('utf8')))
r2 = "😀"
print("😀 len utf8", len(r2.encode('utf8')), "rune chars", len(r2))
print("interface", 255 % len(r2.encode('utf8')))
PYRepository: projectdiscovery/nuclei
Length of output: 10424
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Relevant Go source snippets:\n'
for f in $(git ls-files | rg 'pkg/reporting/exporters/markdown/.*\.go$'); do
if rg -q "sanitizeFilename|invalid.*UTF|UTF.*invalid|InvalidUTF8|RunesToUTF8Truncation" "$f"; then
echo "=== $f ==="
sed -n '1,220p' "$f"
fi
done
printf '\nChanged files/stat:\n'
git diff --stat || true
printf '\nDiff for markdown_test.go if present:\n'
git diff -- pkg/reporting/exporters/markdown/markdown_test.go || trueRepository: projectdiscovery/nuclei
Length of output: 12917
Use an input that truncates inside a four-byte UTF-8 rune.
sanitizeFilenamePart replaces 界, so this test does not exercise UTF-8 boundary repair. Use an input such as an emoji that is not one of the replaced characters, and truncates mid-rune, before calling sanitizeFilename. Check that len(output) <= maxFilenameLength and utf8.ValidString(output) pass.
🤖 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/reporting/exporters/markdown/markdown_test.go` around lines 69 - 74,
Update TestSanitizeFilenameTruncatesAtUTF8Boundary to use a non-replaced
four-byte UTF-8 rune, such as an emoji, repeated enough to truncate mid-rune
before calling sanitizeFilename. Keep assertions that the output byte length is
at most maxFilenameLength and that utf8.ValidString(output) is true.
The experimental -http-api-endpoint server used http.ListenAndServe(addr, nil), which serves Go's global DefaultServeMux. Since net/http/pprof is blank-imported elsewhere in the binary for the separate, opt-in -enable-pprof feature, enabling -http-api-endpoint silently exposed /debug/pprof/* (process cmdline, heap/goroutine dumps, CPU profiling) on the same address. The endpoint's PUT /api/concurrency also had no authentication. Give the server its own ServeMux, add an optional -http-api-token flag (mirroring -dast-server-token) with a warning when unset, and add request timeouts. Also tighten file/directory permissions (0644->0600, 0755/0775->0750) across report exporters, the template installer/signer, custom-template downloaders, and output writers, so scan results and downloaded templates aren't left world-readable on shared systems. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QSCzGpK3LLQDKXtHCew5CG
Security hardening: Add HTTP API token auth and improve file permissions
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/httpapi/apiendpoint.go`:
- Around line 73-84: Update Server.tokenAuthMiddleware to read the token from
the Authorization header first, using the query parameter only as a fallback if
required. Parse the header according to the existing authentication contract,
reject missing or malformed credentials, and compare the supplied token with
s.token via crypto/subtle.ConstantTimeCompare rather than != while preserving
the unauthorized response.
In `@internal/runner/runner.go`:
- Line 1020: Ensure directory setup enforces 0750 for both newly created and
existing directories: update the handling around os.MkdirAll in
internal/runner/runner.go:1020-1020, pkg/catalog/index/index.go:53-53, and
pkg/reporting/exporters/pdf/pdf.go:88-88 to stat each destination after creation
and chmod it to 0750, while preserving existing error handling.
In `@pkg/input/formats/formats.go`:
- Around line 103-104: Enforce restrictive permissions on reruns: in
pkg/input/formats/formats.go:103-104, update WriteOpenAPIVarDumpFile to apply
0600 to the existing dump before truncating. Check and fix existing directories
before writing in pkg/catalog/loader/ai_loader.go:49-54,
pkg/external/customtemplates/azure_blob.go:149-156,
pkg/external/customtemplates/gitlab.go:75, 115, and 165-173,
pkg/input/formats/openapi/downloader.go:107, and
pkg/input/formats/swagger/downloader.go:120; in
pkg/external/customtemplates/s3.go:94-96 keep managed directories restricted and
create files with 0600, and do the same for OpenAPI and Swagger specs instead of
os.Create. Apply 0600 to existing templates in
pkg/templates/template_sign.go:83-85, and add rerun tests covering pre-existing
0755 directories and 0644 files.
In `@pkg/installer/versioncheck.go`:
- Line 79: Enforce owner-only permissions on existing output files: in
pkg/installer/versioncheck.go:79 apply Chmod(0600) to the ignore file after
opening or writing it; in pkg/output/file_output_writer.go:19-21 apply
Chmod(0600) in both resume and non-resume paths; in pkg/output/output.go:650
apply Chmod(0600) before appending debug data; in
pkg/reporting/exporters/jsonexporter/jsonexporter.go:65 and
pkg/reporting/exporters/sarif/sarif.go:191 write through opened descriptors and
enforce 0600; and in pkg/reporting/exporters/jsonl/jsonl.go:73 apply Chmod(0600)
before writing rows.
🪄 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 Plus
Run ID: f0e539e3-789f-480c-836d-b04c3cfc366d
📒 Files selected for processing (23)
cmd/nuclei/main.gointernal/httpapi/apiendpoint.gointernal/runner/runner.gopkg/catalog/index/index.gopkg/catalog/loader/ai_loader.gopkg/external/customtemplates/azure_blob.gopkg/external/customtemplates/gitlab.gopkg/external/customtemplates/s3.gopkg/input/formats/formats.gopkg/input/formats/openapi/downloader.gopkg/input/formats/swagger/downloader.gopkg/installer/template.gopkg/installer/versioncheck.gopkg/output/file_output_writer.gopkg/output/output.gopkg/protocols/headless/engine/page_actions.gopkg/reporting/exporters/jsonexporter/jsonexporter.gopkg/reporting/exporters/jsonl/jsonl.gopkg/reporting/exporters/markdown/markdown.gopkg/reporting/exporters/pdf/pdf.gopkg/reporting/exporters/sarif/sarif.gopkg/templates/template_sign.gopkg/types/types.go
🚧 Files skipped from review as they are similar to previous changes (1)
- pkg/reporting/exporters/markdown/markdown.go
Comment on lines
+73
to
+84
| // tokenAuthMiddleware requires a matching ?token= query parameter on every | ||
| // request when a token has been configured. | ||
| func (s *Server) tokenAuthMiddleware(next http.Handler) http.Handler { | ||
| return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| token := r.URL.Query().Get("token") | ||
| if token == "" || token != s.token { | ||
| http.Error(w, "Unauthorized", http.StatusUnauthorized) | ||
| return | ||
| } | ||
| next.ServeHTTP(w, r) | ||
| }) | ||
| } |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Harden tokenAuthMiddleware: use constant-time comparison and avoid a query-string-only token.
Two issues affect this authentication check:
- Line 78 compares
token != s.tokenwith the standard!=operator. This comparison is not constant-time and creates a timing side-channel that can help an attacker recover the token byte by byte. - The middleware only reads the token from
r.URL.Query().Get("token")(line 77). URL query parameters commonly end up in server access logs, reverse-proxy logs, and browser history. A secret token in the query string risks unintended disclosure through these channels. Support anAuthorizationheader as the primary transport, and treat the query parameter as a fallback only if still needed.
Use crypto/subtle.ConstantTimeCompare for the comparison, and read the token from the Authorization header.
🔒 Proposed fix for constant-time comparison and header-based token
+import (
+ "crypto/subtle"
+ "strings"
+)
+
func (s *Server) tokenAuthMiddleware(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
- token := r.URL.Query().Get("token")
- if token == "" || token != s.token {
+ token := strings.TrimPrefix(r.Header.Get("Authorization"), "Bearer ")
+ if token == "" {
+ token = r.URL.Query().Get("token")
+ }
+ if token == "" || subtle.ConstantTimeCompare([]byte(token), []byte(s.token)) != 1 {
http.Error(w, "Unauthorized", http.StatusUnauthorized)
return
}
next.ServeHTTP(w, r)
})
}🤖 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/httpapi/apiendpoint.go` around lines 73 - 84, Update
Server.tokenAuthMiddleware to read the token from the Authorization header
first, using the query parameter only as a fallback if required. Parse the
header according to the existing authentication contract, reject missing or
malformed credentials, and compare the supplied token with s.token via
crypto/subtle.ConstantTimeCompare rather than != while preserving the
unauthorized response.
| dir := filepath.Dir(path) | ||
| if !fileutil.FolderExists(dir) { | ||
| if err := os.MkdirAll(dir, os.ModePerm); err != nil { | ||
| if err := os.MkdirAll(dir, 0750); err != nil { |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
go doc os.MkdirAll
rg -n -C2 'os\.MkdirAll|os\.Chmod' \
internal/runner/runner.go \
pkg/catalog/index/index.go \
pkg/reporting/exporters/pdf/pdf.goRepository: projectdiscovery/nuclei
Length of output: 1431
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the call sites to confirm existing directory handling and permission guards.
sed -n '1000,1030p' internal/runner/runner.go
printf '\n---\n'
sed -n '40,60p' pkg/catalog/index/index.go
printf '\n---\n'
sed -n '78,92p' pkg/reporting/exporters/pdf/pdf.go
# Behavioral probe outside the repository: os.MkdirAll leaves an existing directory’s mode unchanged.
tmpdir="$(mktemp -d)"
mkdir -m 0777 "$tmpdir/existing"
tmpdir="$tmpdir" /bin/sh -c 'set -e; go run - <<'"'GO'"'
package main
import (
"fmt"
"os"
"path/filepath"
)
func main() {
path := os.Args[1]
_ = os.MkdirAll(path, 0750)
info, _ := os.Stat(path)
fmt.Printf("%o\n", info.Mode().Perm())
}
GO' "$tmpdir/existing"Repository: projectdiscovery/nuclei
Length of output: 2162
Enforce 0750 on existing destination directories.
os.MkdirAll creates only missing directory components and leaves existing directories unchanged. After handling MkdirAll, also call os.Stat/os.Chmod when the directory already exists so resumed config, catalog cache, and PDF report directories cannot keep broader legacy permissions.
📍 Affects 3 files
internal/runner/runner.go#L1020-L1020(this comment)pkg/catalog/index/index.go#L53-L53pkg/reporting/exporters/pdf/pdf.go#L88-L88
🤖 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/runner.go` at line 1020, Ensure directory setup enforces 0750
for both newly created and existing directories: update the handling around
os.MkdirAll in internal/runner/runner.go:1020-1020,
pkg/catalog/index/index.go:53-53, and pkg/reporting/exporters/pdf/pdf.go:88-88
to stat each destination after creation and chmod it to 0750, while preserving
existing error handling.
Comment on lines
103
to
+104
| func WriteOpenAPIVarDumpFile(vars *OpenAPIParamsCfgFile) error { | ||
| f, err := os.OpenFile(DefaultVarDumpFileName, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0644) | ||
| f, err := os.OpenFile(DefaultVarDumpFileName, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0600) |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
go doc os.MkdirAll
go doc os.WriteFile
go doc os.Create
rg -n -C 2 'os\.(MkdirAll|WriteFile|OpenFile|Create)\(' \
pkg/catalog/loader/ai_loader.go \
pkg/external/customtemplates/azure_blob.go \
pkg/external/customtemplates/gitlab.go \
pkg/external/customtemplates/s3.go \
pkg/input/formats/formats.go \
pkg/input/formats/openapi/downloader.go \
pkg/input/formats/swagger/downloader.go \
pkg/templates/template_sign.goRepository: projectdiscovery/nuclei
Length of output: 7990
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "Go version:"
go version
python3 - <<'PY'
import contextlib, io, os, stat, tempfile
def check_name(name, mode=None, create_fn=stat.S_IRUSR|stat.S_IWUSR|stat.S_IXUSR|stat.S_IRGRP|stat.S_IXGRP|stat.S_IROTH|stat.S_IXOTH):
with tempfile.TemporaryDirectory() as tmp:
p = os.path.join(tmp, name)
os.makedirs(os.path.dirname(p))
try:
os.mkdir(p, create_fn)
except FileExistsError:
pass
before = stat.S_IMODE(os.lstat(p).st_mode)
os.makedirs(p, mode)
after = stat.S_IMODE(os.lstat(p).st_mode)
return name, before, after
def check_file(name, before_mode, write_data, file_mode, create_fn=None):
with tempfile.TemporaryDirectory() as tmp:
p = os.path.join(tmp, name)
if create_fn is not None:
fd = os.open(tmp, os.O_CREAT | os.O_EXCL, create_fn)
os.close(fd)
with open(p, 'wb') as f:
f.write(b"*")
before = stat.S_IMODE(os.lstat(p).st_mode)
write_fn = os.open if callable(write_data) else stat.S_IMODE(0)
if callable(write_data):
write_func = write_data
else:
write_func = lambda x, y, z: open(x, 'wb').write(y)
write_func(p, [2,3], file_mode)
after = stat.S_IMODE(os.lstat(p).st_mode)
return name, before, after
print("MkdirAll existing dir modes:")
for perm in (0o755, 0o777):
try:
print(check_name("existing", 0o750, perm))
except Exception as e:
print(repr(e))
print("WriteFile existing file modes:")
for perm in (0o644, 0o755):
with tempfile.TemporaryDirectory as tmp:
with tempfile.NamedTemporaryFile(dir=tmp.delete=False) as f:
p=f.name
try:
os.chmod(p, perm)
except Exception as e:
print(f"chmod failed {perm}", e)
continue
# Use stat.S_IMODE to see default result
import os as o
data_after = None
try:
with o.open(p, "wb") as f: f.write(b"")
except Exception as e:
print(f"write failed {perm}", e)
continue
print("WRITEFILE", perm, stat.S_IMODE(o.lstat(p).st_mode))
PY
sed -n '1,140p' pkg/catalog/loader/ai_loader.go
sed -n '120,180p' pkg/external/customtemplates/gitlab.go
sed -n '80,110p' pkg/external/customtemplates/s3.go
sed -n '95,120p' pkg/input/formats/openapi/downloader.go
sed -n '130,150p' pkg/input/formats/swagger/downloader.go
echo "Static analysis:"
go vet ./pkg/catalog/loader/ai_loader.go ./pkg/external/customtemplates/azure_blob.go ./pkg/external/customtemplates/gitlab.go ./pkg/external/customtemplates/s3.go ./pkg/input/formats/formats.go ./pkg/input/formats/openapi/downloader.go ./pkg/input/formats/swagger/downloader.go ./pkg/templates/template_sign.goRepository: projectdiscovery/nuclei
Length of output: 377
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "Go version:"
go version
python3 - <<'PY'
import os, stat, tempfile
print("MkdirAll existing directory modes")
for before_mode in (0o755, 0o777):
with tempfile.TemporaryDirectory() as tmp:
d = os.path.join(tmp, "existing")
os.makedirs(os.path.join(tmp, "parent"))
os.mkdir(d, before_mode)
os.makedirs(d, 0o750)
after_mode = stat.S_IMODE(os.lstat(d).st_mode)
print("before", oct(before_mode), "after 0o750 MkdirAll", oct(after_mode))
print("WriteFile existing file modes")
for before_mode in (0o644, 0o755):
with tempfile.TemporaryDirectory() as tmp:
f = os.path.join(tmp, "existing")
os.chmod(f, before_mode)
os.chmod(f, before_mode)
with open(f, "wb") as fh:
fh.write(b"")
after_mode = stat.S_IMODE(os.lstat(f).st_mode)
print("before", oct(before_mode), "after WriteFile", oct(after_mode))
print("Open/create style file modes with umask 0000")
old_umask = os.umask(0)
try:
with tempfile.TemporaryDirectory() as tmp:
f = os.path.join(tmp, "create")
fd = os.open(f, os.O_RDWR | os.O_CREAT | os.O_TRUNC, 0o600)
try:
with os.fdopen(fd, "w") as fh:
fh.write("")
finally:
os.close(fd)
print("O_CREATE O_TRUNC default", oct(stat.S_IMODE(os.lstat(f).st_mode)))
os.chmod(f, 0o644)
fd = os.open(f, os.O_RDWR | os.O_CREAT | os.O_TRUNC, 0o600)
try:
with os.fdopen(fd, "w") as fh:
fh.write("")
finally:
os.close(fd)
print("O_CREATE O_TRUNC existing", oct(stat.S_IMODE(os.lstat(f).st_mode)))
finally:
os.umask(old_umask)
PY
sed -n '1,140p' pkg/catalog/loader/ai_loader.go
sed -n '120,180p' pkg/external/customtemplates/gitlab.go
sed -n '80,110p' pkg/external/customtemplates/s3.go
sed -n '95,120p' pkg/input/formats/openapi/downloader.go
sed -n '130,150p' pkg/input/formats/swagger/downloader.go
echo "Static analysis:"
go vet ./pkg/catalog/loader/ai_loader.go ./pkg/external/customtemplates/azure_blob.go ./pkg/external/customtemplates/gitlab.go ./pkg/external/customtemplates/s3.go ./pkg/input/formats/formats.go ./pkg/input/formats/openapi/downloader.go ./pkg/input/formats/swagger/downloader.go ./pkg/templates/template_sign.goRepository: projectdiscovery/nuclei
Length of output: 462
🏁 Script executed:
#!/usr/bin/env/bash
set -euo pipefail
echo "Go version:"
go version
python3 - <<'PY'
import os, stat, tempfile
print("MkdirAll existing directory modes")
for before_mode in (0o755, 0o777):
with tempfile.TemporaryDirectory() as tmp:
d = os.path.join(tmp, "existing")
os.mkdir(d, before_mode)
os.makedirs(d, 0o750)
after_mode = stat.S_IMODE(os.lstat(d).st_mode)
print("before", oct(before_mode), "after MkdirAll 0o750", oct(after_mode))
print("WriteFile existing file modes")
for before_mode in (0o644, 0o755):
with tempfile.TemporaryDirectory() as tmp:
f = os.path.join(tmp, "existing")
open(f, "wb").close()
os.chmod(f, before_mode)
with open(f, "wb") as fh:
fh.write(b"")
after_mode = stat.S_IMODE(os.lstat(f).st_mode)
print("before", oct(before_mode), "after WriteFile", oct(after_mode))
print("OpenFile O_CREATE O_TRUNC file modes with umask 0000")
old_umask = os.umask(0)
try:
with tempfile.TemporaryDirectory() as tmp:
f = os.path.join(tmp, "create")
with os.fdopen(os.open(f, os.O_RDWR | os.O_CREAT | os.O_TRUNC, 0o600), "w") as fh:
fh.write("")
print("new file", oct(stat.S_IMODE(os.lstat(f).st_mode)))
os.chmod(f, 0o644)
with os.fdopen(os.open(f, os.O_RDWR | os.O_CREAT | os.O_TRUNC, 0o600), "w") as fh:
fh.write("")
print("existing file", oct(stat.S_IMODE(os.lstat(f).st_mode)))
finally:
os.umask(old_umask)
PY
sed -n '1,115p' pkg/catalog/loader/ai_loader.go
sed -n '120,180p' pkg/external/customtemplates/gitlab.go
sed -n '80,110p' pkg/external/customtemplates/s3.go
sed -n '95,120p' pkg/input/formats/openapi/downloader.go
sed -n '130,150p' pkg/input/formats/swagger/downloader.go
echo "Static analysis:"
go vet ./pkg/catalog/loader/ai_loader.go ./pkg/external/customtemplates/azure_blob.go ./pkg/external/customtemplates/gitlab.go ./pkg/external/customtemplates/s3.go ./pkg/input/formats/formats.go ./pkg/input/formats/openapi/downloader.go ./pkg/input/formats/swagger/downloader.go ./pkg/templates/template_sign.goRepository: projectdiscovery/nuclei
Length of output: 462
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "Go version:"
go version
python3 - <<'PY'
import os, stat, tempfile
print("MkdirAll existing directory modes")
for before_mode in (0o755, 0o777):
with tempfile.TemporaryDirectory() as tmp:
d = os.path.join(tmp, "existing")
os.mkdir(d, before_mode)
os.makedirs(d, 0o750)
after_mode = stat.S_IMODE(os.lstat(d).st_mode)
print("before", oct(before_mode), "after MkdirAll 0o750", oct(after_mode))
print("WriteFile existing file modes")
for before_mode in (0o644, 0o755):
with tempfile.TemporaryDirectory() as tmp:
f = os.path.join(tmp, "existing")
open(f, "wb").close()
os.chmod(f, before_mode)
with open(f, "wb") as fh:
fh.write(b"")
after_mode = stat.S_IMODE(os.lstat(f).st_mode)
print("before", oct(before_mode), "after WriteFile", oct(after_mode))
print("OpenFile O_CREATE O_TRUNC file modes with umask 0000")
old_umask = os.umask(0)
try:
with tempfile.TemporaryDirectory() as tmp:
f = os.path.join(tmp, "create")
with os.fdopen(os.open(f, os.O_RDWR | os.O_CREAT | os.O_TRUNC, 0o600), "w") as fh:
fh.write("")
print("new file", oct(stat.S_IMODE(os.lstat(f).st_mode)))
os.chmod(f, 0o644)
with os.fdopen(os.open(f, os.O_RDWR | os.O_CREAT | os.O_TRUNC, 0o600), "w") as fh:
fh.write("")
print("existing file", oct(stat.S_IMODE(os.lstat(f).st_mode)))
finally:
os.umask(old_umask)
PY
sed -n '1,115p' pkg/catalog/loader/ai_loader.go
sed -n '120,180p' pkg/external/customtemplates/gitlab.go
sed -n '80,110p' pkg/external/customtemplates/s3.go
sed -n '95,120p' pkg/input/formats/openapi/downloader.go
sed -n '130,150p' pkg/input/formats/swagger/downloader.go
echo "Static analysis:"
go vet ./pkg/catalog/loader/ai_loader.go ./pkg/external/customtemplates/azure_blob.go ./pkg/external/customtemplates/gitlab.go ./pkg/external/customtemplates/s3.go ./pkg/input/formats/formats.go ./pkg/input/formats/openapi/downloader.go ./pkg/input/formats/swagger/downloader.go ./pkg/templates/template_sign.goRepository: projectdiscovery/nuclei
Length of output: 462
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "Go version:"
go version
python3 - <<'PY'
import os, stat, tempfile
print("MkdirAll existing directory modes")
for before_mode in (0o755, 0o777):
with tempfile.TemporaryDirectory() as tmp:
d = os.path.join(tmp, "existing")
os.mkdir(d, before_mode)
try:
os.makedirs(d, 0o750)
except FileExistsError:
pass
after_mode = stat.S_IMODE(os.lstat(d).st_mode)
print("before", oct(before_mode), "after MkdirAll 0o750", oct(after_mode))
print("WriteFile existing file mode")
for before_mode in (0o644, 0o755):
with tempfile.TemporaryDirectory() as tmp:
f = os.path.join(tmp, "existing")
open(f, "wb").close()
os.chmod(f, before_mode)
with open(f, "wb") as fh:
fh.write(b"")
after_mode = stat.S_IMODE(os.lstat(f).st_mode)
print("before", oct(before_mode), "after WriteFile", oct(after_mode))
print("OpenFile O_CREATE O_TRUNC file modes with umask 0000")
old_umask = os.umask(0)
try:
with tempfile.TemporaryDirectory() as tmp:
f = os.path.join(tmp, "create")
with os.fdopen(os.open(f, os.O_RDWR | os.O_CREAT | os.O_TRUNC, 0o600), "w") as fh:
fh.write("")
print("new file", oct(stat.S_IMODE(os.lstat(f).st_mode)))
os.chmod(f, 0o644)
with os.fdopen(os.open(f, os.O_RDWR | os.O_CREAT | os.O_TRUNC, 0o600), "w") as fh:
fh.write("")
print("existing file", oct(stat.S_IMODE(os.lstat(f).st_mode)))
finally:
os.umask(old_umask)
PY
sed -n '1,115p' pkg/catalog/loader/ai_loader.go
sed -n '120,180p' pkg/external/customtemplates/gitlab.go
sed -n '80,110p' pkg/external/customtemplates/s3.go
sed -n '95,120p' pkg/input/formats/openapi/downloader.go
sed -n '130,150p' pkg/input/formats/swagger/downloader.go
echo "Static analysis:"
go vet ./pkg/catalog/loader/ai_loader.go ./pkg/external/customtemplates/azure_blob.go ./pkg/external/customtemplates/gitlab.go ./pkg/external/customtemplates/s3.go ./pkg/input/formats/formats.go ./pkg/input/formats/openapi/downloader.go ./pkg/input/formats/swagger/downloader.go ./pkg/templates/template_sign.goRepository: projectdiscovery/nuclei
Length of output: 8840
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "Inspect remaining files:"
sed -n '1,90p' pkg/input/formats/formats.go
sed -n '1,55p' pkg/input/formats/openapi/downloader.go
sed -n '115,150p' pkg/input/formats/swagger/downloader.go
sed -n '70,90p' pkg/external/customtemplates/azure_blob.go
sed -n '68,78p' pkg/external/customtemplates/gitlab.go
sed -n '110,120p' pkg/external/customtemplates/gitlab.go
sed -n '76,88p' pkg/templates/template_sign.go
echo "Run go vet per affected package:"
for pkg in pkg/catalog/loader pkg/external/customtemplates pkg/input/formats pkg/input/formats/openapi pkg/input/formats/swagger pkg/templates; do
echo "--- $pkg ---"
go vet "./$pkg/..."
done
echo "Find all os.Create/O_CREATE OpenFile in affected paths:"
rg -n 'os\.(Create|OpenFile|WriteFile|MkdirAll)\(' \
pkg/catalog/loader \
pkg/external/customtemplates \
pkg/input/formats \
pkg/templatesRepository: projectdiscovery/nuclei
Length of output: 13588
Enforce file permissions on reruns and default file creation.
MkdirAll and O_CREATE permission hints only affect newly created inodes. Existing directories remain at their current modes, existing files keep their previous modes, and os.Create uses 0666 before umask.
pkg/input/formats/formats.go#L103-L104: apply0600to an existing variable dump before truncating.pkg/catalog/loader/ai_loader.go#L49-L54: check/fix the existingpdcpdirectory before writing the template.pkg/external/customtemplates/azure_blob.go#L149-L156: check/fix existing parent directories before writing.pkg/external/customtemplates/gitlab.go#L75-L173: check/fix existing directories before writing.pkg/external/customtemplates/s3.go#L94-L101: keep managed directories restricted and create the file at0600instead of usingos.Create.pkg/input/formats/openapi/downloader.go#L107-L116: check/fix existing OpenAPI directory and create the spec at0600instead of usingos.Create.pkg/input/formats/swagger/downloader.go#L120-L145: check/fix existing Swagger directory and create the spec at0600instead of usingos.Create.pkg/templates/template_sign.go#L83-L85: apply0600to an existing template before writing.
Add rerun tests with pre-existing 0755 directories and 0644 files.
📍 Affects 8 files
pkg/input/formats/formats.go#L103-L104(this comment)pkg/catalog/loader/ai_loader.go#L49-L54pkg/external/customtemplates/azure_blob.go#L149-L156pkg/external/customtemplates/gitlab.go#L75-L75pkg/external/customtemplates/gitlab.go#L115-L115pkg/external/customtemplates/gitlab.go#L165-L173pkg/external/customtemplates/s3.go#L94-L96pkg/input/formats/openapi/downloader.go#L107-L107pkg/input/formats/swagger/downloader.go#L120-L120pkg/templates/template_sign.go#L83-L85
🤖 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/formats.go` around lines 103 - 104, Enforce restrictive
permissions on reruns: in pkg/input/formats/formats.go:103-104, update
WriteOpenAPIVarDumpFile to apply 0600 to the existing dump before truncating.
Check and fix existing directories before writing in
pkg/catalog/loader/ai_loader.go:49-54,
pkg/external/customtemplates/azure_blob.go:149-156,
pkg/external/customtemplates/gitlab.go:75, 115, and 165-173,
pkg/input/formats/openapi/downloader.go:107, and
pkg/input/formats/swagger/downloader.go:120; in
pkg/external/customtemplates/s3.go:94-96 keep managed directories restricted and
create files with 0600, and do the same for OpenAPI and Swagger specs instead of
os.Create. Apply 0600 to existing templates in
pkg/templates/template_sign.go:83-85, and add rerun tests covering pre-existing
0755 directories and 0644 files.
| return err | ||
| } | ||
| if err := os.WriteFile(config.DefaultConfig.GetIgnoreFilePath(), bin, 0644); err != nil { | ||
| if err := os.WriteFile(config.DefaultConfig.GetIgnoreFilePath(), bin, 0600); err != nil { |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
go doc os.OpenFile os.WriteFile
rg -n -C2 'os\.(OpenFile|WriteFile)|\.Chmod' \
pkg/installer/versioncheck.go \
pkg/output/file_output_writer.go \
pkg/output/output.go \
pkg/reporting/exporters/jsonexporter/jsonexporter.go \
pkg/reporting/exporters/jsonl/jsonl.go \
pkg/reporting/exporters/sarif/sarif.goRepository: projectdiscovery/nuclei
Length of output: 195
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '## Go standard library references (%s)\n' "$(go version 2>/dev/null || true)"
go doc os.OpenFile | sed -n '1,80p' || true
printf '\n## Go standard library references for WriteFile\n'
go doc os.WriteFile | sed -n '1,80p' || true
printf '\n## Affected files and relevant symbols\n'
rg -n -C3 'os\.(OpenFile|WriteFile)|\.Chmod|O_APPEND|O_TRUNC|O_CREATE|0600|0644' \
pkg/installer/versioncheck.go \
pkg/output/file_output_writer.go \
pkg/output/output.go \
pkg/reporting/exporters/jsonexporter/jsonexporter.go \
pkg/reporting/exporters/jsonl/jsonl.go \
pkg/reporting/exporters/sarif/sarif.go || trueRepository: projectdiscovery/nuclei
Length of output: 4509
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '## Behavioral probe for os.OpenFile with existing file\n'
tmp="$(mktemp -d)"
trap 'rm -rf "$tmp"' EXIT
file="$tmp/existing.txt"
printf 'world\n' > "$file"
chmod 0644 "$file"
stat_before="$(stat -c '%a' "$file")"
echo "before: $stat_before"
go run - <<'GO' >/tmp/osprobe.out 2>&1 || true
package main
import (
"fmt"
"os"
"path/filepath"
)
func main() {
tmp := os.Args[1]
file := filepath.Join(tmp, "existing.txt")
f, err := os.OpenFile(file, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0600)
if err != nil {
fmt.Fprintln(os.Stderr, err)
os.Exit(1)
}
defer f.Close()
fmt.Println(f.Name(), os.PathSep)
}
GO
cat /tmp/osprobe.out
stat_after="$(stat -c '%a' "$file")"
echo "after append with O_APPEND|O_CREATE: $stat_after"
printf '\n## Behavioral probe for a read-truncate-open path\n'
file="$tmp/existing2.txt"
printf 'world\n' > "$file"
chmod 0644 "$file"
echo "before: $(stat -c '%a' "$file")"
go run - <<'GO' >/tmp/osprobe2.out 2>&1 || true
package main
import "os"
func main() {
f, err := os.OpenFile("nonexistent_probe.txt", os.O_RDWR|os.O_TRUNC|os.O_CREATE, 0640)
if err != nil {
panic(err)
}
f.Close()
}
GO
cat /tmp/osprobe2.out
stat_after="$(stat -c '%a' "$file")"
echo "after separate read+write/truncate: $stat_after"Repository: projectdiscovery/nuclei
Length of output: 259
Enforce owner-only permissions on existing files.
os.OpenFile and os.WriteFile only set the mode when creating a new file; existing files keep their current permissions. Existing 0644 files can stay readable by group and other users when the application appends, truncates, or overwrites them.
pkg/installer/versioncheck.go#L79: enforce0600on the existing ignore file.pkg/output/file_output_writer.go#L19-L21: applyChmod(0600)in both resume and non-resume paths.pkg/output/output.go#L650: applyChmod(0600)before appending debug data.pkg/reporting/exporters/jsonexporter/jsonexporter.go#L65: write through an opened descriptor and enforce0600.pkg/reporting/exporters/jsonl/jsonl.go#L73: applyChmod(0600)before writing rows.pkg/reporting/exporters/sarif/sarif.go#L191: write through an opened descriptor and enforce0600.
📍 Affects 6 files
pkg/installer/versioncheck.go#L79-L79(this comment)pkg/output/file_output_writer.go#L19-L21pkg/output/output.go#L650-L650pkg/reporting/exporters/jsonexporter/jsonexporter.go#L65-L65pkg/reporting/exporters/jsonl/jsonl.go#L73-L73pkg/reporting/exporters/sarif/sarif.go#L191-L191
🤖 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/installer/versioncheck.go` at line 79, Enforce owner-only permissions on
existing output files: in pkg/installer/versioncheck.go:79 apply Chmod(0600) to
the ignore file after opening or writing it; in
pkg/output/file_output_writer.go:19-21 apply Chmod(0600) in both resume and
non-resume paths; in pkg/output/output.go:650 apply Chmod(0600) before appending
debug data; in pkg/reporting/exporters/jsonexporter/jsonexporter.go:65 and
pkg/reporting/exporters/sarif/sarif.go:191 write through opened descriptors and
enforce 0600; and in pkg/reporting/exporters/jsonl/jsonl.go:73 apply Chmod(0600)
before writing rows.
ParseRawRequest read the request line and headers positionally: the second line was always treated as the Host line regardless of content, and header values were read by skipping exactly one byte past the colon. This meant: - a valueless header (e.g. "X-Empty:") panicked with a slice-bounds error, taking down the whole scan - a Host header anywhere but the second line was silently dropped from the header map, so header-based preconditions never matched - "Host:target" (no space after colon) parsed the host as "arget" - an absolute-form request target, as used by proxy captures (GET http://target/p HTTP/1.1), got appended to the Host rather than replacing it Every request-shaped input mode (burp, jsonl, yaml, openapi) goes through this parser. Parse headers uniformly first, look up Host by name, trim header values instead of assuming one space, and treat an absolute-form request target as replacing the authority outright - matching how pkg/protocols/http/raw.readRawRequest already handles raw template requests. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QSCzGpK3LLQDKXtHCew5CG
Fix ParseRawRequest header/host parsing panics and data loss
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@pkg/input/types/http.go`:
- Around line 257-261: Update the header parsing logic in the HTTP request
parser to recognize the Host header case-insensitively using strings.EqualFold
(or canonicalize names before storage), ensuring lower-case host values populate
rr.URL.Host for relative targets. Preserve existing header storage behavior and
add a regression test covering a lower-case host header.
- Line 264: Update the absolute-target detection in the HTTP request-target
parsing logic to perform case-insensitive HTTP/HTTPS prefix checks by
normalizing only the value used for detection, while continuing to parse the
original requestTarget unchanged. Add a regression test covering an uppercase
scheme such as HTTP:// and verify it uses absolute-form authority handling.
🪄 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 Plus
Run ID: 4cb11b5c-a6f5-4a0a-be5c-d382fc6b2ed5
📒 Files selected for processing (2)
pkg/input/types/http.gopkg/input/types/http_test.go
Comment on lines
+257
to
+261
| headerParts := strings.SplitN(line, ":", 2) | ||
| if len(headerParts) != 2 { | ||
| return nil, fmt.Errorf("invalid header line: %s", line) | ||
| } | ||
| rr.Request.Headers.Set(parts[0], parts[1][1:]) | ||
| rr.Request.Headers.Set(strings.TrimSpace(headerParts[0]), strings.TrimSpace(headerParts[1])) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Resolve the Host header without case sensitivity.
HTTP header names are case-insensitive. The parser preserves the input key casing, but later looks up only "Host". A valid host: target header leaves rr.URL.Host empty for a relative request target.
Capture the Host value with strings.EqualFold while parsing, or canonicalize header names before storage. Add a regression test for a lower-case host header.
Proposed fix
rr.Request.Headers = mapsutil.NewOrderedMap[string, string]()
+var host string
for {
// ...
- rr.Request.Headers.Set(strings.TrimSpace(headerParts[0]), strings.TrimSpace(headerParts[1]))
+ headerName := strings.TrimSpace(headerParts[0])
+ headerValue := strings.TrimSpace(headerParts[1])
+ rr.Request.Headers.Set(headerName, headerValue)
+ if strings.EqualFold(headerName, "Host") {
+ host = headerValue
+ }
}
// ...
- if host, ok := rr.Request.Headers.Get("Host"); ok {
- rr.URL.Host = strings.TrimSpace(host)
+ if host != "" {
+ rr.URL.Host = host
}Also applies to: 279-280
🤖 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/types/http.go` around lines 257 - 261, Update the header parsing
logic in the HTTP request parser to recognize the Host header case-insensitively
using strings.EqualFold (or canonicalize names before storage), ensuring
lower-case host values populate rr.URL.Host for relative targets. Preserve
existing header storage behavior and add a regression test covering a lower-case
host header.
| rr.Request.Headers.Set(strings.TrimSpace(headerParts[0]), strings.TrimSpace(headerParts[1])) | ||
| } | ||
|
|
||
| if strings.HasPrefix(requestTarget, "http://") || strings.HasPrefix(requestTarget, "https://") { |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Detect absolute HTTP targets without case sensitivity.
URI schemes are case-insensitive. An absolute request target beginning with HTTP:// or HTTPS:// does not enter this branch. It therefore misses the required absolute-form authority handling.
Normalize only the value used for prefix detection. Parse the original request target. Add a regression test with an upper-case scheme.
Proposed fix
-if strings.HasPrefix(requestTarget, "http://") || strings.HasPrefix(requestTarget, "https://") {
+normalizedRequestTarget := strings.ToLower(requestTarget)
+if strings.HasPrefix(normalizedRequestTarget, "http://") || strings.HasPrefix(normalizedRequestTarget, "https://") {📝 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 strings.HasPrefix(requestTarget, "http://") || strings.HasPrefix(requestTarget, "https://") { | |
| normalizedRequestTarget := strings.ToLower(requestTarget) | |
| if strings.HasPrefix(normalizedRequestTarget, "http://") || strings.HasPrefix(normalizedRequestTarget, "https://") { |
🤖 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/types/http.go` at line 264, Update the absolute-target detection in
the HTTP request-target parsing logic to perform case-insensitive HTTP/HTTPS
prefix checks by normalizing only the value used for detection, while continuing
to parse the original requestTarget unchanged. Add a regression test covering an
uppercase scheme such as HTTP:// and verify it uses absolute-form authority
handling.
The DAST fuzzing/stats server (-dast-server) constructed its http.Server with no timeouts, leaving it open to a Slowloris-style slow-header DoS. Only ReadHeaderTimeout is set (not ReadTimeout/WriteTimeout) since /fuzz requests can legitimately take a while to process. Same class of fix already applied to internal/httpapi/apiendpoint.go. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QSCzGpK3LLQDKXtHCew5CG
Add ReadHeaderTimeout to DASTServer's http.Server
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
2 participants
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.
Summary
This PR hardens Markdown report filename generation and remediates reachable Go vulnerabilities.
Markdown exporter
..path traversal..mdextension when long prefixes require truncation.The resulting filename format is:
<template-id>-<host>-<uuid>-<matcher-or-extractor>.mdDependency security updates
golang.org/x/textfrom v0.38.0 to v0.39.0.github.com/yuin/goldmarkfrom v1.7.13 to v1.7.17.go-github/v30togo-github/v81.go.modandgo.sum.Security scan result
The initial
govulncheckscan reported 23 reachable vulnerabilities.After remediation, one reachable advisory remains:
GO-2026-5932This advisory originates from the latest available
github.com/projectdiscovery/utils/updatedependency, which transitively usesgo-github/v30andgolang.org/x/crypto/openpgp.The Go vulnerability database does not provide a fixed
golang.org/x/cryptoversion for this advisory. Removing the final findingrequires an upstream migration in
projectdiscovery/utilsor replacement ofthe update subsystem.
Motivation
Markdown filename components contain values derived from templates and scan
targets. Without careful sanitization and truncation, these values could:
The dependency updates address reachable standard-library and third-party
security advisories identified by
govulncheck.Implementation details
Filename construction is divided into:
.mdextension.
The prefix is truncated according to the number of bytes required by the
protected suffix. This ensures the unique UUID is never removed.
UTF-8 truncation backs up from the byte limit until the resulting string is
valid UTF-8.
Testing
Passed:
go test ./pkg/reporting/exporters/markdowngo test ./pkg/external/customtemplates -run '^$'go test ./pkg/reporting/trackers/githubgo vet ./pkg/external/customtemplates ./pkg/reporting/trackers/github ./pkg/reporting/exporters/markdowngo mod verifygit diff --checkSecurity verification:
go run golang.org/x/vuln/cmd/govulncheck@latest -db=https://storage.googleapis.com/go-vulndb ./...The security scan completes and reports one remaining upstream advisory,
GO-2026-5932. It therefore exits nonzero as expected.
A complete
go test ./...run was attempted separately. Some unrelated testsrequire external templates, Interactsh, public DNS, remote targets, or a
Chromium installation and cannot pass in the restricted test environment.
Test coverage added
The Markdown exporter tests cover:
Documentation
Added:
docs/markdown-report-filenames.mddocs/security-remediation.mddocs/markdown-security-change-report.mdRisk assessment
Expected risk: Low to moderate
The filename changes are isolated to the Markdown exporter. Existing filename
consumers may observe a different filename for extractor-only results and long
result names.
The GitHub client upgrade is larger, but affected packages compile successfully
against
go-github/v81.Compatibility considerations
go-github/v30remains transitively present through upstream dependencies,even though Nuclei’s direct imports now use v81.
Rollback plan
If filename compatibility issues are discovered:
If the GitHub client migration causes runtime compatibility issues:
x/text, and Goldmark upgrades.Reviewer checklist
go-github/v30togo-github/v81migration.Merge recommendation
Use Squash and merge with the following commit message:
fix(reporting): harden Markdown filenames and remediate Go vulnerabilitiesSummary by CodeRabbit
New Features
Security
Bug Fixes
Documentation
Tests