From 706603fe4d0e2f0b8b2d42c18c30d21528ae1e72 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9sar=20Arroba?= <19954079+cesararroba@users.noreply.github.com> Date: Thu, 24 Sep 2026 10:24:19 +0200 Subject: [PATCH] feat(api): add an explicit endpoint for S3-compatible scan output storage (#12871) --- .env | 7 +++ .../s3-output-internal-endpoint.added.md | 1 + api/src/backend/config/django/base.py | 3 + api/src/backend/tasks/jobs/export.py | 35 ++++++++---- api/src/backend/tasks/tests/test_export.py | 56 ++++++++++++++++++- 5 files changed, 91 insertions(+), 11 deletions(-) create mode 100644 api/changelog.d/s3-output-internal-endpoint.added.md diff --git a/.env b/.env index 548d346c52..3f80a2460b 100644 --- a/.env +++ b/.env @@ -117,6 +117,13 @@ DJANGO_OUTPUT_S3_AWS_DEFAULT_REGION="" # The name of the S3 bucket where scan output should be stored DJANGO_OUTPUT_S3_AWS_OUTPUT_BUCKET="" +# The storage endpoint the API and Celery workers use to upload and list scan output +# (e.g. "http://minio:9000"). Leave empty on AWS S3. Set it when scan output is stored on +# S3-compatible object storage such as MinIO instead of real S3. +# If set without DJANGO_OUTPUT_S3_AWS_PUBLIC_ENDPOINT_URL below, report download URLs are +# signed against this internal host, and a browser outside the container network cannot open them. +DJANGO_OUTPUT_S3_AWS_ENDPOINT_URL="" + # The storage address the browser can reach, used only to sign report download URLs # (e.g. "https://storage.example.com"). Leave empty on AWS S3. Set it when storage is # only reachable inside the container network, such as MinIO on "http://minio:9000". diff --git a/api/changelog.d/s3-output-internal-endpoint.added.md b/api/changelog.d/s3-output-internal-endpoint.added.md new file mode 100644 index 0000000000..88159f1ef8 --- /dev/null +++ b/api/changelog.d/s3-output-internal-endpoint.added.md @@ -0,0 +1 @@ +Scan output uploads and downloads can now target S3-compatible object storage such as MinIO directly via `DJANGO_OUTPUT_S3_AWS_ENDPOINT_URL`, instead of relying on process-wide AWS environment variables that also hijacked unrelated AWS API calls diff --git a/api/src/backend/config/django/base.py b/api/src/backend/config/django/base.py index a208dda915..202476bd4b 100644 --- a/api/src/backend/config/django/base.py +++ b/api/src/backend/config/django/base.py @@ -295,6 +295,9 @@ DJANGO_OUTPUT_S3_AWS_SECRET_ACCESS_KEY = env.str( ) DJANGO_OUTPUT_S3_AWS_SESSION_TOKEN = env.str("DJANGO_OUTPUT_S3_AWS_SESSION_TOKEN", "") DJANGO_OUTPUT_S3_AWS_DEFAULT_REGION = env.str("DJANGO_OUTPUT_S3_AWS_DEFAULT_REGION", "") +# Storage endpoint the API and Celery workers use to talk to S3-compatible object storage +# such as MinIO. Empty means the real AWS S3 endpoint, which is unaffected. +DJANGO_OUTPUT_S3_AWS_ENDPOINT_URL = env.str("DJANGO_OUTPUT_S3_AWS_ENDPOINT_URL", "") # Browser-reachable storage host used to sign download URLs. Empty means sign against the # same endpoint the API talks to, which is what Prowler Cloud on S3 does. DJANGO_OUTPUT_S3_AWS_PUBLIC_ENDPOINT_URL = env.str( diff --git a/api/src/backend/tasks/jobs/export.py b/api/src/backend/tasks/jobs/export.py index 6e0f2477ef..49b91c219f 100644 --- a/api/src/backend/tasks/jobs/export.py +++ b/api/src/backend/tasks/jobs/export.py @@ -207,15 +207,21 @@ def get_s3_client(): This function attempts to initialize an S3 client by reading the AWS access key, secret key, session token, and region from environment variables. It then validates the client by listing available S3 buckets. If an error occurs during this process (for example, due to missing or - invalid credentials), it falls back to creating an S3 client without explicitly provided credentials, - which may rely on other configuration sources (e.g., IAM roles). + invalid credentials), it falls back to creating an S3 client without explicitly provided + credentials, which may rely on other configuration sources (e.g., IAM roles). + + That fallback is only safe when no explicit endpoint is configured: with an endpoint set, the + explicit client already targets the intended S3-compatible storage, and the fallback client + would go to the AWS default provider chain instead, an unrelated real-AWS account reachable + from the host. So when an endpoint is configured, the original error propagates instead. Returns: boto3.client: A configured S3 client instance. Raises: - ClientError, NoCredentialsError, or ParamValidationError if both attempts to create a client fail. + ClientError, NoCredentialsError, or ParamValidationError if the client cannot be created. """ + endpoint = settings.DJANGO_OUTPUT_S3_AWS_ENDPOINT_URL s3_client = None try: s3_client = boto3.client( @@ -226,9 +232,12 @@ def get_s3_client(): # Storage that has no meaningful region, MinIO among it, is usually configured # without one, and botocore rejects an empty region before any request is made. region_name=settings.DJANGO_OUTPUT_S3_AWS_DEFAULT_REGION or "us-east-1", + endpoint_url=endpoint or None, ) s3_client.list_buckets() except (ClientError, NoCredentialsError, ParamValidationError, ValueError): + if endpoint: + raise s3_client = boto3.client("s3") s3_client.list_buckets() @@ -238,13 +247,19 @@ def get_s3_client(): def get_s3_presign_client(): """Return a client that signs download URLs with SigV4. - It is used when a public storage host is configured, or when the bucket's region is: - boto3 otherwise presigns S3 URLs with SigV2, which S3 rejects for SSE-KMS objects. - None means neither is set and the caller should presign with its own client, which - leaves those deployments with the URL they get today. + It is used when a public or internal storage host is configured, or when the bucket's + region is: boto3 otherwise presigns S3 URLs with SigV2, which S3 rejects for SSE-KMS + objects. None means none of those is set and the caller should presign with its own + client, which leaves those deployments with the URL they get today. + + The public endpoint wins when both are set: the internal endpoint may only be reachable + from inside the cluster, and a URL signed against it would not open in a browser. """ - public_endpoint = settings.DJANGO_OUTPUT_S3_AWS_PUBLIC_ENDPOINT_URL - if not public_endpoint and not settings.DJANGO_OUTPUT_S3_AWS_DEFAULT_REGION: + endpoint = ( + settings.DJANGO_OUTPUT_S3_AWS_PUBLIC_ENDPOINT_URL + or settings.DJANGO_OUTPUT_S3_AWS_ENDPOINT_URL + ) + if not endpoint and not settings.DJANGO_OUTPUT_S3_AWS_DEFAULT_REGION: return None # Blank keys are signed as-is (empty credential scope) instead of deferring to the @@ -268,7 +283,7 @@ def get_s3_presign_client(): # SigV4 puts the region in the credential scope, and MinIO answers to us-east-1 # unless it was told otherwise, so an empty region would sign an unusable URL. region_name=settings.DJANGO_OUTPUT_S3_AWS_DEFAULT_REGION or "us-east-1", - endpoint_url=public_endpoint or None, + endpoint_url=endpoint or None, # The signature covers the host, so the addressing style has to be pinned rather # than guessed from the endpoint: MinIO serves path-style, and on AWS it keeps the # regional host instead of the global one, which redirects for new buckets. diff --git a/api/src/backend/tasks/tests/test_export.py b/api/src/backend/tasks/tests/test_export.py index 1f9aa39a3d..3fca90e065 100644 --- a/api/src/backend/tasks/tests/test_export.py +++ b/api/src/backend/tasks/tests/test_export.py @@ -3,7 +3,7 @@ import uuid import zipfile from datetime import datetime from pathlib import Path -from unittest.mock import MagicMock, patch +from unittest.mock import MagicMock, call, patch from urllib.parse import parse_qs, urlparse import boto3 @@ -64,15 +64,46 @@ class TestOutputs: assert mock_boto_client.call_args.kwargs["region_name"] == "us-east-1" + @patch("tasks.jobs.export.boto3.client") + @override_settings(DJANGO_OUTPUT_S3_AWS_ENDPOINT_URL="http://minio:9000") + def test_get_s3_client_passes_the_endpoint_when_set(self, mock_boto_client): + get_s3_client() + + assert mock_boto_client.call_args.kwargs["endpoint_url"] == "http://minio:9000" + + @patch("tasks.jobs.export.boto3.client") + @override_settings(DJANGO_OUTPUT_S3_AWS_ENDPOINT_URL="") + def test_get_s3_client_endpoint_empty_by_default(self, mock_boto_client): + """Empty keeps today's behavior: no endpoint override, real S3 is used.""" + get_s3_client() + + assert mock_boto_client.call_args.kwargs["endpoint_url"] is None + + @patch("tasks.jobs.export.boto3.client") + @override_settings(DJANGO_OUTPUT_S3_AWS_ENDPOINT_URL="http://minio:9000") + def test_get_s3_client_does_not_fall_back_when_endpoint_set(self, mock_boto_client): + """A configured endpoint means the explicit client failed talking to it. The fallback + goes to the default provider chain (e.g. an EC2 instance role) against real AWS, so it + must not be used: the original error propagates instead.""" + error = ClientError({"Error": {"Code": "403"}}, "ListBuckets") + mock_boto_client.side_effect = error + + with pytest.raises(ClientError): + get_s3_client() + + mock_boto_client.assert_called_once() + @patch("tasks.jobs.export.boto3.client") @patch("tasks.jobs.export.settings") def test_get_s3_client_fallback(self, mock_settings, mock_boto_client): + mock_settings.DJANGO_OUTPUT_S3_AWS_ENDPOINT_URL = "" mock_boto_client.side_effect = [ ClientError({"Error": {"Code": "403"}}, "ListBuckets"), MagicMock(), ] client = get_s3_client() assert client is not None + assert mock_boto_client.call_args_list[1] == call("s3") @patch("tasks.jobs.export.get_s3_client") @patch("tasks.jobs.export.base") @@ -320,6 +351,29 @@ class TestS3PresignClient: assert query["X-Amz-Credential"][0].startswith("role-access-key/") assert "/eu-west-1/s3/aws4_request" in query["X-Amz-Credential"][0] + @override_settings( + **{**PRESIGN_SETTINGS, "DJANGO_OUTPUT_S3_AWS_DEFAULT_REGION": ""}, + DJANGO_OUTPUT_S3_AWS_PUBLIC_ENDPOINT_URL="", + DJANGO_OUTPUT_S3_AWS_ENDPOINT_URL="http://minio:9000", + ) + def test_internal_endpoint_without_public_endpoint_signs_against_it(self): + # No browser-reachable host was configured, so the internal one is the best + # available target instead of falling through to the real AWS host. + url = urlparse(_presign(get_s3_presign_client())) + + assert url.netloc == "minio:9000" + assert url.path == "/output-bucket/tenant/scan/report.zip" + + @override_settings( + **PRESIGN_SETTINGS, + DJANGO_OUTPUT_S3_AWS_PUBLIC_ENDPOINT_URL="https://storage.example.com", + DJANGO_OUTPUT_S3_AWS_ENDPOINT_URL="http://minio:9000", + ) + def test_public_endpoint_wins_over_the_internal_endpoint(self): + url = urlparse(_presign(get_s3_presign_client())) + + assert url.netloc == "storage.example.com" + @override_settings( **PRESIGN_SETTINGS, DJANGO_OUTPUT_S3_AWS_PUBLIC_ENDPOINT_URL="https://storage.example.com",