From 252540bf3d52259ea25a6fc28fb1b45dd52b1fec Mon Sep 17 00:00:00 2001 From: Panos Vagenas Date: Mon, 13 Jul 2026 13:51:19 +0200 Subject: [PATCH] refuse symbolic links when packing pages and assets - Reject symlink page/asset files and directories so pack does not follow link targets into the archive - Centralize copy helpers with explicit symlink checks for files and trees - Document the policy on `pack` and cover file, directory, and nested-directory cases in tests Signed-off-by: Panos Vagenas --- doclang/_packaging.py | 40 ++++++++++++----------- doclang/packaging.py | 5 ++- tests/test_packaging.py | 72 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 98 insertions(+), 19 deletions(-) diff --git a/doclang/_packaging.py b/doclang/_packaging.py index 2ce23a9..0dbf10e 100644 --- a/doclang/_packaging.py +++ b/doclang/_packaging.py @@ -58,6 +58,11 @@ def _require_directory(path: Path, *, label: str) -> None: raise PackagingError(f"{label} not found or not a directory: {path}") +def _reject_symlink(path: Path, *, label: str) -> None: + if path.is_symlink(): + raise PackagingError(f"{label} must not be a symbolic link: {path}") + + def _validate_archive_relative_path(path: str, *, label: str) -> None: if not path or path.startswith("/") or "\\" in path: raise PackagingError(f"Invalid {label} path: {path!r}") @@ -66,12 +71,22 @@ def _validate_archive_relative_path(path: str, *, label: str) -> None: raise PackagingError(f"Invalid {label} path: {path!r}") -def _copy_tree_into(source: Path, destination: Path) -> None: +def _copy_file(source: Path, destination: Path, *, label: str) -> None: + _reject_symlink(source, label=label) + _require_file(source, label=label) + destination.parent.mkdir(parents=True, exist_ok=True) + shutil.copy2(source, destination) + + +def _copy_tree_into(source: Path, destination: Path, *, label: str) -> None: + _reject_symlink(source, label=label) + _require_directory(source, label=label) destination.mkdir(parents=True, exist_ok=True) for item in source.iterdir(): target = destination / item.name + _reject_symlink(item, label=label) if item.is_dir(): - shutil.copytree(item, target, dirs_exist_ok=True) + _copy_tree_into(item, target, label=label) else: shutil.copy2(item, target) @@ -88,24 +103,18 @@ def _place_pages(stage: Path, pages: PagesInput) -> None: if not isinstance(page_number, int) or page_number < 1: raise PackagingError(f"Page numbers must be positive integers, got {page_number!r}") source_path = Path(source) - _require_file(source_path, label="Page file") destination = pages_dir / f"{page_number}{source_path.suffix}" - destination.parent.mkdir(parents=True, exist_ok=True) - shutil.copy2(source_path, destination) + _copy_file(source_path, destination, label="Page file") return if isinstance(pages, str | Path): - source_dir = Path(pages) - _require_directory(source_dir, label="Pages directory") - _copy_tree_into(source_dir, pages_dir) + _copy_tree_into(Path(pages), pages_dir, label="Pages directory") return for index, source in enumerate(pages, start=1): source_path = Path(source) - _require_file(source_path, label="Page file") destination = pages_dir / f"{index}{source_path.suffix}" - destination.parent.mkdir(parents=True, exist_ok=True) - shutil.copy2(source_path, destination) + _copy_file(source_path, destination, label="Page file") def _place_assets(stage: Path, assets: AssetsInput) -> None: @@ -113,16 +122,11 @@ def _place_assets(stage: Path, assets: AssetsInput) -> None: if isinstance(assets, Mapping): for archive_path, source in assets.items(): _validate_archive_relative_path(archive_path, label="asset") - source_path = Path(source) - _require_file(source_path, label="Asset file") destination = assets_dir / archive_path - destination.parent.mkdir(parents=True, exist_ok=True) - shutil.copy2(source_path, destination) + _copy_file(Path(source), destination, label="Asset file") return - source_dir = Path(assets) - _require_directory(source_dir, label="Assets directory") - _copy_tree_into(source_dir, assets_dir) + _copy_tree_into(Path(assets), assets_dir, label="Assets directory") def _write_opc_metadata(stage: Path) -> None: diff --git a/doclang/packaging.py b/doclang/packaging.py index 4aebb8d..c6f73b6 100644 --- a/doclang/packaging.py +++ b/doclang/packaging.py @@ -34,9 +34,12 @@ def pack( ``assets`` may be a directory (copied into ``assets/``) or a mapping of archive-relative asset path to source file. + Symbolic links are refused for ``pages`` and ``assets`` (including entries + inside directories) so pack does not follow link targets into the archive. + Returns the resolved path to the created archive. - Raises :class:`PackagingError` on packaging failure. + Raises :class:`PackagingError` on packaging failure (including symbolic links). Raises :class:`~doclang.ValidationError` when ``validate=True`` and the document fails validation. """ diff --git a/tests/test_packaging.py b/tests/test_packaging.py index c5e107b..76d5a13 100644 --- a/tests/test_packaging.py +++ b/tests/test_packaging.py @@ -140,3 +140,75 @@ def test_pack_invalid_asset_path(tmp_path: Path) -> None: output=tmp_path / "bad-asset.dclx", assets={"../escape.svg": asset_source}, ) + + +def test_pack_rejects_symlink_asset_file(tmp_path: Path) -> None: + document = VALID_DIR / "ok_description_element_head.dclg" + secret = tmp_path / "secret.txt" + secret.write_text("sensitive", encoding="utf-8") + link = tmp_path / "chart.svg" + link.symlink_to(secret) + output = tmp_path / "symlink-asset.dclx" + + with pytest.raises(PackagingError, match="symbolic link"): + pack(document, output=output, assets={"chart.svg": link}) + + assert not output.exists() + + +def test_pack_rejects_symlink_in_assets_directory(tmp_path: Path) -> None: + document = VALID_DIR / "ok_description_element_head.dclg" + secret = tmp_path / "secret.txt" + secret.write_text("sensitive", encoding="utf-8") + assets_dir = tmp_path / "payload" + assets_dir.mkdir() + (assets_dir / "chart.svg").symlink_to(secret) + output = tmp_path / "symlink-assets-dir.dclx" + + with pytest.raises(PackagingError, match="symbolic link"): + pack(document, output=output, assets=assets_dir) + + assert not output.exists() + + +def test_pack_rejects_nested_symlink_directory_in_assets(tmp_path: Path) -> None: + document = VALID_DIR / "ok_description_element_head.dclg" + outside = tmp_path / "outside" + outside.mkdir() + (outside / "leak.png").write_bytes(b"png") + assets_dir = tmp_path / "payload" + assets_dir.mkdir() + (assets_dir / "img").symlink_to(outside) + output = tmp_path / "symlink-assets-subdir.dclx" + + with pytest.raises(PackagingError, match="symbolic link"): + pack(document, output=output, assets=assets_dir) + + assert not output.exists() + + +def test_pack_rejects_symlink_page_file(tmp_path: Path) -> None: + document = VALID_DIR / "ok_description_element_head.dclg" + secret = tmp_path / "secret.png" + secret.write_bytes(b"png") + link = tmp_path / "1.png" + link.symlink_to(secret) + output = tmp_path / "symlink-page.dclx" + + with pytest.raises(PackagingError, match="symbolic link"): + pack(document, output=output, pages=[link]) + + assert not output.exists() + + +def test_pack_rejects_symlink_pages_directory(tmp_path: Path) -> None: + document = VALID_DIR / "ok_description_element_head.dclg" + real_pages = ARCHIVE_DEMO / "pages" + pages_link = tmp_path / "pages" + pages_link.symlink_to(real_pages) + output = tmp_path / "symlink-pages-dir.dclx" + + with pytest.raises(PackagingError, match="symbolic link"): + pack(document, output=output, pages=pages_link) + + assert not output.exists()