diff --git a/README.md b/README.md index 4f5102a..f7191be 100644 --- a/README.md +++ b/README.md @@ -123,7 +123,7 @@ metadata: {} Use `load_manifest(path)` to read a manifest and `validate_artifact_files(manifest, root=...)` when a workflow needs to prove that path-based artifacts exist locally. URI-only artifacts are skipped by local path validation. -Artifact paths in manifest YAML are relative to the directory containing the manifest file; an absolute path, or one that climbs out of that directory with `..`, is rejected when the manifest is loaded. An artifact declares either a `path` or a `uri`, never both. `Artifact` uses Pydantic validation to infer generic fields such as `format` and `media_type` from file paths or URI suffixes when those fields are omitted. Explicit values always win. If an explicit value differs from generic inference, `artifact.inference_issues` reports a warning diagnostic without changing the artifact. +Artifact paths in manifest YAML are relative to the directory containing the manifest file; a path that climbs out of that directory with `..` is rejected when the manifest is loaded. An absolute path names a file on the host that wrote the manifest: the model carries it, so a build tool can record where an output landed, but the registry, materialization and packaging refuse to act on one. An artifact declares either a `path` or a `uri`, never both. `Artifact` uses Pydantic validation to infer generic fields such as `format` and `media_type` from file paths or URI suffixes when those fields are omitted. Explicit values always win. If an explicit value differs from generic inference, `artifact.inference_issues` reports a warning diagnostic without changing the artifact. Artifact `id` is optional. Use it when a tool or human needs a stable short handle for an artifact; otherwise artlink falls back to `name` and then location for display and diagnostics. A manifest rejects duplicate artifact locations instead of silently deduplicating them, because two entries with the same path or URI but different roles or metadata are usually an authoring mistake. diff --git a/artlink/artifact.py b/artlink/artifact.py index 71ab9bf..e8b0f5c 100644 --- a/artlink/artifact.py +++ b/artlink/artifact.py @@ -186,12 +186,13 @@ def _normalize_empty_path(cls, value: Any) -> Any: @field_validator("path") @classmethod - def _validate_path_stays_in_root(cls, value: Path | None) -> Path | None: - """An artifact path is read relative to its manifest; one that is - absolute or climbs out of that directory would make the manifest a - pointer to anywhere on the host.""" - if value is not None and (value.is_absolute() or ".." in value.parts): - raise ValueError(f"artifact path must be relative and stay inside its manifest root: {value.as_posix()}") + def _validate_path_does_not_climb(cls, value: Path | None) -> Path | None: + """A relative artifact path is read against its manifest root and may + not climb out of it. An absolute path names a file on this host; the + model carries it (build tools record where an output landed) but the + registry, materialization and packaging refuse to act on one.""" + if value is not None and ".." in value.parts: + raise ValueError(f"artifact path must not climb out of its manifest root: {value.as_posix()}") return value @field_validator("uri", "kind", "role", mode="before") diff --git a/artlink/packages.py b/artlink/packages.py index b7c3828..4689eb1 100644 --- a/artlink/packages.py +++ b/artlink/packages.py @@ -81,6 +81,8 @@ def build_package_archive( for artifact in packaged_manifest.artifacts: if artifact.path is None: continue + if artifact.path.is_absolute(): + raise PackageError(f"artifact path is absolute; a package carries paths relative to its manifest only: {artifact.path.as_posix()}") source = Path(artifact_root) / artifact.path if not source.exists(): raise PackageError(f"missing artifact path: {source}") diff --git a/artlink/registry.py b/artlink/registry.py index d3f348b..8097bbf 100644 --- a/artlink/registry.py +++ b/artlink/registry.py @@ -297,11 +297,13 @@ def artifact_file_path(self, entry: ArtifactRegistryEntry) -> Path: artifact_path = entry.artifact.path if artifact_path is None: raise RegistryError(f"artifact has no filesystem path: {entry.artifact.display_id}") + if artifact_path.is_absolute(): + raise RegistryError(f"artifact path is absolute; a registry resolves paths against the manifest root only: {artifact_path.as_posix()}") if entry.root is None: return artifact_path - # the path is lexically confined at construction; a symlinked directory - # under the root could still lead outside it, so the resolved path is - # checked against the resolved root + # the path cannot climb lexically; a symlinked directory under the + # root could still lead outside it, so the resolved path is checked + # against the resolved root root = entry.root.resolve(strict=False) resolved = (root / artifact_path).resolve(strict=False) if resolved != root and root not in resolved.parents: diff --git a/artlink/tests/test_artifact.py b/artlink/tests/test_artifact.py index 1a37869..0003617 100644 --- a/artlink/tests/test_artifact.py +++ b/artlink/tests/test_artifact.py @@ -43,11 +43,33 @@ def test_artifact_declares_exactly_one_location() -> None: assert Artifact(uri="https://example.invalid/a.txt", role="data").location == "https://example.invalid/a.txt" -def test_artifact_paths_stay_inside_the_manifest_root() -> None: +def test_artifact_paths_do_not_climb_out_of_the_manifest_root() -> None: """An installed manifest that could point at ../../etc would make every registry a pointer to anywhere on the host.""" - for bad in ("/etc/passwd", "../sibling/payload.txt", "a/../../b"): - with pytest.raises(ManifestError, match="stay inside its manifest root"): + for bad in ("../sibling/payload.txt", "a/../../b"): + with pytest.raises(ManifestError, match="must not climb"): Artifact(path=Path(bad), role="data") - with pytest.raises(ManifestError, match="stay inside its manifest root"): + with pytest.raises(ManifestError, match="must not climb"): Manifest(name="m", artifacts=({"path": "../x", "role": "data"},)) + + +def test_an_absolute_path_is_carried_but_never_acted_on(tmp_path: Path) -> None: + """Build tools record where an output landed on this host; the model keeps + that, and every place artlink would read, copy or pack the file refuses.""" + from artlink import ArtifactRegistry, PackageError, RegistryError, build_materialization_plan, build_package_archive, resolve_manifest + from artlink.materialize import MaterializationError + + payload = tmp_path / "payload.bit" + payload.write_bytes(b"x") + manifest = Manifest(name="built", version="1", artifacts=(Artifact(id="bit", path=payload, role="bitstream"),)) + assert manifest.artifacts[0].path == payload + + registry = ArtifactRegistry.from_manifests((manifest,), root=tmp_path) + with pytest.raises(RegistryError, match="absolute"): + registry.artifact_file_path(registry.find_artifacts(role="bitstream")[0]) + with pytest.raises(PackageError, match="absolute"): + build_package_archive(manifest, artifact_root=tmp_path, output_dir=tmp_path / "dist", package_type="hdl") + + resolution = resolve_manifest(Manifest(name="p", references=(Reference(kind="manifest", target="built"),)), registry) + with pytest.raises(MaterializationError, match="absolute"): + build_materialization_plan(resolution, registry, target_dir=tmp_path / "out")