From 6a4569344e41e4dc27426200f04758d0bd03f601 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Sat, 25 Apr 2026 14:45:55 +0200 Subject: [PATCH 1/4] fix(security/azure): migrate Logic App SCHEDULED_TASK_SECRET to Key Vault connection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Logic App workflows that fan out scheduled task calls to the Container App were still embedding `${var.scheduled_task_secret}` plaintext in their outgoing `Authorization: Bearer ...` headers (3 sites in scheduled-tasks.tf). The secret therefore lived in: - the Logic App workflow definition stored in Azure Resource Manager, - the Terraform state file, - any captured `terraform plan/show` output (CI logs, operator screens). Migrate to Azure Logic Apps Key Vault references via system-assigned managed identity: - Each workflow (recommendations, ri_exchange, cleanup) gets a system-assigned managed identity granted "Key Vault Secrets User" on the existing Key Vault. - A new `get-secret` action (custom HTTP action with `authentication.type = ManagedServiceIdentity`, audience `https://vault.azure.net`) fetches scheduled-task-secret from KV at workflow runtime via the data-plane REST API. - The call-endpoint action references `@body('get-secret')['value']` in its outgoing Authorization header. The plaintext is never interpolated into the workflow definition or Terraform state. - `runtimeConfiguration.secureData` masks the secret in workflow run history (outputs of get-secret, inputs of the downstream call). Module API change: `var.scheduled_task_secret` (sensitive value) is replaced with `var.scheduled_task_secret_name` (just the KV secret name). Env-level caller updated to pass `module.secrets.scheduled_task_secret_name` instead of `.scheduled_task_secret_value`. The KV access uses the existing `var.key_vault_id` and `var.key_vault_uri` already plumbed for the Container App's runtime identity — no new env-level wiring required. The deferred section in known_issues/18_tf_env_azure.md is updated to reflect that the Logic App leak is now closed. Closes #50 --- known_issues/18_tf_env_azure.md | 7 +- .../environments/azure/.terraform.lock.hcl | 20 ++ terraform/environments/azure/compute.tf | 19 +- .../azure/container-apps/scheduled-tasks.tf | 271 +++++++++++++++--- .../compute/azure/container-apps/variables.tf | 5 +- 5 files changed, 266 insertions(+), 56 deletions(-) diff --git a/known_issues/18_tf_env_azure.md b/known_issues/18_tf_env_azure.md index 84d908cf..cb50e35a 100644 --- a/known_issues/18_tf_env_azure.md +++ b/known_issues/18_tf_env_azure.md @@ -95,18 +95,17 @@ **Effort:** `small` (contingent on triage) -## ~~HIGH: `SCHEDULED_TASK_SECRET` injected as plain-text environment variable~~ — PARTIALLY RESOLVED +## ~~HIGH: `SCHEDULED_TASK_SECRET` injected as plain-text environment variable~~ — RESOLVED **File**: `terraform/environments/azure/compute.tf:42` **Description**: The scheduled-task shared secret was injected into the Container App as a plaintext env var, visible via `az containerapp show`, the Azure Portal, and exported ARM templates. -**Status:** ✔️ Partially resolved — runtime env-var leak closed; Logic App path deferred. +**Status:** ✔️ Fully resolved — runtime env-var leak closed AND Logic App path migrated to Key Vault references (issue #50). **Resolved by:** - Container App env var switched from `SCHEDULED_TASK_SECRET = ` to `SCHEDULED_TASK_SECRET_NAME = `. `az containerapp show` now reveals only the secret name. - `ApplicationConfig.ScheduledTaskSecretName` added. `NewApplicationFromDeps` resolves it via the existing `SecretResolver` (Azure Key Vault / AWS Secrets Manager) at startup and populates `ScheduledTaskSecret` in memory. Falls back to the plaintext `SCHEDULED_TASK_SECRET` env var if the lookup fails or the resolver isn't configured, so dev/local runs still work. - -**Deferred:** The Logic App workflow (`scheduled-tasks.tf:48,106`) still interpolates `var.scheduled_task_secret` into its outgoing `Authorization: Bearer …` header. This value is stored inside the Logic App resource + Terraform state; removing it requires migrating to Azure Logic Apps Key Vault connections (`@parameters('kv-secret')`), a larger refactor. The container-app side — which is what was actually flagged in the audit — no longer leaks. +- Logic App workflows now have a system-assigned managed identity granted "Key Vault Secrets User" on the vault; each workflow's first action GETs the secret from KV at runtime via that identity, and the call-endpoint action references `@body('get-secret')['value']` in its outgoing Authorization header. The plaintext value no longer lives in the workflow definition or Terraform state. See `terraform/modules/compute/azure/container-apps/scheduled-tasks.tf` and PR resolving #50. ### Original implementation plan diff --git a/terraform/environments/azure/.terraform.lock.hcl b/terraform/environments/azure/.terraform.lock.hcl index 7261db99..8f9ec233 100644 --- a/terraform/environments/azure/.terraform.lock.hcl +++ b/terraform/environments/azure/.terraform.lock.hcl @@ -159,3 +159,23 @@ provider "registry.terraform.io/hashicorp/random" { "zh:f49fd62aa8c5525a5c17abd51e27ca5e213881d58882fd42fec4a545b53c9699", ] } + +provider "registry.terraform.io/hashicorp/time" { + version = "0.13.1" + constraints = "~> 0.11" + hashes = [ + "h1:ZT5ppCNIModqk3iOkVt5my8b8yBHmDpl663JtXAIRqM=", + "zh:02cb9aab1002f0f2a94a4f85acec8893297dc75915f7404c165983f720a54b74", + "zh:04429b2b31a492d19e5ecf999b116d396dac0b24bba0d0fb19ecaefe193fdb8f", + "zh:26f8e51bb7c275c404ba6028c1b530312066009194db721a8427a7bc5cdbc83a", + "zh:772ff8dbdbef968651ab3ae76d04afd355c32f8a868d03244db3f8496e462690", + "zh:78d5eefdd9e494defcb3c68d282b8f96630502cac21d1ea161f53cfe9bb483b3", + "zh:898db5d2b6bd6ca5457dccb52eedbc7c5b1a71e4a4658381bcbb38cedbbda328", + "zh:8de913bf09a3fa7bedc29fec18c47c571d0c7a3d0644322c46f3aa648cf30cd8", + "zh:9402102c86a87bdfe7e501ffbb9c685c32bbcefcfcf897fd7d53df414c36877b", + "zh:b18b9bb1726bb8cfbefc0a29cf3657c82578001f514bcf4c079839b6776c47f0", + "zh:b9d31fdc4faecb909d7c5ce41d2479dd0536862a963df434be4b16e8e4edc94d", + "zh:c951e9f39cca3446c060bd63933ebb89cedde9523904813973fbc3d11863ba75", + "zh:e5b773c0d07e962291be0e9b413c7a22c044b8c7b58c76e8aa91d1659990dfb5", + ] +} diff --git a/terraform/environments/azure/compute.tf b/terraform/environments/azure/compute.tf index 51f1bc9d..25d20b17 100644 --- a/terraform/environments/azure/compute.tf +++ b/terraform/environments/azure/compute.tf @@ -57,17 +57,14 @@ module "compute_container_apps" { # Scheduled tasks (Logic Apps) # - # scheduled_task_secret still passes the plaintext value here because the - # Logic App workflow definition embeds it in its outgoing "Authorization: - # Bearer ..." header (see scheduled-tasks.tf). Azure Logic Apps DO support - # Key Vault references via @parameters() + a Key Vault connection, but - # that's a larger refactor. This value is stored in Terraform state and - # in the Logic App resource, but is no longer exposed in the Container - # App's env vars — `az containerapp show` now reveals only the secret - # name, not the value. - enable_scheduled_tasks = var.enable_scheduled_tasks - scheduled_task_secret = module.secrets.scheduled_task_secret_value - recommendation_schedule = var.recommendation_schedule + # Each Logic App workflow has a system-assigned managed identity that holds + # "Key Vault Secrets User" on the same Key Vault as the Container App. The + # workflow's first action GETs scheduled-task-secret from KV via that + # identity at runtime; the value never lands in the workflow definition or + # Terraform state. We only pass the *name* of the secret, not the value. + enable_scheduled_tasks = var.enable_scheduled_tasks + scheduled_task_secret_name = module.secrets.scheduled_task_secret_name + recommendation_schedule = var.recommendation_schedule # RI exchange automation enable_ri_exchange_schedule = var.enable_ri_exchange_schedule diff --git a/terraform/modules/compute/azure/container-apps/scheduled-tasks.tf b/terraform/modules/compute/azure/container-apps/scheduled-tasks.tf index 0108942c..48684d3e 100644 --- a/terraform/modules/compute/azure/container-apps/scheduled-tasks.tf +++ b/terraform/modules/compute/azure/container-apps/scheduled-tasks.tf @@ -1,5 +1,17 @@ # Azure Logic Apps for scheduled tasks on Container Apps # This is the Azure equivalent of AWS EventBridge + Lambda or GCP Cloud Scheduler + Cloud Run +# +# SECURITY: The shared scheduled-task secret is NEVER interpolated into the +# workflow definition or Terraform state. Each Logic App workflow has a +# system-assigned managed identity that holds "Key Vault Secrets User" on the +# vault that stores `scheduled-task-secret`. At workflow runtime the first +# action (`get-secret`) calls the Key Vault data-plane REST API authenticated +# by the workflow's managed identity, and the call-endpoint action references +# `@body('get-secret')['value']` in the outgoing Authorization header. +# +# Effect: `terraform show` / `az logicapp show` only ever reveal the Key Vault +# URL the workflow is going to call — the actual secret value is fetched +# in-process by the Logic Apps engine and never lands in any persisted artifact. # Parse cron schedule into Logic Apps recurrence format # Azure Logic Apps uses a different format than cron @@ -8,9 +20,19 @@ locals { # For simplicity, support daily schedules at a specific hour # Full cron parsing would require more complex logic schedule_hour = var.enable_scheduled_tasks ? split(" ", var.recommendation_schedule)[1] : "2" + + # Data-plane URL of the scheduled-task secret in Key Vault. The Logic App + # workflows fetch this at runtime via managed identity. `key_vault_uri` + # already includes the trailing slash (e.g. https://.vault.azure.net/). + scheduled_task_secret_url = ( + var.enable_scheduled_tasks || var.enable_ri_exchange_schedule + ) ? "${var.key_vault_uri}secrets/${var.scheduled_task_secret_name}?api-version=7.4" : "" } +# ============================================== # Logic App workflow for recommendations refresh +# ============================================== + resource "azurerm_logic_app_workflow" "recommendations" { count = var.enable_scheduled_tasks ? 1 : 0 @@ -18,6 +40,12 @@ resource "azurerm_logic_app_workflow" "recommendations" { location = var.location resource_group_name = var.resource_group_name + # System-assigned managed identity used to read the shared secret from + # Key Vault at workflow runtime. See header comment. + identity { + type = "SystemAssigned" + } + tags = var.tags } @@ -33,27 +61,71 @@ resource "azurerm_logic_app_trigger_recurrence" "daily" { time_zone = "UTC" } -# HTTP action to call Container App endpoint -resource "azurerm_logic_app_action_http" "call_recommendations" { +# Step 1: Fetch the shared secret from Key Vault using the workflow's +# system-assigned managed identity. The secret value lives in the workflow +# run's transient state only — never in the workflow definition or TF state. +resource "azurerm_logic_app_action_custom" "recommendations_get_secret" { count = var.enable_scheduled_tasks ? 1 : 0 - name = "call-recommendations-endpoint" + name = "get-secret" logic_app_id = azurerm_logic_app_workflow.recommendations[0].id - method = "POST" - uri = "https://${azurerm_container_app.main.ingress[0].fqdn}/api/scheduled/recommendations" + body = jsonencode({ + type = "Http" + inputs = { + method = "GET" + uri = local.scheduled_task_secret_url + authentication = { + type = "ManagedServiceIdentity" + audience = "https://vault.azure.net" + } + } + runAfter = {} + runtimeConfiguration = { + secureData = { + properties = ["outputs"] + } + } + }) +} - headers = { - "Content-Type" = "application/json" - "Authorization" = "Bearer ${var.scheduled_task_secret}" - "X-Trigger" = "scheduled" - "X-Source" = "azure-logic-apps" - } +# Step 2: Call the Container App scheduled-recommendations endpoint, using +# the secret pulled by the previous action. `@body('get-secret')['value']` is +# evaluated by the Logic Apps engine at runtime; it is never expanded into +# the persisted workflow definition. +resource "azurerm_logic_app_action_custom" "call_recommendations" { + count = var.enable_scheduled_tasks ? 1 : 0 + + name = "call-recommendations-endpoint" + logic_app_id = azurerm_logic_app_workflow.recommendations[0].id body = jsonencode({ - source = "azure-logic-apps" - timestamp = "@{utcNow()}" + type = "Http" + inputs = { + method = "POST" + uri = "https://${azurerm_container_app.main.ingress[0].fqdn}/api/scheduled/recommendations" + headers = { + "Content-Type" = "application/json" + "Authorization" = "Bearer @{body('get-secret')['value']}" + "X-Trigger" = "scheduled" + "X-Source" = "azure-logic-apps" + } + body = { + source = "azure-logic-apps" + timestamp = "@{utcNow()}" + } + } + runAfter = { + "get-secret" = ["Succeeded"] + } + runtimeConfiguration = { + secureData = { + properties = ["inputs"] + } + } }) + + depends_on = [azurerm_logic_app_action_custom.recommendations_get_secret] } # ============================================== @@ -78,6 +150,10 @@ resource "azurerm_logic_app_workflow" "ri_exchange" { location = var.location resource_group_name = var.resource_group_name + identity { + type = "SystemAssigned" + } + tags = var.tags } @@ -92,29 +168,70 @@ resource "azurerm_logic_app_trigger_recurrence" "ri_exchange" { time_zone = "UTC" } -resource "azurerm_logic_app_action_http" "call_ri_exchange" { +resource "azurerm_logic_app_action_custom" "ri_exchange_get_secret" { count = var.enable_ri_exchange_schedule ? 1 : 0 - name = "call-ri-exchange-endpoint" + name = "get-secret" logic_app_id = azurerm_logic_app_workflow.ri_exchange[0].id - method = "POST" - uri = "https://${azurerm_container_app.main.ingress[0].fqdn}/api/scheduled/ri-exchange" + body = jsonencode({ + type = "Http" + inputs = { + method = "GET" + uri = local.scheduled_task_secret_url + authentication = { + type = "ManagedServiceIdentity" + audience = "https://vault.azure.net" + } + } + runAfter = {} + runtimeConfiguration = { + secureData = { + properties = ["outputs"] + } + } + }) +} - headers = { - "Content-Type" = "application/json" - "Authorization" = "Bearer ${var.scheduled_task_secret}" - "X-Trigger" = "scheduled" - "X-Source" = "azure-logic-apps" - } +resource "azurerm_logic_app_action_custom" "call_ri_exchange" { + count = var.enable_ri_exchange_schedule ? 1 : 0 + + name = "call-ri-exchange-endpoint" + logic_app_id = azurerm_logic_app_workflow.ri_exchange[0].id body = jsonencode({ - source = "azure-logic-apps" - timestamp = "@{utcNow()}" + type = "Http" + inputs = { + method = "POST" + uri = "https://${azurerm_container_app.main.ingress[0].fqdn}/api/scheduled/ri-exchange" + headers = { + "Content-Type" = "application/json" + "Authorization" = "Bearer @{body('get-secret')['value']}" + "X-Trigger" = "scheduled" + "X-Source" = "azure-logic-apps" + } + body = { + source = "azure-logic-apps" + timestamp = "@{utcNow()}" + } + } + runAfter = { + "get-secret" = ["Succeeded"] + } + runtimeConfiguration = { + secureData = { + properties = ["inputs"] + } + } }) + + depends_on = [azurerm_logic_app_action_custom.ri_exchange_get_secret] } +# ============================================== # Logic App workflow for cleanup (sessions and executions) +# ============================================== + resource "azurerm_logic_app_workflow" "cleanup" { count = var.enable_scheduled_tasks ? 1 : 0 @@ -122,6 +239,10 @@ resource "azurerm_logic_app_workflow" "cleanup" { location = var.location resource_group_name = var.resource_group_name + identity { + type = "SystemAssigned" + } + tags = var.tags } @@ -137,25 +258,99 @@ resource "azurerm_logic_app_trigger_recurrence" "cleanup_daily" { time_zone = "UTC" } -# HTTP action to call cleanup endpoint -resource "azurerm_logic_app_action_http" "call_cleanup" { +resource "azurerm_logic_app_action_custom" "cleanup_get_secret" { count = var.enable_scheduled_tasks ? 1 : 0 - name = "call-cleanup-endpoint" + name = "get-secret" logic_app_id = azurerm_logic_app_workflow.cleanup[0].id - method = "POST" - uri = "https://${azurerm_container_app.main.ingress[0].fqdn}/api/scheduled/cleanup" + body = jsonencode({ + type = "Http" + inputs = { + method = "GET" + uri = local.scheduled_task_secret_url + authentication = { + type = "ManagedServiceIdentity" + audience = "https://vault.azure.net" + } + } + runAfter = {} + runtimeConfiguration = { + secureData = { + properties = ["outputs"] + } + } + }) +} - headers = { - "Content-Type" = "application/json" - "Authorization" = "Bearer ${var.scheduled_task_secret}" - "X-Trigger" = "scheduled" - "X-Source" = "azure-logic-apps" - } +resource "azurerm_logic_app_action_custom" "call_cleanup" { + count = var.enable_scheduled_tasks ? 1 : 0 + + name = "call-cleanup-endpoint" + logic_app_id = azurerm_logic_app_workflow.cleanup[0].id body = jsonencode({ - dryRun = false - source = "azure-logic-apps" + type = "Http" + inputs = { + method = "POST" + uri = "https://${azurerm_container_app.main.ingress[0].fqdn}/api/scheduled/cleanup" + headers = { + "Content-Type" = "application/json" + "Authorization" = "Bearer @{body('get-secret')['value']}" + "X-Trigger" = "scheduled" + "X-Source" = "azure-logic-apps" + } + body = { + dryRun = false + source = "azure-logic-apps" + } + } + runAfter = { + "get-secret" = ["Succeeded"] + } + runtimeConfiguration = { + secureData = { + properties = ["inputs"] + } + } }) + + depends_on = [azurerm_logic_app_action_custom.cleanup_get_secret] +} + +# ============================================== +# RBAC: grant each Logic App's managed identity read access to the +# scheduled-task secret in Key Vault. +# ============================================== + +# Use a single role assignment per workflow rather than per secret. The grant +# is "Key Vault Secrets User" (read-only) scoped to the whole vault, matching +# the same pattern used by the container app's runtime identity. Vault-scoped +# read access is acceptable here because each workflow only ever reads one +# specific secret URL embedded in its definition; adding a per-secret RBAC +# scope would require splitting the vault, which is out of scope for this +# change. + +resource "azurerm_role_assignment" "recommendations_kv_secrets_user" { + count = var.enable_scheduled_tasks ? 1 : 0 + + scope = var.key_vault_id + role_definition_name = "Key Vault Secrets User" + principal_id = azurerm_logic_app_workflow.recommendations[0].identity[0].principal_id +} + +resource "azurerm_role_assignment" "ri_exchange_kv_secrets_user" { + count = var.enable_ri_exchange_schedule ? 1 : 0 + + scope = var.key_vault_id + role_definition_name = "Key Vault Secrets User" + principal_id = azurerm_logic_app_workflow.ri_exchange[0].identity[0].principal_id +} + +resource "azurerm_role_assignment" "cleanup_kv_secrets_user" { + count = var.enable_scheduled_tasks ? 1 : 0 + + scope = var.key_vault_id + role_definition_name = "Key Vault Secrets User" + principal_id = azurerm_logic_app_workflow.cleanup[0].identity[0].principal_id } diff --git a/terraform/modules/compute/azure/container-apps/variables.tf b/terraform/modules/compute/azure/container-apps/variables.tf index fed6f9ad..22147669 100644 --- a/terraform/modules/compute/azure/container-apps/variables.tf +++ b/terraform/modules/compute/azure/container-apps/variables.tf @@ -204,11 +204,10 @@ variable "enable_scheduled_tasks" { default = true } -variable "scheduled_task_secret" { - description = "Shared secret for authenticating scheduled task HTTP calls" +variable "scheduled_task_secret_name" { + description = "Key Vault secret name (NOT the value) holding the shared secret for authenticating scheduled task HTTP calls. The Logic App workflows fetch this secret at runtime via their managed identity, so the plaintext never lands in the workflow definition or Terraform state." type = string default = "" - sensitive = true } variable "recommendation_schedule" { From 7833f29bf1da70cd4732d2fc6909e47b62164e81 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Sat, 25 Apr 2026 15:07:24 +0200 Subject: [PATCH 2/4] fix(security/azure): address CodeRabbit review on PR #74 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Apply three actionable items from CodeRabbit's review of the Logic App Key Vault migration: 1. **Plan-time precondition guard** (scheduled-tasks.tf): an empty `var.scheduled_task_secret_name` would silently turn the KV URL into `/secrets/?api-version=7.4` (the list-secrets endpoint), which the workflow's managed identity would 403 against at runtime — a late, hard-to-diagnose failure. Add a `terraform_data` resource with two preconditions that fail at plan/apply when scheduled tasks are enabled but the secret name is empty or `key_vault_uri` is missing its trailing slash. 2. **RBAC propagation `depends_on`** (scheduled-tasks.tf): each `*_get_secret` custom action now `depends_on` its corresponding `*_kv_secrets_user` role assignment. Azure RBAC propagation can take 5–10 minutes after `azurerm_role_assignment` apply; without the dependency Terraform may schedule them in parallel, so a manual "trigger now" right after apply could 403 until propagation completes. The 02:00/03:00 UTC scheduled runs almost always fall after propagation, but this keeps the post-apply runbook deterministic. 3. **known_issues audit roll-up** (18_tf_env_azure.md): bump the summary counter from `7 resolved · 1 partially fixed` to `8 resolved · 0 partially fixed` to match the now-resolved scheduled-task-secret entry, and stamp the audit-status date to today (2026-04-25). Variable description for `scheduled_task_secret_name` updated to mention the new precondition. Refs #50, PR #74. --- known_issues/18_tf_env_azure.md | 2 +- .../azure/container-apps/scheduled-tasks.tf | 33 +++++++++++++++++++ .../compute/azure/container-apps/variables.tf | 2 +- 3 files changed, 35 insertions(+), 2 deletions(-) diff --git a/known_issues/18_tf_env_azure.md b/known_issues/18_tf_env_azure.md index cb50e35a..84723138 100644 --- a/known_issues/18_tf_env_azure.md +++ b/known_issues/18_tf_env_azure.md @@ -1,6 +1,6 @@ # Known Issues: Terraform Azure Environment -> **Audit status (2026-04-20):** `0 still valid · 7 resolved · 1 partially fixed · 0 moved · 0 needs triage` +> **Audit status (2026-04-25):** `0 still valid · 8 resolved · 0 partially fixed · 0 moved · 0 needs triage` ## ~~CRITICAL: `nonsensitive()` strips sensitivity from `additional_secrets` before merge~~ — RESOLVED diff --git a/terraform/modules/compute/azure/container-apps/scheduled-tasks.tf b/terraform/modules/compute/azure/container-apps/scheduled-tasks.tf index 48684d3e..ece7e0b4 100644 --- a/terraform/modules/compute/azure/container-apps/scheduled-tasks.tf +++ b/terraform/modules/compute/azure/container-apps/scheduled-tasks.tf @@ -29,6 +29,27 @@ locals { ) ? "${var.key_vault_uri}secrets/${var.scheduled_task_secret_name}?api-version=7.4" : "" } +# Plan-time guard: if any scheduled-task workflow is enabled, the secret name +# and key vault URI must be set correctly. Without these checks, an empty +# scheduled_task_secret_name silently produces `/secrets/?api-version=...` +# (the list-secrets endpoint), and a key_vault_uri without a trailing slash +# breaks the URL. Both surface late as runtime 401/403; the precondition +# fails them at plan/apply instead. +resource "terraform_data" "scheduled_task_secret_preconditions" { + count = (var.enable_scheduled_tasks || var.enable_ri_exchange_schedule) ? 1 : 0 + + lifecycle { + precondition { + condition = length(var.scheduled_task_secret_name) > 0 + error_message = "scheduled_task_secret_name must be set when enable_scheduled_tasks or enable_ri_exchange_schedule is true." + } + precondition { + condition = endswith(var.key_vault_uri, "/") + error_message = "key_vault_uri must end with '/' (e.g. https://.vault.azure.net/)." + } + } +} + # ============================================== # Logic App workflow for recommendations refresh # ============================================== @@ -87,6 +108,12 @@ resource "azurerm_logic_app_action_custom" "recommendations_get_secret" { } } }) + + # Ensure the role assignment exists before this action so the very first + # post-apply manual run doesn't 403 while RBAC propagation completes. + # Scheduled runs (next 02:00 UTC) almost certainly fall after propagation, + # but this keeps `terraform apply && trigger now` deterministic. + depends_on = [azurerm_role_assignment.recommendations_kv_secrets_user] } # Step 2: Call the Container App scheduled-recommendations endpoint, using @@ -191,6 +218,9 @@ resource "azurerm_logic_app_action_custom" "ri_exchange_get_secret" { } } }) + + # See recommendations_get_secret.depends_on rationale. + depends_on = [azurerm_role_assignment.ri_exchange_kv_secrets_user] } resource "azurerm_logic_app_action_custom" "call_ri_exchange" { @@ -281,6 +311,9 @@ resource "azurerm_logic_app_action_custom" "cleanup_get_secret" { } } }) + + # See recommendations_get_secret.depends_on rationale. + depends_on = [azurerm_role_assignment.cleanup_kv_secrets_user] } resource "azurerm_logic_app_action_custom" "call_cleanup" { diff --git a/terraform/modules/compute/azure/container-apps/variables.tf b/terraform/modules/compute/azure/container-apps/variables.tf index 22147669..1d53a682 100644 --- a/terraform/modules/compute/azure/container-apps/variables.tf +++ b/terraform/modules/compute/azure/container-apps/variables.tf @@ -205,7 +205,7 @@ variable "enable_scheduled_tasks" { } variable "scheduled_task_secret_name" { - description = "Key Vault secret name (NOT the value) holding the shared secret for authenticating scheduled task HTTP calls. The Logic App workflows fetch this secret at runtime via their managed identity, so the plaintext never lands in the workflow definition or Terraform state." + description = "Key Vault secret name (NOT the value) holding the shared secret for authenticating scheduled task HTTP calls. The Logic App workflows fetch this secret at runtime via their managed identity, so the plaintext never lands in the workflow definition or Terraform state. Must be non-empty when enable_scheduled_tasks or enable_ri_exchange_schedule is true (enforced by a precondition in scheduled-tasks.tf)." type = string default = "" } From ceb665024fc39c2410f9deb1a51e420991a1dcdd Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Mon, 27 Apr 2026 19:19:49 +0200 Subject: [PATCH 3/4] refactor(azure): add char-set validation on scheduled_task_secret_name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit flagged the missing variable-level validation alongside the existing precondition. Their literal suggestion required `var != ""` unconditionally, which would fail plan for callers that legitimately disable both `enable_scheduled_tasks` and `enable_ri_exchange_schedule` (the variable's default is `""`). Variable validation can't reference sibling variables so the "is non-empty when needed" rule must stay in the resource precondition in scheduled-tasks.tf. Add a narrower validation that only fires when a value is supplied: it must be a bare Key Vault secret name with no `/`, `?`, `#`, or whitespace. The original concern was that an empty value silently produced the list-secrets endpoint URL — that's fully covered by the precondition. The new validation closes the secondary risk that a caller passes a value containing path/query/fragment characters and escapes the `${key_vault_uri}secrets/${name}?api-version=…` URL template. `terraform validate` clean. --- .../compute/azure/container-apps/variables.tf | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/terraform/modules/compute/azure/container-apps/variables.tf b/terraform/modules/compute/azure/container-apps/variables.tf index 1d53a682..4907e46a 100644 --- a/terraform/modules/compute/azure/container-apps/variables.tf +++ b/terraform/modules/compute/azure/container-apps/variables.tf @@ -208,6 +208,18 @@ variable "scheduled_task_secret_name" { description = "Key Vault secret name (NOT the value) holding the shared secret for authenticating scheduled task HTTP calls. The Logic App workflows fetch this secret at runtime via their managed identity, so the plaintext never lands in the workflow definition or Terraform state. Must be non-empty when enable_scheduled_tasks or enable_ri_exchange_schedule is true (enforced by a precondition in scheduled-tasks.tf)." type = string default = "" + + # Validation runs unconditionally on the variable, so we can't predicate it + # on enable_scheduled_tasks / enable_ri_exchange_schedule (variables can't + # see other variables here). The "is non-empty when needed" rule lives in + # scheduled-tasks.tf as a resource precondition. This block only checks the + # character set: if a value is supplied, it must be a bare Key Vault secret + # name and not a path/query/fragment fragment that would let it escape the + # `${key_vault_uri}secrets/${name}?api-version=…` URL template. + validation { + condition = var.scheduled_task_secret_name == "" || !can(regex("[/?# ]", var.scheduled_task_secret_name)) + error_message = "scheduled_task_secret_name must be a bare Key Vault secret name (no '/', '?', '#', or whitespace)." + } } variable "recommendation_schedule" { From 3b467a1e1f5d74d9ab31b245fadd9dfb3e84145a Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Mon, 27 Apr 2026 19:40:13 +0200 Subject: [PATCH 4/4] docs(azure): cross-reference validation/precondition split for scheduled-task secret Make the rule split between `variables.tf` and `scheduled-tasks.tf` self- documenting. The variable's `validation` block can only handle the character-set half (because variable validation runs unconditionally and can't see other variables); the conditional non-empty rule lives on the precondition resource. Comments at both ends now point at each other so a reader landing on either file knows where the other half is, and why they're not unified. Tightened the variable comment in passing (typo'd "fragment fragment"). No functional change. --- .../compute/azure/container-apps/scheduled-tasks.tf | 7 +++++++ .../compute/azure/container-apps/variables.tf | 12 ++++++------ 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/terraform/modules/compute/azure/container-apps/scheduled-tasks.tf b/terraform/modules/compute/azure/container-apps/scheduled-tasks.tf index ece7e0b4..cd295e59 100644 --- a/terraform/modules/compute/azure/container-apps/scheduled-tasks.tf +++ b/terraform/modules/compute/azure/container-apps/scheduled-tasks.tf @@ -35,6 +35,13 @@ locals { # (the list-secrets endpoint), and a key_vault_uri without a trailing slash # breaks the URL. Both surface late as runtime 401/403; the precondition # fails them at plan/apply instead. +# +# Why a precondition rather than `validation` blocks on the variables: variable +# validation can only reference its own `var.` and runs unconditionally, +# so it can't say "non-empty WHEN the schedule is enabled" without breaking +# legitimate disabled-by-default callers. The character-set half of the rule +# (no `/?# ` in the secret name) does live on the variable itself — see the +# `validation` block on `scheduled_task_secret_name` in variables.tf. resource "terraform_data" "scheduled_task_secret_preconditions" { count = (var.enable_scheduled_tasks || var.enable_ri_exchange_schedule) ? 1 : 0 diff --git a/terraform/modules/compute/azure/container-apps/variables.tf b/terraform/modules/compute/azure/container-apps/variables.tf index 4907e46a..b493154a 100644 --- a/terraform/modules/compute/azure/container-apps/variables.tf +++ b/terraform/modules/compute/azure/container-apps/variables.tf @@ -209,12 +209,12 @@ variable "scheduled_task_secret_name" { type = string default = "" - # Validation runs unconditionally on the variable, so we can't predicate it - # on enable_scheduled_tasks / enable_ri_exchange_schedule (variables can't - # see other variables here). The "is non-empty when needed" rule lives in - # scheduled-tasks.tf as a resource precondition. This block only checks the - # character set: if a value is supplied, it must be a bare Key Vault secret - # name and not a path/query/fragment fragment that would let it escape the + # Validation runs unconditionally and can't reference sibling variables, so + # the "non-empty when scheduled tasks are enabled" rule lives in the + # `terraform_data.scheduled_task_secret_preconditions` resource in + # scheduled-tasks.tf. This block handles only the character-set half: if a + # value is supplied, it must be a bare Key Vault secret name — no `/`, `?`, + # `#`, or whitespace — so it can't escape the # `${key_vault_uri}secrets/${name}?api-version=…` URL template. validation { condition = var.scheduled_task_secret_name == "" || !can(regex("[/?# ]", var.scheduled_task_secret_name))