Skip to content

fix: Fixed Access Logging #26

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
erastusndico merged 1 commit into from
Oct 21, 2025
Merged

fix: Fixed Access Logging #26

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .pre-commit-config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ repos:
files: \.(tf|tfvars)$

- repo: https://github.com/bridgecrewio/checkov.git
rev: '3.2.483'
rev: '3.2.484'
hooks:
- id: checkov
name: checkov-aws
Expand Down
13 changes: 9 additions & 4 deletions aws/modules/s3/main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ locals {
#checkov:skip=CKV_AWS_144:Not required for this project
resource "aws_s3_bucket" "this" {
bucket = var.bucket_name
object_lock_enabled = var.enable_object_lock
object_lock_enabled = var.enable_object_lock && !var.is_logging_target # Cannot have object lock on logging destination
tags = local.tags
}

Expand All @@ -25,9 +25,9 @@ resource "aws_s3_bucket_versioning" "this" {
}
}

#Object Lock (if enabled)
#Object Lock (if enabled and not a logging target)
resource "aws_s3_bucket_object_lock_configuration" "this" {
count = var.enable_object_lock ? 1 : 0
count = var.enable_object_lock && !var.is_logging_target ? 1 : 0
bucket = aws_s3_bucket.this.id

rule {
Expand Down Expand Up @@ -67,6 +67,11 @@ resource "aws_s3_bucket_logging" "this" {
bucket = aws_s3_bucket.this.id
target_bucket = var.logging_target_bucket
target_prefix = var.logging_target_prefix != null ? var.logging_target_prefix : "${var.environment}-bucket-logs/"

depends_on = [
aws_s3_bucket.this,
aws_s3_bucket_ownership_controls.this
]
}

# Server-Side Encryption
Expand Down Expand Up @@ -110,7 +115,7 @@ resource "aws_s3_bucket_ownership_controls" "this" {
bucket = aws_s3_bucket.this.id

rule {
object_ownership = var.s3_object_ownership
object_ownership = var.is_logging_target ? "BucketOwnerPreferred" : var.s3_object_ownership
}
}

Expand Down
6 changes: 6 additions & 0 deletions aws/modules/s3/variables.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,12 @@ variable "s3_object_ownership" {
default = "BucketOwnerPreferred"
}

variable "is_logging_target" {
description = "Whether this bucket is used as a logging target for other S3 buckets"
type = bool
default = false
}

variable "additional_bucket_policy_documents" {
description = "Optional list of JSON policy documents to merge with the module's bucket policy"
type = list(string)
Expand Down
32 changes: 32 additions & 0 deletions aws/non-prod-infra/dev/s3-policy.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,5 +52,37 @@ data "aws_iam_policy_document" "s3_core_audit_logs_policy" {
variable = "s3:x-amz-acl"
values = ["bucket-owner-full-control"]
}
condition {
test = "StringEquals"
variable = "aws:SourceAccount"
values = [data.aws_caller_identity.current.account_id]
}
condition {
test = "ArnLike"
variable = "aws:SourceArn"
values = [
"arn:aws:s3:::${local.s3_bucket_kfd_name}",
"arn:aws:s3:::${local.s3_bucket_static_files_name}",
"arn:aws:s3:::${local.s3_bucket_submitted_form_data_name}",
"arn:aws:s3:::${local.s3_bucket_zip_files_name}"
]
}
}
statement {
sid = "AllowS3LoggingGetBucketAcl"
effect = "Allow"
principals {
type = "Service"
identifiers = ["logging.s3.amazonaws.com"]
}
actions = ["s3:GetBucketAcl"]
resources = [
"arn:aws:s3:::${local.s3_bucket_core_audit_logs_name}"
]
condition {
test = "StringEquals"
variable = "aws:SourceAccount"
values = [data.aws_caller_identity.current.account_id]
}
}
}
13 changes: 9 additions & 4 deletions aws/non-prod-infra/dev/s3.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,18 +115,23 @@ module "s3_core_audit_logs" {
kms_key_id = data.aws_kms_alias.s3.target_key_arn
enable_public_access_block = true
enable_mfa_delete = false
enable_object_lock = true
enable_object_lock = false
object_lock_mode = "GOVERNANCE"
object_lock_retention_days = 30
Comment on lines +118 to 120
Copy link

Copilot AI Oct 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Object lock configuration parameters (object_lock_mode and object_lock_retention_days) are specified but enable_object_lock is set to false. These parameters will have no effect and should be removed or the configuration should be updated to conditionally omit them when object lock is disabled.

Copilot uses AI. Check for mistakes.
enable_lifecycle_expiration = true
enable_lifecycle_expiration = false
expiration_days = 30
additional_bucket_policy_documents = [data.aws_iam_policy_document.s3_core_audit_logs_policy.json]
is_logging_target = true
# checkov:CKV2_AWS_62: Not required for this bucket
# checkov:CKV2_AWS_61: Not required for this bucket
# checkov:CKV_AWS_144: Not required for this project
}

resource "aws_s3_bucket_acl" "cloudfront_acl" {
bucket = local.s3_bucket_core_audit_logs_name
resource "aws_s3_bucket_acl" "core_audit_logs_acl" {
bucket = module.s3_core_audit_logs.bucket_id
acl = "log-delivery-write"

depends_on = [
module.s3_core_audit_logs
]
}
32 changes: 32 additions & 0 deletions aws/non-prod-infra/staging/s3-policy.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,5 +51,37 @@ data "aws_iam_policy_document" "s3_core_audit_logs_policy" {
variable = "s3:x-amz-acl"
values = ["bucket-owner-full-control"]
}
condition {
test = "StringEquals"
variable = "aws:SourceAccount"
values = [data.aws_caller_identity.current.account_id]
}
condition {
test = "ArnLike"
variable = "aws:SourceArn"
values = [
"arn:aws:s3:::${local.s3_bucket_kfd_name}",
"arn:aws:s3:::${local.s3_bucket_static_files_name}",
"arn:aws:s3:::${local.s3_bucket_submitted_form_data_name}",
"arn:aws:s3:::${local.s3_bucket_zip_files_name}"
]
}
}
statement {
sid = "AllowS3LoggingGetBucketAcl"
effect = "Allow"
principals {
type = "Service"
identifiers = ["logging.s3.amazonaws.com"]
}
actions = ["s3:GetBucketAcl"]
resources = [
"arn:aws:s3:::${local.s3_bucket_core_audit_logs_name}"
]
condition {
test = "StringEquals"
variable = "aws:SourceAccount"
values = [data.aws_caller_identity.current.account_id]
}
}
}
11 changes: 8 additions & 3 deletions aws/non-prod-infra/staging/s3.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,18 +115,23 @@ module "s3_core_audit_logs" {
kms_key_id = data.aws_kms_alias.s3.target_key_arn
enable_public_access_block = true
enable_mfa_delete = false
enable_object_lock = true
enable_object_lock = false
object_lock_mode = "GOVERNANCE"
object_lock_retention_days = 30
enable_lifecycle_expiration = false
expiration_days = 30
additional_bucket_policy_documents = [data.aws_iam_policy_document.s3_core_audit_logs_policy.json]
is_logging_target = true
# checkov:CKV2_AWS_62: Not required for this bucket
# checkov:CKV2_AWS_61: Not required for this bucket
# checkov:CKV_AWS_144: Not required for this project
}

