Skip to content

Commit e3a0481

Browse files
hotcodemachaAshutosh Gupta
hotcodemacha
and
Ashutosh Gupta
authored
Fix ignored path handling in hashing configuration (#506)
* Fix ignored path handling in hashing configuration Signed-off-by: Ashutosh Gupta <[email protected]> * Normalize CLI ignore paths Signed-off-by: Ashutosh Gupta <[email protected]> --------- Signed-off-by: Ashutosh Gupta <[email protected]> Co-authored-by: Ashutosh Gupta <[email protected]>
1 parent 1ae8e9a commit e3a0481

File tree

4 files changed

+94
-46
lines changed

4 files changed

+94
-46
lines changed

‎scripts/tests/verify_test.py‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ def test_verify_key_v0_3_1(self, base_path: pathlib.Path):
4141
public_key=public_key
4242
).set_hashing_config(
4343
model_signing.hashing.Config().set_ignored_paths(
44-
paths=list(ignore_paths) + [signature],
44+
paths=list(ignore_paths) + [signature.name],
4545
ignore_git_paths=ignore_git_paths,
4646
)
4747
).verify(model_path, signature)
@@ -61,7 +61,7 @@ def test_verify_certificate_v0_3_1(self, base_path: pathlib.Path):
6161
log_fingerprints=log_fingerprints,
6262
).set_hashing_config(
6363
model_signing.hashing.Config().set_ignored_paths(
64-
paths=list(ignore_paths) + [signature],
64+
paths=list(ignore_paths) + [signature.name],
6565
ignore_git_paths=ignore_git_paths,
6666
)
6767
).verify(model_path, signature)
@@ -83,7 +83,7 @@ def test_verify_sigstore_v0_3_1(self, base_path: pathlib.Path):
8383
use_staging=use_staging,
8484
).set_hashing_config(
8585
model_signing.hashing.Config().set_ignored_paths(
86-
paths=list(ignore_paths) + [signature],
86+
paths=list(ignore_paths) + [signature.name],
8787
ignore_git_paths=ignore_git_paths,
8888
)
8989
).verify(model_path, signature)
@@ -101,7 +101,7 @@ def test_verify_key_v1_0_0(self, base_path: pathlib.Path):
101101
public_key=public_key
102102
).set_hashing_config(
103103
model_signing.hashing.Config().set_ignored_paths(
104-
paths=list(ignore_paths) + [signature],
104+
paths=list(ignore_paths) + [signature.name],
105105
ignore_git_paths=ignore_git_paths,
106106
)
107107
).verify(model_path, signature)
@@ -121,7 +121,7 @@ def test_verify_certificate_v1_0_0(self, base_path: pathlib.Path):
121121
log_fingerprints=log_fingerprints,
122122
).set_hashing_config(
123123
model_signing.hashing.Config().set_ignored_paths(
124-
paths=list(ignore_paths) + [signature],
124+
paths=list(ignore_paths) + [signature.name],
125125
ignore_git_paths=ignore_git_paths,
126126
)
127127
).verify(model_path, signature)
@@ -143,7 +143,7 @@ def test_verify_sigstore_v1_0_0(self, base_path: pathlib.Path):
143143
use_staging=use_staging,
144144
).set_hashing_config(
145145
model_signing.hashing.Config().set_ignored_paths(
146-
paths=list(ignore_paths) + [signature],
146+
paths=list(ignore_paths) + [signature.name],
147147
ignore_git_paths=ignore_git_paths,
148148
)
149149
).verify(model_path, signature)

‎src/model_signing/_cli.py‎

Lines changed: 47 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,21 @@ def set_attribute(self, key, value):
149149
)
150150

151151

152+
def _resolve_ignore_paths(
153+
model_path: pathlib.Path, paths: Iterable[pathlib.Path]
154+
) -> list[pathlib.Path]:
155+
model_root = model_path.resolve()
156+
cwd = pathlib.Path.cwd()
157+
resolved_paths = []
158+
for p in paths:
159+
candidate = (p if p.is_absolute() else (cwd / p)).resolve()
160+
try:
161+
resolved_paths.append(candidate.relative_to(model_root))
162+
except ValueError:
163+
continue
164+
return resolved_paths
165+
166+
152167
class _PKICmdGroup(click.Group):
153168
"""A custom group to configure the supported PKI methods."""
154169

