feat(glue): add glue_catalog_connection_no_secrets check#11839
Open
UTKARSH698 wants to merge 7 commits into
Open
feat(glue): add glue_catalog_connection_no_secrets check#11839UTKARSH698 wants to merge 7 commits into
UTKARSH698 wants to merge 7 commits into
Conversation
github-actions
Bot
added
provider/aws
📝 WalkthroughWalkthroughAdds a new AWS Prowler check, ChangesGlue catalog connection secret scan check
Estimated code review effort: 2 (Simple) | ~15 minutes Sequence Diagram(s)sequenceDiagram
participant Check as glue_catalog_connection_no_secrets
participant GlueClient as glue_client
participant Scanner as detect_secrets_scan_batch
Check->>GlueClient: get connections and audit_config
Check->>Scanner: scan batched connection properties
Scanner-->>Check: detections or SecretsScanError
alt scan succeeded
Check->>Check: build PASS or FAIL report per connection
else scan failed
Check->>Check: mark connection as MANUAL
end
Check-->>GlueClient: return findings list
Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
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 |
github-actions
Bot
added
the
has-conflicts
Contributor
|
✅ No Conflicts No conflict markers, and the branch merges cleanly into its base. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/aws/services/glue/glue_catalog_connection_no_secrets/glue_catalog_connection_no_secrets.py`:
- Line 20: The public method execute in glue_catalog_connection_no_secrets lacks
the required type hint and Google-style docstring. Update execute to include an
explicit return type annotation and add a concise docstring describing what the
method does, following the style used by other methods in the same class/module.
🪄 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: fee18b8e-e92a-4289-8668-c609c6decd17
📒 Files selected for processing (5)
prowler/CHANGELOG.mdprowler/providers/aws/services/glue/glue_catalog_connection_no_secrets/__init__.pyprowler/providers/aws/services/glue/glue_catalog_connection_no_secrets/glue_catalog_connection_no_secrets.metadata.jsonprowler/providers/aws/services/glue/glue_catalog_connection_no_secrets/glue_catalog_connection_no_secrets.pytests/providers/aws/services/glue/glue_catalog_connection_no_secrets/glue_catalog_connection_no_secrets_test.py
| should be stored in Secrets Manager or Parameter Store instead. | ||
| """ | ||
|
|
||
| def execute(self): |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Add return type hint and docstring to execute().
As per coding guidelines, prowler/**/*.py requires "Type hints are required for all public functions" and "Docstrings are required for all classes and methods... following Google style documentation". execute() has neither.
♻️ Proposed fix
- def execute(self):
+ def execute(self) -> list[Check_Report_AWS]:
+ """Scan every Glue Data Catalog connection's properties for secrets.
+
+ Returns:
+ list[Check_Report_AWS]: One report per connection with status
+ PASS, FAIL, or MANUAL.
+ """
findings = []📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def execute(self): | |
| def execute(self) -> list[Check_Report_AWS]: | |
| """Scan every Glue Data Catalog connection's properties for secrets. | |
| Returns: | |
| list[Check_Report_AWS]: One report per connection with status | |
| PASS, FAIL, or MANUAL. | |
| """ | |
| findings = [] |
🤖 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/aws/services/glue/glue_catalog_connection_no_secrets/glue_catalog_connection_no_secrets.py`
at line 20, The public method execute in glue_catalog_connection_no_secrets
lacks the required type hint and Google-style docstring. Update execute to
include an explicit return type annotation and add a concise docstring
describing what the method does, following the style used by other methods in
the same class/module.
Source: Coding guidelines
Detects secrets in AWS Glue Data Catalog connection properties (ConnectionProperties), following the same batched Kingfisher secret-scanning pattern as glue_etl_jobs_no_secrets_in_arguments. Closes prowler-cloud#11814.
UTKARSH698
force-pushed
the
feat-glue-secrets-check
branch
from
8d09026 to
d15db5c
Compare
github-actions
Bot
removed
the
has-conflicts
The offline Kingfisher scanner ignores bare high-entropy placeholder values, so the previous fake key was not flagged and the with-secrets test failed in CI. Use the same valid JWT the apigateway stage-variable secrets test relies on.
Kingfisher did not flag a secret embedded in a compact single-line per-property payload, so the with-secrets test failed in CI. Mirror the apigateway stage-variables check: scan each connection's properties as a single indented-JSON document (one value per line) and map a finding back to its property via the finding's line number.
…t failure get_connections in moto 5.1.11 (the pinned CI version) omits ConnectionProperties, so the secret never reached the scanner and the check returned PASS instead of FAIL. Set the properties directly on the built service so the test is deterministic across moto versions.
Codecov Report✅ All modified and coverable lines are covered by tests.
Additional details and impacted files@@ Coverage Diff @@
## master #11839 +/- ##
===========================================
- Coverage 93.80% 3.85% -89.96%
===========================================
Files 261 877 +616
Lines 38141 26201 -11940
===========================================
- Hits 35779 1010 -34769
- Misses 2362 25191 +22829
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Add a test that forces detect_secrets_scan_batch to raise SecretsScanError, exercising the previously-uncovered MANUAL branch and bringing the check to 100% patch coverage.
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
2 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
AWS Glue Data Catalog connections often store hardcoded database credentials in plaintext connection properties (
ConnectionProperties), which is a gap in Prowler'''s existing secret-scanning coverage for Glue.Fix #11814
Description
Adds
glue_catalog_connection_no_secrets, scanning each Data Catalog connection'''sConnectionPropertiesfor hardcoded secrets. Follows the same batched Kingfisher secret-scanning structure asglue_etl_jobs_no_secrets_in_arguments(the reference check linked from the issue): one payload per connection property, keyed by(connection_index, property_name), scanned viadetect_secrets_scan_batch.annotate_verified_secretsconfirms a live secret (matches the issue'''s suggested severity).Reuses the existing
Connectionmodel inglue_service.py(propertiesalready populated fromget_connections), so no service-layer changes were needed.Steps to review
4 tests covering: no connections, connection with no secrets, connection with a secret in properties, and a connection with empty properties. All pass locally, along with the full existing
tests/providers/aws/services/glue/suite (68 passed) andruff format/ruff check.Checklist
SDK/CLI
glue:GetConnectionsread used byglue_database_connections_ssl_enabled.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