Skip to content

Commit 7666030

Browse files
committed
test(manifest): update is_recovered comments to reflect Round-7 lexical guard
Round 8 — addresses Copilot review comment on tests/integrations/test_manifest.py:362. After Round-7 (1dbf0c2), is_recovered() rejects absolute paths and '..' segments up front via a lexical guard, returning False without calling _validate_rel_path at all. The test comments still described the prior "_validate_rel_path raises; we catch" code path, which is misleading for readers. Updated comments in both: - test_is_recovered_absolute_path_returns_false (Copilot's exact target) - test_is_recovered_escaping_path_returns_false (same comment-class issue; fixed preemptively to avoid a Round-9 finding on the same drift) Pure documentation change. Test assertions and behavior unchanged; all manifest tests still green.
1 parent 1dbf0c2 commit 7666030

1 file changed

Lines changed: 14 additions & 7 deletions

File tree

tests/integrations/test_manifest.py

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -356,10 +356,12 @@ def test_load_rejects_non_list_recovered_files(self, tmp_path):
356356

357357
def test_is_recovered_absolute_path_returns_false(self, tmp_path):
358358
# Copilot round-5 finding: passing an absolute path silently returned
359-
# False because the stored keys are relative POSIX strings. Now the
360-
# call normalizes through ``_validate_rel_path`` which raises on
361-
# absolute inputs; we catch and return False so query semantics stay
362-
# exception-free.
359+
# False because the stored keys are relative POSIX strings. Round-7
360+
# made this explicit: ``is_recovered`` now rejects absolute paths
361+
# up front via a lexical ``rel.is_absolute()`` guard and returns
362+
# False without calling ``_validate_rel_path`` at all — matching
363+
# ``record_existing``'s canonical-key guard so the two methods
364+
# agree on which inputs can ever be stored keys.
363365
(tmp_path / "f.txt").write_text("x", encoding="utf-8")
364366
m = IntegrationManifest("test", tmp_path)
365367
m.record_existing("f.txt", recovered=True)
@@ -368,9 +370,14 @@ def test_is_recovered_absolute_path_returns_false(self, tmp_path):
368370
assert m.is_recovered(abs_input) is False
369371

370372
def test_is_recovered_escaping_path_returns_false(self, tmp_path):
371-
# A relative path that resolves outside project_root cannot have been
372-
# recorded; ``_validate_rel_path`` raises and ``is_recovered`` returns
373-
# False rather than letting the ValueError propagate.
373+
# A relative path containing ``..`` segments cannot be a stored key:
374+
# Round-7 added the same lexical ``".." in rel.parts`` guard to
375+
# ``is_recovered`` that ``record_existing`` already enforces, so the
376+
# method returns False immediately without reaching
377+
# ``_validate_rel_path``. The try/except around ``_validate_rel_path``
378+
# remains as defense-in-depth for paths that pass the lexical guard
379+
# but still resolve outside the project root via a symlinked
380+
# ancestor.
374381
m = IntegrationManifest("test", tmp_path)
375382
# Don't record anything — the path is impossible to record anyway.
376383
assert m.is_recovered("../escape.txt") is False

0 commit comments

Comments
 (0)