Harden external file connector boundaries

src/govoplan_files/backend/storage/connector_browse.py

@@ -1,6 +1,6 @@
 from __future__ import annotations
 
-import os
+import json
 from collections.abc import Mapping
 from dataclasses import dataclass, field
 from datetime import datetime, timezone
@@ -12,8 +12,21 @@ from urllib.parse import quote, unquote, urljoin, urlsplit
 import xml.etree.ElementTree as ET  # nosec B405 - typing/element creation only; parsing uses defusedxml below.
 
 from defusedxml import ElementTree as SafeElementTree
-import httpx
 
+from govoplan_core.security.outbound_http import (
+    OutboundHttpError,
+    outbound_http_policy,
+    pinned_outbound_hostname,
+    validate_unpinned_sdk_http_url,
+)
+
+from govoplan_files.backend.storage.http_client import ConnectorHttpError, request_connector_bytes
+from govoplan_files.backend.storage.connector_deployment import (
+    ConnectorDeploymentConfigurationError,
+    connector_ca_bundle_path,
+    connector_secret_env_value,
+    validate_connector_tls_metadata,
+)
 from govoplan_files.backend.storage.connector_profiles import ConnectorProfile
 
 
@@ -170,14 +183,22 @@ def _browse_webdav(profile: ConnectorProfile, *, path: str) -> list[ConnectorBro
   </prop>
 </propfind>"""
     try:
-        response = httpx.request("PROPFIND", url, headers=headers, content=body, auth=auth, timeout=15.0)
-    except httpx.HTTPError as exc:
+        response = request_connector_bytes(
+            "PROPFIND",
+            url,
+            headers=headers,
+            content=body,
+            auth=auth,
+            timeout=15.0,
+            label="WebDAV browse",
+        )
+    except ConnectorHttpError as exc:
         raise ConnectorBrowseError(f"Connector browse failed: {exc}") from exc
     if response.status_code in {401, 403}:
         raise ConnectorBrowseError("Connector credentials were rejected")
     if response.status_code not in {200, 207}:
         raise ConnectorBrowseError(f"Connector browse failed with HTTP {response.status_code}")
-    return parse_webdav_multistatus(root_url=root_url, current_path=path, payload=response.text)
+    return parse_webdav_multistatus(root_url=root_url, current_path=path, payload=response.content)
 
 
 def _browse_smb(profile: ConnectorProfile, *, path: str) -> list[ConnectorBrowseItem]:
@@ -298,7 +319,17 @@ def _s3_client(profile: ConnectorProfile) -> Any:
         raise ConnectorBrowseUnsupported("S3 connector browsing requires the optional boto3 dependency") from exc
     kwargs: dict[str, object] = {}
     if profile.endpoint_url:
-        kwargs["endpoint_url"] = profile.endpoint_url
+        try:
+            kwargs["endpoint_url"] = validate_unpinned_sdk_http_url(
+                profile.endpoint_url,
+                label="S3 connector endpoint",
+            )
+        except OutboundHttpError as exc:
+            raise ConnectorBrowseError(str(exc)) from exc
+    elif not outbound_http_policy().allow_private_networks:
+        raise ConnectorBrowseError(
+            "S3 connector endpoint discovery cannot pin the SDK connection address while private-network access is disabled"
+        )
     region = _metadata_string(profile, "region") or _metadata_string(profile, "aws_region")
     if region:
         kwargs["region_name"] = region
@@ -426,9 +457,16 @@ def _s3_max_keys(profile: ConnectorProfile) -> int:
 
 
 def _s3_verify(profile: ConnectorProfile) -> bool | str | None:
+    try:
+        validate_connector_tls_metadata(profile.metadata)
+    except ConnectorDeploymentConfigurationError as exc:
+        raise ConnectorBrowseError(str(exc)) from exc
     ca_bundle = _metadata_string(profile, "ca_bundle")
     if ca_bundle:
-        return ca_bundle
+        try:
+            return connector_ca_bundle_path(ca_bundle)
+        except ConnectorDeploymentConfigurationError as exc:
+            raise ConnectorBrowseError(str(exc)) from exc
     if "verify_tls" in profile.metadata:
         return _metadata_bool(profile, "verify_tls", default=True)
     if "tls_verify" in profile.metadata:
@@ -484,16 +522,24 @@ def _request_json(
     data: Mapping[str, str] | None = None,
 ) -> object:
     try:
-        response = httpx.request(method, url, headers=dict(headers or {}), params=params, data=data, timeout=15.0)
-    except httpx.HTTPError as exc:
+        response = request_connector_bytes(
+            method,
+            url,
+            headers=dict(headers or {}),
+            params=params,
+            data=data,
+            timeout=15.0,
+            label="Seafile API",
+        )
+    except ConnectorHttpError as exc:
         raise ConnectorBrowseError(f"Connector browse failed: {exc}") from exc
     if response.status_code in {401, 403}:
         raise ConnectorBrowseError("Connector credentials were rejected")
     if response.status_code not in {200, 201}:
         raise ConnectorBrowseError(f"Connector browse failed with HTTP {response.status_code}")
     try:
-        return response.json()
-    except ValueError as exc:
+        return json.loads(response.content)
+    except (UnicodeDecodeError, ValueError) as exc:
         raise ConnectorBrowseError("Connector returned invalid JSON") from exc
 
 
@@ -671,14 +717,14 @@ def _metadata_env(profile: ConnectorProfile, key: str) -> str | None:
     env_name = _metadata_string(profile, key)
     if not env_name:
         return None
-    return _env_required(env_name, profile.id)
+    return _env_required(env_name, profile)
 
 
-def _env_required(name: str, profile_id: str) -> str:
-    value = _clean(os.environ.get(name))
-    if not value:
-        raise ConnectorBrowseError(f"Connector profile {profile_id} is missing required credential environment")
-    return value
+def _env_required(name: str, profile: ConnectorProfile) -> str:
+    try:
+        return connector_secret_env_value(name, source_kind=profile.source_kind)
+    except ConnectorDeploymentConfigurationError as exc:
+        raise ConnectorBrowseError(f"Connector profile {profile.id}: {exc}") from exc
 
 
 def normalize_connector_browse_path(value: object) -> str:
@@ -735,6 +781,11 @@ def _smb_location(profile: ConnectorProfile) -> _SmbLocation:
     server = _clean(parsed.hostname)
     if not server:
         raise ConnectorBrowseError("SMB connector endpoint_url must include a server")
+    port = parsed.port or _int(profile.metadata.get("port")) or 445
+    try:
+        server = pinned_outbound_hostname(server, port=port, label="SMB connector endpoint")
+    except OutboundHttpError as exc:
+        raise ConnectorBrowseError(str(exc)) from exc
     path_parts = [part for part in unquote(parsed.path or "").strip("/").split("/") if part]
     if not path_parts:
         raise ConnectorBrowseError("SMB connector endpoint_url must include a share name")
@@ -744,7 +795,7 @@ def _smb_location(profile: ConnectorProfile) -> _SmbLocation:
     return _SmbLocation(
         server=server,
         share=path_parts[0],
-        port=parsed.port or _int(profile.metadata.get("port")) or 445,
+        port=port,
         root_path=normalize_connector_browse_path("/".join(root_parts)),
     )
 
@@ -780,7 +831,7 @@ def _profile_password(profile: ConnectorProfile) -> str | None:
     if profile.password_value:
         return profile.password_value
     if profile.password_env:
-        return _env_required(profile.password_env, profile.id)
+        return _env_required(profile.password_env, profile)
     return None
 
 
@@ -788,7 +839,7 @@ def _profile_token(profile: ConnectorProfile) -> str | None:
     if profile.token_value:
         return profile.token_value
     if profile.token_env:
-        return _env_required(profile.token_env, profile.id)
+        return _env_required(profile.token_env, profile)
     return None