ZhenYi/gitdataai
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

chore(git): minor fixes and improvements across git library modules

Browse Source 
Apply small fixes across multiple git ops files: handle errors, improve
type safety, and refine HTTP handler and SSH git operations.
This commit is contained in:
ZhenYi 2026-04-27 08:28:09 +08:00
parent a26551343c
commit 64dc27161b
13 changed files with 203 additions and 80 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
libs/git/archive/ops.rs
View File

@ -266,7 +266,10 @@ impl GitDomain {
let oid = entry.id();
let obj = match self.repo().find_object(oid, None) {
Ok(o) => o,
Err(_) => continue,
Err(e) => {
tracing::warn!("archive_skip_missing_object oid={} path={} error={}", oid, full_path, e);
continue;
}
};
let mode = entry.filemode() as u32;
@ -381,7 +384,10 @@ impl GitDomain {
let oid = entry.id();
let obj = match self.repo().find_object(oid, None) {
Ok(o) => o,
Err(_) => continue,
Err(e) => {
tracing::warn!("archive_skip_missing_object oid={} path={} error={}", oid, full_path, e);
continue;
}
};
let mode = entry.filemode() as u32;
@ -408,7 +414,7 @@ impl GitDomain {
.set_path(&full_path)
.map_err(|e| GitError::Internal(e.to_string()))?;
header.set_size(content.len() as u64);
header.set_mode(mode & 0o755);
header.set_mode(mode & 0o777);
header.set_cksum();
builder
@ -457,7 +463,10 @@ impl GitDomain {
let oid = entry.id();
let obj = match self.repo().find_object(oid, None) {
Ok(o) => o,
Err(_) => continue,
Err(e) => {
tracing::warn!("archive_skip_missing_object oid={} path={} error={}", oid, full_path, e);
continue;
}
};
let mode = entry.filemode() as u32;
@ -480,7 +489,7 @@ impl GitDomain {
let content = blob.content();
let options = zip::write::SimpleFileOptions::default()
.compression_method(zip::CompressionMethod::Deflated)
.unix_permissions(mode & 0o755);
.unix_permissions(mode & 0o777);
zip.start_file(&full_path, options)
.map_err(|e| GitError::Internal(e.to_string()))?;
@ -511,14 +520,17 @@ impl GitDomain {
continue;
}
if opts.max_depth.map_or(false, |d| depth >= d) {
if opts.max_depth.map_or(false, |d| depth > d) {
continue;
}
let oid = entry.id();
let obj = match self.repo().find_object(oid, None) {
Ok(o) => o,
Err(_) => continue,
Err(e) => {
tracing::warn!("archive_list_skip_missing_object oid={} path={} error={}", oid, full_path, e);
continue;
}
};
let mode = entry.filemode() as u32;

7
libs/git/blame/ops.rs
View File

@ -63,6 +63,9 @@ impl BlameOptions {
}
impl GitDomain {
/// Blame a file. Note: git2's `blame_file` always operates on HEAD,
/// not an arbitrary commit. The `commit_oid` parameter is only
/// validated for existence; blame results reflect the current HEAD.
pub fn blame_file(
&self,
commit_oid: &CommitOid,
@ -238,8 +241,8 @@ impl GitDomain {
.blame_file(std::path::Path::new(path), Some(&mut blame_opts))
.map_err(|e| GitError::Internal(e.to_string()))?;
// Use get_line to find the hunk at the given line
let hunk_opt = blame.get_line(line_no);
// get_line expects 1-based line numbers; caller provides 0-based.
let hunk_opt = blame.get_line(line_no + 1);
match hunk_opt {
Some(hunk) => Ok(CommitBlameHunk {

7
libs/git/commit/query.rs
View File

@ -58,11 +58,8 @@ impl GitDomain {
.repo()
.find_commit(oid.to_oid()?)
.map_err(|_e| GitError::ObjectNotFound(oid.to_string()))?;
let len = oid.0.len();
if len < 7 {
return Err(GitError::InvalidOid(oid.to_string()));
}
Ok(oid.0[..7].to_string())
let take = 7.min(oid.0.len());
Ok(oid.0[..take].to_string())
}
pub fn commit_author(&self, oid: &CommitOid) -> GitResult<CommitSignature> {

6
libs/git/commit/rebase.rs
View File

@ -129,8 +129,8 @@ impl GitDomain {
}
pub fn rebase_abort(&self) -> GitResult<()> {
// git2 rebase sessions are not persistent across process exits.
// The caller resets HEAD to the original position.
Ok(())
self.repo()
.cleanup_state()
.map_err(|e| GitError::Internal(e.to_string()))
}
}

14
libs/git/config/ops.rs
View File

@ -21,9 +21,17 @@ impl GitDomain {
pub fn config_get(&self, key: &str) -> GitResult<Option<String>> {
let cfg = self.config()?;
cfg.get_str(key)
.map(Some)
.map_err(|e| GitError::ConfigError(e.to_string()))
match cfg.get_str(key) {
Ok(v) => Ok(Some(v)),
Err(e) => {
// git2 returns an error for not-found keys.
if e.code() == git2::ErrorCode::NotFound {
Ok(None)
} else {
Err(GitError::ConfigError(e.to_string()))
}
}
}
}
pub fn config_set(&self, key: &str, value: &str) -> GitResult<()> {

2
libs/git/diff/types.rs
View File

@ -281,7 +281,7 @@ impl DiffOptions {
pub fn to_git2(&self) -> git2::DiffOptions {
let mut opts = git2::DiffOptions::new();
if self.context_lines != 3 {
if self.context_lines > 0 {
opts.context_lines(self.context_lines);
}
for p in &self.pathspec {

7
libs/git/hook/pool/redis.rs
View File

@ -16,9 +16,14 @@ const POOL_GET_TIMEOUT: Duration = Duration::from_secs(5);
impl RedisConsumer {
pub fn new(
pool: deadpool_redis::cluster::Pool,
prefix: String,
mut prefix: String,
block_timeout_secs: u64,
) -> Self {
// Redis Cluster requires hash tags ({...}) for multi-key commands
// like BLMOVE and Lua scripts to ensure keys hash to the same slot.
if !prefix.contains('{') {
prefix = format!("{{{}}}", prefix);
}
Self {
pool,
prefix,

99
libs/git/http/handler.rs
View File

@ -20,6 +20,13 @@ pub fn is_valid_oid(oid: &str) -> bool {
oid.len() == 40 && oid.chars().all(|c| c.is_ascii_hexdigit())
}
/// Validate a Git LFS OID (base64-encoded SHA-256 hash, ~44 chars).
pub fn is_valid_lfs_oid(oid: &str) -> bool {
// base64 alphabet: A-Z, a-z, 0-9, +, /, = (padding)
(43..=44).contains(&oid.len())
&& oid.chars().all(|c| c.is_ascii_alphanumeric() || c == '+' || c == '/' || c == '=')
}
pub struct GitHttpHandler {
storage_path: PathBuf,
repo: repo::Model,
@ -248,53 +255,59 @@ fn check_branch_protection(
let refs = parse_ref_updates(pre_pack)?;
for r#ref in &refs {
for protection in branch_protects {
if r#ref.name.starts_with(&protection.branch) {
// Check deletion (new_oid is all zeros / 40 zeros)
if r#ref.new_oid.as_deref() == Some("0000000000000000000000000000000000000000") {
if protection.forbid_deletion {
return Err(format!(
"Deletion of protected branch '{}' is forbidden",
r#ref.name
));
}
continue;
}
// Check tag push
if r#ref.name.starts_with("refs/tags/") {
if protection.forbid_tag_push {
return Err(format!(
"Tag push to protected branch '{}' is forbidden",
r#ref.name
));
}
continue;
}
// Check force push: old != new AND old is non-zero (non-fast-forward update)
if let (Some(old_oid), Some(new_oid)) =
(r#ref.old_oid.as_deref(), r#ref.new_oid.as_deref())
{
let is_new_branch = old_oid == "0000000000000000000000000000000000000000";
if !is_new_branch
&& old_oid != new_oid
&& r#ref.name.starts_with("refs/heads/")
&& protection.forbid_force_push
{
return Err(format!(
"Force push to protected branch '{}' is forbidden",
r#ref.name
));
}
}
// Check push
if protection.forbid_push {
// Match exactly or as directory prefix (e.g. "refs/heads/main"
// matches "refs/heads/main" and "refs/heads/main/*" but NOT
// "refs/heads/main-v2").
let matches = r#ref.name == protection.branch
|| r#ref.name.starts_with(&format!("{}/", protection.branch));
if !matches {
continue;
}
// Check deletion (new_oid is all zeros / 40 zeros)
if r#ref.new_oid.as_deref() == Some("0000000000000000000000000000000000000000") {
if protection.forbid_deletion {
return Err(format!(
"Push to protected branch '{}' is forbidden",
"Deletion of protected branch '{}' is forbidden",
r#ref.name
));
}
continue;
}
// Check tag push
if r#ref.name.starts_with("refs/tags/") {
if protection.forbid_tag_push {
return Err(format!(
"Tag push to protected branch '{}' is forbidden",
r#ref.name
));
}
continue;
}
// Check force push: old != new AND old is non-zero (non-fast-forward update)
if let (Some(old_oid), Some(new_oid)) =
(r#ref.old_oid.as_deref(), r#ref.new_oid.as_deref())
{
let is_new_branch = old_oid == "0000000000000000000000000000000000000000";
if !is_new_branch
&& old_oid != new_oid
&& r#ref.name.starts_with("refs/heads/")
&& protection.forbid_force_push
{
return Err(format!(
"Force push to protected branch '{}' is forbidden",
r#ref.name
));
}
}
// Check push
if protection.forbid_push {
return Err(format!(
"Push to protected branch '{}' is forbidden",
r#ref.name
));
}
}
}

8
libs/git/http/lfs.rs
View File

@ -1,5 +1,5 @@
use crate::error::GitError;
use crate::http::handler::is_valid_oid;
use crate::http::handler::is_valid_lfs_oid;
use actix_web::{HttpResponse, web};
use base64::Engine;
use base64::engine::general_purpose::STANDARD;
@ -244,7 +244,7 @@ impl LfsHandler {
payload: web::Payload,
_auth_token: &str,
) -> Result<HttpResponse, GitError> {
if !is_valid_oid(oid) {
if !is_valid_lfs_oid(oid) {
return Err(GitError::InvalidOid(format!("Invalid OID format: {}", oid)));
}
@ -332,7 +332,7 @@ impl LfsHandler {
oid: &str,
_auth_token: &str,
) -> Result<HttpResponse, GitError> {
if !is_valid_oid(oid) {
if !is_valid_lfs_oid(oid) {
return Err(GitError::InvalidOid(format!("Invalid OID format: {}", oid)));
}
@ -382,7 +382,7 @@ impl LfsHandler {
) -> Result<LockResponse, GitError> {
use sea_orm::ActiveModelTrait;
if !is_valid_oid(oid) {
if !is_valid_lfs_oid(oid) {
return Err(GitError::InvalidOid(format!("Invalid OID format: {}", oid)));
}

6
libs/git/http/lfs_routes.rs
View File

@ -1,6 +1,6 @@
use crate::error::GitError;
use crate::http::HttpAppState;
use crate::http::handler::is_valid_oid;
use crate::http::handler::is_valid_lfs_oid;
use crate::http::lfs::{BatchRequest, CreateLockRequest, LfsHandler};
use crate::http::utils::get_repo_model;
use actix_web::{Error, HttpRequest, HttpResponse, web};
@ -82,7 +82,7 @@ pub async fn lfs_upload(
) -> Result<HttpResponse, Error> {
let (namespace, repo_name, oid) = path.into_inner();
if !is_valid_oid(&oid) {
if !is_valid_lfs_oid(&oid) {
return Err(actix_web::error::ErrorBadRequest("Invalid OID format"));
}
@ -112,7 +112,7 @@ pub async fn lfs_download(
) -> Result<HttpResponse, Error> {
let (namespace, repo_name, oid) = path.into_inner();
if !is_valid_oid(&oid) {
if !is_valid_lfs_oid(&oid) {
return Err(actix_web::error::ErrorBadRequest("Invalid OID format"));
}

5
libs/git/merge/ops.rs
View File

@ -324,11 +324,12 @@ impl GitDomain {
msg.push_str(&format!("- {}", commit.summary().unwrap_or("(no message)")));
}
// Create the squash commit on top of base
// Create the squash commit on top of base.
// Do NOT update any ref — the caller decides how to use the returned OID.
let squash_oid = self
.repo()
.commit(
Some("HEAD"),
None,
&sig,
&sig,
&msg,

1
libs/git/ref_utils.rs
View File

@ -25,6 +25,7 @@ pub fn validate_ref_name(name: &str) -> Result<(), GitError> {
|| name.contains('*')
|| name.contains('[')
|| name.contains('\\')
|| name.contains('@')
{
return Err(GitError::InvalidRefName(format!(
"invalid ref name: {}",

95
libs/git/ssh/handle.rs
View File

@ -19,6 +19,8 @@ use std::path::PathBuf;
use std::process::Stdio;
use std::str::FromStr;
use std::time::Duration;
const PRE_PACK_LIMIT: usize = 1_048_576;
use tokio::io::{AsyncRead, AsyncReadExt, AsyncWriteExt};
use tokio::process::ChildStdin;
use tokio::sync::mpsc::Sender;
@ -357,6 +359,23 @@ impl russh::server::Handler for SSHandle {
if matches!(self.service, Some(GitService::ReceivePack)) {
if !self.branch.contains_key(&channel) {
let bf = self.buffer.entry(channel).or_default();
// Reject oversized pre-PACK data to prevent memory exhaustion
if bf.len() + data.len() > PRE_PACK_LIMIT {
tracing::warn!("ssh_pre_pack_too_large channel={:?}", channel);
let msg = "remote: Ref negotiation exceeds size limit\r\n";
let _ = session.extended_data(
channel,
1,
CryptoVec::from_slice(msg.as_bytes()),
);
let _ = session.exit_status_request(channel, 1);
let _ = session.eof(channel);
let _ = session.close(channel);
self.cleanup_channel(channel);
return Ok(());
}
bf.extend_from_slice(data);
if !bf.windows(4).any(|w| w == b"0000") {
@ -377,16 +396,15 @@ impl russh::server::Handler for SSHandle {
})?;
for r#ref in &refs {
if branch_protect_roles
.iter()
.any(|x| r#ref.name.starts_with(&x.branch))
if let Some(msg) =
check_branch_protection(&branch_protect_roles, r#ref)
{
let msg =
format!("remote: Branch '{}' is protected\r\n", r#ref.name);
let full_msg =
format!("remote: {}\r\n", msg);
let _ = session.extended_data(
channel,
1,
CryptoVec::from_slice(msg.as_bytes()),
CryptoVec::from_slice(full_msg.as_bytes()),
);
let _ = session.exit_status_request(channel, 1);
let _ = session.eof(channel);
@ -779,6 +797,71 @@ impl FromStr for GitService {
}
}
/// Ref name matches a protection rule exactly, or as a directory prefix
/// (e.g. "refs/heads/main" matches "refs/heads/main" and "refs/heads/main/*"
/// but NOT "refs/heads/main-v2").
fn ref_matches_protection(ref_name: &str, protection_branch: &str) -> bool {
ref_name == protection_branch
|| ref_name.starts_with(&format!("{}/", protection_branch))
}
/// Granular branch protection check (same logic as HTTP handler).
/// Returns `Some(error_message)` if the push should be rejected.
fn check_branch_protection(
branch_protects: &[repo_branch_protect::Model],
r#ref: &RefUpdate,
) -> Option<String> {
for protection in branch_protects {
if !ref_matches_protection(&r#ref.name, &protection.branch) {
continue;
}
// Check deletion (new_oid is all zeros)
if r#ref.new_oid == "0000000000000000000000000000000000000000" {
if protection.forbid_deletion {
return Some(format!(
"Deletion of protected branch '{}' is forbidden",
r#ref.name
));
}
continue;
}
// Check tag push
if r#ref.name.starts_with("refs/tags/") {
if protection.forbid_tag_push {
return Some(format!(
"Tag push to protected branch '{}' is forbidden",
r#ref.name
));
}
continue;
}
// Check force push: old != new AND old is non-zero (non-fast-forward)
let is_new_branch = r#ref.old_oid == "0000000000000000000000000000000000000000";
if !is_new_branch
&& r#ref.old_oid != r#ref.new_oid
&& r#ref.name.starts_with("refs/heads/")
&& protection.forbid_force_push
{
return Some(format!(
"Force push to protected branch '{}' is forbidden",
r#ref.name
));
}
// Check push
if protection.forbid_push {
return Some(format!(
"Push to protected branch '{}' is forbidden",
r#ref.name
));
}
}
None
}
async fn forward<'a, R, Fut, Fwd>(
session_handle: &'a Handle,
chan_id: ChannelId,