fix: address Phase-5 code review (4 findings)

Reviewer flagged two security-relevant items and two operational bugs
on the external-DB + Terraform module merge.

1. Terraform: `enable_execute_command = true` was hardcoded on the app
   and embedder services. Production attack surface (anyone with
   `ecs:ExecuteCommand` on the service gets a container shell) AND
   non-functional today since the task roles have no `ssmmessages:*`
   permission. Added a new `enable_execute_command` boolean input
   variable defaulting to `false`; when flipped on, the SSM messages
   policy is conditionally attached to both task roles so the feature
   actually works. README's variable description tells operators to
   flip on for incidents, off afterward.

2. Terraform: `secret_arns` output was not marked `sensitive`. The ARNs
   themselves aren't secrets, but the embedded secret names print to
   `terraform apply` stdout and CI logs. Marked sensitive on both the
   module output and the example output. Operators wanting the values
   can still `terraform output -json secret_arns`.

3. Terraform: embedder task definition was missing `HOST=0.0.0.0` and
   `PORT=8080`. Fargate awsvpc tasks each get their own ENI; default
   Node HTTP servers bind 127.0.0.1, which would make every
   app→embedder Service Connect call time out. Added both vars to
   `embedder_environment`. Also added `NEXT_TELEMETRY_DISABLED=1` to
   `app_environment` per the spec's hardening checklist.

4. Compose: docker-compose.external-db.yml uses the `!override` YAML
   tag, which requires Docker Compose >= 2.24.0. Silently ignored on
   older Compose, causing the `db` dependency to survive the merge and
   startup to fail. Documented the minimum version in the override
   file's header AND in the main README prerequisites with a deep
   link to the External Postgres section.

terraform fmt + validate (module + examples/basic) both clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

README.md

@@ -44,7 +44,8 @@ memories are scoped per user.
 
 ## Prerequisites
 
