[wangyusheng1985]

Configure the SecOps SOAR HTTP client proxy with the optional SOAR_PROXY environment variable. GET, POST, and PATCH requests use it when set and continue connecting directly when unset. Document the setting and cover both paths with unit tests. TLS verification and error handling are unchanged.

Fixes #177

Validation: python3 -m pytest -q server/secops-soar/tests/unit (13 passed), targeted compileall, and git diff --check.

[gemini-code-assist]

Code Review

This pull request introduces support for an optional HTTP proxy (SOAR_PROXY) for requests made to the SecOps SOAR API, updating the HttpClient, bindings, documentation, and adding unit tests. The review feedback recommends sanitizing the proxy string to handle empty or whitespace-only values to prevent potential ValueError exceptions in aiohttp, and suggests updating the unit tests to cover these edge cases.

[gemini-code-assist]

If the SOAR_PROXY environment variable is set to an empty string or contains only whitespace, self.proxy will be an empty string. Passing an empty string as the proxy argument to aiohttp request methods will cause a ValueError (e.g., ValueError: Only http and https proxies are supported).

To prevent this, sanitize the proxy input in __init__ by converting empty or whitespace-only strings to None.

Suggested change

	def __init__(self, base_url: str, app_key: str, proxy: str = None):
	self.base_url = base_url
	self.app_key = app_key
	self.proxy = proxy
	def __init__(self, base_url: str, app_key: str, proxy: str = None):
	self.base_url = base_url
	self.app_key = app_key
	self.proxy = proxy.strip() if proxy and proxy.strip() else None

[gemini-code-assist]

To ensure that empty or whitespace-only proxy strings are correctly sanitized and treated as None (preventing aiohttp from raising a ValueError), update the test to parameterize and assert this behavior.

@pytest.mark.parametrize(
    "proxy,expected_proxy",
    [
        (None, None),
        ("", None),
        ("   ", None),
        ("http://proxy.example.com:8080", "http://proxy.example.com:8080"),
    ],
)
async def test_http_client_routes_requests_with_optional_proxy(method, proxy, expected_proxy):
    client = HttpClient("https://soar.example.com", "app-key", proxy=proxy)
    session = mock.Mock()
    response = _Response()
    getattr(session, method).return_value = response
    client._session = session

    params = {"page": 1}
    body = {"name": "case"}
    if method == "get":
        result = await client.get("/cases", params=params)
        expected = {"params": params, "headers": {"AppKey": "app-key"}, "proxy": expected_proxy}
    else:
        result = await getattr(client, method)("/cases", req=body, params=params)
        expected = {
            "json": body,
            "params": params,
            "headers": {"AppKey": "app-key"},
            "proxy": expected_proxy,
        }

[gemini-code-assist]

Similarly, update the binding test to verify that empty or whitespace-only SOAR_PROXY environment variables are correctly sanitized to None when initializing the global HttpClient.

@pytest.mark.parametrize(
    "proxy,expected_proxy",
    [
        (None, None),
        ("", None),
        ("   ", None),
        ("http://proxy.example.com:8080", "http://proxy.example.com:8080"),
    ],
)
async def test_bind_passes_optional_soar_proxy_from_environment(monkeypatch, proxy, expected_proxy):
    monkeypatch.setenv("SOAR_URL", "https://soar.example.com")
    monkeypatch.setenv("SOAR_APP_KEY", "app-key")
    if proxy is None:
        monkeypatch.delenv("SOAR_PROXY", raising=False)
    else:
        monkeypatch.setenv("SOAR_PROXY", proxy)

    with mock.patch.object(bindings, "_get_valid_scopes", new=mock.AsyncMock(return_value=set())):
        await bindings.bind()

    assert bindings.http_client.base_url == "https://soar.example.com"
    assert bindings.http_client.app_key == "app-key"
    assert bindings.http_client.proxy == expected_proxy
    await bindings.cleanup()

[dandye]

@wangyusheng1985 Thank you for this submission. I actually already had a fix for this in #311 and it was just waiting on review. We just merged that into main. Could you please try that solution out to ensure it meets your needs?