Skip to content

Commit 754f43c

Browse files
coopernetesclaude
andauthored
fix: ForwardedHeaderFilter for OIDC behind TLS ingress, gitleaks URL override, graceful SIGTERM shutdown (#163)
* fix(oidc,security,build): ForwardedHeaderFilter, redact diff violations, gitleaks URL override ForwardedHeaderFilter (GitProxyWithDashboardApplication): Register Spring's ForwardedHeaderFilter as the first filter in the chain, ahead of Spring Session and Spring Security. When the app runs behind a TLS-terminating ingress (OCP Route, nginx, etc.) the pod only sees plain HTTP internally. Without this filter, Spring Security builds OAuth2 redirect URIs with an http:// scheme, which Entra ID refuses to process for non-localhost origins. The filter consumes X-Forwarded-Proto, X-Forwarded-Host, and X-Forwarded-Port from the ingress and rewraps the request so all downstream code — including the OAuth2 authorization request resolver — sees the correct public-facing scheme and host. Redact regex pattern from blocked-content violation messages (BlockedContentDiffCheck): Violation messages previously included the raw regex pattern string ("blocked pattern: (?i)(password|secret...)"). This leaks internal scanning rules to the committer. Changed to a generic "blocked pattern match" message. The location (file path) is still included so the committer knows which file triggered the check. Gitleaks download URL override (build.gradle): The gitleaks binary download URL is now overridable via the GITLEAKS_DOWNLOAD_URL environment variable. This allows CI environments without direct access to github.com releases to redirect the download to a corporate artifact mirror (e.g. Artifactory) without modifying the build script. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(jetty): graceful shutdown on SIGTERM with 30s drain period Both server entry points (GitProxyJettyApplication and GitProxyWithDashboardApplication) now configure Jetty for graceful shutdown: - server.setStopAtShutdown(true): registers a JVM shutdown hook so that SIGTERM triggers server.stop() rather than an abrupt JVM exit. Without this the OS kills the process immediately on SIGTERM regardless of in-flight work. - server.setStopTimeout(30_000): gives Jetty up to 30s to drain active connections before the stop completes. Requests still being processed when SIGTERM arrives (e.g. a slow git push) are allowed to finish within that window; any that exceed 30s are forcibly closed. This matches the behaviour Spring Boot provides via server.shutdown=graceful + spring.lifecycle.timeout-per-shutdown-phase. It is especially important on Kubernetes/OCP where the kubelet sends SIGTERM during rolling deploys and pod eviction — previously any in-flight git push or proxy stream would be torn down mid-transfer, leaving the client with a broken pack. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * revert: restore BlockedContentDiffCheck violation message Reverts the regex-redaction change introduced in d34d139 — saving it for a dedicated PR with fuller context for issue #152. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 5a8c52f commit 754f43c

3 files changed

Lines changed: 25 additions & 1 deletion

File tree

git-proxy-java-core/build.gradle

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -249,7 +249,11 @@ tasks.register('downloadGitleaks') {
249249
out.parentFile.mkdirs()
250250

251251
def tarName = "gitleaks_${gitleaksVersion}_${gitleaksTarSuffix}.tar.gz"
252-
def tarUrl = "https://github.com/gitleaks/gitleaks/releases/download/v${gitleaksVersion}/${tarName}"
252+
def gitleaksDownloadUrl = 'https://github.com/gitleaks/gitleaks/releases/download'
253+
if (System.getenv('GITLEAKS_DOWNLOAD_URL')) {
254+
gitleaksDownloadUrl = System.getenv('GITLEAKS_DOWNLOAD_URL')
255+
}
256+
def tarUrl = "${gitleaksDownloadUrl}/v${gitleaksVersion}/${tarName}"
253257
def tmpDir = layout.buildDirectory.dir("tmp/gitleaks").get().asFile
254258
tmpDir.mkdirs()
255259
def tarFile = new File(tmpDir, tarName)

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import org.springframework.security.web.context.AbstractSecurityWebApplicationInitializer;
3030
import org.springframework.web.context.support.AnnotationConfigWebApplicationContext;
3131
import org.springframework.web.filter.DelegatingFilterProxy;
32+
import org.springframework.web.filter.ForwardedHeaderFilter;
3233
import org.springframework.web.servlet.DispatcherServlet;
3334

3435
/**
@@ -61,6 +62,11 @@ public static void main(String[] args) throws Exception {
6162
connector.setPort(configBuilder.getServerPort());
6263
server.addConnector(connector);
6364

65+
// Graceful shutdown: drain in-flight requests for up to 30s on SIGTERM before the JVM exits.
66+
// Without this, rolling deploys on Kubernetes/OCP hard-kill active git push/proxy streams.
67+
server.setStopTimeout(30_000);
68+
server.setStopAtShutdown(true);
69+
6470
var tls = configBuilder.getTlsConfig();
6571
if (tls.isServerTlsConfigured()) {
6672
server.addConnector(GitProxyJettyApplication.buildHttpsConnector(server, tls));
@@ -186,6 +192,15 @@ public void contextDestroyed(ServletContextEvent sce) {
186192
holder.setInitOrder(1);
187193
context.addServlet(holder, "/*");
188194

195+
// ForwardedHeaderFilter — must be registered first so that X-Forwarded-Proto/Host/Port
196+
// headers from TLS-terminating ingress (OCP Route, nginx, etc.) are resolved before any
197+
// downstream filter builds absolute URLs (e.g. OAuth2 redirect URIs in Spring Security).
198+
var forwardedHeaderFilter = new FilterHolder(new ForwardedHeaderFilter());
199+
forwardedHeaderFilter.setAsyncSupported(true);
200+
for (String path : new String[] {"/api/*", "/login", "/logout", "/", "/oauth2/*", "/login/oauth2/*"}) {
201+
context.addFilter(forwardedHeaderFilter, path, EnumSet.of(DispatcherType.REQUEST, DispatcherType.ERROR));
202+
}
203+
189204
// Spring Session filter — must be registered before Spring Security so that the distributed
190205
// session store is in place before Security reads authentication state from the session.
191206
// The filter is always wired (even for session-store=none, where it uses an in-memory store

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,11 @@ public static void main(String[] args) throws Exception {
5353
connector.setPort(configBuilder.getServerPort());
5454
server.addConnector(connector);
5555

56+
// Graceful shutdown: drain in-flight requests for up to 30s on SIGTERM before the JVM exits.
57+
// Without this, rolling deploys on Kubernetes/OCP hard-kill active git push/proxy streams.
58+
server.setStopTimeout(30_000);
59+
server.setStopAtShutdown(true);
60+
5661
TlsConfig tls = configBuilder.getTlsConfig();
5762
if (tls.isServerTlsConfigured()) {
5863
server.addConnector(buildHttpsConnector(server, tls));

0 commit comments

Comments
 (0)