Skip to content

Commit f334d1e

Browse files
coopernetesclaude
andcommitted
feat: config-driven approval gateway; auto mode skips UI noise for both push modes
- Add AutoApprovalGateway: immediately approves clean pushes, no polling - Add server.approval-mode config key (auto/ui/servicenow, default: auto) - Thread ApprovalGateway into PushFinalizerFilter so transparent proxy also forwards immediately in auto mode instead of blocking pending re-push - Suppress dashboard links and waiting messages in auto mode for S&F output - Add approvesImmediately() to ApprovalGateway interface Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent eea9014 commit f334d1e

17 files changed

Lines changed: 403 additions & 21 deletions

File tree

jgit-proxy-core/src/main/java/org/finos/gitproxy/approval/ApprovalGateway.java

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,4 +12,14 @@ public interface ApprovalGateway {
1212
* heartbeat progress messages to keep the git client alive.
1313
*/
1414
ApprovalResult waitForApproval(String pushId, ProgressSender progress, Duration timeout);
15+
16+
/**
17+
* Returns {@code true} if this gateway approves pushes immediately without requiring human review.
18+
*
19+
* <p>Used by the transparent-proxy finalizer to decide whether to forward the push on the first attempt or block it
20+
* pending a re-push after dashboard approval.
21+
*/
22+
default boolean approvesImmediately() {
23+
return false;
24+
}
1525
}
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
package org.finos.gitproxy.approval;
2+
3+
import java.time.Duration;
4+
import lombok.extern.slf4j.Slf4j;
5+
import org.finos.gitproxy.db.PushStore;
6+
import org.finos.gitproxy.db.model.Attestation;
7+
8+
/**
9+
* Approval gateway that immediately approves every clean push without waiting for human review. Suitable for standalone
10+
* deployments where no approval UI is available.
11+
*
12+
* <p>This gateway is only ever invoked for pushes that passed all validation checks (status {@code BLOCKED} pending
13+
* review). Pushes that fail validation are rejected before the approval gate is reached.
14+
*/
15+
@Slf4j
16+
public class AutoApprovalGateway implements ApprovalGateway {
17+
18+
private final PushStore pushStore;
19+
20+
public AutoApprovalGateway(PushStore pushStore) {
21+
this.pushStore = pushStore;
22+
}
23+
24+
@Override
25+
public boolean approvesImmediately() {
26+
return true;
27+
}
28+
29+
@Override
30+
public ApprovalResult waitForApproval(String pushId, ProgressSender progress, Duration timeout) {
31+
try {
32+
pushStore.approve(
33+
pushId,
34+
Attestation.builder()
35+
.pushId(pushId)
36+
.type(Attestation.Type.APPROVAL)
37+
.reviewerUsername("auto-approval")
38+
.reason("Automatically approved — no validation issues found")
39+
.automated(true)
40+
.build());
41+
log.info("Auto-approved push: {}", pushId);
42+
} catch (Exception e) {
43+
log.warn("Failed to record auto-approval for push {}", pushId, e);
44+
}
45+
return ApprovalResult.APPROVED;
46+
}
47+
}

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,12 @@ var record = pushStore.findById(validationRecordId).orElse(null);
8181

