Skip to content

fix(security): digest small hosts files on POSIX too (#21051) - #21865

Merged
buluoray merged 1 commit into
mainfrom
fix/hosts-file-key-posix-digest-21051
Oct 21, 2026
Merged

buluoray merged 1 commit into
mainfrom
fix/hosts-file-key-posix-digest-21051

Conversation

@buluoray

Copy link
Copy Markdown
Contributor

Problem / Motivation

Goal: On Linux, rewriting the hosts file to the same size within one coarse ctime tick always makes the ssh/curl self-host floor re-read it, so a name the new content maps to this machine is never answered from the table parsed from the old content.

The self-target floor caches its parsed hosts table under _hosts_file_key: (st_mtime, st_ctime, st_size, digest, published, own). The content digest was taken only on Windows, so on POSIX the key relied on st_ctime to see a same-size rewrite that restores mtime. Linux stamps ctime from a coarse clock (about 10 ms on xfs in the report), so two writes inside one tick leave (mtime, ctime, size) unchanged and the gate keeps serving the old table. The report reproduced a stale remote answer in 176 of 200 tight-loop iterations.

Why it matters

The floor exists to refuse an ssh/scp/sftp/rsync target that is really this machine. A hosts alias rewritten to a local address inside one tick was judged remote until something else moved the key. Writing the hosts file needs privilege, so this is a robustness gap in the cache key rather than a privilege boundary, but the floor should not depend on clock granularity.

Not a goal

  • Closing the same-tick window for a POSIX hosts file over 64 KiB (one read chunk). That needs a gate read past the chunk on every call; the window is written down in the _hosts_file_key docstring instead.
  • Changing the Windows rule that a hosts file over one chunk is never served.
  • Keying on st_ino (the report's option 1): it misses an in-place write, which is the reported case.

What changed (motivation → approach → change)

Symptom: a same-size rewrite inside one ctime tick is served from the stale table. Root cause: on POSIX the key carried no content digest, and ctime alone cannot separate two writes inside one tick. Change (the report's option 3, reusing machinery the module already has):

  • _hosts_file_key now takes the bounded blake2b digest for any file no larger than _HOSTS_FILE_READ_CHUNK (64 KiB) on every platform, not only on Windows. The read is bounded by the chunk size and its failure raises _HostsFileUnreadable, exactly as on Windows today.
  • _parse_and_cache_for_key already hashes and parses from one buffer whenever the key carries a digest, so on POSIX the table is now also built from exactly the content its key names. No new refusal: a small file whose read fails was already pending through the parse path.
  • The Windows-only "file over one chunk is unverifiable and never served" rule is unchanged. Its predicate is renamed from _hosts_content_digest_enabled to _ctime_is_creation_time, since it no longer gates the digest and only says what the platform's ctime means.
  • Docstrings that said POSIX does no extra read are updated, and the owning spec (docs/system-specs/modules/security.md, ssh self-target floor) now states the key shape.

Cost: one bounded read and hash per gate call for a typical hosts file, the same as Windows pays today.

Backwards compatibility

Compatible: the key only gains a field value on POSIX, so a rewrite that changed the key before still changes it; every refusal on main still refuses.

Reader: src/kiro_crew/security/argv_floor.py:_unverifiable_hosts_file -- entry: chat|cron|subagent -- calls the renamed _ctime_is_creation_time, same Windows-only answer; test_windows_large_file_never_serves_a_remote_or_absent_verdict
Reader: test/test_denied_commands_security.py:TestHostsFileWarmUp fixture and _windows -- entry: chat -- patch the renamed seam; the whole class passes

Tests

In test/test_denied_commands_security.py, TestHostsFileWarmUp:

  • test_posix_same_size_rewrite_inside_one_ctime_tick_re_parses: own set holds 10.4.4.4, the hosts file holds 10.9.9.9 farbox, and an os.stat seam returns the first stat for every call, so mtime, ctime and size never move (no real clock involved). The first verdict is remote (False); after an in-place same-size rewrite to 10.4.4.4 farbox the next verdict is local (True) and the cached table is the new one.
  • test_posix_large_file_keeps_a_stat_key_and_is_served: a small POSIX file's key carries a digest; a file over one chunk keeps digest None and is still served after a warm (POSIX is not routed into the Windows unverifiable rule).
  • test_small_file_read_failure_on_the_digest_is_pending[windows|posix] (was the Windows-only test_windows_small_file_read_failure_on_the_digest_is_pending): a digest read that fails answers pending even with a table cached. This is new on POSIX, a fail-closed answer while the file cannot be read, so it is now pinned there too.
  • test_posix_key_does_no_content_read is removed: it asserted the old behaviour this PR changes.

Mutation: restoring the Windows-only condition (if _ctime_is_creation_time() and stat.st_size <= _HOSTS_FILE_READ_CHUNK:) reddens the three POSIX tests (..._re_parses and ..._is_pending[posix] with assert False is True, ..._is_served with assert None is not None). Local runs: the whole of test/test_denied_commands_security.py passes with -n0 (1372 passed, 1 skipped), plus test/test_flake_pattern_ratchet.py.

Manual verification

N/A — unit coverage sufficient: the stat seam reproduces the coarse-tick case exactly (unchanged mtime, ctime and size). Not reproduced on a real coarse-tick filesystem here, and the Windows path is unchanged and not run on a Windows host.

Related Issues

Closes #21051

Pattern harvest

Not generalizable: a cache key that trusts a filesystem timestamp to separate writes is specific to this cache; the module's other stamp checks (the native-loader digest in the same spec) already pair the stamp with a content hash.

Decision for the owner

The decision is whether the POSIX hosts-file key should pay one bounded 64 KiB read and hash per gate call to close the coarse-ctime gap. This draft chose yes, reusing the existing Windows digest path, over keying on st_ino, which would miss the in-place rewrite the issue reports.

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.

@github-actions

github-actions Bot commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

Sensitive Change Review

Sensitive files changed:

  • src/kiro_crew/security/argv_floor.py

  • src/kiro_crew/security/argv_floor_hosts.py

  • src/kiro_crew/security/hosts_file.py

  • @buluoray posted a complete reasoning template for fdc2332.

@github-actions

github-actions Bot commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

UX Review (Opus 5.5) — ⏭️ skipped

Revision fdc2332d83d98a410e38662bbccbd3309e74c554 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 github-actions Bot added the readiness: checking Automated validation is still running label Oct 18, 2026
@github-actions

github-actions Bot commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

Design Review (Opus 5.5) — ✅ PASS

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

Design-Verdict: PASS

Check that the issue-tracker reproduction (176/200 stale answers in a tight loop) now gives zero on a real coarse-ctime filesystem.

[DESIGN-REVIEWED] fdc2332

Description read

[DESCRIPTION-READ] 4cbb51eea4bb9254f2fda30219f844170cd5615485c21a788ce788d33fc463da

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

github-actions Bot commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

First Principles Review (Opus 5.5) — 🟡 CONCERNS

Premise-level review of fdc2332d83d98a410e38662bbccbd3309e74c554 — 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: CONCERNS

The fix is sound for the hosts table, but the same coarse-ctime stat key still guards the governance-profile cache, and "Not generalizable" never mentions it.

Not justified as shipped

  • Item 1 — symptom-level (point patch, siblings unfixed): "Not generalizable: a cache key that trusts a filesystem timestamp to separate writes is specific to this cache". I grepped st_ctime under src/ and found 14 other stat-only cache keys with no content digest. One of them is security-relevant: platform/governance_profiles.py:_dir_fingerprint keys the governance ceiling on (st_mtime_ns, st_size, st_ctime_ns), so it has the same same-size, one-tick blind spot. The general fix is bigger than this PR, so it can be deferred.
    Clears when: a linked issue tracks the governance-profile fingerprint, or the description shows that path cannot hit a same-size rewrite that restores mtime inside one tick.

What this change ships

Inventory (6 items) — 5 justified

Intent: stop the ssh self-target floor on Linux from serving a stale hosts table after a fast same-size rewrite. FIX.

  1. A small hosts file (64 KiB or less) on POSIX is re-read whenever its content changes, even inside one ctime tick — symptom-level; 14 stat-only ctime keys left, governance_profiles is the security one
  2. On POSIX, an unreadable small hosts file now answers pending even when a table is cached — justified
  3. Every POSIX gate call now does one bounded 64 KiB read and hash — justified
  4. Private test seam renamed to _ctime_is_creation_time — justified
  5. Security spec and docstrings now state the key shape and the large-file window — justified
  6. Removed the test_posix_key_does_no_content_read pin — justified

[FIRST-PRINCIPLES-REVIEWED] fdc2332

Description read

[DESCRIPTION-READ] c7b2e371bd6d46dd6fb2b9ffcf572f5e951643e1fbc724786665be0349495e93

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

github-actions Bot commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

Opus 5.5 Review — ✅ no blocking findings

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

Review details

No findings.

[OPUS-REVIEWED] fdc2332

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

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

@github-actions

github-actions Bot commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

GPT 6.1 Review — ✅ no blocking findings

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

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] fdc2332

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

@github-actions

github-actions Bot commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

Security Scope Review -- ✅ PASS

Which legitimate operations does fdc2332d83d98a410e38662bbccbd3309e74c554 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): 12 classified, 0 newly refused, 9 still allowed.
  • ubuntu-latest (platform posix): 12 classified, 0 newly refused, 9 still allowed.
  • windows-latest (platform windows): 7 classified, 0 newly refused, 3 still allowed.

No candidate flipped from allowed to refused.

Scope-Verdict: PASS

TIGHTENING: The cache key for the ssh-self floor's hosts-file table now includes a content digest on POSIX as well as Windows. Any hosts file up to one read chunk (64 KiB) is re-read and hashed on every gate call. Before, POSIX keyed on stat alone. A failed or oversize digest read raises _HostsFileUnreadable, which answers pending, so dotless names are refused. -- The one threat it refuses is a same-size rewrite of the hosts file inside one coarse ctime tick that would keep serving a stale "remote" verdict for an alias that now points to this machine. Hunk: - if _hosts_content_digest_enabled() and stat.st_size <= _HOSTS_FILE_READ_CHUNK: / + if stat.st_size <= _HOSTS_FILE_READ_CHUNK:. The docstring says "st_ctime cannot promise it: Windows stamps creation time, Linux a coarse clock that two writes inside one tick share".

ADJUDICATED: 16 candidates, 0 confirmed newly-refused. The harness handed me no confirmed rows; the per-leg verdicts come from the deterministic fold.

PLATFORM COVERAGE:

  • posix (9 rows): The diff's only behavioural change is on POSIX, for dotless names resolved through a small hosts file.
    • The rows send ssh, scp, rsync and sftp to dotless targets (dev-dsk, builder, buildbox, webhost, filebox, bastion).
    • They include an option before the target (-p 2222) and a BatchMode cron-style probe, which runs on a thread with no event loop, where the in-call read happens.
    • They also cover the read-only diagnostics an agent runs after a refusal note: getent hosts, cat /etc/hosts.
  • any (3 rows): Dotted public names (git@github.com, gitlab.com, an EC2 FQDN). These are the nearest neighbours of the dotless path and must stay classified as remote.
  • windows (4 rows): ssh.exe / scp.exe to dotless names, a backslash local path, a full-hostname call that follows the refusal note's advice, and type C:\Windows\System32\drivers\etc\hosts. Windows behaviour should be unchanged, because the seam was only renamed (_hosts_content_digest_enabled → _ctime_is_creation_time, still os.name == "nt"). These rows check that the rename kept the Windows path the same.
  • Not covered: A string classifier can't vary host state, so no row probes a POSIX digest read that fails while stat succeeds. That needs a file that grows past the chunk between stat and read, or one that becomes unreadable mid-call. The diff adds this read on every POSIX call, so the window is new. Its outcome, though (an unreadable file answers fail closed), is the same as the existing parse-on-miss read. I'm noting it as an environmental edge, not a regression.

[SCOPE-REVIEWED] fdc2332

@buluoray

Copy link
Copy Markdown
Contributor Author

🤖 Kiro Crew Auto-Pipeline [operator: buluoray#ce473fca]:

Sensitive change review

  • Head commit: 542e780
  • What rule changed: the ssh self-target floor's hosts-file cache key (_hosts_file_key in src/kiro_crew/security/argv_floor.py). For a hosts file no larger than 64 KiB it now carries the bounded blake2b content digest on every platform, not only on Windows. The Windows-only rule that a file over 64 KiB is never served is unchanged; its predicate is renamed _ctime_is_creation_time.
  • Before -> after (what was allowed/blocked, now): before, on Linux a same-size rewrite of the hosts file inside one coarse ctime tick kept the old key, so an ssh target the new content maps to this machine could be answered "remote" (allowed) from the old table. After, that rewrite changes the key, the file is re-parsed, and the target is refused. Nothing that was refused before is now allowed. New on POSIX: if the bounded digest read fails, the gate answers pending (refused, retry) even with a table cached, the same as Windows today.
  • Worst case if wrong (breaks commands / leaks secret / locks user out): breaks commands. A POSIX host whose hosts file cannot be read would see dotless ssh targets refused as pending until the file is readable; full hostnames and IP addresses are unaffected. No secret is read or written.
  • Evidence checked (which sandbox result proves it): test_posix_same_size_rewrite_inside_one_ctime_tick_re_parses pins mtime, ctime and size through an os.stat seam and shows the verdict move from remote to local after an in-place same-size rewrite. Reverting the fix (restoring the Windows-only digest condition) reddens it, test_posix_large_file_keeps_a_stat_key_and_is_served, and test_small_file_read_failure_on_the_digest_is_pending[posix]. Not run on a real coarse-tick filesystem or a Windows host.
  • No regression:
    • Tools still work (which tools/commands were run, result): the ssh/scp/sftp/rsync self-target cases in TestSandboxEscapeSshSelf, TestHostsAliasPublicationWindow and TestHostsFileWarmUp pass; a remote alias in a small or large POSIX hosts file is still allowed.
    • Backward compatible (old config / old data / old callers still work, how checked): the cache is in-process only, nothing persisted; the only callers of the renamed predicate are _unverifiable_hosts_file and the test fixtures, found with git grep on main.
    • Existing tests (which suites passed on this head): test/test_denied_commands_security.py with -n0 (1372 passed, 1 skipped) and test/test_flake_pattern_ratchet.py (58 passed), plus the CI checks on this head.
  • How to undo: revert this one commit; the key returns to stat-only on POSIX.

@buluoray

Copy link
Copy Markdown
Contributor Author

🤖 Kiro Crew Auto-Pipeline [operator: buluoray#ce473fca]: Disposition of the Security Scope Review CONCERNS on 542e780, plus the CI red, both on head 8b74a3c.

CI red (self-caused, fixed). Backend Tests shard 2 on Linux and Windows failed test_security_gate_liveness.py::test_the_package_stays_within_its_line_budget: the package was 29,755 lines, 4 over the 29,751 budget, because the _hosts_file_key docstring grew. The docstring is now tighter, argv_floor.py is back to its size on main, and the branch is rebased on main 924c159. Locally: test/test_security_gate_liveness.py plus test/test_denied_commands_security.py, 1382 passed, 1 skipped, -n0.

Unadjudicated item, case 1: an unreadable hosts file (e.g. 0600 root). Not new. On main a POSIX key reads nothing, but a cache miss parses the file. That read fails, _parse_and_cache_for_key returns "failed", nothing is cached, and the gate answers pending. So main already refuses every dotless name on such a host, the same as this PR. Measured with the same probe on both trees: a hosts file with mode 000 (non-root), _hosts_file_verdict("farbox") called twice. origin/main 924c159: True True. This branch: True True. The suggested stat-only fallback would therefore change nothing for that host. It would only add a path where a read failure leaves the key without its digest, so I have not added it.

Case 2: the file grows past 64 KiB between the stat and the read. Transient on both trees. On main the in-call parse is bounded by the same chunk and raises _HostsFileTooLarge, which is pending. Here the digest read raises first, which is also pending. The next call sees the new size and takes the large-file path. No retry-stable refusal is added.

@buluoray
buluoray force-pushed the fix/hosts-file-key-posix-digest-21051 branch from 542e780 to 8b74a3c Compare October 18, 2026 11:15
@buluoray

Copy link
Copy Markdown
Contributor Author

🤖 Kiro Crew Auto-Pipeline [operator: buluoray#ce473fca]:

Sensitive change review

  • Head commit: 8b74a3c
  • What rule changed: the ssh self-target floor's hosts-file cache key (_hosts_file_key in src/kiro_crew/security/argv_floor.py). For a hosts file no larger than 64 KiB it now carries the bounded blake2b content digest on every platform, not only on Windows. The Windows-only rule that a file over 64 KiB is never served is unchanged; its predicate is renamed _ctime_is_creation_time.
  • Before -> after (what was allowed/blocked, now): before, on Linux a same-size rewrite of the hosts file inside one coarse ctime tick kept the old key, so an ssh target the new content maps to this machine could be answered "remote" (allowed) from the old table. After, that rewrite changes the key, the file is re-parsed, and the target is refused. Nothing that was refused before is now allowed. New on POSIX: if the bounded digest read fails, the gate answers pending (refused, retry) even with a table cached, the same as Windows today.
  • Worst case if wrong (breaks commands / leaks secret / locks user out): breaks commands. A POSIX host whose hosts file cannot be read would see dotless ssh targets refused as pending until the file is readable; full hostnames and IP addresses are unaffected. No secret is read or written.
  • Evidence checked (which sandbox result proves it): test_posix_same_size_rewrite_inside_one_ctime_tick_re_parses pins mtime, ctime and size through an os.stat seam and shows the verdict move from remote to local after an in-place same-size rewrite. Reverting the fix (restoring the Windows-only digest condition) reddens it, test_posix_large_file_keeps_a_stat_key_and_is_served, and test_small_file_read_failure_on_the_digest_is_pending[posix]. Not run on a real coarse-tick filesystem or a Windows host.
  • No regression:
    • Tools still work (which tools/commands were run, result): the ssh/scp/sftp/rsync self-target cases in TestSandboxEscapeSshSelf, TestHostsAliasPublicationWindow and TestHostsFileWarmUp pass; a remote alias in a small or large POSIX hosts file is still allowed.
    • Backward compatible (old config / old data / old callers still work, how checked): the cache is in-process only, nothing persisted; the only callers of the renamed predicate are _unverifiable_hosts_file and the test fixtures, found with git grep on main.
    • Existing tests (which suites passed on this head): test/test_denied_commands_security.py with -n0 plus test/test_security_gate_liveness.py (1382 passed, 1 skipped together) and test/test_flake_pattern_ratchet.py (58 passed), plus the CI checks on this head.
  • How to undo: revert this one commit; the key returns to stat-only on POSIX.

@buluoray

Copy link
Copy Markdown
Contributor Author

🤖 Kiro Crew Auto-Pipeline [operator: buluoray#ce473fca]: Disposition of the Security Scope Review CONCERNS on 8b74a3c. No code change. Both states were already pending on main.

(a) hosts file can be stat'ed but not opened. On main a cache miss parses the file. The parse read raises OSError, _parse_and_cache_for_key catches it and returns "failed", and _hosts_file_verdict answers True (pending) without caching. So main never builds a table for such a file, and every dotless name is already refused there. Measured with the same probe on both trees: a mode-000 hosts file, non-root, _hosts_file_verdict("farbox") called twice. origin/main 924c159 gives True True, and this branch gives True True. The measurement is in the comment above.

(b) a file of 64 KiB or less grows past 64 KiB between the stat and the read. This PR adds at most one pending answer, inside a window main answers pending right after. In that call the digest read finds more than a chunk and answers pending. On main the same call could still serve the table cached under the pre-growth stat. From the next call on, both trees stat the larger size. On an event loop both schedule the warm and answer pending until it lands. The suggested fallback to a stat-only key would not avoid that. With a small size in the key, the on-loop parse is bounded by the same chunk, raises _HostsFileTooLarge, and is pending too. It would also add a path where a small file's key carries no digest, which is the stale window this PR closes. So I have left it out.

@buluoray
buluoray marked this pull request as ready for review October 18, 2026 12:10
@buluoray
buluoray requested review from a team and KiroCrewCaptain as code owners October 18, 2026 12:10
@buluoray
buluoray enabled auto-merge (squash) October 18, 2026 12:10
@github-actions github-actions Bot added readiness: passed Eligible automated validation passed for the current revision merge conflict Branch has merge conflicts with its base — author must resolve before merge and removed readiness: checking Automated validation is still running labels Oct 18, 2026
@buluoray
buluoray force-pushed the fix/hosts-file-key-posix-digest-21051 branch from 8b74a3c to 0431f30 Compare October 18, 2026 23:02
@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed readiness: passed Eligible automated validation passed for the current revision merge conflict Branch has merge conflicts with its base — author must resolve before merge labels Oct 18, 2026
@buluoray

Copy link
Copy Markdown
Contributor Author

🤖 Kiro Crew Auto-Pipeline [operator: buluoray#ce473fca]:

Sensitive change review

  • Head commit: 0431f30
  • What rule changed: the ssh self-target floor's hosts-file cache key (_hosts_file_key in src/kiro_crew/security/argv_floor.py). For a hosts file no larger than 64 KiB it now carries the bounded blake2b content digest on every platform, not only on Windows. The Windows-only rule that a file over 64 KiB is never served is unchanged; its predicate is renamed _ctime_is_creation_time. This head is a rebase of 8b74a3c onto main: main moved TestHostsFileWarmUp into test/test_denied_commands_security_hosts_file.py (test: split test_denied_commands_security.py by behaviour (1 of 2) #21858), so the test changes now land in that file. The source and spec changes are byte-identical to the previous head.
  • Before -> after (what was allowed/blocked, now): before, on Linux a same-size rewrite of the hosts file inside one coarse ctime tick kept the old key, so an ssh target the new content maps to this machine could be answered "remote" (allowed) from the old table. After, that rewrite changes the key, the file is re-parsed, and the target is refused. Nothing that was refused before is now allowed. New on POSIX: if the bounded digest read fails, the gate answers pending (refused, retry) even with a table cached, the same as Windows today.
  • Worst case if wrong (breaks commands / leaks secret / locks user out): breaks commands. A POSIX host whose hosts file cannot be read would see dotless ssh targets refused as pending until the file is readable; full hostnames and IP addresses are unaffected. No secret is read or written.
  • Evidence checked (which sandbox result proves it): test_posix_same_size_rewrite_inside_one_ctime_tick_re_parses pins mtime, ctime and size through an os.stat seam and shows the verdict move from remote to local after an in-place same-size rewrite. Reverting the fix (restoring the Windows-only digest condition) reddens it, test_posix_large_file_keeps_a_stat_key_and_is_served, and test_small_file_read_failure_on_the_digest_is_pending[posix]. Not run on a real coarse-tick filesystem or a Windows host.
  • No regression:
    • Tools still work (which tools/commands were run, result): the ssh/scp/sftp/rsync self-target cases in TestSandboxEscapeSshSelf, TestHostsAliasPublicationWindow and TestHostsFileWarmUp pass; a remote alias in a small or large POSIX hosts file is still allowed.
    • Backward compatible (old config / old data / old callers still work, how checked): the cache is in-process only, nothing persisted; on this head the only callers of the renamed predicate are _unverifiable_hosts_file and the test fixtures in test/test_denied_commands_security_hosts_file.py, found with git grep.
    • Existing tests (which suites passed on this head): test/test_denied_commands_security.py, test/test_denied_commands_security_hosts_file.py and test/test_security_gate_liveness.py with -n0 (963 passed together) and test/test_flake_pattern_ratchet.py (60 passed), plus the CI checks on this head.
  • How to undo: revert this one commit; the key returns to stat-only on POSIX.

@github-actions github-actions Bot added readiness: passed Eligible automated validation passed for the current revision merge conflict Branch has merge conflicts with its base — author must resolve before merge and removed readiness: checking Automated validation is still running labels Oct 19, 2026
The ssh self-target floor caches its parsed hosts table under a key
built from os.stat plus the own-address state. On POSIX the key relied
on st_ctime to see a same-size rewrite that restores mtime, but Linux
stamps ctime from a coarse clock, so two writes inside one tick left
the key unchanged and the gate served a table parsed from the old
content.

Take the bounded blake2b digest the Windows key already carries for
any file no larger than one read chunk on every platform. The rule
that a file over one chunk is unverifiable and never served stays
Windows-only, where ctime is creation time; the remaining same-tick
window for a POSIX file over one chunk is written down in the key's
docstring. The platform predicate is renamed to say what it tests.

Closes #21051
@buluoray
buluoray force-pushed the fix/hosts-file-key-posix-digest-21051 branch from 0431f30 to fdc2332 Compare October 19, 2026 01:14
@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed readiness: passed Eligible automated validation passed for the current revision merge conflict Branch has merge conflicts with its base — author must resolve before merge labels Oct 19, 2026
@buluoray

Copy link
Copy Markdown
Contributor Author

🤖 Kiro Crew Auto-Pipeline [operator: buluoray#ce473fca]:

Sensitive change review

  • Head commit: fdc2332
  • What rule changed: the ssh self-target floor's hosts-file cache key (_hosts_file_key, now in src/kiro_crew/security/argv_floor_hosts.py). For a hosts file no larger than 64 KiB it now carries the bounded blake2b content digest on every platform, not only on Windows. The Windows-only rule that a file over 64 KiB is never served is unchanged; its predicate is renamed _ctime_is_creation_time. This head is a rebase of 0431f30 onto main: main split argv_floor.py into owner modules behind a facade (refactor(security): split argv_floor.py into owners behind a facade #21952), moving the hosts-file key, cache and gate into argv_floor_hosts.py. The same edits now land there, and the facade's TYPE_CHECKING import in argv_floor.py names the renamed predicate. The hosts_file.py docstring keeps main's argv_floor_hosts owner name. Behaviour, tests and spec text are unchanged from the previous head.
  • Before -> after (what was allowed/blocked, now): before, on Linux a same-size rewrite of the hosts file inside one coarse ctime tick kept the old key, so an ssh target the new content maps to this machine could be answered "remote" (allowed) from the old table. After, that rewrite changes the key, the file is re-parsed, and the target is refused. Nothing that was refused before is now allowed. New on POSIX: if the bounded digest read fails, the gate answers pending (refused, retry) even with a table cached, the same as Windows today.
  • Worst case if wrong (breaks commands / leaks secret / locks user out): breaks commands. A POSIX host whose hosts file cannot be read would see dotless ssh targets refused as pending until the file is readable; full hostnames and IP addresses are unaffected. No secret is read or written.
  • Evidence checked (which sandbox result proves it): on this head, restoring the Windows-only digest condition in _hosts_file_key reddens test_posix_same_size_rewrite_inside_one_ctime_tick_re_parses, test_posix_large_file_keeps_a_stat_key_and_is_served and test_small_file_read_failure_on_the_digest_is_pending[posix] (3 failed, 42 passed); with the fix restored all 45 pass. Not run on a real coarse-tick filesystem or a Windows host.
  • No regression:
    • Tools still work (which tools/commands were run, result): the ssh/scp/sftp/rsync self-target cases in TestSandboxEscapeSshSelf, TestHostsAliasPublicationWindow and TestHostsFileWarmUp pass; a remote alias in a small or large POSIX hosts file is still allowed.
    • Backward compatible (old config / old data / old callers still work, how checked): the cache is in-process only, nothing persisted; on this head the old predicate name has no remaining reference (git grep _hosts_content_digest_enabled is empty), and the test fixtures patch the renamed one through the argv_floor facade.
    • Existing tests (which suites passed on this head): test/test_argv_floor_facade.py, test/test_argv_floor_inline_and_brace_scope.py and every test/test_denied_commands_security*.py (1628 passed, 1 skipped); test/test_denied_commands_security.py, test/test_denied_commands_security_hosts_file.py, test/test_security_gate_liveness.py and test/test_flake_pattern_ratchet.py with -n0 (1023 passed); black --check clean on the three changed source files.
  • How to undo: revert this one commit; the key returns to stat-only on POSIX.

@github-actions github-actions Bot added readiness: passed Eligible automated validation passed for the current revision and removed readiness: checking Automated validation is still running labels Oct 19, 2026
@iamwhatever

Copy link
Copy Markdown
Contributor

HOLD on fdc2332 (crew-pr-reviewer, read-only).

This PR changes the ssh self-target floor in argv_floor_hosts.py#L262: on POSIX the hosts-table cache key now carries a content digest, and a failed digest read answers pending. That changes what the sandbox floor refuses, so it is reserved for a human maintainer even though the code looks sound. Auto-merge is armed on this PR, so the hold needs a human action to take effect.

One AI concern is unanswered: the First Principles lane (CONCERNS) points out that governance_profiles.py _dir_fingerprint has the same same-size, one-tick blind spot. Please post a disposition, and either open an issue for it and link it in the description, or show the path is unreachable.

The change itself is small and fails closed. The Security Scope Review found 0 newly refused commands on macOS, Ubuntu and Windows, and CI is green on this head.

— crew-pr-reviewer (r111)

@bolichen97 bolichen97 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deep review passed. The POSIX hosts-file digest closes a stale-cache path and fails closed on read errors.

Sensitive change review

  • Rule changed: small POSIX hosts files now join the cache key with a bounded content digest.
  • Before -> after: a same-size rewrite inside one timestamp tick could reuse an old host map; changed bytes now force a parse.
  • Worst case if wrong: a local alias could be mistaken for a remote SSH target, or valid dotless targets could be refused.
  • Evidence checked: the parsed buffer must match the keyed digest; the key is checked again after parsing; reads are bounded; focused tests and mutation checks cover POSIX rewrites and read failures.
  • No regression found: failures remain fail closed, Windows behavior is unchanged, and all required checks pass.
  • Undo: revert this pull request.

Follow-up: add an A-to-B-to-A race test for the digest comparison, and track the similar config-directory fingerprint gap separately.

@buluoray
buluoray merged commit 35c06aa into main Oct 21, 2026
110 of 111 checks passed
@buluoray
buluoray deleted the fix/hosts-file-key-posix-digest-21051 branch October 21, 2026 05:01
@github-actions github-actions Bot removed the readiness: passed Eligible automated validation passed for the current revision label Oct 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Hosts-file cache key misses a same-size rewrite within one coarse ctime tick on Linux

3 participants