Skip to content

feat: resolve webhook secrets from SSM and Secrets Manager #49

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

Open
mtweeman wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: resolve webhook secrets from SSM and Secrets Manager #49

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
22 changes: 21 additions & 1 deletion src/main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ module "repository" {
variables = local.variables
secrets = local.secrets
deploy_keys = var.deploy_keys
webhooks = var.webhooks
webhooks = local.webhooks
labels = var.labels
teams = var.teams
users = var.users
Expand Down Expand Up @@ -108,6 +108,20 @@ locals {
}
}

webhooks = {
for k, v in coalesce(var.webhooks, {}) : k => {
active = v.active
events = v.events
url = v.url
content_type = v.content_type
insecure_ssl = v.insecure_ssl
secret = v.secret != null ? (
startswith(v.secret, "ssm://") ? nonsensitive(data.aws_ssm_parameter.default[v.secret].value) :
startswith(v.secret, "asm://") ? nonsensitive(data.aws_secretsmanager_secret_version.default[v.secret].secret_string) : v.secret
) : null
}
}
Comment on lines +111 to +123
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Apr 21, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

Does Terraform's nonsensitive() function permanently remove the sensitive mark from aws_ssm_parameter value and aws_secretsmanager_secret_version secret_string when passed downstream to a module input?

💡 Result:

Yes, Terraform's nonsensitive function permanently removes the sensitive marking from the specified value, including when that value is passed downstream as a module input. The resulting value is treated as non-sensitive throughout the configuration graph, including in the child module where it becomes a regular (non-sensitive) variable value.

Citations:

🏁 Script executed:

#!/bin/bash
# Verify the code in src/main.tf and check the local.secrets pattern
echo "=== Lines 75-80 (local.secrets pattern) ==="
sed -n '75,80p' src/main.tf
echo ""
echo "=== Lines 111-123 (webhook secrets block) ==="
sed -n '111,123p' src/main.tf
echo ""
echo "=== Check variables.tf for webhooks definition ==="
grep -A 20 "variable.*webhooks" src/variables.tf | head -30

Repository: cloudposse-terraform-components/aws-github-repository

Length of output: 1725

Wrap webhook secret field in sensitive() instead of nonsensitive().

The webhooks block (lines 111–123) uses nonsensitive() to strip the sensitive marker from resolved SSM and ASM values. This contradicts the sensitive = true marking on var.webhooks in variables.tf and exposes resolved secrets in plan output and logs.

The local.secrets block above (lines 75–80) demonstrates the correct pattern: wrap the entire resolution in sensitive() instead of using nonsensitive(). Apply the same approach to the webhook secret field:

Fix 
   webhooks = {
     for k, v in coalesce(var.webhooks, {}) : k => {
       active       = v.active
       events       = v.events
       url          = v.url
       content_type = v.content_type
       insecure_ssl = v.insecure_ssl
-      secret = v.secret != null ? (
-        startswith(v.secret, "ssm://") ? nonsensitive(data.aws_ssm_parameter.default[v.secret].value) :
-        startswith(v.secret, "asm://") ? nonsensitive(data.aws_secretsmanager_secret_version.default[v.secret].secret_string) : v.secret
-      ) : null
+      secret = v.secret != null ? sensitive(
+        startswith(v.secret, "ssm://") ? data.aws_ssm_parameter.default[v.secret].value :
+        startswith(v.secret, "asm://") ? data.aws_secretsmanager_secret_version.default[v.secret].secret_string : v.secret
+      ) : null
     }
   }
📝 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.

Suggested change
webhooks = {
for k, v in coalesce(var.webhooks, {}) : k => {
active = v.active
events = v.events
url = v.url
content_type = v.content_type
insecure_ssl = v.insecure_ssl
secret = v.secret != null ? (
startswith(v.secret, "ssm://") ? nonsensitive(data.aws_ssm_parameter.default[v.secret].value) :
startswith(v.secret, "asm://") ? nonsensitive(data.aws_secretsmanager_secret_version.default[v.secret].secret_string) : v.secret
) : null
}
}
webhooks = {
for k, v in coalesce(var.webhooks, {}) : k => {
active = v.active
events = v.events
url = v.url
content_type = v.content_type
insecure_ssl = v.insecure_ssl
secret = v.secret != null ? sensitive(
startswith(v.secret, "ssm://") ? data.aws_ssm_parameter.default[v.secret].value :
startswith(v.secret, "asm://") ? data.aws_secretsmanager_secret_version.default[v.secret].secret_string : v.secret
) : null
}
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/main.tf` around lines 111 - 123, The webhook secret resolution currently
uses nonsensitive() to unwrap SSM/ASM values (in the webhooks mapping for
var.webhooks), which exposes secrets; change it to wrap the entire resolution in
sensitive() instead. Locate the webhooks block and the secret expression
referencing data.aws_ssm_parameter.default[...] and
data.aws_secretsmanager_secret_version.default[...] and replace the
nonsensitive(...) calls with sensitive(...) so the resolved SSM/ASM values
remain marked sensitive (preserving var.webhooks sensitive = true semantics) and
return null when v.secret is null.


ssm_parameters = merge(flatten([
[
{
Expand All @@ -116,6 +130,9 @@ locals {
{
for k, v in coalesce(var.secrets, {}) : v => trimprefix(v, "ssm://") if startswith(v, "ssm://")
},
{
for k, v in coalesce(var.webhooks, {}) : v.secret => trimprefix(v.secret, "ssm://") if v.secret != null && startswith(v.secret, "ssm://")
},
],
[
for k, v in coalesce(var.environments, {}) : {
Expand All @@ -138,6 +155,9 @@ locals {
{
for k, v in coalesce(var.secrets, {}) : v => trimprefix(v, "asm://") if startswith(v, "asm://")
},
{
for k, v in coalesce(var.webhooks, {}) : v.secret => trimprefix(v.secret, "asm://") if v.secret != null && startswith(v.secret, "asm://")
},
],
[
for k, v in coalesce(var.environments, {}) : {
Expand Down
5 changes: 3 additions & 2 deletions src/variables.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,8 +224,9 @@ variable "webhooks" {
insecure_ssl = optional(bool, false)
secret = optional(string, null)
}))
default = {}
nullable = false
default = {}
sensitive = true
nullable = false

validation {
condition = alltrue([for k, v in var.webhooks : can(regex("^http(s)?://", v.url))])
Expand Down
Loading