test: add SSRF guard coverage for URL credential bypass vectors by teddytennant · Pull Request #50523 · openclaw/openclaw

teddytennant · 2026-03-19T16:19:57Z

Summary

Add 3 new test cases to src/infra/net/fetch-guard.ssrf.test.ts documenting that the SSRF guard correctly handles URL credential bypass attempts:
1. http://attacker.com@127.0.0.1:8080/internal — URL parser extracts hostname as 127.0.0.1, correctly blocked
2. http://user:pass@[::1]:8080/internal — IPv6 loopback with credentials, correctly blocked
3. Redirect to http://public@127.0.0.1:6379/ — redirect chain credential bypass, correctly blocked

Motivation: The SSRF guard correctly blocks these vectors via JavaScript's URL constructor (which extracts the real hostname regardless of credentials), but this behavior was not documented in the test suite. Explicit test coverage prevents regressions if the URL parsing or hostname extraction logic changes, and documents expected behavior for security review.

Test plan

All 17 SSRF guard tests pass (14 existing + 3 new)
No code changes — test-only PR

Copilot

Pull request overview

Adds regression coverage to the SSRF fetch guard test suite to explicitly document/verify that URL “credential” (user[:pass]@host) parsing cannot be used to bypass private/loopback destination blocking.

Changes:

Add a test that blocks an IPv4 loopback destination obscured via attacker.com@127.0.0.1.
Add a test that blocks an IPv6 loopback destination with credentials (user:pass@[::1]).
Add a test that blocks a redirect to a credential-obscured private host (public@127.0.0.1).

You can also share your feedback on Copilot code review. Take the survey.

greptile-apps · 2026-03-19T16:22:16Z

Greptile Summary

This is a test-only PR that adds three new test cases to src/infra/net/fetch-guard.ssrf.test.ts, documenting that the SSRF guard correctly blocks URL credential bypass vectors (userinfo-in-hostname, IPv6 loopback with credentials, and redirect to a credential-obfuscated private host). The tests are well-motivated and the guard's behavior is correctly described: JavaScript's URL constructor always extracts the real hostname regardless of embedded credentials, so the existing resolvePinnedHostnameWithPolicy check correctly blocks these vectors before any network I/O.

The three new test cases accurately reflect the implementation's behavior and use consistent patterns with the rest of the suite.
The redirect-chain test (blocks redirect to a URL using credentials to obscure a private host) is missing a expect(fetchImpl).toHaveBeenCalledTimes(1) assertion — the analogous test at line 136–145 includes this to confirm the guard only blocked at the redirect step, not before. Without it the test passes even if fetchImpl is never called.

Confidence Score: 4/5

This test-only PR is safe to merge; the new tests correctly document existing guard behavior with no production code changes.
The three new tests are logically sound and consistent with the implementation. The only gap is a missing toHaveBeenCalledTimes(1) assertion in the redirect test, which weakens that test slightly but does not introduce any regression risk.
No files require special attention beyond the minor assertion gap noted in the redirect test.

Prompt To Fix All With AI

This is a comment left during a code review.
Path: src/infra/net/fetch-guard.ssrf.test.ts
Line: 304-312

Comment:
**Missing call-count assertion on redirect test**

The analogous existing test at line 136–145 captures the returned `fetchImpl` and asserts `expect(fetchImpl).toHaveBeenCalledTimes(1)`, confirming that the guard allowed the initial request to the public host before blocking the redirect. This new redirect test omits that assertion, making it slightly weaker: it would still pass even if the guard blocked the *first* request (i.e. if `fetchImpl` was never called at all).

Consider capturing the return value of `expectRedirectFailure` and adding `expect(fetchImpl).toHaveBeenCalledTimes(1)` after `await expectRedirectFailure(...)`, consistent with how the existing redirect-chain test is written.

How can I resolve this? If you propose a fix, please make it concise.

_{Last reviewed commit: "security: add SSRF g..."}

greptile-apps · 2026-03-19T16:22:19Z

+  it("blocks redirect to a URL using credentials to obscure a private host", async () => {
+    const lookupFn = createPublicLookup();
+    await expectRedirectFailure({
+      url: "https://public.example/start",
+      responses: [redirectResponse("http://public@127.0.0.1:6379/")],
+      expectedError: /private|internal|blocked/i,
+      lookupFn,
+    });
+  });


Missing call-count assertion on redirect test

