fix(mcp): refuse the environment writes that resolve to a wrong or hidden entry

.apm/skills/backend-configuration/SKILL.md

@@ -309,10 +309,10 @@ unchecked stack is not persisted, and the fix is the user's.
 An **environment switch** — "target prod", "clear the environment" —
 is a write of the `environment` field and nothing else:
 `odd_config_set {"environment": "<name>"}` (kebab-case, never
-`unknown`), or `{"environment": null}` to clear it. It selects which
+`unknown` or `local`), or `{"environment": null}` to clear it. It selects which
 of the configured stack's entries the missions read (`## Check` step
-2's effective entry) and changes no `stack_config` value; on the local
-stack it is inert. A switch that names both ("switch to cloudwatch in
+2's effective entry) and changes no `stack_config` value; the local
+stack refuses it. A switch that names both ("switch to cloudwatch in
 prod") writes both fields in the one call. Like every path it ends at
 verification (step 5).
 

docs/guide/plugin.md

@@ -564,7 +564,7 @@ than the local otel-lgtm container.
 | `odd_stack_status` | Probe whether it is up — and get the container's identity too: `image`, `created`/`started` timestamps, and its user-set `env` (credential-named values redacted to `null`; all four `null` when there is no container) — plus `daemon` (`"ok"`, or `"unreachable"` with `daemon_remedy`, the one-line remedy, when the Docker daemon does not answer or no `docker` binary is on PATH) | — |
 | `odd_stack_reset` | Wipe all stored telemetry and return a fresh, ready stack — the next run starts from a clean slate | `env` (optional) — always applies, the container is recreated; persisted/reapplied like `odd_stack_up` |
 | `odd_config_get` | Read the global configuration — stack backend, the configured deployment environment (`null` when none), local host ports, targeting values per stack and per environment (`stack_config`, keyed `<stack>` or `<environment>-<stack>`), custom stack declarations, and `effective`: the entry resolved for the configured stack and environment (`stack_config_key` and its `stack_config` — the environment's entry when one is persisted, the stack's plain one otherwise, never a merge) — and the installed `oddyssey-mcp` version | — |
-| `odd_config_set` | Update it — a port change resets the stack so the new value applies right away | `config` — partial merge, e.g. `{"local": {"grafana_port": 3300}}`, `{"environment": "prod"}` (`null` clears it; inert on the local stack); `stack_config` keys are `<stack>` or `<environment>-<stack>` (kebab-case environment, never `unknown`; `local` takes no prefix), each accepting the stack's fields; inside `stack_config` and `custom`, `null` deletes a key or an entry; a stack outside the built-in list needs a `custom` declaration, never one that reads as `<environment>-<known stack>` |
+| `odd_config_set` | Update it — a port change resets the stack so the new value applies right away | `config` — partial merge, e.g. `{"local": {"grafana_port": 3300}}`, `{"environment": "prod"}` (`null` clears it; refused on the local stack, cleared by a switch to it); `stack_config` keys are `<stack>` or `<environment>-<stack>` (kebab-case environment, never `unknown` or `local`; `local` takes no prefix), each accepting the stack's fields; inside `stack_config` and `custom`, `null` deletes a key or an entry; a stack outside the built-in list needs a `custom` declaration, never one that reads as `<environment>-<known stack>` or that a built-in ends in |
 
 The server is instrumented with OpenTelemetry and, by default, exports its
 own traces and metrics to the local stack (`http://localhost:4318`, OTLP

integration-tests/mcp-server/test-config-surface.sh

@@ -6,7 +6,8 @@
 # the caller (#228) is accepted, validated and removed, an
 # environment-prefixed stack_config key merges and deletes next to the
 # stack's plain one while the environment field selects the effective
-# entry (#618), grafana persists its gcx context name and nothing else
+# entry (#618) and refuses an environment on the local stack (#656),
+# grafana persists its gcx context name and nothing else
 # (#619), and the tolerant read lists
 # hand-edited invalid values in invalid_ignored instead of crashing,
 # and the read carries the installed oddyssey-mcp version (#395).
@@ -202,17 +203,19 @@ jq -e '.content[0].text | fromjson | .effective.environment == "dev" and .effect
   || { echo "ASSERTION FAILED: fallback to the plain entry did not apply" >&2; cat "$workdir/env-dev.json" >&2; exit 1; }
 
 step "an invalid prefix is rejected and writes nothing (#618)"
-for key in "unknown-cloudwatch" "prod-nagios" "Prod-cloudwatch" "prod-local"; do
+for key in "unknown-cloudwatch" "local-cloudwatch" "prod-nagios" "Prod-cloudwatch" "prod-local"; do
   mcp_call odd_config_set "config={\"stack_config\":{\"$key\":{}}}" > "$workdir/env-bad-key.json" || true
   grep -q "stack_config keys" "$workdir/env-bad-key.json" \
     || { echo "ASSERTION FAILED: key $key was not rejected" >&2; cat "$workdir/env-bad-key.json" >&2; exit 1; }
 done
 mcp_call odd_config_set 'config={"stack_config":{"prod-cloudwatch":{"workspace":"x"}}}' > "$workdir/env-bad-field.json" || true
 grep -q "accepts only" "$workdir/env-bad-field.json" \
   || { echo "ASSERTION FAILED: a field outside the stack's list was not rejected on a prefixed key" >&2; cat "$workdir/env-bad-field.json" >&2; exit 1; }
-mcp_call odd_config_set 'config={"environment":"Prod"}' > "$workdir/env-bad-value.json" || true
-grep -q "environment must be" "$workdir/env-bad-value.json" \
-  || { echo "ASSERTION FAILED: an invalid environment was not rejected" >&2; cat "$workdir/env-bad-value.json" >&2; exit 1; }
+for value in "Prod" "local"; do
+  mcp_call odd_config_set "config={\"environment\":\"$value\"}" > "$workdir/env-bad-value.json" || true
+  grep -q "environment must be" "$workdir/env-bad-value.json" \
+    || { echo "ASSERTION FAILED: environment $value was not rejected" >&2; cat "$workdir/env-bad-value.json" >&2; exit 1; }
+done
 mcp_call odd_config_get > "$workdir/after-env-bad.json"
 jq -e '.content[0].text | fromjson | .environment == "dev" and (.stack_config | keys) == ["cloudwatch","pre-prod-azure-monitor","prod-cloudwatch"] and (.stack_config["prod-cloudwatch"] | has("workspace") | not)' \
   "$workdir/after-env-bad.json" > /dev/null \
@@ -222,25 +225,32 @@ step "a custom name that reads as <environment>-<known stack> is refused (#618)"
 mcp_call odd_config_set 'config={"custom":{"prod-cloudwatch":{"stack_config_fields":["log_group"]}}}' > "$workdir/env-custom.json" || true
 grep -q "reads as the" "$workdir/env-custom.json" \
   || { echo "ASSERTION FAILED: custom name prod-cloudwatch was not refused" >&2; cat "$workdir/env-custom.json" >&2; exit 1; }
+# Nor one a built-in ends in: monitor would make azure-monitor read as azure + monitor (#656).
+mcp_call odd_config_set 'config={"custom":{"monitor":{"stack_config_fields":["base_url"]}}}' > "$workdir/env-custom-builtin.json" || true
+grep -q "would make the built-in stack 'azure-monitor'" "$workdir/env-custom-builtin.json" \
+  || { echo "ASSERTION FAILED: custom name monitor was not refused" >&2; cat "$workdir/env-custom-builtin.json" >&2; exit 1; }
 
-step "null deletes a key and an entry under a prefixed key, and clears the environment (#618)"
+step "null deletes a key and an entry under a prefixed key; a switch to local clears the environment (#618, #656)"
 mcp_call odd_config_set 'config={"stack_config":{"prod-cloudwatch":{"region":null}}}' > "$workdir/env-del-key.json"
 jq -e '.content[0].text | fromjson | .config.stack_config["prod-cloudwatch"] == {"log_group":"/example/prod-logs"}' \
   "$workdir/env-del-key.json" > /dev/null \
   || { echo "ASSERTION FAILED: null key deletion on a prefixed entry" >&2; cat "$workdir/env-del-key.json" >&2; exit 1; }
 mcp_call odd_config_set \
-  'config={"environment":null,"stack":"local","stack_config":{"prod-cloudwatch":null,"pre-prod-azure-monitor":null,"cloudwatch":null}}' \
+  'config={"stack":"local","stack_config":{"prod-cloudwatch":null,"pre-prod-azure-monitor":null,"cloudwatch":null}}' \
   > "$workdir/env-clear.json"
 jq -e '.content[0].text | fromjson | .config.environment == null and .config.stack_config == {} and .config.effective.stack_config_key == "local"' \
   "$workdir/env-clear.json" > /dev/null \
   || { echo "ASSERTION FAILED: clearing the environment and the prefixed entries" >&2; cat "$workdir/env-clear.json" >&2; exit 1; }
 
-step "on the local stack the environment is inert (#618)"
-mcp_call odd_config_set 'config={"environment":"prod"}' > "$workdir/env-local.json"
-jq -e '.content[0].text | fromjson | .config.environment == "prod" and .config.effective == {"stack":"local","environment":null,"stack_config_key":"local","stack_config":{}}' \
-  "$workdir/env-local.json" > /dev/null \
-  || { echo "ASSERTION FAILED: the environment selected something on the local stack" >&2; cat "$workdir/env-local.json" >&2; exit 1; }
-mcp_call odd_config_set 'config={"environment":null}' > /dev/null
+step "the local stack takes no environment; a remote switch in the same call does (#656)"
+mcp_call odd_config_set 'config={"environment":"prod"}' > "$workdir/env-local.json" || true
+grep -q "the local stack takes no environment" "$workdir/env-local.json" \
+  || { echo "ASSERTION FAILED: an environment was stored on the local stack" >&2; cat "$workdir/env-local.json" >&2; exit 1; }
+mcp_call odd_config_set 'config={"stack":"cloudwatch","environment":"prod"}' > "$workdir/env-remote.json"
+jq -e '.content[0].text | fromjson | .config.effective.stack == "cloudwatch" and .config.effective.environment == "prod"' \
+  "$workdir/env-remote.json" > /dev/null \
+  || { echo "ASSERTION FAILED: a remote switch with an environment was not accepted" >&2; cat "$workdir/env-remote.json" >&2; exit 1; }
+mcp_call odd_config_set 'config={"stack":"local"}' > /dev/null
 
 step "a non-scalar stack_config value is rejected and writes nothing"
 mcp_call odd_config_set \

src/mcp-server/app/config.py

@@ -71,8 +71,9 @@
 # a remote backend's targeting values differ per deployment environment,
 # so one stack may hold one entry per environment next to its plain one.
 # The environment is kebab-case with no trailing dash - stricter than
-# CUSTOM_NAME_RE - and never the sentinel the agents record when the
-# telemetry carries no environment. The key is parsed by suffix, the
+# CUSTOM_NAME_RE - never the sentinel the agents record when the
+# telemetry carries no environment, never "local" - the local stack's
+# environment by construction, which takes no other (#656). The key is parsed by suffix, the
 # longest known stack winning ("dev-azure-monitor" is dev + azure-monitor,
 # "pre-prod-cloudwatch" is pre-prod + cloudwatch); "local" takes no
 # prefix - the local stack is the local environment by construction. The
@@ -81,10 +82,11 @@
 # one: two whole entries, never a merge.
 ENVIRONMENT_RE = re.compile(r"^[a-z]([a-z0-9-]*[a-z0-9])?$")
 ENVIRONMENT_SENTINEL = "unknown"
+NOT_ENVIRONMENTS = frozenset({ENVIRONMENT_SENTINEL, "local"})
 KEY_FORMS = (
     "<stack> or <environment>-<stack> - <stack> a built-in or a declared custom"
     " stack, <environment> kebab-case (^[a-z]([a-z0-9-]*[a-z0-9])?$) and never"
-    f" {ENVIRONMENT_SENTINEL!r}; local takes no prefix"
+    f" {ENVIRONMENT_SENTINEL!r} or 'local'; local takes no prefix"
 )
 
 DEFAULTS = {
@@ -128,7 +130,7 @@ def _valid_environment(value: object) -> bool:
     return (
         isinstance(value, str)
         and ENVIRONMENT_RE.fullmatch(value) is not None
-        and value != ENVIRONMENT_SENTINEL
+        and value not in NOT_ENVIRONMENTS
     )
 
 
@@ -169,15 +171,24 @@ def _stack_config_key_allowed(stack: str, key: str, custom: dict) -> bool:
     return fields is None or key in fields
 
 
-def _effective_entry(stack: str, environment: str | None, stack_config: dict) -> dict:
+def _effective_entry(
+    stack: str, environment: str | None, stack_config: dict, custom: dict
+) -> dict:
     """The entry the configured pair resolves to: the environment's when
     one is persisted for it, the stack's plain one otherwise - whole, never
-    merged. On the local stack the environment is inert."""
+    merged. On the local stack the environment is inert. The composed key
+    must parse back to the pair: a known stack's own entry (a hand-edited
+    "monitor" next to azure-monitor, #656) is never another pair's."""
     if stack == "local":
         environment = None
     key = stack
-    if environment is not None and f"{environment}-{stack}" in stack_config:
-        key = f"{environment}-{stack}"
+    prefixed = f"{environment}-{stack}"
+    if (
+        environment is not None
+        and prefixed in stack_config
+        and resolve_stack_config_key(prefixed, custom) == (environment, stack)
+    ):
+        key = prefixed
     return {
         "stack": stack,
         "environment": environment,
@@ -218,15 +229,15 @@ def load(path: Path | None = None) -> dict:
     try:
         stored = json.loads(target.read_text())
     except FileNotFoundError:
-        effective["effective"] = _effective_entry("local", None, {})
+        effective["effective"] = _effective_entry("local", None, {}, {})
         return effective
     except (OSError, ValueError):
         effective["invalid_ignored"] = ["<file>"]
-        effective["effective"] = _effective_entry("local", None, {})
+        effective["effective"] = _effective_entry("local", None, {}, {})
         return effective
     if not isinstance(stored, dict):
         effective["invalid_ignored"] = ["<file>"]
-        effective["effective"] = _effective_entry("local", None, {})
+        effective["effective"] = _effective_entry("local", None, {}, {})
         return effective
 
     # Declarations first: the stack and the stack_config entries below are
@@ -306,7 +317,10 @@ def load(path: Path | None = None) -> dict:
     if invalid:
         effective["invalid_ignored"] = invalid
     effective["effective"] = _effective_entry(
-        effective["stack"], effective["environment"], effective["stack_config"]
+        effective["stack"],
+        effective["environment"],
+        effective["stack_config"],
+        effective["custom"],
     )
     return effective
 
@@ -340,7 +354,8 @@ def _validate_custom(partial: dict, stored_custom: dict) -> dict:
         # A name readable as <environment>-<known stack> would make a
         # stack_config key readable two ways (#618): refused in both
         # directions - the name itself, and a name that would turn an
-        # already-declared one into its environment entry.
+        # already-declared one, or a built-in (#656), into its
+        # environment entry.
         known = frozenset(STACKS) | frozenset(effective) - {name}
         parsed = _split_prefixed(name, known)
         if parsed is not None:
@@ -349,6 +364,15 @@ def _validate_custom(partial: dict, stored_custom: dict) -> dict:
                 f" entry of stack {parsed[1]!r} (stack_config keys are"
                 f" <environment>-<stack>) - pick a name that ends in no known stack"
             )
+        for builtin in STACKS:
+            split = _split_prefixed(builtin, known | {name})
+            if split is not None and split[1] == name:
+                raise ValueError(
+                    f"custom.{name}: declaring {name!r} would make the built-in"
+                    f" stack {builtin!r} read as its environment entry"
+                    f" (stack_config keys are <environment>-<stack>) - pick a"
+                    f" name that no built-in stack ends in"
+                )
         for other in sorted(effective):
             if other != name and _split_prefixed(other, known | {name}) is not None:
                 raise ValueError(
@@ -391,10 +415,18 @@ def save(partial: dict, path: Path | None = None) -> dict:
     ):
         raise ValueError(
             "environment must be a kebab-case name"
-            f" (^[a-z]([a-z0-9-]*[a-z0-9])?$), never {ENVIRONMENT_SENTINEL!r},"
-            f" or null to clear it, got {partial['environment']!r}"
+            f" (^[a-z]([a-z0-9-]*[a-z0-9])?$), never {ENVIRONMENT_SENTINEL!r}"
+            f" or 'local', or null to clear it, got {partial['environment']!r}"
         )
     effective_stack = partial.get("stack", before["stack"])
+    # The local stack is the local environment by construction (#656): an
+    # environment written there would stay invisible and re-arm on the
+    # next remote switch - refused, and a switch to local clears it (below).
+    if effective_stack == "local" and partial.get("environment") is not None:
+        raise ValueError(
+            "the local stack takes no environment - switch the stack first,"
+            " or in the same call"
+        )
     removed = {name for name, decl in partial.get("custom", {}).items() if decl is None}
     if effective_stack in removed:
         raise ValueError(
@@ -482,11 +514,10 @@ def save(partial: dict, path: Path | None = None) -> dict:
         stored = {}
     if "stack" in partial:
         stored["stack"] = partial["stack"]
-    if "environment" in partial:
-        if partial["environment"] is None:
-            stored.pop("environment", None)
-        else:
-            stored["environment"] = partial["environment"]
+    if effective_stack == "local" or partial.get("environment", "") is None:
+        stored.pop("environment", None)
+    elif "environment" in partial:
+        stored["environment"] = partial["environment"]
     if local_partial:
         stored_local = stored.get("local")
         stored["local"] = {

src/mcp-server/app/server.py

@@ -120,7 +120,8 @@ def odd_config_set(config: dict) -> dict:
     {"stack": "seq", "custom": {"seq": {"stack_config_fields":
     ["base_url"]}}}. A custom name is kebab-case, never a built-in one and
     never one that reads as <environment>-<known stack> (prod-cloudwatch,
-    or prod-seq once seq is declared - refused in both directions); its
+    or prod-seq once seq is declared - refused in both directions), and
+    never a built-in one ends in (monitor, for azure-monitor); its
     declaration lists the stack_config fields the stack's guide names (an
     empty list when it persists nothing), and a re-declaration replaces
     the list. The server never reads the stack's guide - the caller derives
@@ -153,7 +154,7 @@ def odd_config_set(config: dict) -> dict:
     deployment environment next to the stack's plain one, e.g.
     {"stack_config": {"prod-cloudwatch": {"log_group": "<log_group>"}}}: the
     environment is kebab-case (^[a-z]([a-z0-9-]*[a-z0-9])?$, never
-    "unknown"), the key is parsed by suffix (dev-azure-monitor is dev +
+    "unknown" or "local"), the key is parsed by suffix (dev-azure-monitor is dev +
     azure-monitor, pre-prod-cloudwatch is pre-prod + cloudwatch - the
     longest known stack wins), a key whose suffix is no known stack is
     rejected, and local takes no prefix. stack_config is merged per key
@@ -178,11 +179,13 @@ def odd_config_set(config: dict) -> dict:
     container either.
     environment selects, for the configured stack, which entry the missions
     read: {"environment": "prod"} persists it (the same pattern as a key's
-    prefix, "unknown" refused), {"environment": null} clears it; absent by
-    default. odd_config_get's effective block resolves the configured pair
-    - the <environment>-<stack> entry when one is persisted, else the
-    plain <stack> entry, whole, never merged. On the local stack the field
-    is inert (the environment is local by construction).
+    prefix, "unknown" and "local" refused), {"environment": null} clears
+    it; absent by default. odd_config_get's effective block resolves the
+    configured pair - the <environment>-<stack> entry when one is
+    persisted, else the plain <stack> entry, whole, never merged. The local
+    stack takes no environment (it is local by construction): the write
+    is refused there - switch the stack first, or in the same call - and a
+    switch to local clears the stored one.
     """
     ports_before = config_ops.load()["local"]
     # Read on the RAW partial, before save validates it: a malformed one