fix(retrieval): fail-closed on uncanonicalisable paths (embedded null byte)
is_within_dir called os.path.realpath OUTSIDE its try block, so a path with an embedded null byte (which makes realpath raise ValueError, not OSError) leaked that ValueError to callers. Via safe_resolve -> okf._load_file (which catches only PathSecurityError) it propagated uncaught out of navigate_bundle, breaking the OKF SPEC §4 guarantee (okf.py:8-9) that a broken cross-link is silently skipped, never raised: an index link like `](a\x00b.md)` has no `/`, slips past the same-dir pre-filter, and reached path resolution. Move both realpath calls inside the existing try so an uncanonicalisable path is treated as not-within (fail-closed): safe_resolve raises PathSecurityError -> _load_file returns None -> the link is skipped. Fixes the class for both callers of the seam (navigate_bundle + retrieve). Tests (load-bearing — RED before, GREEN after): - test_navigate_skips_null_byte_link: the reported regression. - test_null_byte_path_rejected: the seam directly (is_within_dir/safe_resolve). - test_navigate_skips_bundle_escaping_symlink_link: closes the previously untested path-safety-at-link-resolution branch of navigate_bundle. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HVHLJuwBzp7MXkrUXJYARS
This commit is contained in:
parent
0a11af74a4
commit
3b0ee792bd
3 changed files with 50 additions and 4 deletions
|
|
@ -50,13 +50,16 @@ def is_within_dir(candidate: str, docs_dir: str) -> bool:
|
|||
|
||||
Fail-closed against symlink-escape and prefix-collision: uses ``os.path.commonpath`` on
|
||||
canonical paths, so a sibling dir sharing a name prefix (``/a/docs-evil`` vs ``/a/docs``)
|
||||
is correctly rejected (a naive ``startswith`` would not)."""
|
||||
real_root = os.path.realpath(docs_dir)
|
||||
real_candidate = os.path.realpath(candidate)
|
||||
is correctly rejected (a naive ``startswith`` would not). A path that cannot be canonicalised
|
||||
at all — e.g. an embedded null byte makes ``os.path.realpath`` raise ``ValueError`` — is
|
||||
likewise not-within (a path we cannot prove safe is refused, never leaked to the caller)."""
|
||||
try:
|
||||
real_root = os.path.realpath(docs_dir)
|
||||
real_candidate = os.path.realpath(candidate)
|
||||
return os.path.commonpath([real_root, real_candidate]) == real_root
|
||||
except ValueError:
|
||||
# Different drives / mixed absolute-relative -> not within.
|
||||
# Uncanonicalisable (embedded null byte), different drives, or mixed
|
||||
# absolute-relative -> not within.
|
||||
return False
|
||||
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue