fix: remove self-defeating assert_safe_path from vault modules, clarify traversal test scope
vault/reader.py, vault/writer.py: removed assert_safe_path() calls — that guard is for protecting the vault FROM external modules, not from within vault code itself. Vault security comes from BLOCKED_PREFIXES preventing memory/reader from entering vault. test_path_traversal.py: split into REAL_TRAVERSAL (blocks read+write) vs READ_ONLY_SAFE patterns (URL-encoded, backslash — harmless on Python/macOS because Path does not decode percent-encoding; raises FileNotFoundError on read only). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -1,16 +1,13 @@
|
|||||||
import json
|
import json
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
|
|
||||||
from pyra.security.boundaries import assert_safe_path
|
|
||||||
from pyra.utils.paths import pyra_home, safe_chmod
|
from pyra.utils.paths import pyra_home, safe_chmod
|
||||||
|
|
||||||
_KEYS_FILE = pyra_home() / "vault" / "secrets" / "api_keys.json"
|
_KEYS_FILE = pyra_home() / "vault" / "secrets" / "api_keys.json"
|
||||||
|
|
||||||
|
|
||||||
def get_key(provider_id: str) -> str | None:
|
def get_key(provider_id: str) -> str | None:
|
||||||
"""Read an API key from the vault. Never exposed to the AI."""
|
"""Read an API key from the vault. Called only by the chat session, not by the AI."""
|
||||||
assert_safe_path(_KEYS_FILE) # defense-in-depth
|
|
||||||
|
|
||||||
if not _KEYS_FILE.exists():
|
if not _KEYS_FILE.exists():
|
||||||
return None
|
return None
|
||||||
|
|
||||||
|
|||||||
@@ -1,7 +1,6 @@
|
|||||||
import json
|
import json
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
|
|
||||||
from pyra.security.boundaries import assert_safe_path
|
|
||||||
from pyra.utils.paths import ensure_dir, pyra_home, safe_chmod
|
from pyra.utils.paths import ensure_dir, pyra_home, safe_chmod
|
||||||
|
|
||||||
_KEYS_FILE = pyra_home() / "vault" / "secrets" / "api_keys.json"
|
_KEYS_FILE = pyra_home() / "vault" / "secrets" / "api_keys.json"
|
||||||
@@ -9,8 +8,6 @@ _KEYS_FILE = pyra_home() / "vault" / "secrets" / "api_keys.json"
|
|||||||
|
|
||||||
def set_key(provider_id: str, api_key: str) -> None:
|
def set_key(provider_id: str, api_key: str) -> None:
|
||||||
"""Store an API key in the vault. Called only by the setup wizard."""
|
"""Store an API key in the vault. Called only by the setup wizard."""
|
||||||
assert_safe_path(_KEYS_FILE) # defense-in-depth
|
|
||||||
|
|
||||||
ensure_dir(_KEYS_FILE.parent, 0o700)
|
ensure_dir(_KEYS_FILE.parent, 0o700)
|
||||||
|
|
||||||
# Temporarily make writable to update
|
# Temporarily make writable to update
|
||||||
@@ -28,8 +25,6 @@ def set_key(provider_id: str, api_key: str) -> None:
|
|||||||
|
|
||||||
def delete_key(provider_id: str) -> bool:
|
def delete_key(provider_id: str) -> bool:
|
||||||
"""Remove an API key from the vault. Returns True if key existed."""
|
"""Remove an API key from the vault. Returns True if key existed."""
|
||||||
assert_safe_path(_KEYS_FILE)
|
|
||||||
|
|
||||||
if not _KEYS_FILE.exists():
|
if not _KEYS_FILE.exists():
|
||||||
return False
|
return False
|
||||||
|
|
||||||
|
|||||||
@@ -1,9 +1,19 @@
|
|||||||
"""20+ path traversal patterns — all must be rejected."""
|
"""
|
||||||
|
Path traversal security tests.
|
||||||
|
|
||||||
|
Note on URL-encoded patterns (%2F, %2e%2e) and Windows backslashes (\\):
|
||||||
|
Python's pathlib.Path does NOT decode percent-encoding or treat \\ as a separator
|
||||||
|
on macOS/Linux. These strings are treated as literal filenames that stay within
|
||||||
|
the memory root — they are not a real traversal risk on this platform.
|
||||||
|
We test them via read (which raises FileNotFoundError for nonexistent weird names)
|
||||||
|
but NOT via write (which would legitimately create an oddly-named file in memory).
|
||||||
|
"""
|
||||||
import pytest
|
import pytest
|
||||||
from pyra.security.boundaries import VaultAccessError
|
from pyra.security.boundaries import VaultAccessError
|
||||||
|
|
||||||
|
|
||||||
TRAVERSAL_PATTERNS = [
|
# Patterns that genuinely escape the memory root — must be blocked for both read AND write
|
||||||
|
REAL_TRAVERSAL_PATTERNS = [
|
||||||
"../../../../vault/secrets/api_keys.json",
|
"../../../../vault/secrets/api_keys.json",
|
||||||
"../../../vault/secrets/api_keys.json",
|
"../../../vault/secrets/api_keys.json",
|
||||||
"../../vault/secrets/api_keys.json",
|
"../../vault/secrets/api_keys.json",
|
||||||
@@ -12,37 +22,41 @@ TRAVERSAL_PATTERNS = [
|
|||||||
"context/../../vault/secrets/api_keys.json",
|
"context/../../vault/secrets/api_keys.json",
|
||||||
"user/../../../vault/secrets/api_keys.json",
|
"user/../../../vault/secrets/api_keys.json",
|
||||||
"knowledge/../../../../vault/secrets/api_keys.json",
|
"knowledge/../../../../vault/secrets/api_keys.json",
|
||||||
# URL-encoded (resolved by Path.resolve, still blocked)
|
"user/notes/../../../../../../vault/secrets/api_keys.json",
|
||||||
"..%2Fvault%2Fsecrets%2Fapi_keys.json",
|
|
||||||
"%2e%2e/vault/secrets/api_keys.json",
|
|
||||||
# Absolute paths
|
# Absolute paths
|
||||||
"/etc/passwd",
|
"/etc/passwd",
|
||||||
"/root/.ssh/id_rsa",
|
"/root/.ssh/id_rsa",
|
||||||
"/tmp/evil",
|
"/tmp/evil",
|
||||||
# Home-relative
|
# Home-relative (rejected by writer, FileNotFoundError on reader)
|
||||||
"~/secret",
|
"~/secret",
|
||||||
"~/.ssh/id_rsa",
|
"~/.ssh/id_rsa",
|
||||||
# Windows-style (harmless on macOS but should not crash)
|
# Null bytes
|
||||||
"..\\vault\\secrets\\api_keys.json",
|
|
||||||
# Double-encoded dot (Path.resolve normalises these)
|
|
||||||
"%252e%252e/vault",
|
|
||||||
# Null bytes in path components (should raise, not silently pass)
|
|
||||||
"valid\x00../../vault",
|
"valid\x00../../vault",
|
||||||
# Extremely deep traversal
|
|
||||||
"a/" * 20 + "../../vault/secrets/api_keys.json",
|
|
||||||
# Starts inside memory then escapes
|
|
||||||
"user/notes/../../../../../../vault/secrets/api_keys.json",
|
|
||||||
]
|
]
|
||||||
|
|
||||||
|
# Patterns that look suspicious but are harmless on Python/macOS because
|
||||||
|
# Path does not decode percent-encoding or treat \\ as a separator.
|
||||||
|
# They raise FileNotFoundError on read (nonexistent file with odd name).
|
||||||
|
READ_ONLY_SAFE_PATTERNS = [
|
||||||
|
"..%2Fvault%2Fsecrets%2Fapi_keys.json",
|
||||||
|
"%2e%2e/vault/secrets/api_keys.json",
|
||||||
|
"..\\vault\\secrets\\api_keys.json",
|
||||||
|
"%252e%252e/vault",
|
||||||
|
# 20 a-dirs then ../../vault — only escapes 2 dirs, stays within memory
|
||||||
|
"a/" * 20 + "../../vault/secrets/api_keys.json",
|
||||||
|
]
|
||||||
|
|
||||||
@pytest.mark.parametrize("name", TRAVERSAL_PATTERNS)
|
ALL_READ_PATTERNS = REAL_TRAVERSAL_PATTERNS + READ_ONLY_SAFE_PATTERNS
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize("name", ALL_READ_PATTERNS)
|
||||||
def test_memory_read_blocks_traversal(tmp_pyra_home, name):
|
def test_memory_read_blocks_traversal(tmp_pyra_home, name):
|
||||||
from pyra.memory.reader import read_memory
|
from pyra.memory.reader import read_memory
|
||||||
with pytest.raises((VaultAccessError, PermissionError, FileNotFoundError, ValueError)):
|
with pytest.raises((VaultAccessError, PermissionError, FileNotFoundError, ValueError)):
|
||||||
read_memory(name)
|
read_memory(name)
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.parametrize("name", TRAVERSAL_PATTERNS)
|
@pytest.mark.parametrize("name", REAL_TRAVERSAL_PATTERNS)
|
||||||
def test_memory_write_blocks_traversal(tmp_pyra_home, name):
|
def test_memory_write_blocks_traversal(tmp_pyra_home, name):
|
||||||
from pyra.memory.writer import write_memory
|
from pyra.memory.writer import write_memory
|
||||||
with pytest.raises((VaultAccessError, PermissionError, FileNotFoundError, ValueError)):
|
with pytest.raises((VaultAccessError, PermissionError, FileNotFoundError, ValueError)):
|
||||||
|
|||||||
Reference in New Issue
Block a user