From fe3e47e115d01a5e2e435a0d6e7ad07ff98811d1 Mon Sep 17 00:00:00 2001 From: Thomas Cooper Date: Sun, 19 Apr 2026 22:24:57 -0400 Subject: [PATCH 1/2] fix: move per-request credentials from shared Repository config to PushContext MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit closes #176 Repository objects are cached and shared across concurrent pushes to the same upstream in LocalRepositoryCache. Writing per-request values (pushUser, pushToken, repoSlug, pushId, resolvedUser, scmUsername, validationRecordId, upstreamUser) to db.getConfig() raced under concurrent load — request A's token could be overwritten by request B before A's hooks had a chance to read it. Fix: store all per-request transient values on PushContext, which is created fresh per request in StoreAndForwardReceivePackFactory. upstreamUrl stays in repo config as it is per-repo (not per-request) and is intentionally persisted to disk. Co-Authored-By: Claude Sonnet 4.6 --- .../gitproxy/git/ApprovalPreReceiveHook.java | 36 +++++++++--- .../git/BitbucketCredentialRewriteHook.java | 10 ++-- .../git/CheckUserPushPermissionHook.java | 11 ++-- .../git/ForwardingPostReceiveHook.java | 4 +- .../git/IdentityVerificationHook.java | 5 +- .../org/finos/gitproxy/git/PushContext.java | 29 +++++++++- .../git/PushStorePersistenceHook.java | 23 ++++---- .../gitproxy/git/RepositoryUrlRuleHook.java | 4 +- .../StoreAndForwardReceivePackFactory.java | 33 +++++------ .../git/ApprovalPreReceiveHookTest.java | 55 ++++++++++--------- .../git/CheckUserPushPermissionHookTest.java | 23 +++----- .../git/IdentityVerificationHookTest.java | 21 +++---- .../git/PushStorePersistenceHookTest.java | 11 ++-- .../git/RepositoryUrlRuleHookTest.java | 15 ++--- 14 files changed, 149 insertions(+), 131 deletions(-) diff --git a/git-proxy-java-core/src/main/java/org/finos/gitproxy/git/ApprovalPreReceiveHook.java b/git-proxy-java-core/src/main/java/org/finos/gitproxy/git/ApprovalPreReceiveHook.java index c4c9141c..2d4ee47a 100644 --- a/git-proxy-java-core/src/main/java/org/finos/gitproxy/git/ApprovalPreReceiveHook.java +++ b/git-proxy-java-core/src/main/java/org/finos/gitproxy/git/ApprovalPreReceiveHook.java @@ -45,13 +45,14 @@ public class ApprovalPreReceiveHook implements PreReceiveHook { private final Duration timeout; private final String serviceUrl; private final RepoPermissionService repoPermissionService; + private final PushContext pushContext; public ApprovalPreReceiveHook(PushStore pushStore, ApprovalGateway approvalGateway) { - this(pushStore, approvalGateway, DEFAULT_TIMEOUT, null, null); + this(pushStore, approvalGateway, DEFAULT_TIMEOUT, null, null, null); } public ApprovalPreReceiveHook(PushStore pushStore, ApprovalGateway approvalGateway, String serviceUrl) { - this(pushStore, approvalGateway, DEFAULT_TIMEOUT, serviceUrl, null); + this(pushStore, approvalGateway, DEFAULT_TIMEOUT, serviceUrl, null, null); } public ApprovalPreReceiveHook( @@ -59,16 +60,25 @@ public ApprovalPreReceiveHook( ApprovalGateway approvalGateway, String serviceUrl, RepoPermissionService repoPermissionService) { - this(pushStore, approvalGateway, DEFAULT_TIMEOUT, serviceUrl, repoPermissionService); + this(pushStore, approvalGateway, DEFAULT_TIMEOUT, serviceUrl, repoPermissionService, null); + } + + public ApprovalPreReceiveHook( + PushStore pushStore, + ApprovalGateway approvalGateway, + String serviceUrl, + RepoPermissionService repoPermissionService, + PushContext pushContext) { + this(pushStore, approvalGateway, DEFAULT_TIMEOUT, serviceUrl, repoPermissionService, pushContext); } public ApprovalPreReceiveHook(PushStore pushStore, ApprovalGateway approvalGateway, Duration timeout) { - this(pushStore, approvalGateway, timeout, null, null); + this(pushStore, approvalGateway, timeout, null, null, null); } public ApprovalPreReceiveHook( PushStore pushStore, ApprovalGateway approvalGateway, Duration timeout, String serviceUrl) { - this(pushStore, approvalGateway, timeout, serviceUrl, null); + this(pushStore, approvalGateway, timeout, serviceUrl, null, null); } public ApprovalPreReceiveHook( @@ -77,21 +87,31 @@ public ApprovalPreReceiveHook( Duration timeout, String serviceUrl, RepoPermissionService repoPermissionService) { + this(pushStore, approvalGateway, timeout, serviceUrl, repoPermissionService, null); + } + + public ApprovalPreReceiveHook( + PushStore pushStore, + ApprovalGateway approvalGateway, + Duration timeout, + String serviceUrl, + RepoPermissionService repoPermissionService, + PushContext pushContext) { this.pushStore = pushStore; this.approvalGateway = approvalGateway; this.timeout = timeout; this.serviceUrl = serviceUrl; this.repoPermissionService = repoPermissionService; + this.pushContext = pushContext; } @Override public void onPreReceive(ReceivePack rp, Collection commands) { OutputStream msgOut = rp.getMessageOutputStream(); - // Read the validation record ID stored by PushStorePersistenceHook.validationResultHook - String validationRecordId = rp.getRepository().getConfig().getString("gitproxy", null, "validationRecordId"); + String validationRecordId = pushContext != null ? pushContext.getValidationRecordId() : null; if (validationRecordId == null) { - log.warn("No validationRecordId in repo config - skipping approval gate"); + log.warn("No validationRecordId in push context - skipping approval gate"); return; } diff --git a/git-proxy-java-core/src/main/java/org/finos/gitproxy/git/BitbucketCredentialRewriteHook.java b/git-proxy-java-core/src/main/java/org/finos/gitproxy/git/BitbucketCredentialRewriteHook.java index 04eaf016..1d17756c 100644 --- a/git-proxy-java-core/src/main/java/org/finos/gitproxy/git/BitbucketCredentialRewriteHook.java +++ b/git-proxy-java-core/src/main/java/org/finos/gitproxy/git/BitbucketCredentialRewriteHook.java @@ -30,6 +30,7 @@ public class BitbucketCredentialRewriteHook implements GitProxyHook { private static final int ORDER = 148; private final BitbucketProvider provider; + private final PushContext pushContext; @Override public int getOrder() { @@ -43,12 +44,11 @@ public String getName() { @Override public void onPreReceive(ReceivePack rp, Collection commands) { - var repo = rp.getRepository(); - String pushEmail = repo.getConfig().getString("gitproxy", null, "pushUser"); - String pushToken = repo.getConfig().getString("gitproxy", null, "pushToken"); + String pushEmail = pushContext.getPushUser(); + String pushToken = pushContext.getPushToken(); if (pushEmail == null || pushToken == null) { - log.debug("No push credentials in repo config — skipping Bitbucket upstream username resolution"); + log.debug("No push credentials in context — skipping Bitbucket upstream username resolution"); return; } @@ -61,7 +61,7 @@ public void onPreReceive(ReceivePack rp, Collection commands) { } String upstreamUser = identity.get().login(); - repo.getConfig().setString("gitproxy", null, "upstreamUser", upstreamUser); + pushContext.setUpstreamUser(upstreamUser); log.debug("Stored Bitbucket upstream username '{}' for push email '{}'", upstreamUser, pushEmail); } } diff --git a/git-proxy-java-core/src/main/java/org/finos/gitproxy/git/CheckUserPushPermissionHook.java b/git-proxy-java-core/src/main/java/org/finos/gitproxy/git/CheckUserPushPermissionHook.java index 5f51e5d6..d4b9390d 100644 --- a/git-proxy-java-core/src/main/java/org/finos/gitproxy/git/CheckUserPushPermissionHook.java +++ b/git-proxy-java-core/src/main/java/org/finos/gitproxy/git/CheckUserPushPermissionHook.java @@ -64,10 +64,9 @@ public CheckUserPushPermissionHook( @Override public void onPreReceive(ReceivePack rp, Collection commands) { - var config = rp.getRepository().getConfig(); - String pushUser = config.getString("gitproxy", null, "pushUser"); - String pushToken = config.getString("gitproxy", null, "pushToken"); - String repoSlug = config.getString("gitproxy", null, "repoSlug"); + String pushUser = pushContext.getPushUser(); + String pushToken = pushContext.getPushToken(); + String repoSlug = pushContext.getRepoSlug(); if (identityResolver == null) { log.debug("No identity resolver configured (open mode), skipping permission check"); @@ -140,13 +139,13 @@ public void onPreReceive(ReceivePack rp, Collection commands) { user.getUsername(), providerId, repoSlug); - config.setString("gitproxy", null, "resolvedUser", user.getUsername()); + pushContext.setResolvedUser(user.getUsername()); if (provider != null && user.getScmIdentities() != null) { user.getScmIdentities().stream() .filter(id -> provider.getProviderId().equalsIgnoreCase(id.getProvider())) .map(org.finos.gitproxy.user.ScmIdentity::getUsername) .findFirst() - .ifPresent(scmUser -> config.setString("gitproxy", null, "scmUsername", scmUser)); + .ifPresent(pushContext::setScmUsername); } pushContext.addStep(PushStep.builder() .stepName("checkUserPermission") diff --git a/git-proxy-java-core/src/main/java/org/finos/gitproxy/git/ForwardingPostReceiveHook.java b/git-proxy-java-core/src/main/java/org/finos/gitproxy/git/ForwardingPostReceiveHook.java index 8483e1be..acb78669 100644 --- a/git-proxy-java-core/src/main/java/org/finos/gitproxy/git/ForwardingPostReceiveHook.java +++ b/git-proxy-java-core/src/main/java/org/finos/gitproxy/git/ForwardingPostReceiveHook.java @@ -68,10 +68,10 @@ public void onPostReceive(ReceivePack rp, Collection commands) { Repository repo = rp.getRepository(); // If BitbucketCredentialRewriteHook resolved an upstream username, use it instead of the push email. - String upstreamUser = repo.getConfig().getString("gitproxy", null, "upstreamUser"); + String upstreamUser = pushContext.getUpstreamUser(); CredentialsProvider effectiveCreds = credentials; if (upstreamUser != null) { - String pushToken = repo.getConfig().getString("gitproxy", null, "pushToken"); + String pushToken = pushContext.getPushToken(); effectiveCreds = new UsernamePasswordCredentialsProvider(upstreamUser, pushToken != null ? pushToken : ""); log.debug("Using Bitbucket upstream username '{}' for forwarding credentials", upstreamUser); } diff --git a/git-proxy-java-core/src/main/java/org/finos/gitproxy/git/IdentityVerificationHook.java b/git-proxy-java-core/src/main/java/org/finos/gitproxy/git/IdentityVerificationHook.java index e21c8ff9..e2f36ac2 100644 --- a/git-proxy-java-core/src/main/java/org/finos/gitproxy/git/IdentityVerificationHook.java +++ b/git-proxy-java-core/src/main/java/org/finos/gitproxy/git/IdentityVerificationHook.java @@ -74,9 +74,8 @@ public void onPreReceive(ReceivePack rp, Collection commands) { return; } - var config = rp.getRepository().getConfig(); - String pushUser = config.getString("gitproxy", null, "pushUser"); - String pushToken = config.getString("gitproxy", null, "pushToken"); + String pushUser = pushContext.getPushUser(); + String pushToken = pushContext.getPushToken(); if (pushUser == null || pushUser.isEmpty()) { log.debug("No push user in repo config — skipping identity verification"); diff --git a/git-proxy-java-core/src/main/java/org/finos/gitproxy/git/PushContext.java b/git-proxy-java-core/src/main/java/org/finos/gitproxy/git/PushContext.java index bc96dc8a..3e1cf5bc 100644 --- a/git-proxy-java-core/src/main/java/org/finos/gitproxy/git/PushContext.java +++ b/git-proxy-java-core/src/main/java/org/finos/gitproxy/git/PushContext.java @@ -4,16 +4,41 @@ import java.util.Collections; import java.util.List; import java.util.Optional; + +import lombok.Data; import org.finos.gitproxy.db.model.PushStep; /** - * Shared context for accumulating {@link PushStep} records across pre-receive hooks within a single push. Hooks write - * steps (diffs, scan results, etc.) to this context, and the persistence hook reads them when saving the final record. + * Per-request context shared across all pre/post-receive hooks within a single push. Carries both accumulated + * {@link PushStep} records (diffs, scan results, etc.) and transient per-request values that must not be stored on the + * shared cached {@link org.eclipse.jgit.lib.Repository} config. + * + *

