fix(security/azure): migrate Logic App SCHEDULED_TASK_SECRET to Key Vault connection

[cristim]

Summary

Closes #50.

The Logic App workflows that fan out scheduled task calls to the Container App were still embedding the plaintext ${var.scheduled_task_secret} in their outgoing Authorization: Bearer ... headers (3 sites in terraform/modules/compute/azure/container-apps/scheduled-tasks.tf). The secret therefore lived in the Logic App workflow definition (visible via az logicapp show), the Terraform state file, and any captured terraform plan/show output (CI logs, operator screens) — even after the Container App side was migrated to a Key Vault reference.

This PR migrates all three workflows (recommendations, ri_exchange, cleanup) to fetch the secret from Key Vault at runtime via system-assigned managed identity:

• Each azurerm_logic_app_workflow gets identity { type = "SystemAssigned" } and is granted Key Vault Secrets User on the existing Key Vault (var.key_vault_id, already plumbed for the Container App's runtime identity — no env-level wiring required).
• A new get-secret action (custom HTTP action, authentication.type = ManagedServiceIdentity, audience https://vault.azure.net) calls the Key Vault data-plane REST API at workflow runtime.
• 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 (terraform/environments/azure/compute.tf) updated to pass module.secrets.scheduled_task_secret_name instead of .scheduled_task_secret_value.

known_issues/18_tf_env_azure.md is updated — the Logic App deferred section is now closed; the original audit finding is fully resolved on Azure.

Decision: full migration (no canary)

The 3 sites are structurally identical (same secret URL, same auth model, same managed-identity grant). Splitting would have created two near-identical migration paths to maintain — riskier than landing them together. Local terraform validate passes for both the module and the env.

Verification done

• terraform fmt -recursive -check — passed.
• terraform validate in terraform/modules/compute/azure/container-apps/ — passed.
• terraform validate in terraform/environments/azure/ — passed.
• Pre-commit hooks (terraform_validate, terraform_fmt, tflint, secrets scan, trivy config) — all passed.

Test plan

• Operator runs terraform plan against an Azure environment and confirms the diff matches the design (3 workflow identities added, 3 KV role assignments added, 6 custom-action resources, 3 old azurerm_logic_app_action_http removed).
• After apply, az logic show -n <wf> no longer reveals the secret in the workflow definition.
• After apply, a manual workflow run completes with HTTP 200 (the get-secret action returns the value, the call-endpoint action authenticates successfully against the Container App).

The post-apply checks are operator-only (require a live Azure tenant); the static verification is captured in this PR.

Out of scope

• GCP / GKE Logic-App-equivalent paths (separate issue, not flagged here).
• Refactoring the 3 near-identical workflow blocks into a for_each map — would change the resource address shape and force a destroy/create on operator state. Left for a future cleanup if the workflow set keeps growing.

Summary by CodeRabbit

• Bug Fixes

  • Resolved plaintext secret storage vulnerability for scheduled task secrets; secrets are now securely retrieved at runtime using managed identity instead of being stored in workflow definitions or infrastructure state.

• Documentation

  • Updated known issues documentation to reflect the resolution of the scheduled task secret security issue.

[cristim]

@coderabbitai review

[coderabbitai]

Warning

Rate limit exceeded

@cristim has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 28 minutes and 59 seconds before requesting another review.

To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 23d3dde6-836a-424d-b53e-f1c5b24249fd

📥 Commits

Reviewing files that changed from the base of the PR and between 6a45693 and 3b467a1.

📒 Files selected for processing (3)

• known_issues/18_tf_env_azure.md
• terraform/modules/compute/azure/container-apps/scheduled-tasks.tf
• terraform/modules/compute/azure/container-apps/variables.tf

📝 Walkthrough

Walkthrough

The pull request resolves a security vulnerability where the SCHEDULED_TASK_SECRET was embedded in plaintext within Logic App workflow definitions and Terraform state. The fix migrates Logic App workflows to retrieve the secret from Key Vault at runtime using managed identity authentication, eliminating plaintext retention in workflow definitions.

Changes

Cohort / File(s)	Summary
Security Implementation terraform/modules/compute/azure/container-apps/scheduled-tasks.tf	Replaces direct secret interpolation with managed identity-based Key Vault retrieval. Adds system-assigned managed identity blocks, introduces get-secret HTTP custom actions that authenticate to Key Vault, updates subsequent actions to reference fetched secrets, and configures role assignments for Key Vault Secrets User access.
Configuration Updates terraform/environments/azure/compute.tf, terraform/modules/compute/azure/container-apps/variables.tf	Modifies module inputs to pass the Key Vault secret name (scheduled_task_secret_name) instead of the plaintext value. Renames and removes sensitive flag from the variable to reflect the new intent of storing secret names rather than values.
Dependency & Documentation terraform/environments/azure/.terraform.lock.hcl, known_issues/18_tf_env_azure.md	Adds hashicorp/time provider lock entry and updates known-issues documentation from "PARTIALLY RESOLVED" to "RESOLVED," confirming Logic App migration completion.

Sequence Diagram(s)

sequenceDiagram
    participant LogicApp as Logic App Workflow
    participant KeyVault as Azure Key Vault
    participant ManagedId as Managed Identity
    participant Endpoint as Scheduled Tasks Endpoint

    rect rgba(34, 139, 34, 0.5)
    Note over LogicApp,Endpoint: Updated Flow (Secure)
    LogicApp->>ManagedId: Request access token
    ManagedId-->>LogicApp: Access token (authenticated)
    LogicApp->>KeyVault: Authenticate with managed identity token<br/>GET secret
    KeyVault-->>LogicApp: Secret value (fetched at runtime)
    LogicApp->>Endpoint: Call endpoint with retrieved secret<br/>Authorization: Bearer [secret]
    Endpoint-->>LogicApp: Success
    end

    rect rgba(178, 34, 34, 0.5)
    Note over LogicApp,Endpoint: Previous Flow (Insecure)
    LogicApp->>LogicApp: Secret embedded in workflow definition<br/>Authorization: Bearer [plaintext_secret]
    LogicApp->>Endpoint: Call endpoint with hardcoded secret
    Endpoint-->>LogicApp: Success
    Note over LogicApp,Endpoint: ✗ Plaintext visible in: workflow def,<br/>Terraform state, logs
    end

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Poem

🐰 A secret once lived in the plain morning light,
Now hidden in vaults, retrieved through the night,
Managed identities whisper, "Come fetch if you dare,"
No plaintext in logs—just trust floating there! ✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the main change: migrating Logic App scheduled task secret handling from embedded plaintext to Key Vault-backed retrieval via managed identity.
Linked Issues check	✅ Passed	All key requirements from issue #50 are met: plaintext secret removal from Logic App definitions/state, runtime Key Vault retrieval via managed identity, module API change replacing secret value with secret name, and comprehensive multi-workflow migration.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to issue #50 objectives: Logic App migration logic, module/environment configuration updates, and documentation of resolution. GCP/refactoring work explicitly marked out-of-scope.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/azure-logic-app-kv-secret

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[cristim]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

known_issues/18_tf_env_azure.md (1)

3-3: ⚠️ Potential issue | 🟡 Minor

Update the audit counter — it's now stale.

The summary line still reads 1 partially fixed, but this PR closes the only "partially fixed" item (the scheduled-task-secret entry, now flipped to ✔️ Resolved on line 102). Bump 7 resolved → 8 resolved and 1 partially fixed → 0 partially fixed to keep the audit roll-up honest.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@known_issues/18_tf_env_azure.md` at line 3, Update the audit summary line
that currently reads "**Audit status (2026-04-20):** `0 still valid · 7 resolved
· 1 partially fixed · 0 moved · 0 needs triage`" to reflect the closed item:
change `7 resolved` → `8 resolved` and `1 partially fixed` → `0 partially
fixed`; the scheduled-task-secret entry referenced (marked ✔️ Resolved at the
content around the scheduled-task-secret section) is the item causing this
counter change so update that single-line roll-up accordingly.

🧹 Nitpick comments (1)

terraform/modules/compute/azure/container-apps/scheduled-tasks.tf (1)

334-356: Add explicit dependency from workflow actions to role assignments to avoid first-run 403s.

Azure RBAC propagation can take 5–10 minutes after an azurerm_role_assignment is created. Currently, there is no dependency edge from the *_get_secret actions to the corresponding *_kv_secrets_user assignments, so Terraform may create them in parallel. A manual "Run trigger" immediately after apply can 403 until propagation completes. Scheduled runs at 02:00/03:00/06h cadence are unlikely to hit this, but the runbook will benefit from the determinism.

♻️ Proposed adjustment (one of the workflows shown; mirror for the other two)

 resource "azurerm_logic_app_action_custom" "recommendations_get_secret" {
   count = var.enable_scheduled_tasks ? 1 : 0

   name         = "get-secret"
   logic_app_id = azurerm_logic_app_workflow.recommendations[0].id

   body = jsonencode({ ... })
+
+  depends_on = [azurerm_role_assignment.recommendations_kv_secrets_user]
 }

Alternatively, keep the dependency at the workflow level so all actions inherit it.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@terraform/modules/compute/azure/container-apps/scheduled-tasks.tf` around
lines 334 - 356, Add an explicit dependency so each Logic App workflow waits for
its Key Vault role assignment to propagate: update
azurerm_logic_app_workflow.recommendations,
azurerm_logic_app_workflow.ri_exchange, and azurerm_logic_app_workflow.cleanup
to include a depends_on that references their respective azurerm_role_assignment
resources (azurerm_role_assignment.recommendations_kv_secrets_user,
azurerm_role_assignment.ri_exchange_kv_secrets_user,
azurerm_role_assignment.cleanup_kv_secrets_user) so Terraform creates the role
assignment before the workflow (or its _get_secret actions) to prevent first-run
403s.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@terraform/modules/compute/azure/container-apps/scheduled-tasks.tf`:
- Around line 24-30: Add a plan-time guard so the secret name cannot be empty
when scheduled tasks are enabled: add a Terraform precondition/validation that
when var.enable_scheduled_tasks || var.enable_ri_exchange_schedule is true then
var.scheduled_task_secret_name != "" (or length(var.scheduled_task_secret_name)
> 0); this prevents scheduled_task_secret_url from resolving to the list-secrets
endpoint and surfaces the error at plan/apply. Ensure you reference the existing
vars (var.enable_scheduled_tasks, var.enable_ri_exchange_schedule,
var.scheduled_task_secret_name) when implementing the precondition/validation.

In `@terraform/modules/compute/azure/container-apps/variables.tf`:
- Around line 207-211: The variable scheduled_task_secret_name currently allows
an empty string which lets local.scheduled_task_secret_url resolve to the
list-secrets endpoint; add a validation block on variable
"scheduled_task_secret_name" requiring it to be a non-empty string (e.g. length
> 0) so plan-time errors occur when not provided, and also add a precondition
check where local.scheduled_task_secret_url (or the code building the Key Vault
URL in scheduled-tasks.tf) is constructed to assert that enable_scheduled_tasks
== false OR scheduled_task_secret_name != "" so the enabled+empty combination
fails during plan.

---

Outside diff comments:
In `@known_issues/18_tf_env_azure.md`:
- Line 3: Update the audit summary line that currently reads "**Audit status
(2026-04-20):** `0 still valid · 7 resolved · 1 partially fixed · 0 moved · 0
needs triage`" to reflect the closed item: change `7 resolved` → `8 resolved`
and `1 partially fixed` → `0 partially fixed`; the scheduled-task-secret entry
referenced (marked ✔️ Resolved at the content around the scheduled-task-secret
section) is the item causing this counter change so update that single-line
roll-up accordingly.

---

Nitpick comments:
In `@terraform/modules/compute/azure/container-apps/scheduled-tasks.tf`:
- Around line 334-356: Add an explicit dependency so each Logic App workflow
waits for its Key Vault role assignment to propagate: update
azurerm_logic_app_workflow.recommendations,
azurerm_logic_app_workflow.ri_exchange, and azurerm_logic_app_workflow.cleanup
to include a depends_on that references their respective azurerm_role_assignment
resources (azurerm_role_assignment.recommendations_kv_secrets_user,
azurerm_role_assignment.ri_exchange_kv_secrets_user,
azurerm_role_assignment.cleanup_kv_secrets_user) so Terraform creates the role
assignment before the workflow (or its _get_secret actions) to prevent first-run
403s.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 55793367-840d-471e-9415-d34e8fbc5c73

📥 Commits

Reviewing files that changed from the base of the PR and between c8f58e9 and 6a45693.

📒 Files selected for processing (5)

• known_issues/18_tf_env_azure.md
• terraform/environments/azure/.terraform.lock.hcl
• terraform/environments/azure/compute.tf
• terraform/modules/compute/azure/container-apps/scheduled-tasks.tf
• terraform/modules/compute/azure/container-apps/variables.tf

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Add validation so an empty scheduled_task_secret_name can't silently produce a malformed Key Vault URL.

With default = "" and enable_scheduled_tasks defaulting to true, a caller that forgets to pass this name will produce …/secrets/?api-version=7.4 in local.scheduled_task_secret_url — that's the list-secrets endpoint, not a single-secret GET. The managed identity has read access at vault scope, so the call succeeds, body('get-secret')['value'] resolves to an array, and the downstream Authorization header becomes garbage. The failure only surfaces at runtime as a 401 from the Container App.

A validation block keeps the failure at plan time:

🛡️ Proposed fix

 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     = ""
+
+  validation {
+    condition     = !contains(["/", "?", "#", " "], substr(var.scheduled_task_secret_name, 0, 0)) && !can(regex("[/?# ]", var.scheduled_task_secret_name))
+    error_message = "scheduled_task_secret_name must be a bare Key Vault secret name (no '/', '?', '#', or whitespace)."
+  }
 }

Plus a precondition next to where the URL is built (see scheduled-tasks.tf comment) so that the empty-string + feature-enabled combination is caught.

📝 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

	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 "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 = ""
	validation {
	condition = var.scheduled_task_secret_name != "" && !can(regex("[/?# ]", var.scheduled_task_secret_name))
	error_message = "scheduled_task_secret_name must be a non-empty, bare Key Vault secret name (no '/', '?', '#', or whitespace)."
	}
	}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@terraform/modules/compute/azure/container-apps/variables.tf` around lines 207
- 211, The variable scheduled_task_secret_name currently allows an empty string
which lets local.scheduled_task_secret_url resolve to the list-secrets endpoint;
add a validation block on variable "scheduled_task_secret_name" requiring it to
be a non-empty string (e.g. length > 0) so plan-time errors occur when not
provided, and also add a precondition check where
local.scheduled_task_secret_url (or the code building the Key Vault URL in
scheduled-tasks.tf) is constructed to assert that enable_scheduled_tasks ==
false OR scheduled_task_secret_name != "" so the enabled+empty combination fails
during plan.

[cristim]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[cristim]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.