Skip to content

Commit

Permalink
Merge pull request #692 from arpoch/fix
Browse files Browse the repository at this point in the history
[JENKINS-38699] Limit CLI git operations to the workspace
  • Loading branch information
MarkEWaite authored Apr 3, 2021
2 parents f1efd07 + 420cdde commit d7cb0ef
Show file tree
Hide file tree
Showing 6 changed files with 164 additions and 6 deletions.
6 changes: 6 additions & 0 deletions src/main/java/hudson/plugins/git/GitAPI.java
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,12 @@ public boolean hasGitRepo() throws GitException, InterruptedException {
return Git.USE_CLI ? super.hasGitRepo() : jgit.hasGitRepo();
}

/** {@inheritDoc} */
@Override
public boolean hasGitRepo(boolean checkParentDirectories) throws GitException, InterruptedException {
return Git.USE_CLI ? super.hasGitRepo(checkParentDirectories) : jgit.hasGitRepo(checkParentDirectories);
}

/** {@inheritDoc} */
public boolean isCommitInRepo(ObjectId commit) throws GitException, InterruptedException {
return Git.USE_CLI ? super.isCommitInRepo(commit) : jgit.isCommitInRepo(commit);
Expand Down
33 changes: 32 additions & 1 deletion src/main/java/org/jenkinsci/plugins/gitclient/CliGitAPIImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,9 @@ public void init() throws GitException, InterruptedException {
}

/**
* hasGitRepo.
* Returns true if this workspace has a git repository.
* Also returns true if this workspace contains an empty .git directory and
* a parent directory has a git repository.
*
* @return true if this workspace has a git repository
* @throws hudson.plugins.git.GitException if underlying git operation fails.
Expand All @@ -351,6 +353,35 @@ public boolean hasGitRepo() throws GitException, InterruptedException {
return false;
}

/**
* Returns true if this workspace has a git repository.
* If checkParentDirectories is true, searches parent directories.
* If checkParentDirectories is false, checks workspace directory only.
*
* @param checkParentDirectories if true, search upward for a git repository
* @return true if this workspace has a git repository
* @throws hudson.plugins.git.GitException if underlying git operation fails.
* @throws java.lang.InterruptedException if interrupted.
*/
@Override
public boolean hasGitRepo(boolean checkParentDirectories) throws GitException, InterruptedException {
if (checkParentDirectories) {
return hasGitRepo();
}
if (hasGitRepo(".git")) {
// Check if this is a valid git repo with --resolve-git-dir
try {
launchCommand("rev-parse", "--resolve-git-dir",
workspace.getAbsolutePath() + File.separator + ".git");
} catch (Exception ex) {
ex.printStackTrace(listener.error("Workspace has a .git repository, but it appears to be corrupt."));
return false;
}
return true;
}
return false;
}

/**
* Returns true if the parameter GIT_DIR is a directory which
* contains a git repository.
Expand Down
14 changes: 13 additions & 1 deletion src/main/java/org/jenkinsci/plugins/gitclient/GitClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -183,14 +183,26 @@ public interface GitClient {
void commit(String message, PersonIdent author, PersonIdent committer) throws GitException, InterruptedException;

/**
* hasGitRepo.
* Return true if the current workspace has a git repository.
*
* @return true if this workspace has a git repository
* @throws hudson.plugins.git.GitException if underlying git operation fails.
* @throws java.lang.InterruptedException if interrupted.
*/
boolean hasGitRepo() throws GitException, InterruptedException;

/**
* Return true if the current workspace has a git repository.
* If checkParentDirectories is true, searches parent directories.
* If checkParentDirectories is false, checks workspace directory only.
*
* @param checkParentDirectories if true, search upward for a git repository
* @return true if this workspace has a git repository
* @throws hudson.plugins.git.GitException if underlying git operation fails.
* @throws java.lang.InterruptedException if interrupted.
*/
boolean hasGitRepo(boolean checkParentDirectories) throws GitException, InterruptedException;

/**
* isCommitInRepo.
*
Expand Down
48 changes: 45 additions & 3 deletions src/main/java/org/jenkinsci/plugins/gitclient/JGitAPIImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@
import org.eclipse.jgit.revwalk.RevWalk;
import org.eclipse.jgit.revwalk.filter.MaxCountRevFilter;
import org.eclipse.jgit.revwalk.filter.RevFilter;
import org.eclipse.jgit.storage.file.FileRepositoryBuilder;
import org.eclipse.jgit.submodule.SubmoduleWalk;
import org.eclipse.jgit.transport.CredentialsProvider;
import org.eclipse.jgit.transport.FetchConnection;
Expand Down Expand Up @@ -122,8 +123,6 @@
import org.eclipse.jgit.api.RebaseCommand.Operation;
import org.eclipse.jgit.api.RebaseResult;

import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;

/**
* GitClient pure Java implementation using JGit.
* Goal is to eventually get a full java implementation for GitClient
Expand Down Expand Up @@ -1714,7 +1713,8 @@ public Set<String> getRemoteTagNames(String tagPattern) throws GitException {
}

/**
* hasGitRepo.
* Returns true if the current workspace has a git repository.
* Does not search parent directories for a repository.
*
* @return true if this workspace has a git repository
* @throws hudson.plugins.git.GitException if underlying git operation fails.
Expand All @@ -1728,6 +1728,48 @@ public boolean hasGitRepo() throws GitException {
}
}

/**
* Returns true if the current workspace has a git repository.
* If checkParentDirectories is true, searches parent directories.
* If checkParentDirectories is false, checks workspace directory only.
*
* @param checkParentDirectories if true, search upwards for a git repository
* @return true if this workspace has a git repository
* @throws hudson.plugins.git.GitException if underlying git operation fails.
*/
@Override
public boolean hasGitRepo(boolean checkParentDirectories) throws GitException {
if (!checkParentDirectories) { // JGit hasGitRepo() does not check parent directories
return hasGitRepo();
}
if (hasGitRepo(".git")) {
try (Repository repo = getRepository()) {
if (repo.getObjectDatabase().exists()) {
return true;
}
/* Emulate command line git searching up the file system hierarchy */
FileRepositoryBuilder repositoryBuilder = new FileRepositoryBuilder();
FileRepositoryBuilder parentRepoBuilder = repositoryBuilder.findGitDir(workspace);
return parentRepoBuilder.getGitDir() != null;
} catch (GitException e) {
return false;
}
}
return false;
}

private boolean hasGitRepo(String gitDirectory) throws GitException {
try {
File dotGit = new File(workspace, gitDirectory);
return dotGit.exists();
} catch (SecurityException ex) {
throw new GitException("Security error when trying to check for .git. Are you sure you have correct permissions?",
ex);
} catch (Exception e) {
throw new GitException("Couldn't check for .git", e);
}
}

/** {@inheritDoc} */
@Override
public boolean isCommitInRepo(ObjectId commit) throws GitException {
Expand Down
17 changes: 16 additions & 1 deletion src/main/java/org/jenkinsci/plugins/gitclient/RemoteGitImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ public void commit(String message, PersonIdent author, PersonIdent committer) th
}

/**
* hasGitRepo.
* Returns true if the current workspace has a git repository.
*
* @return true if this workspace has a git repository
* @throws hudson.plugins.git.GitException if underlying git operation fails.
Expand All @@ -284,6 +284,21 @@ public boolean hasGitRepo() throws GitException, InterruptedException {
return proxy.hasGitRepo();
}

/**
* Returns true if the current workspace has a git repository.
* If checkParentDirectories is true, searches parent directories.
* If checkParentDirectories is false, checks workspace directory only.
*
* @param checkParentDirectories if true, search upward for a git repository
* @return true if this workspace has a git repository
* @throws hudson.plugins.git.GitException if underlying git operation fails.
* @throws java.lang.InterruptedException if interrupted.
*/
@Override
public boolean hasGitRepo(boolean checkParentDirectories) throws GitException, InterruptedException {
return proxy.hasGitRepo(checkParentDirectories);
}

/** {@inheritDoc} */
public boolean isCommitInRepo(ObjectId commit) throws GitException, InterruptedException {
return proxy.isCommitInRepo(commit);
Expand Down
52 changes: 52 additions & 0 deletions src/test/java/org/jenkinsci/plugins/gitclient/GitClientTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.InvalidPathException;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.Arrays;
Expand Down Expand Up @@ -600,6 +601,57 @@ public void testHasGitRepo() throws Exception {
assertFalse("Empty repo '" + emptyDir.getAbsolutePath() + "' initialized", emptyClient.hasGitRepo());
}

@Test
public void testHasGitRepoFalse() throws Exception {
/* Use system temp directory so that no parent directory has a git repository */
Path tempDir = Files.createTempDirectory("git-client-hasGitRepo");
GitClient noRepoClient = Git.with(TaskListener.NULL, new EnvVars()).in(tempDir.toFile()).using(gitImplName).getClient();
assertFalse("New empty temp dir has a git repo(1)", noRepoClient.hasGitRepo());
assertFalse("New empty temp dir has a git repo(2)", noRepoClient.hasGitRepo(false));
assertFalse("New empty temp dir has a git repo(3)", noRepoClient.hasGitRepo(true));
tempDir.toFile().delete(); // Remove the temporary directory
}

@Issue("JENKINS-38699")
@Test
public void testHasGitRepoNestedDir() throws Exception {
File childDir = tempFolder.newFolder("parentDir", "childDir");
File parentDir = childDir.getParentFile();

GitClient parentDirClient = Git.with(TaskListener.NULL, new EnvVars()).in(parentDir).using(gitImplName).getClient();
assertFalse("Unexpected has git repo before init(1)", parentDirClient.hasGitRepo());
assertFalse("Unexpected has git repo before init(2)", parentDirClient.hasGitRepo(true));
assertFalse("Unexpected has git repo before init(3)", parentDirClient.hasGitRepo(false));

parentDirClient.init();
assertTrue("Missing git repo after init(1)", parentDirClient.hasGitRepo());
assertTrue("Missing git repo after init(2)", parentDirClient.hasGitRepo(true));
assertTrue("Missing git repo after init(3)", parentDirClient.hasGitRepo(false));

GitClient childDirClient = Git.with(TaskListener.NULL, new EnvVars()).in(childDir).using(gitImplName).getClient();
assertFalse("Unexpected has child git repo before child init(1)", childDirClient.hasGitRepo());
assertFalse("Unexpected has child git repo before child init(2)", childDirClient.hasGitRepo(true));
assertFalse("Unexpected has child git repo before child init(3)", childDirClient.hasGitRepo(false));

File childGitDir = new File(childDir, ".git");
boolean dirCreated = childGitDir.mkdir();
assertTrue("Failed to create empty .git dir in childDir", dirCreated);
if (gitImplName.equals("git")) {
// JENKINS-38699 - if an empty .git directory exists, CLI git searches upwards to perform operations
assertTrue("Missing parent git repo before child init(1)", childDirClient.hasGitRepo());
} else {
// JENKINS-38699 - if an empty .git directory exists, JGit does NOT search upwards to perform operations
assertFalse("Unexpected parent git repo detected by JGit before child init(1)", childDirClient.hasGitRepo());
}
assertTrue("Missing parent git repo before child init(2)", childDirClient.hasGitRepo(true));
assertFalse("Unexpected has child repo before child init(3)", childDirClient.hasGitRepo(false));

childDirClient.init();
assertTrue("Missing git repo after child init(1)", childDirClient.hasGitRepo());
assertTrue("Missing git repo after child init(2)", childDirClient.hasGitRepo(true));
assertTrue("Missing git repo after child init(3)", childDirClient.hasGitRepo(false));
}

@Test
public void testIsCommitInRepo() throws Exception {
assertTrue(srcGitClient.isCommitInRepo(upstreamCommit));
Expand Down

0 comments on commit d7cb0ef

Please sign in to comment.