Skip to content

Commit 87b7e4a

Browse files
authored
ci: security guardian changes (#36110)
### Issue # (if applicable) N/A - Enhancement and bug fixes for Security Guardian tool. ### Reason for this change Security guardian is first line of defense for scanning policy, roles and permissions vended by CDK for customers. This GH Action is critical to determine if a PR change is providing secure-by-default policies for any changes introduced in CDK. The Security Guardian tool needed several critical improvements to function properly: 1. **CFN Intrinsic Resolution**: Missing comprehensive support for CloudFormation intrinsic functions like `Fn::Sub`, `Fn::Select`, `Fn::Contains`, etc. 2. **Rule Coverage**: Incomplete security rule coverage across AWS services 3. **Create noise**: Current setup of security guardian does not provide benefit as it fails to exclude cases where the generic cfn-guard rule might not work. Like with KMS secrets or secret manager secrets ### Description of changes Added template preprocessing pipeline with intrinsic resolution and policy normalization, details can be found below **Major Enhancements:** - **Complete CFN Intrinsic Function Resolver**: Added comprehensive support for all CloudFormation intrinsic functions including `Fn::Sub` with literal escaping, `Fn::Select` with bounds checking, `Fn::Contains`, `Fn::Split`, `Fn::Cidr`, `Fn::Base64`, and shorthand forms (`!Ref`, `!GetAtt`, etc.) - **Cross-Stack Resolution**: Implemented resource registry for resolving `Fn::ImportValue` and cross-template references - **Policy Normalization**: Added IAM policy normalizer to resolve intrinsics within policy documents before validation - **Output presentation**: Integrated JUnit XML output with GitHub Actions using pinned `mikepenz/action-junit-report` for rich PR feedback ( suggested by cfn-guard [here](https://github.com/aws-cloudformation/cloudformation-guard/blob/main/guard-examples/ci/.github/workflows/junit-test-and-validate.yml)) - **Security report generation workflow** : Separated out report generation into another workflow due to 2 reasons : - Gives us flexibility to change the trigger type for security-guardian to lower permissive trigger like`pull_request` or `pull_request_review` - Allows fork PRs to run : Fork PRs trigger workflow runs with read permissions, and the separate workflow can execute separate from the fork PR call chain with full permissions needed to update checks and annotation on the PR **Security Rule Expansion:** - Added comprehensive guard rules for 13 AWS services: CodePipeline, DataTrace, DocumentDB, EC2, IAM, Kinesis, Neptune, Redshift, S3, SNS, SQS, and trust scope validation - Enhanced existing rules with better coverage for broad principals, wildcard actions, and cross-account access patterns ### Describe any new or updated permissions being added No new IAM permissions required. All changes are to the static analysis tool and GitHub Actions workflow. ### Description of how you validated changes **Unit Testing**: via - **Guard Rule Syntax**: All 13 guard rule files pass cfn-guard v3 parser validation - **Cross-Stack Testing**: Validated resolution of `Fn::ImportValue` and cross-template references - **Example cases** to test that the current set of rules PASS and FAIL for different scenarios ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 0945692 commit 87b7e4a

File tree

49 files changed

+7377
-348
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

49 files changed

+7377
-348
lines changed

.github/workflows/security-guardian.yml

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
name: Security Guardian
22
on:
3-
pull_request: {}
3+
pull_request_target: {}
44

55
jobs:
66
log-skip:
@@ -34,10 +34,21 @@ jobs:
3434
run: yarn install --frozen-lockfile && cd tools/@aws-cdk/security-guardian && yarn build
3535

3636
- name: Run Security Guardian
37+
id: security-guardian
3738
uses: ./tools/@aws-cdk/security-guardian
3839
with:
3940
base_sha: ${{ github.event.pull_request.base.sha }}
4041
head_sha: ${{ github.event.pull_request.head.sha }}
4142
rule_set_path: './tools/@aws-cdk/security-guardian/rules'
42-
show_summary: 'fail'
43-
output_format: 'json'
43+
- name: Save PR info for security-report
44+
if: always()
45+
run: |
46+
echo "${{ github.event.pull_request.number }}" > ./test-results/pr_number
47+
echo "${{ github.event.pull_request.head.sha }}" > ./test-results/pr_sha
48+
49+
- name: Upload Security Guardian XML Reports
50+
uses: actions/upload-artifact@v4
51+
if: always()
52+
with:
53+
name: security-guardian-reports
54+
path: test-results/
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
name: Security Report
2+
on:
3+
workflow_run:
4+
workflows: ["Security Guardian"]
5+
types: [completed]
6+
7+
jobs:
8+
report:
9+
runs-on: ubuntu-latest
10+
permissions:
11+
checks: write
12+
pull-requests: write
13+
id-token: write
14+
actions: read
15+
steps:
16+
- name: Download artifacts
17+
uses: actions/download-artifact@v5
18+
with:
19+
name: security-guardian-reports
20+
path: test-results/
21+
github-token: ${{ secrets.GITHUB_TOKEN }}
22+
run-id: ${{ github.event.workflow_run.id }}
23+
repository: ${{ github.repository }}
24+
25+
- name: Get PR info
26+
id: pr_info
27+
run: |
28+
echo "pr_number=$(cat test-results/pr_number)" >> "$GITHUB_OUTPUT"
29+
echo "pr_sha=$(cat test-results/pr_sha)" >> "$GITHUB_OUTPUT"
30+
echo "PR: $(cat test-results/pr_number), SHA: $(cat test-results/pr_sha)"
31+
- name: Publish Security Test Results
32+
uses: mikepenz/action-junit-report@v6
33+
if: always()
34+
with:
35+
report_paths: 'test-results/**/cfn-guard-static.xml'
36+
check_name: 'Security Guardian Results'
37+
exclude_sources: 'node_modules,dist'
38+
commit: ${{ steps.pr_info.outputs.pr_sha }}
39+
check_annotations: true
40+
comment: true
41+
pr_id: ${{ steps.pr_info.outputs.pr_number }}
42+
detailed_summary: true
43+
include_passed: false
44+
fail_on_failure: false
45+
group_suite: true
46+
include_skipped: false
47+
check_title_template: '{{TEST_NAME}}'
48+
49+
- name: Publish Security Test Results for resolved templates
50+
uses: mikepenz/action-junit-report@v6
51+
if: always()
52+
with:
53+
report_paths: 'test-results/**/cfn-guard-resolved.xml'
54+
check_name: 'Security Guardian Results with resolved templates'
55+
exclude_sources: 'node_modules,dist'
56+
commit: ${{ steps.pr_info.outputs.pr_sha }}
57+
check_annotations: true
58+
comment: true
59+
pr_id: ${{ steps.pr_info.outputs.pr_number }}
60+
detailed_summary: true
61+
include_passed: false
62+
fail_on_failure: false
63+
group_suite: true
64+
include_skipped: false
65+
check_title_template: '{{TEST_NAME}}'

tools/@aws-cdk/security-guardian/README.md

Lines changed: 138 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ A GitHub Action and CLI tool that helps detect broadly scoped IAM principals in
44

55
- Validating **changed** `*.template.json` files in pull requests using custom [cfn-guard v3](https://github.com/aws-cloudformation/cloudformation-guard) rules.
66
- Detecting **broadly scoped IAM principals** using CloudFormation **intrinsic functions** (e.g., `Fn::Join` with `:root`).
7+
- **Cross-account wildcard detection** - Flags dangerous patterns like `"*"` or `"arn:aws:iam::*:root"`.
8+
- **Extensible exemption system** - Smart exemptions for AWS service default policies.
79

810
---
911

@@ -12,32 +14,92 @@ A GitHub Action and CLI tool that helps detect broadly scoped IAM principals in
1214
Validates **only changed** templates in a PR
1315
Supports **cfn-guard v3** with rule sets
1416
Scans for **broad IAM principals using intrinsics**
17+
**Flags cross-account wildcards** - Always dangerous
18+
**Extensible exemptions** for AWS service defaults
19+
**Future-proof validation** - Detects when AWS changes default policies
1520
Runs locally and in GitHub Actions
1621
Outputs human-readable and machine-parsable summaries
1722

1823
---
1924

25+
## Security Checks
26+
27+
### Rule Organization
28+
Guard rules are organized in service-specific directories for granular control:
29+
30+
```
31+
rules/
32+
├── codepipeline/
33+
│ └── cross-account-role-trust-scope.guard
34+
├── documentdb/
35+
│ └── encryption-enabled.guard
36+
├── ec2/
37+
│ ├── ebs-encryption-enabled.guard
38+
│ └── no-open-security-groups.guard
39+
├── guard-hooks/
40+
│ └── no-root-principals-except-kms-secrets.guard
41+
├── iam/
42+
│ ├── no-overly-permissive-passrole.guard
43+
│ ├── no-wildcard-actions.guard
44+
│ ├── no-world-accessible-trust-policy.guard
45+
│ ├── policy-no-broad-principals.guard
46+
│ └── role-no-broad-principals.guard
47+
├── s3/
48+
│ ├── encryption-enabled.guard
49+
│ ├── no-world-readable.guard
50+
│ └── secure-transport.guard
51+
└── [other services...]
52+
```
53+
54+
### Always Flagged (High Risk)
55+
- **Cross-account wildcards**: `"*"` or `"arn:aws:iam::*:root"`
56+
- **Custom policies with root access**: Non-default policies granting root
57+
- **Broad principals in sensitive resources**: IAM roles, S3 buckets, etc.
58+
59+
### Smart Exemptions (Configurable)
60+
- **AWS KMS default policies**: Standard root access for IAM integration
61+
- **Individual rule control**: Enable/disable specific rules per service
62+
- **Metadata-based suppression**: Use CDK metadata to suppress specific rules
63+
64+
---
65+
2066
## Inputs (GitHub Action)
2167

22-
| Name | Description | Required | Default |
23-
|------------------|------------------------------------------------------|----------|-----------------------|
24-
| `rule_set_path` | Local path to the cfn-guard rules file | No | `./rules` |
25-
| `show_summary` | Show summary (`none`, `all`, `pass`, `fail`, `skip`) | No | `fail` |
26-
| `output_format` | Output format (`single-line-summary`, `json`, etc.) | No | `single-line-summary` |
27-
| `base_sha` | Commit SHA to compare against | No | `origin/main` |
28-
| `head_sha` | The commit SHA for the head (current) branch or PR | No | `HEAD` |
68+
| Name | Description | Required | Default |
69+
|-----------------|-------------------------------------------|----------|---------------|
70+
| `rule_set_path` | Local path to the cfn-guard rules file | Yes | N/A |
71+
| `base_sha` | Commit SHA to compare against | No | `origin/main` |
72+
| `head_sha` | The commit SHA for the head branch or PR | No | `HEAD` |
73+
| `enhance_xml` | Enable XML enhancement for individual failure annotations | No | `true` |
74+
75+
## Outputs (GitHub Action)
76+
77+
| Name | Description |
78+
|--------------|-------------------------------------------|
79+
| `junit_files`| Comma-separated list of JUnit XML files |
80+
| `all_passed` | Whether all validations passed |
2981

3082
---
3183

3284
## Usage (GitHub Action)
3385

3486
```yaml
3587
- name: Run Security Guardian
88+
id: security-guardian
3689
uses: ./tools/@aws-cdk/security-guardian
3790
with:
3891
rule_set_path: './tools/@aws-cdk/security-guardian/rules'
39-
show_summary: 'fail'
40-
output_format: 'single-line-summary'
92+
enhance_xml: 'true' # Optional: Enable individual failure annotations (default: true)
93+
94+
- name: Publish Security Test Results
95+
uses: mikepenz/action-junit-report@e08919a3b1fb83a78393dfb775a9c37f17d8eea6 # v6.0.1
96+
if: always()
97+
with:
98+
report_paths: 'test-results/*.xml'
99+
check_name: 'Security Guardian Results'
100+
detailed_summary: true
101+
include_passed: true
102+
fail_on_failure: true
41103
```
42104
43105
---
@@ -58,32 +120,83 @@ yarn security-guardian
58120

59121
> You can override defaults using:
60122
> - `--base_sha=origin/main`
61-
> - `--output_format=json`
62-
> - `--show_summary=warn`
123+
> - `--rule_set_path=./custom-rules`
63124
64125
---
65126

66127
## Output
67128

68-
In addition to validation results from `cfn-guard`, the tool logs detailed findings from the intrinsic scan (if applicable), such as:
129+
The tool generates JUnit XML reports that can be consumed by GitHub Actions:
130+
131+
- `test-results/cfn-guard-static.xml` - Results from original templates
132+
- `test-results/cfn-guard-resolved.xml` - Results from templates with resolved intrinsics
133+
134+
Use `mikepenz/action-junit-report@e08919a3b1fb83a78393dfb775a9c37f17d8eea6` (v6.0.1) to display rich test results in GitHub PRs with:
135+
- **Enhanced failure messages** - Automatically parses and formats concatenated CFN Guard failures
136+
- **Precise line numbers** - Exact file locations for each violation
137+
- **Resource identification** - Clear resource names and property paths
138+
- **Rule descriptions** - Detailed explanations and remediation guidance
139+
140+
### Enhanced Failure Formatting
69141

142+
The tool automatically enhances CFN Guard failure messages by:
143+
- Splitting concatenated failure messages into individual violations
144+
- Extracting exact line numbers and column positions
145+
- Identifying specific CloudFormation resources and properties
146+
- Formatting output for better readability in CI/CD reports
147+
148+
**Before (Raw CFN Guard Output):**
149+
```
150+
IAM_NO_WILDCARD_ACTIONS_INLINE for Type: ResolvedCheck was not compliant as property [Policies[*].PolicyDocument.Statement[*]] is missing. Value traversed to [Path=/Resources/Role1/Properties[L:324,C:20]]Check was not compliant as property [Policies[*].PolicyDocument.Statement[*]] is missing. Value traversed to [Path=/Resources/Role2/Properties[L:485,C:20]]
151+
```
152+
153+
**After (Enhanced Format):**
70154
```
71-
detailed_output File: changed_templates/example.template.json
72-
{
73-
"Action": "kms:*",
74-
"Effect": "Allow",
75-
"Principal": {
76-
"AWS": {
77-
"Fn::Join": [
78-
"",
79-
["arn:", { "Ref": "AWS::Partition" }, ":iam::", { "Ref": "AWS::AccountId" }, ":root"]
80-
]
81-
}
82-
},
83-
"Resource": "*"
155+
Rule: IAM_NO_WILDCARD_ACTIONS_INLINE (Type: Resolved)
156+
==================================================
157+
158+
- Check was not compliant as property [Policies[*].PolicyDocument.Statement[*]] is missing. Value traversed to [Path=/Resources/Role1/Properties[L:324,C:20]]
159+
- Check was not compliant as property [Policies[*].PolicyDocument.Statement[*]] is missing. Value traversed to [Path=/Resources/Role2/Properties[L:485,C:20]]
160+
```
161+
162+
---
163+
164+
## Suppressing Rules
165+
166+
For integration tests or specific resources that generate false positives, you can suppress individual rules using CDK metadata:
167+
168+
```typescript
169+
import { Bucket } from 'aws-cdk-lib/aws-s3';
170+
import { Stack, StackProps } from 'aws-cdk-lib';
171+
import { Construct } from 'constructs';
172+
173+
export class MyTestStack extends Stack {
174+
constructor(scope: Construct, id: string, props?: StackProps) {
175+
super(scope, id, props);
176+
177+
const testBucket = new Bucket(this, 'TestBucket', {
178+
autoDeleteObjects: true,
179+
removalPolicy: RemovalPolicy.DESTROY,
180+
});
181+
182+
// Suppress S3 encryption rule for integration test
183+
testBucket.node.addMetadata('guard', {
184+
SuppressedRules: ['S3_ENCRYPTION_ENABLED']
185+
});
186+
}
84187
}
85188
```
86189

190+
**Available Rules to Suppress:**
191+
- `S3_ENCRYPTION_ENABLED`
192+
- `IAM_ROLE_NO_BROAD_PRINCIPALS`
193+
- `IAM_NO_WILDCARD_ACTIONS`
194+
- `NO_ROOT_PRINCIPALS_EXCEPT_KMS_SECRETS`
195+
- `EBS_ENCRYPTION_ENABLED`
196+
- `SNS_ENCRYPTION_ENABLED`
197+
- `SQS_ENCRYPTION_ENABLED`
198+
- And others (see individual `.guard` files in service subdirectories)
199+
87200
---
88201

89202
## Acknowledgments

tools/@aws-cdk/security-guardian/action.yml

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,20 +2,25 @@ name: 'Security Guardian'
22
description: 'Security Guardian for custom or granular guard rules'
33

44
inputs:
5-
data_directory:
6-
description: "Path to CloudFormation templates"
7-
required: true
5+
base_sha:
6+
description: "Base commit id"
7+
required: false
8+
head_sha:
9+
description: "Current changes commit id"
10+
required: false
811
rule_set_path:
912
description: "Path to a single .guard file locally"
1013
required: true
11-
show_summary:
12-
description: "cfn-guard summary output. Options are all, pass, fail, skip or none"
14+
enhance_xml:
15+
description: "Enable XML enhancement for individual failure annotations"
1316
required: false
14-
default: "fail"
15-
output_format:
16-
description: "cfn-guard output format. Options: json, yaml, single-line-summary"
17-
required: false
18-
default: "single-line-summary"
17+
default: 'true'
18+
19+
outputs:
20+
junit_files:
21+
description: "Comma-separated list of JUnit XML files"
22+
all_passed:
23+
description: "Whether all validations passed"
1924

2025
runs:
2126
using: node20

0 commit comments

Comments
 (0)