Enhance security.audit tool: rate-limit retest, subdomain probe, CORS wildcard flag, and richer port info#19
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| request = Request(url, headers={"User-Agent": "AgentTools-RateProbe/1.0"}) | ||
| with urlopen(request, timeout=timeout) as resp: | ||
| return {"status": resp.status, "retry_after": resp.headers.get("Retry-After")} | ||
| except Exception as exc: | ||
| return {"status": "error", "error": str(exc)[:120]} |
There was a problem hiding this comment.
When the target enforces rate limiting, urllib.request.urlopen raises an HTTPError for 429 responses, but this helper catches all exceptions and returns {"status": "error"}. As a result, rate_limit_triggered never becomes true even when the server is actively throttling, so the new retest logic yields false negatives in real rate-limited environments. Consider catching HTTPError separately and using its status code (e.g., exc.code) so 429s are surfaced.
Useful? React with 👍 / 👎.
| except Exception as exc: | ||
| results.append({"path": path, "status": "error", "error": str(exc)[:120]}) | ||
| return [r for r in results if r.get("status") not in (404, "error")] |
There was a problem hiding this comment.
urlopen raises HTTPError for any 4xx/5xx response, so protected endpoints that return 401/403 (common for admin/debug routes) are stored as status: "error" and then filtered out. This means existing endpoints are silently ignored unless they return 2xx, yielding false negatives for exposed paths that require auth. Handling HTTPError by capturing the status code (and not filtering it out) would preserve those findings.
Useful? React with 👍 / 👎.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| results.append({"subdomain": fqdn, "resolved": True}) | ||
| except Exception: | ||
| continue | ||
| return {"status": "completed", "subdomains": results} |
There was a problem hiding this comment.
Subdomain probe creates invalid FQDNs for subdomained hosts
The _probe_subdomains method constructs FQDNs by prepending subdomain prefixes to base_host, but base_host is extracted directly from the URL's hostname which may already include a subdomain. For example, if the target URL is https://www.example.com/, base_host will be www.example.com, and the probe will try to resolve api.www.example.com instead of api.example.com. This makes the subdomain enumeration feature produce incorrect DNS lookups and miss legitimate subdomains.
Additional Locations (1)
| for header, data in header_results.items(): | ||
| if not data["present"]: | ||
| score -= 5 | ||
| findings.append(f"Missing {header} header") |
There was a problem hiding this comment.
CORS scoring logic penalizes secure sites, ignores wildcards
The access-control-allow-origin header is incorrectly treated as a "required" security header, causing _score_findings to penalize sites that don't set it. However, a missing CORS header is actually the secure default (same-origin policy applies), while a wildcard value * is the actual security risk. The code computes a wildcard flag but never uses it in scoring. This inverts the security logic: secure configurations get penalized while insecure wildcard CORS configurations receive no penalty.
Additional Locations (1)
1 participant
Motivation
rate_limit_retest) and optional subdomain enumeration (subdomain_probe) without changing the core behavior.Access-Control-Allow-Originand missing Subresource Integrity (SRI) on external assets.tools_v2/tool_registry.lock.json).Description
get_specincludingrate_limit_retest,retest_delay,subdomain_probe, andsubdomains, and preserved the tool under 400 LOC (file is 396 lines)._check_rate_limitsto optionally perform a second probe round and annotate results with `Codex Task
Note
Introduces a compact web security auditing adapter and registers it for discovery.
tools_v2/categories/security_audit_tools.pyimplementingSecurityAuditToolwithget_spec, validation, and execution pipelineServer/X-Powered-By/Viainfosecurity.auditintools_v2/tool_registry.lock.json(also normalizesanalysis.duplicatesentry)Written by Cursor Bugbot for commit a2e5fa7. Configure here.