feat(codepipeline): add codepipeline_pipeline_no_secrets_in_definitio…#11842
Open
Sid-0602 wants to merge 1 commit into
Open
feat(codepipeline): add codepipeline_pipeline_no_secrets_in_definitio…#11842Sid-0602 wants to merge 1 commit into
Sid-0602 wants to merge 1 commit into
Conversation
github-actions
Bot
added
provider/aws
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughAdds a new AWS CodePipeline check that scans stage action configurations for hardcoded secrets, while expanding the service model to expose pipeline stages and actions. Also includes metadata, tests, and a changelog update. ChangesCodePipeline Secrets Detection Check
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant codepipeline_client as codepipeline_client
participant codepipeline_pipeline_no_secrets_in_definition as codepipeline_pipeline_no_secrets_in_definition
participant detect_secrets_scan as detect_secrets_scan
codepipeline_client->>codepipeline_pipeline_no_secrets_in_definition: execute() over discovered pipelines
loop each pipeline stage/action
codepipeline_pipeline_no_secrets_in_definition->>detect_secrets_scan: scan action configuration JSON
detect_secrets_scan-->>codepipeline_pipeline_no_secrets_in_definition: findings, if any
end
alt findings exist
codepipeline_pipeline_no_secrets_in_definition->>codepipeline_pipeline_no_secrets_in_definition: mark FAIL and set status_extended
else no findings
codepipeline_pipeline_no_secrets_in_definition->>codepipeline_pipeline_no_secrets_in_definition: keep PASS
end
codepipeline_pipeline_no_secrets_in_definition-->>codepipeline_client: List of Check_Report_AWS
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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
|
✅ 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: 2
🤖 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/codepipeline/codepipeline_pipeline_no_secrets_in_definition/codepipeline_pipeline_no_secrets_in_definition.py`:
- Around line 22-64: The severity escalation for validated secrets is missing in
CodePipelinePipelineNoSecretsInDefinition.execute, so FAIL results always stay
at the default metadata severity. Update the loop that processes
detect_secrets_scan output to inspect each secret entry’s is_verified flag and,
when any detected secret is verified, set report.check_metadata.Severity to
"critical" before appending the report; keep the existing severity for
unverified findings.
In `@prowler/providers/aws/services/codepipeline/codepipeline_service.py`:
- Around line 89-114: The stage collection in CodePipeline parsing is being
blocked by the source parsing path, so a failure at source_info can leave
pipeline.stages empty. Update the logic in the CodePipeline service method that
builds PipelineStage and PipelineAction objects so stage capture runs before, or
independently from, the source_info / Source construction. Keep the source
parsing isolated so any exception there does not suppress populating
pipeline.stages or prevent the secret-scanning data from being recorded.
🪄 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: 4d093dff-7852-49ee-a030-77b9fe023664
📒 Files selected for processing (7)
prowler/CHANGELOG.mdprowler/providers/aws/services/codepipeline/codepipeline_pipeline_no_secrets_in_definition/__init__.pyprowler/providers/aws/services/codepipeline/codepipeline_pipeline_no_secrets_in_definition/codepipeline_pipeline_no_secrets_in_definition.metadata.jsonprowler/providers/aws/services/codepipeline/codepipeline_pipeline_no_secrets_in_definition/codepipeline_pipeline_no_secrets_in_definition.pyprowler/providers/aws/services/codepipeline/codepipeline_service.pytests/providers/aws/services/codepipeline/codepipeline_pipeline_no_secrets_in_definition/__init__.pytests/providers/aws/services/codepipeline/codepipeline_pipeline_no_secrets_in_definition/codepipeline_pipeline_no_secrets_in_definition_test.py
Sid-0602
force-pushed
the
feat/codepipeline-pipeline-no-secrets-in-definition
branch
from
bfb561a to
be031c9
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
prowler/providers/aws/services/codepipeline/codepipeline_pipeline_no_secrets_in_definition/codepipeline_pipeline_no_secrets_in_definition.py (1)
39-60: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winSeverity escalation for validated secrets still missing.
Metadata
Notes(line 47 of the metadata file) state the severity "may be treated as Critical if the detected secret is validated as active," butexecute()never inspectsis_verifiedon thedetect_secrets_scanoutput to bumpreport.check_metadata.Severity. Every FAIL is reported at the statichighseverity regardless of validation status. This mirrors a concern raised on a prior revision and remains unresolved.🐛 Proposed fix to escalate severity for verified secrets
for stage in pipeline.stages: for action in stage.actions: if action.configuration: detect_secrets_output = detect_secrets_scan( data=json.dumps(action.configuration), excluded_secrets=secrets_ignore_patterns, detect_secrets_plugins=codepipeline_client.audit_config.get( "detect_secrets_plugins" ), ) if detect_secrets_output: secrets_info = [ f"{secret['type']} in stage {stage.name} action {action.name}" for secret in detect_secrets_output ] secrets_found.extend(secrets_info) + if any( + secret.get("is_verified") for secret in detect_secrets_output + ): + report.check_metadata.Severity = "critical" if secrets_found: report.status = "FAIL" report.status_extended = f"CodePipeline pipeline {pipeline.name} has secrets in its definition: {', '.join(secrets_found)}."🤖 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/codepipeline/codepipeline_pipeline_no_secrets_in_definition/codepipeline_pipeline_no_secrets_in_definition.py` around lines 39 - 60, The severity handling in execute() is still static even when detect_secrets_scan returns verified secrets; update the CodePipeline check to inspect each secret’s is_verified flag before building secrets_found. If any detected secret is validated as active, escalate report.check_metadata.Severity from high to critical; otherwise keep the existing severity. Use the existing execute() flow and the detect_secrets_scan results to drive this change so the report status and severity reflect verified secrets correctly.prowler/providers/aws/services/codepipeline/codepipeline_service.py (1)
92-114: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winSource-parsing failure still suppresses stage capture.
source_info = all_stages[0]["actions"][0]and the subsequentSource(...)construction (lines 95-101) still run before the stage/action capture loop. Ifall_stagesis empty, or the first action lacksactionTypeId/configurationkeys, an exception is raised and caught by the broadexcept Exceptionat line 119 — leavingpipeline.stagesempty (its default[]) and silently disabling the secret scan for that pipeline, even for later stages that might contain secrets. This is the same concern raised on a prior revision of this PR and remains unresolved: source parsing and stage capture should be decoupled so a failure in one doesn't suppress the other.🐛 Proposed fix to decouple stage capture from source parsing
pipeline_info = regional_client.get_pipeline(name=pipeline.name) all_stages = pipeline_info["pipeline"]["stages"] - # Capture source info from the first stage/action (existing behaviour) - source_info = all_stages[0]["actions"][0] - repository_id = source_info["configuration"].get("FullRepositoryId", "") - pipeline.source = Source( - type=source_info["actionTypeId"]["provider"], - repository_id=repository_id, - configuration=source_info["configuration"], - ) - # Capture all stages and their action configurations for secret scanning for stage in all_stages: stage_obj = PipelineStage(name=stage["name"]) for action in stage.get("actions", []): stage_obj.actions.append( PipelineAction( name=action["name"], configuration=action.get("configuration"), ) ) pipeline.stages.append(stage_obj) + + # Capture source info from the first stage/action (existing behaviour) + try: + source_info = all_stages[0]["actions"][0] + repository_id = source_info["configuration"].get("FullRepositoryId", "") + pipeline.source = Source( + type=source_info["actionTypeId"]["provider"], + repository_id=repository_id, + configuration=source_info["configuration"], + ) + except (IndexError, KeyError) as error: + logger.warning( + f"{pipeline.region} -- {error.__class__.__name__}[{error.__traceback__.tb_lineno}]: {error}" + )🤖 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/codepipeline/codepipeline_service.py` around lines 92 - 114, The source parsing in CodePipeline handling still blocks stage capture because the Source(...) setup runs before the stage loop in the pipeline processing logic. Move the all_stages iteration and PipelineStage/PipelineAction population so it always runs independently of source extraction, and guard the source_info/Source construction in its own try or conditional checks for empty stages, missing actions, or absent actionTypeId/configuration. Keep the stage capture path in the codepath that builds pipeline.stages, so a failure in source parsing does not suppress later stages from being scanned.
🤖 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/codepipeline/codepipeline_pipeline_no_secrets_in_definition/codepipeline_pipeline_no_secrets_in_definition.py`:
- Around line 41-56: The secret scan in the pipeline action loop is done once
per action via detect_secrets_scan, causing repeated temp file I/O overhead.
Refactor the logic around the pipeline/stage/action iteration in
codepipeline_pipeline_no_secrets_in_definition to batch all action configuration
payloads for a pipeline into a single scan call if detect_secrets_scan can
support it, or otherwise minimize repeated calls by aggregating inputs before
scanning. Keep the per-action context in the results so secrets_info still
reports the stage name and action name for each finding.
---
Duplicate comments:
In
`@prowler/providers/aws/services/codepipeline/codepipeline_pipeline_no_secrets_in_definition/codepipeline_pipeline_no_secrets_in_definition.py`:
- Around line 39-60: The severity handling in execute() is still static even
when detect_secrets_scan returns verified secrets; update the CodePipeline check
to inspect each secret’s is_verified flag before building secrets_found. If any
detected secret is validated as active, escalate report.check_metadata.Severity
from high to critical; otherwise keep the existing severity. Use the existing
execute() flow and the detect_secrets_scan results to drive this change so the
report status and severity reflect verified secrets correctly.
In `@prowler/providers/aws/services/codepipeline/codepipeline_service.py`:
- Around line 92-114: The source parsing in CodePipeline handling still blocks
stage capture because the Source(...) setup runs before the stage loop in the
pipeline processing logic. Move the all_stages iteration and
PipelineStage/PipelineAction population so it always runs independently of
source extraction, and guard the source_info/Source construction in its own try
or conditional checks for empty stages, missing actions, or absent
actionTypeId/configuration. Keep the stage capture path in the codepath that
builds pipeline.stages, so a failure in source parsing does not suppress later
stages from being scanned.
🪄 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: 4fb5afda-988e-4d64-8e39-d7033b764906
📒 Files selected for processing (7)
prowler/CHANGELOG.mdprowler/providers/aws/services/codepipeline/codepipeline_pipeline_no_secrets_in_definition/__init__.pyprowler/providers/aws/services/codepipeline/codepipeline_pipeline_no_secrets_in_definition/codepipeline_pipeline_no_secrets_in_definition.metadata.jsonprowler/providers/aws/services/codepipeline/codepipeline_pipeline_no_secrets_in_definition/codepipeline_pipeline_no_secrets_in_definition.pyprowler/providers/aws/services/codepipeline/codepipeline_service.pytests/providers/aws/services/codepipeline/codepipeline_pipeline_no_secrets_in_definition/__init__.pytests/providers/aws/services/codepipeline/codepipeline_pipeline_no_secrets_in_definition/codepipeline_pipeline_no_secrets_in_definition_test.py
Comment on lines
+41
to
+56
| for stage in pipeline.stages: | ||
| for action in stage.actions: | ||
| if action.configuration: | ||
| detect_secrets_output = detect_secrets_scan( | ||
| data=json.dumps(action.configuration), | ||
| excluded_secrets=secrets_ignore_patterns, | ||
| detect_secrets_plugins=codepipeline_client.audit_config.get( | ||
| "detect_secrets_plugins" | ||
| ), | ||
| ) | ||
| if detect_secrets_output: | ||
| secrets_info = [ | ||
| f"{secret['type']} in stage {stage.name} action {action.name}" | ||
| for secret in detect_secrets_output | ||
| ] | ||
| secrets_found.extend(secrets_info) |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🔵 Trivial | ⚖️ Poor tradeoff
Per-action temp file overhead.
detect_secrets_scan writes a new temp file per call (per action, per pipeline). For pipelines with many stages/actions, this adds file I/O overhead per check run. Consider batching all action configurations for a pipeline into a single scan call if the shared helper's signature can accommodate it, though this may require upstream changes to detect_secrets_scan.
🧰 Tools
🪛 ast-grep (0.44.1)
[info] 44-44: use jsonify instead of json.dumps for JSON output
Context: json.dumps(action.configuration)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
🤖 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/codepipeline/codepipeline_pipeline_no_secrets_in_definition/codepipeline_pipeline_no_secrets_in_definition.py`
around lines 41 - 56, The secret scan in the pipeline action loop is done once
per action via detect_secrets_scan, causing repeated temp file I/O overhead.
Refactor the logic around the pipeline/stage/action iteration in
codepipeline_pipeline_no_secrets_in_definition to batch all action configuration
payloads for a pipeline into a single scan call if detect_secrets_scan can
support it, or otherwise minimize repeated calls by aggregating inputs before
scanning. Keep the per-action context in the results so secrets_info still
reports the stage name and action name for each finding.
Sid-0602
force-pushed
the
feat/codepipeline-pipeline-no-secrets-in-definition
branch
from
be031c9 to
84ed91e
Compare
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.
…n check
Context
Closes #11808.
CodePipeline stage action configurations can contain hardcoded secrets (API keys, tokens, passwords). These configurations are readable by any principal with
codepipeline:GetPipelinepermission, making embedded credentials a lateral movement risk across CI/CD pipelines.This PR implements the check suggested in that issue, using the existing
detect-secretshelper already used bycodebuild_project_no_secrets_in_variables.Description
Adds
codepipeline_pipeline_no_secrets_in_definition— a new AWS check that scans all stage action configurations in a CodePipeline pipeline for hardcoded secrets.Changes:
codepipeline_service.py— AddedPipelineActionandPipelineStagePydantic models; extended_get_pipeline_stateto capture all stages and their action configurations (existingsourcefield andcodepipeline_project_repo_privatecheck are unaffected).codepipeline_pipeline_no_secrets_in_definition/— New check with__init__.py, check implementation, andmetadata.json. Severity is High. Reports stage and action name, never the secret value.Steps to review
codepipeline_service.py— specificallyPipelineAction,PipelineStage, and the updated_get_pipeline_statemethod. The originalsourcecapture on line 1 of that method is preserved.codepipeline_pipeline_no_secrets_in_definition.py— it mirrors the pattern fromcodebuild_project_no_secrets_in_variables.pytest tests/providers/aws/services/codepipeline/ -v # 9 passed (5 new + 4 pre-existing)Checklist
Community Checklist
SDK/CLI
codepipeline:GetPipelinecall already made by the service.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