fix(git): prevent `core.fsmonitor` from executing external commands (#3981)
This commit is contained in:
parent
53a8808e7a
commit
03278e4de4
|
@ -313,6 +313,12 @@ impl<'a> Context<'a> {
|
||||||
let branch = get_current_branch(&repository);
|
let branch = get_current_branch(&repository);
|
||||||
let remote = get_remote_repository_info(&repository, branch.as_deref());
|
let remote = get_remote_repository_info(&repository, branch.as_deref());
|
||||||
let path = repository.path().to_path_buf();
|
let path = repository.path().to_path_buf();
|
||||||
|
|
||||||
|
let fs_monitor_value_is_true = repository
|
||||||
|
.config_snapshot()
|
||||||
|
.boolean("core.fs_monitor")
|
||||||
|
.unwrap_or(false);
|
||||||
|
|
||||||
Ok(Repo {
|
Ok(Repo {
|
||||||
repo: shared_repo,
|
repo: shared_repo,
|
||||||
branch,
|
branch,
|
||||||
|
@ -320,6 +326,7 @@ impl<'a> Context<'a> {
|
||||||
path,
|
path,
|
||||||
state: repository.state(),
|
state: repository.state(),
|
||||||
remote,
|
remote,
|
||||||
|
fs_monitor_value_is_true,
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
@ -589,6 +596,10 @@ pub struct Repo {
|
||||||
|
|
||||||
/// Remote repository
|
/// Remote repository
|
||||||
pub remote: Option<Remote>,
|
pub remote: Option<Remote>,
|
||||||
|
|
||||||
|
/// Contains `true` if the value of `core.fsmonitor` is set to `true`.
|
||||||
|
/// If not `true`, `fsmonitor` is explicitly disabled in git commands.
|
||||||
|
fs_monitor_value_is_true: bool,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl Repo {
|
impl Repo {
|
||||||
|
@ -596,6 +607,47 @@ impl Repo {
|
||||||
pub fn open(&self) -> Repository {
|
pub fn open(&self) -> Repository {
|
||||||
self.repo.to_thread_local()
|
self.repo.to_thread_local()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Wrapper to execute external git commands.
|
||||||
|
/// Handles adding the appropriate `--git-dir` and `--work-tree` flags to the command.
|
||||||
|
/// Also handles additional features required for security, such as disabling `fsmonitor`.
|
||||||
|
/// At this time, mocking is not supported.
|
||||||
|
pub fn exec_git<T: AsRef<OsStr> + Debug>(
|
||||||
|
&self,
|
||||||
|
context: &Context,
|
||||||
|
git_args: &[T],
|
||||||
|
) -> Option<CommandOutput> {
|
||||||
|
let mut command = create_command("git").ok()?;
|
||||||
|
|
||||||
|
// A value of `true` should not execute external commands.
|
||||||
|
let fsm_config_value = if self.fs_monitor_value_is_true {
|
||||||
|
"core.fsmonitor=true"
|
||||||
|
} else {
|
||||||
|
"core.fsmonitor="
|
||||||
|
};
|
||||||
|
|
||||||
|
command.env("GIT_OPTIONAL_LOCKS", "0").args([
|
||||||
|
OsStr::new("-C"),
|
||||||
|
context.current_dir.as_os_str(),
|
||||||
|
OsStr::new("--git-dir"),
|
||||||
|
self.path.as_os_str(),
|
||||||
|
OsStr::new("-c"),
|
||||||
|
OsStr::new(fsm_config_value),
|
||||||
|
]);
|
||||||
|
|
||||||
|
// Bare repositories might not have a workdir, so we need to check for that.
|
||||||
|
if let Some(wt) = self.workdir.as_ref() {
|
||||||
|
command.args([OsStr::new("--work-tree"), wt.as_os_str()]);
|
||||||
|
}
|
||||||
|
|
||||||
|
command.args(git_args);
|
||||||
|
log::trace!("Executing git command: {:?}", command);
|
||||||
|
|
||||||
|
exec_timeout(
|
||||||
|
&mut command,
|
||||||
|
Duration::from_millis(context.root_config.command_timeout),
|
||||||
|
)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Remote repository
|
/// Remote repository
|
||||||
|
|
|
@ -1,5 +1,4 @@
|
||||||
use regex::Regex;
|
use regex::Regex;
|
||||||
use std::ffi::OsStr;
|
|
||||||
|
|
||||||
use crate::{
|
use crate::{
|
||||||
config::ModuleConfig, configs::git_metrics::GitMetricsConfig,
|
config::ModuleConfig, configs::git_metrics::GitMetricsConfig,
|
||||||
|
@ -21,23 +20,12 @@ pub fn module<'a>(context: &'a Context) -> Option<Module<'a>> {
|
||||||
};
|
};
|
||||||
|
|
||||||
let repo = context.get_repo().ok()?;
|
let repo = context.get_repo().ok()?;
|
||||||
let repo_root = repo.workdir.as_ref()?;
|
let mut git_args = vec!["diff", "--shortstat"];
|
||||||
|
|
||||||
let mut args = vec![
|
|
||||||
OsStr::new("--git-dir"),
|
|
||||||
repo.path.as_os_str(),
|
|
||||||
OsStr::new("--work-tree"),
|
|
||||||
repo_root.as_os_str(),
|
|
||||||
OsStr::new("--no-optional-locks"),
|
|
||||||
OsStr::new("diff"),
|
|
||||||
OsStr::new("--shortstat"),
|
|
||||||
];
|
|
||||||
|
|
||||||
if config.ignore_submodules {
|
if config.ignore_submodules {
|
||||||
args.push(OsStr::new("--ignore-submodules"));
|
git_args.push("--ignore-submodules");
|
||||||
}
|
}
|
||||||
|
|
||||||
let diff = context.exec_cmd("git", &args)?.stdout;
|
let diff = repo.exec_git(context, &git_args)?.stdout;
|
||||||
|
|
||||||
let stats = GitDiff::parse(&diff);
|
let stats = GitDiff::parse(&diff);
|
||||||
|
|
||||||
|
|
|
@ -4,9 +4,9 @@ use regex::Regex;
|
||||||
use super::{Context, Module, ModuleConfig};
|
use super::{Context, Module, ModuleConfig};
|
||||||
|
|
||||||
use crate::configs::git_status::GitStatusConfig;
|
use crate::configs::git_status::GitStatusConfig;
|
||||||
|
use crate::context;
|
||||||
use crate::formatter::StringFormatter;
|
use crate::formatter::StringFormatter;
|
||||||
use crate::segment::Segment;
|
use crate::segment::Segment;
|
||||||
use std::ffi::OsStr;
|
|
||||||
use std::sync::Arc;
|
use std::sync::Arc;
|
||||||
|
|
||||||
const ALL_STATUS_FORMAT: &str =
|
const ALL_STATUS_FORMAT: &str =
|
||||||
|
@ -31,10 +31,8 @@ pub fn module<'a>(context: &'a Context) -> Option<Module<'a>> {
|
||||||
let mut module = context.new_module("git_status");
|
let mut module = context.new_module("git_status");
|
||||||
let config: GitStatusConfig = GitStatusConfig::try_load(module.config);
|
let config: GitStatusConfig = GitStatusConfig::try_load(module.config);
|
||||||
|
|
||||||
let info = Arc::new(GitStatusInfo::load(context, config.clone()));
|
// Return None if not in git repository
|
||||||
|
let repo = context.get_repo().ok()?;
|
||||||
//Return None if not in git repository
|
|
||||||
context.get_repo().ok()?;
|
|
||||||
|
|
||||||
if let Some(git_status) = git_status_wsl(context, &config) {
|
if let Some(git_status) = git_status_wsl(context, &config) {
|
||||||
if git_status.is_empty() {
|
if git_status.is_empty() {
|
||||||
|
@ -44,6 +42,8 @@ pub fn module<'a>(context: &'a Context) -> Option<Module<'a>> {
|
||||||
return Some(module);
|
return Some(module);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
let info = Arc::new(GitStatusInfo::load(context, repo, config.clone()));
|
||||||
|
|
||||||
let parsed = StringFormatter::new(config.format).and_then(|formatter| {
|
let parsed = StringFormatter::new(config.format).and_then(|formatter| {
|
||||||
formatter
|
formatter
|
||||||
.map_meta(|variable, _| match variable {
|
.map_meta(|variable, _| match variable {
|
||||||
|
@ -128,15 +128,21 @@ pub fn module<'a>(context: &'a Context) -> Option<Module<'a>> {
|
||||||
|
|
||||||
struct GitStatusInfo<'a> {
|
struct GitStatusInfo<'a> {
|
||||||
context: &'a Context<'a>,
|
context: &'a Context<'a>,
|
||||||
|
repo: &'a context::Repo,
|
||||||
config: GitStatusConfig<'a>,
|
config: GitStatusConfig<'a>,
|
||||||
repo_status: OnceCell<Option<RepoStatus>>,
|
repo_status: OnceCell<Option<RepoStatus>>,
|
||||||
stashed_count: OnceCell<Option<usize>>,
|
stashed_count: OnceCell<Option<usize>>,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl<'a> GitStatusInfo<'a> {
|
impl<'a> GitStatusInfo<'a> {
|
||||||
pub fn load(context: &'a Context, config: GitStatusConfig<'a>) -> Self {
|
pub fn load(
|
||||||
|
context: &'a Context,
|
||||||
|
repo: &'a context::Repo,
|
||||||
|
config: GitStatusConfig<'a>,
|
||||||
|
) -> Self {
|
||||||
Self {
|
Self {
|
||||||
context,
|
context,
|
||||||
|
repo,
|
||||||
config,
|
config,
|
||||||
repo_status: OnceCell::new(),
|
repo_status: OnceCell::new(),
|
||||||
stashed_count: OnceCell::new(),
|
stashed_count: OnceCell::new(),
|
||||||
|
@ -148,19 +154,20 @@ impl<'a> GitStatusInfo<'a> {
|
||||||
}
|
}
|
||||||
|
|
||||||
pub fn get_repo_status(&self) -> &Option<RepoStatus> {
|
pub fn get_repo_status(&self) -> &Option<RepoStatus> {
|
||||||
self.repo_status
|
self.repo_status.get_or_init(|| {
|
||||||
.get_or_init(|| match get_repo_status(self.context, &self.config) {
|
match get_repo_status(self.context, self.repo, &self.config) {
|
||||||
Some(repo_status) => Some(repo_status),
|
Some(repo_status) => Some(repo_status),
|
||||||
None => {
|
None => {
|
||||||
log::debug!("get_repo_status: git status execution failed");
|
log::debug!("get_repo_status: git status execution failed");
|
||||||
None
|
None
|
||||||
}
|
}
|
||||||
})
|
}
|
||||||
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
pub fn get_stashed(&self) -> &Option<usize> {
|
pub fn get_stashed(&self) -> &Option<usize> {
|
||||||
self.stashed_count
|
self.stashed_count
|
||||||
.get_or_init(|| match get_stashed_count(self.context) {
|
.get_or_init(|| match get_stashed_count(self.repo) {
|
||||||
Some(stashed_count) => Some(stashed_count),
|
Some(stashed_count) => Some(stashed_count),
|
||||||
None => {
|
None => {
|
||||||
log::debug!("get_stashed_count: git stash execution failed");
|
log::debug!("get_stashed_count: git stash execution failed");
|
||||||
|
@ -199,37 +206,35 @@ impl<'a> GitStatusInfo<'a> {
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Gets the number of files in various git states (staged, modified, deleted, etc...)
|
/// Gets the number of files in various git states (staged, modified, deleted, etc...)
|
||||||
fn get_repo_status(context: &Context, config: &GitStatusConfig) -> Option<RepoStatus> {
|
fn get_repo_status(
|
||||||
|
context: &Context,
|
||||||
|
repo: &context::Repo,
|
||||||
|
config: &GitStatusConfig,
|
||||||
|
) -> Option<RepoStatus> {
|
||||||
log::debug!("New repo status created");
|
log::debug!("New repo status created");
|
||||||
|
|
||||||
let mut repo_status = RepoStatus::default();
|
let mut repo_status = RepoStatus::default();
|
||||||
let mut args = vec![
|
let mut args = vec!["status", "--porcelain=2"];
|
||||||
OsStr::new("-C"),
|
|
||||||
context.current_dir.as_os_str(),
|
|
||||||
OsStr::new("--no-optional-locks"),
|
|
||||||
OsStr::new("status"),
|
|
||||||
OsStr::new("--porcelain=2"),
|
|
||||||
];
|
|
||||||
|
|
||||||
// for performance reasons, only pass flags if necessary...
|
// for performance reasons, only pass flags if necessary...
|
||||||
let has_ahead_behind = !config.ahead.is_empty() || !config.behind.is_empty();
|
let has_ahead_behind = !config.ahead.is_empty() || !config.behind.is_empty();
|
||||||
let has_up_to_date_diverged = !config.up_to_date.is_empty() || !config.diverged.is_empty();
|
let has_up_to_date_diverged = !config.up_to_date.is_empty() || !config.diverged.is_empty();
|
||||||
if has_ahead_behind || has_up_to_date_diverged {
|
if has_ahead_behind || has_up_to_date_diverged {
|
||||||
args.push(OsStr::new("--branch"));
|
args.push("--branch");
|
||||||
}
|
}
|
||||||
|
|
||||||
// ... and add flags that omit information the user doesn't want
|
// ... and add flags that omit information the user doesn't want
|
||||||
let has_untracked = !config.untracked.is_empty();
|
let has_untracked = !config.untracked.is_empty();
|
||||||
if !has_untracked {
|
if !has_untracked {
|
||||||
args.push(OsStr::new("--untracked-files=no"));
|
args.push("--untracked-files=no");
|
||||||
}
|
}
|
||||||
if config.ignore_submodules {
|
if config.ignore_submodules {
|
||||||
args.push(OsStr::new("--ignore-submodules=dirty"));
|
args.push("--ignore-submodules=dirty");
|
||||||
} else if !has_untracked {
|
} else if !has_untracked {
|
||||||
args.push(OsStr::new("--ignore-submodules=untracked"));
|
args.push("--ignore-submodules=untracked");
|
||||||
}
|
}
|
||||||
|
|
||||||
let status_output = context.exec_cmd("git", &args)?;
|
let status_output = repo.exec_git(context, &args)?;
|
||||||
let statuses = status_output.stdout.lines();
|
let statuses = status_output.stdout.lines();
|
||||||
|
|
||||||
statuses.for_each(|status| {
|
statuses.for_each(|status| {
|
||||||
|
@ -243,8 +248,8 @@ fn get_repo_status(context: &Context, config: &GitStatusConfig) -> Option<RepoSt
|
||||||
Some(repo_status)
|
Some(repo_status)
|
||||||
}
|
}
|
||||||
|
|
||||||
fn get_stashed_count(context: &Context) -> Option<usize> {
|
fn get_stashed_count(repo: &context::Repo) -> Option<usize> {
|
||||||
let repo = context.get_repo().ok()?.open();
|
let repo = repo.open();
|
||||||
let reference = match repo.try_find_reference("refs/stash") {
|
let reference = match repo.try_find_reference("refs/stash") {
|
||||||
Ok(Some(reference)) => reference,
|
Ok(Some(reference)) => reference,
|
||||||
// No stash reference found
|
// No stash reference found
|
||||||
|
@ -742,6 +747,37 @@ mod tests {
|
||||||
repo_dir.close()
|
repo_dir.close()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
#[cfg(unix)]
|
||||||
|
fn doesnt_run_fsmonitor() -> io::Result<()> {
|
||||||
|
use std::os::unix::fs::PermissionsExt;
|
||||||
|
let repo_dir = fixture_repo(FixtureProvider::Git)?;
|
||||||
|
|
||||||
|
let mut f = File::create(repo_dir.path().join("do_not_execute"))?;
|
||||||
|
write!(f, "#!/bin/sh\necho executed > executed\nsync executed")?;
|
||||||
|
let metadata = f.metadata()?;
|
||||||
|
let mut permissions = metadata.permissions();
|
||||||
|
permissions.set_mode(0o700);
|
||||||
|
f.set_permissions(permissions)?;
|
||||||
|
f.sync_all()?;
|
||||||
|
|
||||||
|
create_command("git")?
|
||||||
|
.args(["config", "core.fsmonitor"])
|
||||||
|
.arg(repo_dir.path().join("do_not_execute"))
|
||||||
|
.current_dir(repo_dir.path())
|
||||||
|
.output()?;
|
||||||
|
|
||||||
|
ModuleRenderer::new("git_status")
|
||||||
|
.path(repo_dir.path())
|
||||||
|
.collect();
|
||||||
|
|
||||||
|
let created_file = repo_dir.path().join("executed").exists();
|
||||||
|
|
||||||
|
assert!(!created_file);
|
||||||
|
|
||||||
|
repo_dir.close()
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn shows_stashed() -> io::Result<()> {
|
fn shows_stashed() -> io::Result<()> {
|
||||||
let repo_dir = fixture_repo(FixtureProvider::Git)?;
|
let repo_dir = fixture_repo(FixtureProvider::Git)?;
|
||||||
|
|
Loading…
Reference in New Issue