diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 5a2ae184..69e6eca8 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -97,7 +97,7 @@ jobs: BUILD_TYPE=minimal - id: docker_meta_minimal - uses: docker/metadata-action@902fa8ec7d6ecbf8d84d538b9b233a880e428804 # v5.7.0 + uses: docker/metadata-action@c1e51972afc2121e065aed6d45c65596fe445f3f # v5.8.0 with: images: ${{ steps.build_minimal_image.outputs.image }} tags: type=sha,format=long,type=ref,event=branch @@ -131,7 +131,7 @@ jobs: BUILD_TYPE=full - id: docker_meta_full - uses: docker/metadata-action@902fa8ec7d6ecbf8d84d538b9b233a880e428804 # v5.7.0 + uses: docker/metadata-action@c1e51972afc2121e065aed6d45c65596fe445f3f # v5.8.0 with: images: ${{ steps.build_full_image.outputs.image }} tags: type=sha,format=long,type=ref,event=branch diff --git a/scripts/tests/verify_test.py b/scripts/tests/verify_test.py index d6914f16..d6eee62f 100644 --- a/scripts/tests/verify_test.py +++ b/scripts/tests/verify_test.py @@ -41,7 +41,7 @@ def test_verify_key_v0_3_1(self, base_path: pathlib.Path): public_key=public_key ).set_hashing_config( model_signing.hashing.Config().set_ignored_paths( - paths=list(ignore_paths) + [signature], + paths=list(ignore_paths) + [signature.name], ignore_git_paths=ignore_git_paths, ) ).verify(model_path, signature) @@ -61,7 +61,7 @@ def test_verify_certificate_v0_3_1(self, base_path: pathlib.Path): log_fingerprints=log_fingerprints, ).set_hashing_config( model_signing.hashing.Config().set_ignored_paths( - paths=list(ignore_paths) + [signature], + paths=list(ignore_paths) + [signature.name], ignore_git_paths=ignore_git_paths, ) ).verify(model_path, signature) @@ -83,7 +83,7 @@ def test_verify_sigstore_v0_3_1(self, base_path: pathlib.Path): use_staging=use_staging, ).set_hashing_config( model_signing.hashing.Config().set_ignored_paths( - paths=list(ignore_paths) + [signature], + paths=list(ignore_paths) + [signature.name], ignore_git_paths=ignore_git_paths, ) ).verify(model_path, signature) @@ -101,7 +101,7 @@ def test_verify_key_v1_0_0(self, base_path: pathlib.Path): public_key=public_key ).set_hashing_config( model_signing.hashing.Config().set_ignored_paths( - paths=list(ignore_paths) + [signature], + paths=list(ignore_paths) + [signature.name], ignore_git_paths=ignore_git_paths, ) ).verify(model_path, signature) @@ -121,7 +121,7 @@ def test_verify_certificate_v1_0_0(self, base_path: pathlib.Path): log_fingerprints=log_fingerprints, ).set_hashing_config( model_signing.hashing.Config().set_ignored_paths( - paths=list(ignore_paths) + [signature], + paths=list(ignore_paths) + [signature.name], ignore_git_paths=ignore_git_paths, ) ).verify(model_path, signature) @@ -143,7 +143,7 @@ def test_verify_sigstore_v1_0_0(self, base_path: pathlib.Path): use_staging=use_staging, ).set_hashing_config( model_signing.hashing.Config().set_ignored_paths( - paths=list(ignore_paths) + [signature], + paths=list(ignore_paths) + [signature.name], ignore_git_paths=ignore_git_paths, ) ).verify(model_path, signature) diff --git a/src/model_signing/_cli.py b/src/model_signing/_cli.py index 12df9027..1bf4530f 100644 --- a/src/model_signing/_cli.py +++ b/src/model_signing/_cli.py @@ -149,6 +149,21 @@ def set_attribute(self, key, value): ) +def _resolve_ignore_paths( + model_path: pathlib.Path, paths: Iterable[pathlib.Path] +) -> list[pathlib.Path]: + model_root = model_path.resolve() + cwd = pathlib.Path.cwd() + resolved_paths = [] + for p in paths: + candidate = (p if p.is_absolute() else (cwd / p)).resolve() + try: + resolved_paths.append(candidate.relative_to(model_root)) + except ValueError: + continue + return resolved_paths + + class _PKICmdGroup(click.Group): """A custom group to configure the supported PKI methods.""" @@ -336,6 +351,9 @@ def _sign_sigstore( ) span.set_attribute("sigstore.use_staging", use_staging) try: + ignored = _resolve_ignore_paths( + model_path, list(ignore_paths) + [signature] + ) model_signing.signing.Config().use_sigstore_signer( use_ambient_credentials=use_ambient_credentials, use_staging=use_staging, @@ -346,8 +364,7 @@ def _sign_sigstore( ).set_hashing_config( model_signing.hashing.Config() .set_ignored_paths( - paths=list(ignore_paths) + [signature], - ignore_git_paths=ignore_git_paths, + paths=ignored, ignore_git_paths=ignore_git_paths ) .set_allow_symlinks(allow_symlinks) ).sign(model_path, signature) @@ -394,14 +411,14 @@ def _sign_private_key( management protocols. """ try: + ignored = _resolve_ignore_paths( + model_path, list(ignore_paths) + [signature] + ) model_signing.signing.Config().use_elliptic_key_signer( private_key=private_key, password=password ).set_hashing_config( model_signing.hashing.Config() - .set_ignored_paths( - paths=list(ignore_paths) + [signature], - ignore_git_paths=ignore_git_paths, - ) + .set_ignored_paths(paths=ignored, ignore_git_paths=ignore_git_paths) .set_allow_symlinks(allow_symlinks) ).sign(model_path, signature) except Exception as err: @@ -440,14 +457,14 @@ def _sign_pkcs11_key( management protocols. """ try: + ignored = _resolve_ignore_paths( + model_path, list(ignore_paths) + [signature] + ) model_signing.signing.Config().use_pkcs11_signer( pkcs11_uri=pkcs11_uri ).set_hashing_config( model_signing.hashing.Config() - .set_ignored_paths( - paths=list(ignore_paths) + [signature], - ignore_git_paths=ignore_git_paths, - ) + .set_ignored_paths(paths=ignored, ignore_git_paths=ignore_git_paths) .set_allow_symlinks(allow_symlinks) ).sign(model_path, signature) except Exception as err: @@ -493,16 +510,16 @@ def _sign_certificate( Note that we don't offer certificate and key management protocols. """ try: + ignored = _resolve_ignore_paths( + model_path, list(ignore_paths) + [signature] + ) model_signing.signing.Config().use_certificate_signer( private_key=private_key, signing_certificate=signing_certificate, certificate_chain=certificate_chain, ).set_hashing_config( model_signing.hashing.Config() - .set_ignored_paths( - paths=list(ignore_paths) + [signature], - ignore_git_paths=ignore_git_paths, - ) + .set_ignored_paths(paths=ignored, ignore_git_paths=ignore_git_paths) .set_allow_symlinks(allow_symlinks) ).sign(model_path, signature) except Exception as err: @@ -549,16 +566,16 @@ def _sign_pkcs11_certificate( Note that we don't offer certificate and key management protocols. """ try: + ignored = _resolve_ignore_paths( + model_path, list(ignore_paths) + [signature] + ) model_signing.signing.Config().use_pkcs11_certificate_signer( pkcs11_uri=pkcs11_uri, signing_certificate=signing_certificate, certificate_chain=certificate_chain, ).set_hashing_config( model_signing.hashing.Config() - .set_ignored_paths( - paths=list(ignore_paths) + [signature], - ignore_git_paths=ignore_git_paths, - ) + .set_ignored_paths(paths=ignored, ignore_git_paths=ignore_git_paths) .set_allow_symlinks(allow_symlinks) ).sign(model_path, signature) except Exception as err: @@ -636,6 +653,9 @@ def _verify_sigstore( span.set_attribute("sigstore.oidc_issuer", identity_provider) span.set_attribute("sigstore.use_staging", use_staging) try: + ignored = _resolve_ignore_paths( + model_path, list(ignore_paths) + [signature] + ) model_signing.verifying.Config().use_sigstore_verifier( identity=identity, oidc_issuer=identity_provider, @@ -643,8 +663,7 @@ def _verify_sigstore( ).set_hashing_config( model_signing.hashing.Config() .set_ignored_paths( - paths=list(ignore_paths) + [signature], - ignore_git_paths=ignore_git_paths, + paths=ignored, ignore_git_paths=ignore_git_paths ) .set_allow_symlinks(allow_symlinks) ).set_ignore_unsigned_files(ignore_unsigned_files).verify( @@ -694,14 +713,14 @@ def _verify_private_key( management protocols. """ try: + ignored = _resolve_ignore_paths( + model_path, list(ignore_paths) + [signature] + ) model_signing.verifying.Config().use_elliptic_key_verifier( public_key=public_key ).set_hashing_config( model_signing.hashing.Config() - .set_ignored_paths( - paths=list(ignore_paths) + [signature], - ignore_git_paths=ignore_git_paths, - ) + .set_ignored_paths(paths=ignored, ignore_git_paths=ignore_git_paths) .set_allow_symlinks(allow_symlinks) ).set_ignore_unsigned_files(ignore_unsigned_files).verify( model_path, signature @@ -756,15 +775,15 @@ def _verify_certificate( logging.basicConfig(format="%(message)s", level=logging.INFO) try: + ignored = _resolve_ignore_paths( + model_path, list(ignore_paths) + [signature] + ) model_signing.verifying.Config().use_certificate_verifier( certificate_chain=certificate_chain, log_fingerprints=log_fingerprints, ).set_hashing_config( model_signing.hashing.Config() - .set_ignored_paths( - paths=list(ignore_paths) + [signature], - ignore_git_paths=ignore_git_paths, - ) + .set_ignored_paths(paths=ignored, ignore_git_paths=ignore_git_paths) .set_allow_symlinks(allow_symlinks) ).set_ignore_unsigned_files(ignore_unsigned_files).verify( model_path, signature diff --git a/src/model_signing/hashing.py b/src/model_signing/hashing.py index dd71df4b..db227ab4 100644 --- a/src/model_signing/hashing.py +++ b/src/model_signing/hashing.py @@ -148,18 +148,23 @@ def hash( files_to_hash: Optional[Iterable[PathLike]] = None, ) -> manifest.Manifest: """Hashes a model using the current configuration.""" - # All paths in ignored_paths must have model_path as prefix + # All paths in ``_ignored_paths`` are expected to be relative to the + # model directory. Join them to ``model_path`` and ensure they do not + # escape it. + model_path = pathlib.Path(model_path) ignored_paths = [] for p in self._ignored_paths: - rp = os.path.relpath(p, model_path) - # rp may start with "../" if it is not relative to model_path - if not rp.startswith("../"): - ignored_paths.append(pathlib.Path(os.path.join(model_path, rp))) + full = model_path / p + try: + full.relative_to(model_path) + except ValueError: + continue + ignored_paths.append(full) if self._ignore_git_paths: ignored_paths.extend( [ - os.path.join(model_path, p) + model_path / p for p in [ ".git/", ".gitattributes", @@ -357,11 +362,9 @@ def set_ignored_paths( Returns: The new hashing configuration with a new set of ignored paths. """ - # Use relpath to possibly fix weird paths like '../a/b' -> 'b' - # when '../a/' is a no-op - self._ignored_paths = frozenset( - {pathlib.Path(p).resolve() for p in paths} - ) + # Preserve the user-provided relative paths; they are resolved against + # the model directory later when hashing. + self._ignored_paths = frozenset(pathlib.Path(p) for p in paths) self._ignore_git_paths = ignore_git_paths return self @@ -376,7 +379,15 @@ def add_ignored_paths( the model directory. """ newset = set(self._ignored_paths) - newset.update([os.path.join(model_path, p) for p in paths]) + model_path = pathlib.Path(model_path) + for p in paths: + candidate = pathlib.Path(p) + full = model_path / candidate + try: + full.relative_to(model_path) + except ValueError: + continue + newset.add(candidate) self._ignored_paths = newset def set_allow_symlinks(self, allow_symlinks: bool) -> Self: diff --git a/tests/hashing_config_test.py b/tests/hashing_config_test.py new file mode 100644 index 00000000..a5af03d3 --- /dev/null +++ b/tests/hashing_config_test.py @@ -0,0 +1,18 @@ +from model_signing import hashing + + +def test_set_ignored_paths_relative_to_model(tmp_path, monkeypatch): + model = tmp_path / "model" + model.mkdir() + (model / "ignored.txt").write_text("skip") + (model / "keep.txt").write_text("keep") + + cfg = hashing.Config().set_ignored_paths(paths=["ignored.txt"]) + # Change working directory to ensure ignored paths aren't resolved against + # the current directory used at hashing time. + monkeypatch.chdir(tmp_path) + + manifest = cfg.hash(model) + identifiers = {rd.identifier for rd in manifest.resource_descriptors()} + assert "ignored.txt" not in identifiers + assert "keep.txt" in identifiers