8282
// All clean pushes are BLOCKED pending human review
8383
if (record.getStatus() == PushStatus.BLOCKED) {
84+
if (approvalGateway.approvesImmediately()) {
85+
// Auto-approval: approve silently, no waiting messages or dashboard links
86+
approvalGateway.waitForApproval(validationRecordId, msg -> {}, timeout);
87+
return;
88+
}
89+
8490
sendAndFlush(
8591
rp, msgOut, color(YELLOW, "" + sym(WARNING) + " Push requires review. Waiting for approval..."));
8692
sendAndFlush(rp, msgOut, color(CYAN, "" + sym(KEY) + " Push ID: " + validationRecordId));

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

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ public class PushStorePersistenceHook {
5454
private final GitProxyProvider provider;
5555
private PushContext pushContext;
5656
private String serviceUrl;
57+
private boolean autoApproval;
5758

5859
public PushStorePersistenceHook(PushStore pushStore, GitProxyProvider provider) {
5960
this.pushStore = pushStore;
@@ -70,6 +71,11 @@ public void setServiceUrl(String serviceUrl) {
7071
this.serviceUrl = serviceUrl;
7172
}
7273

74+
/** When {@code true}, suppresses dashboard links in user-facing output (auto-approval mode). */
75+
public void setAutoApproval(boolean autoApproval) {
76+
this.autoApproval = autoApproval;
77+
}
78+
7379
/** Returns a {@link PreReceiveHook} that creates the initial push record. Should be the first hook in the chain. */
7480
public PreReceiveHook preReceiveHook() {
7581
return (ReceivePack rp, Collection<ReceiveCommand> commands) -> {
@@ -161,7 +167,7 @@ public PreReceiveHook validationResultHook(ValidationContext validationContext)
161167
}
162168
rp.sendMessage("────────────────────────────────────────");
163169

164-
if (serviceUrl != null) {
170+
if (serviceUrl != null && !autoApproval) {
165171
rp.sendMessage(color(
166172
CYAN,
167173
"" + sym(LINK) + " View push record: " + serviceUrl + "/#/push/"
@@ -197,7 +203,7 @@ public PreReceiveHook validationResultHook(ValidationContext validationContext)
197203
rp.sendMessage(summary);
198204
}
199205
rp.sendMessage("────────────────────────────────────────");
200-
if (serviceUrl != null) {
206+
if (serviceUrl != null && !autoApproval) {
201207
rp.sendMessage(color(
202208
CYAN,
203209
"" + sym(LINK) + " View push record: " + serviceUrl + "/#/push/" + record.getId()));

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,7 @@ public ReceivePack create(HttpServletRequest req, Repository db)
101101
if (persistenceHook != null) {
102102
persistenceHook.setPushContext(pushContext);
103103
persistenceHook.setServiceUrl(serviceUrl);
104+
persistenceHook.setAutoApproval(approvalGateway != null && approvalGateway.approvesImmediately());
104105
}
105106

106107
// Hook chain - order matters:

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

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import jakarta.servlet.http.HttpServletResponse;
1313
import java.io.IOException;
1414
import java.util.Set;
15+
import org.finos.gitproxy.approval.ApprovalGateway;
1516
import org.finos.gitproxy.git.GitRequestDetails;
1617
import org.finos.gitproxy.git.HttpOperation;
1718

@@ -38,10 +39,12 @@ public class PushFinalizerFilter extends AbstractGitProxyFilter {
3839
private static final int ORDER = 5000;
3940

4041
private final String serviceUrl;
42+
private final ApprovalGateway approvalGateway;
4143

42-
public PushFinalizerFilter(String serviceUrl) {
44+
public PushFinalizerFilter(String serviceUrl, ApprovalGateway approvalGateway) {
4345
super(ORDER, Set.of(HttpOperation.PUSH));
4446
this.serviceUrl = serviceUrl;
47+
this.approvalGateway = approvalGateway;
4548
}
4649

4750
/**
@@ -91,7 +94,13 @@ public void doHttpFilter(HttpServletRequest request, HttpServletResponse respons
9194
return;
9295
}
9396

94-
// First push that passed validation - block pending review
97+
// If the gateway approves immediately (e.g. auto mode), forward the push without blocking
98+
if (approvalGateway.approvesImmediately()) {
99+
details.setResult(GitRequestDetails.GitResult.ALLOWED);
100+
return;
101+
}
102+
103+
// First push that passed validation - block pending review (dashboard/ServiceNow mode)
95104
details.setResult(GitRequestDetails.GitResult.BLOCKED);
96105
String pushId = details.getId().toString();
97106
String summary = buildValidationSummary(details.getSteps());
Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
package org.finos.gitproxy.approval;
2+
3+
import static org.junit.jupiter.api.Assertions.*;
4+
5+
import java.time.Duration;
6+
import java.util.ArrayList;
7+
import java.util.List;
8+
import org.finos.gitproxy.db.memory.InMemoryPushStore;
9+
import org.finos.gitproxy.db.model.PushRecord;
10+
import org.finos.gitproxy.db.model.PushStatus;
11+
import org.junit.jupiter.api.BeforeEach;
12+
import org.junit.jupiter.api.Test;
13+
14+
class AutoApprovalGatewayTest {
15+
16+
private InMemoryPushStore pushStore;
17+
private AutoApprovalGateway gateway;
18+
19+
@BeforeEach
20+
void setUp() {
21+
pushStore = new InMemoryPushStore();
22+
gateway = new AutoApprovalGateway(pushStore);
23+
}
24+
25+
private PushRecord blockedRecord() {
26+
PushRecord r = PushRecord.builder().build();
27+
r.setStatus(PushStatus.BLOCKED);
28+
pushStore.save(r);
29+
return r;
30+
}
31+
32+
@Test
33+
void returnsApproved_immediately() {
34+
PushRecord r = blockedRecord();
35+
36+
ApprovalResult result = gateway.waitForApproval(r.getId(), msg -> {}, Duration.ofSeconds(30));
37+
38+
assertEquals(ApprovalResult.APPROVED, result);
39+
}
40+
41+
@Test
42+
void recordsApprovalInStore() {
43+
PushRecord r = blockedRecord();
44+
45+
gateway.waitForApproval(r.getId(), msg -> {}, Duration.ofSeconds(30));
46+
47+
PushRecord updated = pushStore.findById(r.getId()).orElseThrow();
48+
assertEquals(PushStatus.APPROVED, updated.getStatus());
49+
}
50+
51+
@Test
52+
void attestation_isMarkedAutomated() {
53+
PushRecord r = blockedRecord();
54+
55+
gateway.waitForApproval(r.getId(), msg -> {}, Duration.ofSeconds(30));
56+
57+
PushRecord updated = pushStore.findById(r.getId()).orElseThrow();
58+
assertNotNull(updated.getAttestation(), "Attestation should be set");
59+
assertTrue(updated.getAttestation().isAutomated(), "Attestation should be automated");
60+
assertEquals("auto-approval", updated.getAttestation().getReviewerUsername());
61+
}
62+
63+
@Test
64+
void sendsNoProgressMessages() {
65+
PushRecord r = blockedRecord();
66+
List<String> messages = new ArrayList<>();
67+
68+
gateway.waitForApproval(r.getId(), messages::add, Duration.ofSeconds(30));
69+
70+
assertTrue(messages.isEmpty(), "AutoApprovalGateway should not send any progress messages");
71+
}
72+
73+
@Test
74+
void returnsApproved_evenWhenStoreUpdateFails() {
75+
// Gateway should still return APPROVED if the store throws (e.g. record not found)
76+
ApprovalResult result = gateway.waitForApproval("no-such-id", msg -> {}, Duration.ofSeconds(30));
77+
78+
assertEquals(ApprovalResult.APPROVED, result);
79+
}
80+
}

jgit-proxy-core/src/test/java/org/finos/gitproxy/servlet/filter/PushFinalizerFilterTest.java

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,9 @@
1616
import java.io.ByteArrayOutputStream;
1717
import java.io.IOException;
1818
import java.util.concurrent.atomic.AtomicBoolean;
19+
import org.finos.gitproxy.approval.ApprovalGateway;
20+
import org.finos.gitproxy.approval.AutoApprovalGateway;
21+
import org.finos.gitproxy.db.PushStoreFactory;
1922
import org.finos.gitproxy.git.GitRequestDetails;
2023
import org.finos.gitproxy.git.HttpOperation;
2124
import org.junit.jupiter.api.Test;
@@ -101,7 +104,7 @@ private GitRequestDetails pendingPushDetails() {
101104
@Test
102105
void pendingPush_isBlockedPendingReview() throws Exception {
103106
GitRequestDetails details = pendingPushDetails();
104-
PushFinalizerFilter filter = new PushFinalizerFilter("http://localhost:8080");
107+
PushFinalizerFilter filter = new PushFinalizerFilter("http://localhost:8080", mock(ApprovalGateway.class));
105108
FakeResponse fakeResponse = new FakeResponse();
106109

107110
filter.doHttpFilter(mockPushRequest(details), fakeResponse.mock);
@@ -114,7 +117,7 @@ void pendingPush_isBlockedPendingReview() throws Exception {
114117
void rejectedPush_isLeftAlone() throws Exception {
115118
GitRequestDetails details = pendingPushDetails();
116119
details.setResult(GitRequestDetails.GitResult.REJECTED);
117-
PushFinalizerFilter filter = new PushFinalizerFilter("http://localhost:8080");
120+
PushFinalizerFilter filter = new PushFinalizerFilter("http://localhost:8080", mock(ApprovalGateway.class));
118121
FakeResponse fakeResponse = new FakeResponse();
119122

120123
filter.doHttpFilter(mockPushRequest(details), fakeResponse.mock);
@@ -127,7 +130,7 @@ void rejectedPush_isLeftAlone() throws Exception {
127130
void errorPush_isLeftAlone() throws Exception {
128131
GitRequestDetails details = pendingPushDetails();
129132
details.setResult(GitRequestDetails.GitResult.ERROR);
130-
PushFinalizerFilter filter = new PushFinalizerFilter("http://localhost:8080");
133+
PushFinalizerFilter filter = new PushFinalizerFilter("http://localhost:8080", mock(ApprovalGateway.class));
131134
FakeResponse fakeResponse = new FakeResponse();
132135

133136
filter.doHttpFilter(mockPushRequest(details), fakeResponse.mock);
@@ -141,7 +144,7 @@ void preApprovedPush_isAllowed() throws Exception {
141144
GitRequestDetails details = pendingPushDetails();
142145
HttpServletRequest req = mockPushRequest(details);
143146
when(req.getAttribute(PRE_APPROVED_ATTR)).thenReturn(Boolean.TRUE);
144-
PushFinalizerFilter filter = new PushFinalizerFilter("http://localhost:8080");
147+
PushFinalizerFilter filter = new PushFinalizerFilter("http://localhost:8080", mock(ApprovalGateway.class));
145148
FakeResponse fakeResponse = new FakeResponse();
146149

147150
filter.doHttpFilter(req, fakeResponse.mock);
@@ -154,7 +157,7 @@ void preApprovedPush_isAllowed() throws Exception {
154157
void refDeletion_isAllowed() throws Exception {
155158
GitRequestDetails details = pendingPushDetails();
156159
details.setCommitTo(ZERO_OID);
157-
PushFinalizerFilter filter = new PushFinalizerFilter("http://localhost:8080");
160+
PushFinalizerFilter filter = new PushFinalizerFilter("http://localhost:8080", mock(ApprovalGateway.class));
158161
FakeResponse fakeResponse = new FakeResponse();
159162

160163
filter.doHttpFilter(mockPushRequest(details), fakeResponse.mock);
@@ -167,9 +170,22 @@ void refDeletion_isAllowed() throws Exception {
167170
void nullDetails_doesNotThrow() throws Exception {
168171
HttpServletRequest req = mock(HttpServletRequest.class);
169172
when(req.getAttribute(GIT_REQUEST_ATTR)).thenReturn(null);
170-
PushFinalizerFilter filter = new PushFinalizerFilter("http://localhost:8080");
173+
PushFinalizerFilter filter = new PushFinalizerFilter("http://localhost:8080", mock(ApprovalGateway.class));
171174
FakeResponse fakeResponse = new FakeResponse();
172175

173176
assertDoesNotThrow(() -> filter.doHttpFilter(req, fakeResponse.mock));
174177
}
178+
179+
@Test
180+
void autoApprovalGateway_allowsPushWithoutBlocking() throws Exception {
181+
GitRequestDetails details = pendingPushDetails();
182+
var gateway = new AutoApprovalGateway(PushStoreFactory.inMemory());
183+
PushFinalizerFilter filter = new PushFinalizerFilter("http://localhost:8080", gateway);
184+
FakeResponse fakeResponse = new FakeResponse();
185+
186+
filter.doHttpFilter(mockPushRequest(details), fakeResponse.mock);
187+
188+
assertEquals(GitRequestDetails.GitResult.ALLOWED, details.getResult());
189+
assertFalse(fakeResponse.committed.get(), "Auto-approval must not send a git error to the client");
190+
}
175191
}

jgit-proxy-dashboard/src/main/java/org/finos/gitproxy/dashboard/GitProxyWithDashboardApplication.java

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import org.eclipse.jetty.server.Server;
99
import org.eclipse.jetty.server.ServerConnector;
1010
import org.eclipse.jetty.util.thread.QueuedThreadPool;
11+
import org.finos.gitproxy.approval.UiApprovalGateway;
1112
import org.finos.gitproxy.config.InMemoryProviderConfigurationSource;
1213
import org.finos.gitproxy.config.ProviderConfigurationSource;
1314
import org.finos.gitproxy.db.PushStore;
@@ -51,6 +52,11 @@ public static void main(String[] args) throws Exception {
5152
PushStore pushStore = configBuilder.buildPushStore();
5253
log.info("Push store initialized: {}", pushStore.getClass().getSimpleName());
5354

55+
// Always use UiApprovalGateway when running with the dashboard — the REST API is what drives approval.
56+
// This is intentionally not derived from approval-mode config: the dashboard deployment always needs
57+
// UI-based review regardless of what is set in the config file.
58+
var approvalGateway = new UiApprovalGateway(pushStore);
59+
5460
var storeForwardCache = new LocalRepositoryCache(Files.createTempDirectory("jgit-proxy-sf-"), 0, true);
5561
var proxyCache = new LocalRepositoryCache();
5662

@@ -65,10 +71,10 @@ public static void main(String[] args) throws Exception {
6571
for (GitProxyProvider provider : providerConfig.getProviders()) {
6672
log.info("Registering provider: {}", provider.getName());
6773
GitProxyServletRegistrar.registerGitServlet(
68-
context, provider, storeForwardCache, commitConfig, pushStore, serviceUrl);
74+
context, provider, storeForwardCache, commitConfig, pushStore, serviceUrl, approvalGateway);
6975
GitProxyServletRegistrar.registerProxyServlet(context, provider);
7076
GitProxyServletRegistrar.registerFilters(
71-
context, provider, proxyCache, configBuilder, commitConfig, pushStore, serviceUrl);
77+
context, provider, proxyCache, configBuilder, commitConfig, pushStore, serviceUrl, approvalGateway);
7278
}
7379

7480
// Spring MVC DispatcherServlet at /* - git-specific paths take precedence per servlet spec

jgit-proxy-server/src/main/java/org/finos/gitproxy/jetty/GitProxyJettyApplication.java

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import org.eclipse.jetty.server.Server;
88
import org.eclipse.jetty.server.ServerConnector;
99
import org.eclipse.jetty.util.thread.QueuedThreadPool;
10+
import org.finos.gitproxy.approval.ApprovalGateway;
1011
import org.finos.gitproxy.config.InMemoryProviderConfigurationSource;
1112
import org.finos.gitproxy.db.PushStore;
1213
import org.finos.gitproxy.git.LocalRepositoryCache;
@@ -50,6 +51,8 @@ public static void main(String[] args) throws Exception {
5051
PushStore pushStore = configBuilder.buildPushStore();
5152
log.info("Push store initialized: {}", pushStore.getClass().getSimpleName());
5253

54+
ApprovalGateway approvalGateway = configBuilder.buildApprovalGateway(pushStore);
55+
5356
var storeForwardCache = new LocalRepositoryCache(Files.createTempDirectory("jgit-proxy-sf-"), 0, true);
5457
log.info("Initialized store-and-forward LocalRepositoryCache (full clone)");
5558

@@ -66,10 +69,10 @@ public static void main(String[] args) throws Exception {
6669
for (GitProxyProvider provider : providerConfig.getProviders()) {
6770
log.info("Registering provider: {}", provider.getName());
6871
GitProxyServletRegistrar.registerGitServlet(
69-
context, provider, storeForwardCache, commitConfig, pushStore, serviceUrl);
72+
context, provider, storeForwardCache, commitConfig, pushStore, serviceUrl, approvalGateway);
7073
GitProxyServletRegistrar.registerProxyServlet(context, provider);
7174
GitProxyServletRegistrar.registerFilters(
72-
context, provider, proxyCache, configBuilder, commitConfig, pushStore, serviceUrl);
75+
context, provider, proxyCache, configBuilder, commitConfig, pushStore, serviceUrl, approvalGateway);
7376
}
7477

7578
server.setHandler(context);

0 commit comments

Comments
 (0)