Skip to content

Commit 7cc2a33

Browse files
authored
fix: committer email policy and malformed permission deny URL (#181, #182) (#196)
## Summary - **#182**: Permission deny messages were building the repo URL as `https://{provider.name}{slug}`, producing malformed output like `https://github/github.com/owner/repo`. Fixed to use `provider.getUri()` in both the store-and-forward hook and transparent proxy filter. - **#181**: The email domain check was validating the commit *author* instead of the *committer*, causing false-positive rejections when a developer rebased external contributor commits. The committer (the corporate employee who ran the rebase) is the identity that matters for compliance. Two independent policy knobs are now available: - `commit.committer.email.*` — primary control; enforces that your staff use their work identity. Rebased external commits pass as long as the committer email is valid. - `commit.author.email.*` — optional strict mode; blocks pushes containing rebased commits from contributors outside the allowed domain. When a violation fires, the message explains the rebase restriction and suggests opening a PR from the original author's fork instead. Each violation is labelled explicitly (`committer email (...)` vs `author email (...)`) with a specific fix hint, so developers never need to ask why their push was rejected. ## Test plan - [ ] `./gradlew :git-proxy-java-core:test` passes (677 tests) - [ ] Committer-only config: push with rebased external commit — author email ignored, committer email checked - [ ] Author+committer config: push with rebased external commit — two labelled violations reported - [ ] Permission deny message shows well-formed URL (`https://github.com/owner/repo`)
2 parents a436e4d + 2b7d190 commit 7cc2a33

11 files changed

Lines changed: 258 additions & 99 deletions

File tree

docs/CONFIGURATION.md

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -580,19 +580,32 @@ accepts the internal username, not an email address.
580580

581581
## Commit validation
582582

583-
Per-commit checks (identity, author email, message content) apply to both store-and-forward and transparent proxy modes.
583+
Per-commit checks (identity, email policy, message content) apply to both store-and-forward and transparent proxy modes.
584584

585585
```yaml
586586
commit:
587-
author:
587+
# Committer email policy — the committer is the employee who ran git commit or git rebase.
588+
# This is the primary corporate control: enforce that your staff use their work identity.
589+
# Rebased commits from external contributors still pass as long as the committer email is valid.
590+
committer:
588591
email:
589592
domain:
590-
# Regex the email domain must match. Omit to allow all domains.
591-
allow: "(corp\\.example\\.com|contractors\\.example\\.com)$"
593+
# Regex the committer email domain must match. Omit to allow all domains.
594+
allow: "corp\\.example\\.com$"
592595
local:
593596
# Regex blocking specific local-parts (before @). Omit to allow all.
594597
block: "^(noreply|no-reply|bot|nobody)$"
595598
599+
# Author email policy — the author is whoever originally wrote the commit.
600+
# Configure this only if you want to disallow rebasing external contributors' commits.
601+
# When set, any commit whose author email is outside the allowed domain is blocked —
602+
# developers must open PRs from the original fork rather than rebasing upstream changes.
603+
# Omit this block entirely to allow external author emails (the most common setup).
604+
author:
605+
email:
606+
domain:
607+
allow: "corp\\.example\\.com$"
608+
596609
message:
597610
block:
598611
literals:

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,10 @@ public static IdentityVerificationMode fromString(String value) {
5050
@Builder.Default
5151
private AuthorConfig author = AuthorConfig.builder().build();
5252

53+
/** Configuration for committer email validation. */
54+
@Builder.Default
55+
private CommitterConfig committer = CommitterConfig.builder().build();
56+
5357
/** Configuration for commit message validation. */
5458
@Builder.Default
5559
private MessageConfig message = MessageConfig.builder().build();
@@ -64,6 +68,16 @@ public static class AuthorConfig {
6468
private EmailConfig email = EmailConfig.builder().build();
6569
}
6670

71+
/** Configuration for committer validation. */
72+
@Data
73+
@Builder
74+
public static class CommitterConfig {
75+
76+
/** Configuration for email validation. */
77+
@Builder.Default
78+
private EmailConfig email = EmailConfig.builder().build();
79+
}
80+
6781
/** Configuration for email validation. */
6882
@Data
6983
@Builder

git-proxy-java-core/src/main/java/org/finos/gitproxy/git/CheckUserPushPermissionHook.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -119,8 +119,8 @@ public void onPreReceive(ReceivePack rp, Collection<ReceiveCommand> commands) {
119119
user.getUsername(),
120120
providerId,
121121
repoSlug);
122-
String repoRef = providerId != null && repoSlug != null
123-
? String.format("https://%s%s", providerId, repoSlug)
122+
String repoRef = provider != null && repoSlug != null
123+
? provider.getUri().toString().replaceAll("/$", "") + repoSlug
124124
: repoSlug;
125125
String detail = GitClientUtils.format(
126126
sym(NO_ENTRY) + " Push Blocked - Unauthorized",

git-proxy-java-core/src/main/java/org/finos/gitproxy/servlet/filter/CheckUserPushPermissionFilter.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,8 +107,9 @@ public void doHttpFilter(HttpServletRequest request, HttpServletResponse respons
107107
user.getUsername(),
108108
providerId,
109109
slug);
110-
String repoUrl =
111-
providerId != null && slug != null ? String.format("https://%s%s", providerId, slug) : slug;
110+
String repoUrl = requestDetails.getProvider() != null && slug != null
111+
? requestDetails.getProvider().getUri().toString().replaceAll("/$", "") + slug
112+
: slug;
112113
String title = sym(NO_ENTRY) + " Push Blocked - Unauthorized";
113114
String message = sym(CROSS_MARK) + " " + user.getUsername() + " is not allowed to push to:\n" + " "
114115
+ sym(LINK) + " " + repoUrl;

git-proxy-java-core/src/main/java/org/finos/gitproxy/validation/AuthorEmailCheck.java

Lines changed: 36 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,15 @@
1414
import org.finos.gitproxy.git.Commit;
1515

1616
/**
17-
* Validates that every author email in the pushed commits passes format checks and matches the configured domain/local
18-
* rules.
17+
* Validates committer and author emails in the pushed commits against configured domain/local rules.
18+
*
19+
* <p>Committer validation ({@code commit.committer.email.*}) is the primary corporate control: the committer is the
20+
* employee who authored or rebased the change, and must use their work identity. Author validation
21+
* ({@code commit.author.email.*}) is an optional stricter policy: when configured, it also checks the original author
22+
* email, which effectively disallows rebasing commits from contributors outside the allowed domain.
23+
*
24+
* <p>Each policy is independent — configure one, both, or neither. Violations from each are reported separately with
25+
* explicit labels so developers know exactly which identity triggered the block and what to do about it.
1926
*/
2027
@RequiredArgsConstructor
2128
public class AuthorEmailCheck implements CommitCheck {
@@ -24,22 +31,38 @@ public class AuthorEmailCheck implements CommitCheck {
2431

2532
@Override
2633
public List<Violation> check(List<Commit> commits) {
27-
Set<String> emails = commits.stream().map(c -> c.getAuthor().getEmail()).collect(Collectors.toSet());
28-
2934
List<Violation> violations = new ArrayList<>();
30-
for (String email : emails) {
31-
String reason = violationReason(email);
35+
36+
Set<String> committerEmails =
37+
commits.stream().map(c -> c.getCommitter().getEmail()).collect(Collectors.toSet());
38+
for (String email : committerEmails) {
39+
String reason = violationReason(email, config.getCommitter().getEmail());
3240
if (reason != null) {
33-
String detail = sym(CROSS_MARK) + " " + email + ": " + reason + "\n"
34-
+ " \u2192 git config user.email \"you@example.com\"";
35-
violations.add(new Violation(email, reason, detail));
41+
String detail = sym(CROSS_MARK) + " committer email (" + email + "): " + reason + "\n"
42+
+ " \u2192 The committer is you — the person who ran git commit or git rebase.\n"
43+
+ " \u2192 Fix: git config user.email \"you@corp.com\"";
44+
violations.add(new Violation("committer:" + email, reason, detail));
3645
}
3746
}
47+
48+
Set<String> authorEmails =
49+
commits.stream().map(c -> c.getAuthor().getEmail()).collect(Collectors.toSet());
50+
for (String email : authorEmails) {
51+
String reason = violationReason(email, config.getAuthor().getEmail());
52+
if (reason != null) {
53+
String detail = sym(CROSS_MARK) + " author email (" + email + "): " + reason + "\n"
54+
+ " \u2192 This commit was originally authored by someone outside the allowed domain.\n"
55+
+ " \u2192 Rebasing external commits onto this branch is not permitted by policy.\n"
56+
+ " \u2192 Alternative: open a PR from the original author's fork instead of rebasing.";
57+
violations.add(new Violation("author:" + email, reason, detail));
58+
}
59+
}
60+
3861
return violations;
3962
}
4063

41-
/** Returns the reason the email is rejected, or {@code null} if it is allowed. */
42-
private String violationReason(String email) {
64+
/** Returns the reason the email is rejected under the given email config, or {@code null} if it is allowed. */
65+
private String violationReason(String email, CommitConfig.EmailConfig emailConfig) {
4366
if (email == null || email.isEmpty()) {
4467
return "empty email";
4568
}
@@ -54,12 +77,12 @@ private String violationReason(String email) {
5477
String local = parts[0];
5578
String domain = parts[1];
5679

57-
Pattern localBlock = config.getAuthor().getEmail().getLocal().getBlock();
80+
Pattern localBlock = emailConfig.getLocal().getBlock();
5881
if (localBlock != null && localBlock.matcher(local).find()) {
5982
return "blocked local part (" + local + ")";
6083
}
6184

62-
Pattern domainAllow = config.getAuthor().getEmail().getDomain().getAllow();
85+
Pattern domainAllow = emailConfig.getDomain().getAllow();
6386
if (domainAllow != null && !domainAllow.matcher(domain).find()) {
6487
return "domain not allowed (" + domain + ")";
6588
}

git-proxy-java-core/src/test/java/org/finos/gitproxy/git/AuthorEmailValidationHookTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ private ObjectId createCommit(Git git, String message, String name, String email
5858

5959
private CommitConfig allowExampleCom() {
6060
return CommitConfig.builder()
61-
.author(CommitConfig.AuthorConfig.builder()
61+
.committer(CommitConfig.CommitterConfig.builder()
6262
.email(CommitConfig.EmailConfig.builder()
6363
.domain(CommitConfig.DomainConfig.builder()
6464
.allow(Pattern.compile("example\\.com$"))

git-proxy-java-core/src/test/java/org/finos/gitproxy/servlet/filter/CheckAuthorEmailsFilterTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ private HttpServletRequest mockPushRequest(GitRequestDetails details) throws IOE
9595

9696
private CommitConfig testConfig() {
9797
return CommitConfig.builder()
98-
.author(CommitConfig.AuthorConfig.builder()
98+
.committer(CommitConfig.CommitterConfig.builder()
9999
.email(CommitConfig.EmailConfig.builder()
100100
.domain(CommitConfig.DomainConfig.builder()
101101
.allow(Pattern.compile("(example\\.com|company\\.org)$"))

git-proxy-java-core/src/test/java/org/finos/gitproxy/servlet/filter/FilterChainIntegrationTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ private HttpServletRequest mockPushRequest(GitRequestDetails details) throws IOE
100100

101101
private CommitConfig emailAndMessageConfig() {
102102
return CommitConfig.builder()
103-
.author(CommitConfig.AuthorConfig.builder()
103+
.committer(CommitConfig.CommitterConfig.builder()
104104
.email(CommitConfig.EmailConfig.builder()
105105
.domain(CommitConfig.DomainConfig.builder()
106106
.allow(Pattern.compile("(example\\.com|company\\.org)$"))

0 commit comments

Comments
 (0)