From 8adceffabf579fc37e2ea785af03c6eb61ce5f7b Mon Sep 17 00:00:00 2001 From: sayakpaul Date: Wed, 8 Jul 2026 17:03:29 +0530 Subject: [PATCH 1/2] warn for dirty builds. --- docs/source/builder/security.md | 10 ++ kernel-builder/src/upload.rs | 209 ++++++++++++++++++++++++- kernels/src/kernels/utils.py | 28 ++++ kernels/tests/test_dirty_provenance.py | 71 +++++++++ 4 files changed, 313 insertions(+), 5 deletions(-) create mode 100644 kernels/tests/test_dirty_provenance.py 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 89c6c59a..83ef69c9 100644 --- a/kernel-builder/src/upload.rs +++ b/kernel-builder/src/upload.rs @@ -150,6 +150,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)?; let repo_url = match api @@ -207,6 +210,7 @@ fn run_upload_typed(args: UploadArgs) -> Result<()> { collect_readme_commit_ops( &kernel_dir, + &dirty_variants, operations_by_branch .entry(MAIN_BRANCH.to_owned()) .or_default(), @@ -363,16 +367,79 @@ fn collect_benchmark_commit_ops( } /// Collect README commit operation: upload build/CARD.md as README.md. -fn collect_readme_commit_ops(kernel_dir: &Path, operations: &mut Vec) { +/// +/// When any variant was built from a dirty git tree, a warning banner naming +/// those variants is injected into the card so it is visible on the Hub. +fn collect_readme_commit_ops( + kernel_dir: &Path, + dirty_variants: &[String], + operations: &mut Vec, +) { let Ok(card_path) = discover_build_file(kernel_dir, "CARD.md") else { return; }; + + // Without a dirty build there is nothing to inject, so stream the file + // directly rather than reading it into memory. + if dirty_variants.is_empty() { + operations.push(CommitOperation::Add { + path_in_repo: "README.md".to_owned(), + source: AddSource::File(card_path), + }); + return; + } + + let source = match fs::read_to_string(&card_path) { + Ok(card) => { + AddSource::Bytes(render_card_with_dirty_banner(&card, dirty_variants).into_bytes()) + } + // If the card cannot be read as UTF-8, fall back to uploading it + // verbatim rather than dropping the README entirely. + Err(_) => AddSource::File(card_path), + }; operations.push(CommitOperation::Add { path_in_repo: "README.md".to_owned(), - source: AddSource::File(card_path), + source, }); } +/// Insert a "dirty build" warning banner into a model card. +/// +/// The banner is placed after the YAML front matter (if any) so the front +/// matter stays parseable, otherwise at the very top of the card. +fn render_card_with_dirty_banner(card: &str, dirty_variants: &[String]) -> String { + let banner = format!( + "> [!WARNING]\n\ + > This kernel was built from a dirty git tree (uncommitted changes) for the \ + following variant(s): {}. The recorded git revision does not fully identify \ + the sources they were built from, so the build may not be reproducible.\n", + dirty_variants + .iter() + .map(|v| format!("`{v}`")) + .collect::>() + .join(", ") + ); + + // Detect leading YAML front matter delimited by `---` lines. + let body = card + .strip_prefix("---\n") + .or_else(|| card.strip_prefix("---\r\n")); + if let Some(after_open) = body { + if let Some(end) = after_open.find("\n---") { + // Split just past the closing `---` line (and its newline, if present). + let rest = &after_open[end + "\n---".len()..]; + let rest = rest + .strip_prefix("\r\n") + .or_else(|| rest.strip_prefix('\n')) + .unwrap_or(rest); + let front_matter = &card[..card.len() - rest.len()]; + return format!("{front_matter}\n{banner}\n{rest}"); + } + } + + format!("{banner}\n{card}") +} + /// Collect build artifact commit operations: add variant files, delete stale ones. fn collect_build_commit_ops( build_dir: &Path, @@ -456,6 +523,51 @@ fn discover_build_file( ); } +/// 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(); @@ -513,12 +625,17 @@ mod tests { fs::write(kernel_dir.join("build/CARD.md"), "# Readme").unwrap(); let mut operations = vec![]; - collect_readme_commit_ops(kernel_dir, &mut operations); + collect_readme_commit_ops(kernel_dir, &[], &mut operations); assert_eq!(operations.len(), 1); match &operations[0] { - CommitOperation::Add { path_in_repo, .. } => { + CommitOperation::Add { + path_in_repo, + source, + } => { assert_eq!(path_in_repo, "README.md"); + // A clean build streams the card file verbatim. + assert!(matches!(source, AddSource::File(_))); } _ => panic!("Expected Add operation"), } @@ -528,10 +645,60 @@ mod tests { fn test_collect_readme_commit_ops_no_card() { let temp_dir = tempfile::tempdir().unwrap(); let mut operations = vec![]; - collect_readme_commit_ops(temp_dir.path(), &mut operations); + collect_readme_commit_ops(temp_dir.path(), &[], &mut operations); assert!(operations.is_empty()); } + #[test] + fn test_collect_readme_commit_ops_injects_dirty_banner() { + let temp_dir = tempfile::tempdir().unwrap(); + let kernel_dir = temp_dir.path(); + fs::create_dir_all(kernel_dir.join("build")).unwrap(); + fs::write( + kernel_dir.join("build/CARD.md"), + "---\ntags: [kernel]\n---\n# Readme\n", + ) + .unwrap(); + + let mut operations = vec![]; + collect_readme_commit_ops(kernel_dir, &["torch-cuda".to_owned()], &mut operations); + + assert_eq!(operations.len(), 1); + let CommitOperation::Add { + source: AddSource::Bytes(bytes), + .. + } = &operations[0] + else { + panic!("Expected Add operation with rewritten bytes"); + }; + let rendered = String::from_utf8(bytes.clone()).unwrap(); + assert!(rendered.contains("[!WARNING]")); + assert!(rendered.contains("`torch-cuda`")); + // Front matter is preserved at the top and the body still follows. + assert!(rendered.starts_with("---\ntags: [kernel]\n---\n")); + assert!(rendered.contains("# Readme")); + } + + #[test] + fn test_render_card_with_dirty_banner_after_front_matter() { + let card = "---\ntags: [kernel]\n---\n# Title\n\nBody.\n"; + let rendered = render_card_with_dirty_banner(card, &["torch-cuda".to_owned()]); + // Front matter stays intact at the top, then the banner, then the body. + assert!(rendered.starts_with("---\ntags: [kernel]\n---\n")); + let banner_pos = rendered.find("[!WARNING]").unwrap(); + let title_pos = rendered.find("# Title").unwrap(); + assert!(banner_pos < title_pos); + } + + #[test] + fn test_render_card_with_dirty_banner_no_front_matter() { + let card = "# Title\n\nBody.\n"; + let rendered = render_card_with_dirty_banner(card, &["a".to_owned(), "b".to_owned()]); + assert!(rendered.starts_with("> [!WARNING]")); + assert!(rendered.contains("`a`, `b`")); + assert!(rendered.trim_end().ends_with("Body.")); + } + #[test] fn test_collect_benchmark_commit_ops() { let temp_dir = tempfile::tempdir().unwrap(); @@ -666,6 +833,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) From e2812ac84cb60cd42f13d3bf6d2b6857aacc0bd3 Mon Sep 17 00:00:00 2001 From: sayakpaul Date: Sat, 11 Jul 2026 11:05:49 +0530 Subject: [PATCH 2/2] warn during the uploads and don't amend the readme. --- kernel-builder/src/upload.rs | 127 ++--------------------------------- 1 file changed, 6 insertions(+), 121 deletions(-) diff --git a/kernel-builder/src/upload.rs b/kernel-builder/src/upload.rs index 02da4b5b..8629e3b5 100644 --- a/kernel-builder/src/upload.rs +++ b/kernel-builder/src/upload.rs @@ -232,7 +232,6 @@ fn run_upload_typed(args: UploadArgs) -> Result<()> { collect_readme_commit_ops( &build_dir, - &dirty_variants, operations_by_branch .entry(MAIN_BRANCH.to_owned()) .or_default(), @@ -470,80 +469,17 @@ fn collect_benchmark_commit_ops( /// holds the build variants (as returned by `discover_variants`). This ensures /// the card is taken from the same location as the variants rather than an /// unrelated directory elsewhere in the repository (see issue #659). -/// -/// When any variant was built from a dirty git tree, a warning banner naming -/// those variants is injected into the card so it is visible on the Hub. -fn collect_readme_commit_ops( - build_dir: &Path, - dirty_variants: &[String], - operations: &mut Vec, -) { +fn collect_readme_commit_ops(build_dir: &Path, operations: &mut Vec) { let card_path = build_dir.join("CARD.md"); if !card_path.is_file() { return; } - - // Without a dirty build there is nothing to inject, so stream the file - // directly rather than reading it into memory. - if dirty_variants.is_empty() { - operations.push(CommitOperation::Add { - path_in_repo: "README.md".to_owned(), - source: AddSource::File(card_path), - }); - return; - } - - let source = match fs::read_to_string(&card_path) { - Ok(card) => { - AddSource::Bytes(render_card_with_dirty_banner(&card, dirty_variants).into_bytes()) - } - // If the card cannot be read as UTF-8, fall back to uploading it - // verbatim rather than dropping the README entirely. - Err(_) => AddSource::File(card_path), - }; operations.push(CommitOperation::Add { path_in_repo: "README.md".to_owned(), - source, + source: AddSource::File(card_path), }); } -/// Insert a "dirty build" warning banner into a model card. -/// -/// The banner is placed after the YAML front matter (if any) so the front -/// matter stays parseable, otherwise at the very top of the card. -fn render_card_with_dirty_banner(card: &str, dirty_variants: &[String]) -> String { - let banner = format!( - "> [!WARNING]\n\ - > This kernel was built from a dirty git tree (uncommitted changes) for the \ - following variant(s): {}. The recorded git revision does not fully identify \ - the sources they were built from, so the build may not be reproducible.\n", - dirty_variants - .iter() - .map(|v| format!("`{v}`")) - .collect::>() - .join(", ") - ); - - // Detect leading YAML front matter delimited by `---` lines. - let body = card - .strip_prefix("---\n") - .or_else(|| card.strip_prefix("---\r\n")); - if let Some(after_open) = body { - if let Some(end) = after_open.find("\n---") { - // Split just past the closing `---` line (and its newline, if present). - let rest = &after_open[end + "\n---".len()..]; - let rest = rest - .strip_prefix("\r\n") - .or_else(|| rest.strip_prefix('\n')) - .unwrap_or(rest); - let front_matter = &card[..card.len() - rest.len()]; - return format!("{front_matter}\n{banner}\n{rest}"); - } - } - - format!("{banner}\n{card}") -} - /// Collect build artifact commit operations: add variant files, delete stale ones. fn collect_build_commit_ops( build_dir: &Path, @@ -700,7 +636,7 @@ mod tests { fs::write(build_dir.join("CARD.md"), "# Readme").unwrap(); let mut operations = vec![]; - collect_readme_commit_ops(&build_dir, &[], &mut operations); + collect_readme_commit_ops(&build_dir, &mut operations); assert_eq!(operations.len(), 1); match &operations[0] { @@ -709,7 +645,6 @@ mod tests { source, } => { assert_eq!(path_in_repo, "README.md"); - // A clean build streams the card file verbatim. match source { AddSource::File(path) => assert_eq!(*path, build_dir.join("CARD.md")), _ => panic!("Expected file source"), @@ -723,60 +658,10 @@ mod tests { fn test_collect_readme_commit_ops_no_card() { let temp_dir = tempfile::tempdir().unwrap(); let mut operations = vec![]; - collect_readme_commit_ops(temp_dir.path(), &[], &mut operations); + collect_readme_commit_ops(temp_dir.path(), &mut operations); assert!(operations.is_empty()); } - #[test] - fn test_collect_readme_commit_ops_injects_dirty_banner() { - let temp_dir = tempfile::tempdir().unwrap(); - let build_dir = temp_dir.path().join("build"); - fs::create_dir_all(&build_dir).unwrap(); - fs::write( - build_dir.join("CARD.md"), - "---\ntags: [kernel]\n---\n# Readme\n", - ) - .unwrap(); - - let mut operations = vec![]; - collect_readme_commit_ops(&build_dir, &["torch-cuda".to_owned()], &mut operations); - - assert_eq!(operations.len(), 1); - let CommitOperation::Add { - source: AddSource::Bytes(bytes), - .. - } = &operations[0] - else { - panic!("Expected Add operation with rewritten bytes"); - }; - let rendered = String::from_utf8(bytes.clone()).unwrap(); - assert!(rendered.contains("[!WARNING]")); - assert!(rendered.contains("`torch-cuda`")); - // Front matter is preserved at the top and the body still follows. - assert!(rendered.starts_with("---\ntags: [kernel]\n---\n")); - assert!(rendered.contains("# Readme")); - } - - #[test] - fn test_render_card_with_dirty_banner_after_front_matter() { - let card = "---\ntags: [kernel]\n---\n# Title\n\nBody.\n"; - let rendered = render_card_with_dirty_banner(card, &["torch-cuda".to_owned()]); - // Front matter stays intact at the top, then the banner, then the body. - assert!(rendered.starts_with("---\ntags: [kernel]\n---\n")); - let banner_pos = rendered.find("[!WARNING]").unwrap(); - let title_pos = rendered.find("# Title").unwrap(); - assert!(banner_pos < title_pos); - } - - #[test] - fn test_render_card_with_dirty_banner_no_front_matter() { - let card = "# Title\n\nBody.\n"; - let rendered = render_card_with_dirty_banner(card, &["a".to_owned(), "b".to_owned()]); - assert!(rendered.starts_with("> [!WARNING]")); - assert!(rendered.contains("`a`, `b`")); - assert!(rendered.trim_end().ends_with("Body.")); - } - #[test] fn test_collect_readme_commit_ops_ignores_card_outside_build_dir() { // A card in the kernel directory (or any other directory) must not be @@ -791,7 +676,7 @@ mod tests { fs::write(kernel_dir.join("CARD.md"), "# Stray card").unwrap(); let mut operations = vec![]; - collect_readme_commit_ops(&build_dir, &[], &mut operations); + collect_readme_commit_ops(&build_dir, &mut operations); assert!(operations.is_empty()); } @@ -1026,7 +911,7 @@ mod tests { let build_dir = kernel_dir.join("result"); let mut operations = vec![]; - collect_readme_commit_ops(&build_dir, &[], &mut operations); + collect_readme_commit_ops(&build_dir, &mut operations); assert_eq!(operations.len(), 1); match &operations[0] {