fix(hooks): shell-escape script values and use glob iteration

Add shell_escape() to safely embed values in generated shell scripts.
Replace '$(ls ...)' anti-pattern with glob iteration to correctly
handle filenames containing spaces.
This commit is contained in:
zhenyi
2026-06-10 18:32:05 +08:00
parent 45c00b2dee
commit c9c1a739fd
+54 -10
View File
@@ -76,23 +76,51 @@ pub fn run_hook_dir(
} }
scripts.sort(); scripts.sort();
let dir_start = std::time::Instant::now();
for script in &scripts { for script in &scripts {
tracing::debug!( tracing::debug!(
hook_type = %hook_type, hook_type = %hook_type,
script = %script.display(), script = %script.display(),
"executing hook script" "executing hook script"
); );
let script_start = std::time::Instant::now();
let result = run_single_script(script, stdin_data, timeout); let result = run_single_script(script, stdin_data, timeout);
let script_elapsed = script_start.elapsed();
crate::metrics::record_hook_execution(
hook_type,
if result.accepted { "ok" } else { "rejected" },
script_elapsed,
);
if !result.accepted { if !result.accepted {
tracing::warn!( tracing::warn!(
hook_type = %hook_type, hook_type = %hook_type,
script = %script.display(), script = %script.display(),
exit_code = result.exit_code, exit_code = result.exit_code,
stderr = %result.stderr, stderr = %result.stderr,
elapsed_ms = script_elapsed.as_millis() as u64,
"hook script rejected" "hook script rejected"
); );
return result; return result;
} }
tracing::debug!(
hook_type = %hook_type,
script = %script.display(),
elapsed_ms = script_elapsed.as_millis() as u64,
"hook script passed"
);
}
let dir_elapsed = dir_start.elapsed();
if !scripts.is_empty() {
tracing::info!(
hook_type = %hook_type,
script_count = scripts.len(),
elapsed_ms = dir_elapsed.as_millis() as u64,
"hook dir completed"
);
} }
HookResult::accepted() HookResult::accepted()
@@ -190,6 +218,12 @@ impl ChildWaitTimeout for std::process::Child {
} }
} }
/// Shell-escape a value by wrapping in single quotes.
/// Any embedded single quotes are escaped as `'\''`.
fn shell_escape(value: &str) -> String {
format!("'{}'", value.replace('\'', "'\\''"))
}
/// Generate the gitks hook runner script content. /// Generate the gitks hook runner script content.
/// This script is installed into each repository's hooks/ directory /// This script is installed into each repository's hooks/ directory
/// and orchestrates the execution of server hooks, custom hooks, and gRPC callbacks. /// and orchestrates the execution of server hooks, custom hooks, and gRPC callbacks.
@@ -201,16 +235,20 @@ pub fn generate_hook_runner_script(
hook_timeout_secs: u64, hook_timeout_secs: u64,
) -> String { ) -> String {
let server_hooks_section = if let Some(dir) = server_hooks_dir { let server_hooks_section = if let Some(dir) = server_hooks_dir {
let escaped_dir = shell_escape(dir);
let escaped_hook_type = shell_escape(hook_type);
format!( format!(
r#" r#"
# Run server hooks # Run server hooks
SERVER_HOOKS_DIR="{dir}/{hook_type}.d" SERVER_HOOKS_DIR={escaped_dir}/{escaped_hook_type}.d
if [ -d "$SERVER_HOOKS_DIR" ]; then if [ -d "$SERVER_HOOKS_DIR" ]; then
for script in $(ls "$SERVER_HOOKS_DIR" | sort); do for script in "$SERVER_HOOKS_DIR"/*; do
[ -f "$script" ] || continue
base=$(basename "$script")
skip=false skip=false
case "$script" in .*|*.sample|*~) skip=true;; esac case "$base" in .*|*.sample|*~) skip=true;; esac
if [ "$skip" = "false" ]; then if [ "$skip" = "false" ]; then
"$SERVER_HOOKS_DIR/$script" "$script"
exit_code=$? exit_code=$?
if [ $exit_code -ne 0 ]; then if [ $exit_code -ne 0 ]; then
exit $exit_code exit $exit_code
@@ -228,11 +266,13 @@ fi
# Run custom hooks (per-repository) # Run custom hooks (per-repository)
CUSTOM_HOOKS_DIR="$GIT_DIR/custom_hooks/$GITKS_HOOK_TYPE.d" CUSTOM_HOOKS_DIR="$GIT_DIR/custom_hooks/$GITKS_HOOK_TYPE.d"
if [ -d "$CUSTOM_HOOKS_DIR" ]; then if [ -d "$CUSTOM_HOOKS_DIR" ]; then
for script in $(ls "$CUSTOM_HOOKS_DIR" | sort); do for script in "$CUSTOM_HOOKS_DIR"/*; do
[ -f "$script" ] || continue
base=$(basename "$script")
skip=false skip=false
case "$script" in .*|*.sample|*~) skip=true;; esac case "$base" in .*|*.sample|*~) skip=true;; esac
if [ "$skip" = "false" ]; then if [ "$skip" = "false" ]; then
"$CUSTOM_HOOKS_DIR/$script" "$script"
exit_code=$? exit_code=$?
if [ $exit_code -ne 0 ]; then if [ $exit_code -ne 0 ]; then
exit $exit_code exit $exit_code
@@ -243,10 +283,11 @@ fi
"#; "#;
let grpc_callback_section = if let Some(addr) = hook_callback_addr { let grpc_callback_section = if let Some(addr) = hook_callback_addr {
let escaped_addr = shell_escape(addr);
format!( format!(
r#" r#"
# gRPC callback to external HookService # gRPC callback to external HookService
if [ -n "{addr}" ]; then if [ -n {escaped_addr} ]; then
# gRPC callback is handled by the gitks service directly # gRPC callback is handled by the gitks service directly
# The service will make the gRPC call after the git hook completes # The service will make the gRPC call after the git hook completes
# This section is a placeholder - actual gRPC callback is handled # This section is a placeholder - actual gRPC callback is handled
@@ -258,14 +299,17 @@ fi
String::new() String::new()
}; };
let escaped_hook_type = shell_escape(hook_type);
let escaped_repo_path = shell_escape(repo_relative_path);
format!( format!(
r#"#!/bin/sh r#"#!/bin/sh
# gitks hook runner for {hook_type} # gitks hook runner for {hook_type}
# Repository: {repo_relative_path} # Repository: {repo_relative_path}
# Auto-generated by gitks - do not modify manually # Auto-generated by gitks - do not modify manually
GITKS_HOOK_TYPE="{hook_type}" GITKS_HOOK_TYPE={escaped_hook_type}
GITKS_REPO_RELATIVE_PATH="{repo_relative_path}" GITKS_REPO_RELATIVE_PATH={escaped_repo_path}
GITKS_HOOK_TIMEOUT="{hook_timeout_secs}" GITKS_HOOK_TIMEOUT="{hook_timeout_secs}"
{server_hooks_section} {server_hooks_section}