fix(context): restrict @ references to safe workspace paths (#2601)
fix(context): block @ references from reading secrets outside the workspace. Defaults allowed_root to cwd, adds sensitive file blocklist.
This commit is contained in:
parent
ca2958ff98
commit
2d8fad8230
2 changed files with 93 additions and 1 deletions
|
|
@ -17,6 +17,23 @@ REFERENCE_PATTERN = re.compile(
|
||||||
r"(?<![\w/])@(?:(?P<simple>diff|staged)\b|(?P<kind>file|folder|git|url):(?P<value>\S+))"
|
r"(?<![\w/])@(?:(?P<simple>diff|staged)\b|(?P<kind>file|folder|git|url):(?P<value>\S+))"
|
||||||
)
|
)
|
||||||
TRAILING_PUNCTUATION = ",.;!?"
|
TRAILING_PUNCTUATION = ",.;!?"
|
||||||
|
_SENSITIVE_HOME_DIRS = (".ssh", ".aws", ".gnupg", ".kube")
|
||||||
|
_SENSITIVE_HERMES_DIRS = (Path("skills") / ".hub",)
|
||||||
|
_SENSITIVE_HOME_FILES = (
|
||||||
|
Path(".ssh") / "authorized_keys",
|
||||||
|
Path(".ssh") / "id_rsa",
|
||||||
|
Path(".ssh") / "id_ed25519",
|
||||||
|
Path(".ssh") / "config",
|
||||||
|
Path(".bashrc"),
|
||||||
|
Path(".zshrc"),
|
||||||
|
Path(".profile"),
|
||||||
|
Path(".bash_profile"),
|
||||||
|
Path(".zprofile"),
|
||||||
|
Path(".netrc"),
|
||||||
|
Path(".pgpass"),
|
||||||
|
Path(".npmrc"),
|
||||||
|
Path(".pypirc"),
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
@dataclass(frozen=True)
|
@dataclass(frozen=True)
|
||||||
|
|
@ -128,7 +145,11 @@ async def preprocess_context_references_async(
|
||||||
return ContextReferenceResult(message=message, original_message=message)
|
return ContextReferenceResult(message=message, original_message=message)
|
||||||
|
|
||||||
cwd_path = Path(cwd).expanduser().resolve()
|
cwd_path = Path(cwd).expanduser().resolve()
|
||||||
allowed_root_path = Path(allowed_root).expanduser().resolve() if allowed_root is not None else None
|
# Default to the current working directory so @ references cannot escape
|
||||||
|
# the active workspace unless a caller explicitly widens the root.
|
||||||
|
allowed_root_path = (
|
||||||
|
Path(allowed_root).expanduser().resolve() if allowed_root is not None else cwd_path
|
||||||
|
)
|
||||||
warnings: list[str] = []
|
warnings: list[str] = []
|
||||||
blocks: list[str] = []
|
blocks: list[str] = []
|
||||||
injected_tokens = 0
|
injected_tokens = 0
|
||||||
|
|
@ -222,6 +243,7 @@ def _expand_file_reference(
|
||||||
allowed_root: Path | None = None,
|
allowed_root: Path | None = None,
|
||||||
) -> tuple[str | None, str | None]:
|
) -> tuple[str | None, str | None]:
|
||||||
path = _resolve_path(cwd, ref.target, allowed_root=allowed_root)
|
path = _resolve_path(cwd, ref.target, allowed_root=allowed_root)
|
||||||
|
_ensure_reference_path_allowed(path)
|
||||||
if not path.exists():
|
if not path.exists():
|
||||||
return f"{ref.raw}: file not found", None
|
return f"{ref.raw}: file not found", None
|
||||||
if not path.is_file():
|
if not path.is_file():
|
||||||
|
|
@ -248,6 +270,7 @@ def _expand_folder_reference(
|
||||||
allowed_root: Path | None = None,
|
allowed_root: Path | None = None,
|
||||||
) -> tuple[str | None, str | None]:
|
) -> tuple[str | None, str | None]:
|
||||||
path = _resolve_path(cwd, ref.target, allowed_root=allowed_root)
|
path = _resolve_path(cwd, ref.target, allowed_root=allowed_root)
|
||||||
|
_ensure_reference_path_allowed(path)
|
||||||
if not path.exists():
|
if not path.exists():
|
||||||
return f"{ref.raw}: folder not found", None
|
return f"{ref.raw}: folder not found", None
|
||||||
if not path.is_dir():
|
if not path.is_dir():
|
||||||
|
|
@ -315,6 +338,28 @@ def _resolve_path(cwd: Path, target: str, *, allowed_root: Path | None = None) -
|
||||||
return resolved
|
return resolved
|
||||||
|
|
||||||
|
|
||||||
|
def _ensure_reference_path_allowed(path: Path) -> None:
|
||||||
|
home = Path(os.path.expanduser("~")).resolve()
|
||||||
|
hermes_home = Path(
|
||||||
|
os.getenv("HERMES_HOME", str(home / ".hermes"))
|
||||||
|
).expanduser().resolve()
|
||||||
|
|
||||||
|
blocked_exact = {home / rel for rel in _SENSITIVE_HOME_FILES}
|
||||||
|
blocked_exact.add(hermes_home / ".env")
|
||||||
|
blocked_dirs = [home / rel for rel in _SENSITIVE_HOME_DIRS]
|
||||||
|
blocked_dirs.extend(hermes_home / rel for rel in _SENSITIVE_HERMES_DIRS)
|
||||||
|
|
||||||
|
if path in blocked_exact:
|
||||||
|
raise ValueError("path is a sensitive credential file and cannot be attached")
|
||||||
|
|
||||||
|
for blocked_dir in blocked_dirs:
|
||||||
|
try:
|
||||||
|
path.relative_to(blocked_dir)
|
||||||
|
except ValueError:
|
||||||
|
continue
|
||||||
|
raise ValueError("path is a sensitive credential or internal Hermes path and cannot be attached")
|
||||||
|
|
||||||
|
|
||||||
def _strip_trailing_punctuation(value: str) -> str:
|
def _strip_trailing_punctuation(value: str) -> str:
|
||||||
stripped = value.rstrip(TRAILING_PUNCTUATION)
|
stripped = value.rstrip(TRAILING_PUNCTUATION)
|
||||||
while stripped.endswith((")", "]", "}")):
|
while stripped.endswith((")", "]", "}")):
|
||||||
|
|
|
||||||
|
|
@ -219,3 +219,50 @@ def test_restricts_paths_to_allowed_root(tmp_path: Path):
|
||||||
assert "```\noutside\n```" not in result.message
|
assert "```\noutside\n```" not in result.message
|
||||||
assert "inside" in result.message
|
assert "inside" in result.message
|
||||||
assert any("outside the allowed workspace" in warning for warning in result.warnings)
|
assert any("outside the allowed workspace" in warning for warning in result.warnings)
|
||||||
|
|
||||||
|
|
||||||
|
def test_defaults_allowed_root_to_cwd(tmp_path: Path):
|
||||||
|
from agent.context_references import preprocess_context_references
|
||||||
|
|
||||||
|
workspace = tmp_path / "workspace"
|
||||||
|
workspace.mkdir()
|
||||||
|
secret = tmp_path / "secret.txt"
|
||||||
|
secret.write_text("outside\n", encoding="utf-8")
|
||||||
|
|
||||||
|
result = preprocess_context_references(
|
||||||
|
f"read @file:{secret}",
|
||||||
|
cwd=workspace,
|
||||||
|
context_length=100_000,
|
||||||
|
)
|
||||||
|
|
||||||
|
assert result.expanded
|
||||||
|
assert "```\noutside\n```" not in result.message
|
||||||
|
assert any("outside the allowed workspace" in warning for warning in result.warnings)
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_blocks_sensitive_home_and_hermes_paths(tmp_path: Path, monkeypatch):
|
||||||
|
from agent.context_references import preprocess_context_references_async
|
||||||
|
|
||||||
|
monkeypatch.setenv("HOME", str(tmp_path))
|
||||||
|
monkeypatch.setenv("HERMES_HOME", str(tmp_path / ".hermes"))
|
||||||
|
|
||||||
|
hermes_env = tmp_path / ".hermes" / ".env"
|
||||||
|
hermes_env.parent.mkdir(parents=True)
|
||||||
|
hermes_env.write_text("API_KEY=super-secret\n", encoding="utf-8")
|
||||||
|
|
||||||
|
ssh_key = tmp_path / ".ssh" / "id_rsa"
|
||||||
|
ssh_key.parent.mkdir(parents=True)
|
||||||
|
ssh_key.write_text("PRIVATE-KEY\n", encoding="utf-8")
|
||||||
|
|
||||||
|
result = await preprocess_context_references_async(
|
||||||
|
"read @file:.hermes/.env and @file:.ssh/id_rsa",
|
||||||
|
cwd=tmp_path,
|
||||||
|
allowed_root=tmp_path,
|
||||||
|
context_length=100_000,
|
||||||
|
)
|
||||||
|
|
||||||
|
assert result.expanded
|
||||||
|
assert "API_KEY=super-secret" not in result.message
|
||||||
|
assert "PRIVATE-KEY" not in result.message
|
||||||
|
assert any("sensitive credential" in warning for warning in result.warnings)
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue