From f2dfb6c90eace08af7b430d88d7aae4229649f08 Mon Sep 17 00:00:00 2001 From: Albrecht Degering Date: Tue, 21 Jul 2026 12:10:23 +0200 Subject: [PATCH] Harden external file connector boundaries --- README.md | 10 +- dev/connectors/README.md | 7 +- docs/CONNECTOR_BOUNDARY.md | 5 +- docs/CONNECTOR_SPACES.md | 5 +- pyproject.toml | 2 +- src/govoplan_files/backend/router.py | 111 +++++++- .../backend/storage/backends.py | 73 +++++- .../backend/storage/connector_browse.py | 91 +++++-- .../storage/connector_credential_store.py | 15 +- .../backend/storage/connector_deployment.py | 199 +++++++++++++++ .../backend/storage/connector_imports.py | 28 +- .../storage/connector_profile_store.py | 11 + .../backend/storage/connector_profiles.py | 29 ++- .../backend/storage/http_client.py | 239 ++++++++++++++++++ tests/test_connector_deployment.py | 196 ++++++++++++++ tests/test_connector_providers.py | 20 +- tests/test_http_client.py | 126 +++++++++ tests/test_storage_backends.py | 64 +++++ 18 files changed, 1167 insertions(+), 64 deletions(-) create mode 100644 src/govoplan_files/backend/storage/connector_deployment.py create mode 100644 src/govoplan_files/backend/storage/http_client.py create mode 100644 tests/test_connector_deployment.py create mode 100644 tests/test_http_client.py create mode 100644 tests/test_storage_backends.py diff --git a/README.md b/README.md index f85e096..2604124 100644 --- a/README.md +++ b/README.md @@ -78,7 +78,11 @@ Profiles can be supplied as JSON through `GOVOPLAN_FILES_CONNECTOR_PROFILES_JSON`, or from a JSON file path through `GOVOPLAN_FILES_CONNECTOR_PROFILES_FILE`. Each profile has an `id`, `provider`, `endpoint_url`, governance `scope_type`/`scope_id`, optional `capabilities`, and -credential references such as `password_env`, `token_env`, or `secret_ref`. +deployment-owned credential references such as `password_env`, `token_env`, or +`secret_ref`. Environment references require an exact name in the deployment-wide +`GOVOPLAN_CONNECTOR_SECRET_ENV_ALLOWLIST`; API-managed profiles cannot select +process environment variables and use encrypted stored credentials or scoped +secret-provider references instead. `GET /api/v1/files/connectors/profiles` returns only profiles visible to the current principal (system, tenant, user, group, or accessible campaign scope) and redacts secret values and environment variable names. Use the returned @@ -102,8 +106,8 @@ conflict handling, source provenance, and connector audit path as direct uploads. The Seafile provider uses account-token auth and the native file download-link API; Nextcloud and generic WebDAV profiles use authenticated `GET` requests against the configured WebDAV endpoint. SMB profiles use -`smb://server[:port]/share[/path]` endpoints and environment-backed credentials -through `smbprotocol`. +`smb://server[:port]/share[/path]` endpoints and deployment-owned or encrypted +stored credentials through `smbprotocol`. Local connector development assets live in `dev/connectors/`. The compose stack boots Nextcloud, Seafile, WebDAV, and SMB endpoints for provider development and diff --git a/dev/connectors/README.md b/dev/connectors/README.md index 6accbd9..832b02e 100644 --- a/dev/connectors/README.md +++ b/dev/connectors/README.md @@ -38,8 +38,9 @@ The local fixture data under `data/` is ignored by git. ## GovOPlaN Profile Config Connector profiles are read from `GOVOPLAN_FILES_CONNECTOR_PROFILES_JSON` or -`GOVOPLAN_FILES_CONNECTOR_PROFILES_FILE`. Credentials should be referenced by -secret refs or environment variables; profile API responses only expose the +`GOVOPLAN_FILES_CONNECTOR_PROFILES_FILE`. This deployment-owned configuration +may reference environment variables only when their exact names are listed in +`GOVOPLAN_CONNECTOR_SECRET_ENV_ALLOWLIST`; profile API responses expose only the credential source and configured state. Example local profile file: @@ -123,6 +124,8 @@ Start GovOPlaN with: ```bash export GOVOPLAN_FILES_CONNECTOR_PROFILES_FILE=/mnt/DATA/git/govoplan-files/dev/connectors/profiles.local.json +export GOVOPLAN_CONNECTOR_SECRET_ENV_ALLOWLIST=SEAFILE_ADMIN_PASSWORD,NEXTCLOUD_ADMIN_PASSWORD,WEBDAV_PASSWORD,SMB_PASSWORD,MINIO_ROOT_PASSWORD +export GOVOPLAN_CONNECTOR_ALLOW_PRIVATE_NETWORKS=true ``` ## Smoke Test diff --git a/docs/CONNECTOR_BOUNDARY.md b/docs/CONNECTOR_BOUNDARY.md index 2f17e46..d2829d3 100644 --- a/docs/CONNECTOR_BOUNDARY.md +++ b/docs/CONNECTOR_BOUNDARY.md @@ -49,8 +49,9 @@ any remote write behavior. Every provider must: - enforce GovOPlaN profile visibility and connector policy before browse/import -- keep credentials as environment variables or secret references, never API - response values +- keep credentials as encrypted values or scoped secret references, never API + response values; process-environment references are allowed only in + deployment-owned profiles with an exact deployment allowlist - import external files into managed storage before they are used in campaigns or workflows - preserve source provenance and revision metadata diff --git a/docs/CONNECTOR_SPACES.md b/docs/CONNECTOR_SPACES.md index 65accbf..a859d76 100644 --- a/docs/CONNECTOR_SPACES.md +++ b/docs/CONNECTOR_SPACES.md @@ -34,8 +34,9 @@ Credential profile: - provider: a specific provider or any provider - scope: `system` or `tenant` for administered credentials -- credential mode: anonymous, environment reference, secret reference, or - encrypted stored password/token +- credential mode: anonymous, scoped secret reference, or encrypted stored + password/token; environment references are reserved for deployment-owned JSON + profiles and exact deployment allowlisting - username and redacted secret configuration - credential-local policy for allow/deny rules diff --git a/pyproject.toml b/pyproject.toml index 4129291..4b8ee87 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -11,7 +11,7 @@ requires-python = ">=3.12" license = { file = "LICENSE" } authors = [{ name = "GovOPlaN" }] dependencies = [ - "govoplan-core>=0.1.8", + "govoplan-core>=0.1.9", "defusedxml>=0.7,<1", "python-multipart>=0.0.31,<1", ] diff --git a/src/govoplan_files/backend/router.py b/src/govoplan_files/backend/router.py index 9fc3cc1..cf56016 100644 --- a/src/govoplan_files/backend/router.py +++ b/src/govoplan_files/backend/router.py @@ -7,7 +7,7 @@ from dataclasses import replace from datetime import datetime from io import BytesIO from typing import Any, Literal -from urllib.parse import quote, urljoin +from urllib.parse import quote, urljoin, urlsplit, urlunsplit from fastapi import APIRouter, Depends, File as FastAPIFile, Form, HTTPException, Query, UploadFile, status from fastapi.responses import FileResponse, StreamingResponse from starlette.background import BackgroundTask @@ -120,6 +120,10 @@ from govoplan_files.backend.storage.connector_browse import ( normalize_connector_browse_path, ) from govoplan_files.backend.storage.connector_imports import ConnectorImportError, ConnectorImportUnsupported, read_connector_file +from govoplan_files.backend.storage.connector_deployment import ( + connector_effective_endpoint_url, + reject_api_controlled_deployment_references, +) from govoplan_files.backend.storage.connector_profile_store import ( connector_profile_from_row, create_connector_profile_row, @@ -571,11 +575,48 @@ def _discovery_profile_from_payload( password_value=credentials.password, token_value=credentials.token, capabilities=("browse",), - metadata=payload.metadata, + # Discovery metadata is untrusted API input and must not be able to + # replace the candidate endpoint or inject a static/fake listing. + metadata={}, source_kind="discovery", ) +def _audit_connector_discovery_attempt( + session: Session, + principal: ApiPrincipal, + *, + provider: str, + endpoint_url: str, + base_path: str | None, +) -> None: + audit_from_principal( + session, + principal, + action="files.connector.discovery_attempted", + object_type="connector_endpoint", + object_id=provider, + details={ + "provider": provider, + "endpoint_origin": _redacted_connector_url(endpoint_url), + "base_path": base_path, + }, + commit=True, + ) + + +def _redacted_connector_url(value: str) -> str: + try: + parsed = urlsplit(value) + hostname = parsed.hostname or "" + if ":" in hostname: + hostname = f"[{hostname}]" + netloc = f"{hostname}:{parsed.port}" if parsed.port is not None else hostname + return urlunsplit((parsed.scheme, netloc, "", "", "")) + except ValueError: + return "" + + def _visible_connector_profile( session: Session, principal: ApiPrincipal, @@ -768,7 +809,11 @@ def _download_connector_payload( provider=profile.provider, external_id=f"{payload.library_id}:{source_path}", external_path=source_path, - external_url=profile.endpoint_url, + external_url=connector_effective_endpoint_url( + provider=profile.provider, + endpoint_url=profile.endpoint_url, + metadata=profile.metadata, + ), operation=operation, ), profile.policy_sources, @@ -1108,7 +1153,11 @@ def _connector_space_policy_decision( provider=profile.provider, external_id=external_id, external_path=browse_path, - external_url=profile.endpoint_url, + external_url=connector_effective_endpoint_url( + provider=profile.provider, + endpoint_url=profile.endpoint_url, + metadata=profile.metadata, + ), operation=operation, ), profile.policy_sources, @@ -2373,8 +2422,17 @@ def list_connector_providers( @router.post("/connectors/discover", response_model=FileConnectorDiscoveryResponse) def discover_connector_endpoint( payload: FileConnectorDiscoveryRequest, + session: Session = Depends(get_session), principal: ApiPrincipal = Depends(require_any_scope("files:file:admin", "system:settings:write", "admin:settings:write")), ): + try: + reject_api_controlled_deployment_references( + password_env=payload.credentials.password_env, + token_env=payload.credentials.token_env, + metadata=payload.metadata, + ) + except ValueError as exc: + raise _http_error(exc) from exc if payload.provider not in {"webdav", "nextcloud"}: return FileConnectorDiscoveryResponse( provider=payload.provider, @@ -2387,7 +2445,28 @@ def discover_connector_endpoint( for endpoint_url in _webdav_discovery_candidates(payload): profile = _discovery_profile_from_payload(payload, endpoint_url, principal=principal) try: + _ensure_connector_configuration_allowed( + session, + principal, + connector_id=None, + credential_id=None, + provider=profile.provider, + endpoint_url=endpoint_url, + base_path=profile.base_path, + scope_type="tenant", + scope_id=principal.tenant_id, + operation="discover", + ) + _audit_connector_discovery_attempt( + session, + principal, + provider=profile.provider, + endpoint_url=endpoint_url, + base_path=profile.base_path, + ) browse_connector_profile(profile, path=payload.base_path or "") + except ConnectorPolicyDenied as exc: + raise _connector_policy_error(exc) from exc except (ConnectorBrowseError, ConnectorBrowseUnsupported, OSError, ValueError, json.JSONDecodeError) as exc: message = str(exc) if "credentials were rejected" in message.casefold(): @@ -2400,7 +2479,7 @@ def discover_connector_endpoint( status="credentials_rejected", message="The endpoint was found, but login failed with these credentials.", candidates=candidates, - metadata={**payload.metadata, "discovered_by": "webdav-auth-challenge"}, + metadata={"discovered_by": "webdav-auth-challenge"}, ) candidates.append({"endpoint_url": endpoint_url, "status": "found", "message": "The endpoint exists, but credentials are required or were rejected."}) return FileConnectorDiscoveryResponse( @@ -2410,7 +2489,7 @@ def discover_connector_endpoint( status="found", message="The endpoint was found. Add working credentials before saving or testing the connection.", candidates=candidates, - metadata={**payload.metadata, "discovered_by": "webdav-auth-challenge"}, + metadata={"discovered_by": "webdav-auth-challenge"}, ) candidates.append({"endpoint_url": endpoint_url, "status": "failed", "message": message}) continue @@ -2424,7 +2503,7 @@ def discover_connector_endpoint( status=status_value, message=message, candidates=candidates, - metadata={**payload.metadata, "discovered_by": "webdav-propfind"}, + metadata={"discovered_by": "webdav-propfind"}, ) return FileConnectorDiscoveryResponse( provider=payload.provider, @@ -2741,7 +2820,11 @@ def create_connector_profile( connector_id=payload.id, credential_id=payload.credential_profile_id, provider=payload.provider, - endpoint_url=payload.endpoint_url, + endpoint_url=connector_effective_endpoint_url( + provider=payload.provider, + endpoint_url=payload.endpoint_url, + metadata=payload.metadata, + ), base_path=payload.base_path, scope_type=payload.scope_type, scope_id=payload.scope_id, @@ -2909,7 +2992,11 @@ def browse_connector_profile_items( credential_id=profile.credential_profile_id, provider=profile.provider, external_path=browse_path, - external_url=profile.endpoint_url, + external_url=connector_effective_endpoint_url( + provider=profile.provider, + endpoint_url=profile.endpoint_url, + metadata=profile.metadata, + ), operation="browse", ), profile.policy_sources, @@ -2986,7 +3073,11 @@ def update_connector_profile( connector_id=row.id, credential_id=credential_profile_id, provider=provider, - endpoint_url=payload.endpoint_url if payload.endpoint_url is not None else row.endpoint_url, + endpoint_url=connector_effective_endpoint_url( + provider=provider, + endpoint_url=payload.endpoint_url if payload.endpoint_url is not None else row.endpoint_url, + metadata=payload.metadata if payload.metadata is not None else row.metadata_, + ), base_path=payload.base_path if payload.base_path is not None else row.base_path, scope_type=row.scope_type, scope_id=row.scope_id, diff --git a/src/govoplan_files/backend/storage/backends.py b/src/govoplan_files/backend/storage/backends.py index e427206..5b137dd 100644 --- a/src/govoplan_files/backend/storage/backends.py +++ b/src/govoplan_files/backend/storage/backends.py @@ -4,6 +4,12 @@ from dataclasses import dataclass, field from pathlib import Path from typing import Iterable, Protocol +from govoplan_core.security.outbound_http import ( + OutboundHttpError, + response_limit, + validate_unpinned_sdk_http_url, +) + from govoplan_files.backend.runtime import settings @@ -96,15 +102,25 @@ class S3StorageBackend: import boto3 except ModuleNotFoundError as exc: raise StorageBackendError("boto3 is required for the S3 storage backend") from exc - return boto3.client( - "s3", - endpoint_url=self.endpoint_url, - region_name=self.region_name, - aws_access_key_id=self.access_key_id, - aws_secret_access_key=self.secret_access_key, - ) + try: + endpoint_url = validate_unpinned_sdk_http_url( + self.endpoint_url, + label="File storage S3 endpoint", + ) + return boto3.client( + "s3", + endpoint_url=endpoint_url, + region_name=self.region_name, + aws_access_key_id=self.access_key_id, + aws_secret_access_key=self.secret_access_key, + ) + except OutboundHttpError as exc: + raise StorageBackendError(str(exc)) from exc def put_bytes(self, key: str, data: bytes, *, content_type: str | None = None) -> None: + max_bytes = response_limit("file") + if len(data) > max_bytes: + raise StorageBackendError(f"Stored object exceeds the deployment limit of {max_bytes} bytes") kwargs = {"Bucket": self.bucket, "Key": key, "Body": data} if content_type: kwargs["ContentType"] = content_type @@ -113,19 +129,39 @@ class S3StorageBackend: def get_bytes(self, key: str) -> bytes: try: obj = self.client.get_object(Bucket=self.bucket, Key=key) - return obj["Body"].read() + max_bytes = response_limit("file") + body = obj["Body"] + try: + _reject_declared_object_size(obj, max_bytes=max_bytes) + data = body.read(max_bytes + 1) + if len(data) > max_bytes: + raise StorageBackendError(f"Stored object exceeds the deployment limit of {max_bytes} bytes") + return data + finally: + if hasattr(body, "close"): + body.close() except Exception as exc: # pragma: no cover - depends on S3 backend raise StorageBackendError(str(exc)) from exc def iter_bytes(self, key: str, *, chunk_size: int = 1024 * 1024) -> Iterable[bytes]: try: obj = self.client.get_object(Bucket=self.bucket, Key=key) + max_bytes = response_limit("file") body = obj["Body"] - while True: - chunk = body.read(chunk_size) - if not chunk: - break - yield chunk + try: + _reject_declared_object_size(obj, max_bytes=max_bytes) + total = 0 + while True: + chunk = body.read(chunk_size) + if not chunk: + break + total += len(chunk) + if total > max_bytes: + raise StorageBackendError(f"Stored object exceeds the deployment limit of {max_bytes} bytes") + yield chunk + finally: + if hasattr(body, "close"): + body.close() except Exception as exc: # pragma: no cover - depends on S3 backend raise StorageBackendError(str(exc)) from exc @@ -140,6 +176,17 @@ class S3StorageBackend: return False +def _reject_declared_object_size(obj: object, *, max_bytes: int) -> None: + if not isinstance(obj, dict): + return + try: + declared_size = int(obj.get("ContentLength")) + except (TypeError, ValueError): + return + if declared_size > max_bytes: + raise StorageBackendError(f"Stored object exceeds the deployment limit of {max_bytes} bytes") + + def _fallback_roots() -> tuple[Path, ...]: raw = getattr(settings, "file_storage_local_fallback_roots", "") or "" return tuple(Path(item.strip()) for item in str(raw).split(",") if item.strip()) diff --git a/src/govoplan_files/backend/storage/connector_browse.py b/src/govoplan_files/backend/storage/connector_browse.py index bea83ac..7001c31 100644 --- a/src/govoplan_files/backend/storage/connector_browse.py +++ b/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 """ 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 diff --git a/src/govoplan_files/backend/storage/connector_credential_store.py b/src/govoplan_files/backend/storage/connector_credential_store.py index 44bd3c8..db8e1b1 100644 --- a/src/govoplan_files/backend/storage/connector_credential_store.py +++ b/src/govoplan_files/backend/storage/connector_credential_store.py @@ -10,6 +10,7 @@ from govoplan_core.core.policy import normalize_policy_scope_type, policy_source from govoplan_core.security.secrets import decrypt_secret, encrypt_secret from govoplan_files.backend.db.models import FileConnectorCredential from govoplan_files.backend.storage.common import FileStorageError +from govoplan_files.backend.storage.connector_deployment import reject_api_controlled_deployment_references from govoplan_files.backend.storage.connector_policy import ConnectorPolicySource, connector_policy_sources_from_payload from govoplan_files.backend.storage.connector_profiles import supported_connector_providers @@ -53,7 +54,9 @@ class ConnectorCredential: def credentials_configured(self) -> bool: if self.credential_mode.casefold() in {"", "none", "anonymous"}: return True - return bool(self.secret_ref or self.password_value or self.token_value or self.password_env or self.token_env) + # Environment references are deliberately unavailable to API-managed + # credential rows. Legacy rows remain visible but fail closed. + return bool(self.secret_ref or self.password_value or self.token_value) def to_response(self) -> dict[str, Any]: return { @@ -182,6 +185,11 @@ def create_connector_credential_row( policy: Mapping[str, Any] | None = None, metadata: Mapping[str, Any] | None = None, ) -> FileConnectorCredential: + reject_api_controlled_deployment_references( + password_env=password_env, + token_env=token_env, + metadata=metadata, + ) clean_id = _normalize_id(credential_id) if session.get(FileConnectorCredential, clean_id) is not None: raise FileStorageError(f"Connector credential already exists: {clean_id}") @@ -231,6 +239,11 @@ def update_connector_credential_row( clear_password: bool = False, clear_token: bool = False, ) -> FileConnectorCredential: + reject_api_controlled_deployment_references( + password_env=password_env, + token_env=token_env, + metadata=metadata, + ) if label is not None: row.label = _normalize_label(label) if provider is not None: diff --git a/src/govoplan_files/backend/storage/connector_deployment.py b/src/govoplan_files/backend/storage/connector_deployment.py new file mode 100644 index 0000000..b65b00d --- /dev/null +++ b/src/govoplan_files/backend/storage/connector_deployment.py @@ -0,0 +1,199 @@ +from __future__ import annotations + +import os +import re +from collections.abc import Mapping +from pathlib import Path +from typing import Any + + +_SECRET_ENV_ALLOWLIST = "GOVOPLAN_CONNECTOR_SECRET_ENV_ALLOWLIST" # noqa: S105 # nosec B105 - configuration key. +_CA_BUNDLE_ALLOWLIST = "GOVOPLAN_CONNECTOR_CA_BUNDLE_ALLOWLIST" +_ENV_NAME = re.compile(r"[A-Za-z_][A-Za-z0-9_]*\Z") +_SECRET_ENV_METADATA_KEYS = frozenset( + { + "access_key_id_env", + "secret_access_key_env", + "session_token_env", + } +) +_DEVELOPMENT_ENVIRONMENTS = frozenset({"dev", "development", "local", "test", "testing"}) + + +class ConnectorDeploymentConfigurationError(ValueError): + """Raised when connector data crosses a deployment-owned trust boundary.""" + + +def connector_secret_env_value(name: str, *, source_kind: str) -> str: + clean_name = _validate_secret_env_reference(name, source_kind=source_kind) + value = os.environ.get(clean_name) + if value is None or value == "": + raise ConnectorDeploymentConfigurationError("Connector credential environment variable is not configured") + return value + + +def connector_secret_env_available(name: str | None, *, source_kind: str) -> bool: + if not name: + return False + try: + clean_name = _validate_secret_env_reference(name, source_kind=source_kind) + except ConnectorDeploymentConfigurationError: + return False + return bool(os.environ.get(clean_name)) + + +def validate_deployment_connector_references( + *, + source_kind: str, + password_env: str | None = None, + token_env: str | None = None, + metadata: Mapping[str, Any] | None = None, +) -> None: + """Validate references in deployment-owned connector configuration.""" + + for name in (password_env, token_env): + if name: + _validate_secret_env_reference(name, source_kind=source_kind) + for key in _SECRET_ENV_METADATA_KEYS: + name = _clean((metadata or {}).get(key)) + if name: + _validate_secret_env_reference(name, source_kind=source_kind) + validate_connector_tls_metadata(metadata) + + +def reject_api_controlled_deployment_references( + *, + password_env: str | None = None, + token_env: str | None = None, + metadata: Mapping[str, Any] | None = None, +) -> None: + """Reject process-secret selectors controlled through tenant-facing APIs.""" + + if _clean(password_env) or _clean(token_env): + raise ConnectorDeploymentConfigurationError( + "Environment-backed credentials may only be declared in deployment-owned connector configuration" + ) + for key, value in (metadata or {}).items(): + if _clean(value) and (str(key).strip().casefold() in _SECRET_ENV_METADATA_KEYS or str(key).strip().casefold().endswith("_env")): + raise ConnectorDeploymentConfigurationError( + "Environment-backed credentials may only be declared in deployment-owned connector configuration" + ) + validate_connector_tls_metadata(metadata) + + +def connector_ca_bundle_path(value: str) -> str: + clean_value = _clean(value) + if not clean_value: + raise ConnectorDeploymentConfigurationError("Connector CA bundle path is empty") + candidate = Path(clean_value) + if not candidate.is_absolute(): + raise ConnectorDeploymentConfigurationError("Connector CA bundle paths must be absolute") + try: + resolved = candidate.resolve(strict=True) + except OSError as exc: + raise ConnectorDeploymentConfigurationError("Connector CA bundle path does not exist") from exc + allowed = _allowed_ca_bundle_paths() + if resolved not in allowed: + raise ConnectorDeploymentConfigurationError( + f"Connector CA bundle path is not listed in {_CA_BUNDLE_ALLOWLIST}" + ) + if not resolved.is_file(): + raise ConnectorDeploymentConfigurationError("Connector CA bundle path must be a regular file") + return str(resolved) + + +def validate_connector_tls_metadata(metadata: Mapping[str, Any] | None) -> None: + values = metadata or {} + ca_bundle = _clean(values.get("ca_bundle")) + if ca_bundle: + connector_ca_bundle_path(ca_bundle) + for key in ("verify_tls", "tls_verify"): + if key in values and not _as_bool(values.get(key), default=True) and not _development_runtime(): + raise ConnectorDeploymentConfigurationError( + "Connector TLS certificate verification may only be disabled in dev/test environments" + ) + + +def connector_effective_endpoint_url( + *, + provider: str | None, + endpoint_url: str | None, + metadata: Mapping[str, Any] | None, +) -> str | None: + """Return the endpoint that connector I/O will actually use.""" + + clean_provider = (provider or "").strip().casefold() + values = metadata or {} + webdav_url = _clean(values.get("webdav_endpoint_url")) + browse_protocol = (_clean(values.get("browse_protocol")) or "").casefold() + if webdav_url and (clean_provider in {"seafile", "webdav", "nextcloud"} or browse_protocol == "webdav"): + return webdav_url + return _clean(endpoint_url) + + +def _validate_secret_env_reference(name: str, *, source_kind: str) -> str: + if source_kind.strip().casefold() != "settings": + raise ConnectorDeploymentConfigurationError( + "Environment-backed credentials may only be used by deployment-owned connector configuration" + ) + clean_name = name.strip() + if not _ENV_NAME.fullmatch(clean_name): + raise ConnectorDeploymentConfigurationError("Connector credential environment variable name is invalid") + if clean_name not in _allowed_secret_env_names(): + raise ConnectorDeploymentConfigurationError( + f"Connector credential environment variable is not listed in {_SECRET_ENV_ALLOWLIST}" + ) + return clean_name + + +def _allowed_secret_env_names() -> frozenset[str]: + raw = os.environ.get(_SECRET_ENV_ALLOWLIST, "") + names = frozenset(item.strip() for item in raw.split(",") if item.strip()) + invalid = sorted(name for name in names if not _ENV_NAME.fullmatch(name)) + if invalid: + raise ConnectorDeploymentConfigurationError(f"{_SECRET_ENV_ALLOWLIST} contains an invalid variable name") + return names + + +def _allowed_ca_bundle_paths() -> frozenset[Path]: + raw = os.environ.get(_CA_BUNDLE_ALLOWLIST, "") + paths: set[Path] = set() + for item in (part.strip() for part in raw.split(",")): + if not item: + continue + path = Path(item) + if not path.is_absolute(): + raise ConnectorDeploymentConfigurationError(f"{_CA_BUNDLE_ALLOWLIST} requires absolute paths") + try: + paths.add(path.resolve(strict=True)) + except OSError as exc: + raise ConnectorDeploymentConfigurationError(f"{_CA_BUNDLE_ALLOWLIST} contains a missing path") from exc + return frozenset(paths) + + +def _development_runtime() -> bool: + app_env = os.environ.get("APP_ENV", "dev").strip().casefold() + install_profile = os.environ.get("GOVOPLAN_INSTALL_PROFILE", "").strip().casefold() + return app_env in _DEVELOPMENT_ENVIRONMENTS and ( + not install_profile or install_profile in _DEVELOPMENT_ENVIRONMENTS + ) + + +def _as_bool(value: object, *, default: bool) -> bool: + if value is None: + return default + if isinstance(value, bool): + return value + clean = str(value).strip().casefold() + if clean in {"1", "true", "yes", "on"}: + return True + if clean in {"0", "false", "no", "off"}: + return False + raise ConnectorDeploymentConfigurationError("Connector TLS verification setting must be true or false") + + +def _clean(value: object) -> str | None: + if value is None: + return None + clean = str(value).strip() + return clean or None diff --git a/src/govoplan_files/backend/storage/connector_imports.py b/src/govoplan_files/backend/storage/connector_imports.py index d4a5157..1b24568 100644 --- a/src/govoplan_files/backend/storage/connector_imports.py +++ b/src/govoplan_files/backend/storage/connector_imports.py @@ -4,7 +4,7 @@ from dataclasses import dataclass, field import mimetypes from typing import Any -import httpx +from govoplan_core.security.outbound_http import response_limit from govoplan_files.backend.storage.connector_browse import ( ConnectorBrowseError, @@ -30,6 +30,7 @@ from govoplan_files.backend.storage.connector_browse import ( normalize_connector_browse_path, ) from govoplan_files.backend.storage.connector_profiles import ConnectorProfile +from govoplan_files.backend.storage.http_client import ConnectorHttpError, request_connector_bytes from govoplan_files.backend.storage.paths import filename_from_path @@ -59,6 +60,7 @@ def read_connector_file( path: str, max_bytes: int, ) -> ConnectorDownloadedFile: + max_bytes = min(max_bytes, response_limit("file")) if profile.provider == "seafile": if _metadata_string(profile, "browse_protocol") == "webdav" or _metadata_string(profile, "webdav_endpoint_url"): return _read_webdav_file(profile, path=path, max_bytes=max_bytes) @@ -102,8 +104,15 @@ def _read_seafile_file(profile: ConnectorProfile, *, library_id: str, path: str, if not isinstance(download_url, str) or not download_url.strip(): raise ConnectorImportError("Seafile did not return a file download URL") try: - response = httpx.request("GET", download_url, timeout=30.0) - except httpx.HTTPError as exc: + response = request_connector_bytes( + "GET", + download_url, + timeout=30.0, + kind="file", + max_bytes=max_bytes, + label="Seafile file download", + ) + except ConnectorHttpError as exc: raise ConnectorImportError(f"Seafile file download failed: {exc}") from exc if response.status_code != 200: raise ConnectorImportError(f"Seafile file download failed with HTTP {response.status_code}") @@ -149,8 +158,17 @@ def _read_webdav_file(profile: ConnectorProfile, *, path: str, max_bytes: int) - elif profile.credential_mode.casefold() not in {"", "none", "anonymous"} and profile.secret_ref: raise ConnectorImportError("Secret-ref WebDAV credentials need a runtime secret resolver before live import") try: - response = httpx.request("GET", url, headers=headers, auth=auth, timeout=30.0) - except httpx.HTTPError as exc: + response = request_connector_bytes( + "GET", + url, + headers=headers, + auth=auth, + timeout=30.0, + kind="file", + max_bytes=max_bytes, + label="WebDAV file download", + ) + except ConnectorHttpError as exc: raise ConnectorImportError(f"WebDAV file download failed: {exc}") from exc if response.status_code in {401, 403}: raise ConnectorImportError("Connector credentials were rejected") diff --git a/src/govoplan_files/backend/storage/connector_profile_store.py b/src/govoplan_files/backend/storage/connector_profile_store.py index 869f1d0..3d4d1fb 100644 --- a/src/govoplan_files/backend/storage/connector_profile_store.py +++ b/src/govoplan_files/backend/storage/connector_profile_store.py @@ -9,6 +9,7 @@ from govoplan_core.core.policy import normalize_policy_scope_type from govoplan_core.security.secrets import decrypt_secret, encrypt_secret from govoplan_files.backend.db.models import FileConnectorCredential, FileConnectorProfile from govoplan_files.backend.storage.common import FileStorageError +from govoplan_files.backend.storage.connector_deployment import reject_api_controlled_deployment_references from govoplan_files.backend.storage.connector_credential_store import connector_credential_from_row, credential_rows_by_id from govoplan_files.backend.storage.connector_policy import connector_policy_sources_from_payload from govoplan_files.backend.storage.connector_profiles import ConnectorProfile, supported_connector_providers @@ -124,6 +125,11 @@ def create_connector_profile_row( policy: Mapping[str, Any] | None = None, metadata: Mapping[str, Any] | None = None, ) -> FileConnectorProfile: + reject_api_controlled_deployment_references( + password_env=password_env, + token_env=token_env, + metadata=metadata, + ) clean_id = _normalize_profile_id(profile_id) if session.get(FileConnectorProfile, clean_id) is not None: raise FileStorageError(f"Connector profile already exists: {clean_id}") @@ -181,6 +187,11 @@ def update_connector_profile_row( clear_password: bool = False, clear_token: bool = False, ) -> FileConnectorProfile: + reject_api_controlled_deployment_references( + password_env=password_env, + token_env=token_env, + metadata=metadata, + ) if label is not None: row.label = _normalize_label(label) if provider is not None: diff --git a/src/govoplan_files/backend/storage/connector_profiles.py b/src/govoplan_files/backend/storage/connector_profiles.py index ee26565..440f68d 100644 --- a/src/govoplan_files/backend/storage/connector_profiles.py +++ b/src/govoplan_files/backend/storage/connector_profiles.py @@ -7,6 +7,10 @@ from dataclasses import dataclass, field from typing import Any from govoplan_core.core.policy import normalize_policy_scope_type, policy_source_path +from govoplan_files.backend.storage.connector_deployment import ( + connector_secret_env_available, + validate_deployment_connector_references, +) from govoplan_files.backend.storage.connector_policy import ConnectorPolicySource, connector_policy_sources_from_payload @@ -82,9 +86,9 @@ class ConnectorProfile: if self.secret_ref or self.has_inline_secret or self.password_value or self.token_value: return True if self.token_env: - return bool(os.environ.get(self.token_env)) + return connector_secret_env_available(self.token_env, source_kind=self.source_kind) if self.password_env: - return bool(os.environ.get(self.password_env)) + return connector_secret_env_available(self.password_env, source_kind=self.source_kind) return False def to_response(self) -> dict[str, Any]: @@ -106,7 +110,7 @@ class ConnectorProfile: "username": self.username, "capabilities": list(self.capabilities), "policy_sources": [_policy_source_response(source) for source in self.policy_sources], - "metadata": dict(self.metadata), + "metadata": _response_metadata(self.metadata), "source_kind": self.source_kind, } @@ -146,7 +150,7 @@ def _profile_from_mapping(value: Mapping[str, Any]) -> ConnectorProfile: credential_mode=mode, credentials=credentials, ) - return ConnectorProfile( + result = ConnectorProfile( id=profile.id, label=profile.label, provider=profile.provider, @@ -170,6 +174,13 @@ def _profile_from_mapping(value: Mapping[str, Any]) -> ConnectorProfile: metadata=profile.metadata, source_kind="settings", ) + validate_deployment_connector_references( + source_kind=result.source_kind, + password_env=result.password_env, + token_env=result.token_env, + metadata=result.metadata, + ) + return result def _profile_id_from_mapping(value: Mapping[str, Any]) -> str: @@ -291,6 +302,16 @@ def _public_metadata(value: object) -> Mapping[str, Any]: } +def _response_metadata(value: Mapping[str, Any]) -> dict[str, Any]: + return { + str(key): item + for key, item in value.items() + if str(key).strip().casefold() not in _INLINE_SECRET_FIELDS + and not str(key).strip().casefold().endswith("_env") + and str(key).strip().casefold() != "ca_bundle" + } + + def _string_list(value: object) -> list[str]: if value is None: return [] diff --git a/src/govoplan_files/backend/storage/http_client.py b/src/govoplan_files/backend/storage/http_client.py new file mode 100644 index 0000000..533fc78 --- /dev/null +++ b/src/govoplan_files/backend/storage/http_client.py @@ -0,0 +1,239 @@ +from __future__ import annotations + +import socket +import ssl +import select +from collections.abc import Mapping +from contextlib import contextmanager +from dataclasses import dataclass +from typing import Any, Iterator + +import httpcore +import httpx + +from govoplan_core.security.outbound_http import ( + OutboundHttpError, + bounded_chunks_bytes, + create_outbound_connection, + response_limit, + validate_outbound_http_url, +) + + +class ConnectorHttpError(RuntimeError): + pass + + +@dataclass(frozen=True, slots=True) +class ConnectorHttpResponse: + status_code: int + headers: Mapping[str, str] + content: bytes + + +_HTTPCORE_TRANSPORT_ERRORS = ( + httpcore.TimeoutException, + httpcore.NetworkError, + httpcore.ProtocolError, + httpcore.ProxyError, + httpcore.UnsupportedProtocol, +) + + +class _SocketNetworkStream(httpcore.NetworkStream): + """Public httpcore NetworkStream adapter around an approved socket.""" + + def __init__(self, sock: socket.socket) -> None: + self._socket = sock + + def read(self, max_bytes: int, timeout: float | None = None) -> bytes: + try: + self._socket.settimeout(timeout) + return self._socket.recv(max_bytes) + except socket.timeout as exc: + raise httpcore.ReadTimeout(str(exc)) from exc + except OSError as exc: + raise httpcore.ReadError(str(exc)) from exc + + def write(self, buffer: bytes, timeout: float | None = None) -> None: + try: + self._socket.settimeout(timeout) + self._socket.sendall(buffer) + except socket.timeout as exc: + raise httpcore.WriteTimeout(str(exc)) from exc + except OSError as exc: + raise httpcore.WriteError(str(exc)) from exc + + def close(self) -> None: + self._socket.close() + + def start_tls( + self, + ssl_context: ssl.SSLContext, + server_hostname: str | None = None, + timeout: float | None = None, + ) -> httpcore.NetworkStream: + try: + self._socket.settimeout(timeout) + tls_socket = ssl_context.wrap_socket(self._socket, server_hostname=server_hostname) + except socket.timeout as exc: + self.close() + raise httpcore.ConnectTimeout(str(exc)) from exc + except OSError as exc: + self.close() + raise httpcore.ConnectError(str(exc)) from exc + return _SocketNetworkStream(tls_socket) + + def get_extra_info(self, info: str) -> Any: + if info == "ssl_object" and isinstance(self._socket, ssl.SSLSocket): + return self._socket + if info == "client_addr": + return self._socket.getsockname() + if info == "server_addr": + return self._socket.getpeername() + if info == "socket": + return self._socket + if info == "is_readable": + try: + return bool(select.select([self._socket], [], [], 0)[0]) + except (OSError, ValueError): + return True + return None + + +class _OutboundPolicyNetworkBackend(httpcore.NetworkBackend): + def connect_tcp( + self, + host: str, + port: int, + timeout: float | None = None, + local_address: str | None = None, + socket_options: Any = None, + ) -> httpcore.NetworkStream: + source_address = None if local_address is None else (local_address, 0) + try: + sock = create_outbound_connection( + host, + port, + timeout=timeout, + source_address=source_address, + socket_options=socket_options, + label="File connector HTTP endpoint", + ) + except socket.timeout as exc: + raise httpcore.ConnectTimeout(str(exc)) from exc + except (OSError, OutboundHttpError) as exc: + raise httpcore.ConnectError(str(exc)) from exc + return _SocketNetworkStream(sock) + + def connect_unix_socket(self, path: str, timeout: float | None = None, socket_options: Any = None): # type: ignore[no-untyped-def] + del path, timeout, socket_options + raise httpcore.ConnectError("Unix sockets are not supported for file connectors") + + +class _HttpcoreResponseStream(httpx.SyncByteStream): + def __init__(self, stream: Any, *, request: httpx.Request) -> None: + self._stream = stream + self._request = request + + def __iter__(self): # type: ignore[no-untyped-def] + try: + yield from self._stream + except _HTTPCORE_TRANSPORT_ERRORS as exc: + raise httpx.TransportError(str(exc), request=self._request) from exc + + def close(self) -> None: + if hasattr(self._stream, "close"): + self._stream.close() + + +class _OutboundPolicyHTTPTransport(httpx.BaseTransport): + def __init__(self) -> None: + self._connection_pool = httpcore.ConnectionPool( + ssl_context=httpx.create_ssl_context(verify=True, trust_env=False), + max_connections=100, + max_keepalive_connections=20, + keepalive_expiry=5.0, + network_backend=_OutboundPolicyNetworkBackend(), + ) + + def handle_request(self, request: httpx.Request) -> httpx.Response: + core_request = httpcore.Request( + method=request.method, + url=httpcore.URL( + scheme=request.url.raw_scheme, + host=request.url.raw_host, + port=request.url.port, + target=request.url.raw_path, + ), + headers=request.headers.raw, + content=request.stream, + extensions=request.extensions, + ) + try: + response = self._connection_pool.handle_request(core_request) + except _HTTPCORE_TRANSPORT_ERRORS as exc: + raise httpx.TransportError(str(exc), request=request) from exc + return httpx.Response( + status_code=response.status, + headers=response.headers, + stream=_HttpcoreResponseStream(response.stream, request=request), + extensions=response.extensions, + ) + + def close(self) -> None: + self._connection_pool.close() + + +@contextmanager +def _stream_connector_request(method: str, url: str, **kwargs: Any) -> Iterator[httpx.Response]: + with httpx.Client( + transport=_OutboundPolicyHTTPTransport(), + follow_redirects=False, + timeout=kwargs.pop("timeout", 15.0), + ) as client: + with client.stream(method, url, **kwargs) as response: + yield response + + +def request_connector_bytes( + method: str, + url: str, + *, + headers: Mapping[str, str] | None = None, + params: Mapping[str, str] | None = None, + data: Mapping[str, str] | bytes | str | None = None, + content: bytes | str | None = None, + auth: Any = None, + timeout: float = 15.0, + kind: str = "structured", + max_bytes: int | None = None, + label: str = "File connector", +) -> ConnectorHttpResponse: + try: + validated_url = validate_outbound_http_url(url, label=f"{label} URL") + effective_limit = response_limit(kind) if max_bytes is None else min(int(max_bytes), response_limit(kind)) + with _stream_connector_request( + method, + validated_url, + headers=dict(headers or {}), + params=params, + data=data, + content=content, + auth=auth, + timeout=timeout, + ) as response: + body = bounded_chunks_bytes( + response.iter_bytes(), + headers=response.headers, + max_bytes=effective_limit, + kind=kind, + label=f"{label} response", + ) + return ConnectorHttpResponse( + status_code=response.status_code, + headers=dict(response.headers), + content=body, + ) + except (httpx.HTTPError, OutboundHttpError, ValueError) as exc: + raise ConnectorHttpError(str(exc)) from exc diff --git a/tests/test_connector_deployment.py b/tests/test_connector_deployment.py new file mode 100644 index 0000000..df41159 --- /dev/null +++ b/tests/test_connector_deployment.py @@ -0,0 +1,196 @@ +from __future__ import annotations + +import os +import tempfile +import unittest +from pathlib import Path +from types import SimpleNamespace +from unittest.mock import patch +from unittest.mock import MagicMock + +from fastapi import HTTPException + +from govoplan_files.backend.router import discover_connector_endpoint +from govoplan_files.backend.schemas import FileConnectorDiscoveryRequest +from govoplan_files.backend.storage.connector_browse import ConnectorBrowseError, _profile_password, _s3_verify +from govoplan_files.backend.storage.connector_deployment import ( + ConnectorDeploymentConfigurationError, + connector_effective_endpoint_url, + connector_secret_env_value, + reject_api_controlled_deployment_references, +) +from govoplan_files.backend.storage.connector_profiles import ConnectorProfile +from govoplan_files.backend.storage.connector_profile_store import create_connector_profile_row + + +class ConnectorDeploymentBoundaryTests(unittest.TestCase): + def test_database_profile_cannot_read_arbitrary_process_environment(self) -> None: + profile = ConnectorProfile( + id="tenant-webdav", + label="Tenant WebDAV", + provider="webdav", + password_env="MASTER_KEY_B64", + source_kind="database", + ) + with patch.dict( + os.environ, + { + "MASTER_KEY_B64": "must-not-leave-process", + "GOVOPLAN_CONNECTOR_SECRET_ENV_ALLOWLIST": "MASTER_KEY_B64", + }, + clear=False, + ), self.assertRaisesRegex(ConnectorBrowseError, "deployment-owned"): + _profile_password(profile) + + def test_deployment_profile_requires_exact_secret_allowlist(self) -> None: + with patch.dict( + os.environ, + { + "GOVOPLAN_FILES_WEBDAV_PASSWORD": "deployment-secret", + "GOVOPLAN_CONNECTOR_SECRET_ENV_ALLOWLIST": "GOVOPLAN_FILES_WEBDAV_PASSWORD", + }, + clear=False, + ): + self.assertEqual( + "deployment-secret", + connector_secret_env_value( + "GOVOPLAN_FILES_WEBDAV_PASSWORD", + source_kind="settings", + ), + ) + with self.assertRaisesRegex(ConnectorDeploymentConfigurationError, "not listed"): + connector_secret_env_value("MASTER_KEY_B64", source_kind="settings") + + def test_api_profiles_cannot_select_secret_environment_names(self) -> None: + with self.assertRaisesRegex(ConnectorDeploymentConfigurationError, "deployment-owned"): + reject_api_controlled_deployment_references(password_env="DATABASE_URL") + with self.assertRaisesRegex(ConnectorDeploymentConfigurationError, "deployment-owned"): + reject_api_controlled_deployment_references(metadata={"secret_access_key_env": "MASTER_KEY_B64"}) + + session = MagicMock() + with self.assertRaisesRegex(ConnectorDeploymentConfigurationError, "deployment-owned"): + create_connector_profile_row( + session, + tenant_id="tenant-1", + user_id="user-1", + profile_id="unsafe", + label="Unsafe", + provider="webdav", + scope_type="tenant", + password_env="DATABASE_URL", + ) + session.get.assert_not_called() + + def test_profile_responses_hide_environment_and_local_ca_references(self) -> None: + profile = ConnectorProfile( + id="deployment-s3", + label="Deployment S3", + provider="s3", + metadata={ + "bucket": "documents", + "secret_access_key_env": "GOVOPLAN_FILES_S3_SECRET", + "ca_bundle": "/etc/govoplan/connector-ca.pem", + }, + ) + + self.assertEqual({"bucket": "documents"}, profile.to_response()["metadata"]) + + def test_ca_bundle_must_be_an_exact_deployment_allowlisted_file(self) -> None: + with tempfile.TemporaryDirectory() as temp_dir: + allowed = Path(temp_dir, "connector-ca.pem") + other = Path(temp_dir, "other-ca.pem") + allowed.write_text("test CA", encoding="utf-8") + other.write_text("other CA", encoding="utf-8") + with patch.dict( + os.environ, + { + "APP_ENV": "production", + "GOVOPLAN_CONNECTOR_CA_BUNDLE_ALLOWLIST": str(allowed), + }, + clear=False, + ): + profile = ConnectorProfile( + id="s3", + label="S3", + provider="s3", + metadata={"ca_bundle": str(allowed)}, + ) + self.assertEqual(str(allowed.resolve()), _s3_verify(profile)) + with self.assertRaisesRegex(ConnectorDeploymentConfigurationError, "not listed"): + reject_api_controlled_deployment_references(metadata={"ca_bundle": str(other)}) + + def test_tls_verification_can_only_be_disabled_in_development(self) -> None: + with patch.dict(os.environ, {"APP_ENV": "production"}, clear=False), self.assertRaisesRegex( + ConnectorDeploymentConfigurationError, + "dev/test", + ): + reject_api_controlled_deployment_references(metadata={"verify_tls": False}) + with patch.dict(os.environ, {"APP_ENV": "test"}, clear=False): + reject_api_controlled_deployment_references(metadata={"verify_tls": False}) + + def test_effective_endpoint_uses_webdav_override(self) -> None: + self.assertEqual( + "https://dav.example.test/root", + connector_effective_endpoint_url( + provider="seafile", + endpoint_url="https://seafile.example.test", + metadata={"webdav_endpoint_url": "https://dav.example.test/root"}, + ), + ) + + +class ConnectorDiscoveryBoundaryTests(unittest.TestCase): + @staticmethod + def _principal() -> SimpleNamespace: + return SimpleNamespace(tenant_id="tenant-1", user=SimpleNamespace(id="user-1"), api_key=None) + + def test_discovery_rejects_environment_credentials_before_io(self) -> None: + payload = FileConnectorDiscoveryRequest( + provider="webdav", + endpoint_url="https://dav.example.test", + credential_mode="basic", + credentials={"username": "admin", "password_env": "MASTER_KEY_B64"}, + ) + with patch("govoplan_files.backend.router.browse_connector_profile") as browse, self.assertRaises( + HTTPException + ) as raised: + discover_connector_endpoint(payload, session=object(), principal=self._principal()) # type: ignore[arg-type] + self.assertEqual(400, raised.exception.status_code) + browse.assert_not_called() + + def test_discovery_applies_policy_and_audit_before_each_io_candidate(self) -> None: + payload = FileConnectorDiscoveryRequest( + provider="webdav", + endpoint_url="https://dav.example.test/root", + metadata={ + "webdav_endpoint_url": "https://bypass.example.test", + "static_listing": {"": []}, + }, + ) + events: list[str] = [] + + def ensure(*_args: object, **kwargs: object) -> None: + self.assertEqual("https://dav.example.test/root/", kwargs["endpoint_url"]) + events.append("policy") + + def audit(*_args: object, **_kwargs: object) -> None: + events.append("audit") + + def browse(profile: ConnectorProfile, **_kwargs: object) -> list[object]: + self.assertEqual({}, profile.metadata) + events.append("io") + return [] + + with patch("govoplan_files.backend.router._ensure_connector_configuration_allowed", side_effect=ensure), patch( + "govoplan_files.backend.router._audit_connector_discovery_attempt", + side_effect=audit, + ), patch("govoplan_files.backend.router.browse_connector_profile", side_effect=browse): + response = discover_connector_endpoint(payload, session=object(), principal=self._principal()) # type: ignore[arg-type] + + self.assertEqual("usable", response.status) + self.assertEqual(["policy", "audit", "io"], events) + self.assertEqual({"discovered_by": "webdav-propfind"}, response.metadata) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_connector_providers.py b/tests/test_connector_providers.py index e354bab..610c867 100644 --- a/tests/test_connector_providers.py +++ b/tests/test_connector_providers.py @@ -4,7 +4,7 @@ import unittest from datetime import UTC, datetime from unittest.mock import patch -from govoplan_files.backend.storage.connector_browse import browse_connector_profile +from govoplan_files.backend.storage.connector_browse import _smb_location, browse_connector_profile from govoplan_files.backend.storage.connector_browse import ConnectorBrowseUnsupported from govoplan_files.backend.storage.connector_imports import read_connector_file from govoplan_files.backend.storage.connector_profiles import ConnectorProfile, connector_profiles_from_payload @@ -76,6 +76,24 @@ def s3_profile(**overrides: object) -> ConnectorProfile: class ConnectorProviderTests(unittest.TestCase): + def test_smb_uses_validated_numeric_target_when_private_networks_are_disabled(self) -> None: + profile = ConnectorProfile( + id="public-smb", + label="Public SMB", + provider="smb", + endpoint_url="smb://files.example.test/share", + ) + with patch.dict( + "os.environ", + {"APP_ENV": "production", "GOVOPLAN_CONNECTOR_ALLOW_PRIVATE_NETWORKS": "false"}, + ), patch( + "govoplan_core.security.outbound_http.socket.getaddrinfo", + return_value=[(2, 1, 6, "", ("93.184.216.34", 445))], + ): + location = _smb_location(profile) + + self.assertEqual("93.184.216.34", location.server) + def test_provider_descriptors_include_s3_and_reserved_microsoft_providers(self) -> None: descriptors = {descriptor.provider: descriptor for descriptor in connector_provider_descriptors()} diff --git a/tests/test_http_client.py b/tests/test_http_client.py new file mode 100644 index 0000000..a417993 --- /dev/null +++ b/tests/test_http_client.py @@ -0,0 +1,126 @@ +from __future__ import annotations + +import unittest +from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer +from threading import Thread +from unittest.mock import MagicMock, patch + +import httpcore + +from govoplan_core.security.outbound_http import outbound_http_policy, validate_outbound_http_url +from govoplan_files.backend.storage.http_client import ( + ConnectorHttpError, + _OutboundPolicyNetworkBackend, + _SocketNetworkStream, + request_connector_bytes, +) + + +class _TestHandler(BaseHTTPRequestHandler): + def do_GET(self) -> None: # noqa: N802 - stdlib handler contract + body = b"connector-ok" + self.send_response(200) + self.send_header("Content-Length", str(len(body))) + self.end_headers() + self.wfile.write(body) + + def log_message(self, _format: str, *_args: object) -> None: + return + + +class ConnectorHttpClientTests(unittest.TestCase): + def test_public_transport_adapter_performs_a_real_http_request(self) -> None: + server = ThreadingHTTPServer(("127.0.0.1", 0), _TestHandler) + thread = Thread(target=server.serve_forever, daemon=True) + thread.start() + try: + with patch.dict( + "os.environ", + {"APP_ENV": "test", "GOVOPLAN_CONNECTOR_ALLOW_PRIVATE_NETWORKS": "true"}, + clear=False, + ): + result = request_connector_bytes( + "GET", + f"http://127.0.0.1:{server.server_port}/object", + ) + finally: + server.shutdown() + server.server_close() + thread.join(timeout=2) + self.assertEqual(200, result.status_code) + self.assertEqual(b"connector-ok", result.content) + + def test_connector_responses_are_streamed_without_redirects(self) -> None: + response = MagicMock() + response.status_code = 200 + response.headers = {"content-length": "6"} + response.iter_bytes.return_value = iter((b"abc", b"def")) + context = MagicMock() + context.__enter__.return_value = response + + with patch("govoplan_files.backend.storage.http_client._stream_connector_request", return_value=context): + result = request_connector_bytes("GET", "https://example.test/object", max_bytes=10) + + self.assertEqual(b"abcdef", result.content) + + def test_connector_response_limit_is_enforced_while_streaming(self) -> None: + response = MagicMock() + response.status_code = 200 + response.headers = {} + response.iter_bytes.return_value = iter((b"12345", b"67890", b"!")) + context = MagicMock() + context.__enter__.return_value = response + + with patch( + "govoplan_files.backend.storage.http_client._stream_connector_request", + return_value=context, + ), self.assertRaisesRegex( + ConnectorHttpError, + "configured limit", + ): + request_connector_bytes("GET", "https://example.test/object", max_bytes=10) + + def test_private_destination_is_blocked_before_http_connection(self) -> None: + with patch.dict( + "os.environ", + {"APP_ENV": "production", "GOVOPLAN_CONNECTOR_ALLOW_PRIVATE_NETWORKS": "false"}, + ), patch( + "govoplan_core.security.outbound_http.socket.getaddrinfo", + return_value=[(2, 1, 6, "", ("10.0.0.5", 443))], + ), patch("govoplan_files.backend.storage.http_client._stream_connector_request") as stream, self.assertRaisesRegex( + ConnectorHttpError, + "non-public network", + ): + request_connector_bytes("GET", "https://connector.example.test/object") + stream.assert_not_called() + + def test_connection_backend_rejects_private_rebinding_before_socket_open(self) -> None: + policy = outbound_http_policy({"APP_ENV": "production"}) + public = [(2, 1, 6, "", ("93.184.216.34", 443))] + private = [(2, 1, 6, "", ("127.0.0.1", 443))] + with patch.dict( + "os.environ", + {"APP_ENV": "production", "GOVOPLAN_CONNECTOR_ALLOW_PRIVATE_NETWORKS": "false"}, + ), patch( + "govoplan_core.security.outbound_http.socket.getaddrinfo", + side_effect=(public, private), + ), patch("govoplan_core.security.outbound_http.socket.socket") as socket_factory: + validate_outbound_http_url("https://connector.example.test/object", policy=policy) + with self.assertRaises(httpcore.ConnectError): + _OutboundPolicyNetworkBackend().connect_tcp("connector.example.test", 443) + socket_factory.assert_not_called() + + def test_network_backend_returns_public_network_stream_adapter(self) -> None: + sock = MagicMock() + with patch( + "govoplan_files.backend.storage.http_client.create_outbound_connection", + return_value=sock, + ): + stream = _OutboundPolicyNetworkBackend().connect_tcp("connector.example.test", 443) + + self.assertIsInstance(stream, _SocketNetworkStream) + self.assertIs(stream.get_extra_info("socket"), sock) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_storage_backends.py b/tests/test_storage_backends.py new file mode 100644 index 0000000..f0c856a --- /dev/null +++ b/tests/test_storage_backends.py @@ -0,0 +1,64 @@ +from __future__ import annotations + +import io +import unittest +from unittest.mock import MagicMock, PropertyMock, patch + +from govoplan_files.backend.storage.backends import S3StorageBackend, StorageBackendError + + +def _backend() -> S3StorageBackend: + return S3StorageBackend( + bucket="files", + endpoint_url="https://objects.example.test", + region_name="test", + access_key_id="access", + secret_access_key="secret", + ) + + +class S3StorageBackendTests(unittest.TestCase): + def test_get_bytes_rejects_declared_oversize_object_without_reading(self) -> None: + body = MagicMock() + client = MagicMock() + client.get_object.return_value = {"ContentLength": 6, "Body": body} + with patch.dict("os.environ", {"GOVOPLAN_CONNECTOR_MAX_FILE_TRANSFER_BYTES": "5"}), patch.object( + S3StorageBackend, + "client", + new_callable=PropertyMock, + return_value=client, + ), self.assertRaisesRegex(StorageBackendError, "deployment limit"): + _backend().get_bytes("large.bin") + body.read.assert_not_called() + body.close.assert_called_once_with() + + def test_iter_bytes_rejects_undeclared_oversize_object(self) -> None: + client = MagicMock() + client.get_object.return_value = {"Body": io.BytesIO(b"123456")} + with patch.dict("os.environ", {"GOVOPLAN_CONNECTOR_MAX_FILE_TRANSFER_BYTES": "5"}), patch.object( + S3StorageBackend, + "client", + new_callable=PropertyMock, + return_value=client, + ), self.assertRaisesRegex(StorageBackendError, "deployment limit"): + list(_backend().iter_bytes("large.bin", chunk_size=3)) + + def test_iter_bytes_closes_streaming_body_when_consumer_stops_early(self) -> None: + body = MagicMock() + body.read.side_effect = (b"123", b"456", b"") + client = MagicMock() + client.get_object.return_value = {"ContentLength": 6, "Body": body} + with patch.dict("os.environ", {"GOVOPLAN_CONNECTOR_MAX_FILE_TRANSFER_BYTES": "10"}), patch.object( + S3StorageBackend, + "client", + new_callable=PropertyMock, + return_value=client, + ): + chunks = _backend().iter_bytes("object.bin", chunk_size=3) + self.assertEqual(b"123", next(chunks)) + chunks.close() + body.close.assert_called_once_with() + + +if __name__ == "__main__": + unittest.main()