[bolichen97]

Problem / Motivation

Goal: On a cold worker, the first ssh to a dotless name the hosts file maps to a remote address (10.4.4.4 devbox, then ssh devbox uptime) is allowed even when the own-host enrichment worker publishes while that call is parsing the hosts file.

Maintainer takeover of #20861 by @jeeshofone, carried on a branch of this repository because the Sensitive Change Review gate refuses a fork PR that touches src/kiro_crew/security/. The code and tests are the author's commit, unchanged; the author is credited with Co-authored-by:.

Supersedes #20861

Follow-up to #20964. That PR made the in-call parse hand back its table uncached when only the own-address half of the hosts-cache key moves mid-parse, so a name the file does not hold (the case #20842 reported) or a local entry is answered from it. A remote entry in that table was judged against the older own set, so the own-state re-check before return still refuses it as pending. Reported as #21228.

Why it matters

A user whose hosts file names a remote machine gets a false sandbox-escape refusal on the first ssh after the gateway starts, and the same command passes on the next call. The refusal comes from a security gate, so it reads as an attack rather than a race.

Not a goal

• Changing fix(security): keep a hosts-file miss open when the own set grows mid-parse #20964's split of the key or its uncached-table answers for absent and local names.
• Judging again a file that changes while it is parsed, or a failed re-stat. Both stay pending at once, as on main.
• Changing which names count as self, the deny-floor direction of a name that maps local on any line, or the enrichment worker.

What changed (motivation → approach → change)

A cold ssh to a remote hosts-file alias is no longer refused when the own-host worker publishes during the gate's first parse; an alias for this host's own address is still refused.

Root cause: _hosts_file_verdict in src/kiro_crew/security/argv_floor.py judged each table under the key it read before parsing. When the worker published mid-parse, a remote entry's key no longer matched the own state at return, so the gate answered pending and refused.

Change: the body of _hosts_file_verdict runs in a loop of at most _HOSTS_KEY_MOVED_ATTEMPTS = 3 passes. The only retry is a remote entry whose own state moved: every path is judged again on the current key, so an alias for a newly published own address is refused on the retry. After the last pass the answer is pending and the warm is scheduled, as before. Every other exit is unchanged. One enrichment pass moves the key at most twice (netlink publish, then DNS merge), so three passes see a settled key. Tests in test/test_denied_commands_security.py pin each exit.

Backwards compatibility

Compatible: every answer main gives is unchanged, except a remote hosts-file entry whose own state moves during the call. That one is judged again on the current key instead of being refused as pending. A name that maps local on any line is still refused.

Removes nothing: no function, constant, key, field or record goes away.

Tests

In TestHostsFileWarmUp (test/test_denied_commands_security.py):

Test	Locks in
test_a_cold_ssh_to_a_remote_alias_is_allowed_when_publication_lands_mid_parse (new, the regression)	the worker's real publish step inside the first parse; ssh farbox uptime allowed after exactly two parses, new key cached, no warm
test_the_second_judgement_uses_the_newly_published_own_set (new)	an alias for the address published mid-parse is refused on the retry
test_an_own_set_that_moves_before_a_remote_answer_is_judged_again (new)	an own set growing before the re-check is judged again
test_a_key_still_moving_after_every_attempt_is_pending (new)	exactly three parses, then pending, nothing cached, one warm
test_a_file_edited_during_the_gate_parse_is_pending_at_once, test_a_failed_re_stat_after_the_parse_is_pending_at_once (new)	one parse, pending, one warm
test_own_set_merge_after_the_key_check_is_a_deny (changed pin)	the merged set is judged in the same call, so the alias is cached local

Determinism: TestHostsFileWarmUp (40 tests) passed 10 of 10 serial repeats, under -n 4, and in 3 shuffled orders (seeds 1, 2, 3). No clock or sleep is used; the race is forced by running the real publish step inside the parse. prove.py --base origin/main reverts the production hunk and reports PROVEN (the new tests go red).

