From 4f9d5cf4a711f1d681553fc2ecad23ecb4329879 Mon Sep 17 00:00:00 2001 From: "Xingdi (Eric) Yuan" <4028684+xingdi-eric-yuan@users.noreply.github.com> Date: Wed, 23 Sep 2026 00:40:53 -0400 Subject: [PATCH 1/2] Constrain reconciler writes to the shadow tree Validate untrusted manifest paths and symlink-resolved destinations before publication, including lower-level writers and archive/metadata outputs. Preserve valid Unicode and spaced relative paths and fail explicitly on unsafe destinations. Fixes #36 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- CHANGELOG.md | 6 + RESPONSIBLE_AI.md | 6 + skills/shadow-frog-dream/SKILL.md | 5 + skills/shadow-frog-dream/dream-reconcile.py | 195 +++++++++++-- tests/conftest.py | 4 +- .../shadow_frog_dream/test_dream_reconcile.py | 53 ++-- .../test_dream_reconcile_paths.py | 261 ++++++++++++++++++ 7 files changed, 482 insertions(+), 48 deletions(-) create mode 100644 tests/skills/shadow_frog_dream/test_dream_reconcile_paths.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 875a339..773f964 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,12 @@ shadow knowledge bases for any codebase. ## Unreleased +### Fixed +- **Reconciliation path containment** — validate untrusted manifest destinations + and filesystem aliases before writing discoveries, cross-references, or + archives. Unsafe paths fail explicitly, including during dry runs, rather + than modifying files outside the shadow tree. + ### Changed - **More concise documentation** — consolidated README onboarding and workflow guidance, with advanced operations linked to the skill references. Condensed diff --git a/RESPONSIBLE_AI.md b/RESPONSIBLE_AI.md index 94ea957..f5ad374 100644 --- a/RESPONSIBLE_AI.md +++ b/RESPONSIBLE_AI.md @@ -46,6 +46,12 @@ them only while active or resumed work needs them. Hash verification detects unexpected changes to the pinned bundle; it is not an attestation that arbitrary third-party code is trustworthy. +Reconciliation treats remote manifest paths as untrusted and confines discovery, +back-pointer, archive, and metadata writes to the shadow tree. Unsafe paths and +outward-pointing symlinks stop reconciliation before publication. This does not +validate discovery truth, sandbox experiment code, or protect against concurrent +local filesystem tampering. + A detailed discussion of ShadowFrog, including how it was developed and tested, can be found in our [blog post](https://microsoft.github.io/debug-gym/blog/2026/06/shadow-frog/). ### Intended Uses diff --git a/skills/shadow-frog-dream/SKILL.md b/skills/shadow-frog-dream/SKILL.md index 5183f90..c643813 100644 --- a/skills/shadow-frog-dream/SKILL.md +++ b/skills/shadow-frog-dream/SKILL.md @@ -948,6 +948,11 @@ or manually imitate its branch-deletion steps. ### What the Reconciler Does +Manifest destinations are preflighted before any writes, including in dry runs. +Absolute/traversal paths and symlinks escaping `.shadow/` fail with a nonzero +exit. Repair the indicated manifest field or filesystem alias before retrying; +do not bypass containment checks. + 1. **Discovers** new branches (namespace-filtered, not in `_index.md`) 2. **Reads/validates** manifests from remote branches 3. **Merges** discoveries into main's per-file shadows (semantic dedup; on an exact-text duplicate it upgrades the existing entry's metadata — unions labels, raises source trust, promotes `uncertain`→`verified` — but never alters a `refuted` status) diff --git a/skills/shadow-frog-dream/dream-reconcile.py b/skills/shadow-frog-dream/dream-reconcile.py index 34df2b0..1f719de 100755 --- a/skills/shadow-frog-dream/dream-reconcile.py +++ b/skills/shadow-frog-dream/dream-reconcile.py @@ -28,7 +28,8 @@ 8. Verify all artifacts present 9. (Optional) Prune unprotected reconciled branches — only after push -Exits 0 on success, 1 on verification or coherent-lineage read failure. +Exits 0 on success, 1 on verification, unsafe output paths, or lineage failure. +Manifest destinations are validated before writes, including in dry runs. On a lineage read failure, restore the indexed manifest or repair its stale index entry after checking descendants; no branches are deleted. """ @@ -40,7 +41,7 @@ import subprocess import sys from datetime import datetime, timezone -from pathlib import Path +from pathlib import Path, PureWindowsPath # Shared safety gate for `rm -rf `. Lives next to this script so # bash callers (dream-cleanup.sh, dream-gc.sh) and this module share ONE @@ -150,6 +151,125 @@ def _canonical_header_lines(shadow_path): ] +class UnsafeShadowPath(ValueError): + """Untrusted metadata or a filesystem alias escapes the shadow output tree.""" + + +def _relative_parts(value, field, *, single=False): + if not isinstance(value, str) or not value: + raise UnsafeShadowPath(f"{field}: expected a nonempty relative path, got {value!r}") + parts = value.split('/') + if ( + any(not part.rstrip(' .') for part in parts) + or any(char in value for char in ('\\', ':', '\0', '\r', '\n')) + or PureWindowsPath(value).drive + or (single and len(parts) != 1) + ): + raise UnsafeShadowPath(f"{field}: unsafe relative path {value!r}") + return parts + + +def _checked_shadow_destination(repo_root, destination, field): + """Check lexical and resolved containment, including root and leaf symlinks.""" + try: + repo = Path(repo_root).absolute() + shadow = repo / '.shadow' + target = Path(destination).absolute() + try: + relative = target.relative_to(shadow) + except ValueError: + raise UnsafeShadowPath(f"{field}: destination is outside .shadow: {destination!r}") from None + _relative_parts(relative.as_posix(), field) + resolved_repo = repo.resolve() + resolved_shadow = shadow.resolve() + resolved_target = target.resolve() + if resolved_shadow == resolved_repo or not resolved_shadow.is_relative_to(resolved_repo): + raise UnsafeShadowPath(f"{field}: .shadow resolves outside the repository") + if resolved_target == resolved_shadow or not resolved_target.is_relative_to(resolved_shadow): + raise UnsafeShadowPath(f"{field}: destination resolves outside .shadow: {destination!r}") + return str(target) + except (OSError, RuntimeError) as exc: + raise UnsafeShadowPath(f"{field}: cannot establish destination containment: {exc}") from exc + + +def _shadow_output_path(repo_root, relative, field): + parts = _relative_parts(relative, field) + return _checked_shadow_destination( + repo_root, Path(repo_root).absolute().joinpath('.shadow', *parts), field, + ) + + +def _shadow_file_path(repo_root, file_part, field): + _relative_parts(file_part, field) + return _shadow_output_path(repo_root, file_part + '.md', field) + + +def _anchor_file_part(anchor, field): + if not isinstance(anchor, str): + raise UnsafeShadowPath(f"{field}: expected anchor text, got {anchor!r}") + if '::' not in anchor: + return None + file_part = anchor.split('::', 1)[0] + _relative_parts(file_part, field) + return file_part + + +def _manifest_entries(manifest, key, dream_id): + entries = manifest.get(key, []) + if not isinstance(entries, list): + raise UnsafeShadowPath(f"dream {dream_id} {key}: expected a list") + for index, entry in enumerate(entries): + if isinstance(entry, str): + entry = ( + {'anchor': '', 'text': entry} if key == 'discoveries' else + {'slug': re.sub(r'[^a-z0-9]+', '-', entry[:60].lower()).strip('-'), + 'description': entry} + ) + if not isinstance(entry, dict): + raise UnsafeShadowPath(f"dream {dream_id} {key}[{index}]: expected an object") + yield index, entry + + +def _validate_refs(repo_root, refs, field): + if not isinstance(refs, list): + raise UnsafeShadowPath(f"{field}: expected a list") + for index, ref in enumerate(refs): + label = f"{field}[{index}]" + file_part = _anchor_file_part(ref, label) + if file_part is not None: + _shadow_file_path(repo_root, file_part, label) + + +def _validate_manifest_paths(repo_root, dream_id, manifest): + label = f"dream {dream_id}" + _relative_parts(dream_id, f"{label} dream_id", single=True) + if not isinstance(manifest, dict): + raise UnsafeShadowPath(f"{label}: manifest must be an object") + for filename in ('report.md', 'manifest.json', 'patch.diff'): + _shadow_output_path(repo_root, f'_dreams/{dream_id}/{filename}', f"{label} {filename}") + for index, disc in _manifest_entries(manifest, 'discoveries', dream_id): + field = f"{label} discoveries[{index}].anchor" + file_part = _anchor_file_part(disc.get('anchor', ''), field) + if file_part is not None: + _shadow_file_path(repo_root, file_part, field) + for index, cross in _manifest_entries(manifest, 'cross_cutting', dream_id): + field = f"{label} cross_cutting[{index}]" + slug = cross.get('slug', '') + if not slug: + continue + _relative_parts(slug, f"{field}.slug", single=True) + _shadow_output_path(repo_root, f'_cross/{slug}.md', f"{field}.slug") + _validate_refs(repo_root, cross.get('refs', []) or [], f"{field}.refs") + + +def _validate_reconciliation_paths(repo_root, manifests): + """Preflight the entire batch before publishing any discovery or metadata.""" + for relative in ('_dreams/_index.md', '_meta/state.json', '_index.md'): + _shadow_output_path(repo_root, relative, f"reconciliation {relative}") + for _, dream_id, manifest in manifests: + _validate_manifest_paths(repo_root, dream_id, manifest) + + # --- Git helpers --- def git(*args, cwd=None, check=True): @@ -248,6 +368,7 @@ def load_manifests(repo_root, branches): skipped = [] for branch, dream_id in branches: + _relative_parts(dream_id, f"dream {dream_id} dream_id", single=True) manifest_path = f'.shadow/_dreams/{dream_id}/manifest.json' raw = git_show(f'origin/{branch}', manifest_path, cwd=repo_root) @@ -261,6 +382,8 @@ def load_manifests(repo_root, branches): skipped.append((branch, dream_id, f"invalid JSON: {e}")) continue + if not isinstance(manifest, dict): + raise UnsafeShadowPath(f"dream {dream_id}: manifest must be an object") # Validate dream_id consistency m_did = manifest.get('dream_id', '') if m_did != dream_id: @@ -274,6 +397,7 @@ def load_manifests(repo_root, branches): manifests.append((branch, dream_id, manifest)) + _validate_reconciliation_paths(repo_root, manifests) return manifests, skipped @@ -434,8 +558,9 @@ def _format_meta_line(status, source, labels): return f' _({", ".join(parts)})_\n' -def merge_discovery_into_file(shadow_path, anchor_symbol, discovery, dream_id): +def merge_discovery_into_file(shadow_path, anchor_symbol, discovery, dream_id, *, repo_root): """Merge a single discovery into a shadow file. Returns True if written.""" + shadow_path = _checked_shadow_destination(repo_root, shadow_path, f"dream {dream_id} discovery") text = discovery.get('text', '').strip() if not text: return False @@ -544,7 +669,10 @@ def add_cross_reference_backpointer(repo_root, file_part, slug, title, dream_id) `_cross/.md` must have a matching entry in each referenced file's ## Cross-References section. Idempotent (skips if back-pointer exists). """ - shadow_path = os.path.join(repo_root, '.shadow', file_part + '.md') + field = f"dream {dream_id} back-pointer" + shadow_path = _shadow_file_path(repo_root, file_part, field) + _relative_parts(slug, f"{field} slug", single=True) + _shadow_output_path(repo_root, f'_cross/{slug}.md', f"{field} cross file") # Relative link from .shadow/.md back up to .shadow/_cross/.md. # For a top-level file (no slashes) the prefix is empty; each directory of # depth adds one "../". Otherwise the markdown link is broken and the @@ -610,7 +738,7 @@ def add_cross_reference_backpointer(repo_root, file_part, slug, title, dream_id) return True -def _merge_refs_into_cross_file(cross_path, new_refs): +def _merge_refs_into_cross_file(cross_path, new_refs, *, repo_root): """Union new refs into an existing _cross/.md **Refs**: section. When two dreams use the same cross-cutting slug, the later one must not @@ -618,6 +746,8 @@ def _merge_refs_into_cross_file(cross_path, new_refs): at this cross file (below), so its **Refs**: block must list them or the bidirectional-reference invariant breaks. Returns True if modified. """ + cross_path = _checked_shadow_destination(repo_root, cross_path, "cross-cutting destination") + _validate_refs(repo_root, new_refs, "cross-cutting refs") try: with open(cross_path, encoding="utf-8") as f: content = f.read() @@ -651,22 +781,21 @@ def _merge_refs_into_cross_file(cross_path, new_refs): def merge_discoveries(repo_root, manifests, dry_run=False): """Merge all discoveries from manifests into main's shadow files.""" + _validate_reconciliation_paths(repo_root, manifests) merged_count = 0 skipped_count = 0 for branch, dream_id, manifest in manifests: - discoveries = manifest.get('discoveries', []) - for disc in discoveries: - # Normalize string discoveries to dicts - if isinstance(disc, str): - disc = {'anchor': '', 'text': disc} + for index, disc in _manifest_entries(manifest, 'discoveries', dream_id): anchor = disc.get('anchor', '') if '::' not in anchor: skipped_count += 1 continue file_part, symbol = anchor.split('::', 1) - shadow_path = os.path.join(repo_root, '.shadow', file_part + '.md') + shadow_path = _shadow_file_path( + repo_root, file_part, f"dream {dream_id} discoveries[{index}].anchor", + ) if dry_run: print(f" Would merge: {anchor} <- {disc.get('text', '')[:60]}") @@ -676,22 +805,22 @@ def merge_discoveries(repo_root, manifests, dry_run=False): # Ensure shadow directory exists os.makedirs(os.path.dirname(shadow_path), exist_ok=True) - if merge_discovery_into_file(shadow_path, symbol, disc, dream_id): + if merge_discovery_into_file( + shadow_path, symbol, disc, dream_id, repo_root=repo_root, + ): merged_count += 1 else: skipped_count += 1 # Handle cross-cutting discoveries - cross_cutting = manifest.get('cross_cutting', []) - for cross in cross_cutting: - # Normalize string entries to dicts - if isinstance(cross, str): - cross = {'slug': re.sub(r'[^a-z0-9]+', '-', cross[:60].lower()).strip('-'), 'description': cross} + for index, cross in _manifest_entries(manifest, 'cross_cutting', dream_id): slug = cross.get('slug', '') if not slug: continue - cross_path = os.path.join(repo_root, '.shadow', '_cross', f'{slug}.md') + cross_path = _shadow_output_path( + repo_root, f'_cross/{slug}.md', f"dream {dream_id} cross_cutting[{index}].slug", + ) refs = cross.get('refs', []) or [] title = cross.get('title', slug) @@ -722,7 +851,7 @@ def merge_discoveries(repo_root, manifests, dry_run=False): # Cross file already exists (e.g. a prior dream used the same # slug). Union our refs into its **Refs**: block so it stays # consistent with the back-pointers added below. - if _merge_refs_into_cross_file(cross_path, refs): + if _merge_refs_into_cross_file(cross_path, refs, repo_root=repo_root): merged_count += 1 else: skipped_count += 1 @@ -749,11 +878,18 @@ def merge_discoveries(repo_root, manifests, dry_run=False): def mirror_reports(repo_root, manifests, dry_run=False): """Copy report.md, manifest.json, patch.diff from branches to main.""" + _validate_reconciliation_paths(repo_root, manifests) mirrored = 0 corrupted = [] for branch, dream_id, manifest in manifests: - dream_dir = os.path.join(repo_root, '.shadow', '_dreams', dream_id) + dream_dir = _shadow_output_path(repo_root, f'_dreams/{dream_id}', f"dream {dream_id} archive") + paths = { + filename: _shadow_output_path( + repo_root, f'_dreams/{dream_id}/{filename}', f"dream {dream_id} {filename}", + ) + for filename in ('report.md', 'manifest.json', 'patch.diff') + } if dry_run: print(f" Would mirror: {dream_id}/") @@ -778,16 +914,16 @@ def mirror_reports(repo_root, manifests, dry_run=False): # and patch below are still mirrored unconditionally so a # single bad frontmatter line never discards valid # artifacts (discoveries are read from the manifest). - with open(os.path.join(dream_dir, 'report.md'), 'w', encoding="utf-8") as f: + with open(paths['report.md'], 'w', encoding="utf-8") as f: f.write(f"# Corrupted Report\n\nContained content from {report_did}.\n" f"Original on branch: {branch}\n") if not report_corrupt: - with open(os.path.join(dream_dir, 'report.md'), 'w', encoding="utf-8") as f: + with open(paths['report.md'], 'w', encoding="utf-8") as f: f.write(report) # Mirror manifest - with open(os.path.join(dream_dir, 'manifest.json'), 'w', encoding="utf-8") as f: + with open(paths['manifest.json'], 'w', encoding="utf-8") as f: json.dump(manifest, f, indent=2) # Mirror patch @@ -802,7 +938,7 @@ def mirror_reports(repo_root, manifests, dry_run=False): # write only when truly absent. patch = git_show(f'origin/{branch}', f'.shadow/_dreams/{dream_id}/patch.diff', cwd=repo_root) if patch is not None: - with open(os.path.join(dream_dir, 'patch.diff'), 'w', encoding="utf-8") as f: + with open(paths['patch.diff'], 'w', encoding="utf-8") as f: f.write(patch) mirrored += 1 @@ -892,7 +1028,8 @@ def _resolve_parent_branch(repo_root, branch, dream_id, manifest): def update_index(repo_root, manifests, dry_run=False): """Add entries to _dreams/_index.md for reconciled branches.""" - index_path = os.path.join(repo_root, '.shadow', '_dreams', '_index.md') + _validate_reconciliation_paths(repo_root, manifests) + index_path = _shadow_output_path(repo_root, '_dreams/_index.md', "dream index") if dry_run: for branch, dream_id, manifest in manifests: @@ -970,7 +1107,7 @@ def _count_discoveries(shadow_path): def update_state(repo_root, manifests, dry_run=False): """Update _meta/state.json with dream reconciliation metadata.""" - state_path = os.path.join(repo_root, '.shadow', '_meta', 'state.json') + state_path = _shadow_output_path(repo_root, '_meta/state.json', "state.json") if not os.path.isfile(state_path): if dry_run: @@ -1132,7 +1269,7 @@ def rebuild_top_index(repo_root, dry_run=False): init provenance survives reconciler rewrites. Honors `dry_run`. """ shadow_dir = os.path.join(repo_root, '.shadow') - index_path = os.path.join(shadow_dir, '_index.md') + index_path = _shadow_output_path(repo_root, '_index.md', "shadow index") if dry_run: print(" Would regenerate _index.md") @@ -1743,6 +1880,8 @@ def main(): print("ERROR: Not in a git repository", file=sys.stderr) sys.exit(1) + _validate_reconciliation_paths(repo_root, []) + # Resolve namespace dream_ns = namespace_override or os.environ.get('DREAM_NAMESPACE', '') if not dream_ns: @@ -1927,6 +2066,6 @@ def main(): if __name__ == '__main__': try: main() - except CoherentLineageError as exc: + except (CoherentLineageError, UnsafeShadowPath) as exc: print(f"ERROR: {exc}", file=sys.stderr) sys.exit(1) diff --git a/tests/conftest.py b/tests/conftest.py index d0aa123..25cd182 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -98,9 +98,9 @@ def nap(repo_root): @pytest.fixture def make_symlink(): """Create a symlink, skipping only when Windows denies the privilege.""" - def create(link, target): + def create(link, target, *, target_is_directory=False): try: - link.symlink_to(target) + link.symlink_to(target, target_is_directory=target_is_directory) except OSError as exc: if os.name == "nt" and exc.winerror == 1314: pytest.skip( diff --git a/tests/skills/shadow_frog_dream/test_dream_reconcile.py b/tests/skills/shadow_frog_dream/test_dream_reconcile.py index bbb67d4..49b15d6 100644 --- a/tests/skills/shadow_frog_dream/test_dream_reconcile.py +++ b/tests/skills/shadow_frog_dream/test_dream_reconcile.py @@ -354,6 +354,12 @@ def test_add_cross_reference_backpointer_replaces_placeholder( # merge_discovery_into_file # =========================================================================== +def _shadow_under(repo, name="foo.md"): + shadow = repo / ".shadow" / name + shadow.parent.mkdir(parents=True, exist_ok=True) + return shadow + + def test_merge_discovery_creates_new_shadow(dream_reconcile, tmp_path): shadow = tmp_path / ".shadow" / "src" / "foo.py.md" written = dream_reconcile.merge_discovery_into_file( @@ -362,6 +368,7 @@ def test_merge_discovery_creates_new_shadow(dream_reconcile, tmp_path): {"text": "Returns None on empty input.", "status": "verified", "source": "exploration"}, "20260101-000000Z-x", + repo_root=str(tmp_path), ) assert written is True assert shadow.is_file() @@ -374,7 +381,7 @@ def test_merge_discovery_creates_new_shadow(dream_reconcile, tmp_path): def test_merge_discovery_replaces_no_discoveries_placeholder(dream_reconcile, tmp_path): - shadow = tmp_path / "foo.md" + shadow = _shadow_under(tmp_path) shadow.write_text( "## `foo`\n\n_No discoveries yet._\n\n## Cross-References\n\n_No cross-cutting discoveries yet._\n", encoding="utf-8" ) @@ -383,6 +390,7 @@ def test_merge_discovery_replaces_no_discoveries_placeholder(dream_reconcile, tm {"text": "Actual discovery.", "status": "verified", "source": "exploration"}, "20260101-000000Z-x", + repo_root=str(tmp_path), ) body = shadow.read_text(encoding="utf-8") assert "_No discoveries yet._" not in body @@ -390,7 +398,7 @@ def test_merge_discovery_replaces_no_discoveries_placeholder(dream_reconcile, tm def test_merge_discovery_appends_after_existing_bullets(dream_reconcile, tmp_path): - shadow = tmp_path / "foo.md" + shadow = _shadow_under(tmp_path) shadow.write_text( "## `foo`\n\n- existing discovery\n _(verified, source: exploration)_\n\n" "## Cross-References\n\n_No cross-cutting discoveries yet._\n", encoding="utf-8" @@ -400,6 +408,7 @@ def test_merge_discovery_appends_after_existing_bullets(dream_reconcile, tmp_pat {"text": "Another discovery.", "status": "verified", "source": "exploration"}, "20260101-000000Z-x", + repo_root=str(tmp_path), ) body = shadow.read_text(encoding="utf-8") assert "- existing discovery" in body @@ -409,7 +418,7 @@ def test_merge_discovery_appends_after_existing_bullets(dream_reconcile, tmp_pat def test_merge_discovery_skips_duplicate(dream_reconcile, tmp_path): - shadow = tmp_path / "foo.md" + shadow = _shadow_under(tmp_path) shadow.write_text( "## `foo`\n\n- already here\n _(verified, source: exploration)_\n\n" "## Cross-References\n\n_No cross-cutting discoveries yet._\n", encoding="utf-8" @@ -418,14 +427,15 @@ def test_merge_discovery_skips_duplicate(dream_reconcile, tmp_path): str(shadow), "foo", {"text": "already here", "status": "verified", "source": "exploration"}, "20260101-000000Z-x", + repo_root=str(tmp_path), ) assert written is False def test_merge_discovery_empty_text_returns_false(dream_reconcile, tmp_path): - shadow = tmp_path / "foo.md" + shadow = _shadow_under(tmp_path) written = dream_reconcile.merge_discovery_into_file( - str(shadow), "foo", {"text": " "}, "20260101-000000Z-x" + str(shadow), "foo", {"text": " "}, "20260101-000000Z-x", repo_root=str(tmp_path), ) assert written is False assert not shadow.exists() @@ -441,7 +451,7 @@ def _xref_footer(): def test_merge_discovery_unions_labels_on_exact_match(dream_reconcile, tmp_path): - shadow = tmp_path / "foo.md" + shadow = _shadow_under(tmp_path) shadow.write_text( "## `foo`\n\n- claim text here\n _(verified, source: exploration, labels: [bug])_\n" + _xref_footer(), encoding="utf-8" @@ -451,6 +461,7 @@ def test_merge_discovery_unions_labels_on_exact_match(dream_reconcile, tmp_path) {"text": "claim text here", "status": "verified", "source": "exploration", "labels": ["security"]}, "20260101-000000Z-x", + repo_root=str(tmp_path), ) assert written is True body = shadow.read_text(encoding="utf-8") @@ -460,7 +471,7 @@ def test_merge_discovery_unions_labels_on_exact_match(dream_reconcile, tmp_path) def test_merge_discovery_upgrades_source_trust(dream_reconcile, tmp_path): - shadow = tmp_path / "foo.md" + shadow = _shadow_under(tmp_path) shadow.write_text( "## `foo`\n\n- some claim\n _(verified, source: exploration)_\n" + _xref_footer(), encoding="utf-8" @@ -469,6 +480,7 @@ def test_merge_discovery_upgrades_source_trust(dream_reconcile, tmp_path): str(shadow), "foo", {"text": "some claim", "status": "verified", "source": "user"}, "20260101-000000Z-x", + repo_root=str(tmp_path), ) assert written is True body = shadow.read_text(encoding="utf-8") @@ -477,7 +489,7 @@ def test_merge_discovery_upgrades_source_trust(dream_reconcile, tmp_path): def test_merge_discovery_upgrades_uncertain_to_verified(dream_reconcile, tmp_path): - shadow = tmp_path / "foo.md" + shadow = _shadow_under(tmp_path) shadow.write_text( "## `foo`\n\n- a claim\n _(uncertain, source: exploration)_\n" + _xref_footer(), encoding="utf-8" @@ -486,6 +498,7 @@ def test_merge_discovery_upgrades_uncertain_to_verified(dream_reconcile, tmp_pat str(shadow), "foo", {"text": "a claim", "status": "verified", "source": "exploration"}, "20260101-000000Z-x", + repo_root=str(tmp_path), ) assert written is True body = shadow.read_text(encoding="utf-8") @@ -493,7 +506,7 @@ def test_merge_discovery_upgrades_uncertain_to_verified(dream_reconcile, tmp_pat def test_merge_discovery_never_downgrades_verified(dream_reconcile, tmp_path): - shadow = tmp_path / "foo.md" + shadow = _shadow_under(tmp_path) shadow.write_text( "## `foo`\n\n- a claim\n _(verified, source: exploration)_\n" + _xref_footer(), encoding="utf-8" @@ -502,6 +515,7 @@ def test_merge_discovery_never_downgrades_verified(dream_reconcile, tmp_path): str(shadow), "foo", {"text": "a claim", "status": "uncertain", "source": "exploration"}, "20260101-000000Z-x", + repo_root=str(tmp_path), ) # Nothing to upgrade → no write. assert written is False @@ -509,7 +523,7 @@ def test_merge_discovery_never_downgrades_verified(dream_reconcile, tmp_path): def test_merge_discovery_never_touches_refuted(dream_reconcile, tmp_path): - shadow = tmp_path / "foo.md" + shadow = _shadow_under(tmp_path) shadow.write_text( "## `foo`\n\n- a claim\n _(refuted, source: exploration)_\n" + _xref_footer(), encoding="utf-8" @@ -520,6 +534,7 @@ def test_merge_discovery_never_touches_refuted(dream_reconcile, tmp_path): {"text": "a claim", "status": "verified", "source": "exploration", "labels": ["bug"]}, "20260101-000000Z-x", + repo_root=str(tmp_path), ) body = shadow.read_text(encoding="utf-8") # Status stays refuted; labels may still union. @@ -532,7 +547,7 @@ def test_merge_discovery_fuzzy_match_still_skips(dream_reconcile, tmp_path): they may be genuinely different claims.""" existing = ("- the function returns none when the input list is " "completely empty or missing entirely") - shadow = tmp_path / "foo.md" + shadow = _shadow_under(tmp_path) shadow.write_text( f"## `foo`\n\n{existing}\n _(uncertain, source: exploration)_\n" + _xref_footer(), encoding="utf-8" @@ -544,6 +559,7 @@ def test_merge_discovery_fuzzy_match_still_skips(dream_reconcile, tmp_path): str(shadow), "foo", {"text": new_text, "status": "verified", "source": "exploration"}, "20260101-000000Z-x", + repo_root=str(tmp_path), ) # Treated as fuzzy duplicate → skipped, metadata untouched. assert written is False @@ -585,7 +601,7 @@ def test_merge_meta_refuted_untouched(self, dream_reconcile): def test_merge_discovery_with_also_involves_and_labels(dream_reconcile, tmp_path): - shadow = tmp_path / "foo.md" + shadow = _shadow_under(tmp_path) dream_reconcile.merge_discovery_into_file( str(shadow), "foo", { @@ -594,6 +610,7 @@ def test_merge_discovery_with_also_involves_and_labels(dream_reconcile, tmp_path "also_involves": ["other.py::thing", "more.py::stuff"], }, "20260101-000000Z-x", + repo_root=str(tmp_path), ) body = shadow.read_text(encoding="utf-8") assert "labels: [bug, security]" in body @@ -3223,7 +3240,7 @@ def test_canonical_header_unknown_language(self, dream_reconcile): class TestMergeRefsIntoCrossFile: def _write_cross(self, tmp_path, refs): - p = tmp_path / "slug.md" + p = _shadow_under(tmp_path, "_cross/slug.md") body = ["# Title", "", "**Category**: pattern", "**Refs**:"] body += [f"- `{r}`" for r in refs] body += ["", "**Discovery**: something", ""] @@ -3233,7 +3250,7 @@ def _write_cross(self, tmp_path, refs): def test_unions_new_refs(self, dream_reconcile, tmp_path): p = self._write_cross(tmp_path, ["src/a.py::f"]) changed = dream_reconcile._merge_refs_into_cross_file( - str(p), ["src/b.py::g", "src/a.py::f"] + str(p), ["src/b.py::g", "src/a.py::f"], repo_root=str(tmp_path), ) assert changed is True text = p.read_text(encoding="utf-8") @@ -3246,21 +3263,21 @@ def test_no_change_when_all_present(self, dream_reconcile, tmp_path): p = self._write_cross(tmp_path, ["src/a.py::f"]) before = p.read_text(encoding="utf-8") changed = dream_reconcile._merge_refs_into_cross_file( - str(p), ["src/a.py::f"] + str(p), ["src/a.py::f"], repo_root=str(tmp_path), ) assert changed is False assert p.read_text(encoding="utf-8") == before def test_missing_file_returns_false(self, dream_reconcile, tmp_path): assert dream_reconcile._merge_refs_into_cross_file( - str(tmp_path / "nope.md"), ["x::y"] + str(tmp_path / ".shadow/_cross/nope.md"), ["x::y"], repo_root=str(tmp_path), ) is False def test_no_refs_block_returns_false(self, dream_reconcile, tmp_path): - p = tmp_path / "norefs.md" + p = _shadow_under(tmp_path, "_cross/norefs.md") p.write_text("# Title\n\nNo refs section here.\n", encoding="utf-8") assert dream_reconcile._merge_refs_into_cross_file( - str(p), ["x::y"] + str(p), ["x::y"], repo_root=str(tmp_path), ) is False # File untouched. assert "No refs section here." in p.read_text(encoding="utf-8") diff --git a/tests/skills/shadow_frog_dream/test_dream_reconcile_paths.py b/tests/skills/shadow_frog_dream/test_dream_reconcile_paths.py new file mode 100644 index 0000000..b9e5b64 --- /dev/null +++ b/tests/skills/shadow_frog_dream/test_dream_reconcile_paths.py @@ -0,0 +1,261 @@ +"""Untrusted manifest destinations must stay inside the shadow output tree.""" + +from copy import deepcopy +import subprocess +import sys + +import pytest + +from tests.skills.shadow_frog_dream.test_dream_reconcile import ( + SCRIPT, + _add_bare_remote, + _default_manifest, + _default_report, + _seed_repo, + make_dream_branch, +) + + +DREAM_ID = "20260923-040000Z-paths" + + +def batch(anchor="src/caf\u00e9 tools.py::run", refs=None, slug="shared-behavior"): + manifest = _default_manifest(DREAM_ID) + manifest["discoveries"] = [ + {"anchor": anchor, "text": "Empty input is rejected before dispatch."}, + ] + if refs is not None: + manifest["cross_cutting"] = [ + {"slug": slug, "title": "Shared behavior", "text": "Callers share state.", "refs": refs}, + ] + return [(manifest["branch"], DREAM_ID, manifest)] + + +def tree_bytes(root): + return { + path.relative_to(root).as_posix(): path.read_bytes() + for path in root.rglob("*") if path.is_file() and not path.is_symlink() + } + + +@pytest.mark.parametrize("cross_ref", [False, True]) +@pytest.mark.parametrize("dry_run", [False, True]) +@pytest.mark.slow +def test_cli_rejects_unsafe_manifest_before_any_shadow_writes( + tmp_git_repo, cross_ref, dry_run, +): + env = _seed_repo(tmp_git_repo) + _add_bare_remote(tmp_git_repo, env) + outside = tmp_git_repo.parent / "outside.md" + outside.write_text("keep this file\n", encoding="utf-8") + malicious = f"{outside.with_suffix('').as_posix()}::outside" + entries = batch() + manifest = entries[0][2] + if cross_ref: + manifest["cross_cutting"] = [{ + "slug": "cross", "text": "Invalid reference.", + "refs": ["src/valid.py::run", malicious], + }] + else: + manifest["discoveries"].append({"anchor": malicious, "text": "Do not write here."}) + make_dream_branch( + tmp_git_repo, env, "proj", DREAM_ID, manifest, + report=_default_report(DREAM_ID), + ) + command = [ + sys.executable, str(SCRIPT), str(tmp_git_repo), "--namespace", "proj", + ] + if dry_run: + command.append("--dry-run") + result = subprocess.run(command, capture_output=True, text=True, encoding="utf-8") + assert result.returncode == 1, result.stdout + result.stderr + assert "ERROR:" in result.stderr and DREAM_ID in result.stderr + assert "Traceback" not in result.stderr + assert "Reconciliation complete" not in result.stdout + assert outside.read_text(encoding="utf-8") == "keep this file\n" + assert not (tmp_git_repo / ".shadow").exists() + + +@pytest.mark.parametrize("file_part", [ + "../outside", "src/../../outside", "/absolute/file", + "C:/Users/example/file", "C:relative", "//server/share/file", + r"\\server\share\file", r"src\..\outside", "src/file:stream", + "", ".", "..", "src/./file", "src//file", "src/file\0name", + "src/.. /outside", "src/.../outside", "src/\nfile", "src/\rfile", +]) +@pytest.mark.parametrize("cross_ref", [False, True]) +@pytest.mark.parametrize("dry_run", [False, True]) +def test_manifest_paths_are_validated_before_writing_any_entry( + dream_reconcile, tmp_path, file_part, cross_ref, dry_run, +): + entries = batch() + manifest = entries[0][2] + invalid = f"{file_part}::run" + if cross_ref: + manifest["cross_cutting"] = [ + {"slug": "good", "refs": ["good.py::run"], "text": "Safe first entry."}, + {"slug": "bad", "refs": [invalid], "text": "Unsafe second entry."}, + ] + else: + manifest["discoveries"].append({"anchor": invalid, "text": "Unsafe second entry."}) + with pytest.raises(dream_reconcile.UnsafeShadowPath, match=DREAM_ID): + dream_reconcile.merge_discoveries(str(tmp_path), entries, dry_run=dry_run) + assert list(tmp_path.iterdir()) == [] + + +@pytest.mark.parametrize("field", ["slug", "dream_id"]) +@pytest.mark.parametrize("value", ["../outside", "/outside", "C:/outside", r"..\outside", ".", ".."]) +def test_artifact_names_cannot_escape_their_subdirectory(dream_reconcile, tmp_path, field, value): + entries = batch(refs=["a.py::run"]) + if field == "slug": + entries[0][2]["cross_cutting"][0]["slug"] = value + else: + entries = [(entries[0][0], value, entries[0][2])] + with pytest.raises(dream_reconcile.UnsafeShadowPath): + dream_reconcile.merge_discoveries(str(tmp_path), entries) + assert list(tmp_path.iterdir()) == [] + + +def test_later_invalid_manifest_cannot_partially_publish_the_batch(dream_reconcile, tmp_path): + entries = batch() + invalid = batch("../outside::run") + with pytest.raises(dream_reconcile.UnsafeShadowPath): + dream_reconcile.merge_discoveries(str(tmp_path), entries + invalid) + assert list(tmp_path.iterdir()) == [] + + +@pytest.mark.parametrize("value", [None, 1, [], {}]) +def test_nontext_anchor_is_rejected_before_outputs(dream_reconcile, tmp_path, value): + with pytest.raises(dream_reconcile.UnsafeShadowPath): + dream_reconcile.merge_discoveries(str(tmp_path), batch(value)) + assert list(tmp_path.iterdir()) == [] + + +@pytest.mark.parametrize("target", [ + "src", "src/target.py.md", "_cross", "_cross/shared-behavior.md", + "_dreams", f"_dreams/{DREAM_ID}", f"_dreams/{DREAM_ID}/report.md", + f"_dreams/{DREAM_ID}/manifest.json", f"_dreams/{DREAM_ID}/patch.diff", + "_dreams/_index.md", "_meta", "_meta/state.json", "_index.md", +]) +@pytest.mark.parametrize("dangling", [False, True]) +def test_symlink_escapes_are_rejected_before_other_outputs( + dream_reconcile, tmp_path, make_symlink, target, dangling, +): + repo = tmp_path / "repo" + shadow = repo / ".shadow" + shadow.mkdir(parents=True) + destination = tmp_path / "outside" + is_file = target.endswith((".md", ".json", ".diff")) + if not dangling: + if is_file: + destination.write_text("keep outside\n", encoding="utf-8") + else: + destination.mkdir() + (destination / "sentinel").write_text("keep outside\n", encoding="utf-8") + link = shadow / target + link.parent.mkdir(parents=True, exist_ok=True) + make_symlink(link, destination, target_is_directory=not is_file) + before = tree_bytes(tmp_path) + with pytest.raises(dream_reconcile.UnsafeShadowPath): + dream_reconcile.merge_discoveries( + str(repo), batch("src/target.py::run", refs=["src/target.py::run"]), + ) + assert tree_bytes(tmp_path) == before + assert link.is_symlink() + assert destination.exists() is not dangling + + +def test_outward_shadow_root_symlink_is_not_a_trusted_root( + dream_reconcile, tmp_path, make_symlink, +): + repo = tmp_path / "repo" + repo.mkdir() + outside = tmp_path / "outside" + outside.mkdir() + make_symlink(repo / ".shadow", outside, target_is_directory=True) + with pytest.raises(dream_reconcile.UnsafeShadowPath): + dream_reconcile.merge_discoveries(str(repo), batch()) + assert list(outside.iterdir()) == [] + + +def test_direct_writers_require_contained_destinations(dream_reconcile, tmp_path): + repo = tmp_path / "repo" + repo.mkdir() + outside = tmp_path / "outside.md" + outside.write_text("# Original\n\n**Refs**:\n", encoding="utf-8") + with pytest.raises(dream_reconcile.UnsafeShadowPath): + dream_reconcile.merge_discovery_into_file( + str(outside), "run", {"text": "Must not write."}, DREAM_ID, repo_root=str(repo), + ) + with pytest.raises(dream_reconcile.UnsafeShadowPath): + dream_reconcile._merge_refs_into_cross_file( + str(outside), ["a.py::run"], repo_root=str(repo), + ) + with pytest.raises(dream_reconcile.UnsafeShadowPath): + dream_reconcile.add_cross_reference_backpointer( + str(repo), "../outside", "slug", "Title", DREAM_ID, + ) + assert outside.read_text(encoding="utf-8") == "# Original\n\n**Refs**:\n" + assert list(repo.iterdir()) == [] + + +def test_common_path_prefix_is_not_containment(dream_reconcile, tmp_path): + outside = tmp_path / ".shadow-other/file.md" + with pytest.raises(dream_reconcile.UnsafeShadowPath): + dream_reconcile.merge_discovery_into_file( + str(outside), "run", {"text": "Must not write."}, DREAM_ID, + repo_root=str(tmp_path), + ) + assert not outside.parent.exists() + + +def test_direct_cross_merge_validates_new_ref_before_mutation(dream_reconcile, tmp_path): + cross = tmp_path / ".shadow/_cross/shared.md" + cross.parent.mkdir(parents=True) + cross.write_text("# Shared\n\n**Refs**:\n- `a.py::run`\n", encoding="utf-8") + before = cross.read_bytes() + with pytest.raises(dream_reconcile.UnsafeShadowPath): + dream_reconcile._merge_refs_into_cross_file( + str(cross), ["b.py::run", "../outside::run"], repo_root=str(tmp_path), + ) + assert cross.read_bytes() == before + + +def test_valid_unicode_spaces_and_internal_directory_symlink( + dream_reconcile, tmp_path, make_symlink, +): + shadow = tmp_path / ".shadow" + (shadow / "real").mkdir(parents=True) + make_symlink(shadow / "alias", shadow / "real", target_is_directory=True) + anchor = "alias/caf\u00e9 tools..py::run" + entries = batch(anchor, refs=[anchor, "lib/other.py::call"]) + original = deepcopy(entries) + merged, skipped = dream_reconcile.merge_discoveries(str(tmp_path), entries) + assert (merged, skipped) == (2, 0) + assert entries == original + path = shadow / "real/caf\u00e9 tools..py.md" + assert "## `run`" in path.read_text(encoding="utf-8") + assert "../_cross/shared-behavior.md" in path.read_text(encoding="utf-8") + assert anchor in (shadow / "_cross/shared-behavior.md").read_text(encoding="utf-8") + + +@pytest.mark.parametrize("writer", ["mirror_reports", "update_index", "update_state", "rebuild_top_index"]) +def test_other_writers_refuse_symlinked_outputs(dream_reconcile, tmp_path, make_symlink, writer): + repo = tmp_path / "repo" + shadow = repo / ".shadow" + shadow.mkdir(parents=True) + outside = tmp_path / "outside" + outside.mkdir() + target = { + "mirror_reports": "_dreams", "update_index": "_dreams", + "update_state": "_meta", "rebuild_top_index": "_index.md", + }[writer] + if writer == "rebuild_top_index": + outside = outside / "index.md" + outside.write_text("keep\n", encoding="utf-8") + make_symlink(shadow / target, outside, target_is_directory=writer != "rebuild_top_index") + before = tree_bytes(tmp_path) + args = [str(repo)] if writer == "rebuild_top_index" else [str(repo), batch()] + with pytest.raises(dream_reconcile.UnsafeShadowPath): + getattr(dream_reconcile, writer)(*args) + assert tree_bytes(tmp_path) == before From 5c6f2c9824a6ed2a32c1296d00d060298bdea178 Mon Sep 17 00:00:00 2001 From: Chinmay Singh Date: Wed, 23 Sep 2026 17:01:45 -0700 Subject: [PATCH 2/2] Reject Windows device paths during reconciliation Prevent reserved Windows device basenames from being treated as contained shadow files, and extend path-validation coverage across anchors, refs, slugs, and dream IDs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9876beba-3398-40e7-a556-b76e7aed9327 --- skills/shadow-frog-dream/dream-reconcile.py | 14 ++++++++++++++ .../test_dream_reconcile_paths.py | 7 ++++++- 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/skills/shadow-frog-dream/dream-reconcile.py b/skills/shadow-frog-dream/dream-reconcile.py index 1f719de..62253d3 100755 --- a/skills/shadow-frog-dream/dream-reconcile.py +++ b/skills/shadow-frog-dream/dream-reconcile.py @@ -155,12 +155,26 @@ class UnsafeShadowPath(ValueError): """Untrusted metadata or a filesystem alias escapes the shadow output tree.""" +_WINDOWS_RESERVED_NAMES = { + 'CON', 'PRN', 'AUX', 'NUL', 'CONIN$', 'CONOUT$', + *(f'COM{suffix}' for suffix in '123456789¹²³'), + *(f'LPT{suffix}' for suffix in '123456789¹²³'), +} + + +def _is_windows_reserved_part(part): + """Return whether one path component names a Windows device.""" + basename = part.rstrip(' .').split('.', 1)[0].upper() + return basename in _WINDOWS_RESERVED_NAMES + + def _relative_parts(value, field, *, single=False): if not isinstance(value, str) or not value: raise UnsafeShadowPath(f"{field}: expected a nonempty relative path, got {value!r}") parts = value.split('/') if ( any(not part.rstrip(' .') for part in parts) + or any(_is_windows_reserved_part(part) for part in parts) or any(char in value for char in ('\\', ':', '\0', '\r', '\n')) or PureWindowsPath(value).drive or (single and len(parts) != 1) diff --git a/tests/skills/shadow_frog_dream/test_dream_reconcile_paths.py b/tests/skills/shadow_frog_dream/test_dream_reconcile_paths.py index b9e5b64..e48533d 100644 --- a/tests/skills/shadow_frog_dream/test_dream_reconcile_paths.py +++ b/tests/skills/shadow_frog_dream/test_dream_reconcile_paths.py @@ -82,6 +82,8 @@ def test_cli_rejects_unsafe_manifest_before_any_shadow_writes( r"\\server\share\file", r"src\..\outside", "src/file:stream", "", ".", "..", "src/./file", "src//file", "src/file\0name", "src/.. /outside", "src/.../outside", "src/\nfile", "src/\rfile", + "NUL", "src/con.py", "aux.txt", "COM1", "lib/lpt9.log", + "COM¹", "LPT³.txt", "CONIN$", "CONOUT$.log", ]) @pytest.mark.parametrize("cross_ref", [False, True]) @pytest.mark.parametrize("dry_run", [False, True]) @@ -104,7 +106,10 @@ def test_manifest_paths_are_validated_before_writing_any_entry( @pytest.mark.parametrize("field", ["slug", "dream_id"]) -@pytest.mark.parametrize("value", ["../outside", "/outside", "C:/outside", r"..\outside", ".", ".."]) +@pytest.mark.parametrize("value", [ + "../outside", "/outside", "C:/outside", r"..\outside", ".", "..", + "NUL", "con.txt", "COM1", "lpt³.md", "CONIN$", +]) def test_artifact_names_cannot_escape_their_subdirectory(dream_reconcile, tmp_path, field, value): entries = batch(refs=["a.py::run"]) if field == "slug":