diff --git a/docs/source/builder/security.md b/docs/source/builder/security.md index 48dc9a5e..45a2ea7e 100644 --- a/docs/source/builder/security.md +++ b/docs/source/builder/security.md @@ -69,3 +69,13 @@ Before building a kernel, ensure that all changes are committed. This makes it possible to reproduce a build from exactly the same source code. We bake the git shorthash into the ops name, so that it is clear from which git hash a kernel was built. + +The build [`provenance`](kernel-requirements.md) records a `dirty` flag when +a variant was built from a working tree with uncommitted changes. To keep +these builds from slipping by unnoticed: + +- `kernel-builder upload` prints a warning listing every dirty variant it is + about to publish, and injects a warning banner into the uploaded + `README.md` (built from `CARD.md`) naming those variants. +- The `kernels` loader emits a warning when a kernel variant it loads was + built from a dirty tree. diff --git a/kernel-builder/src/upload.rs b/kernel-builder/src/upload.rs index 48160475..8629e3b5 100644 --- a/kernel-builder/src/upload.rs +++ b/kernel-builder/src/upload.rs @@ -155,6 +155,9 @@ fn run_upload_typed(args: UploadArgs) -> Result<()> { build_dir.display() ); + let dirty_variants = dirty_variant_names(&variants); + warn_dirty_variants(&dirty_variants); + let (repo_id, branch) = get_repo_and_branch(&kernel_dir, args.repo_id, args.branch, &variants)?; // With --create-pr, write access is not required and we must not create a @@ -531,6 +534,51 @@ fn collect_build_commit_ops( Ok(()) } +/// Warn about build variants whose provenance is dirty. +/// +/// A dirty variant was built from a working tree with uncommitted changes, so +/// its recorded git SHA does not fully identify the sources it was built from +/// and the build may not be reproducible. Metadata that cannot be read is +/// silently skipped: this is a best-effort warning, not a hard check. +fn warn_dirty_variants(dirty: &[String]) { + if dirty.is_empty() { + return; + } + + eprintln!( + "warning: uploading {} build variant(s) built from a dirty git tree \ + (uncommitted changes): {}. Their recorded git revision does not fully \ + identify the sources they were built from, so the build may not be \ + reproducible.", + dirty.len(), + dirty.join(", ") + ); +} + +/// Names of the build variants whose provenance is dirty, sorted. +/// +/// Metadata that cannot be read is silently skipped: this backs a best-effort +/// warning, not a hard check. +fn dirty_variant_names(variants: &[PathBuf]) -> Vec { + let mut dirty: Vec = variants + .iter() + .filter(|variant| { + File::open(variant.join("metadata.json")) + .ok() + .and_then(|f| Metadata::from_reader(BufReader::new(f)).ok()) + .and_then(|m| m.provenance) + .is_some_and(|p| p.is_dirty()) + }) + .filter_map(|variant| { + variant + .file_name() + .map(|n| n.to_string_lossy().into_owned()) + }) + .collect(); + dirty.sort(); + dirty +} + /// Determine the branch name (`v{version}`) from variant metadata. fn detect_branch_from_metadata(variants: &[PathBuf]) -> Result> { let mut versions: HashSet = HashSet::new(); @@ -767,6 +815,38 @@ mod tests { const METADATA_V3: &str = r#"{"name": "test-kernel", "id": "kernel_id", "version": 3, "license": "Apache-2.0", "python-depends": [], "backend": {"type": "cuda"}}"#; const METADATA_V0: &str = r#"{"name": "test-kernel", "id": "kernel_id", "version": 0, "license": "Apache-2.0", "python-depends": [], "backend": {"type": "cuda"}}"#; + const METADATA_DIRTY: &str = r#"{"name": "test-kernel", "id": "kernel_id", "version": 1, "license": "Apache-2.0", "python-depends": [], "backend": {"type": "cuda"}, "provenance": {"kernel-builder": {"version": "0.1.0", "sha": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", "dirty": false}, "kernel": {"sha": "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", "dirty": true}}}"#; + + #[test] + fn test_dirty_variant_names() { + let temp_dir = tempfile::tempdir().unwrap(); + + let clean = temp_dir.path().join("torch-cpu"); + fs::create_dir_all(&clean).unwrap(); + fs::write(clean.join("metadata.json"), METADATA_V3).unwrap(); + + let dirty = temp_dir.path().join("torch-cuda"); + fs::create_dir_all(&dirty).unwrap(); + fs::write(dirty.join("metadata.json"), METADATA_DIRTY).unwrap(); + + // A variant with unreadable metadata is skipped rather than failing. + let broken = temp_dir.path().join("torch-rocm"); + fs::create_dir_all(&broken).unwrap(); + + let names = dirty_variant_names(&[clean, dirty, broken]); + assert_eq!(names, vec!["torch-cuda".to_owned()]); + } + + #[test] + fn test_dirty_variant_names_none_dirty() { + let temp_dir = tempfile::tempdir().unwrap(); + let clean = temp_dir.path().join("torch-cpu"); + fs::create_dir_all(&clean).unwrap(); + fs::write(clean.join("metadata.json"), METADATA_V3).unwrap(); + + assert!(dirty_variant_names(&[clean]).is_empty()); + } + #[test] fn test_detect_branch_from_metadata() { let temp_dir = tempfile::tempdir().unwrap(); diff --git a/kernels/src/kernels/utils.py b/kernels/src/kernels/utils.py index c54a5922..cd2f1000 100644 --- a/kernels/src/kernels/utils.py +++ b/kernels/src/kernels/utils.py @@ -204,11 +204,39 @@ def _validate_variant_dependencies(variant_path: Path) -> None: validate_dependencies(metadata.name.python_name, metadata.python_depends, _backend()) +def _warn_if_dirty(metadata: Metadata, variant_str: str) -> None: + """Warn when a kernel variant was built from a dirty git tree. + + A dirty build was produced from a working tree with uncommitted changes, + so its git SHA does not fully identify the sources it was built from and + the build may not be reproducible. + """ + provenance = metadata.provenance + if provenance is None or not provenance.dirty: + return + + dirty_sources = [] + if provenance.kernel is not None and provenance.kernel.dirty: + dirty_sources.append("kernel source") + builder_git = provenance.kernel_builder.git + if builder_git is not None and builder_git.dirty: + dirty_sources.append("kernel-builder") + + warnings.warn( + f"Kernel '{metadata.name}' variant '{variant_str}' was built from a dirty " + f"git tree ({', '.join(dirty_sources)} had uncommitted changes). Its " + "recorded git revision does not fully identify the sources it was built " + "from, so the build may not be reproducible.", + stacklevel=3, + ) + + def _import_from_path(variant_path: Path, repo_info: RepoInfo | None = None) -> ModuleType: if (loaded_kernel := _loaded_kernels.get(variant_path)) is not None: return loaded_kernel.module metadata = Metadata.read_from_file(variant_path / "metadata.json") + _warn_if_dirty(metadata, variant_path.name) module_name = metadata.name.python_name file_path = variant_path / "__init__.py" diff --git a/kernels/tests/test_dirty_provenance.py b/kernels/tests/test_dirty_provenance.py new file mode 100644 index 00000000..436ec15a --- /dev/null +++ b/kernels/tests/test_dirty_provenance.py @@ -0,0 +1,71 @@ +import json + +import pytest +from kernels_data import Metadata + +from kernels.utils import _import_from_path, _loaded_kernels, _warn_if_dirty + + +def _write_variant(tmp_path, provenance): + variant_dir = tmp_path / "build" / "torch28-cxx11-cu128-x86_64-linux" + variant_dir.mkdir(parents=True) + metadata = { + "id": "activation_1_cuda", + "name": "activation", + "version": 1, + "license": "Apache-2.0", + "python-depends": ["torch"], + "backend": {"type": "cuda"}, + } + if provenance is not None: + metadata["provenance"] = provenance + (variant_dir / "metadata.json").write_text(json.dumps(metadata)) + return variant_dir + + +CLEAN_PROVENANCE = { + "kernel-builder": {"version": "0.1.0", "sha": "a" * 40, "dirty": False}, + "kernel": {"sha": "b" * 40, "dirty": False}, +} +DIRTY_KERNEL = { + "kernel-builder": {"version": "0.1.0", "sha": "a" * 40, "dirty": False}, + "kernel": {"sha": "b" * 40, "dirty": True}, +} +DIRTY_BUILDER = { + "kernel-builder": {"version": "0.1.0", "sha": "a" * 40, "dirty": True}, + "kernel": {"sha": "b" * 40, "dirty": False}, +} + + +@pytest.mark.parametrize("provenance", [None, CLEAN_PROVENANCE]) +def test_no_warning_when_clean(tmp_path, recwarn, provenance): + variant_dir = _write_variant(tmp_path, provenance) + metadata = Metadata.read_from_file(variant_dir / "metadata.json") + _warn_if_dirty(metadata, variant_dir.name) + assert len(recwarn) == 0 + + +def test_warns_on_dirty_kernel_source(tmp_path): + variant_dir = _write_variant(tmp_path, DIRTY_KERNEL) + metadata = Metadata.read_from_file(variant_dir / "metadata.json") + with pytest.warns(UserWarning, match="dirty git tree"): + _warn_if_dirty(metadata, variant_dir.name) + + +def test_warns_names_dirty_sources(tmp_path): + variant_dir = _write_variant(tmp_path, DIRTY_BUILDER) + metadata = Metadata.read_from_file(variant_dir / "metadata.json") + with pytest.warns(UserWarning, match="kernel-builder"): + _warn_if_dirty(metadata, variant_dir.name) + + +def test_import_from_path_warns_on_dirty(tmp_path): + variant_dir = _write_variant(tmp_path, DIRTY_KERNEL) + (variant_dir / "__init__.py").write_text("value = 42\n") + _loaded_kernels.pop(variant_dir, None) + try: + with pytest.warns(UserWarning, match="dirty git tree"): + module = _import_from_path(variant_dir) + assert module.value == 42 + finally: + _loaded_kernels.pop(variant_dir, None)