Suites run on this head: test/test_denied_commands_security.py (1327 passed, 1 skipped), test/test_security.py (1455 passed, 1 skipped), test/test_mcp_cron_security.py (425), test/test_agent_refactor_facade.py (49), and every other test file that names argv_floor (all green), plus test/test_flake_pattern_ratchet.py and Semgrep with the repo rules (0 findings). The full static gate floor (black, isort, flake8, mypy, comment history, file line cap and the rest) passed.

Manual verification

Real-process check: a fresh Python process per call with the real enrichment worker, real netlink read and real threads; only the hosts file path is pointed at a temp file (10.4.4.4 devbox plus a line aliasing this host's own interface address as ownalias) and its parse slowed by a sleep. 20 calls per cell:

Command	delay	main allowed	this head allowed
ssh devbox uptime	0 ms	20/20	20/20
ssh devbox uptime	20 ms	0/20	20/20
ssh devbox uptime	200 ms	0/20	20/20
ssh ownalias uptime	0, 20, 200 ms	0/20	0/20
ssh -o ConnectTimeout=10 backup@host uptime (#20842)	200 ms	20/20	20/20

The namespace sandbox lane (Backend Tests (namespace sandbox)) cannot run on the host used here (unprivileged user namespaces are refused); CI runs it on this head.

Related Issues

Fixes #21228
Refs #20842
Supersedes #20861
Follow-up to #20964, which covers the case #20842 reported.
Known limit, not changed here: #21051 (on Linux a same-size rewrite within one coarse ctime tick can keep the hosts-cache key; the same on main).
#17666 (draft) moves this code to its own module. Whichever lands second re-homes the loop.

Pattern harvest

Rule candidate: review-prompt
Pattern: a check-then-use answer of "pending" returned on the first race, when re-reading the moved state and judging again a bounded number of times would answer correctly.

Checklist

• At most two commits (one is the norm), with a Conventional Commits title (feat|fix|docs|style|refactor|perf|test|chore|ci|build|revert: ...)
• Existing tests pass and new tests added for new functionality
• Self-review completed; code follows project style guidelines
• Documentation updated (if applicable)
• No secrets, credentials, or internal references in the diff

Contribution License Agreement

Contributions are accepted under the project's Apache-2.0 license; see the Licensing section of CONTRIBUTING.md. Original work by @jeeshofone in #20861.

[bolichen97]

Sensitive change review

• Head commit: 75d828b
• What rule changed: the ssh-to-self floor's hosts-file lookup (_hosts_file_verdict in the security/argv_floor.py module). A remote hosts-file entry whose own-host state moved during the call is judged again on the current key, at most 3 passes, instead of being refused as pending at once.
• Before -> after (what was allowed/blocked, now): before, on a cold worker the first ssh devbox uptime (10.4.4.4 devbox in the hosts file) was refused as sandbox-escape-ssh-self whenever the own-host worker published during the parse. Now it is allowed. An alias for this host's own address is refused before and after. A file changed mid-parse, a failed re-stat and an unreadable or oversized file stay pending at once, as before.
• Worst case if wrong (breaks commands / leaks secret / locks user out): a hosts-file alias for this host's own address could be judged remote and an ssh to self allowed. The retry judges every remote entry under the own set read at return, the return check still compares that own state with each table's key, and exhaustion answers pending (deny), so the failure direction stays closed.
• Evidence checked (which sandbox result proves it): fresh processes with the real enrichment worker, real netlink read and real threads, only the hosts path redirected and its parse slowed. With 20 and 200 ms parses, main allowed ssh devbox uptime 0 of 20 and this head 20 of 20; ssh ownalias uptime (an alias for this host's own interface address) was refused 20 of 20 on both; the Flaky: ssh lookalike allow-case refused as ssh-to-self in the cron command vet #20842 command was allowed 20 of 20 on both. The namespace sandbox lane cannot run on this host (user namespaces refused), so it is left to CI's Backend Tests (namespace sandbox) job on this head.
• No regression:
  • Tools still work (which tools/commands were run, result): the public gate is_denied was run on ssh devbox uptime, ssh ownalias uptime and ssh -o ConnectTimeout=10 backup@host uptime in fresh processes, with the results above; all argv floor suites pass.
  • Backward compatible (old config / old data / old callers still work, how checked): no config, key, function or record is added or removed; callers of _hosts_file_verdict see the same return contract (True, False, None). Checked by the unchanged existing TestHostsFileWarmUp tests and the other argv floor suites.
  • Existing tests (which suites passed on this head): test/test_denied_commands_security.py 1327 passed, test/test_security.py 1455 passed, test/test_mcp_cron_security.py 425 passed, test/test_agent_refactor_facade.py 49 passed, every other test file naming argv_floor green, flake pattern ratchet, Semgrep repo rules, and the static gate floor including mypy.
• How to undo: revert this single commit; _hosts_file_verdict returns to answering pending on the first moved own state, which is the stricter behavior on main.

[github-actions]

UX Review (Opus 5.5) — ⏭️ skipped

Revision a61afcdb500889dad102e2dd9fa34d13b5f902ce touches no user-facing surface (no changes under website/ or committed screenshots), so the UX review was skipped. Advisory — does not block merge.

[github-actions]

Sensitive Change Review

Sensitive files changed:

• src/kiro_crew/security/argv_floor.py

• @bolichen97 posted a complete reasoning template for a61afcd.

[github-actions]

Design Review (Opus 5.5) — ✅ PASS

Design-level review of a61afcdb500889dad102e2dd9fa34d13b5f902ce — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: PASS

Check that the three-pass bound rests on a real limit: "one enrichment pass moves the key at most twice" should be stated in the worker code, not only in this comment.

[DESIGN-REVIEWED] a61afcd

Description read

[DESCRIPTION-READ] fc08bdfcaa868669f18b9b7167599eaf3194875d577db7c109bfdaf95baa4f33

That sha256 names everything this verdict read: the PR description, plus the 1 evidence file(s) the workflow collected from it and handed this review. It covers that evidence because the media strip replaces every attachment URL with the same placeholder, so an edit swapping one attachment for another would leave the description's captured text identical while what this review judged changed. Recompute it with .github/scripts/pr-description-capture.sh over the same inputs; if it differs, the description or that evidence changed after this verdict was formed and any finding drawn from either is unproven.

[github-actions]

First Principles Review (Opus 5.5) — ✅ PASS

Premise-level review of a61afcdb500889dad102e2dd9fa34d13b5f902ce — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

First-Principles-Verdict: PASS

Check the bound before merging: three passes assumes "one enrichment pass moves the key at most twice (netlink publish, then DNS merge)". Writes at argv_floor.py:2127 and :2236 agree, but :2267 reseeds the set as well.

What this change ships

Inventory (4 items) — 4 justified

Intent: the first ssh to a remote hosts-file alias on a cold worker should not be falsely refused when the own-host worker publishes mid-parse. This is a FIX (#21228).

1. A cold ssh to a remote hosts-file alias is now allowed when publication lands during the gate's parse (it is judged again on the current key) — justified
2. An alias for a newly published own address is still refused on the retry — justified
3. Re-judging is capped at three passes per call, then the answer is pending and the warm is scheduled, as before — justified
4. Changed pin: an own-set merge after the key check is still denied, and is now cached local in the same call instead of scheduling a pending warm — justified

[FIRST-PRINCIPLES-REVIEWED] a61afcd

Description read

[DESCRIPTION-READ] 6e5947f43405ba55f99b1f5b19a3261470f23522e52d4d59ebcb882e0aa13fdf

That sha256 names the PR description this verdict read. Recompute it with .github/scripts/pr-description-capture.sh; if it differs, the description changed after this verdict was formed and any finding drawn from the description is unproven.

[github-actions]

Opus 5.5 Review — ✅ no blocking findings

Reviewed a61afcdb500889dad102e2dd9fa34d13b5f902ce — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] a61afcd

Verdict parsed from the review's SHA-scoped output markers for commit a61afcdb500889dad102e2dd9fa34d13b5f902ce.

False positive or not applicable? A repository writer can comment:
/ai-review override fable a61afcdb500889dad102e2dd9fa34d13b5f902ce: <one-sentence reason>

[github-actions]

GPT 6.1 Review — ✅ no blocking findings

GPT 6.1 completed its review of a61afcdb500889dad102e2dd9fa34d13b5f902ce and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] a61afcd

False positive or not applicable? A repository writer can comment:
/ai-review override gpt a61afcdb500889dad102e2dd9fa34d13b5f902ce: <one-sentence reason>

[bolichen97]

• Three passes cover one enrichment pass: rebutted

The own state in the key is (publication bit, own set). One enrichment pass writes it twice: _publish_netlink_addresses sets the own set and the publication bit together under the own-host lock, then _resolve_own_host_names_into_cache merges the DNS addresses once. So a third pass reads a settled key.
test_a_cold_ssh_to_a_remote_alias_is_allowed_when_publication_lands_mid_parse runs the worker's real publish step inside the first parse and pins exactly two parses; test_own_set_merge_after_the_key_check_is_a_deny pins the DNS merge move.
The bound decides availability only, never safety: exhaustion answers pending, which the floor refuses, and test_a_key_still_moving_after_every_attempt_is_pending pins that. A later TTL re-resolve during one call can only cost a pending answer, which is the behavior on main.

[bolichen97]

• Three passes cover one enrichment pass: rebutted

The own state in the key is (publication bit, own set). One enrichment pass writes it twice: _publish_netlink_addresses sets the own set and the publication bit together under the own-host lock, then _resolve_own_host_names_into_cache merges the DNS addresses once. So a third pass reads a settled key.
test_a_cold_ssh_to_a_remote_alias_is_allowed_when_publication_lands_mid_parse runs the worker's real publish step inside the first parse and pins exactly two parses; test_own_set_merge_after_the_key_check_is_a_deny pins the DNS merge move.
The bound decides availability only, never safety: exhaustion answers pending, which the floor refuses, and test_a_key_still_moving_after_every_attempt_is_pending pins that. A later TTL re-resolve during one call can only cost a pending answer, which is the behavior on main.

[github-actions]

Security Scope Review -- ✅ PASS

Which legitimate operations does a61afcdb500889dad102e2dd9fa34d13b5f902ce newly refuse? Updated in place on each push. A script-confirmed regression blocks; the model proposes candidates and never decides a refusal.

Verdict basis: adjudicated, zero confirmed regressions.

Security scope review -- adjudicated candidates

• macos-latest (platform posix): 14 classified, 0 newly refused, 8 still allowed.
• ubuntu-latest (platform posix): 14 classified, 0 newly refused, 8 still allowed.
• windows-latest (platform windows): 12 classified, 0 newly refused, 7 still allowed.

No candidate flipped from allowed to refused.

Scope-Verdict: PASS

TIGHTENING: the hosts-file part of the ssh-to-self host check (_hosts_file_verdict in src/kiro_crew/security/argv_floor.py). Its threat is a remote-looking alias that actually maps to this host's own address, judged before the own-address set was published. Before this patch, the gate refused as soon as the own state moved (if any((key[-2], key[-1]) != own_state for key in remote): _schedule_hosts_file_warm(); return True). Now it judges the file again on the current key, up to _HOSTS_KEY_MOVED_ATTEMPTS = 3 times. It refuses only if the key is still moving after those passes (_schedule_hosts_file_warm(); return True after the loop), or if a pass maps the name local. Every early return True exit (unreadable file, unverifiable key, a file that changed mid-parse, a large file on the event loop) is kept unchanged. So this patch loosens the check rather than tightening it. A pass that ends in refusal could only refuse a name the old code also refused when its own state moved.

ADJUDICATED: 16 candidates, 0 confirmed newly-refused

PLATFORM COVERAGE: The diff makes no new assumption about paths, separators or executable names. It reuses the existing _hosts_file_paths() split: /etc/hosts on posix, %SystemRoot%\System32\drivers\etc\hosts on nt.

• posix: rows cover ssh, scp, rsync and sftp with posix local paths and ~/.ssh key paths.
• windows: rows use ssh.exe and scp.exe with .\dist\... backslash local paths, so the nt hosts-file branch is reached through the Windows executable spelling.
• any: the platform-neutral rows are host and URL operands: a bare alias, a dotted FQDN, an IP literal, scp-style git@github.com:..., a non-default port, and https/http URL hosts for curl. These reach every caller of _host_is_self.

One limit applies to every platform: the retry loop only runs when the enrichment worker publishes during the gate's parse. A static classification cannot trigger that race. The patch's own unit tests are what exercise it.

[SCOPE-REVIEWED] a61afcd

[bolichen97]

Rebased: main moved in acp/session_handle.py, acp/transport_errors.py and security_posture.py (import overlap) since this head went green (6ed866f -> 5dd888c); CI re-running.

[bolichen97]

Sensitive change review

• Head commit: a61afcd
• What rule changed: the ssh-to-self floor's hosts-file lookup (_hosts_file_verdict in the security/argv_floor.py module). A remote hosts-file entry whose own-host state moved during the call is judged again on the current key, at most 3 passes, instead of being refused as pending at once.
• Before -> after (what was allowed/blocked, now): before, on a cold worker the first ssh devbox uptime (10.4.4.4 devbox in the hosts file) was refused as sandbox-escape-ssh-self whenever the own-host worker published during the parse. Now it is allowed. An alias for this host's own address is refused before and after. A file changed mid-parse, a failed re-stat and an unreadable or oversized file stay pending at once, as before.
• Worst case if wrong (breaks commands / leaks secret / locks user out): a hosts-file alias for this host's own address could be judged remote and an ssh to self allowed. The retry judges every remote entry under the own set read at return, the return check still compares that own state with each table's key, and exhaustion answers pending (deny), so the failure direction stays closed.
• Evidence checked (which sandbox result proves it): fresh processes with the real enrichment worker, real netlink read and real threads, only the hosts path redirected and its parse slowed. With 20 and 200 ms parses, main allowed ssh devbox uptime 0 of 20 and this head 20 of 20; ssh ownalias uptime (an alias for this host's own interface address) was refused 20 of 20 on both; the Flaky: ssh lookalike allow-case refused as ssh-to-self in the cron command vet #20842 command was allowed 20 of 20 on both. CI's Backend Tests (namespace sandbox) job (real user namespaces) passed on the previous head 75d828b, which carries the same diff before a rebase onto main; the user namespace sandbox cannot run on the host used for the real-process check.
• No regression:
  • Tools still work (which tools/commands were run, result): the public gate is_denied was run on ssh devbox uptime, ssh ownalias uptime and ssh -o ConnectTimeout=10 backup@host uptime in fresh processes, with the results above; all argv floor suites pass.
  • Backward compatible (old config / old data / old callers still work, how checked): no config, key, function or record is added or removed; callers of _hosts_file_verdict see the same return contract (True, False, None). Checked by the unchanged existing TestHostsFileWarmUp tests and the other argv floor suites.
  • Existing tests (which suites passed on this head): test/test_denied_commands_security.py 1327 passed, test/test_security.py 1455 passed, test/test_mcp_cron_security.py 425 passed, test/test_agent_refactor_facade.py 49 passed, every other test file naming argv_floor green, flake pattern ratchet, Semgrep repo rules, and the static gate floor including mypy.
• How to undo: revert this single commit; _hosts_file_verdict returns to answering pending on the first moved own state, which is the stricter behavior on main.

[iamwhatever]

HOLD at a61afcd: human review is required by policy.

This PR changes _hosts_file_verdict in the ssh-to-self sandbox-escape floor (argv_floor.py#L1658-L1749). A remote hosts-file entry whose own-host state moves during the call used to be refused as pending. It is now judged again on the current key, up to _HOSTS_KEY_MOVED_ATTEMPTS = 3 passes, and can be allowed. That loosens what a sandbox gate allows, so a maintainer has to sign off on it.

I found no defect. Each pass re-reads the key, so a stale table is parsed again under the new own set. A local verdict on any line still denies first. An exhausted bound still denies. The file-changed, re-stat-failure, unreadable and oversized exits are unchanged. Security Scope Review reports 0 newly refused commands, and the new tests pin each exit. The Design and First Principles punchlines on the three-pass bound were rebutted on the pre-rebase head. That rebuttal does not name the :2267 reseed. I read that line: it seeds the own set only when the set is still empty, so it adds no later move. Either way the bound decides availability, not safety.

To clear: a maintainer reviews and approves this sandbox-gate change.

— crew-pr-reviewer (r110)