diff --git a/api/CHANGELOG.md b/api/CHANGELOG.md index a1f73e0700..213ee215de 100644 --- a/api/CHANGELOG.md +++ b/api/CHANGELOG.md @@ -2,6 +2,14 @@ All notable changes to the **Prowler API** are documented in this file. +## [1.25.3] (Prowler v5.24.3) + +### 🐞 Fixed + +- Finding groups aggregated `status` now treats muted findings as resolved: a group is `FAIL` only while at least one non-muted FAIL remains, otherwise it is `PASS` (including fully-muted groups). The `filter[status]` filter and the `sort=status` ordering share the same semantics, keeping `status` consistent with `fail_count` and the orthogonal `muted` flag [(#10825)](https://github.com/prowler-cloud/prowler/pull/10825) + +--- + ## [1.25.2] (Prowler v5.24.2) ### πŸ”„ Changed diff --git a/api/src/backend/api/tests/test_views.py b/api/src/backend/api/tests/test_views.py index 3d2107a03e..b2e5271bda 100644 --- a/api/src/backend/api/tests/test_views.py +++ b/api/src/backend/api/tests/test_views.py @@ -15446,15 +15446,15 @@ class TestFindingGroupViewSet: # iam_password_policy has only PASS findings assert data[0]["attributes"]["status"] == "PASS" - def test_finding_groups_fully_muted_group_reflects_underlying_status( + def test_finding_groups_fully_muted_group_is_pass( self, authenticated_client, finding_groups_fixture ): - """A fully-muted group still surfaces its underlying status (no MUTED). + """A fully-muted group reports status=PASS and muted=True. - rds_encryption has 2 muted FAIL findings, so the group must report - status=FAIL (the orthogonal `muted` boolean signals it isn't actionable). - The statusΓ—muted breakdown lets clients answer 'how many failing - findings are muted in this group'. + rds_encryption has 2 muted FAIL findings. Muted findings are treated + as resolved/accepted, so the group is no longer actionable and its + status must be PASS. The `muted` flag is True because every finding + in the group is muted. """ response = authenticated_client.get( reverse("finding-group-list"), @@ -15464,7 +15464,7 @@ class TestFindingGroupViewSet: data = response.json()["data"] assert len(data) == 1 attrs = data[0]["attributes"] - assert attrs["status"] == "FAIL" + assert attrs["status"] == "PASS" assert attrs["muted"] is True assert attrs["fail_count"] == 0 assert attrs["fail_muted_count"] == 2 @@ -15479,6 +15479,83 @@ class TestFindingGroupViewSet: == attrs["muted_count"] ) + def test_finding_groups_status_ignores_muted_failures( + self, + authenticated_client, + tenants_fixture, + scans_fixture, + resources_fixture, + ): + """Muted FAIL findings must not drive the aggregated status. + + When a group mixes one non-muted PASS with one muted FAIL, the + actionable outcome is PASS: there are no unmuted failures left. The + aggregated `status` must reflect that (not FAIL), while `muted` + stays False because the group still has a non-muted finding. + """ + tenant = tenants_fixture[0] + scan1, *_ = scans_fixture + resource1, *_ = resources_fixture + + pass_finding = Finding.objects.create( + tenant_id=tenant.id, + uid="fg_mixed_muted_pass", + scan=scan1, + delta=None, + status=Status.PASS, + severity=Severity.low, + impact=Severity.low, + check_id="mixed_muted_check", + check_metadata={ + "CheckId": "mixed_muted_check", + "checktitle": "Mixed muted check", + "Description": "Fixture for muted status aggregation.", + }, + first_seen_at="2024-01-11T00:00:00Z", + muted=False, + ) + pass_finding.add_resources([resource1]) + + fail_muted_finding = Finding.objects.create( + tenant_id=tenant.id, + uid="fg_mixed_muted_fail", + scan=scan1, + delta=None, + status=Status.FAIL, + severity=Severity.high, + impact=Severity.high, + check_id="mixed_muted_check", + check_metadata={ + "CheckId": "mixed_muted_check", + "checktitle": "Mixed muted check", + "Description": "Fixture for muted status aggregation.", + }, + first_seen_at="2024-01-12T00:00:00Z", + muted=True, + ) + fail_muted_finding.add_resources([resource1]) + + # filter[region] forces finding-level aggregation so we exercise the + # raw-findings path without touching the daily summary fixture. + response = authenticated_client.get( + reverse("finding-group-list"), + { + "filter[inserted_at]": TODAY, + "filter[check_id]": "mixed_muted_check", + "filter[region]": "us-east-1", + }, + ) + assert response.status_code == status.HTTP_200_OK + data = response.json()["data"] + assert len(data) == 1 + attrs = data[0]["attributes"] + assert attrs["status"] == "PASS" + assert attrs["muted"] is False + assert attrs["pass_count"] == 1 + assert attrs["fail_count"] == 0 + assert attrs["fail_muted_count"] == 1 + assert attrs["muted_count"] == 1 + def test_finding_groups_status_filter( self, authenticated_client, finding_groups_fixture ): diff --git a/api/src/backend/api/v1/views.py b/api/src/backend/api/v1/views.py index 5a2c790655..2f74d0283d 100644 --- a/api/src/backend/api/v1/views.py +++ b/api/src/backend/api/v1/views.py @@ -7281,14 +7281,18 @@ class FindingGroupViewSet(BaseRLSViewSet): # finding-level aggregation path. row.pop("nonmuted_count", None) - # Compute aggregated status from non-muted counts first, then - # fall back to muted counts so fully-muted groups still reflect - # the underlying check outcome. - total_fail = row.get("fail_count", 0) + row.get("fail_muted_count", 0) - total_pass = row.get("pass_count", 0) + row.get("pass_muted_count", 0) - if total_fail > 0: + # Muted findings are treated as resolved/accepted, so they do not + # contribute to a failing status. A group is FAIL only when there + # is at least one non-muted FAIL; otherwise any pass (muted or + # not) or any muted fail makes the group PASS. Only groups whose + # findings are exclusively MANUAL fall through to MANUAL. + if row.get("fail_count", 0) > 0: row["status"] = "FAIL" - elif total_pass > 0: + elif ( + row.get("pass_count", 0) > 0 + or row.get("pass_muted_count", 0) > 0 + or row.get("fail_muted_count", 0) > 0 + ): row["status"] = "PASS" else: row["status"] = "MANUAL" @@ -7388,12 +7392,11 @@ class FindingGroupViewSet(BaseRLSViewSet): if computed_params.get("status") or computed_params.getlist("status__in"): queryset = queryset.annotate( - total_fail=F("fail_count") + F("fail_muted_count"), - total_pass=F("pass_count") + F("pass_muted_count"), - ).annotate( aggregated_status=Case( - When(total_fail__gt=0, then=Value("FAIL")), - When(total_pass__gt=0, then=Value("PASS")), + When(fail_count__gt=0, then=Value("FAIL")), + When(pass_count__gt=0, then=Value("PASS")), + When(pass_muted_count__gt=0, then=Value("PASS")), + When(fail_muted_count__gt=0, then=Value("PASS")), default=Value("MANUAL"), output_field=CharField(), ) @@ -7798,12 +7801,11 @@ class FindingGroupViewSet(BaseRLSViewSet): if ordering: if any(field.lstrip("-") == "status_order" for field in ordering): aggregated_queryset = aggregated_queryset.annotate( - total_fail_for_sort=F("fail_count") + F("fail_muted_count"), - total_pass_for_sort=F("pass_count") + F("pass_muted_count"), - ).annotate( status_order=Case( - When(total_fail_for_sort__gt=0, then=Value(3)), - When(total_pass_for_sort__gt=0, then=Value(2)), + When(fail_count__gt=0, then=Value(3)), + When(pass_count__gt=0, then=Value(2)), + When(pass_muted_count__gt=0, then=Value(2)), + When(fail_muted_count__gt=0, then=Value(2)), default=Value(1), output_field=IntegerField(), ) diff --git a/docs/user-guide/providers/aws/authentication.mdx b/docs/user-guide/providers/aws/authentication.mdx index b919cde8ec..22deb6ab99 100644 --- a/docs/user-guide/providers/aws/authentication.mdx +++ b/docs/user-guide/providers/aws/authentication.mdx @@ -7,6 +7,11 @@ Prowler requires AWS credentials to function properly. Authentication is availab - Static Credentials - Assumed Role +When using **Assumed Role**, the Prowler UI exposes two credential sources for calling `sts:AssumeRole`. The labels differ between Prowler Cloud and self-hosted Prowler App, but both map to the same underlying credential types: + +- **AWS SDK Default** (shown as *"Prowler Cloud will assume your IAM role"* in Prowler Cloud and *"AWS SDK Default"* in self-hosted Prowler App): Prowler uses the credentials already available to the API and worker containers through the [AWS SDK default credential chain](https://boto3.amazonaws.com/v1/documentation/api/latest/guide/credentials.html). This is the default in Prowler Cloud and requires extra configuration in self-hosted Prowler App (see [Configuring AWS SDK Default for Self-Hosted Prowler App](#configuring-aws-sdk-default-for-self-hosted-prowler-app)). +- **Access & Secret Key**: You paste an IAM user's `AWS_ACCESS_KEY_ID`, `AWS_SECRET_ACCESS_KEY`, and optionally `AWS_SESSION_TOKEN` into the form. Prowler uses those keys to call `sts:AssumeRole`. + ## Required Permissions To ensure full functionality, attach the following AWS managed policies to the designated user or role: @@ -76,6 +81,68 @@ This method grants permanent access and is the recommended setup for production --- +## Configuring AWS SDK Default for Self-Hosted Prowler App + +When self-hosting Prowler App with Docker Compose, the API and worker containers do not have AWS credentials by default. Selecting **AWS SDK Default** without configuring those credentials produces: + +``` +AWSAssumeRoleError[1012]: AWS assume role error - An error occurred (InvalidClientTokenId) when calling the AssumeRole operation: The security token included in the request is invalid. +``` + +To fix this, expose an IAM identity with `sts:AssumeRole` permission on the target role to both the `api` and `worker` services. + +### Option 1: Environment Variables in `.env` + +Add the following keys to the `.env` file used by `docker-compose.yml`: + +```bash +AWS_ACCESS_KEY_ID="" +AWS_SECRET_ACCESS_KEY="" +AWS_SESSION_TOKEN="" +AWS_DEFAULT_REGION="us-east-1" +``` + +The existing `docker-compose.yml` already loads `.env` into the `api`, `worker`, and `worker-beat` services, so `boto3` will pick them up through the default credential chain. + + +Treat the `.env` file as a secret. Do not commit it to version control, scope the IAM identity to the minimum permissions required (`sts:AssumeRole` on the target `ProwlerScan` role only), prefer short-lived credentials over long-lived access keys, and rotate the keys immediately if you suspect exposure. + + +Recreate the containers to apply the change. A plain `docker compose restart` will **not** reload values from a modified `.env` file β€” you must force-recreate: + +```bash +docker compose up -d --force-recreate api worker worker-beat +``` + +### Option 2: IAM Role (Host with Instance Metadata) + +If you run Prowler App on an EC2 instance, ECS task, or EKS pod with an attached IAM role that can assume the scan role, no extra configuration is needed β€” `boto3` resolves credentials through instance or task metadata automatically. + +### Trust Policy: Align `IAMPrincipal` With Your Identity + +The [Prowler scan role CloudFormation template](https://github.com/prowler-cloud/prowler/blob/master/permissions/templates/cloudformation/prowler-scan-role.yml) restricts the trust policy with: + +``` +aws:PrincipalArn StringLike arn:aws:iam::: +``` + +`IAMPrincipal` defaults to `role/prowler*`, which only allows IAM roles whose name starts with `prowler`. If the identity hosting the API and worker containers is anything else, the `sts:AssumeRole` call fails with `AccessDenied` even when the credentials themselves are valid. + +Redeploy (or update) the CloudFormation stack with an `IAMPrincipal` that matches your identity: + +| Your identity on the API/worker containers | `IAMPrincipal` value | +| --- | --- | +| IAM user (for example `prowler-app`) | `user/prowler-app` | +| IAM role whose name doesn't start with `prowler` | `role/` | + +`AccountId` must also point to the account where that identity lives β€” the default is Prowler Cloud's account and only applies when assuming from Prowler Cloud. + + +The same `External ID` entered in the Prowler UI must match the `ExternalId` parameter used when deploying the CloudFormation stack. A mismatch produces `AccessDenied` on `sts:AssumeRole`, not `InvalidClientTokenId`. + + +--- + ## Credentials diff --git a/docs/user-guide/providers/aws/getting-started-aws.mdx b/docs/user-guide/providers/aws/getting-started-aws.mdx index 2c94700b15..8127e873e7 100644 --- a/docs/user-guide/providers/aws/getting-started-aws.mdx +++ b/docs/user-guide/providers/aws/getting-started-aws.mdx @@ -46,15 +46,15 @@ Before proceeding, choose the preferred authentication mode: **Credentials** -* Quick scan as current user -* No extra setup -* Credentials time out +* Quick scan using an IAM user's access keys +* No extra setup in AWS +* Static keys can be rotated or revoked at any time **Assumed Role** -* Preferred Setup -* Permanent Credentials -* Requires access to create role +* Recommended for production +* With AWS SDK Default as the credential source, no long-lived keys are stored in Prowler (Access & Secret Key still requires pasted keys) +* Requires permission to create an IAM role in the target account --- @@ -67,18 +67,23 @@ This method grants permanent access and is the recommended setup for production For detailed instructions on how to create the role, see [Authentication > Assume Role](/user-guide/providers/aws/authentication#assume-role-recommended). -8. Once the role is created, go to the **IAM Console**, click on the "ProwlerScan" role to open its details: +7. Once the role is created, go to the **IAM Console**, click on the "ProwlerScan" role to open its details: ![ProwlerScan role info](/images/providers/prowler-scan-pre-info.png) -9. Copy the **Role ARN** +8. Copy the **Role ARN** ![New Role Info](/images/providers/get-role-arn.png) -10. Paste the ARN into the corresponding field in Prowler Cloud or Prowler App +9. Paste the ARN into the corresponding field in Prowler Cloud or Prowler App ![Input the Role ARN](/images/providers/paste-role-arn-prowler.png) +10. Select the credential source Prowler should use to call `sts:AssumeRole`. The option label differs between deployments but both map to the same `aws-sdk-default` credential type: + + - **"Prowler Cloud will assume your IAM role"** (default in Prowler Cloud) / **"AWS SDK Default"** (in self-hosted Prowler App): Prowler uses the credentials available in the API and worker environment through the [AWS SDK default credential chain](https://boto3.amazonaws.com/v1/documentation/api/latest/guide/credentials.html). In self-hosted Prowler App, these containers have no AWS credentials by default β€” see [Configuring AWS SDK Default for Self-Hosted Prowler App](/user-guide/providers/aws/authentication#configuring-aws-sdk-default-for-self-hosted-prowler-app) before choosing this option, or the connection test will fail with `InvalidClientTokenId`. + - **Access & Secret Key**: Paste an IAM user's `AWS_ACCESS_KEY_ID` and `AWS_SECRET_ACCESS_KEY` (and optional `AWS_SESSION_TOKEN`) into the form. The IAM principal must be allowed to assume the target role and must match the `IAMPrincipal` parameter of the scan role template (default: `role/prowler*`). + 11. Click "Next", then "Launch Scan" ![Next button in Prowler Cloud](/images/providers/next-button-prowler-cloud.png) diff --git a/prowler/CHANGELOG.md b/prowler/CHANGELOG.md index a3a9150933..250f05678c 100644 --- a/prowler/CHANGELOG.md +++ b/prowler/CHANGELOG.md @@ -7,11 +7,20 @@ All notable changes to the **Prowler SDK** are documented in this file. ### πŸš€ Added - Support for external/custom providers, checks, and compliance frameworks without modifying core code [(#10700)](https://github.com/prowler-cloud/prowler/pull/10700) +## [5.24.3] (Prowler v5.24.3) + +### 🐞 Fixed + +- CloudTrail resource timeline uses resource name as fallback in `LookupEvents` [(#10828)](https://github.com/prowler-cloud/prowler/pull/10828) --- ## [5.24.1] (Prowler v5.24.1) +### πŸš€ Added + +- `--repo-list-file` CLI flag for GitHub provider to load repositories from a file [(#10501)](https://github.com/prowler-cloud/prowler/pull/10501) + ### πŸ”„ Changed - `msgraph-sdk` from 1.23.0 to 1.55.0 and `azure-mgmt-resource` from 23.3.0 to 24.0.0, removing `marshmallow` as is a transitively dev dependency [(#10733)](https://github.com/prowler-cloud/prowler/pull/10733) diff --git a/prowler/providers/aws/lib/cloudtrail_timeline/cloudtrail_timeline.py b/prowler/providers/aws/lib/cloudtrail_timeline/cloudtrail_timeline.py index 45dff270d0..b73d070078 100644 --- a/prowler/providers/aws/lib/cloudtrail_timeline/cloudtrail_timeline.py +++ b/prowler/providers/aws/lib/cloudtrail_timeline/cloudtrail_timeline.py @@ -135,25 +135,54 @@ class CloudTrailTimeline(TimelineService): ) -> List[Dict[str, Any]]: """Query CloudTrail for events related to a specific resource. - Uses MaxResults to limit the number of events returned, preparing - for API-level pagination. Currently returns up to max_results events - from the first page only. + CloudTrail's ResourceName attribute is populated per-service by AWS + and is not consistent: KMS and SNS store full ARNs, while S3, IAM, + EC2, Lambda, RDS and others store only the resource name or ID. We + first look up using the identifier as-is, and if no events come back + we retry with the last segment extracted from the ARN. """ client = self._get_client(region) start_time = datetime.now(timezone.utc) - timedelta(days=self._lookback_days) - # Use direct API call with MaxResults instead of paginator - # This limits CloudTrail to return only max_results events + events = self._lookup_events_by_name(client, resource_identifier, start_time) + + if not events and resource_identifier.startswith("arn:"): + short_name = self._extract_short_name(resource_identifier) + if short_name and short_name != resource_identifier: + logger.debug( + f"CloudTrail timeline: no events for '{resource_identifier}', " + f"retrying lookup with short name '{short_name}'" + ) + events = self._lookup_events_by_name(client, short_name, start_time) + + return events + + def _lookup_events_by_name( + self, client, resource_name: str, start_time: datetime + ) -> List[Dict[str, Any]]: response = client.lookup_events( LookupAttributes=[ - {"AttributeKey": "ResourceName", "AttributeValue": resource_identifier} + {"AttributeKey": "ResourceName", "AttributeValue": resource_name} ], StartTime=start_time, MaxResults=self._max_results, ) - return response.get("Events", []) + @staticmethod + def _extract_short_name(identifier: str) -> str: + """Return the last segment of an ARN or identifier. + + ARNs take the form `arn:partition:service:region:account:resource-info` + where resource-info is one of `name`, `type/name`, or `type:name`. + Splitting on the final `/` and then the final `:` yields the value + CloudTrail stores for most services: S3 bucket name, IAM user/role + name, EC2 resource ID, Lambda function name, RDS DB identifier, etc. + """ + if not identifier: + return identifier + return identifier.rsplit("/", 1)[-1].rsplit(":", 1)[-1] + def _parse_event(self, raw_event: Dict[str, Any]) -> Optional[Dict[str, Any]]: """Parse a raw CloudTrail event into a TimelineEvent dictionary.""" try: diff --git a/prowler/providers/common/provider.py b/prowler/providers/common/provider.py index df7d5997aa..5217dba7f2 100644 --- a/prowler/providers/common/provider.py +++ b/prowler/providers/common/provider.py @@ -363,6 +363,7 @@ class Provider(ABC): mutelist_path=arguments.mutelist_file, config_path=arguments.config_file, repositories=repos, + repo_list_file=getattr(arguments, "repo_list_file", None), organizations=orgs, ) elif "googleworkspace" in provider_class_name.lower(): diff --git a/prowler/providers/github/exceptions/exceptions.py b/prowler/providers/github/exceptions/exceptions.py index b49cce8ebe..b0f472a76c 100644 --- a/prowler/providers/github/exceptions/exceptions.py +++ b/prowler/providers/github/exceptions/exceptions.py @@ -34,6 +34,14 @@ class GithubBaseException(ProwlerException): "message": "The provided provider ID does not match with the authenticated user or accessible organizations", "remediation": "Check the provider ID and ensure it matches the authenticated user or an organization you have access to.", }, + (5007, "GithubRepoListFileNotFoundError"): { + "message": "The repo list file was not found", + "remediation": "Check the file path and ensure it exists.", + }, + (5008, "GithubRepoListFileReadError"): { + "message": "Error reading the repo list file", + "remediation": "Check the file permissions and format.", + }, } def __init__(self, code, file=None, original_exception=None, message=None): @@ -104,3 +112,21 @@ class GithubInvalidProviderIdError(GithubCredentialsError): super().__init__( 5006, file=file, original_exception=original_exception, message=message ) + + +class GithubRepoListFileNotFoundError(GithubBaseException): + """Exception raised when the repo list file is not found.""" + + def __init__(self, file=None, original_exception=None, message=None): + super().__init__( + 5007, file=file, original_exception=original_exception, message=message + ) + + +class GithubRepoListFileReadError(GithubBaseException): + """Exception raised when the repo list file cannot be read.""" + + def __init__(self, file=None, original_exception=None, message=None): + super().__init__( + 5008, file=file, original_exception=original_exception, message=message + ) diff --git a/prowler/providers/github/github_provider.py b/prowler/providers/github/github_provider.py index 16d13f7434..ab3441b81c 100644 --- a/prowler/providers/github/github_provider.py +++ b/prowler/providers/github/github_provider.py @@ -22,6 +22,8 @@ from prowler.providers.github.exceptions.exceptions import ( GithubInvalidCredentialsError, GithubInvalidProviderIdError, GithubInvalidTokenError, + GithubRepoListFileNotFoundError, + GithubRepoListFileReadError, GithubSetUpIdentityError, GithubSetUpSessionError, ) @@ -90,6 +92,8 @@ class GithubProvider(Provider): _type: str = "github" _auth_method: str = None + MAX_REPO_LIST_LINES: int = 10_000 + MAX_REPO_NAME_LENGTH: int = 500 _session: GithubSession _identity: GithubIdentityInfo _audit_config: dict @@ -113,6 +117,7 @@ class GithubProvider(Provider): mutelist_path: str = None, mutelist_content: dict = None, repositories: list = None, + repo_list_file: str = None, organizations: list = None, ): """ @@ -130,6 +135,7 @@ class GithubProvider(Provider): mutelist_path (str): Path to the mutelist file. mutelist_content (dict): Mutelist content. repositories (list): List of repository names to scan in 'owner/repo-name' format. + repo_list_file (str): Path to a file containing repository names (one per line). organizations (list): List of organization or user names to scan repositories for. """ logger.info("Instantiating GitHub Provider...") @@ -147,6 +153,10 @@ class GithubProvider(Provider): else: self._repositories = list(repositories) + # Load repos from file if provided + if repo_list_file: + self._load_repos_from_file(repo_list_file) + if organizations is None: self._organizations = [] elif isinstance(organizations, str): @@ -256,6 +266,46 @@ class GithubProvider(Provider): """ return self._organizations + def _load_repos_from_file(self, file_path: str) -> None: + """Load repository names from a file (one per line).""" + try: + repo_count = 0 + before = len(self._repositories) + with open(file_path, "r") as f: + for line in f: + line = line.strip() + if not line or line.startswith("#"): + continue + repo_count += 1 + if repo_count > self.MAX_REPO_LIST_LINES: + raise GithubRepoListFileReadError( + file=file_path, + message=f"Repo list file exceeds maximum of {self.MAX_REPO_LIST_LINES} lines.", + ) + if len(line) > self.MAX_REPO_NAME_LENGTH: + logger.warning( + f"Skipping repo name exceeding {self.MAX_REPO_NAME_LENGTH} chars at line {repo_count} in {file_path}" + ) + continue + self._repositories.append(line) + self._repositories = list(dict.fromkeys(self._repositories)) + logger.info( + f"Loaded {len(self._repositories) - before} repositories from {file_path}" + ) + except FileNotFoundError: + raise GithubRepoListFileNotFoundError( + file=file_path, + message=f"Repo list file not found: {file_path}", + ) + except (GithubRepoListFileReadError, GithubRepoListFileNotFoundError): + raise + except Exception as error: + raise GithubRepoListFileReadError( + file=file_path, + original_exception=error, + message=f"Error reading repo list file: {error}", + ) + @staticmethod def setup_session( personal_access_token: str = None, diff --git a/prowler/providers/github/lib/arguments/arguments.py b/prowler/providers/github/lib/arguments/arguments.py index 748d77a927..946029ab43 100644 --- a/prowler/providers/github/lib/arguments/arguments.py +++ b/prowler/providers/github/lib/arguments/arguments.py @@ -50,6 +50,12 @@ def init_parser(self): default=None, metavar="REPOSITORY", ) + github_scoping_subparser.add_argument( + "--repo-list-file", + dest="repo_list_file", + default=None, + help="Path to a file containing a list of repositories to scan (one per line in 'owner/repo-name' format). Lines starting with # are treated as comments.", + ) github_scoping_subparser.add_argument( "--organization", "--organizations", diff --git a/tests/providers/aws/lib/cloudtrail_timeline/cloudtrail_timeline_test.py b/tests/providers/aws/lib/cloudtrail_timeline/cloudtrail_timeline_test.py index 1c1c10bfd3..aeca0f7c1d 100644 --- a/tests/providers/aws/lib/cloudtrail_timeline/cloudtrail_timeline_test.py +++ b/tests/providers/aws/lib/cloudtrail_timeline/cloudtrail_timeline_test.py @@ -120,7 +120,7 @@ class TestCloudTrailTimeline: assert result[0]["event_name"] == "RunInstances" def test_get_resource_timeline_prefers_uid_over_id(self, mock_session): - """When both resource_id and resource_uid are provided, UID should be used.""" + """When both resource_id and resource_uid are provided, UID is tried first.""" mock_client = MagicMock() mock_client.lookup_events.return_value = {"Events": []} mock_session.client.return_value = mock_client @@ -132,9 +132,9 @@ class TestCloudTrailTimeline: resource_uid="arn:aws:ec2:us-east-1:123:instance/i-1234", ) - # Verify UID was used in the lookup - call_args = mock_client.lookup_events.call_args - lookup_attrs = call_args.kwargs["LookupAttributes"] + # Verify UID was used on the first lookup call + first_call = mock_client.lookup_events.call_args_list[0] + lookup_attrs = first_call.kwargs["LookupAttributes"] assert ( lookup_attrs[0]["AttributeValue"] == "arn:aws:ec2:us-east-1:123:instance/i-1234" @@ -606,3 +606,159 @@ class TestIsReadOnlyEvent: """Verify write events are not marked as read-only.""" timeline = CloudTrailTimeline(session=mock_session) assert timeline._is_read_only_event(event_name) is False + + +class TestExtractShortName: + """Tests for _extract_short_name static method.""" + + @pytest.mark.parametrize( + "identifier,expected", + [ + ("arn:aws:s3:::my-bucket", "my-bucket"), + ("arn:aws:iam::123456789012:user/alice", "alice"), + ("arn:aws:iam::123456789012:role/MyRole", "MyRole"), + ( + "arn:aws:ec2:us-east-1:123456789012:instance/i-0abc1234", + "i-0abc1234", + ), + ( + "arn:aws:lambda:us-east-1:123456789012:function:my-func", + "my-func", + ), + ("arn:aws:rds:us-east-1:123456789012:db:mydb", "mydb"), + ("arn:aws:dynamodb:us-east-1:123456789012:table/MyTable", "MyTable"), + ( + "arn:aws:kms:us-east-1:123456789012:key/abcd-efgh", + "abcd-efgh", + ), + ("i-0abc1234", "i-0abc1234"), + ("my-bucket", "my-bucket"), + ("", ""), + ], + ) + def test_extract_short_name(self, identifier, expected): + assert CloudTrailTimeline._extract_short_name(identifier) == expected + + +class TestLookupEventsFallback: + """Tests for the ARN-to-short-name fallback in _lookup_events.""" + + @pytest.fixture + def mock_session(self): + return MagicMock() + + @pytest.fixture + def sample_event(self): + return { + "EventId": "a1b2c3d4-e5f6-7890-abcd-ef1234567890", + "EventTime": datetime(2024, 1, 15, 10, 30, 0, tzinfo=timezone.utc), + "EventName": "CreateBucket", + "EventSource": "s3.amazonaws.com", + "CloudTrailEvent": json.dumps( + { + "userIdentity": { + "type": "IAMUser", + "arn": "arn:aws:iam::123456789012:user/admin", + "userName": "admin", + } + } + ), + } + + def test_no_fallback_when_arn_returns_events(self, mock_session, sample_event): + """When the ARN lookup returns events, we do not retry with the short name.""" + mock_client = MagicMock() + mock_client.lookup_events.return_value = {"Events": [sample_event]} + mock_session.client.return_value = mock_client + + timeline = CloudTrailTimeline(session=mock_session) + result = timeline.get_resource_timeline( + region="us-east-1", + resource_uid="arn:aws:kms:us-east-1:123456789012:key/abcd-efgh", + ) + + assert len(result) == 1 + assert mock_client.lookup_events.call_count == 1 + call = mock_client.lookup_events.call_args + assert ( + call.kwargs["LookupAttributes"][0]["AttributeValue"] + == "arn:aws:kms:us-east-1:123456789012:key/abcd-efgh" + ) + + def test_fallback_to_short_name_when_arn_returns_empty( + self, mock_session, sample_event + ): + """When the ARN lookup returns nothing, we retry with the short name.""" + mock_client = MagicMock() + mock_client.lookup_events.side_effect = [ + {"Events": []}, + {"Events": [sample_event]}, + ] + mock_session.client.return_value = mock_client + + timeline = CloudTrailTimeline(session=mock_session) + result = timeline.get_resource_timeline( + region="us-east-1", resource_uid="arn:aws:s3:::my-bucket" + ) + + assert len(result) == 1 + assert mock_client.lookup_events.call_count == 2 + first_call, second_call = mock_client.lookup_events.call_args_list + assert ( + first_call.kwargs["LookupAttributes"][0]["AttributeValue"] + == "arn:aws:s3:::my-bucket" + ) + assert ( + second_call.kwargs["LookupAttributes"][0]["AttributeValue"] == "my-bucket" + ) + + def test_no_fallback_when_identifier_has_no_short_name(self, mock_session): + """A non-ARN identifier collapses to itself; no retry should fire.""" + mock_client = MagicMock() + mock_client.lookup_events.return_value = {"Events": []} + mock_session.client.return_value = mock_client + + timeline = CloudTrailTimeline(session=mock_session) + result = timeline.get_resource_timeline( + region="us-east-1", resource_id="i-0abc1234" + ) + + assert result == [] + assert mock_client.lookup_events.call_count == 1 + + def test_no_fallback_when_identifier_is_not_arn(self, mock_session): + """A non-ARN identifier with / or : must not trigger the retry.""" + mock_client = MagicMock() + mock_client.lookup_events.return_value = {"Events": []} + mock_session.client.return_value = mock_client + + timeline = CloudTrailTimeline(session=mock_session) + result = timeline.get_resource_timeline( + region="us-east-1", resource_id="some-prefix/weird:value" + ) + + assert result == [] + assert mock_client.lookup_events.call_count == 1 + + def test_both_lookups_empty_returns_empty_list(self, mock_session): + """If both the ARN and short-name lookups return empty, we return [].""" + mock_client = MagicMock() + mock_client.lookup_events.return_value = {"Events": []} + mock_session.client.return_value = mock_client + + timeline = CloudTrailTimeline(session=mock_session) + result = timeline.get_resource_timeline( + region="us-east-1", + resource_uid="arn:aws:ec2:us-east-1:123456789012:instance/i-0abc1234", + ) + + assert result == [] + assert mock_client.lookup_events.call_count == 2 + first_call, second_call = mock_client.lookup_events.call_args_list + assert ( + first_call.kwargs["LookupAttributes"][0]["AttributeValue"] + == "arn:aws:ec2:us-east-1:123456789012:instance/i-0abc1234" + ) + assert ( + second_call.kwargs["LookupAttributes"][0]["AttributeValue"] == "i-0abc1234" + ) diff --git a/tests/providers/github/github_provider_test.py b/tests/providers/github/github_provider_test.py index a87a35e038..00a3c33cbc 100644 --- a/tests/providers/github/github_provider_test.py +++ b/tests/providers/github/github_provider_test.py @@ -12,6 +12,8 @@ from prowler.providers.github.exceptions.exceptions import ( GithubInvalidCredentialsError, GithubInvalidProviderIdError, GithubInvalidTokenError, + GithubRepoListFileNotFoundError, + GithubRepoListFileReadError, GithubSetUpIdentityError, GithubSetUpSessionError, ) @@ -708,3 +710,81 @@ class Test_GithubProvider_Scoping: assert provider_none.repositories == [] assert provider_none.organizations == [] + + +class TestGitHubProviderLoadReposFromFile: + """Tests for GithubProvider._load_repos_from_file""" + + def _make_provider(self): + """Create a GithubProvider instance with mocked session/identity.""" + with ( + patch( + "prowler.providers.github.github_provider.GithubProvider.setup_session", + return_value=GithubSession(token=PAT_TOKEN, id="", key=""), + ), + patch( + "prowler.providers.github.github_provider.GithubProvider.setup_identity", + return_value=GithubIdentityInfo( + account_id=ACCOUNT_ID, + account_name=ACCOUNT_NAME, + account_url=ACCOUNT_URL, + ), + ), + ): + provider = GithubProvider( + personal_access_token=PAT_TOKEN, + ) + return provider + + def test_load_repos_from_file_happy_path(self, tmp_path): + provider = self._make_provider() + repo_file = tmp_path / "repos.txt" + repo_file.write_text("owner/repo-a\nowner/repo-b\nowner/repo-c\n") + + provider._load_repos_from_file(str(repo_file)) + + assert "owner/repo-a" in provider.repositories + assert "owner/repo-b" in provider.repositories + assert "owner/repo-c" in provider.repositories + + def test_load_repos_from_file_comments_and_blanks(self, tmp_path): + provider = self._make_provider() + repo_file = tmp_path / "repos.txt" + repo_file.write_text( + "# This is a comment\n" + "\n" + "owner/repo-a\n" + " # Another comment\n" + " \n" + "owner/repo-b\n" + ) + + provider._load_repos_from_file(str(repo_file)) + + assert provider.repositories == ["owner/repo-a", "owner/repo-b"] + + def test_load_repos_from_file_not_found(self): + provider = self._make_provider() + + with pytest.raises(GithubRepoListFileNotFoundError): + provider._load_repos_from_file("/nonexistent/path/repos.txt") + + def test_load_repos_from_file_exceeds_max_lines(self, tmp_path): + provider = self._make_provider() + repo_file = tmp_path / "repos.txt" + # Write MAX_REPO_LIST_LINES + 1 lines to trigger the guard + lines = [f"owner/repo-{i}" for i in range(provider.MAX_REPO_LIST_LINES + 1)] + repo_file.write_text("\n".join(lines) + "\n") + + with pytest.raises(GithubRepoListFileReadError): + provider._load_repos_from_file(str(repo_file)) + + def test_load_repos_from_file_skips_long_names(self, tmp_path): + provider = self._make_provider() + repo_file = tmp_path / "repos.txt" + long_name = "a" * (provider.MAX_REPO_NAME_LENGTH + 1) + repo_file.write_text(f"owner/valid-repo\n{long_name}\nowner/also-valid\n") + + provider._load_repos_from_file(str(repo_file)) + + assert provider.repositories == ["owner/valid-repo", "owner/also-valid"] diff --git a/tests/providers/github/lib/arguments/github_arguments_test.py b/tests/providers/github/lib/arguments/github_arguments_test.py index fd2f813b6e..20fd3f18be 100644 --- a/tests/providers/github/lib/arguments/github_arguments_test.py +++ b/tests/providers/github/lib/arguments/github_arguments_test.py @@ -82,13 +82,14 @@ class Test_GitHubArguments: arguments.init_parser(mock_github_args) # Verify scoping arguments were added - assert self.mock_scoping_group.add_argument.call_count == 2 + assert self.mock_scoping_group.add_argument.call_count == 3 # Check that all scoping arguments are present calls = self.mock_scoping_group.add_argument.call_args_list scoping_args = [call[0][0] for call in calls] assert "--repository" in scoping_args + assert "--repo-list-file" in scoping_args assert "--organization" in scoping_args def test_repository_argument_configuration(self): @@ -277,6 +278,33 @@ class Test_GitHubArguments_Integration: assert args.repository == ["owner1/repo1"] assert args.organization == ["org1"] + def test_real_argument_parsing_with_repo_list_file(self): + """Test parsing arguments with repo-list-file scoping""" + parser = argparse.ArgumentParser() + subparsers = parser.add_subparsers() + common_parser = argparse.ArgumentParser(add_help=False) + + mock_github_args = MagicMock() + mock_github_args.subparsers = subparsers + mock_github_args.common_providers_parser = common_parser + + arguments.init_parser(mock_github_args) + + # Parse arguments with repo-list-file + args = parser.parse_args( + [ + "github", + "--personal-access-token", + "test-token", + "--repo-list-file", + "/path/to/repos.txt", + ] + ) + + assert args.personal_access_token == "test-token" + assert args.repo_list_file == "/path/to/repos.txt" + assert args.repository is None + def test_real_argument_parsing_empty_scoping(self): """Test parsing arguments with empty scoping values""" parser = argparse.ArgumentParser() diff --git a/ui/CHANGELOG.md b/ui/CHANGELOG.md index d4f378adf8..5cd038666b 100644 --- a/ui/CHANGELOG.md +++ b/ui/CHANGELOG.md @@ -11,6 +11,14 @@ All notable changes to the **Prowler UI** are documented in this file. --- +## [1.24.2] (Prowler v5.24.2) + +### 🐞 Fixed + +- Default muted filter now applied consistently on the findings page and the finding-group resource drill-down, keeping muted findings hidden unless the "include muted findings" checkbox is opted in [(#10818)](https://github.com/prowler-cloud/prowler/pull/10818) + +--- + ## [1.24.1] (Prowler v5.24.1) ### 🐞 Fixed diff --git a/ui/app/(prowler)/findings/page.test.ts b/ui/app/(prowler)/findings/page.test.ts index 444ed47f9e..2038b92291 100644 --- a/ui/app/(prowler)/findings/page.test.ts +++ b/ui/app/(prowler)/findings/page.test.ts @@ -36,4 +36,8 @@ describe("findings page", () => { it("guards errors array access with a length check", () => { expect(source).toContain("errors?.length > 0"); }); + + it("applies the shared default muted filter so muted findings are hidden unless the caller opts in", () => { + expect(source).toContain("applyDefaultMutedFilter"); + }); }); diff --git a/ui/app/(prowler)/findings/page.tsx b/ui/app/(prowler)/findings/page.tsx index 22c90330bb..dea6e9b4fb 100644 --- a/ui/app/(prowler)/findings/page.tsx +++ b/ui/app/(prowler)/findings/page.tsx @@ -16,6 +16,7 @@ import { import { ContentLayout } from "@/components/ui"; import { FilterTransitionWrapper } from "@/contexts"; import { + applyDefaultMutedFilter, createScanDetailsMapping, extractFiltersAndQuery, extractSortAndKey, @@ -39,14 +40,16 @@ export default async function Findings({ getScans({ pageSize: 50 }), ]); - const filtersWithScanDates = await resolveFindingScanDateFilters({ - filters, - scans: scansData?.data || [], - loadScan: async (scanId: string) => { - const response = await getScan(scanId); - return response?.data; - }, - }); + const filtersWithScanDates = applyDefaultMutedFilter( + await resolveFindingScanDateFilters({ + filters, + scans: scansData?.data || [], + loadScan: async (scanId: string) => { + const response = await getScan(scanId); + return response?.data; + }, + }), + ); const hasHistoricalData = hasDateOrScanFilter(filtersWithScanDates); diff --git a/ui/hooks/use-finding-group-resource-state.test.ts b/ui/hooks/use-finding-group-resource-state.test.ts index 789ea592c2..c0ed5a4abe 100644 --- a/ui/hooks/use-finding-group-resource-state.test.ts +++ b/ui/hooks/use-finding-group-resource-state.test.ts @@ -12,4 +12,9 @@ describe("useFindingGroupResourceState", () => { it("enables muted findings only for the finding-group resource drawer", () => { expect(source).toContain("includeMutedInOtherFindings: true"); }); + + it("applies the shared default muted filter before fetching group resources", () => { + expect(source).toContain("applyDefaultMutedFilter(filters)"); + expect(source).toContain("filters: effectiveFilters"); + }); }); diff --git a/ui/hooks/use-finding-group-resource-state.ts b/ui/hooks/use-finding-group-resource-state.ts index 309a69a6dc..1249f8c5a8 100644 --- a/ui/hooks/use-finding-group-resource-state.ts +++ b/ui/hooks/use-finding-group-resource-state.ts @@ -6,6 +6,7 @@ import { useState } from "react"; import { canMuteFindingResource } from "@/components/findings/table/finding-resource-selection"; import { useResourceDetailDrawer } from "@/components/findings/table/resource-detail-drawer"; import { useFindingGroupResources } from "@/hooks/use-finding-group-resources"; +import { applyDefaultMutedFilter } from "@/lib"; import { FindingGroupRow, FindingResourceRow } from "@/types"; interface UseFindingGroupResourceStateOptions { @@ -67,11 +68,13 @@ export function useFindingGroupResourceState({ setIsLoading(loading); }; + const effectiveFilters = applyDefaultMutedFilter(filters); + const { sentinelRef, refresh, loadMore, totalCount } = useFindingGroupResources({ checkId: group.checkId, hasDateOrScanFilter: hasHistoricalData, - filters, + filters: effectiveFilters, onSetResources: handleSetResources, onAppendResources: handleAppendResources, onSetLoading: handleSetLoading, diff --git a/ui/lib/findings-filters.test.ts b/ui/lib/findings-filters.test.ts new file mode 100644 index 0000000000..3f1143b4d7 --- /dev/null +++ b/ui/lib/findings-filters.test.ts @@ -0,0 +1,42 @@ +import { describe, expect, it } from "vitest"; + +import { applyDefaultMutedFilter, MUTED_FILTER } from "./findings-filters"; + +describe("applyDefaultMutedFilter", () => { + it("injects filter[muted]=false when the caller has not set it", () => { + const input: Record = { "filter[status__in]": "FAIL" }; + const result = applyDefaultMutedFilter(input); + + expect(result["filter[muted]"]).toBe(MUTED_FILTER.EXCLUDE); + expect(result["filter[status__in]"]).toBe("FAIL"); + }); + + it("preserves an explicit filter[muted]=include opt-in from the checkbox", () => { + const result = applyDefaultMutedFilter({ + "filter[muted]": MUTED_FILTER.INCLUDE, + }); + + expect(result["filter[muted]"]).toBe(MUTED_FILTER.INCLUDE); + }); + + it("preserves an explicit filter[muted]=false (no silent overwrite)", () => { + const result = applyDefaultMutedFilter({ + "filter[muted]": MUTED_FILTER.EXCLUDE, + }); + + expect(result["filter[muted]"]).toBe(MUTED_FILTER.EXCLUDE); + }); + + it("does not mutate the input object", () => { + const input = { "filter[status__in]": "FAIL" }; + applyDefaultMutedFilter(input); + + expect(input).not.toHaveProperty("filter[muted]"); + }); + + it("returns a default-filled object when called with no caller filters", () => { + const result = applyDefaultMutedFilter({} as Record); + + expect(result["filter[muted]"]).toBe(MUTED_FILTER.EXCLUDE); + }); +}); diff --git a/ui/lib/findings-filters.ts b/ui/lib/findings-filters.ts new file mode 100644 index 0000000000..9a2aae1d80 --- /dev/null +++ b/ui/lib/findings-filters.ts @@ -0,0 +1,36 @@ +/** + * Shared helpers for findings filter handling. + * + * The `/findings` SSR page and the finding-group resource drill-down both + * need to hide muted findings by default β€” unless the user has opted in via + * the "include muted findings" checkbox. Keeping that default in one place + * prevents surfaces from drifting. + */ + +export const MUTED_FILTER = { + /** Wire value sent to the API to exclude muted findings. */ + EXCLUDE: "false", + /** + * Sentinel value that tells the API to return both muted and non-muted + * findings. The checkbox writes this to the URL when the user opts in. + */ + INCLUDE: "include", +} as const; + +export type MutedFilterValue = (typeof MUTED_FILTER)[keyof typeof MUTED_FILTER]; + +/** + * Returns a new filter object with the default muted behaviour applied: + * hide muted findings unless the caller already set `filter[muted]`. + * + * The default is spread BEFORE the caller filters so any explicit value + * (including `"false"` or the `"include"` opt-in) wins. + */ +export function applyDefaultMutedFilter< + T extends Record, +>(filters: T): T { + return { + "filter[muted]": MUTED_FILTER.EXCLUDE, + ...filters, + }; +} diff --git a/ui/lib/index.ts b/ui/lib/index.ts index 316348a0d6..75250e5c68 100644 --- a/ui/lib/index.ts +++ b/ui/lib/index.ts @@ -1,5 +1,6 @@ export * from "./error-mappings"; export * from "./external-urls"; +export * from "./findings-filters"; export * from "./helper"; export * from "./helper-filters"; export * from "./menu-list";