fix(security): re-judge a remote hosts alias after a mid-parse publish

src/kiro_crew/security/argv_floor.py

@@ -1425,6 +1425,7 @@ def _hosts_file_paths() -> "tuple[str, ...]":
 # parse in the same call on a miss: a typical file in well under a
 # millisecond, one at the cap in a few.
 _HOSTS_FILE_READ_CHUNK = 64 * 1024
+_HOSTS_KEY_MOVED_ATTEMPTS = 3  # passes per call; one enrichment pass moves the key at most twice
 
 
 # path -> (key, {name -> maps-to-local}), key = (mtime, ctime, size, content
@@ -1559,7 +1560,7 @@ def _parse_and_cache_for_key(
     vet is what starts the worker), the table is returned but NOT cached:
     it was judged against the older own set, so a remote entry could be an
     alias for the newly learned address, and the gate's own-state re-check
-    holds exactly those entries pending.  The file itself is the one the
+    judges exactly those entries again.  The file itself is the one the
     key names, so the table's NAME SET is exact: a local entry stays local
     under a grown own set, and a name absent from the file is absent under
     any own set.  Answering those two from the uncached table is what keeps
@@ -1692,60 +1693,59 @@ def _hosts_file_verdict(host: str) -> "bool | None":
     hostname or an IP address.  An in-call parse whose FILE changed
     while it ran is also pending; one whose own-address state moved (the
     enrichment worker publishing mid-parse) still answers a local or an
