Skip to content

fix: pass proxy-aware fetchFn to media understanding providers#27093

Merged
steipete merged 3 commits intoopenclaw:mainfrom
mcaxtr:fix/13299-media-provider-proxy-support
Mar 2, 2026
Merged

fix: pass proxy-aware fetchFn to media understanding providers#27093
steipete merged 3 commits intoopenclaw:mainfrom
mcaxtr:fix/13299-media-provider-proxy-support

Conversation

@mcaxtr
Copy link
Contributor

@mcaxtr mcaxtr commented Feb 26, 2026

Summary

  • Problem: runProviderEntry in runner.entries.ts never passed fetchFn to transcribeAudio() / describeVideo(). Node's built-in fetch (undici) ignores HTTPS_PROXY/HTTP_PROXY, so all media provider API calls bypass configured proxies.
  • Why it matters: Users behind corporate/enterprise proxies cannot use media understanding (audio transcription, video description) at all — API calls fail silently or time out.
  • What changed: New shared resolveProxyFetchFromEnv() reads standard proxy env vars and returns a proxy-aware fetch via undici's EnvHttpProxyAgent (respects NO_PROXY). runProviderEntry now passes this as fetchFn to audio and video provider calls.
  • What did NOT change (scope boundary): Image understanding path (uses different model-level fetch), CLI-based providers, SSRF guard logic, existing proxy support in Telegram.

Change Type (select all)

  • Bug fix
  • Feature
  • Refactor
  • Docs
  • Security hardening
  • Chore/infra

Scope (select all touched areas)

  • Gateway / orchestration
  • Skills / tool execution
  • Auth / tokens
  • Memory / storage
  • Integrations
  • API / contracts
  • UI / DX
  • CI/CD / infra

Linked Issue/PR

User-visible / Behavior Changes

Media understanding API calls (audio transcription via OpenAI/Deepgram/Mistral, video description) now honor HTTPS_PROXY, HTTP_PROXY, https_proxy, http_proxy environment variables. NO_PROXY/no_proxy exclusions are respected via undici's EnvHttpProxyAgent.

Security Impact (required)

  • New permissions/capabilities? No
  • Secrets/tokens handling changed? No
  • New/changed network calls? Yes — media provider API calls now route through the user's configured HTTP proxy when env vars are set.
  • Command/tool execution surface changed? No
  • Data access scope changed? No
  • Risk + mitigation: Proxy routing changes the network path for media API calls. The SSRF pre-flight check (resolvePinnedHostnameWithPolicy) still runs before any fetch, blocking private-IP targets. When no proxy env vars are set, behavior is identical to before (returns undefined, providers fall back to globalThis.fetch).

Repro + Verification

Environment

  • OS: Any (Linux/macOS/Windows)
  • Runtime/container: Node 22+
  • Model/provider: Any media understanding provider (OpenAI, Deepgram, Mistral, etc.)
  • Integration/channel (if any): Any channel with audio/video media

Steps

  1. Set HTTPS_PROXY=http://your-proxy:8080 in the gateway environment
  2. Send an audio message to trigger media understanding
  3. Observe the API call routes through the proxy

Expected

  • Media provider API calls go through the configured proxy

Actual

  • Media provider API calls bypass the proxy entirely, failing or timing out behind corporate firewalls

Evidence

  • Failing test/log before + passing after
  • Trace/log snippets
  • Screenshot/recording
  • Perf numbers (if relevant)

10 new tests:

  • proxy-fetch.test.ts (7): makeProxyFetch dispatcher wiring, env var resolution (all 4 variants + precedence), empty env = undefined, malformed URL = graceful undefined
  • runner.proxy.test.ts (3): audio provider receives fetchFn when proxy is set, video provider receives fetchFn, no fetchFn when no proxy env vars

