diff --git a/packages/syft-job/src/syft_job/client.py b/packages/syft-job/src/syft_job/client.py index 436bef64413..ee0a36a0156 100644 --- a/packages/syft-job/src/syft_job/client.py +++ b/packages/syft-job/src/syft_job/client.py @@ -669,7 +669,7 @@ def _job_has_all_files(self, ref: JobRef) -> bool: submission_dir = self.manager.submission_dir(ref) declared = metadata.headers.get(SUBMISSION_HASH_HEADER) if declared: - return code_hash(submission_dir) == declared + return code_hash(submission_dir, as_sent=True) == declared return has_listed_files(submission_dir, metadata.files) def _request_has_all_files(self, ref: RequestRef) -> bool: @@ -681,7 +681,10 @@ def _request_has_all_files(self, ref: RequestRef) -> bool: request = self.manager.read_request(ref) except METADATA_ERRORS: return False # request.yaml is still arriving - return request_hash(self.manager.request_dir(ref)) == request.files_hash + return ( + request_hash(self.manager.request_dir(ref), as_sent=True) + == request.files_hash + ) def _lock_submission(self, ref: JobRef) -> None: """Make the job's inbox folder read-only for its submitter. diff --git a/packages/syft-job/src/syft_job/submission.py b/packages/syft-job/src/syft_job/submission.py index c682b726489..140cbcb09f8 100644 --- a/packages/syft-job/src/syft_job/submission.py +++ b/packages/syft-job/src/syft_job/submission.py @@ -87,24 +87,45 @@ def _is_skipped(relative: Path) -> bool: return any(part in SYNC_EXCLUDED_NAMES for part in relative.parts) +def _as_sent(path: Path) -> bytes: + """The bytes the data owner ends up with for ``path``. + + The submitter's syncer reads a file as text and the owner writes it back + as UTF-8, so line endings and the local encoding do not survive. + """ + try: + with open(path, "r", encoding="utf-8") as f: + return f.read().encode("utf-8") + except UnicodeDecodeError: + return path.read_bytes() + + + def _digest( submission_dir: Path, entries: Optional[frozenset[str]], skip: frozenset[str] = frozenset(), + as_sent: bool = False, ) -> str: digest = hashlib.sha256() - files = sorted( - path - for path in submission_dir.rglob("*") - if path.is_file() - and not _is_skipped(relative := path.relative_to(submission_dir)) - and (entries is None or relative.parts[0] in entries) - and relative.as_posix() not in skip - ) - for path in files: - digest.update(path.relative_to(submission_dir).as_posix().encode()) + file_entries = [] + for path in submission_dir.rglob("*"): + if not path.is_file(): + continue + relative = path.relative_to(submission_dir) + if _is_skipped(relative): + continue + if entries is not None and relative.parts[0] not in entries: + continue + rel_posix = relative.as_posix() + if rel_posix not in skip: + file_entries.append((rel_posix, path)) + file_entries.sort(key=lambda item: item[0]) + for rel_posix, path in file_entries: + content = _as_sent(path) if as_sent else path.read_bytes() + digest.update(rel_posix.encode()) digest.update(b"\0") - digest.update(hashlib.sha256(path.read_bytes()).digest()) + digest.update(hashlib.sha256(content).digest()) return digest.hexdigest() @@ -247,3 +268,5 @@ def lock_for_submitter(folder: Path, ds_email: str) -> None: """ ruleset = RuleSet(rules=[Rule(pattern="**", access=Access(read=[ds_email]))]) ruleset.save(folder / PERMISSION_FILE_NAME) + + diff --git a/packages/syft-job/tests/test_submission_hashing.py b/packages/syft-job/tests/test_submission_hashing.py new file mode 100644 index 00000000000..0c56c046119 --- /dev/null +++ b/packages/syft-job/tests/test_submission_hashing.py @@ -0,0 +1,249 @@ +# -*- coding: utf-8 -*- +"""Regression tests for syft-job issue #9551: + +- code_hash(as_sent=True) normalizes CRLF and LF identically. +- request_hash(as_sent=True) normalizes CRLF and LF identically. +- _digest uses cross-platform deterministic sorting by relative posix paths. +- _as_sent correctly reads UTF-8 files with non-ASCII characters. +- _job_has_all_files and _request_has_all_files accept CRLF files when declared with as_sent=True. +""" + +import hashlib +from datetime import datetime, timezone +from pathlib import Path +from unittest.mock import MagicMock + +import pytest + +from syft_job.client import JobClient +from syft_job.models import JobSubmissionMetadata, RequestMetadata +from syft_job.submission import ( + SUBMISSION_HASH_HEADER, + _as_sent, + code_hash, + request_hash, + submission_hash, +) + + +def test_code_hash_crlf_lf_equivalence(tmp_path: Path): + crlf_dir = tmp_path / "crlf" + lf_dir = tmp_path / "lf" + for d in (crlf_dir, lf_dir): + (d / "code").mkdir(parents=True) + + (crlf_dir / "run.sh").write_bytes(b"#!/bin/sh\r\npython code/main.py\r\n") + (crlf_dir / "code" / "main.py").write_bytes(b"print('hello')\r\n") + + (lf_dir / "run.sh").write_bytes(b"#!/bin/sh\npython code/main.py\n") + (lf_dir / "code" / "main.py").write_bytes(b"print('hello')\n") + + # Both must produce the exact same declared hash + assert code_hash(crlf_dir, as_sent=True) == code_hash(lf_dir, as_sent=True) + # The declared hash must equal the LF raw hash + assert code_hash(crlf_dir, as_sent=True) == code_hash(lf_dir, as_sent=False) + + +def test_request_hash_crlf_lf_equivalence(tmp_path: Path): + crlf_dir = tmp_path / "crlf" + lf_dir = tmp_path / "lf" + crlf_dir.mkdir() + lf_dir.mkdir() + + (crlf_dir / "request.yaml").write_bytes(b"api: test\r\n") + (crlf_dir / "params.json").write_bytes(b'{"key": "value"}\r\n') + + (lf_dir / "request.yaml").write_bytes(b"api: test\n") + (lf_dir / "params.json").write_bytes(b'{"key": "value"}\n') + + assert request_hash(crlf_dir, as_sent=True) == request_hash(lf_dir, as_sent=True) + assert request_hash(crlf_dir, as_sent=True) == request_hash(lf_dir, as_sent=False) + + +def test_digest_deterministic_file_ordering( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +): + sub_dir = tmp_path / "sub" + (sub_dir / "code").mkdir(parents=True) + (sub_dir / "run.sh").write_bytes(b"echo 1\n") + # Mixed-case filenames: 'B.py' (ASCII 66) vs 'a.py' (ASCII 97) + (sub_dir / "code" / "B.py").write_bytes(b"b\n") + (sub_dir / "code" / "a.py").write_bytes(b"a\n") + + orig_rglob = Path.rglob + + # Traversal order 1: ascending + monkeypatch.setattr( + Path, + "rglob", + lambda self, *args, **kwargs: sorted(orig_rglob(self, *args, **kwargs)), + ) + hash_forward = submission_hash(sub_dir) + + # Traversal order 2: descending / reversed + monkeypatch.setattr( + Path, + "rglob", + lambda self, *args, **kwargs: sorted( + orig_rglob(self, *args, **kwargs), reverse=True + ), + ) + hash_reversed = submission_hash(sub_dir) + + # Verify digest is identical regardless of filesystem discovery order + assert hash_forward == hash_reversed + + # In canonical POSIX code-point order: + # "code/B.py" < "code/a.py" < "run.sh" + expected = hashlib.sha256() + for rel_posix, content in [ + ("code/B.py", b"b\n"), + ("code/a.py", b"a\n"), + ("run.sh", b"echo 1\n"), + ]: + expected.update(rel_posix.encode()) + expected.update(b"\0") + expected.update(hashlib.sha256(content).digest()) + + assert hash_forward == expected.hexdigest() + + +def test_as_sent_utf8_non_ascii(tmp_path: Path): + py_file = tmp_path / "unicode.py" + greeting = "Héllo wörld" + pi = "π" + content = f'greeting = "{greeting}"\r\npi = "{pi}"\r\n'.encode("utf-8") + py_file.write_bytes(content) + + # Must normalize newlines and preserve UTF-8 encoding + expected = f'greeting = "{greeting}"\npi = "{pi}"\n'.encode("utf-8") + assert _as_sent(py_file) == expected + + +def test_as_sent_binary_fallback(tmp_path: Path): + bin_file = tmp_path / "raw.bin" + # Invalid UTF-8 bytes containing CRLF + raw_bytes = b"\xff\xfe\x00\x01\r\n\x80\x81" + bin_file.write_bytes(raw_bytes) + + # Must fall back to raw read_bytes() without raising UnicodeDecodeError + assert _as_sent(bin_file) == raw_bytes + + +def test_job_has_all_files_accepts_crlf(tmp_path: Path): + job_dir = tmp_path / "test.job" + (job_dir / "code").mkdir(parents=True) + (job_dir / "run.sh").write_bytes(b"#!/bin/sh\r\npython code/main.py\r\n") + (job_dir / "code" / "main.py").write_bytes(b"print('hello')\r\n") + + declared_hash = code_hash(job_dir, as_sent=True) + + client = MagicMock() + client.manager.submission_dir.return_value = job_dir + client.manager.read_submission.return_value = JobSubmissionMetadata( + name="test.job", + submitted_by="ds@test.org", + datasite_email="do@test.org", + submitted_at=datetime.now(timezone.utc), + headers={SUBMISSION_HASH_HEADER: declared_hash}, + ) + + # Verify that CRLF files on disk match the declared hash + assert JobClient._job_has_all_files(client, MagicMock()) is True + + # Verify that tampered content is rejected + (job_dir / "code" / "main.py").write_bytes(b"print('tampered')\r\n") + assert JobClient._job_has_all_files(client, MagicMock()) is False + + +def test_request_has_all_files_accepts_crlf(tmp_path: Path): + req_dir = tmp_path / "call.job" + req_dir.mkdir() + (req_dir / "request.yaml").write_bytes(b"api: test\r\n") + (req_dir / "params.json").write_bytes(b'{"key": "value"}\r\n') + + declared_files_hash = request_hash(req_dir, as_sent=True) + + client = MagicMock() + client.manager.request_dir.return_value = req_dir + client.manager.read_request.return_value = RequestMetadata( + api="test", + submitted_at=datetime.now(timezone.utc), + files=["params.json"], + files_hash=declared_files_hash, + ) + + # Verify that CRLF files on disk match the declared files_hash + assert JobClient._request_has_all_files(client, MagicMock()) is True + + # Verify that tampered content is rejected + (req_dir / "params.json").write_bytes(b'{"key": "tampered"}\r\n') + assert JobClient._request_has_all_files(client, MagicMock()) is False + + +def test_code_hash_mixed_newlines_and_empty_files(tmp_path: Path): + """Verify mixed CRLF/LF newlines, empty files, and nested paths hash identically.""" + mix_dir = tmp_path / "mix" + lf_dir = tmp_path / "lf" + for d in (mix_dir, lf_dir): + (d / "code" / "sub").mkdir(parents=True) + (d / "code" / "empty.py").write_bytes(b"") + + (mix_dir / "run.sh").write_bytes(b"#!/bin/sh\r\necho 1\necho 2\r\n") + (mix_dir / "code" / "sub" / "mod.py").write_bytes(b"x = 1\r\ny = 2\n") + + (lf_dir / "run.sh").write_bytes(b"#!/bin/sh\necho 1\necho 2\n") + (lf_dir / "code" / "sub" / "mod.py").write_bytes(b"x = 1\ny = 2\n") + + assert code_hash(mix_dir, as_sent=True) == code_hash(lf_dir, as_sent=True) + assert code_hash(mix_dir, as_sent=True) == code_hash(lf_dir, as_sent=False) + + +def test_job_has_all_files_rejects_missing_or_extra_files(tmp_path: Path): + """Verify that missing files or extra unexpected files reject reception.""" + job_dir = tmp_path / "missing_extra.job" + (job_dir / "code").mkdir(parents=True) + (job_dir / "run.sh").write_bytes(b"#!/bin/sh\npython code/main.py\n") + (job_dir / "code" / "main.py").write_bytes(b"print('hello')\n") + + declared_hash = code_hash(job_dir, as_sent=True) + + client = MagicMock() + client.manager.submission_dir.return_value = job_dir + client.manager.read_submission.return_value = JobSubmissionMetadata( + name="missing_extra.job", + submitted_by="ds@test.org", + datasite_email="do@test.org", + submitted_at=datetime.now(timezone.utc), + headers={SUBMISSION_HASH_HEADER: declared_hash}, + ) + + # Sanity check: valid state accepted + assert JobClient._job_has_all_files(client, MagicMock()) is True + + # Missing file rejected + (job_dir / "code" / "main.py").unlink() + assert JobClient._job_has_all_files(client, MagicMock()) is False + + # Restore main.py, add extra unexpected file + (job_dir / "code" / "main.py").write_bytes(b"print('hello')\n") + (job_dir / "code" / "extra.py").write_bytes(b"print('extra')\n") + assert JobClient._job_has_all_files(client, MagicMock()) is False + + +def test_submission_hash_preserves_raw_disk_integrity(tmp_path: Path): + """submission_hash must NOT normalize line endings; it pins raw on-disk state.""" + crlf_sub = tmp_path / "crlf_sub" + lf_sub = tmp_path / "lf_sub" + for d in (crlf_sub, lf_sub): + (d / "code").mkdir(parents=True) + (d / "config.yaml").write_bytes(b"name: job\n") + + (crlf_sub / "run.sh").write_bytes(b"#!/bin/sh\r\n") + (crlf_sub / "code" / "main.py").write_bytes(b"print(1)\r\n") + + (lf_sub / "run.sh").write_bytes(b"#!/bin/sh\n") + (lf_sub / "code" / "main.py").write_bytes(b"print(1)\n") + + # Raw on-disk bytes differ, so submission_hash must differ + assert submission_hash(crlf_sub) != submission_hash(lf_sub)