Tests - Fix TestHistoryPathTraversal on macOS, and cover the containment check (#4390)

* Compare against a resolved data_dir in the history path-traversal test

Watch.history resolves entries with os.path.realpath, so
test_normal_snapshot_entry_is_accepted compared a resolved path against an
unresolved data_dir. On macOS the datastore lives under /tmp, which is a
symlink to /private/tmp, so the assertion fails for a path that is in fact
inside the directory. The guard is correct; the test was not.

Resolve both sides, matching what the production code does.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* Add a test that actually exercises the history containment check

Disabling the containment check in Watch.history left every test in
TestHistoryPathTraversal passing. os.path.basename() reduces both traversal
fixtures ('/etc/passwd', '../../etc/passwd') to 'passwd', so neither reaches
the check — they stop at the os.path.exists() test below it.

A bare '..' survives basename() and resolves to the parent of data_dir, which
exists, so the containment check is what rejects it. With that check disabled
this new test is the only one in the class that fails.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: GG5533 <285285461+GG5533@users.noreply.github.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
GG5533
2026-09-09 15:43:54 +02:00
committed by GitHub
co-authored by Claude Opus 5 GG5533
parent cd15016465
commit 455e0228ca
@@ -370,6 +370,23 @@ class TestHistoryPathTraversal(unittest.TestCase):
history = watch.history
self.assertEqual(history, {}, "Path traversal entry must be rejected")
def test_parent_dir_entry_is_rejected_by_the_containment_check(self):
"""A bare '..' is the shortest entry that reaches and fails the containment check.
os.path.basename() reduces '/etc/passwd' and '../../etc/passwd' to
'passwd', so those two stop at the os.path.exists() check below the
guard rather than at the guard itself. '..' survives basename() intact
and resolves to the parent of data_dir, which does exist, so the
containment check is what rejects it. Not the only such input —
'../..' and 'foo/..' collapse to the same thing — and not the only
reason the check exists: it also blocks a filename inside data_dir
that is itself a symlink pointing outside.
"""
watch = self._make_watch()
self._write_history_txt(watch, ['1000000000,..\n'])
history = watch.history
self.assertEqual(history, {}, "Parent-directory entry must be rejected")
def test_normal_snapshot_entry_is_accepted(self):
"""A bare filename written by save_history_blob must still load correctly."""
import uuid as uuid_builder
@@ -377,8 +394,11 @@ class TestHistoryPathTraversal(unittest.TestCase):
watch.save_history_blob(contents="hello world", timestamp=1000000000, snapshot_id=str(uuid_builder.uuid4()))
history = watch.history
self.assertEqual(len(history), 1, "Normal snapshot entry must be accepted")
# Watch.history resolves entries with os.path.realpath, so compare against a
# resolved data_dir. On macOS the datastore lives under /tmp, which is a symlink
# to /private/tmp, and an unresolved comparison fails there for a correct path.
self.assertTrue(
list(history.values())[0].startswith(watch.data_dir),
list(history.values())[0].startswith(os.path.realpath(watch.data_dir)),
"Resolved path must be inside the watch data directory"
)