fix(issue-26): the wrapper reads .mam.env through the Python loader's parser, never source

- lib_py/delegation/envfile.py (stdlib only) holds the one set of rules:
  comments and blank lines skipped, key and value stripped, one matching
  quote pair removed, no shell interpretation, keys must match
  ^[A-Za-z_][A-Za-z0-9_]*$ (bad or `=`-less lines warn with the line
  number, never the value).
- mqtt_common._parse_env_file delegates to envfile.apply.
- The delegate-job wrapper runs envfile.py --shell by path and evals the
  shlex-quoted exports; values never reach a command line. The environment
  now wins over the file in both loaders.
- .mam.env.example: commented assignments no longer use $HOME, which the
  parser keeps literally.

The shell parsed the env file in one place only (wrapper :38), not three.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

.agents/skills/lib_py/delegation/envfile.py

@@ -0,0 +1,78 @@
+"""The one `.mam.env` parser (Issue #26).
+
+Rules: blank lines and `#` comments are skipped; `KEY=VALUE` with the key and
+the value stripped; one matching pair of surrounding quotes is removed; no
+shell interpretation (`$(...)`, `${...}` and backticks stay literal). A key
+must match ``^[A-Za-z_][A-Za-z0-9_]*$``; a line that does not, or that has no
+`=`, is skipped with a warning naming the line (never the value — the file
+holds secrets). Variables already in the environment win over the file.
+
+Stdlib only: the delegate-job wrapper runs this file by path
+(``envfile.py --shell <path>``) and evals the ``export`` lines it prints, so
+values travel over a pipe, never on a command line.
+"""
+import os
+import re
+import shlex
+import sys
+from typing import List, MutableMapping, Tuple
+
+_KEY = re.compile(r"[A-Za-z_][A-Za-z0-9_]*")
+
+
+def parse(path: str) -> Tuple[List[Tuple[str, str]], List[str]]:
+    """Return ([(key, value)], [warning]) for the file at ``path``."""
+    pairs: List[Tuple[str, str]] = []
+    warnings: List[str] = []
+    try:
+        with open(path, "r", encoding="utf-8") as f:
+            lines = f.read().splitlines()
+    except (OSError, UnicodeDecodeError) as e:
+        return pairs, [f"{path}: cannot read ({type(e).__name__})"]
+    for n, raw in enumerate(lines, 1):
+        line = raw.strip()
+        if not line or line.startswith("#"):
+            continue
+        key, sep, val = line.partition("=")
+        key = key.strip()
+        if not sep:
+            warnings.append(f"{path}:{n}: skipped (no '=')")
+            continue
+        if not _KEY.fullmatch(key):
+            warnings.append(f"{path}:{n}: skipped (invalid key)")
+            continue
+        val = val.strip()
+        if len(val) >= 2 and val[0] == val[-1] and val[0] in "\"'":
+            val = val[1:-1]
+        pairs.append((key, val))
+    return pairs, warnings
+
+
+def apply(path: str, environ: MutableMapping[str, str]) -> Tuple[List[Tuple[str, str]], List[str]]:
+    """Set each pair not already in ``environ``; return (the pairs set, warnings)."""
+    pairs, warnings = parse(path)
+    applied = []
+    for key, val in pairs:
+        if key not in environ:
+            environ[key] = val
+            applied.append((key, val))
+    return applied, warnings
+
+
+def main(argv: List[str]) -> int:
+    if len(argv) != 2 or argv[0] != "--shell":
+        print("usage: envfile.py --shell <path>", file=sys.stderr)
+        return 2
+    path = argv[1]
+    if not os.path.isfile(path):
+        return 0
+    applied, warnings = apply(path, dict(os.environ))
+    for w in warnings:
+        print(f"WARNING: {w}", file=sys.stderr)
+    for key, val in applied:
+        print(f"export {key}={shlex.quote(val)}")
+    return 0
+
+
+if __name__ == "__main__":
+    sys.exit(main(sys.argv[1:]))

.agents/skills/lib_py/delegation/mqtt_common.py

@@ -31,6 +31,8 @@ from typing import Any, Callable, Dict, Iterable, List, Optional
 
 import paho.mqtt.client as mqtt
 
+from lib_py.delegation import envfile
+
 logger = logging.getLogger("delegate_job.mqtt_common")
 
 _warned_deprecated_env = False
