fix(followups): F3 - .mam.env BOM, repeated keys and a missing MAM_ENV_FILE

PR #43 review (grok):
(a) A UTF-8 byte-order mark at the start of the file made the first line an
    "invalid key", so its setting was dropped. The one parser (envfile.py,
    used by the wrapper and mqtt_common) now reads with utf-8-sig.
(b) A key given twice keeps its first value (a later line does not replace
    it). The behaviour is unchanged; .mam.env.example's FORMAT section and
    envfile.py's docstring now say so.
(c) MESSAGING.md said a MAM_ENV_FILE naming no file logs an error and makes
    connections fail closed. Both loaders skip it silently and refuse the
    ambient search; a file that exists but cannot be read is the error
    (EnvFileError, one ERROR line and rc 1). The text now says that.

Tests A7: BOM in both loaders, first-value-wins in both loaders plus the
FORMAT text, and a missing explicit file loading neither it nor the ambient
.mam.env without an error plus the MESSAGING text (all three fail at 0051fd6).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HCGpJXhSZasJLU38ksDghe
This commit is contained in:
GodopuandClaude Opus 5.5 committed 2026-10-02 08:10:42 +09:00
1 parent 5edf913d22
commit 71ea8095c5
4 files changed
+52 -3

No files matched your search

+5 -2
View File
@@ -7,7 +7,9 @@ must match ``^[A-Za-z_][A-Za-z0-9_]*$`` and not be one of bash's own variables
(RESERVED); a line that breaks either, or that has no `=`, is skipped with a (RESERVED); a line that breaks either, or that has no `=`, is skipped with a
warning naming the line (never the value — the file holds secrets). A line warning naming the line (never the value — the file holds secrets). A line
holding a NUL byte is skipped the same way (no environment can carry it). holding a NUL byte is skipped the same way (no environment can carry it).
Variables already in the environment win over the file. A file that exists A UTF-8 byte-order mark at the start of the file is dropped. Variables
already in the environment win over the file, and a key given twice keeps its
first value. A file that exists
but cannot be read or decoded as a whole is an error (EnvFileError; the but cannot be read or decoded as a whole is an error (EnvFileError; the
``--shell`` command exits 1), not an empty configuration. ``--shell`` command exits 1), not an empty configuration.
@@ -55,7 +57,8 @@ def parse(path: str) -> Tuple[List[Tuple[str, str]], List[str]]:
pairs: List[Tuple[str, str]] = [] pairs: List[Tuple[str, str]] = []
warnings: List[str] = [] warnings: List[str] = []
try: try:
with open(path, "r", encoding="utf-8") as f: # utf-8-sig: a byte-order mark at the start is not part of the first key
with open(path, "r", encoding="utf-8-sig") as f:
lines = f.read().splitlines() lines = f.read().splitlines()
except (OSError, UnicodeDecodeError) as e: except (OSError, UnicodeDecodeError) as e:
raise EnvFileError(f"{path}: cannot read ({type(e).__name__})") from None raise EnvFileError(f"{path}: cannot read ({type(e).__name__})") from None
+2
View File
@@ -21,7 +21,9 @@
# - `export KEY=...`, a key that is not [A-Za-z_][A-Za-z0-9_]*, a line # - `export KEY=...`, a key that is not [A-Za-z_][A-Za-z0-9_]*, a line
# without `=`, a line with a NUL byte, and bash's own variables (IFS, PS4, # without `=`, a line with a NUL byte, and bash's own variables (IFS, PS4,
# RANDOM, BASH_*, ...) are skipped with a warning naming the line number; # RANDOM, BASH_*, ...) are skipped with a warning naming the line number;
# - a UTF-8 byte-order mark at the start of the file is ignored;
# - a variable already set in the environment wins over this file; # - a variable already set in the environment wins over this file;
# - a key given twice keeps its FIRST value (a later line does not replace it);
# - a file that exists but cannot be read is an error (the wrapper exits 1). # - a file that exists but cannot be read is an error (the wrapper exits 1).
# #
# SECURITY: never put real secrets in this template. Secret-bearing vars use a # SECURITY: never put real secrets in this template. Secret-bearing vars use a
+1 -1
View File
@@ -366,7 +366,7 @@ The subscriber acts as the central execution watchdog:
* **`.mam.env` Resolution Hierarchy (`_load_dotenv`)**: * **`.mam.env` Resolution Hierarchy (`_load_dotenv`)**:
Configuration files are resolved with strict precedence rules: Configuration files are resolved with strict precedence rules:
1. **OS Environment Precedence**: Any variable already defined in `os.environ` is preserved and never overwritten by file-based configs. 1. **OS Environment Precedence**: Any variable already defined in `os.environ` is preserved and never overwritten by file-based configs.
2. **Explicit Override (`MAM_ENV_FILE`)**: If `MAM_ENV_FILE` is set, only that specific file is parsed. If the specified file does not exist, an error is logged and ambient search is refused (preventing silent fallback to unintended parent configs). Connection attempts fail-closed (`RuntimeError`). 2. **Explicit Override (`MAM_ENV_FILE`)**: If `MAM_ENV_FILE` is set, only that specific file is parsed. If the specified file does not exist, it is skipped silently and ambient search is refused (no fallback to unintended parent configs): the process runs on its environment and the defaults. If it exists but cannot be read or decoded, that is an error (`EnvFileError`): the wrapper and the Python entry points print one `ERROR: <path>: cannot read (<exception type>)` line and exit 1.
3. **Workspace Root Auto-Discovery**: If `MAM_ENV_FILE` is not set, the resolver searches candidate paths in order: `MAM_REAL_ROOT`, `WORKSPACE_ROOT`, upward directory walk searching for `.agents` or `.git` boundary markers, and `os.getcwd()`. 3. **Workspace Root Auto-Discovery**: If `MAM_ENV_FILE` is not set, the resolver searches candidate paths in order: `MAM_REAL_ROOT`, `WORKSPACE_ROOT`, upward directory walk searching for `.agents` or `.git` boundary markers, and `os.getcwd()`.
4. **Public Broker Security Alert (B-17)**: If the final resolved host falls back to the public sandbox `broker.hivemq.com`, a prominent security warning is emitted. 4. **Public Broker Security Alert (B-17)**: If the final resolved host falls back to the public sandbox `broker.hivemq.com`, a prominent security warning is emitted.
* **Broker Config Resolution (`broker_config_from_job`)**: * **Broker Config Resolution (`broker_config_from_job`)**:
+44
View File
@@ -405,3 +405,47 @@ def test_a6_the_file_cannot_replace_the_wrappers_own_variables(tmp_path):
assert not (tmp_path / "pwned-lib").exists() assert not (tmp_path / "pwned-lib").exists()
got = _wrapper_values(_env(tmp_path, f), ["SCRIPT_DIR", "TARGET_ENV"], tmp_path) got = _wrapper_values(_env(tmp_path, f), ["SCRIPT_DIR", "TARGET_ENV"], tmp_path)
assert _split(got.stdout) == {"SCRIPT_DIR": str(WRAPPER.parent), "TARGET_ENV": str(f)} assert _split(got.stdout) == {"SCRIPT_DIR": str(WRAPPER.parent), "TARGET_ENV": str(f)}
# ---------------------------------------------------------------------------
# A7 - follow-ups F3 (PR #43 review grok)
# ---------------------------------------------------------------------------
def test_a7_a_leading_bom_is_not_part_of_the_first_key(tmp_path):
"""An editor's byte-order mark used to turn the first line into
"invalid key" and drop the first setting, in both loaders."""
f = tmp_path / "bom.env"
f.write_bytes(b"\xef\xbb\xbfFIRST=one\nSECOND=two\n")
env = _env(tmp_path, f)
res = _wrapper_values(env, ["FIRST", "SECOND"], tmp_path)
assert res.returncode == 0, res.stderr
assert _split(res.stdout) == {"FIRST": "one", "SECOND": "two"}
assert "skipped" not in res.stderr, res.stderr
assert _python_values(env, ["FIRST", "SECOND"]) == {"FIRST": "one", "SECOND": "two"}
assert envfile.parse(str(f)) == ([("FIRST", "one"), ("SECOND", "two")], [])
def test_a7_a_repeated_key_keeps_its_first_value_as_documented(tmp_path):
f = tmp_path / "dup.env"
f.write_text("DUP=first\nDUP=second\n")
env = _env(tmp_path, f)
res = _wrapper_values(env, ["DUP"], tmp_path)
assert res.returncode == 0, res.stderr
assert _split(res.stdout) == {"DUP": "first"} == _python_values(env, ["DUP"])
fmt = (REPO_ROOT / ".mam.env.example").read_text().split("# SECURITY:")[0]
assert "a key given twice keeps its FIRST value" in fmt, "document it in the FORMAT section"
def test_a7_a_missing_explicit_file_loads_nothing_and_says_nothing(tmp_path):
"""MAM_ENV_FILE naming no file: no error, and no fallback to the file the
ambient search would find - in both loaders, and so in MESSAGING.md."""
(tmp_path / ".mam.env").write_text("PROBE_AMBIENT=found\n")
absent = tmp_path / "absent.env"
env = _env(tmp_path, absent)
res = _wrapper_values(env, ["PROBE_AMBIENT"], tmp_path)
assert res.returncode == 0, res.stderr
assert _split(res.stdout) == {"PROBE_AMBIENT": "<unset>"}
assert "ERROR" not in res.stderr and str(absent) not in res.stderr, res.stderr
assert _python_values(env, ["PROBE_AMBIENT"]) == {"PROBE_AMBIENT": "<unset>"}
doc = next(l for l in (REPO_ROOT / "MESSAGING.md").read_text().splitlines() if "Explicit Override (`MAM_ENV_FILE`)" in l)
assert "does not exist, it is skipped silently" in doc and "an error is logged" not in doc, doc