fix(sys-token-store): hand a written store to every reader on its stack host

Swarm workers loaded sys_token_store_users before the manager wrote new tokens in pass 1, so they rendered msmtprc with an empty password (CI run 42128590251, WordPress#0). write_many.yml now notes once, before the block, whether the merge changed the store; delegates the merged store to every play host that shares the writer's STACK_HOST; and re-sets the writer's own merge afterwards, so a peer writer's hand-over cannot overwrite it. Verified with lockstep plays on ansible-core 2.20 and 2.21.

🎯 Fix: 93 %
💥 Break: 4 %
💸 Cost: 108 %
⏱️ Time: 116 %
🛡️ Security: 100 %
🏆 Score: 71
🚦 Gate: ⚠️ cost 108 % > 100, time 116 % > 100: ask the operator before the commit

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

roles/sys-token-store/README.md

@@ -100,6 +100,7 @@ Persists several tokens with one store write and at most one re-render of the us
 
 - Merges every entry into the store in list order, so a later entry for the same user and app wins
 - Writes the store file and re-renders the users SPOT once, and only when the merged store differs from the loaded one
+- Hands the merged store to every play host whose `STACK_HOST` holds the written file, so a swarm worker that loaded the store earlier resolves the new token in the same play
 
 ---
 

roles/sys-token-store/tasks/write_many.yml

@@ -28,8 +28,13 @@
       {{ sys_token_store_users | combine(layers, recursive=True) }}
   no_log: "{{ MASK_CREDENTIALS_IN_LOGS | bool }}"
 
+- name: "🧮 Note whether the merge changed the store"
+  ansible.builtin.set_fact:
+    sys_token_store_changed: "{{ sys_token_store_users_merged != sys_token_store_users }}"
+  no_log: "{{ MASK_CREDENTIALS_IN_LOGS | bool }}"
+
 - name: "💾 Write the token store when the merge changed it"
-  when: sys_token_store_users_merged != sys_token_store_users
+  when: sys_token_store_changed | bool
   block:
     - name: "💾 Persist store (idempotent, single-writer via STACK_HOST)"
       ansible.builtin.copy:
@@ -56,6 +61,17 @@
       register: sys_token_store_mirrored
 
     - name: "🔄 Hand the written store to the lookups of the rest of the play"
+      ansible.builtin.set_fact:
+        sys_token_store_users: "{{ sys_token_store_users_merged }}"
+      delegate_to: "{{ sys_token_store_reader }}"
+      delegate_facts: true
+      loop: "{{ ansible_play_hosts }}"
+      loop_control:
+        loop_var: sys_token_store_reader
+      when: hostvars[sys_token_store_reader].STACK_HOST == STACK_HOST
+      no_log: "{{ MASK_CREDENTIALS_IN_LOGS | bool }}"
+
+    - name: "📌 Keep the writer's own merge over a peer writer's hand-over"
       ansible.builtin.set_fact:
         sys_token_store_users: "{{ sys_token_store_users_merged }}"
       no_log: "{{ MASK_CREDENTIALS_IN_LOGS | bool }}"

tests/integration/roles/sys-token-store/__init__.py

@@ -0,0 +1,5 @@
+from pathlib import Path
+
+PROJECT_ROOT: Path = Path(__file__).resolve().parents[4]
+
+__all__ = ["PROJECT_ROOT"]

tests/integration/roles/sys-token-store/test_write_reaches_every_reader.py

@@ -0,0 +1,221 @@
+from __future__ import annotations
+
+import json
+import os
+import shutil
+import subprocess
+import sys
+import tempfile
+import unittest
+from pathlib import Path
+from typing import ClassVar
+
+from utils.cache.yaml import dump_yaml, load_yaml_any
+
+from . import PROJECT_ROOT
+
+WRITE_MANY = PROJECT_ROOT / "roles" / "sys-token-store" / "tasks" / "write_many.yml"
+MERGE = "🔨 Build merged structure"
+CHANGED = "🧮 Note whether the merge changed the store"
+WRITES = (
+    "💾 Persist store (idempotent, single-writer via STACK_HOST)",
+    "🪞 Mirror the store onto the controller, where every users lookup reads",
+)
+
+LOADED = {"no-reply": {"tokens": {"web-app-matomo": "loaded"}}}
+
+
+def _with(app: str, token: str) -> dict:
+    return {"no-reply": {"tokens": {"web-app-matomo": "loaded", app: token}}}
+
+
+def _entries(app: str, token: str) -> list[dict]:
+    return [{"user_key": "no-reply", "app": app, "token": token}]
+
+
+PLAYS = {
+    "one_writer": {
+        "mgr": ("mgr", LOADED, _entries("web-app-stalwart", "minted")),
+        "wrk": ("mgr", LOADED, None),
+        "solo": ("solo", LOADED, None),
+    },
+    "writers_on_two_stack_hosts": {
+        "c1": ("c1", LOADED, _entries("web-app-c1", "one")),
+        "r1": ("c1", LOADED, None),
+        "c2": ("c2", LOADED, _entries("web-app-c2", "two")),
+    },
+    "unchanged_first_writer": {
+        "e1": ("e1", _with("web-app-e", "same"), _entries("web-app-e", "same")),
+        "e2": ("e1", LOADED, _entries("web-app-e", "same")),
+        "e3": ("e1", LOADED, None),
+    },
+    "writers_on_one_stack_host": {
+        "w1": ("w1", LOADED, _entries("web-app-w1", "one")),
+        "w2": ("w1", LOADED, _entries("web-app-w2", "two")),
+        "r2": ("w1", LOADED, None),
+    },
+}
+
+
+def _writer_tasks() -> list[dict]:
+    """Return the merge, the change check and the store-write block of ``write_many.yml``.
+
+    Returns:
+        The real merge and change-check tasks, then the block with its own
+        ``when`` gate minus the stack host write and the controller mirror.
+    """
+    tasks = load_yaml_any(str(WRITE_MANY))
+    named = {task.get("name"): task for task in tasks}
+    blocks = [task for task in tasks if "block" in task]
+    if MERGE not in named or CHANGED not in named or len(blocks) != 1:
+        raise AssertionError(f"{WRITE_MANY} lost {MERGE!r}, {CHANGED!r} or its block")
+    kept = [task for task in blocks[0]["block"] if task.get("name") not in WRITES]
+    if len(kept) != len(blocks[0]["block"]) - len(WRITES):
+        raise AssertionError(f"{WRITE_MANY} lost one of {WRITES!r}")
+    return [named[MERGE], named[CHANGED], {**blocks[0], "block": kept}]
+
+
+def _host(stack_host: str, loaded: dict, entries: list[dict] | None) -> dict:
+    host = {
+        "ansible_connection": "local",
+        "ansible_python_interpreter": sys.executable,
+        "STACK_HOST": stack_host,
+        "MASK_CREDENTIALS_IN_LOGS": False,
+        "sys_token_store_loaded": loaded,
+    }
+    if entries is not None:
+        host["sys_token_store_entries"] = entries
+    return host
+
+
+def _play(group: str, writer: Path, views: Path) -> dict:
+    return {
+        "hosts": group,
+        "gather_facts": False,
+        "tasks": [
+            {
+                "name": "Load the store on every host",
+                "ansible.builtin.set_fact": {
+                    "sys_token_store_users": "{{ sys_token_store_loaded }}"
+                },
+            },
+            {
+                "name": "Write the tokens of the writers in lockstep",
+                "ansible.builtin.include_tasks": str(writer),
+                "when": "sys_token_store_entries is defined",
+            },
+            {
+                "name": "Report the store every host resolves",
+                "ansible.builtin.copy": {
+                    "content": "{{ sys_token_store_users | to_json }}",
+                    "dest": f"{views}/{{{{ inventory_hostname }}}}.json",
+                    "mode": "0644",
+                },
+            },
+        ],
+    }
+
+
+def _run_plays(workdir: Path) -> dict[str, dict]:
+    """Load the store on every host, write on the writers only, report every view.
+
+    Args:
+        workdir: an empty directory to build the inventory and the plays in.
+
+    Returns:
+        The ``sys_token_store_users`` fact of each host after the write.
+
+    Raises:
+        RuntimeError: when ansible-playbook exits non-zero.
+    """
+    views = workdir / "views"
+    views.mkdir()
+    writer = workdir / "writer.yml"
+    groups = {
+        group: {"hosts": {name: _host(*spec) for name, spec in hosts.items()}}
+        for group, hosts in PLAYS.items()
+    }
+    dump_yaml(workdir / "inventory.yml", {"all": {"children": groups}})
+    dump_yaml(writer, _writer_tasks())
+    dump_yaml(workdir / "play.yml", [_play(group, writer, views) for group in PLAYS])
+    env = dict(os.environ)
+    env.update(
+        {
+            "ANSIBLE_CONFIG": str(PROJECT_ROOT / "ansible.cfg"),
+            "ANSIBLE_LOCAL_TEMP": str(workdir / "ansible-local"),
+            "ANSIBLE_REMOTE_TEMP": str(workdir / "ansible-remote"),
+            "ANSIBLE_NOCOLOR": "1",
+        }
+    )
+    result = subprocess.run(
+        [
+            "ansible-playbook",
+            "-i",
+            str(workdir / "inventory.yml"),
+            str(workdir / "play.yml"),
+        ],
+        cwd=str(PROJECT_ROOT),
+        env=env,
+        capture_output=True,
+        text=True,
+        check=False,
+    )
+    if result.returncode != 0:
+        raise RuntimeError(
+            f"ansible-playbook failed\nSTDOUT:\n{result.stdout}\nSTDERR:\n{result.stderr}"
+        )
+    return {
+        name: json.loads((views / f"{name}.json").read_text())
+        for hosts in PLAYS.values()
+        for name in hosts
+    }
+
+
+@unittest.skipUnless(shutil.which("ansible-playbook"), "ansible-playbook is required")
+class TestWriteReachesEveryReader(unittest.TestCase):
+    views: ClassVar[dict[str, dict]] = {}
+
+    @classmethod
+    def setUpClass(cls) -> None:
+        with tempfile.TemporaryDirectory() as tmp:
+            cls.views = _run_plays(Path(tmp))
+
+    def test_the_writer_resolves_its_own_write(self) -> None:
+        self.assertEqual(self.views["mgr"], _with("web-app-stalwart", "minted"))
+
+    def test_a_reader_of_the_same_stack_host_resolves_the_write(self) -> None:
+        self.assertEqual(
+            self.views["wrk"],
+            _with("web-app-stalwart", "minted"),
+            "a swarm worker that loaded the store before the write must not "
+            "render the new token empty for the rest of the play",
+        )
+
+    def test_a_host_with_its_own_stack_host_keeps_its_store(self) -> None:
+        self.assertEqual(self.views["solo"], LOADED)
+
+    def test_lockstep_writers_on_two_stack_hosts_keep_their_own_writes(self) -> None:
+        self.assertEqual(self.views["c1"], _with("web-app-c1", "one"))
+        self.assertEqual(self.views["c2"], _with("web-app-c2", "two"))
+
+    def test_a_reader_resolves_only_the_writer_of_its_stack_host(self) -> None:
+        self.assertEqual(self.views["r1"], _with("web-app-c1", "one"))
+
+    def test_a_changed_writer_behind_an_unchanged_one_still_hands_over(self) -> None:
+        for name in ("e1", "e2", "e3"):
+            with self.subTest(host=name):
+                self.assertEqual(self.views[name], _with("web-app-e", "same"))
+
+    def test_lockstep_writers_on_one_stack_host_keep_their_own_writes(self) -> None:
+        self.assertEqual(self.views["w1"], _with("web-app-w1", "one"))
+        self.assertEqual(self.views["w2"], _with("web-app-w2", "two"))
+
+    def test_a_reader_of_two_lockstep_writers_resolves_one_write(self) -> None:
+        self.assertIn(
+            self.views["r2"],
+            [_with("web-app-w1", "one"), _with("web-app-w2", "two")],
+        )
+
+
+if __name__ == "__main__":
+    unittest.main()

tests/unit/python/roles/sys-token-store/test_write_many.py

@@ -15,6 +15,8 @@ EMPTY = "🛑 Abort if a token is empty (do not persist empties)"
 PERSIST = "💾 Persist store (idempotent, single-writer via STACK_HOST)"
 MIRROR = "🪞 Mirror the store onto the controller, where every users lookup reads"
 HAND_OVER = "🔄 Hand the written store to the lookups of the rest of the play"
+KEEP_OWN = "📌 Keep the writer's own merge over a peer writer's hand-over"
+CHANGED = "🧮 Note whether the merge changed the store"
 RERENDER = "🧊 Re-render the users SPOT the new tokens invalidated"
 
 USERS = {
@@ -74,6 +76,15 @@ def _merged(entries: list[dict], users: dict = USERS) -> dict:
     )
 
 
+def _state(merged: dict, users: dict = USERS) -> dict:
+    variables = {"sys_token_store_users_merged": merged, "sys_token_store_users": users}
+    changed = _render(
+        _task(CHANGED)["ansible.builtin.set_fact"]["sys_token_store_changed"],
+        **variables,
+    )
+    return {**variables, "sys_token_store_changed": changed}
+
+
 def _when(name: str, **variables) -> bool:
     conditions = _gated(name)[1]
     if not conditions:
@@ -89,6 +100,14 @@ def _entry(user_key: str, app: str, token: str) -> dict:
     return {"user_key": user_key, "app": app, "token": token}
 
 
+def _reader(reader: str, reader_stack_host: str) -> dict:
+    return {
+        "sys_token_store_reader": reader,
+        "hostvars": {reader: {"STACK_HOST": reader_stack_host}},
+        "STACK_HOST": "mgr",
+    }
+
+
 class TestWriteMany(unittest.TestCase):
     def test_every_entry_lands_in_one_merge(self):
         merged = _merged(
@@ -124,10 +143,11 @@ class TestWriteMany(unittest.TestCase):
             ]
         )
         self.assertEqual(merged, USERS)
