Skip to content

Commit 85e007b

Browse files
committed
feat(hm-common): add GitSha newtype, validate commit sites
Introduce a validated git object-id type (40/64 hex, lowercase, null-oid sentinel) with transparent-string serde, and thread it through every place a commit id is produced or carried: - git::head_commit() -> Option<GitSha>, parsed from rev-parse. - exec::SourceMeta.commit: GitSha, converted to String only at the harmont-cloud SDK boundary. - hm run derives the commit as GitSha::zero() when git has none. - cloud run's --commit parses to GitSha at clap time, rejecting a malformed id before submission.
1 parent 96d370c commit 85e007b

6 files changed

Lines changed: 170 additions & 20 deletions

File tree

‎crates/hm-common/src/git.rs‎

Lines changed: 158 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
//! Running git.
1+
//! Git integration: running the `git` CLI and git value types.
22
33
use std::path::{Path, PathBuf};
44
use std::process::Command;
@@ -7,6 +7,98 @@ use bstr::{BStr, BString, ByteSlice};
77

88
use crate::process::{CapturedStreams as _, CommandExt as _};
99

10+
/// A git object identifier: a full 40-char SHA-1 or 64-char SHA-256 hex
11+
/// digest, normalized to lowercase.
12+
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
13+
pub struct GitSha(String);
14+
15+
/// A string that is not a valid [`GitSha`].
16+
#[derive(Debug, thiserror::Error, PartialEq, Eq)]
17+
pub enum GitShaError {
18+
/// The digest is not 40 (SHA-1) or 64 (SHA-256) hex characters long.
19+
#[error("git sha must be 40 or 64 hex chars, got {0}")]
20+
BadLength(usize),
21+
/// The digest contains a non-hex character.
22+
#[error("git sha contains a non-hex character")]
23+
NotHex,
24+
}
25+
26+
impl GitSha {
27+
/// The all-zero SHA-1 null oid (`0000…`, 40 chars) git uses to mean
28+
/// "no commit".
29+
#[must_use]
30+
pub fn zero() -> Self {
31+
Self("0".repeat(40))
32+
}
33+
34+
/// The digest as a lowercase hex string.
35+
#[must_use]
36+
pub fn as_str(&self) -> &str {
37+
&self.0
38+
}
39+
40+
/// Whether this is the all-zero null oid git uses to mean "no commit".
41+
#[must_use]
42+
pub fn is_zero(&self) -> bool {
43+
self.0.bytes().all(|b| b == b'0')
44+
}
45+
}
46+
47+
impl std::str::FromStr for GitSha {
48+
type Err = GitShaError;
49+
50+
fn from_str(s: &str) -> Result<Self, Self::Err> {
51+
if s.len() != 40 && s.len() != 64 {
52+
return Err(GitShaError::BadLength(s.len()));
53+
}
54+
if !s.bytes().all(|b| b.is_ascii_hexdigit()) {
55+
return Err(GitShaError::NotHex);
56+
}
57+
Ok(Self(s.to_ascii_lowercase()))
58+
}
59+
}
60+
61+
impl TryFrom<&str> for GitSha {
62+
type Error = GitShaError;
63+
64+
fn try_from(s: &str) -> Result<Self, Self::Error> {
65+
s.parse()
66+
}
67+
}
68+
69+
impl TryFrom<String> for GitSha {
70+
type Error = GitShaError;
71+
72+
fn try_from(s: String) -> Result<Self, Self::Error> {
73+
s.parse()
74+
}
75+
}
76+
77+
impl std::fmt::Display for GitSha {
78+
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
79+
f.write_str(&self.0)
80+
}
81+
}
82+
83+
impl AsRef<str> for GitSha {
84+
fn as_ref(&self) -> &str {
85+
&self.0
86+
}
87+
}
88+
89+
impl serde::Serialize for GitSha {
90+
fn serialize<S: serde::Serializer>(&self, serializer: S) -> Result<S::Ok, S::Error> {
91+
serializer.serialize_str(&self.0)
92+
}
93+
}
94+
95+
impl<'de> serde::Deserialize<'de> for GitSha {
96+
fn deserialize<D: serde::Deserializer<'de>>(deserializer: D) -> Result<Self, D::Error> {
97+
let s = String::deserialize(deserializer)?;
98+
s.parse().map_err(serde::de::Error::custom)
99+
}
100+
}
101+
10102
/// A path that is not a git repository.
11103
#[derive(Debug, thiserror::Error)]
12104
#[error("`{path}` is not a git repository")]
@@ -95,11 +187,12 @@ impl<'r, 'g, 'bin> GitBranch<'r, 'g, 'bin> {
95187
self.name.as_bstr()
96188
}
97189

