feat: add conftest policies for S3 lifecycle and JSON policy enforcement (#199)

* feat: add conftest policies for S3 lifecycle and JSON policy enforcement

* refactor: convert aws_iam_policy_document to jsonencode

* fix: install conftest and opa in precommit CI workflow

Author: marekaf

.github/workflows/precommit.yaml

@@ -45,6 +45,19 @@ jobs:
           chmod +x ./shfmt
           sudo mv ./shfmt /usr/local/bin/shfmt
 
+          # conftest
+          curl -sSLo ./conftest.tar.gz \
+            https://github.com/open-policy-agent/conftest/releases/download/v0.66.0/conftest_0.66.0_Linux_x86_64.tar.gz
+          tar -xzf ./conftest.tar.gz conftest
+          chmod +x ./conftest
+          sudo mv ./conftest /usr/local/bin/conftest
+
+          # opa
+          curl -sSLo ./opa \
+            https://github.com/open-policy-agent/opa/releases/download/v1.13.2/opa_linux_amd64_static
+          chmod +x ./opa
+          sudo mv ./opa /usr/local/bin/opa
+
           pip install pre-commit
           pip install checkov==3.2.500
 

.pre-commit-config.yaml

@@ -62,3 +62,18 @@ repos:
     hooks:
       - id: check-github-actions
       - id: check-github-workflows
+
+  - repo: https://github.com/anderseknert/pre-commit-opa
+    rev: v1.5.1
+    hooks:
+      - id: opa-fmt
+      - id: conftest-verify
+        args: [--policy, conftest-policies/]
+
+  - repo: local
+    hooks:
+      - id: conftest-terraform
+        name: Conftest Terraform policy check
+        entry: conftest test --parser hcl2 --policy conftest-policies/ --no-color
+        language: system
+        files: ^(examples|modules)/.*\.tf$

README.md

@@ -45,6 +45,9 @@ The configuration is directly in [.pre-commit-config.yaml](.pre-commit-config.ya
 - `check-executables-have-shebangs` / `check-shebang-scripts-are-executable` - script permission sanity
 - `no-commit-to-branch` - prevents accidental direct commits to `master` and `main`
 - `check-github-actions` / `check-github-workflows` - validates GitHub Actions workflow schema
+- `opa-fmt` - OPA/Rego policy file formatting
+- `conftest-verify` - runs unit tests for custom Conftest policies
+- `conftest-terraform` - validates Terraform files against custom OPA policies
 
 It is possible to manually run all checks on all files using
 ```
@@ -177,6 +180,97 @@ Lambda functions live in `src/<lambda-name>/` directories with an `index.mjs` (o
 ## checkov
 [Checkov](https://www.checkov.io) is an amazing tool to lint terraform (and other) resources, we use the non-official pre-commit hook by antonbabenko
 
+## Conftest Policies
+
+We use [Conftest](https://www.conftest.dev/) (built on [OPA/Rego](https://www.openpolicyagent.org/)) to enforce custom rules on Terraform files that can't be caught by tflint or checkov. Policies live in `conftest-policies/`.
+
+**Current policies:**
+
+- **JSON policy enforcement** (`json_policy.rego`) -- Bans `data "aws_iam_policy_document"` data sources and raw JSON heredoc strings in policy fields across all AWS resource types (IAM, S3, SNS, SQS, KMS, ECR, OpenSearch, CloudWatch Logs, etc.). Use `jsonencode()` instead.
+
+- **S3 lifecycle rule prefix validation** (`s3_lifecycle.rego`) -- Prevents placing `prefix` as a top-level key in S3 lifecycle rules instead of inside a `filter` block. The wrong syntax causes the expiration rule to apply to ALL objects in the bucket, not just the intended prefix.
+
+**Running manually:**
+
+```bash
+# Test a specific file
+conftest test --parser hcl2 --policy conftest-policies/ examples/05-aws-complete/storage.tf
+
+# Run policy unit tests
+conftest verify --policy conftest-policies/
+```
+
+**Adding new policies:**
+
+1. Create a `.rego` file in `conftest-policies/` with `deny_` rules
+2. Add tests in a corresponding `_test.rego` file
+3. Run `conftest verify --policy conftest-policies/` to validate
+4. Run `opa fmt -w conftest-policies/` to format
+
+## JSON Policy Standards
+
+Use `jsonencode()` for all AWS policy fields -- not just IAM, but also S3 bucket policies, KMS key policies, SNS/SQS policies, ECR repository policies, OpenSearch access policies, etc. Do NOT use `data "aws_iam_policy_document"` or raw JSON heredoc strings.
+
+This is enforced by `conftest-policies/json_policy.rego` across all examples and modules.
+
+### Preferred (jsonencode)
+
+```hcl
+resource "aws_iam_policy" "example" {
+  policy = jsonencode({
+    Version = "2012-10-17"
+    Statement = [{
+      Effect   = "Allow"
+      Action   = ["s3:GetObject"]
+      Resource = ["arn:aws:s3:::bucket/*"]
+    }]
+  })
+}
+
+resource "aws_s3_bucket_policy" "example" {
+  bucket = aws_s3_bucket.example.id
+  policy = jsonencode({
+    Version = "2012-10-17"
+    Statement = [{
+      Effect    = "Allow"
+      Principal = "*"
+      Action    = ["s3:GetObject"]
+      Resource  = ["${aws_s3_bucket.example.arn}/*"]
+    }]
+  })
+}
+```
+
+### Avoid (data source)
+
+```hcl
+data "aws_iam_policy_document" "example" {
+  statement {
+    actions   = ["s3:GetObject"]
+    resources = ["arn:aws:s3:::bucket/*"]
+  }
+}
+```
+
+### Avoid (raw JSON heredoc)
+
+```hcl
+resource "aws_iam_policy" "example" {
+  policy = <<EOF
+{
+  "Version": "2012-10-17",
+  "Statement": [{
+    "Effect": "Allow",
+    "Action": ["s3:GetObject"],
+    "Resource": ["arn:aws:s3:::bucket/*"]
+  }]
+}
+EOF
+}
+```
+
+**Rationale**: jsonencode keeps policy definition inline with the resource, is more readable, avoids extra data source lookups, and produces cleaner terraform plan output.
+
 ## Managed Databases
 
 Example 05 shows production-grade Aurora PostgreSQL using the community `terraform-aws-modules/rds-aurora/aws` module with KMS encryption, Performance Insights, CloudWatch log exports, 35-day backup retention, and S3 lifecycle tiering. See `examples/05-aws-complete/database.tf`.

conftest-policies/json_policy.rego

@@ -0,0 +1,55 @@
+package main
+
+import rego.v1
+
+deny_iam_policy_document contains msg if {
+	some name
+	input.data.aws_iam_policy_document[name]
+	msg := sprintf(
+		"data.aws_iam_policy_document.%s: use jsonencode() for IAM policies instead of aws_iam_policy_document data sources",
+		[name],
+	)
+}
+
+policy_fields := {
+	"aws_iam_policy": ["policy"],
+	"aws_iam_role": ["assume_role_policy"],
+	"aws_iam_role_policy": ["policy"],
+	"aws_iam_group_policy": ["policy"],
+	"aws_iam_user_policy": ["policy"],
+	"aws_s3_bucket_policy": ["policy"],
+	"aws_s3_bucket": ["policy"],
+	"aws_s3_access_point": ["policy"],
+	"aws_s3control_access_point_policy": ["policy"],
+	"aws_sns_topic_policy": ["policy"],
+	"aws_sns_topic": ["policy"],
+	"aws_sqs_queue_policy": ["policy"],
+	"aws_sqs_queue": ["policy"],
+	"aws_kms_key": ["policy"],
+	"aws_kms_key_policy": ["policy"],
+	"aws_ecr_repository_policy": ["policy"],
+	"aws_secretsmanager_secret_policy": ["policy"],
+	"aws_cloudwatch_log_resource_policy": ["policy_document"],
+	"aws_opensearch_domain": ["access_policies"],
+	"aws_opensearch_domain_policy": ["access_policies"],
+	"aws_elasticsearch_domain_policy": ["access_policies"],
+	"aws_api_gateway_rest_api_policy": ["policy"],
+	"aws_glacier_vault": ["access_policy"],
+	"aws_media_store_container_policy": ["policy"],
+	"aws_backup_vault_policy": ["policy"],
+	"aws_efs_file_system_policy": ["policy"],
+}
+
+deny_heredoc_json_policy contains msg if {
+	some res_type, fields in policy_fields
+	some name
+	some block in input.resource[res_type][name]
+	some field in fields
+	value := object.get(block, field, "")
+	is_string(value)
+	startswith(trim_space(value), "{")
+	msg := sprintf(
+		"%s.%s: '%s' uses a raw JSON string — use jsonencode() instead",
+		[res_type, name, field],
+	)
+}

conftest-policies/json_policy_test.rego

@@ -0,0 +1,121 @@
+package main
+
+import rego.v1
+
+test_iam_policy_document_denied if {
+	result := deny_iam_policy_document with input as {"data": {"aws_iam_policy_document": {"my_policy": [{}]}}}
+	count(result) == 1
+}
+
+test_no_iam_policy_document_allowed if {
+	result := deny_iam_policy_document with input as {"data": {}}
+	count(result) == 0
+}
+
+test_other_data_source_allowed if {
+	result := deny_iam_policy_document with input as {"data": {"aws_caller_identity": {"current": [{}]}}}
+	count(result) == 0
+}
+
+test_heredoc_iam_policy_denied if {
+	result := deny_heredoc_json_policy with input as {"resource": {"aws_iam_policy": {"bad": [{"name": "test", "policy": "{\n  \"Version\": \"2012-10-17\"\n}"}]}}}
+	count(result) == 1
+}
+
+test_heredoc_iam_role_policy_denied if {
+	result := deny_heredoc_json_policy with input as {"resource": {"aws_iam_role_policy": {"bad": [{"name": "test", "role": "r", "policy": "{ \"Version\": \"2012-10-17\" }"}]}}}
+	count(result) == 1
+}
+
+test_heredoc_assume_role_policy_denied if {
+	result := deny_heredoc_json_policy with input as {"resource": {"aws_iam_role": {"bad": [{"name": "test", "assume_role_policy": "{\n  \"Version\": \"2012-10-17\"\n}"}]}}}
+	count(result) == 1
+}
+
+test_heredoc_s3_bucket_policy_denied if {
+	result := deny_heredoc_json_policy with input as {"resource": {"aws_s3_bucket_policy": {"bad": [{"bucket": "b", "policy": "{ \"Version\": \"2012-10-17\" }"}]}}}
+	count(result) == 1
+}
+
+test_heredoc_sns_topic_policy_denied if {
+	result := deny_heredoc_json_policy with input as {"resource": {"aws_sns_topic_policy": {"bad": [{"arn": "a", "policy": "{ \"Version\": \"2012-10-17\" }"}]}}}
+	count(result) == 1
+}
+
+test_heredoc_sqs_queue_policy_denied if {
+	result := deny_heredoc_json_policy with input as {"resource": {"aws_sqs_queue_policy": {"bad": [{"queue_url": "q", "policy": "{ \"Version\": \"2012-10-17\" }"}]}}}
+	count(result) == 1
+}
+
+test_heredoc_kms_key_denied if {
+	result := deny_heredoc_json_policy with input as {"resource": {"aws_kms_key": {"bad": [{"policy": "{ \"Version\": \"2012-10-17\" }"}]}}}
+	count(result) == 1
+}
+
+test_heredoc_ecr_repository_policy_denied if {
+	result := deny_heredoc_json_policy with input as {"resource": {"aws_ecr_repository_policy": {"bad": [{"repository": "r", "policy": "{ \"Version\": \"2012-10-17\" }"}]}}}
+	count(result) == 1
+}
+
+test_heredoc_opensearch_domain_denied if {
+	result := deny_heredoc_json_policy with input as {"resource": {"aws_opensearch_domain": {"bad": [{"domain_name": "d", "access_policies": "{ \"Version\": \"2012-10-17\" }"}]}}}
+	count(result) == 1
+}
+
+test_heredoc_cloudwatch_log_resource_policy_denied if {
+	result := deny_heredoc_json_policy with input as {"resource": {"aws_cloudwatch_log_resource_policy": {"bad": [{"policy_name": "p", "policy_document": "{ \"Version\": \"2012-10-17\" }"}]}}}
+	count(result) == 1
+}
+
+test_heredoc_glacier_vault_denied if {
+	result := deny_heredoc_json_policy with input as {"resource": {"aws_glacier_vault": {"bad": [{"name": "v", "access_policy": "{ \"Version\": \"2012-10-17\" }"}]}}}
+	count(result) == 1
+}
+
+test_jsonencode_iam_policy_allowed if {
+	result := deny_heredoc_json_policy with input as {"resource": {"aws_iam_policy": {"good": [{"name": "test", "policy": "${jsonencode({\n    Version = \"2012-10-17\"\n  })}"}]}}}
+	count(result) == 0
+}
+
+test_jsonencode_assume_role_allowed if {
+	result := deny_heredoc_json_policy with input as {"resource": {"aws_iam_role": {"good": [{"name": "test", "assume_role_policy": "${jsonencode({\n    Version = \"2012-10-17\"\n  })}"}]}}}
+	count(result) == 0
+}
+
+test_jsonencode_s3_bucket_policy_allowed if {
+	result := deny_heredoc_json_policy with input as {"resource": {"aws_s3_bucket_policy": {"good": [{"bucket": "b", "policy": "${jsonencode({\n    Version = \"2012-10-17\"\n  })}"}]}}}
+	count(result) == 0
+}
+
+test_jsonencode_kms_key_allowed if {
+	result := deny_heredoc_json_policy with input as {"resource": {"aws_kms_key": {"good": [{"policy": "${jsonencode({\n    Version = \"2012-10-17\"\n  })}"}]}}}
+	count(result) == 0
+}
+
+test_unrelated_resource_ignored if {
+	result := deny_heredoc_json_policy with input as {"resource": {"aws_instance": {"test": [{"ami": "ami-123"}]}}}
+	count(result) == 0
+}
+
+test_missing_field_ignored if {
+	result := deny_heredoc_json_policy with input as {"resource": {"aws_iam_policy": {"test": [{"name": "test"}]}}}
+	count(result) == 0
+}
+
+test_non_string_value_ignored if {
+	result := deny_heredoc_json_policy with input as {"resource": {"aws_iam_policy": {"test": [{"name": "test", "policy": 42}]}}}
+	count(result) == 0
+}
+
+test_empty_resource_block_ignored if {
+	result := deny_heredoc_json_policy with input as {"resource": {"aws_iam_policy": {"test": [{}]}}}
+	count(result) == 0
+}
+
+test_multiple_violations if {
+	result := deny_heredoc_json_policy with input as {"resource": {
+		"aws_iam_policy": {"a": [{"policy": "{ \"Version\": \"2012-10-17\" }"}]},
+		"aws_s3_bucket_policy": {"b": [{"policy": "{ \"Version\": \"2012-10-17\" }"}]},
+	}}
+	count(result) == 2
+}

conftest-policies/s3_lifecycle.rego

@@ -0,0 +1,56 @@
+package main
+
+import rego.v1
+
+deny_s3_lifecycle_prefix_in_locals contains msg if {
+	some block in input.locals
+	some name, rules in block
+	is_array(rules)
+	some rule in rules
+	is_lifecycle_rule(rule)
+	object.get(rule, "prefix", "__absent__") != "__absent__"
+	msg := sprintf(
+		"locals.%s: lifecycle rule '%s' has 'prefix' as a top-level key — move it inside a filter block: filter = { prefix = \"...\" }",
+		[name, rule.id],
+	)
+}
+
+deny_s3_lifecycle_prefix_in_module contains msg if {
+	some mod_name
+	some block in input.module[mod_name]
+	rules := block.lifecycle_rule
+	is_array(rules)
+	some rule in rules
+	is_lifecycle_rule(rule)
+	object.get(rule, "prefix", "__absent__") != "__absent__"
+	msg := sprintf(
+		"module.%s: lifecycle rule '%s' has 'prefix' as a top-level key — move it inside a filter block: filter = { prefix = \"...\" }",
+		[mod_name, rule.id],
+	)
+}
+
+deny_s3_lifecycle_prefix_in_resource contains msg if {
+	some name
+	some block in input.resource.aws_s3_bucket_lifecycle_configuration[name]
+	some rule in block.rule
+	object.get(rule, "prefix", "__absent__") != "__absent__"
+	msg := sprintf(
+		"aws_s3_bucket_lifecycle_configuration.%s: rule '%s' has 'prefix' as a top-level key — use a filter block instead",
+		[name, rule.id],
+	)
+}
+
+is_lifecycle_rule(rule) if {
+	rule.id
+	object.get(rule, "expiration", null) != null
+}
+
+is_lifecycle_rule(rule) if {
+	rule.id
+	object.get(rule, "transition", null) != null
+}
+
+is_lifecycle_rule(rule) if {
+	rule.id
+	object.get(rule, "noncurrent_version_expiration", null) != null
+}