-    absent name from the table it built, and holds a remote entry pending
-    through the own-state check below.  A remote answer is given
-    only if publication and the own set still match the table's key at the
-    moment of return.  A name on several lines is local if ANY of them maps
-    local (deny-floor direction).
+    absent name from the table it built.  A remote answer is given only if
+    publication and the own set still match its table's key at return;
+    otherwise every path is judged again on the current key, at most
+    ``_HOSTS_KEY_MOVED_ATTEMPTS`` passes per call (so at most that many parses
+    of one file), then the answer is pending.  A name on several lines is
+    local if ANY of them maps local (deny-floor direction).
     """
-    tables: "list[tuple[_HostsKey, dict[str, bool]]]" = []
-    for path in _hosts_file_paths():
-        try:
-            key = _hosts_file_key(path)
-        except _HostsFileUnreadable:
-            _schedule_hosts_file_warm()
-            return True
-        except OSError:
-            continue
-        if _unverifiable_hosts_file(key):
-            return True
-        cached = _HOSTS_FILE_CACHE.get(path)
-        if cached is None or cached[0] != key:
-            on_loop = _event_loop_running()
-            if key[2] > _HOSTS_FILE_READ_CHUNK and on_loop:
-                # Every call on an uncached key re-schedules the warm, so a
-                # retry does not hang on one thread that failed to start.
+    for _attempt in range(_HOSTS_KEY_MOVED_ATTEMPTS):
+        tables: "list[tuple[_HostsKey, dict[str, bool]]]" = []
+        for path in _hosts_file_paths():
+            try:
+                key = _hosts_file_key(path)
+            except _HostsFileUnreadable:
                 _schedule_hosts_file_warm()
                 return True
-            limit = _HOSTS_FILE_READ_CHUNK if on_loop else None
-            try:
-                result = _parse_and_cache_for_key(path, key, limit=limit)
             except OSError:
-                result = "changed"
-            if isinstance(result, str):
-                _schedule_hosts_file_warm()
+                continue
+            if _unverifiable_hosts_file(key):
                 return True
-            cached = (key, result)
-        tables.append(cached)
-    remote = []
-    for key, table in tables:
-        verdict = table.get(host)
-        if verdict is True:
-            return True
-        if verdict is False:
-            remote.append(key)
-    if not remote:
-        return None
-    # The worker may have published or grown the own set since the key was
-    # read; a not-local entry judged by the older state is not served.
-    own_state = (_NETLINK_ADDRS_PUBLISHED, _own_host_names())
-    if any((key[-2], key[-1]) != own_state for key in remote):
-        _schedule_hosts_file_warm()
-        return True
-    # Not-local counts only when judged after publication; while the
-    # own-address set is incomplete it defers to the async verdict layer.
-    # (Every remote key's publication bit equals own_state[0] here.)
-    return False if own_state[0] else None
+            cached = _HOSTS_FILE_CACHE.get(path)
+            if cached is None or cached[0] != key:
+                on_loop = _event_loop_running()
+                if key[2] > _HOSTS_FILE_READ_CHUNK and on_loop:
+                    # Each uncached call re-schedules: a failed warm start cannot hang a later call.
+                    _schedule_hosts_file_warm()
+                    return True
+                limit = _HOSTS_FILE_READ_CHUNK if on_loop else None
+                try:
+                    result = _parse_and_cache_for_key(path, key, limit=limit)
+                except OSError:
+                    result = "changed"
+                if isinstance(result, str):
+                    _schedule_hosts_file_warm()
+                    return True
+                cached = (key, result)
+            tables.append(cached)
+        remote = []
+        for key, table in tables:
+            verdict = table.get(host)
+            if verdict is True:
+                return True
+            if verdict is False:
+                remote.append(key)
+        if not remote:
+            return None
+        # A not-local entry judged under an own state that has since moved is judged again.
+        own_state = (_NETLINK_ADDRS_PUBLISHED, _own_host_names())
+        if all((key[-2], key[-1]) == own_state for key in remote):
+            # Not-local counts only when judged after publication; while the
+            # own-address set is incomplete it defers to the async verdict layer.
+            return False if own_state[0] else None
+    _schedule_hosts_file_warm()
+    return True
 
 
 def _resolved_host_verdict(host: str, *, fail_closed: bool = True) -> bool:

test/test_denied_commands_security.py

@@ -10794,6 +10794,123 @@ def _parse_then_grow(*args, **kwargs):
         assert _denied_by("ssh -o ConnectTimeout=10 backup@host uptime") is None
         assert _denied_by("ssh loopalias uptime") == self._RULE
 
+    def _publish_during_first_gate_parse(self, monkeypatch, addrs):
+        """The enrichment worker publishes *addrs* while the gate parses the file."""
+        monkeypatch.setattr(_argv_floor, "_NETLINK_ADDRS_PUBLISHED", False)
+        real_parse = _argv_floor._parse_hosts_file
+        parses: "list[bool]" = []
+
+        def _parse(*args, **kwargs):
+            parses.append(_argv_floor._NETLINK_ADDRS_PUBLISHED)
+            if len(parses) == 1:
+                _argv_floor._publish_netlink_addresses(addrs)
+            return real_parse(*args, **kwargs)
+
+        monkeypatch.setattr(_argv_floor, "_parse_hosts_file", _parse)
+        return parses
+
+    def test_a_cold_ssh_to_a_remote_alias_is_allowed_when_publication_lands_mid_parse(
+        self, monkeypatch
+    ):
+        # The first ssh on a cold worker: the enrichment worker's netlink
+        # publish (its real publish step) lands inside the gate's parse, as it
+        # does whenever that parse is slow.  The remote alias is judged again
+        # on the new key and allowed, not refused as ssh-to-self.
+        path = self._hosts(monkeypatch, "10.4.4.4 farbox\n")
+        parses = self._publish_during_first_gate_parse(monkeypatch, {"203.0.113.66"})
+        assert _denied_by("ssh farbox uptime") is None
+        assert parses == [False, True]
+        assert _argv_floor._HOSTS_FILE_CACHE[path][0][-2] is True
+        assert "kirocrew-hosts-warm" not in self.started
+
+    def test_the_second_judgement_uses_the_newly_published_own_set(self, monkeypatch):
+        # The address published mid-parse is this host's, and a line aliases
+        # a name to it: the retry judges that alias local, never remote.
+        self._hosts(monkeypatch, "10.4.4.4 farbox\n203.0.113.66 ownalias\n")
+        parses = self._publish_during_first_gate_parse(monkeypatch, {"203.0.113.66"})
+        # The first table, judged before publication, maps ownalias remote:
+        # only the own-state re-check stops that table from being served.
+        assert _argv_floor._hosts_file_verdict("ownalias") is True
+        assert parses == [False, True]
+        assert _denied_by("ssh ownalias uptime") == self._RULE
+        assert _denied_by("ssh farbox uptime") is None
+
+    def test_a_file_edited_during_the_gate_parse_is_pending_at_once(self, monkeypatch):
+        # Only a moved OWN state is judged again: a FILE that changes
+        # mid-parse is refused as pending after one parse, with the warm
+        # scheduled.
+        path = self._hosts(monkeypatch, "10.4.4.4 farbox\n")
+        real_parse = _argv_floor._parse_hosts_file
+        parses: "list[int]" = []
+
+        def _parse(*args, **kwargs):
+            parses.append(1)
+            table = real_parse(*args, **kwargs)
+            with open(path, "a", encoding="utf-8") as fh:
+                fh.write("10.5.5.5 newbox\n")
+            return table
+
+        monkeypatch.setattr(_argv_floor, "_parse_hosts_file", _parse)
+        assert _argv_floor._hosts_file_verdict("farbox") is True
+        assert len(parses) == 1
+        assert _argv_floor._HOSTS_FILE_CACHE == {}
+        assert self.started.count("kirocrew-hosts-warm") == 1
+
+    def test_an_own_set_that_moves_before_a_remote_answer_is_judged_again(self, monkeypatch):
+        # The own set grows between the table's key and the remote answer's
+        # re-check: the gate judges again on the grown set and serves it.
+        self._hosts(monkeypatch, "10.4.4.4 farbox\n")
+        reads: "list[int]" = []
+
+        def _own():
+            reads.append(1)
+            grown = len(reads) > 2  # the key read and its re-check see the old set
+            return frozenset({"127.0.0.1"} | ({"198.51.100.9"} if grown else set()))
+
+        monkeypatch.setattr(_argv_floor, "_own_host_names", _own)
+        assert _argv_floor._hosts_file_verdict("farbox") is False
+        assert "kirocrew-hosts-warm" not in self.started
+
+    def test_a_key_still_moving_after_every_attempt_is_pending(self, monkeypatch):
+        # The bound: an own set that moves on every read is judged a fixed
+        # number of times, then a remote entry is refused as pending.
+        self._hosts(monkeypatch, "10.4.4.4 farbox\n")
+        reads = iter(range(10_000))
+        monkeypatch.setattr(
+            _argv_floor, "_own_host_names", lambda: frozenset({f"10.200.0.{next(reads) % 250}"})
+        )
+        real_parse = _argv_floor._parse_hosts_file
+        parses: "list[int]" = []
+
+        def _parse(*args, **kwargs):
+            parses.append(1)
+            return real_parse(*args, **kwargs)
+
+        monkeypatch.setattr(_argv_floor, "_parse_hosts_file", _parse)
+        assert _argv_floor._hosts_file_verdict("farbox") is True
+        assert len(parses) == _argv_floor._HOSTS_KEY_MOVED_ATTEMPTS == 3
+        assert _argv_floor._HOSTS_FILE_CACHE == {}
+        assert self.started.count("kirocrew-hosts-warm") == 1
+
+    def test_a_failed_re_stat_after_the_parse_is_pending_at_once(self, monkeypatch):
+        # The key cannot be re-read after a real parse: a read failure, not a
+        # moved own state, so the gate refuses at once, unretried.
+        self._hosts(monkeypatch, "10.4.4.4 farbox\n")
+        real_key = _argv_floor._hosts_file_key
+        calls: "list[int]" = []
+
+        def _key(path):
+            calls.append(1)
+            if len(calls) == 2:  # the re-stat after the parse
+                raise PermissionError(path)
+            return real_key(path)
+
+        monkeypatch.setattr(_argv_floor, "_hosts_file_key", _key)
+        assert _argv_floor._hosts_file_verdict("farbox") is True
+        assert len(calls) == 2
+        assert _argv_floor._HOSTS_FILE_CACHE == {}
+        assert self.started.count("kirocrew-hosts-warm") == 1
+
     def test_an_own_address_learned_later_re_marks_a_cached_alias(self, monkeypatch):
         self._hosts(monkeypatch, "198.51.100.44 lateownalias\n")
         _argv_floor._warm_hosts_file_cache()
@@ -10936,7 +11053,11 @@ def _merge():
 
         self._fire_after_lookup(monkeypatch, _merge)
         assert _argv_floor._host_is_self("ownalias") is True
-        assert "kirocrew-hosts-warm" in self.started
+        # The gate judged the file again on the merged set, in the same call:
+        # the alias is cached local, so no pending warm was needed.
+        assert dict(_argv_floor._HOSTS_FILE_CACHE)[str(self.tmp_path / "hosts")][1] == {
+            "ownalias": True
+        }
 
     def test_publication_after_the_key_check_is_a_deny(self, monkeypatch):
         # A table judged before publication must not be read as authoritative