Skip to content

Commit 5c3b6fe

Browse files
Copilotcoopernetes
Copilot
and
coopernetes
committed
Address review feedback: Pattern types, remove userEmail, stub secret scanning, add auth headers, shallow clones
Co-authored-by: coopernetes <57812123+coopernetes@users.noreply.github.com>
1 parent 5494cb3 commit 5c3b6fe

12 files changed

Lines changed: 136 additions & 293 deletions

‎jgit-proxy-core/src/main/java/org/finos/gitproxy/config/CommitConfig.java‎

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import java.util.ArrayList;
44
import java.util.List;
5+
import java.util.regex.Pattern;
56
import lombok.Builder;
67
import lombok.Data;
78

@@ -49,9 +50,9 @@ public static class DomainConfig {
4950

5051
/**
5152
* Regex pattern for allowed email domains. If set, only emails matching this pattern are allowed. Example:
52-
* ".*\\.company\\.com$" to allow only company.com domains.
53+
* Pattern.compile(".*\\.company\\.com$") to allow only company.com domains.
5354
*/
54-
private String allow;
55+
private Pattern allow;
5556
}
5657

5758
/** Configuration for email local part validation. */
@@ -61,9 +62,9 @@ public static class LocalConfig {
6162

6263
/**
6364
* Regex pattern for blocked email local parts. If set, emails matching this pattern are blocked. Example:
64-
* "^(noreply|no-reply)$" to block noreply addresses.
65+
* Pattern.compile("^(noreply|no-reply)$") to block noreply addresses.
6566
*/
66-
private String block;
67+
private Pattern block;
6768
}
6869

6970
/** Configuration for commit message validation. */
@@ -88,12 +89,9 @@ public static class BlockConfig {
8889
@Builder.Default
8990
private List<String> literals = new ArrayList<>();
9091

91-
/**
92-
* List of regex patterns that are blocked in commit messages. Messages matching any of these patterns will be
93-
* rejected.
94-
*/
92+
/** List of compiled regex patterns that are blocked in commit messages. */
9593
@Builder.Default
96-
private List<String> patterns = new ArrayList<>();
94+
private List<Pattern> patterns = new ArrayList<>();
9795
}
9896

9997
/**

‎jgit-proxy-core/src/main/java/org/finos/gitproxy/config/SecretScanningConfig.java‎

Lines changed: 0 additions & 64 deletions
This file was deleted.

‎jgit-proxy-core/src/main/java/org/finos/gitproxy/git/CommitInspectionService.java‎

Lines changed: 3 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,9 @@ private static Commit convertToCommit(RevCommit revCommit) {
172172
}
173173

174174
// Extract GPG signature if present
175-
String signature = extractGpgSignature(revCommit);
175+
// Note: Signature extraction is handled by GitReceivePackParser during push parsing
176+
// and is available in the Commit object passed from the push packet data
177+
String signature = null;
176178

177179
return Commit.builder()
178180
.sha(revCommit.getName())
@@ -190,47 +192,4 @@ private static Commit convertToCommit(RevCommit revCommit) {
190192
.signature(signature)
191193
.build();
192194
}
193-
194-
/**
195-
* Extract GPG signature from a commit if present.
196-
*
197-
* @param revCommit The JGit commit
198-
* @return The GPG signature, or null if not present
199-
*/
200-
private static String extractGpgSignature(RevCommit revCommit) {
201-
byte[] rawBuffer = revCommit.getRawBuffer();
202-
String raw = new String(rawBuffer);
203-
204-
int sigStart = raw.indexOf("gpgsig ");
205-
if (sigStart < 0) {
206-
return null;
207-
}
208-
209-
// Find the start of the signature block
210-
sigStart += 7; // length of "gpgsig "
211-
int sigEnd = raw.indexOf("\n", sigStart);
212-
213-
// Build the complete signature by collecting continuation lines
214-
StringBuilder signature = new StringBuilder();
215-
int pos = sigStart;
216-
217-
while (pos < raw.length()) {
218-
int nextNewline = raw.indexOf("\n", pos);
219-
if (nextNewline < 0) {
220-
break;
221-
}
222-
223-
String line = raw.substring(pos, nextNewline);
224-
if (!line.startsWith(" ")) {
225-
// End of signature block
226-
break;
227-
}
228-
229-
// Remove leading space and add to signature
230-
signature.append(line.substring(1)).append("\n");
231-
pos = nextNewline + 1;
232-
}
233-
234-
return signature.toString().trim();
235-
}
236195
}

‎jgit-proxy-core/src/main/java/org/finos/gitproxy/git/GitRequestDetails.java‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,8 @@ public class GitRequestDetails {
1717
private HttpOperation operation;
1818
private Repository repository;
1919
private String branch; // null for fetch requests
20-
private Commit commit;
21-
private List<Commit> commits = new ArrayList<>(); // All commits in a push
22-
private String userEmail; // Email of the user performing the operation
20+
private Commit commit; // Head/parent commit from the push
21+
private List<Commit> pushedCommits = new ArrayList<>(); // All commits received in this push
2322
private GitProxyProvider provider;
2423
private List<GitProxyFilter> filters = new ArrayList<>();
2524
private GitResult result = GitResult.PENDING;

‎jgit-proxy-core/src/main/java/org/finos/gitproxy/git/LocalRepositoryCache.java‎

Lines changed: 41 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -22,22 +22,40 @@ public class LocalRepositoryCache {
2222

2323
private final Path cacheDirectory;
2424
private final Map<String, CachedRepository> cache = new ConcurrentHashMap<>();
25+
private final int cloneDepth;
26+
private final boolean registerShutdownHook;
2527

26-
/** Default constructor that uses system temp directory. */
28+
/** Default constructor that uses system temp directory with shutdown hook. */
2729
public LocalRepositoryCache() throws IOException {
28-
this(Files.createTempDirectory("jgit-proxy-cache-"));
30+
this(Files.createTempDirectory("jgit-proxy-cache-"), 100, true);
2931
}
3032

3133
/**
32-
* Constructor with custom cache directory.
34+
* Constructor with custom cache directory - Spring-friendly (no shutdown hook).
3335
*
3436
* @param cacheDirectory The directory to use for caching repositories
37+
* @param registerShutdownHook Whether to register shutdown hook (false for Spring apps)
3538
*/
36-
public LocalRepositoryCache(Path cacheDirectory) {
39+
public LocalRepositoryCache(Path cacheDirectory, boolean registerShutdownHook) throws IOException {
40+
this(cacheDirectory, 100, registerShutdownHook);
41+
}
42+
43+
/**
44+
* Full constructor with custom cache directory and clone depth.
45+
*
46+
* @param cacheDirectory The directory to use for caching repositories
47+
* @param cloneDepth The depth for shallow clones (0 for full clone)
48+
* @param registerShutdownHook Whether to register shutdown hook (false for Spring apps)
49+
*/
50+
public LocalRepositoryCache(Path cacheDirectory, int cloneDepth, boolean registerShutdownHook) {
3751
this.cacheDirectory = cacheDirectory;
38-
log.info("Initialized LocalRepositoryCache at: {}", cacheDirectory);
39-
// Register shutdown hook to clean up
40-
Runtime.getRuntime().addShutdownHook(new Thread(this::cleanup));
52+
this.cloneDepth = cloneDepth;
53+
this.registerShutdownHook = registerShutdownHook;
54+
log.info("Initialized LocalRepositoryCache at: {} with clone depth: {}", cacheDirectory, cloneDepth);
55+
// Register shutdown hook to clean up (skip for Spring apps that use @PreDestroy)
56+
if (registerShutdownHook) {
57+
Runtime.getRuntime().addShutdownHook(new Thread(this::cleanup));
58+
}
4159
}
4260

4361
/**
@@ -85,16 +103,26 @@ private synchronized Repository cloneOrFetch(String remoteUrl, String cacheKey)
85103
if (repoDir.exists()) {
86104
log.debug("Repository directory exists, opening and fetching: {}", repoDir);
87105
Git git = Git.open(repoDir);
88-
// Fetch latest changes
89-
git.fetch().setRemote("origin").call();
106+
// Fetch latest changes with depth if configured
107+
if (cloneDepth > 0) {
108+
git.fetch().setRemote("origin").setDepth(cloneDepth).call();
109+
} else {
110+
git.fetch().setRemote("origin").call();
111+
}
90112
repository = git.getRepository();
91113
} else {
92-
log.debug("Cloning repository to: {}", repoDir);
93-
Git git = Git.cloneRepository()
114+
log.debug("Cloning repository to: {} with depth: {}", repoDir, cloneDepth);
115+
var cloneCommand = Git.cloneRepository()
94116
.setURI(remoteUrl)
95117
.setDirectory(repoDir)
96-
.setBare(true)
97-
.call();
118+
.setBare(true);
119+
120+
// Set shallow clone depth if configured
121+
if (cloneDepth > 0) {
122+
cloneCommand.setDepth(cloneDepth);
123+
}
124+
125+
Git git = cloneCommand.call();
98126
repository = git.getRepository();
99127
}
100128

‎jgit-proxy-core/src/main/java/org/finos/gitproxy/git/TemporaryRepositoryResolver.java‎

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,18 @@ public Repository open(HttpServletRequest req, String name)
4747
* @return The remote repository URL
4848
*/
4949
private String extractRemoteUrl(HttpServletRequest req, String name) {
50+
// Extract authentication from Authorization header
51+
String authHeader = req.getHeader("Authorization");
52+
String authCredentials = "";
53+
54+
if (authHeader != null && authHeader.startsWith("Basic ")) {
55+
// Extract credentials from Basic auth
56+
String base64Credentials = authHeader.substring("Basic ".length()).trim();
57+
String credentials = new String(java.util.Base64.getDecoder().decode(base64Credentials));
58+
// credentials format is "username:password"
59+
authCredentials = credentials + "@";
60+
}
61+
5062
// This is a simplified implementation
5163
// In a real implementation, you would extract the provider and construct the full URL
5264
// For now, we'll use the request info
@@ -55,9 +67,13 @@ private String extractRemoteUrl(HttpServletRequest req, String name) {
5567
// Remove leading slash and any git-receive-pack or git-upload-pack suffixes
5668
String cleanPath = pathInfo.substring(1).replaceAll("/(git-receive-pack|git-upload-pack|info/refs).*$", "");
5769

58-
// Construct URL based on the host from request
59-
// This is simplified - in production you'd need more sophisticated URL construction
60-
return "https://" + cleanPath + ".git";
70+
// Construct URL with authentication if present
71+
// Format: https://username:password@host/path.git
72+
if (!authCredentials.isEmpty()) {
73+
return "https://" + authCredentials + cleanPath + ".git";
74+
} else {
75+
return "https://" + cleanPath + ".git";
76+
}
6177
}
6278

6379
return name;

‎jgit-proxy-core/src/main/java/org/finos/gitproxy/servlet/filter/CheckAuthorEmailsFilter.java‎

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ public void doHttpFilter(HttpServletRequest request, HttpServletResponse respons
3939
return;
4040
}
4141

42-
List<Commit> commits = requestDetails.getCommits();
42+
List<Commit> commits = requestDetails.getPushedCommits();
4343
if (commits == null || commits.isEmpty()) {
4444
log.debug("No commits found in request details");
4545
return;
@@ -93,23 +93,19 @@ private boolean isEmailAllowed(String email) {
9393
String emailDomain = parts[1];
9494

9595
// Check domain allow pattern
96-
String domainAllowPattern =
96+
Pattern domainAllowPattern =
9797
commitConfig.getAuthor().getEmail().getDomain().getAllow();
98-
if (domainAllowPattern != null && !domainAllowPattern.isEmpty()) {
99-
if (!Pattern.compile(domainAllowPattern, Pattern.CASE_INSENSITIVE)
100-
.matcher(emailDomain)
101-
.find()) {
98+
if (domainAllowPattern != null) {
99+
if (!domainAllowPattern.matcher(emailDomain).find()) {
102100
return false;
103101
}
104102
}
105103

106104
// Check local part block pattern
107-
String localBlockPattern =
105+
Pattern localBlockPattern =
108106
commitConfig.getAuthor().getEmail().getLocal().getBlock();
109-
if (localBlockPattern != null && !localBlockPattern.isEmpty()) {
110-
if (Pattern.compile(localBlockPattern, Pattern.CASE_INSENSITIVE)
111-
.matcher(emailLocal)
112-
.find()) {
107+
if (localBlockPattern != null) {
108+
if (localBlockPattern.matcher(emailLocal).find()) {
113109
return false;
114110
}
115111
}

‎jgit-proxy-core/src/main/java/org/finos/gitproxy/servlet/filter/CheckCommitMessagesFilter.java‎

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
import java.util.List;
99
import java.util.Set;
1010
import java.util.regex.Pattern;
11-
import java.util.regex.PatternSyntaxException;
1211
import java.util.stream.Collectors;
1312
import lombok.extern.slf4j.Slf4j;
1413
import org.finos.gitproxy.config.CommitConfig;
@@ -39,7 +38,7 @@ public void doHttpFilter(HttpServletRequest request, HttpServletResponse respons
3938
return;
4039
}
4140

42-
List<Commit> commits = requestDetails.getCommits();
41+
List<Commit> commits = requestDetails.getPushedCommits();
4342
if (commits == null || commits.isEmpty()) {
4443
log.debug("No commits found in request details");
4544
return;
@@ -89,7 +88,7 @@ private boolean isMessageAllowed(String commitMessage) {
8988
}
9089

9190
List<String> blockedLiterals = commitConfig.getMessage().getBlock().getLiterals();
92-
List<String> blockedPatterns = commitConfig.getMessage().getBlock().getPatterns();
91+
List<Pattern> blockedPatterns = commitConfig.getMessage().getBlock().getPatterns();
9392

9493
// Check blocked literals (case-insensitive)
9594
for (String literal : blockedLiterals) {
@@ -100,16 +99,9 @@ private boolean isMessageAllowed(String commitMessage) {
10099
}
101100

102101
// Check blocked patterns
103-
for (String pattern : blockedPatterns) {
104-
try {
105-
Pattern compiledPattern = Pattern.compile(pattern, Pattern.CASE_INSENSITIVE);
106-
if (compiledPattern.matcher(commitMessage).find()) {
107-
log.debug("Commit message matches blocked pattern: {}", pattern);
108-
return false;
109-
}
110-
} catch (PatternSyntaxException e) {
111-
log.error("Invalid regex pattern: {}", pattern, e);
112-
// Treat invalid patterns as not matching
102+
for (Pattern pattern : blockedPatterns) {
103+
if (pattern.matcher(commitMessage).find()) {
104+
log.debug("Commit message matches blocked pattern");
113105
return false;
114106
}
115107
}

0 commit comments

Comments
 (0)