Merge commit from fork
This commit is contained in:
@@ -6,6 +6,7 @@ use std::process::Command;
|
||||
use crate::session::{build_session, SessionBuilderConfig};
|
||||
|
||||
use goose::checks::{discover, DiscoveredReview};
|
||||
use goose::subprocess::git_command;
|
||||
|
||||
use super::orchestrator::{
|
||||
emit_findings, run_checks_in_parallel, run_main_pass_in_parallel, Severity,
|
||||
@@ -312,7 +313,7 @@ fn print_discovered_summary(d: &DiscoveredReview) {
|
||||
}
|
||||
|
||||
fn find_repo_root() -> Result<PathBuf> {
|
||||
let out = Command::new("git")
|
||||
let out = git_command()
|
||||
.args(["rev-parse", "--show-toplevel"])
|
||||
.output()
|
||||
.context("failed to invoke git")?;
|
||||
@@ -331,15 +332,15 @@ fn find_repo_root() -> Result<PathBuf> {
|
||||
/// C-style escapes (`"dir/\303\251.txt"`), which downstream parsers
|
||||
/// would have to round-trip-decode just to spell the filename. We turn
|
||||
/// it off everywhere so callers always get clean UTF-8 paths.
|
||||
fn git_command(repo_root: &Path) -> Command {
|
||||
let mut cmd = Command::new("git");
|
||||
fn review_git_command(repo_root: &Path) -> Command {
|
||||
let mut cmd = git_command();
|
||||
cmd.current_dir(repo_root)
|
||||
.args(["-c", "core.quotePath=off"]);
|
||||
cmd
|
||||
}
|
||||
|
||||
fn touched_files(repo_root: &Path, range: Option<&str>, files: &[String]) -> Result<Vec<String>> {
|
||||
let mut cmd = git_command(repo_root);
|
||||
let mut cmd = review_git_command(repo_root);
|
||||
cmd.arg("diff").arg("--name-only");
|
||||
match range {
|
||||
Some(r) => {
|
||||
@@ -370,7 +371,7 @@ fn touched_files(repo_root: &Path, range: Option<&str>, files: &[String]) -> Res
|
||||
}
|
||||
|
||||
fn collect_diff(repo_root: &Path, range: Option<&str>, files: &[String]) -> Result<String> {
|
||||
let mut cmd = git_command(repo_root);
|
||||
let mut cmd = review_git_command(repo_root);
|
||||
cmd.arg("diff");
|
||||
match range {
|
||||
Some(r) => {
|
||||
@@ -394,7 +395,7 @@ fn collect_diff(repo_root: &Path, range: Option<&str>, files: &[String]) -> Resu
|
||||
}
|
||||
|
||||
fn collect_diff_stat(repo_root: &Path, range: Option<&str>, files: &[String]) -> Result<String> {
|
||||
let mut cmd = git_command(repo_root);
|
||||
let mut cmd = review_git_command(repo_root);
|
||||
cmd.arg("diff").arg("--stat");
|
||||
match range {
|
||||
Some(r) => {
|
||||
@@ -425,7 +426,7 @@ fn collect_diff_stat(repo_root: &Path, range: Option<&str>, files: &[String]) ->
|
||||
/// brand-new files to the review when no `--range` is given (default
|
||||
/// `git diff HEAD` would silently drop them).
|
||||
fn untracked_files(repo_root: &Path, files: &[String]) -> Result<Vec<String>> {
|
||||
let mut cmd = git_command(repo_root);
|
||||
let mut cmd = review_git_command(repo_root);
|
||||
cmd.args(["ls-files", "--others", "--exclude-standard"]);
|
||||
if !files.is_empty() {
|
||||
cmd.arg("--");
|
||||
|
||||
@@ -5,7 +5,7 @@ use goose::recipe::RECIPE_FILE_EXTENSIONS;
|
||||
use serde::{Deserialize, Serialize};
|
||||
|
||||
use goose::recipe::read_recipe_file_content::RecipeFile;
|
||||
use goose::subprocess::SubprocessExt;
|
||||
use goose::subprocess::{git_command, SubprocessExt};
|
||||
use std::env;
|
||||
use std::fs;
|
||||
|
||||
@@ -190,7 +190,7 @@ fn ensure_repo_cloned(recipe_repo_full_name: &str) -> Result<PathBuf> {
|
||||
|
||||
fn fetch_origin(local_repo_path: &Path) -> Result<()> {
|
||||
let error_message: String = format!("Failed to fetch at {}", local_repo_path.to_str().unwrap());
|
||||
let status = Command::new("git")
|
||||
let status = git_command()
|
||||
.args(["fetch", "origin"])
|
||||
.current_dir(local_repo_path)
|
||||
.set_no_window()
|
||||
@@ -213,7 +213,7 @@ fn get_folder_from_github(local_repo_path: &Path, recipe_name: &str) -> Result<P
|
||||
}
|
||||
fs::create_dir_all(&output_dir)?;
|
||||
|
||||
let archive_output = Command::new("git")
|
||||
let archive_output = git_command()
|
||||
.args(["archive", &ref_and_path])
|
||||
.current_dir(local_repo_path)
|
||||
.stdout(Stdio::piped())
|
||||
|
||||
@@ -3,14 +3,13 @@ pub mod formats;
|
||||
pub mod mcp_servers;
|
||||
|
||||
use crate::config::paths::Paths;
|
||||
use crate::subprocess::SubprocessExt;
|
||||
use crate::subprocess::{git_command, SubprocessExt};
|
||||
use anyhow::{anyhow, bail, Result};
|
||||
use chrono::{DateTime, Duration, Utc};
|
||||
use fs_err as fs;
|
||||
use serde::{Deserialize, Serialize};
|
||||
use std::collections::HashSet;
|
||||
use std::path::{Path, PathBuf};
|
||||
use std::process::Command;
|
||||
use tracing::warn;
|
||||
|
||||
const INSTALL_METADATA: &str = ".goose-plugin-install.json";
|
||||
@@ -290,7 +289,7 @@ fn install_from_checkout_at_root(
|
||||
}
|
||||
|
||||
fn clone_git_repo(source: &str, destination: &Path) -> Result<()> {
|
||||
let output = Command::new("git")
|
||||
let output = git_command()
|
||||
.arg("clone")
|
||||
.arg("--depth")
|
||||
.arg("1")
|
||||
@@ -551,7 +550,7 @@ mod tests {
|
||||
}
|
||||
|
||||
fn run_git(repo: &Path, args: &[&str]) {
|
||||
let output = Command::new("git")
|
||||
let output = git_command()
|
||||
.args(args)
|
||||
.current_dir(repo)
|
||||
.set_no_window()
|
||||
|
||||
@@ -26,6 +26,19 @@ pub trait SubprocessExt {
|
||||
fn set_no_window(&mut self) -> &mut Self;
|
||||
}
|
||||
|
||||
/// Creates a Git command that rejects implicit bare repositories and cannot run a
|
||||
/// repository-configured fsmonitor hook.
|
||||
pub fn git_command() -> std::process::Command {
|
||||
let mut command = std::process::Command::new("git");
|
||||
command.args([
|
||||
"-c",
|
||||
"safe.bareRepository=explicit",
|
||||
"-c",
|
||||
"core.fsmonitor=false",
|
||||
]);
|
||||
command
|
||||
}
|
||||
|
||||
impl SubprocessExt for Command {
|
||||
fn set_no_window(&mut self) -> &mut Self {
|
||||
#[cfg(windows)]
|
||||
|
||||
@@ -0,0 +1,85 @@
|
||||
use goose::subprocess::git_command;
|
||||
use std::fs;
|
||||
use std::path::Path;
|
||||
use std::process::{Command, Output};
|
||||
|
||||
fn run_git(cwd: &Path, args: &[&str]) -> Output {
|
||||
Command::new("git")
|
||||
.args(args)
|
||||
.current_dir(cwd)
|
||||
.output()
|
||||
.unwrap()
|
||||
}
|
||||
|
||||
fn assert_git_succeeded(output: &Output) {
|
||||
assert!(
|
||||
output.status.success(),
|
||||
"git failed: {}",
|
||||
String::from_utf8_lossy(&output.stderr)
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn rejects_implicitly_discovered_bare_repository() {
|
||||
let temp_dir = tempfile::tempdir().unwrap();
|
||||
assert_git_succeeded(&run_git(
|
||||
temp_dir.path(),
|
||||
&["init", "--bare", "embedded.git"],
|
||||
));
|
||||
|
||||
let nested_dir = temp_dir.path().join("embedded.git/nested");
|
||||
fs::create_dir(&nested_dir).unwrap();
|
||||
|
||||
assert_git_succeeded(&run_git(
|
||||
&nested_dir,
|
||||
&["-c", "safe.bareRepository=all", "rev-parse", "--git-dir"],
|
||||
));
|
||||
|
||||
let output = git_command()
|
||||
.args(["rev-parse", "--git-dir"])
|
||||
.current_dir(&nested_dir)
|
||||
.output()
|
||||
.unwrap();
|
||||
|
||||
assert!(!output.status.success(), "implicit bare repo was accepted");
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn does_not_execute_repository_fsmonitor_hook() {
|
||||
use std::os::unix::fs::PermissionsExt;
|
||||
|
||||
let temp_dir = tempfile::tempdir().unwrap();
|
||||
let repo_dir = temp_dir.path().join("repo");
|
||||
fs::create_dir(&repo_dir).unwrap();
|
||||
assert_git_succeeded(&run_git(&repo_dir, &["init"]));
|
||||
|
||||
fs::write(repo_dir.join("tracked.txt"), "content").unwrap();
|
||||
assert_git_succeeded(&run_git(&repo_dir, &["add", "tracked.txt"]));
|
||||
|
||||
let marker_path = temp_dir.path().join("fsmonitor-ran");
|
||||
let hook_path = temp_dir.path().join("fsmonitor-hook");
|
||||
fs::write(
|
||||
&hook_path,
|
||||
format!("#!/bin/sh\n: > '{}'\n", marker_path.display()),
|
||||
)
|
||||
.unwrap();
|
||||
fs::set_permissions(&hook_path, fs::Permissions::from_mode(0o755)).unwrap();
|
||||
assert_git_succeeded(&run_git(
|
||||
&repo_dir,
|
||||
&["config", "core.fsmonitor", hook_path.to_str().unwrap()],
|
||||
));
|
||||
|
||||
assert_git_succeeded(&run_git(&repo_dir, &["status", "--porcelain"]));
|
||||
assert!(marker_path.exists(), "fsmonitor hook fixture did not run");
|
||||
fs::remove_file(&marker_path).unwrap();
|
||||
|
||||
let output = git_command()
|
||||
.args(["status", "--porcelain"])
|
||||
.current_dir(&repo_dir)
|
||||
.output()
|
||||
.unwrap();
|
||||
|
||||
assert_git_succeeded(&output);
|
||||
assert!(!marker_path.exists(), "repository fsmonitor hook ran");
|
||||
}
|
||||
+11
-1
@@ -252,7 +252,17 @@ function listGitWorktreeDirs(dir: string): Promise<string[]> {
|
||||
|
||||
execFile(
|
||||
'git',
|
||||
['-C', dir, 'worktree', 'list', '--porcelain'],
|
||||
[
|
||||
'-c',
|
||||
'safe.bareRepository=explicit',
|
||||
'-c',
|
||||
'core.fsmonitor=false',
|
||||
'-C',
|
||||
dir,
|
||||
'worktree',
|
||||
'list',
|
||||
'--porcelain',
|
||||
],
|
||||
{ timeout: 3000 },
|
||||
(error, stdout) => {
|
||||
if (error) {
|
||||
|
||||
@@ -16,10 +16,21 @@ export interface SlashCommand {
|
||||
}
|
||||
|
||||
function isGitRepo(cwd: string): boolean {
|
||||
const result = spawnSync("git", ["rev-parse", "--is-inside-work-tree"], {
|
||||
cwd,
|
||||
stdio: ["ignore", "ignore", "ignore"],
|
||||
});
|
||||
const result = spawnSync(
|
||||
"git",
|
||||
[
|
||||
"-c",
|
||||
"safe.bareRepository=explicit",
|
||||
"-c",
|
||||
"core.fsmonitor=false",
|
||||
"rev-parse",
|
||||
"--is-inside-work-tree",
|
||||
],
|
||||
{
|
||||
cwd,
|
||||
stdio: ["ignore", "ignore", "ignore"],
|
||||
},
|
||||
);
|
||||
return result.status === 0;
|
||||
}
|
||||
|
||||
@@ -28,7 +39,15 @@ const MAX_DIFF_BYTES = 2_000_000;
|
||||
function readDiff(cwd: string): { text: string; truncated: boolean } | null {
|
||||
const result = spawnSync(
|
||||
"git",
|
||||
["--no-pager", "diff", "--no-color"],
|
||||
[
|
||||
"-c",
|
||||
"safe.bareRepository=explicit",
|
||||
"-c",
|
||||
"core.fsmonitor=false",
|
||||
"--no-pager",
|
||||
"diff",
|
||||
"--no-color",
|
||||
],
|
||||
{
|
||||
cwd,
|
||||
encoding: "utf8",
|
||||
|
||||
Reference in New Issue
Block a user