Skip to content

fix(security): SSRF guard on Azure endpoint + validate Apify actor#163

Open
chakkiyar102 wants to merge 1 commit into
vasu-devs:mainfrom
chakkiyar102:fix/ssrf-guard-llm-endpoints-and-apify-actor
Open

fix(security): SSRF guard on Azure endpoint + validate Apify actor#163
chakkiyar102 wants to merge 1 commit into
vasu-devs:mainfrom
chakkiyar102:fix/ssrf-guard-llm-endpoints-and-apify-actor

Conversation

@chakkiyar102

Copy link
Copy Markdown

Summary

Two small, surgical security hardening fixes found during a review of the discovery + LLM-provider surfaces. No behavior change for legitimate/public endpoints; only internal/loopback/malformed values are now refused at the same boundary the rest of the app already enforces.

1. SSRF guard on the Azure OpenAI endpoint (api/routers/settings.py)

llm/client.py already validates provider base URLs via _validate_base_url() (which reuses core.url_guard.is_public_host — loopback / 169.254.169.254 metadata / private LAN are refused). However, api/routers/settings.py rebuilds the Azure endpoint independently in probe_provider_key and list_provider_models and fetches it without that guard.

A configured azure_openai_endpoint of e.g. http://127.0.0.1 or http://169.254.169.254/... would therefore be probed, unlike every discovery fetch (which goes through guarded_async_client).

Fix: reuse the existing llm.client._validate_base_url so the probe reflects exactly what the real client would accept:

  • loopback / private / metadata endpoint → _validate_base_url raises → probe_provider_key returns "unreachable", list_provider_models returns []no request made.

Note: _OPENAI_COMPAT_BASE_URLS are hardcoded public hosts and were not affected; only the user-configurable Azure endpoint was the gap.

2. Validate Apify actor (discovery/sources/apify.py)

run_actor interpolates actor straight into https://api.apify.com/v2/acts/{actor}/.... The actor comes from user settings, so a malformed value could reshape the URL path. Added an allowlist: each <owner>/<name> segment must start and end alphanumeric (dots/dashes/underscores allowed inside), rejecting .., ../x, empty, and whitespace.

Tests

  • tests/test_provider_endpoint_ssrf.py — Azure loopback/metadata/private endpoints resolve to "unreachable"; missing endpoint stays "unchecked" (no network).
  • tests/test_apify_actor_validation.py — regex accept/reject matrix + run_actor raises ValueError on traversal/empty actor before any HTTP call.
  • Existing tests/test_url_guard.py still passes.
backend $ uv run python -m pytest tests/test_apify_actor_validation.py tests/test_provider_endpoint_ssrf.py tests/test_url_guard.py -q
47 passed
backend $ uv run ruff check <changed files>   # All checks passed

Out of scope (not changed here)

  • The documented DNS-rebinding residual in core/url_guard.py (check-time vs connect-time TOCTOU) — closing it fully needs IP-pinning at the httpcore transport layer; deliberately left as a larger follow-up rather than shipping an unvalidated transport rewrite here.
  • Apify token is still sent as a query param; header transport is a minor hygiene improvement left for a separate change to avoid any auth-compat risk.

Type of change

  • Security hardening (defense-in-depth; no new external behavior for valid config)
  • Tests added

…RF/path injection)

The settings router rebuilt the Azure OpenAI endpoint from user config and
fetched it to validate the key / list models, duplicating the LLM client's
URL construction but skipping its _validate_base_url SSRF guard. A configured
loopback / metadata / private endpoint could therefore be reached by the
probe and model-list paths, unlike the guarded discovery fetchers.

- api/routers/settings.py: reuse llm.client._validate_base_url on the Azure
  endpoint in both probe_provider_key and list_provider_models, so the probe
  reflects exactly what the client would accept (loopback/private -> raises
  -> 'unreachable' / empty model list, no request made).
- discovery/sources/apify.py: allowlist the actor interpolated into the
  acts/{actor}/... path so a configured actor cannot reshape the URL (path
  traversal / host smuggling). Each <owner>/<name> segment must start and end
  alphanumeric.
- Tests: tests/test_provider_endpoint_ssrf.py and
  tests/test_apify_actor_validation.py cover both fixes without network.

All targeted unit tests + ruff pass.
@chakkiyar102
chakkiyar102 requested a review from vasu-devs as a code owner July 2, 2026 14:22
@vercel

vercel Bot commented Jul 2, 2026

Copy link
Copy Markdown

Someone is attempting to deploy a commit to the Vasu-Devs' projects Team on Vercel.

A member of the Team first needs to authorize it.

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