diff --git a/lib/load-project.sh b/lib/load-project.sh index e42d6dc..5ad23cc 100755 --- a/lib/load-project.sh +++ b/lib/load-project.sh @@ -85,22 +85,8 @@ 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_ 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 @@ -166,16 +152,8 @@ 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 diff --git a/tests/lib-load-project.bats b/tests/lib-load-project.bats index f0c583a..89e82be 100644 --- a/tests/lib-load-project.bats +++ b/tests/lib-load-project.bats @@ -184,70 +184,3 @@ 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" <&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"* ]] -}