Merge pull request 'fix: bug: generator emits invalid env var name FORGE_BOT_USER_<service>^^ when service name contains hyphen (#852)' (#873) from fix/issue-852 into main

lib/load-project.sh

@@ -85,8 +85,22 @@ if mirrors:
 # environment.  The TOML carries host-perspective values (localhost, /home/admin/…)
 # that would break container API calls and path resolution.  Skip overriding
 # any env var that is already set when running inside the container.
+#
+# #852 defence: validate that $_key is a legal shell identifier before
+# `export`.  A hand-edited TOML can smuggle in keys that survive the
+# Python emitter but fail `export`'s identifier rule — e.g.
+# `[mirrors] my-mirror = "..."` becomes `MIRROR_MY-MIRROR` because the
+# MIRROR_<NAME> emitter only upper-cases, it does not dash-to-underscore.
+# Without this guard `export "MIRROR_MY-MIRROR=…"` returns non-zero, and
+# under `set -euo pipefail` in the caller the whole file aborts — which
+# is how the original #852 crash-loop presented.  Warn-and-skip keeps
+# the rest of the TOML loadable.
 while IFS='=' read -r _key _val; do
   [ -z "$_key" ] && continue
+  if ! [[ "$_key" =~ ^[A-Za-z_][A-Za-z0-9_]*$ ]]; then
+    echo "WARNING: load-project: skipping invalid shell identifier from TOML: $_key" >&2
+    continue
+  fi
   if [ "${DISINTO_CONTAINER:-}" = "1" ] && [ -n "${!_key:-}" ]; then
     continue
   fi
@@ -152,8 +166,16 @@ for name, config in agents.items():
 " "$_PROJECT_TOML" 2>/dev/null) || true
 
   if [ -n "$_AGENT_VARS" ]; then
+    # #852 defence: same warn-and-skip guard as the main loop above.  The
+    # Python emitter already normalizes dashed agent names (#862), but a
+    # quoted TOML section like `[agents."weird name"]` could still produce
+    # an invalid identifier.  Fail loudly but keep other agents loadable.
     while IFS='=' read -r _key _val; do
       [ -z "$_key" ] && continue
+      if ! [[ "$_key" =~ ^[A-Za-z_][A-Za-z0-9_]*$ ]]; then
+        echo "WARNING: load-project: skipping invalid shell identifier from [agents.*]: $_key" >&2
+        continue
+      fi
       export "$_key=$_val"
     done <<< "$_AGENT_VARS"
   fi

tests/lib-load-project.bats

@@ -184,3 +184,70 @@ EOF
   [ "$status" -ne 0 ]
   [[ "$output" == *"invalid agent name"* ]]
 }
+
+# -------------------------------------------------------------------------
+# #852 defence: the export loops must warn-and-skip invalid identifiers
+# rather than tank `set -euo pipefail`. Hire-agent's up-front reject
+# (tests above) is the primary line of defence, but a hand-edited TOML —
+# e.g. [mirrors] my-mirror = "…" or a quoted [agents."weird name"] — can
+# still produce invalid shell identifiers downstream. The guard keeps
+# the factory loading the rest of the file instead of crash-looping.
+# -------------------------------------------------------------------------
+
+@test "[mirrors] dashed key: warn-and-skip, does not crash under set -e" {
+  cat > "$TOML" <<EOF
+name      = "test"
+repo      = "test-owner/test-repo"
+forge_url = "http://localhost:3000"
+
+[mirrors]
+good = "https://example.com/good"
+bad-name = "https://example.com/bad"
+EOF
+
+  run bash -c "
+    set -euo pipefail
+    source '${ROOT}/lib/load-project.sh' '$TOML' 2>&1
+    echo \"GOOD=\${MIRROR_GOOD:-MISSING}\"
+  "
+
+  # Whole load did not abort under set -e.
+  [ "$status" -eq 0 ]
+  # The valid mirror still loads.
+  [[ "$output" == *"GOOD=https://example.com/good"* ]]
+  # The invalid one triggers a warning; load continues instead of crashing.
+  [[ "$output" == *"skipping invalid shell identifier"* ]]
+  [[ "$output" == *"MIRROR_BAD-NAME"* ]]
+}
+
+@test "[agents.*] quoted section with space: warn-and-skip, does not crash" {
+  # TOML permits quoted keys with arbitrary characters. A hand-edited
+  # `[agents."weird name"]` would survive the Python .replace('-', '_')
+  # (because it has no dash) but still contains a space, which would
+  # yield AGENT_WEIRD NAME_BASE_URL — not a valid identifier.
+  cat > "$TOML" <<'EOF'
+name      = "test"
+repo      = "test-owner/test-repo"
+forge_url = "http://localhost:3000"
+
+[agents.llama]
+base_url = "http://10.10.10.1:8081"
+model    = "qwen"
+
+[agents."weird name"]
+base_url = "http://10.10.10.1:8082"
+model    = "qwen-bad"
+EOF
+
+  run bash -c "
+    set -euo pipefail
+    source '${ROOT}/lib/load-project.sh' '$TOML' 2>&1
+    echo \"LLAMA=\${AGENT_LLAMA_BASE_URL:-MISSING}\"
+  "
+
+  # The sane sibling must still be loaded despite the malformed neighbour.
+  [ "$status" -eq 0 ]
+  [[ "$output" == *"LLAMA=http://10.10.10.1:8081"* ]]
+  # The invalid agent's identifier triggers a warning and is skipped.
+  [[ "$output" == *"skipping invalid shell identifier"* ]]
+}