diff --git a/.agents/skills/lib_py/delegation/envfile.py b/.agents/skills/lib_py/delegation/envfile.py index 6efbefb..fff4c28 100644 --- a/.agents/skills/lib_py/delegation/envfile.py +++ b/.agents/skills/lib_py/delegation/envfile.py @@ -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 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). -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 ``--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]] = [] warnings: List[str] = [] 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() except (OSError, UnicodeDecodeError) as e: raise EnvFileError(f"{path}: cannot read ({type(e).__name__})") from None diff --git a/.mam.env.example b/.mam.env.example index 02d6939..92f428b 100644 --- a/.mam.env.example +++ b/.mam.env.example @@ -21,7 +21,9 @@ # - `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, # 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 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). # # SECURITY: never put real secrets in this template. Secret-bearing vars use a diff --git a/MESSAGING.md b/MESSAGING.md index f6155ea..ada0492 100644 --- a/MESSAGING.md +++ b/MESSAGING.md @@ -366,7 +366,7 @@ The subscriber acts as the central execution watchdog: * **`.mam.env` Resolution Hierarchy (`_load_dotenv`)**: 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. - 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: : cannot read ()` 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()`. 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`)**: diff --git a/tests/test_issue26_envfile.py b/tests/test_issue26_envfile.py index f7b6c76..4017832 100644 --- a/tests/test_issue26_envfile.py +++ b/tests/test_issue26_envfile.py @@ -405,3 +405,47 @@ def test_a6_the_file_cannot_replace_the_wrappers_own_variables(tmp_path): assert not (tmp_path / "pwned-lib").exists() 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)} + + +# --------------------------------------------------------------------------- +# 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": ""} + assert "ERROR" not in res.stderr and str(absent) not in res.stderr, res.stderr + assert _python_values(env, ["PROBE_AMBIENT"]) == {"PROBE_AMBIENT": ""} + 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