Human Verification (required)

  • Verified scenarios: Unit tests confirm fetchFn passthrough for audio/video, env var precedence, graceful handling of malformed URLs
  • Edge cases checked: No proxy vars set (undefined, no behavior change), whitespace-only proxy URL, malformed proxy URL (graceful fallback)
  • What you did not verify: Live proxy environment with real corporate proxy

Compatibility / Migration

  • Backward compatible? Yes
  • Config/env changes? No (reads existing standard proxy env vars)
  • Migration needed? No

Failure Recovery (if this breaks)

  • How to disable/revert this change quickly: Unset proxy env vars (HTTPS_PROXY, etc.) to restore previous behavior
  • Files/config to restore: src/media-understanding/runner.entries.ts (remove fetchFn passthrough)
  • Known bad symptoms reviewers should watch for: Media understanding calls failing when proxy is set (would indicate proxy agent misconfiguration)

Risks and Mitigations

  • Risk: Proxy agent overrides SSRF guard's pinned dispatcher for media API calls
    • Mitigation: SSRF pre-flight hostname validation still runs before any fetch. Media provider URLs are hardcoded API endpoints (not user-supplied). When proxy is active, the proxy handles DNS resolution — pinned dispatcher is irrelevant in proxy scenarios.

@mcaxtr
Copy link
Contributor Author

mcaxtr commented Feb 26, 2026

@greptile please review this PR.

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 26, 2026

Greptile Summary

This PR adds proxy support for media understanding API calls (audio transcription and video description) by passing proxy-aware fetch functions to providers.

Key Changes

  • Implemented resolveProxyFetchFromEnv() that creates a fetch function using undici's EnvHttpProxyAgent when proxy environment variables are detected
  • Modified runProviderEntry to pass the proxy-aware fetch to both transcribeAudio() and describeVideo() calls
  • All existing media providers (OpenAI, Deepgram, Google, Moonshot) already support the optional fetchFn parameter with proper fallback to global fetch

Implementation Details

  • Respects standard proxy env vars: HTTPS_PROXY, HTTP_PROXY, https_proxy, http_proxy (in precedence order)
  • EnvHttpProxyAgent automatically handles NO_PROXY exclusions
  • Graceful error handling: returns undefined on malformed proxy URLs, falling back to direct fetch
  • SSRF pre-flight validation (hostname + DNS resolution checks) still runs before fetch execution
  • Backward compatible: no behavior change when proxy env vars are not set

Test Coverage

Added 10 new tests covering proxy fetch creation, environment variable resolution, error handling, and integration with audio/video providers.

Confidence Score: 5/5

  • This PR is safe to merge with no significant risks identified
  • Clean implementation with comprehensive test coverage, proper error handling, and backward compatibility. All existing providers already support the fetchFn parameter. SSRF pre-flight validation continues to run. The proxy DNS resolution behavior is expected and appropriate for this use case where media provider URLs are hardcoded API endpoints.
  • No files require special attention

Last reviewed commit: 53bd6d4

@mcaxtr mcaxtr force-pushed the fix/13299-media-provider-proxy-support branch 2 times, most recently from 56692b2 to 344259d Compare February 28, 2026 04:41
mcaxtr added 2 commits March 1, 2026 19:49
Move makeProxyFetch to src/infra/net/proxy-fetch.ts and add
resolveProxyFetchFromEnv which reads standard proxy env vars
(HTTPS_PROXY, HTTP_PROXY, and lowercase variants) and returns a
proxy-aware fetch via undici's EnvHttpProxyAgent. Telegram re-exports
from the shared location to avoid duplication.
runProviderEntry now calls resolveProxyFetchFromEnv() and passes the
result as fetchFn to transcribeAudio/describeVideo, so media provider
API calls respect HTTPS_PROXY/HTTP_PROXY behind corporate proxies.
@mcaxtr mcaxtr force-pushed the fix/13299-media-provider-proxy-support branch from 344259d to 8b15600 Compare March 1, 2026 22:53
@aisle-research-bot
Copy link

aisle-research-bot bot commented Mar 1, 2026

