fix(wiki): reject hardlinked page files in allowlist + read revalidation (#4375 Codex gate)
Gate-found (Codex CORE, repro'd): a hardlink at a clean *.md page name can carry an arbitrary inode (incl. one outside the wiki) through the O_NOFOLLOW + inode-identity read check — O_NOFOLLOW does not block hardlinks. Reject any page file with st_nlink > 1 in BOTH the allowlist walk and the read-path revalidation. Pre-existing on master (the inode recapture predates this PR's cache); closing it here while on the read path. +1 toothed regression test (prime cache -> hardlink listed page to outside secret -> assert 404, no leak). Co-authored-by: rodboev <rodboev@users.noreply.github.com>
This commit is contained in:
1 parent
9e92d7a8f4
commit
0fe707b2bf
2 files changed
+59
No files matched your search
@@ -5917,6 +5917,16 @@ def _llm_wiki_page_files_uncached(wiki_path: Path) -> list[Path]:
|
||||
rel = item.relative_to(section)
|
||||
if not item.is_file() or not _is_clean_relpath(rel):
|
||||
continue
|
||||
# Reject multi-link (hardlinked) page files. A hardlink at a
|
||||
# clean *.md name can point at an arbitrary inode (incl. one
|
||||
# outside the wiki); O_NOFOLLOW + inode-identity at read time
|
||||
# cannot tell such a hardlink apart from the real page, so
|
||||
# exclude any file with st_nlink > 1 from the allowlist. (#4375)
|
||||
try:
|
||||
if item.lstat().st_nlink > 1:
|
||||
continue
|
||||
except OSError:
|
||||
continue
|
||||
# Resolve the real target and require it to live under BOTH the
|
||||
# real wiki root and the real section, with no dot-prefixed
|
||||
# segment on the resolved-relative path. This closes symlink
|
||||
@@ -6005,6 +6015,12 @@ def _llm_wiki_allowlisted_entries(wiki_path: Path) -> dict[str, tuple[Path, tupl
|
||||
resolved_target.relative_to(section_real)
|
||||
if not resolved_target.is_file():
|
||||
continue
|
||||
# Reject hardlinked targets (st_nlink > 1): a multi-link file at
|
||||
# a clean page name can carry an arbitrary inode through the
|
||||
# O_NOFOLLOW + identity read check. Defense in depth alongside
|
||||
# the same rejection in the allowlist walk. (#4375)
|
||||
if resolved_target.stat().st_nlink > 1:
|
||||
continue
|
||||
rel_resolved = resolved_target.relative_to(wiki_real)
|
||||
if not _wiki_read_relpath_is_clean(rel_resolved):
|
||||
continue
|
||||
|
||||
@@ -262,6 +262,49 @@ def test_wiki_page_cached_nested_entry_rechecks_resolved_containment(monkeypatch
|
||||
assert b"stale_cache_marker" not in handler.body, "stale cached nested symlink leaked hidden content"
|
||||
|
||||
|
||||
def test_wiki_page_cached_entry_rejects_hardlink_swap_to_outside(monkeypatch, tmp_path):
|
||||
"""A hardlink swap to an outside file within the TTL must not leak (#4375).
|
||||
|
||||
O_NOFOLLOW + inode-identity cannot distinguish a hardlink at a clean page
|
||||
name from the real page, so multi-link (st_nlink > 1) page files are
|
||||
rejected by both the allowlist walk and the read-path revalidation. Prime
|
||||
the cache with a real page, replace it with a hardlink to an outside secret
|
||||
within the TTL, and assert the read 404s without leaking the secret.
|
||||
"""
|
||||
import os as _os
|
||||
from api import routes
|
||||
|
||||
wiki_root = tmp_path / "wiki"
|
||||
section = wiki_root / "concepts"
|
||||
section.mkdir(parents=True)
|
||||
page = section / "page.md"
|
||||
page.write_text("# real\n", encoding="utf-8")
|
||||
outside = tmp_path / "outside_secret"
|
||||
outside.write_text("DONOTLEAK=hardlink_cache_marker\n", encoding="utf-8")
|
||||
|
||||
routes._llm_wiki_clear_page_files_cache()
|
||||
monkeypatch.setattr(routes, "_WIKI_ALLOWLIST_TTL", 60.0)
|
||||
monkeypatch.setattr(routes, "_llm_wiki_resolve_path", lambda: (wiki_root, None, None))
|
||||
|
||||
# Prime the cached name list while the page is a genuine single-link file.
|
||||
assert routes._llm_wiki_page_files(wiki_root) == [page]
|
||||
|
||||
# Swap the listed page name to a hardlink pointing at the outside secret,
|
||||
# within the TTL window (cache still warm for the page NAME).
|
||||
page.unlink()
|
||||
try:
|
||||
_os.link(outside, page)
|
||||
except (OSError, NotImplementedError, AttributeError):
|
||||
import pytest
|
||||
pytest.skip("hardlinks not supported on this platform")
|
||||
|
||||
handler = _FakeHandler()
|
||||
routes.handle_get(handler, urlparse("http://example.com/api/wiki/page?path=concepts/page.md"))
|
||||
|
||||
assert handler.status == 404, f"cached page hardlinked to outside file must 404, got {handler.status}"
|
||||
assert b"hardlink_cache_marker" not in handler.body, "hardlink swap leaked outside content"
|
||||
|
||||
|
||||
def test_wiki_browse_cached_entry_rechecks_resolved_containment(monkeypatch, tmp_path):
|
||||
import os as _os
|
||||
from api import routes
|
||||
|
||||
Reference in new issue
Block a user