@@ -94,20 +96,9 @@ def _load_dotenv(workspace_dir: Optional[str] = None) -> None:
         _parse_env_file(legacy_env_path)
 
 def _parse_env_file(path: str) -> None:
-    try:
+    """Issue #26: the rules live in envfile.py, shared with the wrapper."""
-        with open(path, "r", encoding="utf-8") as f:
+    for warning in envfile.apply(path, os.environ)[1]:
-            for line in f:
+        logger.warning("%s", warning)
-                line = line.strip()
-                if not line or line.startswith("#"):
-                    continue
-                if "=" in line:
-                    key, _, val = line.partition("=")
-                    key = key.strip()
-                    val = val.strip().strip('"').strip("'")
-                    if key and key not in os.environ:
-                        os.environ[key] = val
-    except Exception as e:
-        logger.warning("Failed to parse env file %s: %s", path, e)
 
 _env_loaded = False
 

.agents/skills/multi-agent-mux-delegate-job/multi-agent-mux-delegate-job

@@ -34,8 +34,12 @@ if [[ -z "$TARGET_ENV" ]]; then
   fi
 fi
 
+# Issue #26: parsed by envfile.py (the Python loader's rules; environment wins),
+# never sourced. Values come back as shlex-quoted exports over the pipe.
 if [[ -n "$TARGET_ENV" && -f "$TARGET_ENV" ]]; then
-  set -a; source "$TARGET_ENV"; set +a
+  _env_exports="$("${DELEGATE_JOB_PYTHON:-python3}" -I "$SCRIPT_DIR/../lib_py/delegation/envfile.py" --shell "$TARGET_ENV")" \
+    || { echo "ERROR: cannot read $TARGET_ENV" >&2; exit 1; }
+  eval "$_env_exports"; unset _env_exports
 fi
 
 # Source EARLY (before any herdr usage in run_agent) — this is what turns

.mam.env.example

@@ -29,11 +29,11 @@
 
 # Root directory that holds Claude Code per-project conversation logs (*.jsonl).
 #default: $HOME/.claude/projects
-# CLAUDE_PROJECT_DIR=$HOME/.claude/projects
+# CLAUDE_PROJECT_DIR=/home/you/.claude/projects
 
 # Directory scanned for per-session launcher wrappers (~/.local/bin/<session>).
 #default: $HOME/.local/bin
-# LOCAL_BIN=$HOME/.local/bin
+# LOCAL_BIN=/home/you/.local/bin
 
 # Isolated Herdr session socket name (`herdr --session <name>`).
 #default: mam-<workspace-slug>
@@ -46,7 +46,7 @@
 
 # Path to the Herdr Unix domain socket.
 #default: $HOME/.config/herdr/herdr.sock
-# HERDR_SOCKET_PATH=$HOME/.config/herdr/herdr.sock
+# HERDR_SOCKET_PATH=/home/you/.config/herdr/herdr.sock
 
 # Explicit Herdr workspace scope identifier (`herdr pane list --workspace <id>`).
 # Hard filters pane/agent resolution to this workspace. When unset, falls back
@@ -107,7 +107,7 @@
 
 # Optional override for Codex CLI state / session directory.
 #default: $HOME/.codex
-# CODEX_HOME=$HOME/.codex
+# CODEX_HOME=/home/you/.codex
 
 # Optional inline JSON permissions configuration for OpenCode.
 #default: (unset → defaults to --auto CLI bypass)

tests/test_issue26_envfile.py