98-
/// The commit the branch points at, as a hex object id. `None` if git fails.
190+
/// The commit the branch points at. `None` if git fails or its output is
191+
/// not a valid object id.
99192
#[tracing::instrument(skip(self))]
100-
pub fn head_commit(&self) -> Option<BString> {
193+
pub fn head_commit(&self) -> Option<GitSha> {
101194
let name = self.name.to_str().ok()?;
102-
self.repo.run(&["rev-parse", name])
195+
self.repo.run(&["rev-parse", name])?.to_str().ok()?.parse().ok()
103196
}
104197
}
105198

@@ -182,6 +275,66 @@ mod tests {
182275
use super::*;
183276
use rstest::rstest;
184277

278+
#[rstest]
279+
#[case::sha1_lower(
280+
"0123456789abcdef0123456789abcdef01234567",
281+
"0123456789abcdef0123456789abcdef01234567"
282+
)]
283+
#[case::sha1_upper(
284+
"0123456789ABCDEF0123456789ABCDEF01234567",
285+
"0123456789abcdef0123456789abcdef01234567"
286+
)]
287+
#[case::sha256(
288+
"ABCDEF0123456789abcdef0123456789ABCDEF0123456789abcdef0123456789",
289+
"abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789"
290+
)]
291+
fn parses_and_lowercases(#[case] input: &str, #[case] expected: &str) {
292+
let sha: GitSha = input.parse().unwrap();
293+
assert_eq!(sha.as_str(), expected);
294+
}
295+
296+
#[rstest]
297+
#[case::empty(0)]
298+
#[case::short(39)]
299+
#[case::between(41)]
300+
#[case::over(65)]
301+
fn rejects_wrong_length(#[case] len: usize) {
302+
assert_eq!("a".repeat(len).parse::<GitSha>(), Err(GitShaError::BadLength(len)));
303+
}
304+
305+
#[rstest]
306+
fn rejects_non_hex() {
307+
let with_g = format!("{}g", "a".repeat(39));
308+
assert_eq!(with_g.parse::<GitSha>(), Err(GitShaError::NotHex));
309+
}
310+
311+
#[rstest]
312+
#[case::sha1(40)]
313+
#[case::sha256(64)]
314+
fn zeros_are_the_null_oid(#[case] len: usize) {
315+
let sha: GitSha = "0".repeat(len).parse().unwrap();
316+
assert!(sha.is_zero());
317+
}
318+
319+
#[rstest]
320+
fn non_zero_is_not_the_null_oid() {
321+
let sha: GitSha = "0123456789abcdef0123456789abcdef01234567".parse().unwrap();
322+
assert!(!sha.is_zero());
323+
}
324+
325+
#[rstest]
326+
fn serde_round_trips_as_a_bare_string() {
327+
let sha: GitSha = "0123456789abcdef0123456789abcdef01234567".parse().unwrap();
328+
let json = serde_json::to_string(&sha).unwrap();
329+
assert_eq!(json, "\"0123456789abcdef0123456789abcdef01234567\"");
330+
assert_eq!(serde_json::from_str::<GitSha>(&json).unwrap(), sha);
331+
}
332+
333+
#[rstest]
334+
fn deserialize_rejects_an_invalid_digest() {
335+
assert!(serde_json::from_str::<GitSha>("\"not-a-sha\"").is_err());
336+
}
337+
185338
#[rstest]
186339
#[case::https("https://github.com/acme/web.git", Some("acme/web"))]
187340
#[case::https_no_suffix("https://github.com/acme/web", Some("acme/web"))]
@@ -251,7 +404,7 @@ mod tests {
251404

252405
let branch = repo.current_branch().unwrap();
253406
assert_eq!(branch.name(), "main");
254-
assert_eq!(branch.head_commit().unwrap().len(), 40);
407+
assert_eq!(branch.head_commit().unwrap().as_str().len(), 40);
255408

256409
let remote = repo.remote("origin").unwrap();
257410
assert_eq!(remote.name(), "origin");

‎crates/hm-core/src/exec/cloud/backend.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ impl ExecutionBackend for CloudBackend {
104104
org: self.org.clone(),
105105
pipeline: slug,
106106
branch: req.source.branch.clone(),
107-
commit: req.source.commit.clone(),
107+
commit: req.source.commit.to_string(),
108108
message: req.source.message.clone(),
109109
pipeline_ir: req.plan.ir_json.clone(), // verbatim
110110
source_tgz,
@@ -126,7 +126,7 @@ impl ExecutionBackend for CloudBackend {
126126
repo_name,
127127
source_slug: req.pipeline_slug.clone(),
128128
branch: req.source.branch.clone(),
129-
commit: req.source.commit.clone(),
129+
commit: req.source.commit.to_string(),
130130
message: req.source.message.clone(),
131131
pipeline_ir: req.plan.ir_json.clone(), // verbatim
132132
source_tgz,

‎crates/hm-core/src/exec/request.rs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ use std::collections::BTreeMap;
44
use std::path::PathBuf;
55
use std::time::Duration;
66

7+
use hm_common::git::GitSha;
78
use hm_pipeline_ir::PipelineGraph;
89
use hm_plugin_protocol::events::PlanSummary;
910

@@ -61,7 +62,7 @@ fn summarize(graph: &PipelineGraph) -> PlanSummary {
6162
#[derive(Debug, Clone)]
6263
pub struct SourceMeta {
6364
pub branch: String,
64-
pub commit: String,
65+
pub commit: GitSha,
6566
pub message: Option<String>,
6667
/// `owner/repo` from the worktree's git remote, when one exists. `None` for
6768
/// a remoteless worktree; the cloud backend requires it to resolve the

‎crates/hm-core/tests/backend_contract.rs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
)]
99

1010
use futures::StreamExt;
11+
use hm_common::git::GitSha;
1112
use hm_core::exec::*;
1213
use hm_plugin_protocol::events::{BuildEvent, BuildRef};
1314
use hm_plugin_protocol::ir::DurationMs;
@@ -136,7 +137,7 @@ fn fake_request() -> RunRequest {
136137
env: Default::default(),
137138
source: SourceMeta {
138139
branch: "main".into(),
139-
commit: "0".repeat(40),
140+
commit: GitSha::zero(),
140141
message: None,
141142
repo_name: None,
142143
},

‎crates/hm/src/commands/cloud/verbs/run.rs‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ use std::collections::BTreeMap;
1212
use anyhow::Result;
1313
use clap::Parser;
1414
use harmont_cloud::builds::NewBuild;
15+
use hm_common::git::GitSha;
1516
use hm_core::app_ctx::AppCtx;
1617

1718
use crate::commands::cloud::settings;
@@ -24,12 +25,8 @@ pub struct RunArgs {
2425
#[arg(short, long, default_value = "main")]
2526
pub branch: String,
2627
/// Commit SHA to record on the build.
27-
#[arg(
28-
short,
29-
long,
30-
default_value = "0000000000000000000000000000000000000000"
31-
)]
32-
pub commit: String,
28+
#[arg(short, long, default_value_t = GitSha::zero())]
29+
pub commit: GitSha,
3330
/// Build message.
3431
#[arg(short, long)]
3532
pub message: Option<String>,
@@ -57,7 +54,7 @@ pub(crate) async fn run(env: &BTreeMap<String, String>, args: RunArgs, app: &App
5754
org: org.clone(),
5855
pipeline: args.pipeline.clone(),
5956
branch: args.branch.clone(),
60-
commit: args.commit.clone(),
57+
commit: args.commit.to_string(),
6158
message: args.message.clone(),
6259
pipeline_ir,
6360
// Full worktree archiving lands in `hm run --cloud`.

‎crates/hm/src/commands/run/mod.rs‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use std::collections::HashMap;
33
use anyhow::{Context, Result};
44

55
use bstr::ByteSlice as _;
6-
use hm_common::git::{GitBranch, GitRemote, GitRepo};
6+
use hm_common::git::{GitBranch, GitRemote, GitRepo, GitSha};
77
use hm_core::app_ctx::AppCtx;
88
use hm_core::config::domain::BackendConfig;
99
use hm_dsl_engine::{DslEngine, detect};
@@ -178,9 +178,7 @@ pub async fn handle(args: RunArgs, ctx: RunContext<'_>) -> Result<i32> {
178178
let commit = head
179179
.as_ref()
180180
.and_then(GitBranch::head_commit)
181-
.map(|c| c.to_str_lossy().into_owned())
182-
.filter(|s| !s.is_empty())
183-
.unwrap_or_else(|| "0".repeat(40));
181+
.unwrap_or_else(GitSha::zero);
184182
let repo_name = remote
185183
.as_ref()
186184
.and_then(GitRemote::gh_repo_name)

0 commit comments

Comments
 (0)