🔒 Aisle Security Analysis

We found 1 potential security issue(s) in this PR:

# Severity Title
1 🔵 Low Potential proxy credential leakage via logging raw EnvHttpProxyAgent error message

1. 🔵 Potential proxy credential leakage via logging raw EnvHttpProxyAgent error message

Property Value
Severity Low
CWE CWE-532
Location src/infra/net/proxy-fetch.ts:42-45

Description

resolveProxyFetchFromEnv() catches errors from EnvHttpProxyAgent construction and logs err.message directly.

If the proxy environment variable contains credentials (e.g. http://user:pass@​proxy:8080) and undici includes the invalid proxy URL in the thrown error message, those credentials can be written to console/file logs.

  • Input: process.env.HTTPS_PROXY / HTTP_PROXY / https_proxy / http_proxy
  • Trigger: malformed/unsupported proxy URL causing new EnvHttpProxyAgent() to throw
  • Sink: logWarn(...) with ${err.message}

Vulnerable code:

logWarn(
  `Proxy env var set but agent creation failed — falling back to direct fetch: ${err instanceof Error ? err.message : String(err)}`,
);

Recommendation

Avoid logging raw error strings that may include secrets/credentials. Instead:

  1. Log a generic error message without embedding raw values, or
  2. Explicitly sanitize the proxy URL before logging, and use centralized error formatting/redaction.

Example (sanitize proxy URL credentials + safer error formatting):

import { formatErrorMessage } from "../infra/errors.js"; // or similar helper

function redactProxyUrl(raw: string): string {
  try {
    const u = new URL(raw);
    if (u.username || u.password) {
      u.username = "***";
      u.password = "***";
    }
    return u.toString();
  } catch {
    return "<invalid-proxy-url>";
  }
}// ...
} catch (err) {
  logWarn(
    `Proxy env var set but agent creation failed (proxy=${redactProxyUrl(proxyUrl)}): ${formatErrorMessage(err)}`,
  );
  return undefined;
}

Also consider adding a redaction rule for scheme://user:pass@​host-style URLs if proxy credentials are expected to be used.


Analyzed PR: #27093 at commit a8fbb69

Last updated on: 2026-03-01T23:58:57Z

@openclaw-barnacle openclaw-barnacle bot added the channel: telegram Channel integration: telegram label Mar 1, 2026
@mcaxtr
Copy link
Contributor Author

mcaxtr commented Mar 1, 2026

Addressed the aisle security findings:

Finding 1 (fail-open proxy resolution): Valid — added a logWarn() when a proxy env var is set but EnvHttpProxyAgent construction fails, so the fallback to direct fetch is no longer silent. Full fail-closed (throwing) would be a breaking change for users with misconfigured env vars, so warning + graceful fallback matches the existing codebase pattern (fetch-guard.ts also uses new EnvHttpProxyAgent() without hard failure).

Finding 2 (env proxy exposing API keys): This is the intended behavior of the PR — HTTP(S)_PROXY is a well-established Unix convention and the codebase already routes credential-bearing requests through env-configured proxies in fetch-guard.ts (used by web tools). This PR extends the same pattern to media provider calls. Requiring a separate opt-in flag would diverge from how the rest of the codebase handles proxy configuration. HTTPS protects credentials in transit even through a proxy (the proxy only sees the CONNECT target, not headers/body).

@steipete steipete merged commit 58cde87 into openclaw:main Mar 2, 2026
22 of 26 checks passed
@steipete
Copy link
Contributor

steipete commented Mar 2, 2026

Landed via temp rebase onto main.

  • Gate: pnpm vitest src/infra/net/proxy-fetch.test.ts src/media-understanding/runner.proxy.test.ts
  • Land commit: f416d11d64c89cfc3830c9715c2f7ff87764c3ad
  • Merge commit: 58cde87

Thanks @mcaxtr!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

channel: telegram Channel integration: telegram size: M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Media Understanding providers don't respect proxy settings

2 participants