The analogous existing test at line 136–145 captures the returned fetchImpl and asserts expect(fetchImpl).toHaveBeenCalledTimes(1), confirming that the guard allowed the initial request to the public host before blocking the redirect. This new redirect test omits that assertion, making it slightly weaker: it would still pass even if the guard blocked the first request (i.e. if fetchImpl was never called at all).

Consider capturing the return value of expectRedirectFailure and adding expect(fetchImpl).toHaveBeenCalledTimes(1) after await expectRedirectFailure(...), consistent with how the existing redirect-chain test is written.

Prompt To Fix With AI

This is a comment left during a code review. Path: src/infra/net/fetch-guard.ssrf.test.ts Line: 304-312 Comment: **Missing call-count assertion on redirect test** The analogous existing test at line 136–145 captures the returned `fetchImpl` and asserts `expect(fetchImpl).toHaveBeenCalledTimes(1)`, confirming that the guard allowed the initial request to the public host before blocking the redirect. This new redirect test omits that assertion, making it slightly weaker: it would still pass even if the guard blocked the *first* request (i.e. if `fetchImpl` was never called at all). Consider capturing the return value of `expectRedirectFailure` and adding `expect(fetchImpl).toHaveBeenCalledTimes(1)` after `await expectRedirectFailure(...)`, consistent with how the existing redirect-chain test is written. How can I resolve this? If you propose a fix, please make it concise.

WingedDragon

✅ Approved

Scope: Security — adds SSRF guard test coverage for URL credential bypass vectors (http://attacker.com@127.0.0.1, http://user:pass@[::1]).

Strengths:

Tests verify that credentials in URLs don't obscure private host detection
Covers both direct requests and redirect-to-credential-obscured-URL scenarios
IPv6 loopback ([::1]) is covered

Note: These tests only add coverage for existing behavior. If the SSRF guard already blocks these, great. If these tests fail, it means there's a real bypass that needs fixing. Verify CI is green.

No concerns. Good security test coverage addition. Ship it.

vincentkoc · 2026-03-20T19:45:01Z

Added the missing redirect call-count assertion so the test now proves the initial public request is attempted before the redirect gets blocked.

…claw#50523) * security: add SSRF guard tests for URL credential bypass vectors * test(security): strengthen SSRF redirect guard coverage --------- Co-authored-by: Vincent Koc <vincentkoc@ieee.org>

…claw#50523) * security: add SSRF guard tests for URL credential bypass vectors * test(security): strengthen SSRF redirect guard coverage --------- Co-authored-by: Vincent Koc <vincentkoc@ieee.org> (cherry picked from commit a20ba74)

…claw#50523) * security: add SSRF guard tests for URL credential bypass vectors * test(security): strengthen SSRF redirect guard coverage --------- Co-authored-by: Vincent Koc <vincentkoc@ieee.org>

security: add SSRF guard tests for URL credential bypass vectors

ad615d1

Copilot AI review requested due to automatic review settings March 19, 2026 16:19

openclaw-barnacle Bot added the size: XS label Mar 19, 2026

Copilot AI reviewed Mar 19, 2026

View reviewed changes

greptile-apps Bot reviewed Mar 19, 2026

View reviewed changes

WingedDragon reviewed Mar 19, 2026

View reviewed changes

test(security): strengthen SSRF redirect guard coverage

f29b651

vincentkoc merged commit a20ba74 into openclaw:main Mar 20, 2026
3 checks passed

github-actions Bot mentioned this pull request Mar 20, 2026

📡 Upstream Digest — 2026-03-20 20:23 UTC curtismercier/openclaw-mods#318

Open

alexey-pelykh mentioned this pull request Mar 23, 2026

Cherry-pick batch: Security vulnerability fixes (18 commits) remoteclaw/remoteclaw#1845

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

test: add SSRF guard coverage for URL credential bypass vectors#50523

test: add SSRF guard coverage for URL credential bypass vectors#50523
vincentkoc merged 2 commits intoopenclaw:mainfrom
teddytennant:security/ssrf-url-credential-test

teddytennant commented Mar 19, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

greptile-apps Bot commented Mar 19, 2026

Uh oh!

greptile-apps Bot Mar 19, 2026

Uh oh!

WingedDragon left a comment

Uh oh!

vincentkoc commented Mar 20, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

teddytennant commented Mar 19, 2026

Summary

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

greptile-apps Bot commented Mar 19, 2026

Greptile Summary

Confidence Score: 4/5

Uh oh!

greptile-apps Bot Mar 19, 2026

Choose a reason for hiding this comment

Uh oh!

WingedDragon left a comment

Choose a reason for hiding this comment

Uh oh!

vincentkoc commented Mar 20, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants