Skip to content

Add validation checks for user provided arguments in git commands #9092

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Jul 7, 2025
Merged

Add validation checks for user provided arguments in git commands #9092

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6b334ee
add validation checks for user provided arguments in git cmds
daniel-mohedano Jul 3, 2025
42ef5da
make isValidArg package private
daniel-mohedano Jul 3, 2025
123f587
also accept short form SHAs on git commands
daniel-mohedano Jul 3, 2025
f8bed95
some refactoring
daniel-mohedano Jul 4, 2025
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
47 changes: 41 additions & 6 deletions ...agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/ShellGitClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import datadog.trace.api.civisibility.telemetry.CiVisibilityDistributionMetric;
import datadog.trace.api.civisibility.telemetry.CiVisibilityMetricCollector;
import datadog.trace.api.civisibility.telemetry.tag.Command;
import datadog.trace.api.git.GitUtils;
import datadog.trace.civisibility.diff.LineDiff;
import datadog.trace.civisibility.utils.ShellCommandExecutor;
import datadog.trace.util.Strings;
Expand Down Expand Up @@ -149,13 +150,13 @@ public void unshallow(@Nullable String remoteCommitReference)
.trim();

// refetch data from the server for the given period of time
if (remoteCommitReference != null) {
if (remoteCommitReference != null && GitUtils.isValidRef(remoteCommitReference)) {
String headSha = getSha(remoteCommitReference);
commandExecutor.executeCommand(
ShellCommandExecutor.OutputParser.IGNORE,
"git",
"fetch",
String.format("--shallow-since=='%s'", latestCommitsSince),
String.format("--shallow-since='%s'", latestCommitsSince),
"--update-shallow",
"--filter=blob:none",
"--recurse-submodules=no",
Expand All @@ -166,7 +167,7 @@ public void unshallow(@Nullable String remoteCommitReference)
ShellCommandExecutor.OutputParser.IGNORE,
"git",
"fetch",
String.format("--shallow-since=='%s'", latestCommitsSince),
String.format("--shallow-since='%s'", latestCommitsSince),
"--update-shallow",
"--filter=blob:none",
"--recurse-submodules=no",
Expand Down Expand Up @@ -231,6 +232,9 @@ public String getRepoRoot() throws IOException, TimeoutException, InterruptedExc
@Override
public String getRemoteUrl(String remoteName)
throws IOException, TimeoutException, InterruptedException {
if (!GitUtils.isValidRef(remoteName)) {
return null;
}
return executeCommand(
Command.GET_REPOSITORY,
() ->
Expand Down Expand Up @@ -274,6 +278,9 @@ public String getCurrentBranch() throws IOException, TimeoutException, Interrupt
@Override
public List<String> getTags(String commit)
throws IOException, TimeoutException, InterruptedException {
if (GitUtils.isNotValidCommit(commit)) {
return Collections.emptyList();
}
return executeCommand(
Command.OTHER,
() -> {
Expand Down Expand Up @@ -302,6 +309,9 @@ public List<String> getTags(String commit)
@Override
public String getSha(String reference)
throws IOException, TimeoutException, InterruptedException {
if (GitUtils.isNotValidCommit(reference)) {
return null;
}
return executeCommand(
Command.OTHER,
() ->
Expand All @@ -324,6 +334,9 @@ public String getSha(String reference)
@Override
public String getFullMessage(String commit)
throws IOException, TimeoutException, InterruptedException {
if (GitUtils.isNotValidCommit(commit)) {
return null;
}
return executeCommand(
Command.OTHER,
() ->
Expand All @@ -346,6 +359,9 @@ public String getFullMessage(String commit)
@Override
public String getAuthorName(String commit)
throws IOException, TimeoutException, InterruptedException {
if (GitUtils.isNotValidCommit(commit)) {
return null;
}
return executeCommand(
Command.OTHER,
() ->
Expand All @@ -368,6 +384,9 @@ public String getAuthorName(String commit)
@Override
public String getAuthorEmail(String commit)
throws IOException, TimeoutException, InterruptedException {
if (GitUtils.isNotValidCommit(commit)) {
return null;
}
return executeCommand(
Command.OTHER,
() ->
Expand All @@ -390,6 +409,9 @@ public String getAuthorEmail(String commit)
@Override
public String getAuthorDate(String commit)
throws IOException, TimeoutException, InterruptedException {
if (GitUtils.isNotValidCommit(commit)) {
return null;
}
return executeCommand(
Command.OTHER,
() ->
Expand All @@ -412,6 +434,9 @@ public String getAuthorDate(String commit)
@Override
public String getCommitterName(String commit)
throws IOException, TimeoutException, InterruptedException {
if (GitUtils.isNotValidCommit(commit)) {
return null;
}
return executeCommand(
Command.OTHER,
() ->
Expand All @@ -434,6 +459,9 @@ public String getCommitterName(String commit)
@Override
public String getCommitterEmail(String commit)
throws IOException, TimeoutException, InterruptedException {
if (GitUtils.isNotValidCommit(commit)) {
return null;
}
return executeCommand(
Command.OTHER,
() ->
Expand All @@ -456,6 +484,9 @@ public String getCommitterEmail(String commit)
@Override
public String getCommitterDate(String commit)
throws IOException, TimeoutException, InterruptedException {
if (GitUtils.isNotValidCommit(commit)) {
return null;
}
return executeCommand(
Command.OTHER,
() ->
Expand Down Expand Up @@ -601,6 +632,10 @@ private Path createTempDirectory() throws IOException {
public String getBaseCommitSha(
@Nullable String baseBranch, @Nullable String settingsDefaultBranch)
throws IOException, TimeoutException, InterruptedException {
if ((baseBranch != null && !GitUtils.isValidRef(baseBranch))
|| (settingsDefaultBranch != null && !GitUtils.isValidRef(settingsDefaultBranch))) {
return null;
}
return executeCommand(
Command.BASE_COMMIT_SHA,
() -> {
Expand Down Expand Up @@ -956,10 +991,10 @@ String getMergeBase(String baseBranch, String sourceBranch)
@Override
public LineDiff getGitDiff(String baseCommit, String targetCommit)
throws IOException, TimeoutException, InterruptedException {
if (Strings.isBlank(baseCommit)) {
if (Strings.isBlank(baseCommit) || !GitUtils.isValidCommitSha(baseCommit)) {
LOGGER.debug("Base commit info is not available, returning empty git diff");
return null;
} else if (Strings.isNotBlank(targetCommit)) {
} else if (Strings.isNotBlank(targetCommit) && GitUtils.isValidCommitSha(targetCommit)) {
return executeCommand(
Command.DIFF,
() ->
Expand Down Expand Up @@ -1041,7 +1076,7 @@ public Factory(Config config, CiVisibilityMetricCollector metricCollector) {
@Override
public GitClient create(@Nullable String repoRoot) {
long commandTimeoutMillis = config.getCiVisibilityGitCommandTimeoutMillis();
if (repoRoot != null) {
if (repoRoot != null && GitUtils.isValidPath(repoRoot)) {
ShellGitClient client =
new ShellGitClient(
metricCollector, repoRoot, "1 month ago", 1000, commandTimeoutMillis);
Expand Down
77 changes: 75 additions & 2 deletions internal-api/src/main/java/datadog/trace/api/git/GitUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,16 @@
import static datadog.trace.api.git.RawParseUtils.decode;
import static datadog.trace.api.git.RawParseUtils.nextLF;

import datadog.trace.util.Strings;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.net.URI;
import java.nio.charset.StandardCharsets;
import java.util.regex.Pattern;
import java.util.zip.DataFormatException;
import java.util.zip.Inflater;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand All @@ -19,6 +22,26 @@ public class GitUtils {
private static final Pattern REFS_HEADS_PATTERN = Pattern.compile("refs/heads/", Pattern.LITERAL);
private static final Pattern REFS_TAGS_PATTERN = Pattern.compile("refs/tags/", Pattern.LITERAL);
private static final Pattern TAGS_PATTERN = Pattern.compile("tags/", Pattern.LITERAL);
// Based on https://git-scm.com/docs/git-check-ref-format
private static final Pattern INVALID_REF_PATTERN =
Pattern.compile(
"^/" // starts with /
+ "|//" // contains //
+ "|\\.\\." // contains ..
+ "|@\\{" // contains @{
+ "|\\.$" // ends with a dot
+ "|\\.lock(/|$)" // ends with .lock in any path component
+ "|(?:^|/)\\." // any component starts with a dot
+ "|\\s" // contains space
+ "|[~^:?*\\[\\\\]" // contains ~ ^ : ? * [ \
+ "|^@$" // is only @
+ "|/$" // ends with slash
);
private static final Pattern PATH_PATTERN = Pattern.compile("^[a-zA-Z0-9_./-]+$");
private static final Pattern SHELL_METACHAR_PATTERN = Pattern.compile(".*[`$&|;<>\n\r#].*");

private static final int SHORT_SHA_LENGTH = 7;
private static final int FULL_SHA_LENGTH = 40;

private static final Logger log = LoggerFactory.getLogger(GitUtils.class);

Expand Down Expand Up @@ -205,12 +228,28 @@ private static void logErrorInflating(final String reason) {
* Checks if the provided string is a valid commit SHA:
*
* <ul>
* <li>length >= 40
* <li>length >= 7
* <li>every character is a hexadecimal digit
* </ul>
*/
public static boolean isValidCommitSha(final String commitSha) {
if (commitSha == null || commitSha.length() < 40) {
return isValidCommitSha(commitSha, SHORT_SHA_LENGTH);
}

/**
* Checks if the provided string is a valid commit SHA of full length:
*
* <ul>
* <li>length >= 40
* <li>every character is a hexadecimal digit
* </ul>
*/
public static boolean isValidCommitShaFull(final String commitSha) {
return isValidCommitSha(commitSha, FULL_SHA_LENGTH);
}

private static boolean isValidCommitSha(final String commitSha, final int minLength) {
if (commitSha == null || commitSha.length() < minLength) {
return false;
}
for (char c : commitSha.toCharArray()) {
Expand All @@ -224,4 +263,38 @@ public static boolean isValidCommitSha(final String commitSha) {
private static boolean isHex(char c) {
return (c >= '0' && c <= '9') || (c >= 'a' && c <= 'f') || (c >= 'A' && c <= 'F');
}

/** Checks if a string that will be used as a Git command argument is valid and safe */
static boolean isValidArg(@Nullable String arg) {
if (Strings.isBlank(arg)) {
return false;
}
return !SHELL_METACHAR_PATTERN.matcher(arg).find();
}

/** Checks if the provided string is a valid Git reference (branch, tag, etc.) */
public static boolean isValidRef(@Nullable String ref) {
if (Strings.isBlank(ref)) {
return false;
}

for (char c : ref.toCharArray()) {
if (c < 32 || c == 127) return false;
}

if (INVALID_REF_PATTERN.matcher(ref).find()) {
return false;
}
return isValidArg(ref);
}

/** Checks if the provided string is a valid system path for Git operations */
public static boolean isValidPath(@Nonnull String path) {
return PATH_PATTERN.matcher(path).matches();
}

/** Checks if the provided string is neither a valid commit SHA nor a valid Git reference */
public static boolean isNotValidCommit(@Nullable String commit) {
return !isValidCommitSha(commit) && !isValidRef(commit);
}
}
2 changes: 1 addition & 1 deletion internal-api/src/main/java/datadog/trace/api/git/UserSuppliedGitInfoBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ public GitInfo build(@Nullable String repositoryPath) {
}

String commitSha = gitInfo.getCommit().getSha();
if (!GitUtils.isValidCommitSha(commitSha)) {
if (!GitUtils.isValidCommitShaFull(commitSha)) {
log.error(
"Git commit SHA could not be resolved or is invalid: "
+ commitSha
Expand Down
Loading