@@ -0,0 +1,253 @@
+"""Issue #26 (A): one `.mam.env` parser for the wrapper and the Python loader.
+
+- A1: the issue's file no longer kills the real wrapper, runs no shell, warns
+  per bad line without leaking values.
+- A2: the real wrapper and the real Python loader read one table the same way.
+- A3: an environment variable beats the file, in both loaders.
+- A4: the parser's command line carries the path only.
+- A5: the files this repo ships or generates parse without a warning.
+"""
+
+from __future__ import annotations
+
+import json
+import os
+import re
+import subprocess
+import sys
+from pathlib import Path
+
+import pytest
+
+REPO_ROOT = Path(__file__).resolve().parent.parent
+SKILLS = REPO_ROOT / ".agents" / "skills"
+WRAPPER = SKILLS / "multi-agent-mux-delegate-job" / "multi-agent-mux-delegate-job"
+ENVFILE = SKILLS / "lib_py" / "delegation" / "envfile.py"
+
+if str(SKILLS) not in sys.path:
+    sys.path.insert(0, str(SKILLS))
+
+from lib_py.delegation import envfile  # noqa: E402
+
+ISSUE_FILE = (
+    "AGENT_SESSIONS_YAML= .mam/agent-sessions.yaml\n"
+    "X=$(touch pwned)\n"
+    "BAD KEY=1\n"
+    "noequals\n"
+    'MQTT_PASSWORD="s3cr3t-a1"\n'
+)
+CLEAN_FILE = (
+    "# comment\n"
+    "\n"
+    "AGENT_SESSIONS_YAML=.mam/agent-sessions.yaml\n"
+    'MQTT_PASSWORD="s3cr3t-a1"\n'
+)
+# key -> value both loaders must produce (A2)
+TABLE_FILE = (
+    "  SPACED  =  padded value  \n"
+    "# ignored=1\n"
+    "\n"
+    "DQ=\"double quoted\"\n"
+    "SQ='single quoted'\n"
+    "HALF=\"unmatched\n"
+    "MIXED=\"a'\n"
+    "EQ=a=b=c\n"
+    "EMPTY=\n"
+    "LIT=$(echo no) ${HOME} `echo no`\n"
+    "export EXPORTED=1\n"
+    "1BAD=1\n"
+)
+TABLE = {
+    "SPACED": "padded value",
+    "DQ": "double quoted",
+    "SQ": "single quoted",
+    "HALF": '"unmatched',
+    "MIXED": "\"a'",
+    "EQ": "a=b=c",
+    "EMPTY": "",
+    "LIT": "$(echo no) ${HOME} `echo no`",
+}
+TABLE_SKIPPED = {"EXPORTED", "export EXPORTED", "1BAD"}
+
+
+def _env(home: Path, env_file: Path, extra=None) -> dict:
+    env = {k: v for k, v in os.environ.items() if k not in TABLE and k not in ("MAM_JOB_TIMEOUT_S", "MQTT_PASSWORD",
+                                                                               "AGENT_SESSIONS_YAML", "X")}
+    env.update({"HOME": str(home), "WORKSPACE_ROOT": str(home), "MAM_ENV_FILE": str(env_file),
+                "DELEGATE_JOB_PYTHON": sys.executable, "AGENT_PYTHON_BIN": sys.executable,
+                "PYTHONPATH": str(SKILLS), "DELEGATE_JOB_LOGS_DIR": str(home / "logs")})
+    if extra:
+        env.update(extra)
+    return env
+
+
+def _wrapper_values(env: dict, keys, cwd: Path) -> subprocess.CompletedProcess:
+    """The values the real wrapper exported after loading its env file."""
+    probe = "; ".join(f'printf "%s=%s\\0" {k} "${{{k}-<unset>}}"' for k in keys)
+    return subprocess.run(["bash", "-c", f'source "{WRAPPER}"; {probe}'], capture_output=True, text=True,
+                          env={**env, "MAM_DELEGATE_JOB_NO_MAIN": "1"}, cwd=str(cwd), timeout=60)
+
+
+def _python_values(env: dict, keys) -> dict:
+    """The values the real Python loader (mqtt_common) set from the same file."""
+    code = ("import json, os; from lib_py.delegation import mqtt_common; mqtt_common.ensure_env_loaded(); "
+            f"print(json.dumps({{k: os.environ.get(k, '<unset>') for k in {list(keys)!r}}}))")
+    res = subprocess.run([sys.executable, "-c", code], capture_output=True, text=True, env=env, timeout=60)
+    assert res.returncode == 0, res.stderr
+    return json.loads(res.stdout.strip().splitlines()[-1])
+
+
+def _split(out: str) -> dict:
+    return dict(item.split("=", 1) for item in out.split("\0") if item)
+
+
+# ---------------------------------------------------------------------------
+# A1 - the issue's file
+# ---------------------------------------------------------------------------
+
+def test_a1_the_issue_file_does_not_kill_the_wrapper_nor_run_shell(tmp_path):
+    f = tmp_path / "issue.env"
+    f.write_text(ISSUE_FILE)
+    res = subprocess.run(["bash", str(WRAPPER), "--help"], capture_output=True, text=True,
+                         env=_env(tmp_path, f), cwd=str(tmp_path), timeout=60)
+    assert res.returncode == 0, res.stderr
+    assert "submit" in res.stdout
+    assert not (tmp_path / "pwned").exists(), "a value must never be run as shell"
+    for n, reason in ((3, "invalid key"), (4, "no '='")):
+        assert f"{f}:{n}: skipped ({reason})" in res.stderr, res.stderr
+    assert ":1: skipped" not in res.stderr and ":2: skipped" not in res.stderr
+    assert "s3cr3t-a1" not in res.stdout + res.stderr
+
+    pairs, warnings = envfile.parse(str(f))
+    assert dict(pairs) == {"AGENT_SESSIONS_YAML": ".mam/agent-sessions.yaml", "X": "$(touch pwned)",
+                           "MQTT_PASSWORD": "s3cr3t-a1"}
+    assert len(warnings) == 2 and not any("s3cr3t" in w for w in warnings)
+
+
+def test_a1_a_clean_file_loads_silently(tmp_path):
+    f = tmp_path / "clean.env"
+    f.write_text(CLEAN_FILE)
+    res = subprocess.run(["bash", str(WRAPPER), "--help"], capture_output=True, text=True,
+                         env=_env(tmp_path, f), cwd=str(tmp_path), timeout=60)
+    assert res.returncode == 0, res.stderr
+    assert "skipped" not in res.stderr and "WARNING" not in res.stderr, res.stderr
+    got = _wrapper_values(_env(tmp_path, f), ["AGENT_SESSIONS_YAML", "MQTT_PASSWORD"], tmp_path)
+    assert _split(got.stdout) == {"AGENT_SESSIONS_YAML": ".mam/agent-sessions.yaml", "MQTT_PASSWORD": "s3cr3t-a1"}
+
+
+def test_a1_an_unreadable_parser_fails_loudly(tmp_path):
+    """The wrapper stops when the parser cannot run instead of going on unconfigured."""
+    f = tmp_path / "clean.env"
+    f.write_text(CLEAN_FILE)
+    res = subprocess.run(["bash", str(WRAPPER), "--help"], capture_output=True, text=True,
+                         env=_env(tmp_path, f, {"DELEGATE_JOB_PYTHON": str(tmp_path / "no-python")}),
+                         cwd=str(tmp_path), timeout=60)
+    assert res.returncode == 1
+    assert f"ERROR: cannot read {f}" in res.stderr
+
+
+# ---------------------------------------------------------------------------
+# A2 - one table, two real loaders, same values
+# ---------------------------------------------------------------------------
+
+def test_a2_the_wrapper_and_the_python_loader_read_the_same_values(tmp_path):
+    f = tmp_path / "table.env"
+    f.write_text(TABLE_FILE)
+    env = _env(tmp_path, f)
+    keys = list(TABLE) + ["EXPORTED"]
+    res = _wrapper_values(env, keys, tmp_path)
+    assert res.returncode == 0, res.stderr
+    wrapper = _split(res.stdout)
+    python = _python_values(env, keys)
+    assert wrapper == python == {**TABLE, "EXPORTED": "<unset>"}
+    for n in (11, 12):
+        assert f"{f}:{n}: skipped (invalid key)" in res.stderr, res.stderr
+    pairs, _ = envfile.parse(str(f))
+    assert not TABLE_SKIPPED & {k for k, _ in pairs}
+
+
+# ---------------------------------------------------------------------------
+# A3 - the environment beats the file, in both loaders
+# ---------------------------------------------------------------------------
+
+def _dry_budget(env: dict, cwd: Path) -> int:
+    reg = cwd / "reg"
+    res = subprocess.run(["bash", str(WRAPPER), "submit", "--agent", "human", "--dry-run",
+                          "--registry-dir", str(reg), "--prompt", "p"],
+                         capture_output=True, text=True, env=env, cwd=str(cwd), timeout=120)
+    assert res.returncode == 0, res.stdout + res.stderr
+    jid = re.search(r"registered job:\s*(\S+)", res.stdout).group(1)
+    return json.loads((reg / f"{jid}.json").read_text())["timeout_sec"]
+
+
+def test_a3_an_environment_value_beats_the_file(tmp_path):
+    f = tmp_path / "budget.env"
+    f.write_text("MAM_JOB_TIMEOUT_S=50\n")
+    (tmp_path / "file").mkdir()
+    (tmp_path / "both").mkdir()
+    assert _dry_budget(_env(tmp_path, f), tmp_path / "file") == 50
+    both = _env(tmp_path, f, {"MAM_JOB_TIMEOUT_S": "70"})
+    assert _dry_budget(both, tmp_path / "both") == 70
+    assert _python_values(both, ["MAM_JOB_TIMEOUT_S"]) == {"MAM_JOB_TIMEOUT_S": "70"}
+    assert _python_values(_env(tmp_path, f), ["MAM_JOB_TIMEOUT_S"]) == {"MAM_JOB_TIMEOUT_S": "50"}
+
+
+# ---------------------------------------------------------------------------
+# A4 - the parser sees the path only
+# ---------------------------------------------------------------------------
+
+def test_a4_the_parser_command_line_carries_the_path_only():
+    calls = [l for l in WRAPPER.read_text().splitlines() if "envfile.py" in l and not l.lstrip().startswith("#")]
+    assert len(calls) == 1, calls
+    assert re.search(r'envfile\.py" --shell "\$TARGET_ENV"\)"', calls[0]), calls[0]
+    src = WRAPPER.read_text()
+    assert not re.search(r'^\s*(source|\.)\s+"?\$TARGET_ENV', src, re.M), "the env file must never be sourced"
+    assert "set -a" not in src
+
+
+# ---------------------------------------------------------------------------
+# A5 - the files the repo ships or generates follow the format
+# ---------------------------------------------------------------------------
+
+def _install_seed(tmp_path: Path) -> Path:
+    """The .mam.env install.sh writes, from its own heredoc run by bash."""
+    src = (REPO_ROOT / "deploy" / "install.sh").read_text()
+    m = re.search(r'cat <<EOF > "\$MAM_ENV"\n(.*?\nEOF)\n', src, re.S)
+    assert m, "install.sh no longer seeds .mam.env with a heredoc"
+    script = f'MAM_ENV="{tmp_path / "seed.env"}"; MAM_PORT=1883; MAM_CLIENT_PREFIX=hermes\ncat <<EOF > "$MAM_ENV"\n{m.group(1)}\n'
+    subprocess.run(["bash", "-c", script], check=True, env={**os.environ, "HOME": str(tmp_path)}, timeout=30)
+    return tmp_path / "seed.env"
+
+
+def test_a5_shipped_and_generated_env_files_parse_without_warnings(tmp_path):
+    ws = tmp_path / "ws"
+    (ws / "deploy").mkdir(parents=True)
+    (ws / ".mam.env.example").write_text((REPO_ROOT / ".mam.env.example").read_text())
+    (ws / "deploy" / "generate-env.sh").write_text((REPO_ROOT / "deploy" / "generate-env.sh").read_text())
+    subprocess.run(["bash", str(ws / "deploy" / "generate-env.sh")], check=True, capture_output=True, timeout=30)
+    for f in (REPO_ROOT / ".mam.env.example", ws / ".mam.env", _install_seed(tmp_path)):
+        pairs, warnings = envfile.parse(str(f))
+        assert warnings == [], (f, warnings)
+        assert not any("$" in v or "`" in v for _, v in pairs), (f, pairs)
+
+    # The template's commented assignments are what a user uncomments: they must
+    # parse too, and hold no shell syntax the parser would keep literally.
+    commented = tmp_path / "uncommented.env"
+    lines = [l[2:] for l in (REPO_ROOT / ".mam.env.example").read_text().splitlines()
+             if re.match(r"# [A-Za-z_][A-Za-z0-9_]*=", l)]
+    assert len(lines) > 20
+    commented.write_text("\n".join(lines) + "\n")
+    pairs, warnings = envfile.parse(str(commented))
+    assert warnings == [] and len(pairs) == len(lines)
+    assert not [(k, v) for k, v in pairs if "$" in v or "`" in v]
+
+
+@pytest.mark.parametrize("missing", [True, False])
+def test_a5_shell_mode_exit_code_is_zero_for_absent_and_present_files(tmp_path, missing):
+    f = tmp_path / "x.env"
+    if not missing:
+        f.write_text("A=1\n")
+    res = subprocess.run([sys.executable, "-I", str(ENVFILE), "--shell", str(f)], capture_output=True, text=True,
+                         env={k: v for k, v in os.environ.items() if k != "A"}, timeout=30)
+    assert res.returncode == 0
+    assert res.stdout == ("" if missing else "export A=1\n")