diff --git a/graphlink_app/graphlink_plugins/gitlink/agent.py b/graphlink_app/graphlink_plugins/gitlink/agent.py index 7107635..7d83111 100644 --- a/graphlink_app/graphlink_plugins/gitlink/agent.py +++ b/graphlink_app/graphlink_plugins/gitlink/agent.py @@ -86,6 +86,19 @@ def _normalize_repo_path(path_text): normalized = PurePosixPath(raw_path) if normalized.is_absolute() or ".." in normalized.parts: raise RuntimeError("Repository paths must stay inside the selected repository.") + # A Windows drive letter ("C:/config/app.txt", "C:app.txt") or an NTFS + # Alternate Data Stream ("file.txt:hidden") isn't "absolute" by POSIX's + # is_absolute() (no leading '/'), so it slipped past the check above - + # not as a real escape (_safe_local_target's resolve()+containment check + # still catches a genuine cross-drive path), but as silent aliasing: + # Windows pathlib treats a same-drive "C:" path segment as a no-op + # drive-relative reference rather than a literal folder, so + # "C:/config/app.txt" resolved to the exact same target as the plain + # "config/app.txt" - not rejected as "absolute-looking input" the way + # this guard's own contract says it should be. Reject any ':' in a path + # segment loudly instead of letting it collapse silently. + if any(":" in part for part in normalized.parts): + raise RuntimeError("Repository paths must stay inside the selected repository.") return normalized.as_posix() diff --git a/graphlink_app/tests/test_gitlink_path_safety.py b/graphlink_app/tests/test_gitlink_path_safety.py index e0bd866..7bf7886 100644 --- a/graphlink_app/tests/test_gitlink_path_safety.py +++ b/graphlink_app/tests/test_gitlink_path_safety.py @@ -61,6 +61,41 @@ def test_unc_style_path_is_rejected(self): with pytest.raises(RuntimeError): _normalize_repo_path("//server/share/evil.txt") + def test_windows_drive_letter_path_is_rejected(self): + # Bug-scan finding: a drive-lettered path isn't "absolute" by POSIX's + # is_absolute() (no leading '/'), so it slipped past the check above. + # Not a real escape - _safe_local_target's containment check still + # holds - but silent ALIASING: Windows pathlib treats a same-drive + # "C:" path segment as a no-op drive-relative reference rather than a + # literal folder, so this used to resolve to the exact same target as + # the plain "config/app.txt", contrary to this guard's own contract + # of rejecting absolute-looking input. + with pytest.raises(RuntimeError): + _normalize_repo_path("C:/config/app.txt") + + def test_windows_drive_relative_path_with_no_slash_is_rejected(self): + # "C:app.txt" (no slash) is the same drive-relative hazard as above, + # via a different textual shape - PurePosixPath keeps it as one part + # ("C:app.txt") rather than splitting off "C:", but the substring + # check still catches it. + with pytest.raises(RuntimeError): + _normalize_repo_path("C:app.txt") + + def test_cross_drive_path_is_rejected(self): + with pytest.raises(RuntimeError): + _normalize_repo_path("D:/config/app.txt") + + def test_alternate_data_stream_style_path_is_rejected(self): + # An NTFS Alternate Data Stream marker - same defensive "reject any + # colon" guard, though ADS addressing doesn't actually work through + # this path-join code path. + with pytest.raises(RuntimeError): + _normalize_repo_path("file.txt:hidden_stream") + + def test_a_colon_deeper_in_the_path_is_still_rejected(self): + with pytest.raises(RuntimeError): + _normalize_repo_path("src/weird:name.py") + class TestSafeLocalTarget: def test_normal_path_resolves_under_root(self, tmp_path): @@ -71,12 +106,12 @@ def test_parent_traversal_is_rejected(self, tmp_path): with pytest.raises(RuntimeError): _safe_local_target(tmp_path, "../../outside.py") - def test_windows_drive_letter_injection_stays_contained(self, tmp_path): - # A path that looks like a Windows absolute path (drive letter) must not - # be able to escape the repo root once re-joined onto it. - target = _safe_local_target(tmp_path, "C:/Windows/System32/evil.txt") - resolved_root = tmp_path.resolve() - assert target == resolved_root or resolved_root in target.parents + def test_windows_drive_letter_path_is_rejected_at_normalization(self, tmp_path): + # _safe_local_target normalizes first, and normalization now rejects + # a drive-lettered path outright (see TestNormalizeRepoPath) instead + # of silently aliasing it onto some path under the repo root. + with pytest.raises(RuntimeError): + _safe_local_target(tmp_path, "C:/Windows/System32/evil.txt") def test_unc_style_path_is_rejected_at_normalization(self, tmp_path): # _safe_local_target normalizes first, and normalization now rejects