Checkov #111
Checkov #111
Conversation
WalkthroughA new Terraform configuration file has been added, defining several AWS resources with intentionally insecure settings for testing purposes. The file sets up local variables, configures the AWS provider with hardcoded credentials, and declares two security groups—one with minimal restrictions and another with overly permissive rules. Additionally, a public S3 bucket is created with a "public-read" ACL and website hosting enabled. The Terraform block specifies required versions for both Terraform and the AWS provider. Comments are included to explain resource purposes and provide instructions for security scanning. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Terraform
participant AWS
User->>Terraform: Apply configuration
Terraform->>AWS: Create security group (this)
Terraform->>AWS: Create security group (insecure)
Terraform->>AWS: Create public S3 bucket with website config
AWS-->>Terraform: Resources created
Terraform-->>User: Provisioning complete
Poem
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
sampleTerraformFile.tf (3)
9-15: Ensure explicit, secure defaults for ports and CIDR ranges
Using strings for port values and defaulting the CIDR block to["0.0.0.0/0"]increases the risk of misconfiguration and unintended exposure. Convert port values to numeric types and consider requiring explicit CIDR input via variables instead of a wide-open default.Proposed diff:
locals { sg_name = "checkov-test" aws_vpc_id = "vpc-#####" # enter vpc id here cidr_block = ["0.0.0.0/0"] - from_port = "80" - to_port = "80" + from_port = 80 + to_port = 80 }
43-48: Egress rule is overly permissive (-1protocol, all ports)
Allowing all outbound traffic by default increases your attack surface. Consider restricting egress to only the necessary ports and destinations following least‑privilege principles.
68-74: Consider tightening outbound rules for the insecure group
Even for a test group, restricting egress prevents unintended data exfiltration. Define explicit egress rules rather than allowing all traffic.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
sampleTerraformFile.tf(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: Security Checks
sampleTerraformFile.tf
[error] 24-24: terraform.aws.security.aws-provider-static-credentials.aws-provider-static-credentials: A hard-coded credential was detected. It is not recommended to store credentials in source-code, as this risks secrets being leaked. Use environment variables or secure vaults instead.
🪛 CircleCI: semgrep
sampleTerraformFile.tf
[error] 24-24: terraform.aws.security.aws-provider-static-credentials.aws-provider-static-credentials: Hard-coded credential detected. Storing credentials in source code risks secrets being leaked. Use environment variables or secure vaults to provide credentials.
| ingress { | ||
| description = "Ingress from VPC" | ||
| from_port = local.from_port | ||
| to_port = local.to_port | ||
| protocol = "tcp" | ||
| cidr_blocks = local.cidr_block | ||
| } |
There was a problem hiding this comment.
Ingress rule allows traffic from any IP despite the description
The cidr_blocks = local.cidr_block default of ["0.0.0.0/0"] contradicts the "Ingress from VPC" description and opens port 80 globally. Restrict this to your VPC CIDR or specific IP ranges.
Example using a data source:
data "aws_vpc" "selected" {
id = var.aws_vpc_id
}
resource "aws_security_group" "this" {
ingress {
from_port = local.from_port
to_port = local.to_port
protocol = "tcp"
- cidr_blocks = local.cidr_block
+ cidr_blocks = [data.aws_vpc.selected.cidr_block]
}
# ...
}| ingress { | ||
| description = "Allow all TCP traffic" | ||
| from_port = 0 | ||
| to_port = 65535 | ||
| protocol = "tcp" | ||
| cidr_blocks = ["0.0.0.0/0"] | ||
| } |
There was a problem hiding this comment.
Critical: Insecure security group permitting all TCP ports from the internet
This aws_security_group.insecure resource opens ports 0–65535 to 0.0.0.0/0, undermining network security. Unless strictly required for testing, remove or narrow this rule to specific ports and trusted CIDRs.
| provider "aws" { | ||
| region = "us-east-1" | ||
| access_key = "AKIA123456789EXAMPLE" | ||
| secret_key = "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY" | ||
| } |
There was a problem hiding this comment.
Remove hard‑coded AWS credentials from source
Storing access_key and secret_key in your Terraform code risks leaking sensitive credentials. Use environment variables (e.g., AWS_ACCESS_KEY_ID, AWS_SECRET_ACCESS_KEY), shared credentials files, or a secrets manager.
Proposed refactor:
- provider "aws" {
- region = "us-east-1"
- access_key = "AKIA123456789EXAMPLE"
- secret_key = "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY"
- }
+ provider "aws" {
+ region = var.aws_region
+ # credentials supplied via environment variables or shared credentials file
+ }Let me know if you’d like sample variable definitions and documentation.
📝 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.
| provider "aws" { | |
| region = "us-east-1" | |
| access_key = "AKIA123456789EXAMPLE" | |
| secret_key = "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY" | |
| } | |
| provider "aws" { | |
| region = var.aws_region | |
| # credentials supplied via environment variables or shared credentials file | |
| } |
🧰 Tools
🪛 GitHub Actions: Security Checks
[error] 24-24: terraform.aws.security.aws-provider-static-credentials.aws-provider-static-credentials: A hard-coded credential was detected. It is not recommended to store credentials in source-code, as this risks secrets being leaked. Use environment variables or secure vaults instead.
🪛 CircleCI: semgrep
[error] 24-24: terraform.aws.security.aws-provider-static-credentials.aws-provider-static-credentials: Hard-coded credential detected. Storing credentials in source code risks secrets being leaked. Use environment variables or secure vaults to provide credentials.
| resource "aws_s3_bucket" "public" { | ||
| bucket = "checkov-public-bucket-demo-12345" | ||
| acl = "public-read" # Vulnerability: Bucket is publicly readable | ||
|
|
||
| versioning { | ||
| enabled = false | ||
| } | ||
|
|
||
| # Intentionally not configuring block public access to expose potential risk | ||
| website { | ||
| index_document = "index.html" | ||
| } | ||
| } |
There was a problem hiding this comment.
Public S3 bucket with public‑read ACL and no public access block
This configuration exposes bucket contents publicly and disables protective controls. For production, set acl = "private", enable public_access_block, and turn on versioning.
Proposed diff:
resource "aws_s3_bucket" "public" {
bucket = "checkov-public-bucket-demo-12345"
- acl = "public-read"
+ acl = "private"
+ public_access_block {
+ block_public_acls = true
+ block_public_policy = true
+ ignore_public_acls = true
+ restrict_public_buckets = true
+ }
versioning {
- enabled = false
+ enabled = true
}
# website configuration (only enable if hosting a static site via CloudFront)
website {
index_document = "index.html"
}
}Let me know if you’d like a full refactor for private hosting behind CloudFront.
Committable suggestion skipped: line range outside the PR's diff.
|
@alexcrtestapp full review |
|
Tip For best results, initiate chat on the files or code changes. @alexcoderabbitai I'll initiate a full review of your PR now. This will attempt to re-analyze all files in your PR from scratch. I see there was a previous issue with cloning the repository. This full review will try again, but if the problem persists, you might want to:
I'll proceed with the full review process now. |
|
@alexcrtestapp full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
sampleTerraformFile.tf (4)
21-25: Remove hard‑coded AWS credentials
Storingaccess_keyandsecret_keyin source code risks leaking sensitive credentials. Use environment variables or a shared credentials file instead per best practices.🧰 Tools
🪛 GitHub Actions: Security Checks
[error] 24-24: terraform.aws.security.aws-provider-static-credentials.aws-provider-static-credentials: A hard-coded credential was detected. It is not recommended to store credentials in source-code, as this risks secrets being leaked and used by malicious adversaries. Use environment variables or secure vaults instead.
🪛 CircleCI: semgrep
[error] 24-24: terraform.aws.security.aws-provider-static-credentials.aws-provider-static-credentials: Hard-coded AWS credential detected. Storing credentials in source code risks secret leaks. Use environment variables or secure vaults to provide credentials.
35-41: Ingress rule allows traffic from any IP
Thecidr_blocks = local.cidr_blockdefault of["0.0.0.0/0"]contradicts the "Ingress from VPC" description and opens port 80 globally. Restrict this to your VPC CIDR or specific IP ranges.
60-66: Critical: Insecure security group permitting all TCP ports from the internet
Thisaws_security_group.insecureresource opens ports 0–65535 to0.0.0.0/0, undermining network security. Unless strictly required for testing, remove or narrow this rule to specific ports and trusted CIDRs.
80-92: Public S3 bucket with public‑read ACL and no public access block
This configuration exposes bucket contents publicly and disables protective controls. For production, setacl = "private", enablepublic_access_block, and turn on versioning.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
sampleTerraformFile.tf(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: Security Checks
sampleTerraformFile.tf
[error] 24-24: terraform.aws.security.aws-provider-static-credentials.aws-provider-static-credentials: A hard-coded credential was detected. It is not recommended to store credentials in source-code, as this risks secrets being leaked and used by malicious adversaries. Use environment variables or secure vaults instead.
🪛 CircleCI: semgrep
sampleTerraformFile.tf
[error] 24-24: terraform.aws.security.aws-provider-static-credentials.aws-provider-static-credentials: Hard-coded AWS credential detected. Storing credentials in source code risks secret leaks. Use environment variables or secure vaults to provide credentials.
🔇 Additional comments (2)
sampleTerraformFile.tf (2)
1-8: No issues detected in the file header comments.
97-105: No issues detected in the Terraform block settings.
| locals { | ||
| sg_name = "checkov-test" | ||
| aws_vpc_id = "vpc-#####" #enter vpc id here | ||
| cidr_block = ["0.0.0.0/0"] | ||
| from_port = "80" | ||
| to_port = "80" | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Incorrect variable types and insecure defaults in locals.
Defining cidr_block as ["0.0.0.0/0"] opens all traffic, contradicting the intended VPC-only access. Also, from_port and to_port are strings rather than numbers, which may cause type mismatches. Suggest using a data source to retrieve the VPC CIDR and defining ports as numeric values:
locals {
- cidr_block = ["0.0.0.0/0"]
- from_port = "80"
- to_port = "80"
+ cidr_block = [data.aws_vpc.selected.cidr_block]
+ from_port = 80
+ to_port = 80
}And add:
data "aws_vpc" "selected" {
id = local.aws_vpc_id
}
Summary by CodeRabbit