-        state = {"sys_token_store_users_merged": merged, "sys_token_store_users": USERS}
+        state = _state(merged)
         self.assertFalse(_when(PERSIST, **state))
         self.assertFalse(_when(MIRROR, **state))
         self.assertFalse(_when(HAND_OVER, **state))
+        self.assertFalse(_when(KEEP_OWN, **state))
         self.assertFalse(
             _when(
                 RERENDER,
@@ -138,9 +158,9 @@ class TestWriteMany(unittest.TestCase):
 
     def test_a_changed_store_is_written_and_rerendered_once(self):
         merged = _merged([_entry("bob", "web-app-b", "token-b")])
-        state = {"sys_token_store_users_merged": merged, "sys_token_store_users": USERS}
+        state = _state(merged)
         self.assertTrue(_when(PERSIST, **state))
-        self.assertTrue(_when(HAND_OVER, **state))
+        self.assertTrue(_when(HAND_OVER, **state, **_reader("wrk", "mgr")))
         self.assertTrue(
             _when(
                 RERENDER,
@@ -154,6 +174,37 @@ class TestWriteMany(unittest.TestCase):
         self.assertEqual(len(rerenders), 1)
         self.assertNotIn("loop", rerenders[0])
 
+    def test_the_written_store_reaches_only_the_readers_of_its_stack_host(self):
+        merged = _merged([_entry("bob", "web-app-b", "token-b")])
+        state = _state(merged)
+        self.assertTrue(_when(HAND_OVER, **state, **_reader("wrk", "mgr")))
+        self.assertFalse(_when(HAND_OVER, **state, **_reader("solo", "solo")))
+        hand_over = _task(HAND_OVER)
+        self.assertEqual(hand_over["delegate_to"], "{{ sys_token_store_reader }}")
+        self.assertIs(hand_over["delegate_facts"], True)
+
+    def test_every_changed_writer_hands_over_its_own_store(self):
+        self.assertNotIn("run_once", _task(HAND_OVER))
+
+    def test_every_writer_ends_on_its_own_merge(self):
+        names = [task.get("name") for task, _ in _flat("write_many.yml")]
+        self.assertGreater(names.index(KEEP_OWN), names.index(HAND_OVER))
+        keep_own = _task(KEEP_OWN)
+        self.assertEqual(
+            keep_own["ansible.builtin.set_fact"],
+            {"sys_token_store_users": "{{ sys_token_store_users_merged }}"},
+        )
+        for keyword in ("delegate_to", "delegate_facts", "run_once", "loop"):
+            with self.subTest(keyword=keyword):
+                self.assertNotIn(keyword, keep_own)
+        merged = _merged([_entry("bob", "web-app-b", "token-b")])
+        self.assertTrue(_when(KEEP_OWN, **_state(merged)))
+
+    def test_the_hand_over_outlives_the_writers_own_fact_update(self):
+        merged = _merged([_entry("bob", "web-app-b", "token-b")])
+        after_writer_item = {**_state(merged), "sys_token_store_users": merged}
+        self.assertTrue(_when(HAND_OVER, **after_writer_item, **_reader("wrk", "mgr")))
+
     def test_the_store_has_one_writer(self):
         persist = _task(PERSIST)
         self.assertEqual(persist["delegate_to"], "{{ STACK_HOST }}")