-- A host with **Docker** and **Docker Compose v2** installed.
+- A host with **Docker** and **Docker Compose v2** installed (≥ 2.24.0 if you
+  plan to use the [external Postgres override](#external-postgres-rds-cloud-sql-etc)).
 - An **OIDC identity provider** you control (Authentik, EntraID, Keycloak,
   Okta, Auth0, Zitadel, …). The setup walkthrough below uses Authentik
   because that's what we run; other IdPs need equivalent settings.

docker-compose.external-db.yml

@@ -18,6 +18,13 @@
 # The DB user needs privileges to `CREATE EXTENSION` for pgvector, pg_trgm,
 # and pgcrypto on first run — on RDS that means the `rds_superuser` role, or
 # pre-create the extensions yourself. See README "External Postgres".
+#
+# REQUIRES DOCKER COMPOSE >= 2.24.0 (Docker Desktop >= 4.25, or Compose plugin
+# 2.24.0+). The `!override` YAML tag on the depends_on blocks below is what
+# fully replaces — rather than merges — the base file's `depends_on: db`
+# entries. On older Compose the tag is silently ignored, the `db` dependency
+# survives the merge, and startup fails with "depends on undefined service
+# db". Check with: docker compose version
 # =============================================================================
 
 services:

terraform/ecs.tf

@@ -61,6 +61,7 @@ locals {
     { name = "EMBEDDER_URL", value = "http://embedder:${local.embedder_port}" },
     { name = "EMBEDDING_MODEL", value = var.embedding_model },
     { name = "EMBEDDING_DIM", value = tostring(var.embedding_dim) },
+    { name = "NEXT_TELEMETRY_DISABLED", value = "1" },
   ]
 
   # `secrets` block format that ECS expects: name = env-var name, valueFrom
@@ -73,6 +74,12 @@ locals {
   ]
 
   embedder_environment = [
+    # awsvpc network mode gives every task its own ENI — bind to 0.0.0.0
+    # explicitly so Service Connect reaches the embedder on the task's
+    # ENI address. Default Node servers often bind 127.0.0.1, which
+    # would silently make every app→embedder call time out.
+    { name = "HOST", value = "0.0.0.0" },
+    { name = "PORT", value = tostring(local.embedder_port) },
     { name = "LOG_LEVEL", value = var.log_level },
     { name = "EMBEDDING_MODEL", value = var.embedding_model },
     { name = "EMBEDDING_DIM", value = tostring(var.embedding_dim) },
@@ -272,7 +279,7 @@ resource "aws_ecs_service" "embedder" {
   task_definition        = aws_ecs_task_definition.embedder.arn
   desired_count          = var.embedder_desired_count
   launch_type            = "FARGATE"
-  enable_execute_command = true
+  enable_execute_command = var.enable_execute_command
 
   network_configuration {
     subnets          = var.private_subnet_ids
@@ -322,7 +329,7 @@ resource "aws_ecs_service" "app" {
   task_definition        = aws_ecs_task_definition.app.arn
   desired_count          = var.app_desired_count
   launch_type            = "FARGATE"
-  enable_execute_command = true
+  enable_execute_command = var.enable_execute_command
 
   network_configuration {
     subnets          = var.private_subnet_ids

terraform/examples/basic/outputs.tf

@@ -51,4 +51,5 @@ output "migrator_log_group_name" {
 output "secret_arns" {
   description = "Visibility into where the module stored its secrets."
   value       = module.shared_memory.secret_arns
+  sensitive   = true
 }

terraform/iam.tf

@@ -67,3 +67,38 @@ resource "aws_iam_role" "migrator_task" {
   assume_role_policy = data.aws_iam_policy_document.ecs_tasks_assume.json
   tags               = local.tags
 }
+
+# ---- ECS Execute Command (opt-in via var.enable_execute_command) ----
+#
+# When the operator flips this on for incident debugging, the task role
+# needs the SSM messages permissions for the channel to open. We attach
+# the policy conditionally to both app and embedder task roles — the
+# migrator is short-lived and doesn't get exec.
+
+data "aws_iam_policy_document" "exec_command" {
+  count = var.enable_execute_command ? 1 : 0
+  statement {
+    sid = "AllowECSExecuteCommand"
+    actions = [
+      "ssmmessages:CreateControlChannel",
+      "ssmmessages:CreateDataChannel",
+      "ssmmessages:OpenControlChannel",
+      "ssmmessages:OpenDataChannel",
+    ]
+    resources = ["*"]
+  }
+}
+
+resource "aws_iam_role_policy" "app_exec_command" {
+  count  = var.enable_execute_command ? 1 : 0
+  name   = "${var.name_prefix}-app-exec-command"
+  role   = aws_iam_role.app_task.id
+  policy = data.aws_iam_policy_document.exec_command[0].json
+}
+
+resource "aws_iam_role_policy" "embedder_exec_command" {
+  count  = var.enable_execute_command ? 1 : 0
+  name   = "${var.name_prefix}-embedder-exec-command"
+  role   = aws_iam_role.embedder_task.id
+  policy = data.aws_iam_policy_document.exec_command[0].json
+}

terraform/outputs.tf

@@ -84,6 +84,7 @@ output "private_subnet_ids_for_run_task" {
 }
 
 output "secret_arns" {
-  description = "Map of env-var name to Secrets Manager ARN. For visibility only — do not re-feed back into the module."
+  description = "Map of env-var name to Secrets Manager ARN. For visibility only — do not re-feed back into the module. Marked sensitive so the secret names don't print in `terraform apply` stdout or CI logs."
   value       = local.secret_arns
+  sensitive   = true
 }

terraform/variables.tf

@@ -190,6 +190,22 @@ variable "log_retention_days" {
   default     = 14
 }
 
+variable "enable_execute_command" {
+  description = <<-EOT
+    Enable AWS ECS Execute Command on the app + embedder services. When true,
+    operators with the appropriate IAM permission can `aws ecs execute-command`
+    into a running task — useful for debugging, dangerous as a standing
+    posture (any principal with `ecs:ExecuteCommand` on these services gets a
+    shell inside the container). Defaults `false`. Flip to `true` for an
+    incident, then back to `false` and re-apply when done.
+
+    When enabled, the module also attaches the SSM messages policy to both
+    task roles so the feature actually works.
+  EOT
+  type        = bool
+  default     = false
+}
+
 variable "tags" {
   description = "Tags merged onto every resource the module creates."
   type        = map(string)