@@ -336,6 +351,9 @@ def _sign_sigstore(
336351
)
337352
span.set_attribute("sigstore.use_staging", use_staging)
338353
try:
354+
ignored = _resolve_ignore_paths(
355+
model_path, list(ignore_paths) + [signature]
356+
)
339357
model_signing.signing.Config().use_sigstore_signer(
340358
use_ambient_credentials=use_ambient_credentials,
341359
use_staging=use_staging,
@@ -346,8 +364,7 @@ def _sign_sigstore(
346364
).set_hashing_config(
347365
model_signing.hashing.Config()
348366
.set_ignored_paths(
349-
paths=list(ignore_paths) + [signature],
350-
ignore_git_paths=ignore_git_paths,
367+
paths=ignored, ignore_git_paths=ignore_git_paths
351368
)
352369
.set_allow_symlinks(allow_symlinks)
353370
).sign(model_path, signature)
@@ -394,14 +411,14 @@ def _sign_private_key(
394411
management protocols.
395412
"""
396413
try:
414+
ignored = _resolve_ignore_paths(
415+
model_path, list(ignore_paths) + [signature]
416+
)
397417
model_signing.signing.Config().use_elliptic_key_signer(
398418
private_key=private_key, password=password
399419
).set_hashing_config(
400420
model_signing.hashing.Config()
401-
.set_ignored_paths(
402-
paths=list(ignore_paths) + [signature],
403-
ignore_git_paths=ignore_git_paths,
404-
)
421+
.set_ignored_paths(paths=ignored, ignore_git_paths=ignore_git_paths)
405422
.set_allow_symlinks(allow_symlinks)
406423
).sign(model_path, signature)
407424
except Exception as err:
@@ -440,14 +457,14 @@ def _sign_pkcs11_key(
440457
management protocols.
441458
"""
442459
try:
460+
ignored = _resolve_ignore_paths(
461+
model_path, list(ignore_paths) + [signature]
462+
)
443463
model_signing.signing.Config().use_pkcs11_signer(
444464
pkcs11_uri=pkcs11_uri
445465
).set_hashing_config(
446466
model_signing.hashing.Config()
447-
.set_ignored_paths(
448-
paths=list(ignore_paths) + [signature],
449-
ignore_git_paths=ignore_git_paths,
450-
)
467+
.set_ignored_paths(paths=ignored, ignore_git_paths=ignore_git_paths)
451468
.set_allow_symlinks(allow_symlinks)
452469
).sign(model_path, signature)
453470
except Exception as err:
@@ -493,16 +510,16 @@ def _sign_certificate(
493510
Note that we don't offer certificate and key management protocols.
494511
"""
495512
try:
513+
ignored = _resolve_ignore_paths(
514+
model_path, list(ignore_paths) + [signature]
515+
)
496516
model_signing.signing.Config().use_certificate_signer(
497517
private_key=private_key,
498518
signing_certificate=signing_certificate,
499519
certificate_chain=certificate_chain,
500520
).set_hashing_config(
501521
model_signing.hashing.Config()
502-
.set_ignored_paths(
503-
paths=list(ignore_paths) + [signature],
504-
ignore_git_paths=ignore_git_paths,
505-
)
522+
.set_ignored_paths(paths=ignored, ignore_git_paths=ignore_git_paths)
506523
.set_allow_symlinks(allow_symlinks)
507524
).sign(model_path, signature)
508525
except Exception as err:
@@ -549,16 +566,16 @@ def _sign_pkcs11_certificate(
549566
Note that we don't offer certificate and key management protocols.
550567
"""
551568
try:
569+
ignored = _resolve_ignore_paths(
570+
model_path, list(ignore_paths) + [signature]
571+
)
552572
model_signing.signing.Config().use_pkcs11_certificate_signer(
553573
pkcs11_uri=pkcs11_uri,
554574
signing_certificate=signing_certificate,
555575
certificate_chain=certificate_chain,
556576
).set_hashing_config(
557577
model_signing.hashing.Config()
558-
.set_ignored_paths(
559-
paths=list(ignore_paths) + [signature],
560-
ignore_git_paths=ignore_git_paths,
561-
)
578+
.set_ignored_paths(paths=ignored, ignore_git_paths=ignore_git_paths)
562579
.set_allow_symlinks(allow_symlinks)
563580
).sign(model_path, signature)
564581
except Exception as err:
@@ -636,15 +653,17 @@ def _verify_sigstore(
636653
span.set_attribute("sigstore.oidc_issuer", identity_provider)
637654
span.set_attribute("sigstore.use_staging", use_staging)
638655
try:
656+
ignored = _resolve_ignore_paths(
657+
model_path, list(ignore_paths) + [signature]
658+
)
639659
model_signing.verifying.Config().use_sigstore_verifier(
640660
identity=identity,
641661
oidc_issuer=identity_provider,
642662
use_staging=use_staging,
643663
).set_hashing_config(
644664
model_signing.hashing.Config()
645665
.set_ignored_paths(
646-
paths=list(ignore_paths) + [signature],
647-
ignore_git_paths=ignore_git_paths,
666+
paths=ignored, ignore_git_paths=ignore_git_paths
648667
)
649668
.set_allow_symlinks(allow_symlinks)
650669
).set_ignore_unsigned_files(ignore_unsigned_files).verify(
@@ -694,14 +713,14 @@ def _verify_private_key(
694713
management protocols.
695714
"""
696715
try:
716+
ignored = _resolve_ignore_paths(
717+
model_path, list(ignore_paths) + [signature]
718+
)
697719
model_signing.verifying.Config().use_elliptic_key_verifier(
698720
public_key=public_key
699721
).set_hashing_config(
700722
model_signing.hashing.Config()
701-
.set_ignored_paths(
702-
paths=list(ignore_paths) + [signature],
703-
ignore_git_paths=ignore_git_paths,
704-
)
723+
.set_ignored_paths(paths=ignored, ignore_git_paths=ignore_git_paths)
705724
.set_allow_symlinks(allow_symlinks)
706725
).set_ignore_unsigned_files(ignore_unsigned_files).verify(
707726
model_path, signature
@@ -756,15 +775,15 @@ def _verify_certificate(
756775
logging.basicConfig(format="%(message)s", level=logging.INFO)
757776

758777
try:
778+
ignored = _resolve_ignore_paths(
779+
model_path, list(ignore_paths) + [signature]
780+
)
759781
model_signing.verifying.Config().use_certificate_verifier(
760782
certificate_chain=certificate_chain,
761783
log_fingerprints=log_fingerprints,
762784
).set_hashing_config(
763785
model_signing.hashing.Config()
764-
.set_ignored_paths(
765-
paths=list(ignore_paths) + [signature],
766-
ignore_git_paths=ignore_git_paths,
767-
)
786+
.set_ignored_paths(paths=ignored, ignore_git_paths=ignore_git_paths)
768787
.set_allow_symlinks(allow_symlinks)
769788
).set_ignore_unsigned_files(ignore_unsigned_files).verify(
770789
model_path, signature

‎src/model_signing/hashing.py‎

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -148,18 +148,23 @@ def hash(
148148
files_to_hash: Optional[Iterable[PathLike]] = None,
149149
) -> manifest.Manifest:
150150
"""Hashes a model using the current configuration."""
151-
# All paths in ignored_paths must have model_path as prefix
151+
# All paths in ``_ignored_paths`` are expected to be relative to the
152+
# model directory. Join them to ``model_path`` and ensure they do not
153+
# escape it.
154+
model_path = pathlib.Path(model_path)
152155
ignored_paths = []
153156
for p in self._ignored_paths:
154-
rp = os.path.relpath(p, model_path)
155-
# rp may start with "../" if it is not relative to model_path
156-
if not rp.startswith("../"):
157-
ignored_paths.append(pathlib.Path(os.path.join(model_path, rp)))
157+
full = model_path / p
158+
try:
159+
full.relative_to(model_path)
160+
except ValueError:
161+
continue
162+
ignored_paths.append(full)
158163

159164
if self._ignore_git_paths:
160165
ignored_paths.extend(
161166
[
162-
os.path.join(model_path, p)
167+
model_path / p
163168
for p in [
164169
".git/",
165170
".gitattributes",
@@ -357,11 +362,9 @@ def set_ignored_paths(
357362
Returns:
358363
The new hashing configuration with a new set of ignored paths.
359364
"""
360-
# Use relpath to possibly fix weird paths like '../a/b' -> 'b'
361-
# when '../a/' is a no-op
362-
self._ignored_paths = frozenset(
363-
{pathlib.Path(p).resolve() for p in paths}
364-
)
365+
# Preserve the user-provided relative paths; they are resolved against
366+
# the model directory later when hashing.
367+
self._ignored_paths = frozenset(pathlib.Path(p) for p in paths)
365368
self._ignore_git_paths = ignore_git_paths
366369
return self
367370

@@ -376,7 +379,15 @@ def add_ignored_paths(
376379
the model directory.
377380
"""
378381
newset = set(self._ignored_paths)
379-
newset.update([os.path.join(model_path, p) for p in paths])
382+
model_path = pathlib.Path(model_path)
383+
for p in paths:
384+
candidate = pathlib.Path(p)
385+
full = model_path / candidate
386+
try:
387+
full.relative_to(model_path)
388+
except ValueError:
389+
continue
390+
newset.add(candidate)
380391
self._ignored_paths = newset
381392

382393
def set_allow_symlinks(self, allow_symlinks: bool) -> Self:

‎tests/hashing_config_test.py‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
from model_signing import hashing
2+
3+
4+
def test_set_ignored_paths_relative_to_model(tmp_path, monkeypatch):
5+
model = tmp_path / "model"
6+
model.mkdir()
7+
(model / "ignored.txt").write_text("skip")
8+
(model / "keep.txt").write_text("keep")
9+
10+
cfg = hashing.Config().set_ignored_paths(paths=["ignored.txt"])
11+
# Change working directory to ensure ignored paths aren't resolved against
12+
# the current directory used at hashing time.
13+
monkeypatch.chdir(tmp_path)
14+
15+
manifest = cfg.hash(model)
16+
identifiers = {rd.identifier for rd in manifest.resource_descriptors()}
17+
assert "ignored.txt" not in identifiers
18+
assert "keep.txt" in identifiers

0 commit comments

Comments
 (0)