Skip to content

fix(ssrf): use async event-hook for AsyncClient SSRF redirect check#39

Merged
hoootan merged 1 commit into
mainfrom
fix/ssrf-async-event-hook
Apr 28, 2026
Merged

fix(ssrf): use async event-hook for AsyncClient SSRF redirect check#39
hoootan merged 1 commit into
mainfrom
fix/ssrf-async-event-hook

Conversation

@hoootan
Copy link
Copy Markdown
Owner

@hoootan hoootan commented Apr 28, 2026

Summary

_check_redirect is a plain def registered as a response event hook on both httpx.Client (sync) and httpx.AsyncClient (async). httpx awaits the return value of every async-client event hook on every response; a sync function returns None, and await None raises:

TypeError: object NoneType can't be used in 'await' expression

This fires on every webhook-tool invocation through the inline executor:

agent → InlineExecutor._execute_webhook_tool
     → AsyncClient.request(...)
     → _check_redirect(response) returns None
     → httpx awaits None → TypeError

The exception bubbles up as the LLM's tool-result string "Error executing tool: object NoneType can't be used in 'await' expression", so the agent never gets a real answer from any webhook tool and the inline pipeline stalls / fails.

This blocks any inline function whose tools are tool_type='webhook'.

Reproduce on main

# 1. Create a webhook tool pointing at any public URL that returns 200 JSON
curl -X POST http://localhost:8000/api/v1/tools \
  -H "X-FlowForge-API-Key: ff_..." -H "Content-Type: application/json" \
  -d '{"name":"echo","description":"echo","parameters":{"type":"object","properties":{"x":{"type":"string"}},"required":["x"]},
       "tool_type":"webhook","webhook_url":"https://httpbin.org/post","webhook_method":"POST"}'

# 2. Inline function that uses it
curl -X POST http://localhost:8000/api/v1/functions/inline \
  -H "X-FlowForge-API-Key: ff_..." -H "Content-Type: application/json" \
  -d '{"id":"smoke","name":"Smoke","trigger":{"type":"event","value":"smoke.start"},
       "system_prompt":"Call echo(x=hi) and stop.","tools":["echo"],
       "agent_config":{"model":"claude-haiku-4-5","max_iterations":3,"max_tool_calls":3}}'

# 3. Trigger
curl -X POST http://localhost:8000/api/v1/events \
  -H "X-FlowForge-API-Key: ff_..." -H "Content-Type: application/json" \
  -d '{"name":"smoke.start","data":{"prompt":"call echo with hi"}}'

# 4. The completed run's output.messages contains:
#    {"role":"tool","content":"Error executing tool: object NoneType can't be used in 'await' expression"}

Fix

Split _check_redirect into two wrappers — _async_check_redirect (async) and _sync_check_redirect (sync) — over a shared _check_redirect_impl. Register the async one on AsyncClient and the sync one on Client. The original _check_redirect name is kept as an alias to the sync version for backward compatibility with any external callers.

Test plan

  • _check_redirect_impl unchanged behaviour for redirects (still raises ValueError when target resolves to a private IP)
  • Webhook tool invoked from an inline function returns a real response body to the LLM (no more NoneType TypeError)
  • Sync Client from create_ssrf_safe_sync_client still works
  • Optional follow-up: regression test that constructs a real AsyncClient from create_ssrf_safe_client and calls httpbin.org to verify no event-hook error

`_check_redirect` was a plain `def` registered as a response event
hook on both the sync `httpx.Client` and the async
`httpx.AsyncClient`. httpx awaits the return value of every async-
client event hook on every response; a sync function returns `None`,
and `await None` raises:

  TypeError: object NoneType can't be used in 'await' expression

That fired on every webhook tool call from the inline executor:

  agent → InlineExecutor._execute_webhook_tool
       → AsyncClient.request(...)
       → _check_redirect(response) returns None
       → httpx awaits None → TypeError

The exception bubbles up as the tool result string
"Error executing tool: object NoneType can't be used in 'await'
expression", so every webhook tool returns an error to the LLM and
the agent never makes progress. Inline functions with webhook tools
were unusable.

Reproduce on main:

  POST /api/v1/tools          (tool_type=webhook, any public URL)
  POST /api/v1/functions/inline  (system_prompt + ["that_tool"])
  POST /api/v1/events         (matching trigger, with prompt field)

The agent's first tool call returns the NoneType error; the run
either fails or the agent gives up.

Fix: split into `_async_check_redirect` (async wrapper) and
`_sync_check_redirect` (sync wrapper) over a shared sync impl,
register the right shape on the right client. The original
`_check_redirect` symbol is kept as an alias to the sync version
for any external callers.
Copilot AI review requested due to automatic review settings April 28, 2026 22:50
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes SSRF-safe httpx client behavior by ensuring the redirect-check response event hook is compatible with httpx.AsyncClient (async hooks must be awaitable), preventing webhook tool invocations from failing with TypeError: object NoneType can't be used in 'await' expression.

Changes:

  • Refactors redirect validation into _check_redirect_impl with dedicated async/sync wrappers.
  • Registers _async_check_redirect for httpx.AsyncClient and _sync_check_redirect for httpx.Client.
  • Keeps _check_redirect as an alias to the sync wrapper for backwards compatibility.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 94 to 100
def create_ssrf_safe_client(**kwargs: object) -> httpx.AsyncClient:
"""Create an async httpx client that validates redirect targets."""
kwargs.setdefault("follow_redirects", True)
kwargs.setdefault("timeout", 30.0)
return httpx.AsyncClient(
event_hooks={"response": [_check_redirect]}, **kwargs # type: ignore[arg-type]
event_hooks={"response": [_async_check_redirect]}, **kwargs # type: ignore[arg-type]
)
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a regression test to cover the async event-hook path (the original bug was await None when a sync hook was registered on AsyncClient). You can unit-test this without real network by using httpx.MockTransport with an AsyncClient created via create_ssrf_safe_client() and asserting a request/redirect does not raise TypeError and still enforces the SSRF redirect validation.

Copilot uses AI. Check for mistakes.
@hoootan hoootan merged commit 305e9e7 into main Apr 28, 2026
8 checks passed
@hoootan hoootan deleted the fix/ssrf-async-event-hook branch April 28, 2026 23:04
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.

2 participants