feat(sdk): replace detect-secrets library with kingfisher#11694
Open
danibarranqueroo wants to merge 22 commits into
Open
feat(sdk): replace detect-secrets library with kingfisher#11694danibarranqueroo wants to merge 22 commits into
danibarranqueroo wants to merge 22 commits into
Conversation
github-actions
Bot
added
documentation
provider/aws
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughSecret scanning now uses Kingfisher, stays offline by default, and can optionally validate discovered secrets through new config and CLI wiring. Provider checks pass the validation flag into scans and annotate verified findings. Documentation, changelog, dependencies, and tests were updated. ChangesSecret scanning validation and Kingfisher migration
Estimated code review effort🎯 5 (Critical) | ⏱️ ~90+ minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
|
✅ Conflict Markers Resolved All conflict markers have been successfully resolved in this pull request. |
danibarranqueroo
changed the title
Contributor
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
Contributor
|
✅ All necessary |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Contributor
🔒 Container Security ScanImage: ✅ No Vulnerabilities DetectedThe container image passed all security checks. No known CVEs were found.📋 Resources:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/user-guide/cli/tutorials/configuration_file.mdx`:
- Around line 90-95: The section in the configuration_file.mdx tutorial has the
VersionBadge separated from the “### Validating Discovered Secrets” heading by
an import statement, so update the MDX so the badge appears immediately under
the header and the import is moved to the top-level file imports. Keep the
VersionBadge usage directly after the heading on its own line, followed by a
blank line before any other content, and use the existing VersionBadge symbol to
locate the section.
In `@docs/user-guide/cli/tutorials/pentesting.mdx`:
- Around line 7-9: The “Detect Secrets” section introduces new secret-scanning
behavior, so add the Version Badge component directly under the heading in the
pentesting tutorial. Update the relevant markdown in the Detect Secrets section
to include the badge using the documented Version Badge pattern, keeping the
rest of the Kingfisher and live-validation description unchanged.
In `@prowler/CHANGELOG.md`:
- Line 11: The release note in CHANGELOG.md refers to the new setting
generically, but the actual config key added under prowler/config/config.yaml is
aws.secrets_validate. Update the note tied to the secrets validation feature so
it uses the fully qualified key name, and keep the mention aligned with the
--scan-secrets-validate flag and the related secrets_validate option wording.
In `@prowler/config/config.yaml`:
- Around line 426-431: The secrets config block still exposes
detect_secrets_plugins even though the Kingfisher scan path ignores it, which
can make tuned configurations silently stop working after upgrade. Update the
config handling around the secrets validation settings to either
remove/deprecate detect_secrets_plugins from this block or explicitly map it to
the Kingfisher equivalent, and make sure the config schema/comments around
secrets_validate clearly reflect the supported behavior.
In `@prowler/lib/cli/parser.py`:
- Around line 476-487: The CLI option in parser.py currently only supports
enabling live validation via the existing scan-secrets-validate flag, so it
cannot explicitly force the feature off when config enables
aws.secrets_validate. Update the argument handling around the
config_parser.add_argument setup for scan-secrets-validate to support an
explicit opt-out path (for example via a paired disable flag or equivalent
tri-state parsing), and make sure the parsed value can override provider
plumbing in the secrets_validate flow back to offline mode when requested.
In `@prowler/lib/utils/utils.py`:
- Around line 182-245: The Kingfisher invocation in scan_for_secrets only sets
provider validation limits, but subprocess.run still has no real process
timeout, so a hung binary can block indefinitely. Update the subprocess.run call
to use an actual timeout and handle the timeout exception explicitly in the same
scan_for_secrets flow, alongside the existing returncode and generic exception
handling, so the caller gets a clean failure instead of a hang.
In
`@prowler/providers/aws/services/awslambda/awslambda_function_no_secrets_in_code/awslambda_function_no_secrets_in_code.py`:
- Around line 29-34: The Lambda secrets scan only uses
next(os.walk(tmp_dir_name))[2], so nested files inside directories like src/,
package/, or vendored modules are skipped. Update
awslambda_function_no_secrets_in_code’s zip extraction and detect_secrets_scan
loop to traverse all files recursively under tmp_dir_name (not just the top
level), and ensure every discovered file is passed through the existing secrets
collection logic so the final finding cannot incorrectly stay PASS.
In
`@tests/providers/aws/services/autoscaling/autoscaling_find_secrets_ec2_launch_configuration/autoscaling_find_secrets_ec2_launch_configuration_test.py`:
- Line 107: The test fixture includes an inline secret-like value in the
Autoscaling launch configuration data, which will continue to trigger the secret
scanner. Update the autoscaling_find_secrets_ec2_launch_configuration_test
fixture to avoid embedding the sensitive-looking string directly in UserData by
using the project’s standard suppression mechanism or by mocking the scanner
result in the relevant test setup, so the
autoscaling_find_secrets_ec2_launch_configuration path remains covered without a
false-positive leak.
In
`@tests/providers/aws/services/awslambda/awslambda_function_no_secrets_in_variables/awslambda_function_no_secrets_in_variables_test.py`:
- Line 129: Add a mocked `secrets_validate=True` test case in the AWS Lambda
no-secrets-in-variables suite to cover the verified-secret path now exercised by
`annotate_verified_secrets(...)`. Update the relevant test(s) around
`test_awslambda_function_no_secrets_in_variables` to assert the critical
escalation and the `"confirmed to be live"` suffix for a verified finding,
alongside the existing offline Kingfisher label checks, so the new forwarding of
`secrets_validate` is locked in.
In
`@tests/providers/aws/services/codebuild/codebuild_project_no_secrets_in_variables/codebuild_project_no_secrets_in_variables_test.py`:
- Around line 238-249: Add a `secrets_validate=True` test in the CodeBuild
project no-secrets-in-variables suite that includes at least one verified
finding so the `all_secrets -> annotate_verified_secrets(...)` path is
exercised. Reuse the existing `result` assertions pattern in
`codebuild_project_no_secrets_in_variables_test.py`, and verify the aggregated
report includes the critical escalation plus the appended live-secret note for
verified secrets. Make sure the new case covers the branch alongside the
existing `SensitiveProject`-style checks so the verified-secret behavior is
protected in `codebuild_project_no_secrets_in_variables`.
In
`@tests/providers/aws/services/ec2/ec2_instance_secrets_user_data/ec2_instance_secrets_user_data_test.py`:
- Around line 358-390: The verified-secret test does not assert that the
`validate=True` path is actually exercised, so it can pass even if
`ec2_instance_secrets_user_data()` stops forwarding the secrets validation flag
to `detect_secrets_scan`. Update the test around
`ec2_instance_secrets_user_data` to enable
`ec2_client.audit_config["secrets_validate"]` and make the mock for
`detect_secrets_scan` assert it was called with `validate=True`, so the check’s
config plumbing is covered.
In
`@tests/providers/aws/services/ec2/ec2_launch_template_no_secrets/ec2_launch_template_no_secrets_test.py`:
- Line 169: Add a verified-secret test case for the new aggregated secret path
in ec2_launch_template_no_secrets_test by exercising
ec2_launch_template_no_secrets.execute() with multiple collected all_secrets
entries and asserting the annotate_verified_secrets(report, all_secrets)
behavior, including the critical escalation and appended confirmation text.
Update the existing test coverage around the EC2 Launch Template scenarios so it
verifies both the offline message and the verified-secret output for the
relevant test helpers and fixtures.
In
`@tests/providers/aws/services/ssm/ssm_document_secrets/ssm_document_secrets_test.py`:
- Line 36: The fixture in ssm_document_secrets_test.py is triggering the secret
scanner because the db_password value is hardcoded inline. Move the synthetic
secret out of the fixture or suppress/mock the scanner using the project’s
standard approach so the test still exercises the same SSM document behavior
without an inline secret literal. Keep the change localized to the test data
around the content fixture in the ssm_document_secrets test.
In
`@tests/providers/openstack/services/blockstorage/blockstorage_snapshot_metadata_sensitive_data/blockstorage_snapshot_metadata_sensitive_data_test.py`:
- Line 144: Add a regression test case for the verified-secret path in the
blockstorage snapshot metadata sensitive-data suite: the current cases only
cover the generic FAIL path, so extend the relevant test(s) around the
blockstorage snapshot metadata scenarios to use
audit_config={"secrets_validate": True} with a stubbed verified finding. Make
the test assert that secrets_validate reaches detect_secrets_scan(), and that
annotate_verified_secrets() appends the verified-secret note and escalates
severity as expected; use the existing test helpers and fixtures in this module
to locate and update the cases currently using the db_password metadata payload.
In
`@tests/providers/openstack/services/blockstorage/blockstorage_volume_metadata_sensitive_data/blockstorage_volume_metadata_sensitive_data_test.py`:
- Line 162: Add a test case in blockstorage_volume_metadata_sensitive_data_test
that sets secrets_validate to true and returns a finding with is_verified=True
so the verified-secret branch is exercised. Make sure the assertions cover the
verified path’s behavior in the relevant test helpers around the blockstorage
volume metadata checks, especially that status_extended and severity are updated
correctly when validate is enabled. Use the existing sensitive data detection
flow in the blockstorage_volume_metadata_sensitive_data suite to locate the
right test method(s) and extend them with this verified-finding scenario.
In
`@tests/providers/openstack/services/compute/compute_instance_metadata_sensitive_data/compute_instance_metadata_sensitive_data_test.py`:
- Line 184: Add a targeted test in the compute_instance_metadata_sensitive_data
suite that stubs a verified scan result and exercises the secrets_validate flow.
Update the existing fixture-based cases around the compute instance metadata
sensitive data tests to assert that a verified finding appends the live-secret
note and escalates the report as expected, using the relevant test helpers and
scan-result setup already present in this module.
In
`@tests/providers/openstack/services/objectstorage/objectstorage_container_metadata_sensitive_data/objectstorage_container_metadata_sensitive_data_test.py`:
- Line 160: The object-storage metadata sensitive-data test only exercises the
generic FAIL path and misses the verified-secret branch. Update the relevant
test case(s) in objectstorage_container_metadata_sensitive_data_test around the
existing metadata/password fixtures to mock a finding with is_verified=True,
then assert the scanner emits the additional verified-status text and the
expected severity change. Use the existing test helpers and assertion patterns
in the object storage sensitive-data suite to keep the new case aligned with the
current flow.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 61c27edf-48e5-4f4b-aa5f-31cfe0cf57c1
⛔ Files ignored due to path filters (4)
tests/providers/aws/services/autoscaling/autoscaling_find_secrets_ec2_launch_configuration/fixtures/fixture.gzis excluded by!**/*.gztests/providers/aws/services/ec2/ec2_instance_secrets_user_data/fixtures/fixture.gzis excluded by!**/*.gztests/providers/aws/services/ec2/ec2_launch_template_no_secrets/fixtures/fixture.gzis excluded by!**/*.gzuv.lockis excluded by!**/*.lock,!**/uv.lock
📒 Files selected for processing (43)
docs/user-guide/cli/tutorials/configuration_file.mdxdocs/user-guide/cli/tutorials/pentesting.mdxprowler/CHANGELOG.mdprowler/config/config.yamlprowler/lib/cli/parser.pyprowler/lib/utils/utils.pyprowler/providers/aws/services/autoscaling/autoscaling_find_secrets_ec2_launch_configuration/autoscaling_find_secrets_ec2_launch_configuration.pyprowler/providers/aws/services/awslambda/awslambda_function_no_secrets_in_code/awslambda_function_no_secrets_in_code.pyprowler/providers/aws/services/awslambda/awslambda_function_no_secrets_in_variables/awslambda_function_no_secrets_in_variables.pyprowler/providers/aws/services/cloudformation/cloudformation_stack_outputs_find_secrets/cloudformation_stack_outputs_find_secrets.pyprowler/providers/aws/services/cloudwatch/cloudwatch_log_group_no_secrets_in_logs/cloudwatch_log_group_no_secrets_in_logs.pyprowler/providers/aws/services/codebuild/codebuild_project_no_secrets_in_variables/codebuild_project_no_secrets_in_variables.pyprowler/providers/aws/services/ec2/ec2_instance_secrets_user_data/ec2_instance_secrets_user_data.pyprowler/providers/aws/services/ec2/ec2_launch_template_no_secrets/ec2_launch_template_no_secrets.pyprowler/providers/aws/services/ecs/ecs_task_definitions_no_environment_secrets/ecs_task_definitions_no_environment_secrets.pyprowler/providers/aws/services/glue/glue_etl_jobs_no_secrets_in_arguments/glue_etl_jobs_no_secrets_in_arguments.pyprowler/providers/aws/services/ssm/ssm_document_secrets/ssm_document_secrets.pyprowler/providers/aws/services/stepfunctions/stepfunctions_statemachine_no_secrets_in_definition/stepfunctions_statemachine_no_secrets_in_definition.pyprowler/providers/common/models.pyprowler/providers/openstack/services/blockstorage/blockstorage_snapshot_metadata_sensitive_data/blockstorage_snapshot_metadata_sensitive_data.pyprowler/providers/openstack/services/blockstorage/blockstorage_volume_metadata_sensitive_data/blockstorage_volume_metadata_sensitive_data.pyprowler/providers/openstack/services/compute/compute_instance_metadata_sensitive_data/compute_instance_metadata_sensitive_data.pyprowler/providers/openstack/services/objectstorage/objectstorage_container_metadata_sensitive_data/objectstorage_container_metadata_sensitive_data.pypyproject.tomltests/lib/utils/utils_test.pytests/providers/aws/services/autoscaling/autoscaling_find_secrets_ec2_launch_configuration/autoscaling_find_secrets_ec2_launch_configuration_test.pytests/providers/aws/services/autoscaling/autoscaling_find_secrets_ec2_launch_configuration/fixtures/fixturetests/providers/aws/services/awslambda/awslambda_function_no_secrets_in_code/awslambda_function_no_secrets_in_code_test.pytests/providers/aws/services/awslambda/awslambda_function_no_secrets_in_variables/awslambda_function_no_secrets_in_variables_test.pytests/providers/aws/services/cloudformation/cloudformation_stack_outputs_find_secrets/cloudformation_stack_outputs_find_secrets_test.pytests/providers/aws/services/cloudwatch/cloudwatch_log_group_no_secrets_in_logs/cloudwatch_log_group_no_secrets_in_logs_test.pytests/providers/aws/services/codebuild/codebuild_project_no_secrets_in_variables/codebuild_project_no_secrets_in_variables_test.pytests/providers/aws/services/ec2/ec2_instance_secrets_user_data/ec2_instance_secrets_user_data_test.pytests/providers/aws/services/ec2/ec2_instance_secrets_user_data/fixtures/fixturetests/providers/aws/services/ec2/ec2_launch_template_no_secrets/ec2_launch_template_no_secrets_test.pytests/providers/aws/services/ec2/ec2_launch_template_no_secrets/fixtures/fixturetests/providers/aws/services/ecs/ecs_task_definitions_no_environment_secrets/ecs_task_definitions_no_environment_secrets_test.pytests/providers/aws/services/ssm/ssm_document_secrets/ssm_document_secrets_test.pytests/providers/aws/services/stepfunctions/stepfunctions_statemachine_no_secrets_in_definition/stepfunctions_statemachine_no_secrets_in_definition_test.pytests/providers/openstack/services/blockstorage/blockstorage_snapshot_metadata_sensitive_data/blockstorage_snapshot_metadata_sensitive_data_test.pytests/providers/openstack/services/blockstorage/blockstorage_volume_metadata_sensitive_data/blockstorage_volume_metadata_sensitive_data_test.pytests/providers/openstack/services/compute/compute_instance_metadata_sensitive_data/compute_instance_metadata_sensitive_data_test.pytests/providers/openstack/services/objectstorage/objectstorage_container_metadata_sensitive_data/objectstorage_container_metadata_sensitive_data_test.py
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #11694 +/- ##
===========================================
- Coverage 93.72% 68.29% -25.44%
===========================================
Files 258 1977 +1719
Lines 37801 66663 +28862
===========================================
+ Hits 35430 45527 +10097
- Misses 2371 21136 +18765 Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
docs/user-guide/cli/tutorials/pentesting.mdx (1)
7-10: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd the Version Badge under
## Detect Secrets.This section introduces new secret-scanning behavior, so it still needs the Version Badge directly below the heading. As per coding guidelines, “Use the Version Badge component to indicate when a feature or functionality was introduced in Prowler.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/user-guide/cli/tutorials/pentesting.mdx` around lines 7 - 10, The Detect Secrets section is missing the Version Badge that should appear directly under the heading to mark when this feature was introduced. Update the markdown in the Detect Secrets tutorial section to place the Version Badge component immediately below the `## Detect Secrets` heading, matching the existing documentation pattern used for newly introduced CLI features.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@prowler/providers/openstack/services/blockstorage/blockstorage_snapshot_metadata_sensitive_data/blockstorage_snapshot_metadata_sensitive_data.metadata.json`:
- Line 39: The Notes text in
blockstorage_snapshot_metadata_sensitive_data.metadata.json still references the
old detect-secrets scanner and should be updated to mention Kingfisher instead.
Edit the metadata entry so the check description reflects the current scanner
while keeping the rest of the warning about false positives and
secrets_ignore_patterns intact.
In
`@prowler/providers/openstack/services/blockstorage/blockstorage_volume_metadata_sensitive_data/blockstorage_volume_metadata_sensitive_data.metadata.json`:
- Line 37: The metadata note for the blockstorage volume metadata sensitive data
check still references the old detect-secrets scanner and should be updated to
mention Kingfisher instead. Edit the Notes field in
blockstorage_volume_metadata_sensitive_data.metadata.json so it reflects the
current scanner terminology while keeping the existing guidance about false
positives and secrets_ignore_patterns, using the
blockstorage_volume_metadata_sensitive_data check metadata as the location cue.
In
`@prowler/providers/openstack/services/compute/compute_instance_metadata_sensitive_data/compute_instance_metadata_sensitive_data.metadata.json`:
- Line 37: The check metadata note still references the old detect-secrets
scanner and needs to be updated to reflect Kingfisher instead. Edit the Notes
text in compute_instance_metadata_sensitive_data.metadata.json so it mentions
Kingfisher as the scanner, while keeping the false-positive guidance,
secrets_ignore_patterns reference, and metadata readability context intact.
In
`@prowler/providers/openstack/services/objectstorage/objectstorage_container_metadata_sensitive_data/objectstorage_container_metadata_sensitive_data.metadata.json`:
- Line 38: Update the Notes metadata for the object storage container metadata
sensitive data check so it no longer mentions detect-secrets and instead
references Kingfisher. Adjust the existing Notes entry in
objectstorage_container_metadata_sensitive_data.metadata.json to keep the
scanner description aligned with the migrated implementation, while preserving
the rest of the guidance about false positives and secrets_ignore_patterns.
---
Duplicate comments:
In `@docs/user-guide/cli/tutorials/pentesting.mdx`:
- Around line 7-10: The Detect Secrets section is missing the Version Badge that
should appear directly under the heading to mark when this feature was
introduced. Update the markdown in the Detect Secrets tutorial section to place
the Version Badge component immediately below the `## Detect Secrets` heading,
matching the existing documentation pattern used for newly introduced CLI
features.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 664661ee-1163-47f4-8fd7-f01c9b2fa3a9
📒 Files selected for processing (30)
docs/developer-guide/configurable-checks.mdxdocs/user-guide/cli/tutorials/configuration_file.mdxdocs/user-guide/cli/tutorials/pentesting.mdxprowler/CHANGELOG.mdprowler/config/config.yamlprowler/config/schema/aws.pyprowler/providers/aws/services/autoscaling/autoscaling_find_secrets_ec2_launch_configuration/autoscaling_find_secrets_ec2_launch_configuration.pyprowler/providers/aws/services/awslambda/awslambda_function_no_secrets_in_code/awslambda_function_no_secrets_in_code.pyprowler/providers/aws/services/awslambda/awslambda_function_no_secrets_in_variables/awslambda_function_no_secrets_in_variables.pyprowler/providers/aws/services/cloudformation/cloudformation_stack_outputs_find_secrets/cloudformation_stack_outputs_find_secrets.pyprowler/providers/aws/services/cloudwatch/cloudwatch_log_group_no_secrets_in_logs/cloudwatch_log_group_no_secrets_in_logs.pyprowler/providers/aws/services/codebuild/codebuild_project_no_secrets_in_variables/codebuild_project_no_secrets_in_variables.pyprowler/providers/aws/services/ec2/ec2_instance_secrets_user_data/ec2_instance_secrets_user_data.pyprowler/providers/aws/services/ec2/ec2_launch_template_no_secrets/ec2_launch_template_no_secrets.pyprowler/providers/aws/services/ecs/ecs_task_definitions_no_environment_secrets/ecs_task_definitions_no_environment_secrets.pyprowler/providers/aws/services/glue/glue_etl_jobs_no_secrets_in_arguments/glue_etl_jobs_no_secrets_in_arguments.pyprowler/providers/aws/services/ssm/ssm_document_secrets/ssm_document_secrets.pyprowler/providers/aws/services/stepfunctions/stepfunctions_statemachine_no_secrets_in_definition/stepfunctions_statemachine_no_secrets_in_definition.pyprowler/providers/openstack/services/blockstorage/blockstorage_snapshot_metadata_sensitive_data/blockstorage_snapshot_metadata_sensitive_data.metadata.jsonprowler/providers/openstack/services/blockstorage/blockstorage_snapshot_metadata_sensitive_data/blockstorage_snapshot_metadata_sensitive_data.pyprowler/providers/openstack/services/blockstorage/blockstorage_volume_metadata_sensitive_data/blockstorage_volume_metadata_sensitive_data.metadata.jsonprowler/providers/openstack/services/blockstorage/blockstorage_volume_metadata_sensitive_data/blockstorage_volume_metadata_sensitive_data.pyprowler/providers/openstack/services/compute/compute_instance_metadata_sensitive_data/compute_instance_metadata_sensitive_data.metadata.jsonprowler/providers/openstack/services/compute/compute_instance_metadata_sensitive_data/compute_instance_metadata_sensitive_data.pyprowler/providers/openstack/services/objectstorage/objectstorage_container_metadata_sensitive_data/objectstorage_container_metadata_sensitive_data.metadata.jsonprowler/providers/openstack/services/objectstorage/objectstorage_container_metadata_sensitive_data/objectstorage_container_metadata_sensitive_data.pytests/config/schema/aws_schema_test.pytests/config/schema/bounds_test.pytests/providers/aws/services/ec2/ec2_launch_template_no_secrets/ec2_launch_template_no_secrets_test.pytests/providers/aws/services/ssm/ssm_document_secrets/ssm_document_secrets_test.py
💤 Files with no reviewable changes (20)
- prowler/providers/aws/services/ssm/ssm_document_secrets/ssm_document_secrets.py
- prowler/providers/openstack/services/objectstorage/objectstorage_container_metadata_sensitive_data/objectstorage_container_metadata_sensitive_data.py
- prowler/providers/aws/services/cloudformation/cloudformation_stack_outputs_find_secrets/cloudformation_stack_outputs_find_secrets.py
- prowler/providers/aws/services/ec2/ec2_instance_secrets_user_data/ec2_instance_secrets_user_data.py
- prowler/providers/aws/services/autoscaling/autoscaling_find_secrets_ec2_launch_configuration/autoscaling_find_secrets_ec2_launch_configuration.py
- prowler/providers/aws/services/glue/glue_etl_jobs_no_secrets_in_arguments/glue_etl_jobs_no_secrets_in_arguments.py
- prowler/providers/openstack/services/blockstorage/blockstorage_volume_metadata_sensitive_data/blockstorage_volume_metadata_sensitive_data.py
- docs/developer-guide/configurable-checks.mdx
- prowler/providers/openstack/services/blockstorage/blockstorage_snapshot_metadata_sensitive_data/blockstorage_snapshot_metadata_sensitive_data.py
- prowler/providers/aws/services/stepfunctions/stepfunctions_statemachine_no_secrets_in_definition/stepfunctions_statemachine_no_secrets_in_definition.py
- prowler/providers/openstack/services/compute/compute_instance_metadata_sensitive_data/compute_instance_metadata_sensitive_data.py
- prowler/providers/aws/services/ecs/ecs_task_definitions_no_environment_secrets/ecs_task_definitions_no_environment_secrets.py
- prowler/providers/aws/services/codebuild/codebuild_project_no_secrets_in_variables/codebuild_project_no_secrets_in_variables.py
- prowler/providers/aws/services/awslambda/awslambda_function_no_secrets_in_variables/awslambda_function_no_secrets_in_variables.py
- tests/config/schema/bounds_test.py
- prowler/config/config.yaml
- prowler/providers/aws/services/ec2/ec2_launch_template_no_secrets/ec2_launch_template_no_secrets.py
- prowler/config/schema/aws.py
- tests/config/schema/aws_schema_test.py
- prowler/providers/aws/services/awslambda/awslambda_function_no_secrets_in_code/awslambda_function_no_secrets_in_code.py
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
prowler/providers/openstack/services/blockstorage/blockstorage_snapshot_metadata_sensitive_data/blockstorage_snapshot_metadata_sensitive_data.py (1)
22-58: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftExtract the Shared OpenStack Metadata-Scan Flow.
This batch-scan / key-mapping / report-assembly block is now copied across the snapshot, volume, instance, and container checks. It has already drifted (
compute_instance_metadata_sensitive_data.pyuses a different bounds guard), so the next detector or message change will be easy to apply inconsistently. A shared helper would remove that divergence.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@prowler/providers/openstack/services/blockstorage/blockstorage_snapshot_metadata_sensitive_data/blockstorage_snapshot_metadata_sensitive_data.py` around lines 22 - 58, The metadata batch-scan, key-mapping, and report assembly logic in blockstorage_snapshot_metadata_sensitive_data is duplicated across the OpenStack metadata-sensitive checks and has already started to diverge. Extract this flow into a shared helper used by the snapshot, volume, instance, and container detectors, and route the existing payloads, detect_secrets_scan_batch, and CheckReportOpenStack handling through that common helper so future detector or message changes stay consistent.prowler/providers/openstack/services/compute/compute_instance_metadata_sensitive_data/compute_instance_metadata_sensitive_data.py (1)
39-54: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winGuard the Lower Bound When Mapping Findings Back to Metadata Keys.
This mapper only enforces
< len(original_metadata_keys). Ifline_numberis1or0, Python will index from the end of the list and attribute the secret to the wrong key instead of skipping it. The sibling OpenStack metadata checks already use0 <= ... < len(...).Suggested Fix
- if secret["line_number"] - 2 < len(original_metadata_keys) + if 0 + <= secret["line_number"] - 2 + < len(original_metadata_keys)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@prowler/providers/openstack/services/compute/compute_instance_metadata_sensitive_data/compute_instance_metadata_sensitive_data.py` around lines 39 - 54, The metadata-to-key mapping in compute_instance_metadata_sensitive_data currently only checks the upper bound when using secret["line_number"] - 2, so line numbers 0 or 1 can incorrectly index from the end of original_metadata_keys. Update the list comprehension in the instance.metadata handling block to guard both bounds before indexing, matching the pattern used by the sibling OpenStack metadata checks, and keep the mapping logic in the same secrets_string/report.status_extended flow.
♻️ Duplicate comments (1)
prowler/lib/utils/utils.py (1)
190-195: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winAdd a real subprocess timeout here.
--validation-timeoutonly bounds Kingfisher's provider-side probes. If thekingfisher scanprocess itself hangs, this shared helper can block every secret-scanning check indefinitely.Suggested fix
- process = subprocess.run(command, capture_output=True, text=True) + process = subprocess.run( + command, + capture_output=True, + text=True, + timeout=60, + ) @@ - except Exception as e: + except subprocess.TimeoutExpired as error: + logger.error(f"Error scanning for secrets: Kingfisher timed out: {error}") + except Exception as e: logger.error(f"Error scanning for secrets: {e}")Also applies to: 230-231
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@prowler/lib/utils/utils.py` around lines 190 - 195, The Kingfisher subprocess call in the shared helper is missing a real process-level timeout, so a hung scan can block secret scanning indefinitely. Update the subprocess.run invocation in the helper that builds and executes _build_kingfisher_command to pass a timeout value, and handle the timeout case by terminating the process and surfacing a clear failure. Apply the same timeout behavior to the other kingfisher scan call site noted in the comment so both paths are protected.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/developer-guide/secret-scanning-checks.mdx`:
- Around line 1-7: The page title in this MDX doc needs a release marker because
it introduces newly added secret-scanning behavior. Update the content
immediately below the title in the frontmatter/title area to include the
appropriate VersionBadge component, using the same pattern as other docs that
document feature introductions, so readers can see when this functionality was
added.
- Around line 88-92: The documentation in the secret-scanning checks guide
incorrectly says both helpers accept validate; update the wording around
detect_secrets_scan_batch and annotate_verified_secrets so only
detect_secrets_scan_batch is described as accepting validate, while
annotate_verified_secrets is described as consuming findings and escalating
verified secrets. Keep the rest of the validation/severity flow intact, and
adjust the surrounding text to match the actual API behavior in the secret
scanning section.
In `@prowler/lib/utils/utils.py`:
- Around line 238-244: The `detect_secrets_scan_batch` signature has incomplete
typing: `payloads` is unannotated even though it accepts either a mapping or an
iterable of key-data pairs, and `excluded_secrets` is nullable but not marked as
such. Update the function signature to annotate `payloads` with the appropriate
union type using the existing `detect_secrets_scan_batch` symbol, change
`excluded_secrets` to `Optional[list[str]]`, and import any missing typing
symbols needed for the new annotations.
In
`@prowler/providers/aws/services/autoscaling/autoscaling_find_secrets_ec2_launch_configuration/autoscaling_find_secrets_ec2_launch_configuration.py`:
- Around line 47-54: The `execute()` flow in
`autoscaling_find_secrets_ec2_launch_configuration` is dropping launch
configurations when user data decoding fails by adding them to `skipped` and
then filtering them out of `findings`, which violates the
one-report-per-resource requirement. Update the handling around the user-data
decode exception and the `findings` construction so every iterated launch
configuration still produces exactly one result with a PASS/FAIL status, and use
the existing `logger.error` path plus a clear failure reason instead of omitting
the resource entirely.
In
`@prowler/providers/aws/services/cloudwatch/cloudwatch_log_group_no_secrets_in_logs/cloudwatch_log_group_no_secrets_in_logs.py`:
- Around line 88-90: Phase 3 rescans in
cloudwatch_log_group_no_secrets_in_logs.py are skipping the existing
secrets_ignore_patterns filter, which can reintroduce ignored secrets into
multiline event results. Update the Phase 3 batch scan path around
detect_secrets_scan_batch and rescan_results so it applies the same
ignore-pattern exclusion used in Phase 1 before generating the final report
details. Keep the filtering logic consistent across both scan phases by reusing
the existing ignore-pattern handling in this check.
In
`@prowler/providers/aws/services/ec2/ec2_instance_secrets_user_data/ec2_instance_secrets_user_data.py`:
- Around line 41-52: The decode-failure path in
ec2_instance_secrets_user_data.execute() is skipping instances instead of
producing a report, which breaks the required one PASS/FAIL finding per active
resource. Update the handling around the UnicodeDecodeError and the skipped set
so that unreadable user data still yields a FAIL report for the current
instance, and adjust the logic at the report suppression point to ensure every
iterated EC2 instance always generates exactly one finding with an appropriate
status and message.
In
`@prowler/providers/aws/services/ec2/ec2_launch_template_no_secrets/ec2_launch_template_no_secrets.py`:
- Around line 39-48: Track EC2 Launch Template versions whose User Data could
not be decoded or decompressed inside payloads() rather than silently
continuing, so the final report in ec2_launch_template_no_secrets does not emit
PASS when some versions were never scanned. Update the finding/report generation
logic to distinguish fully scanned templates from those with skipped versions,
and use the existing template/version handling in payloads() and the report
builder to return a clear FAIL or other non-PASS status_extended when any
unscanned User Data remains.
---
Outside diff comments:
In
`@prowler/providers/openstack/services/blockstorage/blockstorage_snapshot_metadata_sensitive_data/blockstorage_snapshot_metadata_sensitive_data.py`:
- Around line 22-58: The metadata batch-scan, key-mapping, and report assembly
logic in blockstorage_snapshot_metadata_sensitive_data is duplicated across the
OpenStack metadata-sensitive checks and has already started to diverge. Extract
this flow into a shared helper used by the snapshot, volume, instance, and
container detectors, and route the existing payloads, detect_secrets_scan_batch,
and CheckReportOpenStack handling through that common helper so future detector
or message changes stay consistent.
In
`@prowler/providers/openstack/services/compute/compute_instance_metadata_sensitive_data/compute_instance_metadata_sensitive_data.py`:
- Around line 39-54: The metadata-to-key mapping in
compute_instance_metadata_sensitive_data currently only checks the upper bound
when using secret["line_number"] - 2, so line numbers 0 or 1 can incorrectly
index from the end of original_metadata_keys. Update the list comprehension in
the instance.metadata handling block to guard both bounds before indexing,
matching the pattern used by the sibling OpenStack metadata checks, and keep the
mapping logic in the same secrets_string/report.status_extended flow.
---
Duplicate comments:
In `@prowler/lib/utils/utils.py`:
- Around line 190-195: The Kingfisher subprocess call in the shared helper is
missing a real process-level timeout, so a hung scan can block secret scanning
indefinitely. Update the subprocess.run invocation in the helper that builds and
executes _build_kingfisher_command to pass a timeout value, and handle the
timeout case by terminating the process and surfacing a clear failure. Apply the
same timeout behavior to the other kingfisher scan call site noted in the
comment so both paths are protected.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 19f86927-6d4f-4f0d-a2d9-919399f4b1a5
📒 Files selected for processing (22)
docs/developer-guide/checks.mdxdocs/developer-guide/secret-scanning-checks.mdxdocs/docs.jsonprowler/lib/utils/utils.pyprowler/providers/aws/services/autoscaling/autoscaling_find_secrets_ec2_launch_configuration/autoscaling_find_secrets_ec2_launch_configuration.pyprowler/providers/aws/services/awslambda/awslambda_function_no_secrets_in_code/awslambda_function_no_secrets_in_code.pyprowler/providers/aws/services/awslambda/awslambda_function_no_secrets_in_variables/awslambda_function_no_secrets_in_variables.pyprowler/providers/aws/services/cloudformation/cloudformation_stack_outputs_find_secrets/cloudformation_stack_outputs_find_secrets.pyprowler/providers/aws/services/cloudwatch/cloudwatch_log_group_no_secrets_in_logs/cloudwatch_log_group_no_secrets_in_logs.pyprowler/providers/aws/services/codebuild/codebuild_project_no_secrets_in_variables/codebuild_project_no_secrets_in_variables.pyprowler/providers/aws/services/ec2/ec2_instance_secrets_user_data/ec2_instance_secrets_user_data.pyprowler/providers/aws/services/ec2/ec2_launch_template_no_secrets/ec2_launch_template_no_secrets.pyprowler/providers/aws/services/ecs/ecs_task_definitions_no_environment_secrets/ecs_task_definitions_no_environment_secrets.pyprowler/providers/aws/services/glue/glue_etl_jobs_no_secrets_in_arguments/glue_etl_jobs_no_secrets_in_arguments.pyprowler/providers/aws/services/ssm/ssm_document_secrets/ssm_document_secrets.pyprowler/providers/aws/services/stepfunctions/stepfunctions_statemachine_no_secrets_in_definition/stepfunctions_statemachine_no_secrets_in_definition.pyprowler/providers/openstack/services/blockstorage/blockstorage_snapshot_metadata_sensitive_data/blockstorage_snapshot_metadata_sensitive_data.pyprowler/providers/openstack/services/blockstorage/blockstorage_volume_metadata_sensitive_data/blockstorage_volume_metadata_sensitive_data.pyprowler/providers/openstack/services/compute/compute_instance_metadata_sensitive_data/compute_instance_metadata_sensitive_data.pyprowler/providers/openstack/services/objectstorage/objectstorage_container_metadata_sensitive_data/objectstorage_container_metadata_sensitive_data.pytests/lib/utils/utils_test.pytests/providers/aws/services/ec2/ec2_instance_secrets_user_data/ec2_instance_secrets_user_data_test.py
2 tasks
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
prowler/lib/utils/utils.py (1)
199-240: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftDo Not Collapse Scanner Failures Into “No Findings”
A non-zero Kingfisher exit, timeout, JSON parse error, or any other exception here only logs and returns, so the failed chunk contributes nothing to
results. Becausedetect_secrets_scan_batch()omits keys without findings and downstream checks treat missing keys as clean, a broken scan turns into false PASS results instead of a surfaced scan failure. Please propagate an explicit error state that callers can map to an error/reporting path rather than silently dropping the chunk.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@prowler/lib/utils/utils.py` around lines 199 - 240, The secret-scanning flow in the batch helper is swallowing failures and leaving affected chunks out of results, which makes scan errors look like clean “no findings” cases. Update the logic around the Kingfisher run and the parsing loop in the utility that builds results so non-zero exit codes, timeouts, JSON decode problems, and other exceptions are recorded as an explicit error state for that batch instead of just logging and returning. Make sure the caller-facing path from detect_secrets_scan_batch can distinguish “scan failed” from “no findings,” using the existing results aggregation and any related error handling in the scanner helper.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@prowler/lib/utils/utils.py`:
- Around line 199-240: The secret-scanning flow in the batch helper is
swallowing failures and leaving affected chunks out of results, which makes scan
errors look like clean “no findings” cases. Update the logic around the
Kingfisher run and the parsing loop in the utility that builds results so
non-zero exit codes, timeouts, JSON decode problems, and other exceptions are
recorded as an explicit error state for that batch instead of just logging and
returning. Make sure the caller-facing path from detect_secrets_scan_batch can
distinguish “scan failed” from “no findings,” using the existing results
aggregation and any related error handling in the scanner helper.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e4009b5e-6aeb-46a4-ba76-2ae6abe27e58
📒 Files selected for processing (16)
docs/developer-guide/secret-scanning-checks.mdxdocs/user-guide/cli/tutorials/configuration_file.mdxprowler/CHANGELOG.mdprowler/lib/utils/utils.pyprowler/providers/openstack/services/blockstorage/blockstorage_snapshot_metadata_sensitive_data/blockstorage_snapshot_metadata_sensitive_data.metadata.jsonprowler/providers/openstack/services/blockstorage/blockstorage_volume_metadata_sensitive_data/blockstorage_volume_metadata_sensitive_data.metadata.jsonprowler/providers/openstack/services/compute/compute_instance_metadata_sensitive_data/compute_instance_metadata_sensitive_data.metadata.jsonprowler/providers/openstack/services/objectstorage/objectstorage_container_metadata_sensitive_data/objectstorage_container_metadata_sensitive_data.metadata.jsontests/providers/aws/services/awslambda/awslambda_function_no_secrets_in_variables/awslambda_function_no_secrets_in_variables_test.pytests/providers/aws/services/codebuild/codebuild_project_no_secrets_in_variables/codebuild_project_no_secrets_in_variables_test.pytests/providers/aws/services/ec2/ec2_instance_secrets_user_data/ec2_instance_secrets_user_data_test.pytests/providers/aws/services/ec2/ec2_launch_template_no_secrets/ec2_launch_template_no_secrets_test.pytests/providers/openstack/services/blockstorage/blockstorage_snapshot_metadata_sensitive_data/blockstorage_snapshot_metadata_sensitive_data_test.pytests/providers/openstack/services/blockstorage/blockstorage_volume_metadata_sensitive_data/blockstorage_volume_metadata_sensitive_data_test.pytests/providers/openstack/services/compute/compute_instance_metadata_sensitive_data/compute_instance_metadata_sensitive_data_test.pytests/providers/openstack/services/objectstorage/objectstorage_container_metadata_sensitive_data/objectstorage_container_metadata_sensitive_data_test.py
Member
|
Blocking this for now. There are two issues here that we should not merge as-is. 1. Kingfisher failures currently fail openIn That means several checks can end up reporting That is not acceptable for a security check. “Scanner failed” and “no secrets found” are completely different states. We cannot collapse both into the same result. Please make scanner failures explicit:
2. CloudWatch multiline rescan can still produce false FAILsIn That leaves us with a If all findings are ignored after the rescan, that event should not contribute to the final Please only add multiline timestamp entries when there is at least one non-ignored secret left, and add a regression test for this scenario. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
detect-secrets(the library powering Prowler's secret-scanning checks) is effectively in maintenance-only mode, so this PR replaces it with an actively-maintained engine.After evaluating the candidates, Kingfisher (MongoDB, Apache-2.0) was selected because of its active maintenance, detection coverage, and distribution: it ships a cross-platform
pipwheel (kingfisher-bin), so it is bundled withpip install prowlerand needs no separate install for self-hosted users.Description
Replaces the
detect-secretsdependency withkingfisher-bin==1.104.0and rewritesdetect_secrets_scan()(prowler/lib/utils/utils.py) to shell out to Kingfisher. The function keeps the same signature and return shape, so the 16 secret-scanning checks (12 AWS + 4 OpenStack) need no contract changes.--no-validate --no-update-check, so no data leaves the audited environment. Uses Kingfisher's built-in ruleset atlowconfidence to retain the generic keyword/password coveragedetect-secretsprovided.--scan-secrets-validateflag andsecrets_validateconfig option validate discovered secrets against the provider APIs (the secret itself is used as the credential — no extra Prowler permissions needed). Secrets confirmed live are escalated to critical and annotated instatus_extended. Off by default.AKIA…EXAMPLE,password=1234…) are no longer reported — a precision improvement that reduces false positives. Test fixtures were updated to realistic fake secrets accordingly.Known coverage differences vs detect-secrets (acceptable; documented):
Steps to review
uv sync(pullskingfisher-bin, dropsdetect-secrets).uv run python prowler-cli.py aws -p <profile> -c ec2_instance_secrets_user_data ec2_launch_template_no_secrets ssm_document_secrets cloudwatch_log_group_no_secrets_in_logs …→ confirm FAIL findings render with Kingfisher type names (e.g.Generic Password,Stripe Secret / Restricted Key).--scan-secrets-validate(or setsecrets_validate: Truein the config) → confirm it runs; live secrets would escalate to critical (offline-only environments will see no live confirmations).uv run pytest -p no:randomly tests/lib/utils/utils_test.py tests/providers/aws/services/{autoscaling,awslambda,cloudformation,cloudwatch,codebuild,ec2,ecs,glue,ssm,stepfunctions}/tests/providers/openstack/→ 543 pass.Checklist
Community Checklist
SDK/CLI
UI
API
License
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
Summary by CodeRabbit
aws.secrets_validateand--scan-secrets-validateto confirm whether discovered secrets are live against provider APIs.detect_secrets_pluginsconfiguration support and updated related schema and tests.