Skip to content

security: fix SSRF in serverless agent discovery (CodeQL alert #32)#490

Merged
AbirAbbas merged 1 commit intomainfrom
security/codeql-32-ssrf-serverless-discovery
Apr 20, 2026
Merged

security: fix SSRF in serverless agent discovery (CodeQL alert #32)#490
AbirAbbas merged 1 commit intomainfrom
security/codeql-32-ssrf-serverless-discovery

Conversation

@AbirAbbas
Copy link
Copy Markdown
Contributor

Summary

  • Fixes CodeQL go/request-forgery alert #32 (CWE-918) on RegisterServerlessAgentHandler.
  • Splits normalizeServerlessDiscoveryURL into a parse helper that returns a freshly constructed *url.URL (validated scheme literal, allowlisted host, path.Clean-ed path — no user-info / query / fragment ever propagated).
  • Handler now builds discoveryURL from those sanitized components rather than string-concatenating the caller-supplied value, so no tainted string flows into http.NewRequestWithContext.
  • Also rejects opaque URLs (e.g. http:foo/path).

Test plan

  • go test ./control-plane/internal/handlers/... — full suite green (69.7s)
  • New unit tests in nodes_serverless_ssrf_test.go:
    • Path-traversal cleaning (./, ../, trailing slashes)
    • Rebuilds from validated components (no User/RawQuery/Fragment/Opaque)
    • Rejects opaque URLs
    • Normalizes IPv6 zero-host [::] to localhost
  • New functional test TestRegisterServerlessAgentHandler_DiscoverySanitizedFunctional:
    • Non-loopback host outside allowlist is rejected without any outbound network call
    • End-to-end: handler contacts the allowlisted server only on the sanitized /discover path even when input contains ./ and ../ segments
    • Opaque URL rejected before any network call
  • Coverage on touched symbols: normalizeServerlessDiscoveryURL 100%, parseServerlessDiscoveryURL 90.6%, RegisterServerlessAgentHandler 79.2%
  • go vet ./control-plane/internal/handlers/... clean

🤖 Generated with Claude Code

…ted parts

Fixes CodeQL go/request-forgery (CWE-918) alert #32 on
RegisterServerlessAgentHandler by splitting normalizeServerlessDiscoveryURL
into a parse helper that returns a freshly constructed *url.URL (validated
scheme literal, allowlisted host, path.Clean-ed path, no user/query/fragment
propagated) and having the handler build the /discover URL from those
sanitized components rather than string-concatenating the caller-supplied
value. Also rejects opaque URLs.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@AbirAbbas AbirAbbas requested a review from a team as a code owner April 20, 2026 20:22
@github-actions
Copy link
Copy Markdown
Contributor

📊 Coverage gate

Thresholds from .coverage-gate.toml: per-surface ≥ 86%, aggregate ≥ 88%, max per-surface regression ≤ 1.0 pp, max aggregate regression ≤ 0.50 pp.

Surface Current Baseline Δ
control-plane 87.30% 87.30% → +0.00 pp 🟡
sdk-go 91.00% 90.70% ↑ +0.30 pp 🟢
sdk-python 93.63% 93.63% ↑ +0.00 pp 🟢
sdk-typescript 92.65% 92.56% ↑ +0.09 pp 🟢
web-ui 90.03% 90.01% ↑ +0.02 pp 🟢
aggregate 89.02% 89.01% ↑ +0.01 pp 🟡

✅ Gate passed

No surface regressed past the allowed threshold and the aggregate stayed above the floor.

@github-actions
Copy link
Copy Markdown
Contributor

📐 Patch coverage gate

Threshold: 80% on lines this PR touches vs origin/main (from .coverage-gate.toml:thresholds.min_patch).

Surface Touched lines Patch coverage Status
control-plane 46 93.00%
sdk-go 0 ➖ no changes
sdk-python 0 ➖ no changes
sdk-typescript 0 ➖ no changes
web-ui 0 ➖ no changes

✅ Patch gate passed

Every surface whose lines were touched by this PR has patch coverage at or above the threshold.

@AbirAbbas AbirAbbas merged commit e21ced7 into main Apr 20, 2026
28 checks passed
@AbirAbbas AbirAbbas deleted the security/codeql-32-ssrf-serverless-discovery branch April 20, 2026 20:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant