Skip to content

Commit 6d5ef7d

Browse files
committed
feat: Add expected-commit verification to git sources
This commit implements the feature requested in issue #1895, adding an optional `expected-commit` field to git source configurations. Changes: - Added `expected_commit` field to GitSource struct in source.rs - Added GitCommitMismatch error variant to SourceError enum for structured error handling with expected, actual, and rev fields - Implemented verification logic in git_source.rs to compare actual commit against expected commit after checkout - Added comprehensive unit tests for serialization/deserialization - Added integration tests for both success and failure cases - Created test recipe demonstrating the feature - Updated documentation in recipe_file.md with usage examples The feature helps ensure security and reproducibility by detecting when a git tag or branch has been moved to point to a different commit than expected. Inspired by Wolfi/Melange. Closes #1895
1 parent a3e91e8 commit 6d5ef7d

File tree

6 files changed

+214
-1
lines changed

6 files changed

+214
-1
lines changed

docs/reference/recipe_file.md

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -246,6 +246,19 @@ source:
246246
lfs: true # note: defaults to false
247247
```
248248
249+
##### Verifying commit hash with `expected-commit`
250+
251+
For security and reproducibility, you can specify an `expected-commit` field to verify that the checked out commit matches the expected SHA hash. This is useful to detect if a tag or branch has been moved to point to a different commit:
252+
253+
```yaml
254+
source:
255+
git: https://github.com/ilanschnell/bsdiff4.git
256+
tag: "1.1.4"
257+
expected-commit: 50a1f7ed6c168eb0815d424cba2df62790f168f0
258+
```
259+
260+
If the actual commit does not match the expected commit, the build will fail with an error message indicating the mismatch. This feature is inspired by [Wolfi/Melange](https://github.com/wolfi-dev/wolfi-os) and provides an additional layer of security for your builds.
261+
249262
#### Source from a local path
250263

251264
If the path is relative, it is taken relative to the recipe directory. The
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
---
2+
source: src/recipe/parser/source.rs
3+
assertion_line: 735
4+
expression: yaml
5+
---
6+
git: https://test.com/test.git
7+
rev: refs/tags/v1.0.0
8+
expected_commit: abc123def456

src/recipe/parser/source.rs

Lines changed: 62 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,9 @@ pub struct GitSource {
192192
/// Optionally request the lfs pull in git source
193193
#[serde(default, skip_serializing_if = "should_not_serialize_lfs")]
194194
pub lfs: bool,
195+
/// Optionally an expected commit hash to verify after checkout
196+
#[serde(default, skip_serializing_if = "Option::is_none")]
197+
pub expected_commit: Option<String>,
195198
}
196199

197200
/// A helper method to skip serializing the lfs flag if it is false.
@@ -217,6 +220,7 @@ impl GitSource {
217220
patches,
218221
target_directory,
219222
lfs,
223+
expected_commit: None,
220224
}
221225
}
222226

@@ -249,6 +253,11 @@ impl GitSource {
249253
pub const fn lfs(&self) -> bool {
250254
self.lfs
251255
}
256+
257+
/// Get the expected commit hash.
258+
pub fn expected_commit(&self) -> Option<&str> {
259+
self.expected_commit.as_deref()
260+
}
252261
}
253262

254263
impl TryConvertNode<GitSource> for RenderedMappingNode {
@@ -259,6 +268,7 @@ impl TryConvertNode<GitSource> for RenderedMappingNode {
259268
let mut patches = Vec::new();
260269
let mut target_directory = None;
261270
let mut lfs = false;
271+
let mut expected_commit = None;
262272

263273
self.iter().map(|(k, v)| {
264274
match k.as_str() {
@@ -317,11 +327,14 @@ impl TryConvertNode<GitSource> for RenderedMappingNode {
317327
"lfs" => {
318328
lfs = v.try_convert("lfs")?;
319329
}
330+
"expected-commit" => {
331+
expected_commit = Some(v.try_convert("expected-commit")?);
332+
}
320333
_ => {
321334
return Err(vec![_partialerror!(
322335
*k.span(),
323336
ErrorKind::InvalidField(k.as_str().to_owned().into()),
324-
help = "valid fields for git `source` are `git`, `rev`, `tag`, `branch`, `depth`, `patches`, `lfs` and `target_directory`"
337+
help = "valid fields for git `source` are `git`, `rev`, `tag`, `branch`, `depth`, `patches`, `lfs`, `expected-commit` and `target_directory`"
325338
)])
326339
}
327340
}
@@ -354,6 +367,7 @@ impl TryConvertNode<GitSource> for RenderedMappingNode {
354367
patches,
355368
target_directory,
356369
lfs,
370+
expected_commit,
357371
})
358372
}
359373
}
@@ -652,6 +666,7 @@ mod tests {
652666
patches: Vec::new(),
653667
target_directory: None,
654668
lfs: false,
669+
expected_commit: None,
655670
};
656671

657672
let yaml = serde_yaml::to_string(&git).unwrap();
@@ -672,6 +687,7 @@ mod tests {
672687
patches: Vec::new(),
673688
target_directory: None,
674689
lfs: false,
690+
expected_commit: None,
675691
};
676692

677693
let yaml = serde_yaml::to_string(&git).unwrap();
@@ -701,4 +717,49 @@ mod tests {
701717
let json = serde_json::to_string(&path_source).unwrap();
702718
serde_json::from_str::<PathSource>(&json).unwrap();
703719
}
720+
721+
#[test]
722+
fn test_git_source_with_expected_commit() {
723+
let git = GitSource {
724+
url: GitUrl::Url(Url::parse("https://test.com/test.git").unwrap()),
725+
rev: GitRev::Tag("v1.0.0".into()),
726+
depth: None,
727+
patches: Vec::new(),
728+
target_directory: None,
729+
lfs: false,
730+
expected_commit: Some("abc123def456".to_string()),
731+
};
732+
733+
let yaml = serde_yaml::to_string(&git).unwrap();
734+
735+
insta::assert_snapshot!(yaml);
736+
737+
let parsed_git: GitSource = serde_yaml::from_str(&yaml).unwrap();
738+
739+
assert_eq!(parsed_git.expected_commit, git.expected_commit);
740+
assert_eq!(parsed_git.expected_commit(), Some("abc123def456"));
741+
}
742+
743+
#[test]
744+
fn test_git_source_without_expected_commit() {
745+
let git = GitSource {
746+
url: GitUrl::Url(Url::parse("https://test.com/test.git").unwrap()),
747+
rev: GitRev::Tag("v1.0.0".into()),
748+
depth: None,
749+
patches: Vec::new(),
750+
target_directory: None,
751+
lfs: false,
752+
expected_commit: None,
753+
};
754+
755+
let yaml = serde_yaml::to_string(&git).unwrap();
756+
757+
// expected_commit should not appear in serialized yaml when None
758+
assert!(!yaml.contains("expected"));
759+
760+
let parsed_git: GitSource = serde_yaml::from_str(&yaml).unwrap();
761+
762+
assert_eq!(parsed_git.expected_commit, None);
763+
assert_eq!(parsed_git.expected_commit(), None);
764+
}
704765
}

src/source/git_source.rs

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -280,6 +280,18 @@ pub fn git_src(
280280
.trim()
281281
.to_owned();
282282

283+
// Verify expected commit if specified
284+
if let Some(expected) = source.expected_commit() {
285+
if ref_git != expected {
286+
return Err(SourceError::GitCommitMismatch {
287+
expected: expected.to_string(),
288+
actual: ref_git,
289+
rev: rev.to_string(),
290+
});
291+
}
292+
tracing::info!("Verified expected commit: {}", expected);
293+
}
294+
283295
// only do lfs pull if a requirement!
284296
if source.lfs() {
285297
git_lfs_pull(&ref_git)?;
@@ -392,4 +404,104 @@ mod tests {
392404
);
393405
}
394406
}
407+
408+
#[tracing_test::traced_test]
409+
#[test]
410+
fn test_expected_commit_success() {
411+
let temp_dir = tempfile::tempdir().unwrap();
412+
let cache_dir = temp_dir.path().join("rattler-build-test-expected-commit");
413+
let recipe_dir = temp_dir.path().join("recipe");
414+
fs_err::create_dir_all(&recipe_dir).unwrap();
415+
416+
let system_tools = crate::system_tools::SystemTools::new();
417+
418+
// First, fetch without expected commit to find out what the actual commit is
419+
let source_without_expected = GitSource {
420+
url: GitUrl::Url(
421+
"https://github.com/prefix-dev/rattler-build"
422+
.parse()
423+
.unwrap(),
424+
),
425+
rev: GitRev::Tag("v0.1.0".to_owned()),
426+
depth: None,
427+
patches: vec![],
428+
target_directory: None,
429+
lfs: false,
430+
expected_commit: None,
431+
};
432+
433+
let (_, actual_commit) =
434+
git_src(&system_tools, &source_without_expected, &cache_dir, &recipe_dir).unwrap();
435+
436+
// Now test with the correct expected commit
437+
let source_with_expected = GitSource {
438+
url: GitUrl::Url(
439+
"https://github.com/prefix-dev/rattler-build"
440+
.parse()
441+
.unwrap(),
442+
),
443+
rev: GitRev::Tag("v0.1.0".to_owned()),
444+
depth: None,
445+
patches: vec![],
446+
target_directory: None,
447+
lfs: false,
448+
expected_commit: Some(actual_commit.clone()),
449+
};
450+
451+
let result = git_src(&system_tools, &source_with_expected, &cache_dir, &recipe_dir);
452+
assert!(result.is_ok());
453+
let (_, commit) = result.unwrap();
454+
assert_eq!(commit, actual_commit);
455+
}
456+
457+
#[tracing_test::traced_test]
458+
#[test]
459+
fn test_expected_commit_mismatch() {
460+
let temp_dir = tempfile::tempdir().unwrap();
461+
let cache_dir = temp_dir
462+
.path()
463+
.join("rattler-build-test-expected-commit-mismatch");
464+
let recipe_dir = temp_dir.path().join("recipe");
465+
fs_err::create_dir_all(&recipe_dir).unwrap();
466+
467+
let system_tools = crate::system_tools::SystemTools::new();
468+
469+
// Test with an incorrect expected commit
470+
let source_with_wrong_expected = GitSource {
471+
url: GitUrl::Url(
472+
"https://github.com/prefix-dev/rattler-build"
473+
.parse()
474+
.unwrap(),
475+
),
476+
rev: GitRev::Tag("v0.1.0".to_owned()),
477+
depth: None,
478+
patches: vec![],
479+
target_directory: None,
480+
lfs: false,
481+
expected_commit: Some("0000000000000000000000000000000000000000".to_string()),
482+
};
483+
484+
let result = git_src(
485+
&system_tools,
486+
&source_with_wrong_expected,
487+
&cache_dir,
488+
&recipe_dir,
489+
);
490+
assert!(result.is_err());
491+
492+
// Verify the error is specifically a GitCommitMismatch
493+
let err = result.unwrap_err();
494+
match err {
495+
crate::source::SourceError::GitCommitMismatch {
496+
expected,
497+
actual,
498+
rev,
499+
} => {
500+
assert_eq!(expected, "0000000000000000000000000000000000000000");
501+
assert!(!actual.is_empty());
502+
assert_eq!(rev, "refs/tags/v0.1.0");
503+
}
504+
_ => panic!("Expected GitCommitMismatch error, got: {:?}", err),
505+
}
506+
}
395507
}

src/source/mod.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,13 @@ pub enum SourceError {
9191
#[error("Failed to run git command: {0}")]
9292
GitErrorStr(&'static str),
9393

94+
#[error("Git commit mismatch: expected commit '{expected}' but got '{actual}' for revision '{rev}'")]
95+
GitCommitMismatch {
96+
expected: String,
97+
actual: String,
98+
rev: String,
99+
},
100+
94101
#[error("{0}")]
95102
UnknownError(String),
96103

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
package:
2+
name: "git_source_expected_commit"
3+
version: "1"
4+
5+
source:
6+
- git: https://github.com/prefix-dev/rattler-build
7+
tag: v0.1.0
8+
expected-commit: df83c1edf287a756b8fc995e03e9632af0344777
9+
10+
build:
11+
script:
12+
- test -f README.md

0 commit comments

Comments
 (0)