Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

make static analysis optional in git hooks #484

Merged
merged 4 commits into from
Aug 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
130 changes: 70 additions & 60 deletions crates/bins/src/bin/datadog-static-analyzer-git-hook.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ fn main() -> Result<()> {
);
opts.optflag("t", "include-testing-rules", "include testing rules");
opts.optflag("", "secrets", "enable secrets detection (BETA)");
opts.optflag("", "static-analysis", "enable static-analysis");

let matches = match opts.parse(&args[1..]) {
Ok(m) => m,
Expand All @@ -182,6 +183,7 @@ fn main() -> Result<()> {
let use_confirmation = matches.opt_present("confirmation");

let secrets_enabled = matches.opt_present("secrets");
let static_analysis_enabled = matches.opt_present("static-analysis");
let default_branch_opt = matches.opt_str("default-branch");
let sha_start_opt = matches.opt_str("sha-start");
let sha_end_opt = matches.opt_str("sha-end");
Expand Down Expand Up @@ -212,6 +214,12 @@ fn main() -> Result<()> {
exit(1)
}

if !static_analysis_enabled && !secrets_enabled {
eprintln!("either --static-analysis or --secrets should be specified");
print_usage(&program, opts);
exit(1)
}

let configuration_file: Option<ConfigFile> =
match read_config_file(directory_to_analyze.as_str()) {
Ok(cfg) => cfg,
Expand Down Expand Up @@ -417,74 +425,76 @@ fn main() -> Result<()> {

// static analysis part
let mut fail_for_static_analysis = false;
let languages = get_languages_for_rules(&rules);
let mut all_rule_results: Vec<RuleResult> = vec![];
for language in &languages {
let files_for_language = filter_files_for_language(&files_to_analyze, language);
if static_analysis_enabled {
let languages = get_languages_for_rules(&rules);
for language in &languages {
let files_for_language = filter_files_for_language(&files_to_analyze, language);

if files_for_language.is_empty() {
continue;
}
if files_for_language.is_empty() {
continue;
}

let rules_for_language: Vec<RuleInternal> =
convert_rules_to_rules_internal(&configuration, language)?;
let rules_for_language: Vec<RuleInternal> =
convert_rules_to_rules_internal(&configuration, language)?;

// take the relative path for the analysis
let rule_results: Vec<RuleResult> = files_for_language
.into_par_iter()
.flat_map(|path| {
let relative_path = path
.strip_prefix(directory_path)
.unwrap()
.to_str()
.expect("path contains non-Unicode characters");
let relative_path: Arc<str> = Arc::from(relative_path);
let rule_config = configuration
.rule_config_provider
.config_for_file(relative_path.as_ref());
if let Ok(file_content) = fs::read_to_string(&path) {
let file_content = Arc::from(file_content);
analyze(
language,
&rules_for_language,
&relative_path,
&file_content,
&rule_config,
&analysis_options,
)
} else {
vec![]
}
})
.collect();

// take the relative path for the analysis
let rule_results: Vec<RuleResult> = files_for_language
.into_par_iter()
.flat_map(|path| {
let relative_path = path
.strip_prefix(directory_path)
.unwrap()
.to_str()
.expect("path contains non-Unicode characters");
let relative_path: Arc<str> = Arc::from(relative_path);
let rule_config = configuration
.rule_config_provider
.config_for_file(relative_path.as_ref());
if let Ok(file_content) = fs::read_to_string(&path) {
let file_content = Arc::from(file_content);
analyze(
language,
&rules_for_language,
&relative_path,
&file_content,
&rule_config,
&analysis_options,
)
all_rule_results.extend(rule_results);
}

all_rule_results.iter_mut().for_each(|rr| {
let path = PathBuf::from(&rr.filename);
rr.violations.retain(|v| {
if let Some(lines) = modifications.get(&path) {
lines.contains(&v.start.line)
} else {
vec![]
false
}
})
.collect();

all_rule_results.extend(rule_results);
}

all_rule_results.iter_mut().for_each(|rr| {
let path = PathBuf::from(&rr.filename);
rr.violations.retain(|v| {
if let Some(lines) = modifications.get(&path) {
lines.contains(&v.start.line)
} else {
false
}
});
});
});

for rule_result in &all_rule_results {
let path = PathBuf::from(&rule_result.filename);
for violation in &rule_result.violations {
println!(
"{}",
format_error(
path.display().to_string().as_str(),
violation.start.line,
&rule_result.rule_name,
IssueType::StaticAnalysis,
)
);
fail_for_static_analysis = true;
for rule_result in &all_rule_results {
let path = PathBuf::from(&rule_result.filename);
for violation in &rule_result.violations {
println!(
"{}",
format_error(
path.display().to_string().as_str(),
violation.start.line,
&rule_result.rule_name,
IssueType::StaticAnalysis,
)
);
fail_for_static_analysis = true;
}
}
}

Expand Down
5 changes: 3 additions & 2 deletions doc/git-hooks.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,12 @@ datadog-static-analyzer-git-hook -r <path/to/repo> --secrets --default-branch m
To compare the commit `<sha1>` and `<sha2>` (`<sha1>` has been committed before `<sha2>`), enter the following command.

```shell
datadog-static-analyzer-git-hook -r <path/to/repo> --secrets --sha-start <sha1> --sha-end <sha2>
datadog-static-analyzer-git-hook -r <path/to/repo> --static-analysis --secrets --sha-start <sha1> --sha-end <sha2>
```

### Options

- `--static-analysis`: check for any static analysis error
- `--secrets`: also validate secrets with [Datadog Sensitive Data Scanner](https://docs.datadoghq.com/sensitive_data_scanner/)
- `--confirmation`: prompts the user if they want to bypass the warning

Expand Down Expand Up @@ -64,7 +65,7 @@ repo_path=$(git rev-parse --show-toplevel)
# Make sure the user can provide some input
exec < /dev/tty

datadog-static-analyzer-git-hook -r $repo_path --secrets --confirmation --default-branch main
datadog-static-analyzer-git-hook -r $repo_path --secrets --static-analysis --confirmation --default-branch main

if [ $? -eq 0 ]; then
echo "datadog-static-analyzer check passed"
Expand Down
20 changes: 18 additions & 2 deletions misc/integration-git-hooks.sh
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ echo "Starting test: secrets should be found using the default branch"
./target/release-dev/datadog-static-analyzer-git-hook --repository "${REPO_DIR}" --secrets --debug yes --default-branch main --output /tmp/git-hook.sarif >/tmp/plop 2>&1

if [ $? -ne 1 ]; then
cat /tmp/plop
echo "secrets should have been found"
exit 1
fi
Expand All @@ -43,8 +44,8 @@ cat /tmp/plop
NB_OCCURRENCES=$(grep "secret found on file foobar" /tmp/plop | wc -l)
echo "Found ${NB_OCCURRENCES} secret"
if [ "${NB_OCCURRENCES}" -ne "1" ]; then
echo "secrets should have been found"
cat /tmp/plop
echo "secrets should have been found"
exit 1
fi

Expand All @@ -62,6 +63,7 @@ echo "Starting test: secrets should be found using two sha"
./target/release-dev/datadog-static-analyzer-git-hook --repository "${REPO_DIR}" --secrets --debug yes --sha-start $SHA1 --sha-end $SHA2 >/tmp/plop 2>&1

if [ $? -ne 1 ]; then
cat /tmp/plop
echo "secrets should have been found"
exit 1
fi
Expand Down Expand Up @@ -93,7 +95,7 @@ SHA3=$(cd $REPO_DIR && git rev-parse HEAD)

echo "starting analyzer between $SHA2 and $SHA3"

./target/release-dev/datadog-static-analyzer-git-hook --repository "${REPO_DIR}" --secrets --debug yes --sha-start $SHA2 --sha-end $SHA3 >/tmp/plop 2>&1
./target/release-dev/datadog-static-analyzer-git-hook --repository "${REPO_DIR}" --static-analysis --secrets --debug yes --sha-start $SHA2 --sha-end $SHA3 >/tmp/plop 2>&1

if [ $? -ne 1 ]; then
echo "static analysis issues should have been found"
Expand Down Expand Up @@ -134,6 +136,20 @@ if [ "${NB_OCCURRENCES}" -ne "1" ]; then
exit 1
fi

###############################################################
# TEST: Do not pass --static-analysis or --secrets and it fails
###############################################################

echo "Starting test: error when not specifying --static-analysis or --secret"

./target/release-dev/datadog-static-analyzer-git-hook --repository "${REPO_DIR}" --debug yes --default-branch mainwefwef >/tmp/plop 2>&1

if [ $? -ne 1 ]; then
cat /tmp/plop
echo "program should return an error if --static-analysis or --secrets are not passed"
exit 1
fi

echo "All tests passed"
rm -rf "${REPO_DIR}"
exit 0
Loading