resource "aws_s3_bucket_acl" "cloudfront_acl" {
bucket = local.s3_bucket_core_audit_logs_name
resource "aws_s3_bucket_acl" "core_audit_logs_acl" {
bucket = module.s3_core_audit_logs.bucket_id
acl = "log-delivery-write"

depends_on = [
module.s3_core_audit_logs
]
}
32 changes: 32 additions & 0 deletions aws/prod-infra/prod/s3-policy.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,5 +51,37 @@ data "aws_iam_policy_document" "s3_core_audit_logs_policy" {
variable = "s3:x-amz-acl"
values = ["bucket-owner-full-control"]
}
condition {
test = "StringEquals"
variable = "aws:SourceAccount"
values = [data.aws_caller_identity.current.account_id]
}
condition {
test = "ArnLike"
variable = "aws:SourceArn"
values = [
"arn:aws:s3:::${local.s3_bucket_kfd_name}",
"arn:aws:s3:::${local.s3_bucket_static_files_name}",
"arn:aws:s3:::${local.s3_bucket_submitted_form_data_name}",
"arn:aws:s3:::${local.s3_bucket_zip_files_name}"
]
}
}
statement {
sid = "AllowS3LoggingGetBucketAcl"
effect = "Allow"
principals {
type = "Service"
identifiers = ["logging.s3.amazonaws.com"]
}
actions = ["s3:GetBucketAcl"]
resources = [
"arn:aws:s3:::${local.s3_bucket_core_audit_logs_name}"
]
condition {
test = "StringEquals"
variable = "aws:SourceAccount"
values = [data.aws_caller_identity.current.account_id]
}
}
}
11 changes: 8 additions & 3 deletions aws/prod-infra/prod/s3.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,18 +115,23 @@ module "s3_core_audit_logs" {
kms_key_id = data.aws_kms_alias.s3.target_key_arn
enable_public_access_block = true
enable_mfa_delete = false
enable_object_lock = true
enable_object_lock = false
object_lock_mode = "GOVERNANCE"
object_lock_retention_days = 30
Comment on lines +118 to 120
Copy link

Copilot AI Oct 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Object lock configuration parameters (object_lock_mode and object_lock_retention_days) are specified but enable_object_lock is set to false. These parameters will have no effect and should be removed or the configuration should be updated to conditionally omit them when object lock is disabled.

Copilot uses AI. Check for mistakes.
enable_lifecycle_expiration = false
expiration_days = 30
additional_bucket_policy_documents = [data.aws_iam_policy_document.s3_core_audit_logs_policy.json]
is_logging_target = true
# checkov:CKV2_AWS_62: Not required for this bucket
# checkov:CKV2_AWS_61: Not required for this bucket
# checkov:CKV_AWS_144: Not required for this project
}

resource "aws_s3_bucket_acl" "cloudfront_acl" {
bucket = local.s3_bucket_core_audit_logs_name
resource "aws_s3_bucket_acl" "core_audit_logs_acl" {
bucket = module.s3_core_audit_logs.bucket_id
acl = "log-delivery-write"

depends_on = [
module.s3_core_audit_logs
]
}