All fields are written once (by {@link StoreAndForwardReceivePackFactory} or early hooks) and read by later hooks. + * No synchronisation is needed because hooks in a single push execute sequentially on the same thread. */ +@Data public class PushContext { private final List steps = new ArrayList<>(); + // Per-request credentials — written by StoreAndForwardReceivePackFactory before any hook runs. + private String pushUser; + private String pushToken; + private String repoSlug; + + // Resolved upstream username for Bitbucket pushes — written by BitbucketCredentialRewriteHook. + private String upstreamUser; + + // Push record ID — written by PushStorePersistenceHook.preReceiveHook(). + private String pushId; + + // Identity resolved by CheckUserPushPermissionHook — written after permission check passes. + private String resolvedUser; + private String scmUsername; + + // Validation record ID — written by PushStorePersistenceHook.validationResultHook(). + private String validationRecordId; + /** Add a step to the context. */ public void addStep(PushStep step) { steps.add(step); diff --git a/git-proxy-java-core/src/main/java/org/finos/gitproxy/git/PushStorePersistenceHook.java b/git-proxy-java-core/src/main/java/org/finos/gitproxy/git/PushStorePersistenceHook.java index 577a9251..8e2ea963 100644 --- a/git-proxy-java-core/src/main/java/org/finos/gitproxy/git/PushStorePersistenceHook.java +++ b/git-proxy-java-core/src/main/java/org/finos/gitproxy/git/PushStorePersistenceHook.java @@ -80,8 +80,7 @@ public void setAutoApproval(boolean autoApproval) { public PreReceiveHook preReceiveHook() { return (ReceivePack rp, Collection commands) -> { String pushId = UUID.randomUUID().toString(); - // Store push ID in repo config so post-receive can find it - rp.getRepository().getConfig().setString("gitproxy", null, "pushId", pushId); + pushContext.setPushId(pushId); try { PushRecord record = buildInitialRecord(pushId, rp, commands); @@ -102,14 +101,14 @@ public PreReceiveHook preReceiveHook() { */ public PreReceiveHook validationResultHook(ValidationContext validationContext) { return (ReceivePack rp, Collection commands) -> { - String pushId = rp.getRepository().getConfig().getString("gitproxy", null, "pushId"); + String pushId = pushContext != null ? pushContext.getPushId() : null; if (pushId == null) return; - // Re-read resolvedUser and scmUsername here — both are set by CheckUserPushPermissionHook - // (order 150), which runs after preReceiveHook(), so they were not available when the - // RECEIVED record was written. validationResultHook fires after all validation hooks complete. - String resolvedUserLate = rp.getRepository().getConfig().getString("gitproxy", null, "resolvedUser"); - String scmUsernameLate = rp.getRepository().getConfig().getString("gitproxy", null, "scmUsername"); + // Read resolvedUser and scmUsername from pushContext — both are set by CheckUserPushPermissionHook + // (order 150) after preReceiveHook() ran, so they were not available when the RECEIVED record + // was written. validationResultHook fires after all validation hooks complete. + String resolvedUserLate = pushContext.getResolvedUser(); + String scmUsernameLate = pushContext.getScmUsername(); try { pushStore.findById(pushId).ifPresent(initial -> { @@ -156,9 +155,7 @@ public PreReceiveHook validationResultHook(ValidationContext validationContext) record.setBlockedMessage(validationContext.getIssues().size() + " validation issue(s) found"); record.setSteps(allSteps); pushStore.save(record); - rp.getRepository() - .getConfig() - .setString("gitproxy", null, "validationRecordId", record.getId()); + if (pushContext != null) pushContext.setValidationRecordId(record.getId()); log.debug( "Saved validation result record: id={}, status=REJECTED (auto-rejected)", record.getId()); @@ -221,7 +218,7 @@ public PreReceiveHook validationResultHook(ValidationContext validationContext) } pushStore.save(record); - rp.getRepository().getConfig().setString("gitproxy", null, "validationRecordId", record.getId()); + if (pushContext != null) pushContext.setValidationRecordId(record.getId()); log.debug( "Saved validation result record: id={}, status=PENDING (awaiting review)", record.getId()); }); @@ -239,7 +236,7 @@ public PreReceiveHook validationResultHook(ValidationContext validationContext) */ public PostReceiveHook postReceiveHook() { return (ReceivePack rp, Collection commands) -> { - String pushId = rp.getRepository().getConfig().getString("gitproxy", null, "pushId"); + String pushId = pushContext != null ? pushContext.getPushId() : null; if (pushId == null) return; try { diff --git a/git-proxy-java-core/src/main/java/org/finos/gitproxy/git/RepositoryUrlRuleHook.java b/git-proxy-java-core/src/main/java/org/finos/gitproxy/git/RepositoryUrlRuleHook.java index 4590a2b5..e59d1df9 100644 --- a/git-proxy-java-core/src/main/java/org/finos/gitproxy/git/RepositoryUrlRuleHook.java +++ b/git-proxy-java-core/src/main/java/org/finos/gitproxy/git/RepositoryUrlRuleHook.java @@ -41,9 +41,9 @@ public RepositoryUrlRuleHook( @Override public void onPreReceive(ReceivePack rp, Collection commands) { - String repoSlug = rp.getRepository().getConfig().getString("gitproxy", null, "repoSlug"); + String repoSlug = pushContext.getRepoSlug(); if (repoSlug == null || repoSlug.isBlank()) { - log.warn("No repoSlug in repo config — cannot evaluate URL rules, blocking push (fail-closed)"); + log.warn("No repoSlug in push context — cannot evaluate URL rules, blocking push (fail-closed)"); blockPush(rp, commands, "Repository path unavailable"); return; } diff --git a/git-proxy-java-core/src/main/java/org/finos/gitproxy/git/StoreAndForwardReceivePackFactory.java b/git-proxy-java-core/src/main/java/org/finos/gitproxy/git/StoreAndForwardReceivePackFactory.java index 4a91fe5a..2676441e 100644 --- a/git-proxy-java-core/src/main/java/org/finos/gitproxy/git/StoreAndForwardReceivePackFactory.java +++ b/git-proxy-java-core/src/main/java/org/finos/gitproxy/git/StoreAndForwardReceivePackFactory.java @@ -157,35 +157,27 @@ public ReceivePack create(HttpServletRequest req, Repository db) creds = extractBasicAuth(req); } - // Store push credentials in repo config so hooks can read them for identity resolution. - // pushToken is the PAT/password — stored only in-process repo config, never persisted to disk. + // Per-request shared contexts — created before credentials are extracted so they can be + // stored on pushContext rather than the shared cached Repository config. + var validationContext = new ValidationContext(); + var pushContext = new PushContext(); + + // Store per-request credentials on pushContext, not the shared Repository config. + // Writing to db.getConfig() would race with concurrent pushes to the same cached repo. String[] userPass = extractUserPass(req); - String pushUser = userPass != null ? userPass[0] : null; - String pushToken = userPass != null ? userPass[1] : null; - if (pushUser != null) { - db.getConfig().setString("gitproxy", null, "pushUser", pushUser); - } - if (pushToken != null) { - db.getConfig().setString("gitproxy", null, "pushToken", pushToken); - } + pushContext.setPushUser(userPass != null ? userPass[0] : null); + pushContext.setPushToken(userPass != null ? userPass[1] : null); - // Store the repo slug (/owner/repo) so permission hooks can read it without re-parsing the URL. String pathInfo = req.getPathInfo(); if (pathInfo != null) { - // pathInfo is e.g. /owner/repo.git — strip .git suffix String slug = pathInfo.replaceAll("\\.git$", ""); - // Normalise to /owner/repo (at most two path segments after leading /) String[] segments = slug.split("/", 4); if (segments.length >= 3) { slug = "/" + segments[1] + "/" + segments[2]; } - db.getConfig().setString("gitproxy", null, "repoSlug", slug); + pushContext.setRepoSlug(slug); } - // Per-request shared contexts - var validationContext = new ValidationContext(); - var pushContext = new PushContext(); - // Persistence hook (records push to database) var persistenceHook = pushStore != null ? new PushStorePersistenceHook(pushStore, provider) : null; if (persistenceHook != null) { @@ -247,7 +239,7 @@ public ReceivePack create(HttpServletRequest req, Repository db) new GpgSignatureHook(gpgConfig, validationContext, pushContext), new SecretScanningHook(secretScanConfig, validationContext, pushContext))); if (provider instanceof BitbucketProvider bitbucketProvider) { - validationHooks.add(new BitbucketCredentialRewriteHook(bitbucketProvider)); + validationHooks.add(new BitbucketCredentialRewriteHook(bitbucketProvider, pushContext)); } validationHooks.sort(Comparator.comparingInt(GitProxyHook::getOrder)); @@ -257,7 +249,8 @@ public ReceivePack create(HttpServletRequest req, Repository db) hooks.add(persistenceHook.preReceiveHook()); hooks.addAll(validationHooks); hooks.add(persistenceHook.validationResultHook(validationContext)); - hooks.add(new ApprovalPreReceiveHook(pushStore, approvalGateway, serviceUrl, repoPermissionService)); + hooks.add(new ApprovalPreReceiveHook( + pushStore, approvalGateway, serviceUrl, repoPermissionService, pushContext)); preHooks = hooks.toArray(PreReceiveHook[]::new); } else { preHooks = validationHooks.toArray(PreReceiveHook[]::new); diff --git a/git-proxy-java-core/src/test/java/org/finos/gitproxy/git/ApprovalPreReceiveHookTest.java b/git-proxy-java-core/src/test/java/org/finos/gitproxy/git/ApprovalPreReceiveHookTest.java index ac0ae805..409cc40e 100644 --- a/git-proxy-java-core/src/test/java/org/finos/gitproxy/git/ApprovalPreReceiveHookTest.java +++ b/git-proxy-java-core/src/test/java/org/finos/gitproxy/git/ApprovalPreReceiveHookTest.java @@ -24,6 +24,7 @@ import org.finos.gitproxy.db.model.PushRecord; import org.finos.gitproxy.db.model.PushStatus; import org.finos.gitproxy.permission.RepoPermissionService; +import org.finos.gitproxy.git.PushContext; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; @@ -76,8 +77,8 @@ void noValidationRecordId_skipsApprovalGate() throws Exception { @Test void validationRecordNotInStore_skipsApprovalGate() throws Exception { String recordId = UUID.randomUUID().toString(); - repo.getConfig().setString("gitproxy", null, "validationRecordId", recordId); - repo.getConfig().save(); + PushContext pushContext = new PushContext(); + pushContext.setValidationRecordId(recordId); when(pushStore.findById(recordId)).thenReturn(Optional.empty()); RevCommit c1 = createCommit("init"); @@ -85,7 +86,7 @@ void validationRecordNotInStore_skipsApprovalGate() throws Exception { ReceivePack rp = new ReceivePack(repo); ReceiveCommand cmd = new ReceiveCommand(c1.getId(), c2.getId(), "refs/heads/main", ReceiveCommand.Type.UPDATE); - new ApprovalPreReceiveHook(pushStore, approvalGateway).onPreReceive(rp, List.of(cmd)); + new ApprovalPreReceiveHook(pushStore, approvalGateway, Duration.ofMinutes(30), null, null, pushContext).onPreReceive(rp, List.of(cmd)); assertEquals(ReceiveCommand.Result.NOT_ATTEMPTED, cmd.getResult()); verifyNoInteractions(approvalGateway); @@ -94,8 +95,8 @@ void validationRecordNotInStore_skipsApprovalGate() throws Exception { @Test void alreadyApproved_passesImmediately() throws Exception { String recordId = UUID.randomUUID().toString(); - repo.getConfig().setString("gitproxy", null, "validationRecordId", recordId); - repo.getConfig().save(); + PushContext pushContext = new PushContext(); + pushContext.setValidationRecordId(recordId); PushRecord record = PushRecord.builder().id(recordId).status(PushStatus.APPROVED).build(); when(pushStore.findById(recordId)).thenReturn(Optional.of(record)); @@ -105,7 +106,7 @@ void alreadyApproved_passesImmediately() throws Exception { ReceivePack rp = new ReceivePack(repo); ReceiveCommand cmd = new ReceiveCommand(c1.getId(), c2.getId(), "refs/heads/main", ReceiveCommand.Type.UPDATE); - new ApprovalPreReceiveHook(pushStore, approvalGateway).onPreReceive(rp, List.of(cmd)); + new ApprovalPreReceiveHook(pushStore, approvalGateway, Duration.ofMinutes(30), null, null, pushContext).onPreReceive(rp, List.of(cmd)); assertEquals(ReceiveCommand.Result.NOT_ATTEMPTED, cmd.getResult()); verifyNoInteractions(approvalGateway); @@ -114,8 +115,8 @@ void alreadyApproved_passesImmediately() throws Exception { @Test void blockedPush_gatewayApproves_commandNotRejected() throws Exception { String recordId = UUID.randomUUID().toString(); - repo.getConfig().setString("gitproxy", null, "validationRecordId", recordId); - repo.getConfig().save(); + PushContext pushContext = new PushContext(); + pushContext.setValidationRecordId(recordId); PushRecord record = PushRecord.builder().id(recordId).status(PushStatus.PENDING).build(); when(pushStore.findById(recordId)).thenReturn(Optional.of(record)); @@ -127,7 +128,7 @@ void blockedPush_gatewayApproves_commandNotRejected() throws Exception { ReceivePack rp = new ReceivePack(repo); ReceiveCommand cmd = new ReceiveCommand(c1.getId(), c2.getId(), "refs/heads/main", ReceiveCommand.Type.UPDATE); - new ApprovalPreReceiveHook(pushStore, approvalGateway, Duration.ofSeconds(5)).onPreReceive(rp, List.of(cmd)); + new ApprovalPreReceiveHook(pushStore, approvalGateway, Duration.ofSeconds(5), null, null, pushContext).onPreReceive(rp, List.of(cmd)); assertEquals(ReceiveCommand.Result.NOT_ATTEMPTED, cmd.getResult()); } @@ -135,8 +136,8 @@ void blockedPush_gatewayApproves_commandNotRejected() throws Exception { @Test void blockedPush_gatewayRejects_commandRejected() throws Exception { String recordId = UUID.randomUUID().toString(); - repo.getConfig().setString("gitproxy", null, "validationRecordId", recordId); - repo.getConfig().save(); + PushContext pushContext = new PushContext(); + pushContext.setValidationRecordId(recordId); PushRecord record = PushRecord.builder().id(recordId).status(PushStatus.PENDING).build(); PushRecord updatedRecord = @@ -150,7 +151,7 @@ void blockedPush_gatewayRejects_commandRejected() throws Exception { ReceivePack rp = new ReceivePack(repo); ReceiveCommand cmd = new ReceiveCommand(c1.getId(), c2.getId(), "refs/heads/main", ReceiveCommand.Type.UPDATE); - new ApprovalPreReceiveHook(pushStore, approvalGateway, Duration.ofSeconds(5)).onPreReceive(rp, List.of(cmd)); + new ApprovalPreReceiveHook(pushStore, approvalGateway, Duration.ofSeconds(5), null, null, pushContext).onPreReceive(rp, List.of(cmd)); assertEquals(ReceiveCommand.Result.REJECTED_OTHER_REASON, cmd.getResult()); } @@ -160,8 +161,8 @@ void blockedPush_gatewayRejects_commandRejected() throws Exception { @Test void selfApproved_alreadyApprovedAtHookStart_noPerm_rejected() throws Exception { String recordId = UUID.randomUUID().toString(); - repo.getConfig().setString("gitproxy", null, "validationRecordId", recordId); - repo.getConfig().save(); + PushContext pushContext = new PushContext(); + pushContext.setValidationRecordId(recordId); Attestation att = Attestation.builder() .pushId(recordId) .type(Attestation.Type.APPROVAL) @@ -185,7 +186,7 @@ void selfApproved_alreadyApprovedAtHookStart_noPerm_rejected() throws Exception ReceivePack rp = new ReceivePack(repo); ReceiveCommand cmd = new ReceiveCommand(c1.getId(), c2.getId(), "refs/heads/main", ReceiveCommand.Type.UPDATE); - new ApprovalPreReceiveHook(pushStore, approvalGateway, Duration.ofSeconds(5), null, perms) + new ApprovalPreReceiveHook(pushStore, approvalGateway, Duration.ofSeconds(5), null, perms, pushContext) .onPreReceive(rp, List.of(cmd)); assertEquals(ReceiveCommand.Result.REJECTED_OTHER_REASON, cmd.getResult()); @@ -195,8 +196,8 @@ void selfApproved_alreadyApprovedAtHookStart_noPerm_rejected() throws Exception @Test void selfApproved_alreadyApprovedAtHookStart_withPerm_passes() throws Exception { String recordId = UUID.randomUUID().toString(); - repo.getConfig().setString("gitproxy", null, "validationRecordId", recordId); - repo.getConfig().save(); + PushContext pushContext = new PushContext(); + pushContext.setValidationRecordId(recordId); Attestation att = Attestation.builder() .pushId(recordId) .type(Attestation.Type.APPROVAL) @@ -220,7 +221,7 @@ void selfApproved_alreadyApprovedAtHookStart_withPerm_passes() throws Exception ReceivePack rp = new ReceivePack(repo); ReceiveCommand cmd = new ReceiveCommand(c1.getId(), c2.getId(), "refs/heads/main", ReceiveCommand.Type.UPDATE); - new ApprovalPreReceiveHook(pushStore, approvalGateway, Duration.ofSeconds(5), null, perms) + new ApprovalPreReceiveHook(pushStore, approvalGateway, Duration.ofSeconds(5), null, perms, pushContext) .onPreReceive(rp, List.of(cmd)); assertEquals(ReceiveCommand.Result.NOT_ATTEMPTED, cmd.getResult()); @@ -229,8 +230,8 @@ void selfApproved_alreadyApprovedAtHookStart_withPerm_passes() throws Exception @Test void selfApproved_viaWaitForApproval_noPerm_rejected() throws Exception { String recordId = UUID.randomUUID().toString(); - repo.getConfig().setString("gitproxy", null, "validationRecordId", recordId); - repo.getConfig().save(); + PushContext pushContext = new PushContext(); + pushContext.setValidationRecordId(recordId); // Initial fetch returns PENDING (no attestation yet); after approval, returns APPROVED with attestation // showing the pusher self-approved. PushRecord pending = PushRecord.builder() @@ -265,7 +266,7 @@ void selfApproved_viaWaitForApproval_noPerm_rejected() throws Exception { ReceivePack rp = new ReceivePack(repo); ReceiveCommand cmd = new ReceiveCommand(c1.getId(), c2.getId(), "refs/heads/main", ReceiveCommand.Type.UPDATE); - new ApprovalPreReceiveHook(pushStore, approvalGateway, Duration.ofSeconds(5), null, perms) + new ApprovalPreReceiveHook(pushStore, approvalGateway, Duration.ofSeconds(5), null, perms, pushContext) .onPreReceive(rp, List.of(cmd)); assertEquals(ReceiveCommand.Result.REJECTED_OTHER_REASON, cmd.getResult()); @@ -276,8 +277,8 @@ void selfApproved_viaWaitForApproval_noPerm_rejected() throws Exception { void differentApproverThanPusher_noReVerifyNeeded() throws Exception { // Approver != pusher → defense-in-depth check skipped; push is forwarded. String recordId = UUID.randomUUID().toString(); - repo.getConfig().setString("gitproxy", null, "validationRecordId", recordId); - repo.getConfig().save(); + PushContext pushContext = new PushContext(); + pushContext.setValidationRecordId(recordId); Attestation att = Attestation.builder() .pushId(recordId) .type(Attestation.Type.APPROVAL) @@ -299,7 +300,7 @@ void differentApproverThanPusher_noReVerifyNeeded() throws Exception { ReceivePack rp = new ReceivePack(repo); ReceiveCommand cmd = new ReceiveCommand(c1.getId(), c2.getId(), "refs/heads/main", ReceiveCommand.Type.UPDATE); - new ApprovalPreReceiveHook(pushStore, approvalGateway, Duration.ofSeconds(5), null, perms) + new ApprovalPreReceiveHook(pushStore, approvalGateway, Duration.ofSeconds(5), null, perms, pushContext) .onPreReceive(rp, List.of(cmd)); assertEquals(ReceiveCommand.Result.NOT_ATTEMPTED, cmd.getResult()); @@ -309,8 +310,8 @@ void differentApproverThanPusher_noReVerifyNeeded() throws Exception { @Test void blockedPush_gatewayTimesOut_commandRejected() throws Exception { String recordId = UUID.randomUUID().toString(); - repo.getConfig().setString("gitproxy", null, "validationRecordId", recordId); - repo.getConfig().save(); + PushContext pushContext = new PushContext(); + pushContext.setValidationRecordId(recordId); PushRecord record = PushRecord.builder().id(recordId).status(PushStatus.PENDING).build(); when(pushStore.findById(recordId)).thenReturn(Optional.of(record)); @@ -322,7 +323,7 @@ void blockedPush_gatewayTimesOut_commandRejected() throws Exception { ReceivePack rp = new ReceivePack(repo); ReceiveCommand cmd = new ReceiveCommand(c1.getId(), c2.getId(), "refs/heads/main", ReceiveCommand.Type.UPDATE); - new ApprovalPreReceiveHook(pushStore, approvalGateway, Duration.ofSeconds(5)).onPreReceive(rp, List.of(cmd)); + new ApprovalPreReceiveHook(pushStore, approvalGateway, Duration.ofSeconds(5), null, null, pushContext).onPreReceive(rp, List.of(cmd)); assertEquals(ReceiveCommand.Result.REJECTED_OTHER_REASON, cmd.getResult()); } diff --git a/git-proxy-java-core/src/test/java/org/finos/gitproxy/git/CheckUserPushPermissionHookTest.java b/git-proxy-java-core/src/test/java/org/finos/gitproxy/git/CheckUserPushPermissionHookTest.java index 9b212603..006ce061 100644 --- a/git-proxy-java-core/src/test/java/org/finos/gitproxy/git/CheckUserPushPermissionHookTest.java +++ b/git-proxy-java-core/src/test/java/org/finos/gitproxy/git/CheckUserPushPermissionHookTest.java @@ -94,8 +94,6 @@ void noPushUser_failsClosed() throws Exception { @Test void resolverReturnsEmpty_addsNotRegisteredIssue() throws Exception { - repo.getConfig().setString("gitproxy", null, "pushUser", "unknown-user"); - repo.getConfig().save(); when(resolver.resolve(nullable(GitProxyProvider.class), eq("unknown-user"), any())) .thenReturn(Optional.empty()); @@ -104,6 +102,7 @@ void resolverReturnsEmpty_addsNotRegisteredIssue() throws Exception { ReceivePack rp = new ReceivePack(repo); ReceiveCommand cmd = new ReceiveCommand(c1.getId(), c2.getId(), "refs/heads/main", ReceiveCommand.Type.UPDATE); PushContext pushContext = new PushContext(); + pushContext.setPushUser("unknown-user"); ValidationContext validationContext = new ValidationContext(); hook(validationContext, pushContext).onPreReceive(rp, List.of(cmd)); @@ -122,9 +121,6 @@ void resolverReturnsEmpty_addsNotRegisteredIssue() throws Exception { @Test void userNotAuthorized_addsUnauthorizedIssue() throws Exception { - repo.getConfig().setString("gitproxy", null, "pushUser", "corp-user"); - repo.getConfig().setString("gitproxy", null, "repoSlug", "/owner/repo"); - repo.getConfig().save(); GitProxyProvider github = new GitHubProvider("/push"); when(resolver.resolve(eq(github), eq("corp-user"), any())).thenReturn(Optional.of(userEntry("alice"))); when(permService.isAllowedToPush("alice", "github/github.com", "/owner/repo")) @@ -135,6 +131,8 @@ void userNotAuthorized_addsUnauthorizedIssue() throws Exception { ReceivePack rp = new ReceivePack(repo); ReceiveCommand cmd = new ReceiveCommand(c1.getId(), c2.getId(), "refs/heads/main", ReceiveCommand.Type.UPDATE); PushContext pushContext = new PushContext(); + pushContext.setPushUser("corp-user"); + pushContext.setRepoSlug("/owner/repo"); ValidationContext validationContext = new ValidationContext(); new CheckUserPushPermissionHook(resolver, permService, validationContext, pushContext, github, null) @@ -153,10 +151,6 @@ void userNotAuthorized_addsUnauthorizedIssue() throws Exception { @Test void resolvedAndAuthorized_recordsPass() throws Exception { - repo.getConfig().setString("gitproxy", null, "pushUser", "corp-user"); - repo.getConfig().setString("gitproxy", null, "pushToken", "ghp_secret"); - repo.getConfig().setString("gitproxy", null, "repoSlug", "/owner/repo"); - repo.getConfig().save(); GitProxyProvider github = new GitHubProvider("/push"); when(resolver.resolve(eq(github), eq("corp-user"), eq("ghp_secret"))) .thenReturn(Optional.of(userEntry("alice"))); @@ -168,6 +162,9 @@ void resolvedAndAuthorized_recordsPass() throws Exception { ReceivePack rp = new ReceivePack(repo); ReceiveCommand cmd = new ReceiveCommand(c1.getId(), c2.getId(), "refs/heads/main", ReceiveCommand.Type.UPDATE); PushContext pushContext = new PushContext(); + pushContext.setPushUser("corp-user"); + pushContext.setPushToken("ghp_secret"); + pushContext.setRepoSlug("/owner/repo"); ValidationContext validationContext = new ValidationContext(); new CheckUserPushPermissionHook(resolver, permService, validationContext, pushContext, github, null) @@ -182,9 +179,6 @@ void resolvedAndAuthorized_recordsPass() throws Exception { @Test void nullResolver_withPushUser_passesInOpenMode() throws Exception { - repo.getConfig().setString("gitproxy", null, "pushUser", "anyone"); - repo.getConfig().save(); - RevCommit c1 = createCommit("init"); RevCommit c2 = createCommit("second"); ReceivePack rp = new ReceivePack(repo); @@ -205,9 +199,6 @@ void nullResolver_withPushUser_passesInOpenMode() throws Exception { @Test void provider_isPassedToResolver() throws Exception { - repo.getConfig().setString("gitproxy", null, "pushUser", "my-user"); - repo.getConfig().setString("gitproxy", null, "repoSlug", "/owner/repo"); - repo.getConfig().save(); GitProxyProvider github = new GitHubProvider("/push"); when(resolver.resolve(eq(github), eq("my-user"), any())).thenReturn(Optional.of(userEntry("my-user"))); when(permService.isAllowedToPush("my-user", "github/github.com", "/owner/repo")) @@ -218,6 +209,8 @@ void provider_isPassedToResolver() throws Exception { ReceivePack rp = new ReceivePack(repo); ReceiveCommand cmd = new ReceiveCommand(c1.getId(), c2.getId(), "refs/heads/main", ReceiveCommand.Type.UPDATE); PushContext pushContext = new PushContext(); + pushContext.setPushUser("my-user"); + pushContext.setRepoSlug("/owner/repo"); ValidationContext validationContext = new ValidationContext(); new CheckUserPushPermissionHook(resolver, permService, validationContext, pushContext, github, null) diff --git a/git-proxy-java-core/src/test/java/org/finos/gitproxy/git/IdentityVerificationHookTest.java b/git-proxy-java-core/src/test/java/org/finos/gitproxy/git/IdentityVerificationHookTest.java index 742e6477..75010bf8 100644 --- a/git-proxy-java-core/src/test/java/org/finos/gitproxy/git/IdentityVerificationHookTest.java +++ b/git-proxy-java-core/src/test/java/org/finos/gitproxy/git/IdentityVerificationHookTest.java @@ -130,8 +130,6 @@ void noPushUser_skipsCheck_recordsPass() throws Exception { @Test void emailsMatch_recordsPass() throws Exception { - repo.getConfig().setString("gitproxy", null, "pushUser", "alice-git"); - repo.getConfig().save(); when(resolver.resolve(any(GitProxyProvider.class), eq("alice-git"), isNull())) .thenReturn(Optional.of(alice())); @@ -140,6 +138,7 @@ void emailsMatch_recordsPass() throws Exception { ReceivePack rp = new ReceivePack(repo); ReceiveCommand cmd = new ReceiveCommand(c1.getId(), c2.getId(), "refs/heads/main", ReceiveCommand.Type.UPDATE); PushContext pc = new PushContext(); + pc.setPushUser("alice-git"); ValidationContext vc = new ValidationContext(); hook(CommitConfig.IdentityVerificationMode.STRICT, vc, pc).onPreReceive(rp, List.of(cmd)); @@ -153,8 +152,6 @@ void emailsMatch_recordsPass() throws Exception { @Test void strictMode_emailMismatch_addsIssue() throws Exception { - repo.getConfig().setString("gitproxy", null, "pushUser", "alice-git"); - repo.getConfig().save(); when(resolver.resolve(any(GitProxyProvider.class), eq("alice-git"), isNull())) .thenReturn(Optional.of(alice())); @@ -163,6 +160,7 @@ void strictMode_emailMismatch_addsIssue() throws Exception { ReceivePack rp = new ReceivePack(repo); ReceiveCommand cmd = new ReceiveCommand(c1.getId(), c2.getId(), "refs/heads/main", ReceiveCommand.Type.UPDATE); PushContext pc = new PushContext(); + pc.setPushUser("alice-git"); ValidationContext vc = new ValidationContext(); hook(CommitConfig.IdentityVerificationMode.STRICT, vc, pc).onPreReceive(rp, List.of(cmd)); @@ -176,8 +174,6 @@ void strictMode_emailMismatch_addsIssue() throws Exception { @Test void warnMode_emailMismatch_noIssue_recordsPass() throws Exception { - repo.getConfig().setString("gitproxy", null, "pushUser", "alice-git"); - repo.getConfig().save(); when(resolver.resolve(any(GitProxyProvider.class), eq("alice-git"), isNull())) .thenReturn(Optional.of(alice())); @@ -186,6 +182,7 @@ void warnMode_emailMismatch_noIssue_recordsPass() throws Exception { ReceivePack rp = new ReceivePack(repo); ReceiveCommand cmd = new ReceiveCommand(c1.getId(), c2.getId(), "refs/heads/main", ReceiveCommand.Type.UPDATE); PushContext pc = new PushContext(); + pc.setPushUser("alice-git"); ValidationContext vc = new ValidationContext(); hook(CommitConfig.IdentityVerificationMode.WARN, vc, pc).onPreReceive(rp, List.of(cmd)); @@ -199,8 +196,6 @@ void warnMode_emailMismatch_noIssue_recordsPass() throws Exception { @Test void deleteCommand_skipped() throws Exception { - repo.getConfig().setString("gitproxy", null, "pushUser", "alice-git"); - repo.getConfig().save(); when(resolver.resolve(any(GitProxyProvider.class), eq("alice-git"), isNull())) .thenReturn(Optional.of(alice())); @@ -210,6 +205,7 @@ void deleteCommand_skipped() throws Exception { ReceiveCommand cmd = new ReceiveCommand( c1.getId(), org.eclipse.jgit.lib.ObjectId.zeroId(), "refs/heads/main", ReceiveCommand.Type.DELETE); PushContext pc = new PushContext(); + pc.setPushUser("alice-git"); ValidationContext vc = new ValidationContext(); hook(CommitConfig.IdentityVerificationMode.STRICT, vc, pc).onPreReceive(rp, List.of(cmd)); @@ -221,8 +217,6 @@ void deleteCommand_skipped() throws Exception { @Test void strictMode_authorAndCommitterSameEmail_singleViolation() throws Exception { - repo.getConfig().setString("gitproxy", null, "pushUser", "alice-git"); - repo.getConfig().save(); when(resolver.resolve(any(GitProxyProvider.class), eq("alice-git"), isNull())) .thenReturn(Optional.of(alice())); // alice only has alice@example.com @@ -232,6 +226,7 @@ void strictMode_authorAndCommitterSameEmail_singleViolation() throws Exception { ReceivePack rp = new ReceivePack(repo); ReceiveCommand cmd = new ReceiveCommand(c1.getId(), c2.getId(), "refs/heads/main", ReceiveCommand.Type.UPDATE); PushContext pc = new PushContext(); + pc.setPushUser("alice-git"); ValidationContext vc = new ValidationContext(); hook(CommitConfig.IdentityVerificationMode.STRICT, vc, pc).onPreReceive(rp, List.of(cmd)); @@ -246,8 +241,6 @@ void strictMode_authorAndCommitterSameEmail_singleViolation() throws Exception { @Test void warnMode_emailMismatch_stepContentContainsViolations() throws Exception { - repo.getConfig().setString("gitproxy", null, "pushUser", "alice-git"); - repo.getConfig().save(); when(resolver.resolve(any(GitProxyProvider.class), eq("alice-git"), isNull())) .thenReturn(Optional.of(alice())); @@ -256,6 +249,7 @@ void warnMode_emailMismatch_stepContentContainsViolations() throws Exception { ReceivePack rp = new ReceivePack(repo); ReceiveCommand cmd = new ReceiveCommand(c1.getId(), c2.getId(), "refs/heads/main", ReceiveCommand.Type.UPDATE); PushContext pc = new PushContext(); + pc.setPushUser("alice-git"); ValidationContext vc = new ValidationContext(); hook(CommitConfig.IdentityVerificationMode.WARN, vc, pc).onPreReceive(rp, List.of(cmd)); @@ -274,8 +268,6 @@ void warnMode_emailMismatch_stepContentContainsViolations() throws Exception { @Test void resolverEmpty_skipsCheck_noStep() throws Exception { - repo.getConfig().setString("gitproxy", null, "pushUser", "unknown"); - repo.getConfig().save(); when(resolver.resolve(any(GitProxyProvider.class), eq("unknown"), any())) .thenReturn(Optional.empty()); @@ -284,6 +276,7 @@ void resolverEmpty_skipsCheck_noStep() throws Exception { ReceivePack rp = new ReceivePack(repo); ReceiveCommand cmd = new ReceiveCommand(c1.getId(), c2.getId(), "refs/heads/main", ReceiveCommand.Type.UPDATE); PushContext pc = new PushContext(); + pc.setPushUser("unknown"); ValidationContext vc = new ValidationContext(); hook(CommitConfig.IdentityVerificationMode.STRICT, vc, pc).onPreReceive(rp, List.of(cmd)); diff --git a/git-proxy-java-core/src/test/java/org/finos/gitproxy/git/PushStorePersistenceHookTest.java b/git-proxy-java-core/src/test/java/org/finos/gitproxy/git/PushStorePersistenceHookTest.java index f2cee36d..d16ddc84 100644 --- a/git-proxy-java-core/src/test/java/org/finos/gitproxy/git/PushStorePersistenceHookTest.java +++ b/git-proxy-java-core/src/test/java/org/finos/gitproxy/git/PushStorePersistenceHookTest.java @@ -43,6 +43,7 @@ class PushStorePersistenceHookTest { ObjectId commitId; PushStore pushStore; PushStorePersistenceHook hook; + PushContext pushContext; @BeforeEach void setUp() throws Exception { @@ -64,6 +65,8 @@ void setUp() throws Exception { pushStore = new InMemoryPushStore(); hook = new PushStorePersistenceHook(pushStore, new GitHubProvider("/push")); + pushContext = new PushContext(); + hook.setPushContext(pushContext); } private ReceivePack makeReceivePack() { @@ -84,7 +87,7 @@ void preReceiveHook_createsReceivedRecord() { hook.preReceiveHook().onPreReceive(rp, List.of(cmd)); // The push ID is stored in the repo config by the pre-receive hook - String pushId = repo.getConfig().getString("gitproxy", null, "pushId"); + String pushId = pushContext.getPushId(); assertNotNull(pushId, "push ID should be stamped into repo config"); var record = pushStore.findById(pushId); @@ -99,7 +102,7 @@ void preReceiveHook_branchAndCommitToAreRecorded() { hook.preReceiveHook().onPreReceive(rp, List.of(cmd)); - String pushId = repo.getConfig().getString("gitproxy", null, "pushId"); + String pushId = pushContext.getPushId(); var record = pushStore.findById(pushId).orElseThrow(); assertEquals("refs/heads/test", record.getBranch()); @@ -119,7 +122,7 @@ void validationResultHook_noIssues_transitionsToBlocked() { ValidationContext ctx = new ValidationContext(); // no issues hook.validationResultHook(ctx).onPreReceive(rp, List.of(cmd)); - String pushId = repo.getConfig().getString("gitproxy", null, "pushId"); + String pushId = pushContext.getPushId(); // The validation-result hook creates a new record with a fresh UUID (copyBase pattern); // we verify by querying for PENDING status rather than by ID. var records = pushStore.find(org.finos.gitproxy.db.model.PushQuery.builder() @@ -155,7 +158,7 @@ void validationResultHook_withIssues_transitionsToRejected() { ctx.addIssue("checkAuthorEmails", "Email blocked", "noreply@ address is not allowed"); hook.validationResultHook(ctx).onPreReceive(rp, List.of(cmd)); - String pushId = repo.getConfig().getString("gitproxy", null, "pushId"); + String pushId = pushContext.getPushId(); var records = pushStore.find(org.finos.gitproxy.db.model.PushQuery.builder() .status(PushStatus.REJECTED) .build()); diff --git a/git-proxy-java-core/src/test/java/org/finos/gitproxy/git/RepositoryUrlRuleHookTest.java b/git-proxy-java-core/src/test/java/org/finos/gitproxy/git/RepositoryUrlRuleHookTest.java index f2e4027b..71109b4d 100644 --- a/git-proxy-java-core/src/test/java/org/finos/gitproxy/git/RepositoryUrlRuleHookTest.java +++ b/git-proxy-java-core/src/test/java/org/finos/gitproxy/git/RepositoryUrlRuleHookTest.java @@ -60,8 +60,6 @@ void noRepoSlug_blocksWithFailClosed() { @Test void withRepoSlug_allowRuleMatches_recordsPass() throws Exception { - repo.getConfig().setString("gitproxy", null, "repoSlug", "/myorg/myrepo"); - repo.getConfig().save(); var allowRule = AccessRule.builder() .ruleOrder(100) @@ -70,6 +68,7 @@ void withRepoSlug_allowRuleMatches_recordsPass() throws Exception { .owner("myorg") .build(); var pushContext = new PushContext(); + pushContext.setRepoSlug("/myorg/myrepo"); var registry = new InMemoryUrlRuleRegistry(); registry.save(allowRule); var hook = new RepositoryUrlRuleHook(registry, GITHUB, null, pushContext); @@ -83,8 +82,6 @@ void withRepoSlug_allowRuleMatches_recordsPass() throws Exception { @Test void withRepoSlug_noMatchingAllowRule_rejectsCommand() throws Exception { - repo.getConfig().setString("gitproxy", null, "repoSlug", "/myorg/myrepo"); - repo.getConfig().save(); var allowRule = AccessRule.builder() .ruleOrder(100) @@ -93,6 +90,7 @@ void withRepoSlug_noMatchingAllowRule_rejectsCommand() throws Exception { .owner("other-org") .build(); var pushContext = new PushContext(); + pushContext.setRepoSlug("/myorg/myrepo"); var registry = new InMemoryUrlRuleRegistry(); registry.save(allowRule); var hook = new RepositoryUrlRuleHook(registry, GITHUB, null, pushContext); @@ -106,8 +104,6 @@ void withRepoSlug_noMatchingAllowRule_rejectsCommand() throws Exception { @Test void withRepoSlug_denyRuleAtLowerOrder_rejectsEvenWithAllowRule() throws Exception { - repo.getConfig().setString("gitproxy", null, "repoSlug", "/myorg/myrepo"); - repo.getConfig().save(); var denyRule = AccessRule.builder() .ruleOrder(100) @@ -122,6 +118,7 @@ void withRepoSlug_denyRuleAtLowerOrder_rejectsEvenWithAllowRule() throws Excepti .owner("myorg") .build(); var pushContext = new PushContext(); + pushContext.setRepoSlug("/myorg/myrepo"); var registry = new InMemoryUrlRuleRegistry(); registry.save(denyRule); registry.save(allowRule); @@ -136,8 +133,6 @@ void withRepoSlug_denyRuleAtLowerOrder_rejectsEvenWithAllowRule() throws Excepti @Test void fetchOnlyAllowRule_doesNotEngageForPush() throws Exception { - repo.getConfig().setString("gitproxy", null, "repoSlug", "/myorg/myrepo"); - repo.getConfig().save(); var fetchOnlyAllow = AccessRule.builder() .ruleOrder(100) @@ -146,6 +141,7 @@ void fetchOnlyAllowRule_doesNotEngageForPush() throws Exception { .owner("myorg") .build(); var pushContext = new PushContext(); + pushContext.setRepoSlug("/myorg/myrepo"); var registry = new InMemoryUrlRuleRegistry(); registry.save(fetchOnlyAllow); var hook = new RepositoryUrlRuleHook(registry, GITHUB, null, pushContext); @@ -159,8 +155,6 @@ void fetchOnlyAllowRule_doesNotEngageForPush() throws Exception { @Test void fetchOnlyDenyRule_doesNotBlockPush() throws Exception { - repo.getConfig().setString("gitproxy", null, "repoSlug", "/myorg/myrepo"); - repo.getConfig().save(); var fetchDeny = AccessRule.builder() .ruleOrder(100) @@ -175,6 +169,7 @@ void fetchOnlyDenyRule_doesNotBlockPush() throws Exception { .owner("myorg") .build(); var pushContext = new PushContext(); + pushContext.setRepoSlug("/myorg/myrepo"); var registry = new InMemoryUrlRuleRegistry(); registry.save(fetchDeny); registry.save(pushAllow); From 5235973899d60e139ba1985f55e12b8a724c91df Mon Sep 17 00:00:00 2001 From: Thomas Cooper Date: Sun, 19 Apr 2026 22:37:34 -0400 Subject: [PATCH 2/2] chore: apply spotless formatting --- .../java/org/finos/gitproxy/git/PushContext.java | 1 - .../gitproxy/git/ApprovalPreReceiveHookTest.java | 16 ++++++++++------ 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/git-proxy-java-core/src/main/java/org/finos/gitproxy/git/PushContext.java b/git-proxy-java-core/src/main/java/org/finos/gitproxy/git/PushContext.java index 3e1cf5bc..6646368e 100644 --- a/git-proxy-java-core/src/main/java/org/finos/gitproxy/git/PushContext.java +++ b/git-proxy-java-core/src/main/java/org/finos/gitproxy/git/PushContext.java @@ -4,7 +4,6 @@ import java.util.Collections; import java.util.List; import java.util.Optional; - import lombok.Data; import org.finos.gitproxy.db.model.PushStep; diff --git a/git-proxy-java-core/src/test/java/org/finos/gitproxy/git/ApprovalPreReceiveHookTest.java b/git-proxy-java-core/src/test/java/org/finos/gitproxy/git/ApprovalPreReceiveHookTest.java index 409cc40e..952d3cfe 100644 --- a/git-proxy-java-core/src/test/java/org/finos/gitproxy/git/ApprovalPreReceiveHookTest.java +++ b/git-proxy-java-core/src/test/java/org/finos/gitproxy/git/ApprovalPreReceiveHookTest.java @@ -24,7 +24,6 @@ import org.finos.gitproxy.db.model.PushRecord; import org.finos.gitproxy.db.model.PushStatus; import org.finos.gitproxy.permission.RepoPermissionService; -import org.finos.gitproxy.git.PushContext; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; @@ -86,7 +85,8 @@ void validationRecordNotInStore_skipsApprovalGate() throws Exception { ReceivePack rp = new ReceivePack(repo); ReceiveCommand cmd = new ReceiveCommand(c1.getId(), c2.getId(), "refs/heads/main", ReceiveCommand.Type.UPDATE); - new ApprovalPreReceiveHook(pushStore, approvalGateway, Duration.ofMinutes(30), null, null, pushContext).onPreReceive(rp, List.of(cmd)); + new ApprovalPreReceiveHook(pushStore, approvalGateway, Duration.ofMinutes(30), null, null, pushContext) + .onPreReceive(rp, List.of(cmd)); assertEquals(ReceiveCommand.Result.NOT_ATTEMPTED, cmd.getResult()); verifyNoInteractions(approvalGateway); @@ -106,7 +106,8 @@ void alreadyApproved_passesImmediately() throws Exception { ReceivePack rp = new ReceivePack(repo); ReceiveCommand cmd = new ReceiveCommand(c1.getId(), c2.getId(), "refs/heads/main", ReceiveCommand.Type.UPDATE); - new ApprovalPreReceiveHook(pushStore, approvalGateway, Duration.ofMinutes(30), null, null, pushContext).onPreReceive(rp, List.of(cmd)); + new ApprovalPreReceiveHook(pushStore, approvalGateway, Duration.ofMinutes(30), null, null, pushContext) + .onPreReceive(rp, List.of(cmd)); assertEquals(ReceiveCommand.Result.NOT_ATTEMPTED, cmd.getResult()); verifyNoInteractions(approvalGateway); @@ -128,7 +129,8 @@ void blockedPush_gatewayApproves_commandNotRejected() throws Exception { ReceivePack rp = new ReceivePack(repo); ReceiveCommand cmd = new ReceiveCommand(c1.getId(), c2.getId(), "refs/heads/main", ReceiveCommand.Type.UPDATE); - new ApprovalPreReceiveHook(pushStore, approvalGateway, Duration.ofSeconds(5), null, null, pushContext).onPreReceive(rp, List.of(cmd)); + new ApprovalPreReceiveHook(pushStore, approvalGateway, Duration.ofSeconds(5), null, null, pushContext) + .onPreReceive(rp, List.of(cmd)); assertEquals(ReceiveCommand.Result.NOT_ATTEMPTED, cmd.getResult()); } @@ -151,7 +153,8 @@ void blockedPush_gatewayRejects_commandRejected() throws Exception { ReceivePack rp = new ReceivePack(repo); ReceiveCommand cmd = new ReceiveCommand(c1.getId(), c2.getId(), "refs/heads/main", ReceiveCommand.Type.UPDATE); - new ApprovalPreReceiveHook(pushStore, approvalGateway, Duration.ofSeconds(5), null, null, pushContext).onPreReceive(rp, List.of(cmd)); + new ApprovalPreReceiveHook(pushStore, approvalGateway, Duration.ofSeconds(5), null, null, pushContext) + .onPreReceive(rp, List.of(cmd)); assertEquals(ReceiveCommand.Result.REJECTED_OTHER_REASON, cmd.getResult()); } @@ -323,7 +326,8 @@ void blockedPush_gatewayTimesOut_commandRejected() throws Exception { ReceivePack rp = new ReceivePack(repo); ReceiveCommand cmd = new ReceiveCommand(c1.getId(), c2.getId(), "refs/heads/main", ReceiveCommand.Type.UPDATE); - new ApprovalPreReceiveHook(pushStore, approvalGateway, Duration.ofSeconds(5), null, null, pushContext).onPreReceive(rp, List.of(cmd)); + new ApprovalPreReceiveHook(pushStore, approvalGateway, Duration.ofSeconds(5), null, null, pushContext) + .onPreReceive(rp, List.of(cmd)); assertEquals(ReceiveCommand.Result.REJECTED_OTHER_REASON, cmd.getResult()); }