Checkov #110
Checkov #110
Conversation
|
@alexcrtestapp full review |
|
@alexcrtestapp are you there |
2 similar comments
|
@alexcrtestapp are you there |
|
@alexcrtestapp are you there |
|
Tip For best results, initiate chat on the files or code changes. @alexcoderabbitai Yes, I'm here and ready to help! I see your previous request for a full review. Is there anything specific you'd like me to help with regarding this PR or do you have any other questions? |
|
@alexcrtestapp full review |
✅ Actions performedFull review triggered. |
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... 📒 Files selected for processing (1)
🪧 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 (
|
![alexcrtestapp[bot]](https://avatars.githubusercontent.com/u/180435472?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
sampleTerraformFile.tf (1)
9-15: Local variable types should be numbers, not strings
Definingfrom_portandto_portas strings can lead to unexpected type casting. It’s clearer and safer to declare them as numeric literals.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 }
📜 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 AWS credential detected. Storing credentials in source code risks secret leaks. Use environment variables or secure vaults instead.
🔇 Additional comments (2)
sampleTerraformFile.tf (2)
1-8: Skip: Usage instructions are for demonstration only
This section provides sample Checkov CLI commands and references; no functional Terraform code is changed here.
97-105: Terraform version constraints are appropriate
Therequired_versionand provider pinning follow best practices, ensuring compatibility and preventing unintentional upgrades.
| 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
Storing static access_key and secret_key in source code risks credential leaks. Use Terraform input variables or environment variables with sensitive = true.
Suggested refactor:
provider "aws" {
region = var.aws_region
- access_key = "AKIA123456789EXAMPLE"
- secret_key = "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY"
+ # Use variables or environment injection to avoid hard‑coding
+ access_key = var.aws_access_key
+ secret_key = var.aws_secret_key
}And add a variables.tf:
variable "aws_access_key" {
type = string
sensitive = true
}
variable "aws_secret_key" {
type = string
sensitive = true
}🧰 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 AWS credential detected. Storing credentials in source code risks secret leaks. Use environment variables or secure vaults instead.
| 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 no block-public-access and disabled versioning
Using public-read ACL exposes your bucket contents, and disabling block-public-access and versioning increases risk and reduces recoverability.
Proposed secure configuration:
resource "aws_s3_bucket" "public" {
- bucket = "checkov-public-bucket-demo-12345"
- acl = "public-read"
+ bucket = "checkov-public-bucket-demo-12345"
+ acl = "private"
- versioning {
- enabled = false
- }
+ versioning {
+ enabled = true
+ }
+ block_public_access {
+ block_public_acls = true
+ block_public_policy = true
+ ignore_public_acls = true
+ restrict_public_buckets = true
+ }
website {
index_document = "index.html"
}
}| resource "aws_security_group" "insecure" { | ||
| name = "insecure-sg" | ||
| description = "Insecure SG exposing all TCP ports to the world" | ||
| vpc_id = local.aws_vpc_id | ||
|
|
||
| ingress { | ||
| description = "Allow all TCP traffic" | ||
| from_port = 0 | ||
| to_port = 65535 | ||
| protocol = "tcp" | ||
| cidr_blocks = ["0.0.0.0/0"] | ||
| } | ||
|
|
||
| egress { | ||
| description = "Allow all outbound traffic" | ||
| from_port = 0 | ||
| to_port = 0 | ||
| protocol = "-1" | ||
| cidr_blocks = ["0.0.0.0/0"] | ||
| } | ||
| } |
There was a problem hiding this comment.
Security group allows ALL TCP ports from ANYwhere
Allowing ports 0–65535 to 0.0.0.0/0 is a critical risk. Limit ingress to only required ports and trusted CIDRs.
Sample refactor to SSH-only:
resource "aws_security_group" "insecure" {
name = "insecure-sg"
description = "Insecure SG exposing all TCP ports to the world"
vpc_id = local.aws_vpc_id
- ingress {
- description = "Allow all TCP traffic"
- from_port = 0
- to_port = 65535
- protocol = "tcp"
- cidr_blocks = ["0.0.0.0/0"]
- }
+ ingress {
+ description = "Allow SSH from trusted network"
+ from_port = 22
+ to_port = 22
+ protocol = "tcp"
+ cidr_blocks = ["203.0.113.0/24"] # replace with your office CIDR
+ }📝 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.
| resource "aws_security_group" "insecure" { | |
| name = "insecure-sg" | |
| description = "Insecure SG exposing all TCP ports to the world" | |
| vpc_id = local.aws_vpc_id | |
| ingress { | |
| description = "Allow all TCP traffic" | |
| from_port = 0 | |
| to_port = 65535 | |
| protocol = "tcp" | |
| cidr_blocks = ["0.0.0.0/0"] | |
| } | |
| egress { | |
| description = "Allow all outbound traffic" | |
| from_port = 0 | |
| to_port = 0 | |
| protocol = "-1" | |
| cidr_blocks = ["0.0.0.0/0"] | |
| } | |
| } | |
| resource "aws_security_group" "insecure" { | |
| name = "insecure-sg" | |
| description = "Insecure SG exposing all TCP ports to the world" | |
| vpc_id = local.aws_vpc_id | |
| ingress { | |
| description = "Allow SSH from trusted network" | |
| from_port = 22 | |
| to_port = 22 | |
| protocol = "tcp" | |
| cidr_blocks = ["203.0.113.0/24"] # replace with your office CIDR | |
| } | |
| egress { | |
| description = "Allow all outbound traffic" | |
| from_port = 0 | |
| to_port = 0 | |
| protocol = "-1" | |
| cidr_blocks = ["0.0.0.0/0"] | |
| } | |
| } |
| resource "aws_security_group" "this" { | ||
| name = local.sg_name | ||
| description = "Security group " | ||
| vpc_id = local.aws_vpc_id | ||
|
|
||
| ingress { | ||
| description = "Ingress from VPC" | ||
| from_port = local.from_port | ||
| to_port = local.to_port | ||
| protocol = "tcp" | ||
| cidr_blocks = local.cidr_block | ||
| } | ||
|
|
||
| egress { | ||
| from_port = 0 | ||
| to_port = 0 | ||
| protocol = "-1" | ||
| cidr_blocks = ["0.0.0.0/0"] | ||
| } | ||
| } |
There was a problem hiding this comment.
Ingress description misleads and opens port 80 to the world
The rule’s cidr_blocks is ["0.0.0.0/0"] (internet), yet the description says “from VPC.” Either restrict it to your VPC CIDR or correct the description to match intent.
Example correction:
ingress {
- description = "Ingress from VPC"
+ description = "Ingress from Internet on port 80"
+ # To restrict to VPC, set cidr_blocks = [var.vpc_cidr_block]
from_port = local.from_port
to_port = local.to_port
protocol = "tcp"
cidr_blocks = local.cidr_block
}
1 participant
Summary by CodeRabbit