Summary
graphify/security.py's _ssrf_guarded_socket context manager mutates socket.getaddrinfo globally (socket.getaddrinfo = guarded) and restores it on exit. The DNS-rebinding TOCTOU mitigation itself is well-thought-out, but the global-state approach has an acknowledged hole:
- Any other thread doing a network call inside the guard window inherits the SSRF filter (probably fine).
- Any thread doing a network call outside the window during a parallel ingestion run (e.g.
requests from an MCP server, an httpx background task, urllib3 from a dependency) can see either the patched or original resolver depending on ordering -- nondeterministic and not what either caller expects.
Proposed fix
Two options:
-
Per-request resolver (cleanest): Use requests/httpx's mount/transport API to install a custom adapter that does the SSRF check during getaddrinfo for that specific connection pool, without touching the global. Bonus: callers that don't go through the guarded session aren't accidentally protected and then surprised when they aren't.
-
Thread-local sentinel: Keep the monkey-patch but gate the SSRF check on a threading.local flag, so other threads bypass the guard. Cheaper diff, but still globally mutating a stdlib function which makes reasoning hard.
Option 1 is the correct fix. Option 2 is a band-aid.
Context
Surfaced during an external code review pass. The TOCTOU resolution-cache logic is excellent and should be preserved verbatim in either fix; only the installation mechanism needs to change.
Summary
graphify/security.py's_ssrf_guarded_socketcontext manager mutatessocket.getaddrinfoglobally (socket.getaddrinfo = guarded) and restores it on exit. The DNS-rebinding TOCTOU mitigation itself is well-thought-out, but the global-state approach has an acknowledged hole:requestsfrom an MCP server, anhttpxbackground task,urllib3from a dependency) can see either the patched or original resolver depending on ordering -- nondeterministic and not what either caller expects.Proposed fix
Two options:
Per-request resolver (cleanest): Use
requests/httpx's mount/transport API to install a custom adapter that does the SSRF check duringgetaddrinfofor that specific connection pool, without touching the global. Bonus: callers that don't go through the guarded session aren't accidentally protected and then surprised when they aren't.Thread-local sentinel: Keep the monkey-patch but gate the SSRF check on a
threading.localflag, so other threads bypass the guard. Cheaper diff, but still globally mutating a stdlib function which makes reasoning hard.Option 1 is the correct fix. Option 2 is a band-aid.
Context
Surfaced during an external code review pass. The TOCTOU resolution-cache logic is excellent and should be preserved verbatim in either fix; only the installation mechanism needs to change.