From 455e0228caa133d53dbeca84d48b2fe8f8d2b356 Mon Sep 17 00:00:00 2001 From: GG5533 Date: Wed, 9 Sep 2026 09:43:54 -0400 Subject: [PATCH] Tests - Fix TestHistoryPathTraversal on macOS, and cover the containment check (#4390) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 * 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 --------- Co-authored-by: GG5533 <285285461+GG5533@users.noreply.github.com> Co-authored-by: Claude Opus 5 --- .../tests/unit/test_watch_model.py | 22 ++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/changedetectionio/tests/unit/test_watch_model.py b/changedetectionio/tests/unit/test_watch_model.py index f7cb6159f..ae53041fb 100644 --- a/changedetectionio/tests/unit/test_watch_model.py +++ b/changedetectionio/tests/unit/test_watch_model.py @@ -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" )