feat: add security.txt detection flag#2472
feat: add security.txt detection flag#2472iacker wants to merge 2 commits intoprojectdiscovery:devfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughAdded a new Changes
Sequence Diagram(s)sequenceDiagram
participant CLI
participant Runner
participant HTTP as "Target HTTP"
participant Analyzer
CLI->>Runner: start with --security-txt
Runner->>Runner: extend match strings/status/URIs, force JSON, normalize URIs
Runner->>HTTP: request /.well-known/security.txt (and /security.txt)
HTTP-->>Runner: response (status, headers, body)
Runner->>Analyzer: analyze response
Analyzer-->>Runner: isSecurityTxt? (200 && content-type ok && contains "Contact:")
Runner-->>Runner: set Result.SecurityTxt and include in JSON output
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Neo - PR Security ReviewNo security issues found Hardening Notes
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@runner/runner.go`:
- Around line 333-341: The code appends "200" to options.OutputMatchStatusCode
when options.SecurityTxt is true, but parsing into the internal matchStatusCode
slice happens earlier in ValidateOptions()/ParseOptions(), so the appended 200
is ignored; to fix, ensure the status-code parsing runs after you modify
OutputMatchStatusCode — either move the append logic for
options.OutputMatchStatusCode (and any related options.RequestURIs changes) to
before ValidateOptions()/New() in ParseOptions(), or call the same parsing
routine that produces matchStatusCode again after the append so matchStatusCode
includes "200" (refer to options.SecurityTxt, options.OutputMatchStatusCode,
ValidateOptions(), ParseOptions(), New(), and the matchStatusCode parsing
logic).
- Around line 3021-3031: The isSecurityTxt function only checks for the exact
case "Contact:" which can miss valid security.txt field names; update the check
in isSecurityTxt to perform a case-insensitive search for the "contact:" field
(e.g., normalize resp.RawData to lowercase and check for "contact:" or use a
case-insensitive regex) so that variants like "contact:" or "CONTACT:" are
detected while keeping the existing Content-Type and status checks intact.
🪄 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: 91bc8858-d780-49ff-84dd-f3e20a1b675c
📒 Files selected for processing (4)
runner/options.gorunner/runner.gorunner/runner_test.gorunner/types.go
| if options.SecurityTxt { | ||
| options.OutputMatchString = append(options.OutputMatchString, "Contact:") | ||
| options.OutputMatchStatusCode = appendCommaSeparatedValue(options.OutputMatchStatusCode, "200") | ||
| options.RequestURIs = appendCommaSeparatedValue(options.RequestURIs, "/.well-known/security.txt,/security.txt") | ||
| options.JSONOutput = true | ||
| } | ||
| if options.RequestURIs != "" { | ||
| options.requestURIs = normalizeRequestURIs(options.RequestURIs) | ||
| } |
There was a problem hiding this comment.
Critical: OutputMatchStatusCode modification occurs after parsing, making status code filter ineffective.
When --security-txt is enabled, this code appends "200" to options.OutputMatchStatusCode. However, ValidateOptions() (called before New() in ParseOptions()) has already parsed OutputMatchStatusCode into the matchStatusCode slice used for filtering at line 1204.
As a result, the 200 status code match won't be enforced when using --security-txt alone, potentially returning non-200 responses.
🐛 Proposed fix: Parse the status code after modification
if options.SecurityTxt {
options.OutputMatchString = append(options.OutputMatchString, "Contact:")
options.OutputMatchStatusCode = appendCommaSeparatedValue(options.OutputMatchStatusCode, "200")
options.RequestURIs = appendCommaSeparatedValue(options.RequestURIs, "/.well-known/security.txt,/security.txt")
options.JSONOutput = true
+ // Re-parse matchStatusCode since we modified OutputMatchStatusCode after initial parsing
+ if parsed, err := stringz.StringToSliceInt(options.OutputMatchStatusCode); err == nil {
+ options.matchStatusCode = parsed
+ }
}📝 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.
| if options.SecurityTxt { | |
| options.OutputMatchString = append(options.OutputMatchString, "Contact:") | |
| options.OutputMatchStatusCode = appendCommaSeparatedValue(options.OutputMatchStatusCode, "200") | |
| options.RequestURIs = appendCommaSeparatedValue(options.RequestURIs, "/.well-known/security.txt,/security.txt") | |
| options.JSONOutput = true | |
| } | |
| if options.RequestURIs != "" { | |
| options.requestURIs = normalizeRequestURIs(options.RequestURIs) | |
| } | |
| if options.SecurityTxt { | |
| options.OutputMatchString = append(options.OutputMatchString, "Contact:") | |
| options.OutputMatchStatusCode = appendCommaSeparatedValue(options.OutputMatchStatusCode, "200") | |
| options.RequestURIs = appendCommaSeparatedValue(options.RequestURIs, "/.well-known/security.txt,/security.txt") | |
| options.JSONOutput = true | |
| // Re-parse matchStatusCode since we modified OutputMatchStatusCode after initial parsing | |
| if parsed, err := stringz.StringToSliceInt(options.OutputMatchStatusCode); err == nil { | |
| options.matchStatusCode = parsed | |
| } | |
| } | |
| if options.RequestURIs != "" { | |
| options.requestURIs = normalizeRequestURIs(options.RequestURIs) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@runner/runner.go` around lines 333 - 341, The code appends "200" to
options.OutputMatchStatusCode when options.SecurityTxt is true, but parsing into
the internal matchStatusCode slice happens earlier in
ValidateOptions()/ParseOptions(), so the appended 200 is ignored; to fix, ensure
the status-code parsing runs after you modify OutputMatchStatusCode — either
move the append logic for options.OutputMatchStatusCode (and any related
options.RequestURIs changes) to before ValidateOptions()/New() in
ParseOptions(), or call the same parsing routine that produces matchStatusCode
again after the append so matchStatusCode includes "200" (refer to
options.SecurityTxt, options.OutputMatchStatusCode, ValidateOptions(),
ParseOptions(), New(), and the matchStatusCode parsing logic).
| func isSecurityTxt(resp *httpx.Response) bool { | ||
| if resp == nil || resp.StatusCode != 200 { | ||
| return false | ||
| } | ||
| contentType := strings.ToLower(resp.GetHeaderPart("Content-Type", ";")) | ||
| if contentType != "" && !strings.Contains(contentType, "text/plain") { | ||
| return false | ||
| } | ||
| body := string(resp.RawData) | ||
| return strings.Contains(body, "Contact:") | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
RFC 9116 security.txt field name case sensitivity
💡 Result:
RFC 9116 specifies that security.txt field names are case-insensitive. It defines the field “name” as a field-name per RFC 5322 and then explicitly states: “Field names are case insensitive (as per Section 2.3 of RFC 5234).” (datatracker.ietf.org)
Citations:
Case-sensitive "Contact:" check may miss valid security.txt files.
Per RFC 9116, field names in security.txt are case-insensitive. The current check strings.Contains(body, "Contact:") will miss valid files using lowercase contact: or mixed case variants.
Suggested fix
func isSecurityTxt(resp *httpx.Response) bool {
if resp == nil || resp.StatusCode != 200 {
return false
}
contentType := strings.ToLower(resp.GetHeaderPart("Content-Type", ";"))
if contentType != "" && !strings.Contains(contentType, "text/plain") {
return false
}
- body := string(resp.RawData)
- return strings.Contains(body, "Contact:")
+ body := strings.ToLower(string(resp.RawData))
+ return strings.Contains(body, "contact:")
}📝 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.
| func isSecurityTxt(resp *httpx.Response) bool { | |
| if resp == nil || resp.StatusCode != 200 { | |
| return false | |
| } | |
| contentType := strings.ToLower(resp.GetHeaderPart("Content-Type", ";")) | |
| if contentType != "" && !strings.Contains(contentType, "text/plain") { | |
| return false | |
| } | |
| body := string(resp.RawData) | |
| return strings.Contains(body, "Contact:") | |
| } | |
| func isSecurityTxt(resp *httpx.Response) bool { | |
| if resp == nil || resp.StatusCode != 200 { | |
| return false | |
| } | |
| contentType := strings.ToLower(resp.GetHeaderPart("Content-Type", ";")) | |
| if contentType != "" && !strings.Contains(contentType, "text/plain") { | |
| return false | |
| } | |
| body := strings.ToLower(string(resp.RawData)) | |
| return strings.Contains(body, "contact:") | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@runner/runner.go` around lines 3021 - 3031, The isSecurityTxt function only
checks for the exact case "Contact:" which can miss valid security.txt field
names; update the check in isSecurityTxt to perform a case-insensitive search
for the "contact:" field (e.g., normalize resp.RawData to lowercase and check
for "contact:" or use a case-insensitive regex) so that variants like "contact:"
or "CONTACT:" are detected while keeping the existing Content-Type and status
checks intact.
Addresses CodeRabbit review finding that docstring coverage was 20%. Adds Go doc comments to all new helper functions and tests: - appendCommaSeparatedValue - normalizeRequestURIs - isSecurityTxt - TestNormalizeRequestURIs - TestAppendCommaSeparatedValue - TestIsSecurityTxt
1 participant
Summary
--security-txtflag to probe/.well-known/security.txtand/security.txtContact:field, and reject non-plain-text responses to reduce soft-404 noisesecurity_txtboolean in structured output and add focused tests for the helper logicTesting
go test ./runner -run 'Test(NormalizeRequestURIs|AppendCommaSeparatedValue|IsSecurityTxt|Runner_CSVRow)' -count=1go test ./...Closes #2468
Summary by CodeRabbit
New Features
Tests