diff --git a/CLAUDE.md b/CLAUDE.md index 41cde36..a388d77 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -99,8 +99,10 @@ overrides, and provider-specific settings. ## Commit conventions -- Always squash related commits into one before pushing — use `git reset --soft` to squash, not `git rebase -i` (requires TTY). -- Always include a `Co-Authored-By: Claude Sonnet 4.6 ` trailer in every commit. This is a project transparency requirement. +- Always squash related commits into one before pushing — use `git reset --soft` to squash, not `git rebase -i` + (requires TTY). +- Always include a `Co-Authored-By: Claude Sonnet 4.6 ` trailer in every commit. This is a + project transparency requirement. - Always include `closes #N` / `resolves #N` in commit messages when addressing a GitHub issue. - Never add `[ci skip]` to commits unless explicitly asked. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 3150a0b..d4af167 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -431,13 +431,20 @@ architecture and [docs/internals/GIT_INTERNALS.md](docs/internals/GIT_INTERNALS. ## Releases -Releases follow a two-phase process to ensure every published image is identical to what was already scanned and running as `:edge`. +Releases follow a two-phase process to ensure every published image is identical to what was already scanned and running +as `:edge`. -**Phase 1 — version bump.** Create a `release/` branch, update `version` in `build.gradle`, open a PR, and enable auto-merge. The PR must pass all CI, CodeQL, CVE, and container scan checks before it can merge. Use the `/release` Claude command to automate this. +**Phase 1 — version bump.** Create a `release/` branch, update `version` in `build.gradle`, open a PR, and +enable auto-merge. The PR must pass all CI, CodeQL, CVE, and container scan checks before it can merge. Use the +`/release` Claude command to automate this. -**Phase 2 — tag.** Once the version bump lands on `main`, push an annotated tag (`v`). The tag ruleset enforces the same checks must have passed on that commit. The publish workflow then promotes the already-built `:edge` image directly to the release tags (`:v1.0.0`, `:latest`, etc.) — no rebuild occurs. Use the `/release-tag` Claude command for this step. +**Phase 2 — tag.** Once the version bump lands on `main`, push an annotated tag (`v`). The tag ruleset enforces +the same checks must have passed on that commit. The publish workflow then promotes the already-built `:edge` image +directly to the release tags (`:v1.0.0`, `:latest`, etc.) — no rebuild occurs. Use the `/release-tag` Claude command for +this step. -This means every release image is byte-for-byte identical to the `:edge` image that was scanned when the version bump merged. +This means every release image is byte-for-byte identical to the `:edge` image that was scanned when the version bump +merged. ## Issues and pull requests diff --git a/git-proxy-java-core/src/main/java/org/finos/gitproxy/servlet/filter/UrlRuleAggregateFilter.java b/git-proxy-java-core/src/main/java/org/finos/gitproxy/servlet/filter/UrlRuleAggregateFilter.java index 1316413..ba81f8c 100644 --- a/git-proxy-java-core/src/main/java/org/finos/gitproxy/servlet/filter/UrlRuleAggregateFilter.java +++ b/git-proxy-java-core/src/main/java/org/finos/gitproxy/servlet/filter/UrlRuleAggregateFilter.java @@ -160,11 +160,13 @@ private void applyInfoRefsRules(HttpServletRequest request, HttpServletResponse switch (result) { case UrlRuleEvaluator.Result.Denied d -> { log.debug("Blocking /info/refs — matched deny rule: {}", d.ruleId()); + if (effectiveOp == HttpOperation.FETCH && fetchStore != null) recordFetch(request, false); setResult(request, GitRequestDetails.GitResult.REJECTED, "Repository blocked by deny rule"); response.sendError(provider.getBlockedInfoRefsStatus()); } case UrlRuleEvaluator.Result.NotAllowed n -> { log.debug("Blocking /info/refs — no rule matched"); + if (effectiveOp == HttpOperation.FETCH && fetchStore != null) recordFetch(request, false); setResult(request, GitRequestDetails.GitResult.REJECTED, "Repository not in allow rules"); response.sendError(provider.getBlockedInfoRefsStatus()); } diff --git a/git-proxy-java-core/src/test/java/org/finos/gitproxy/servlet/filter/UrlRuleFilterTest.java b/git-proxy-java-core/src/test/java/org/finos/gitproxy/servlet/filter/UrlRuleFilterTest.java index ebd8bbd..4e16f57 100644 --- a/git-proxy-java-core/src/test/java/org/finos/gitproxy/servlet/filter/UrlRuleFilterTest.java +++ b/git-proxy-java-core/src/test/java/org/finos/gitproxy/servlet/filter/UrlRuleFilterTest.java @@ -16,14 +16,17 @@ import java.util.HashMap; import java.util.Map; import java.util.concurrent.atomic.AtomicBoolean; +import org.finos.gitproxy.db.FetchStore; import org.finos.gitproxy.db.memory.InMemoryUrlRuleRegistry; import org.finos.gitproxy.db.model.AccessRule; +import org.finos.gitproxy.db.model.FetchRecord; import org.finos.gitproxy.git.GitRequestDetails; import org.finos.gitproxy.git.HttpOperation; import org.finos.gitproxy.provider.GenericProxyProvider; import org.finos.gitproxy.provider.GitHubProvider; import org.finos.gitproxy.provider.GitProxyProvider; import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; class UrlRuleFilterTest { @@ -345,6 +348,80 @@ void infoRefs_receivePack_deniedByRule_returns403() throws Exception { verify(resp.mock).sendError(403); } + // --- Gap 2: recordFetch on blocked /info/refs --- + + @Test + void infoRefs_fetchBlocked_notAllowed_recordsFetch() throws Exception { + FetchStore fetchStore = mock(FetchStore.class); + var registry = new InMemoryUrlRuleRegistry(); + var aggregate = new UrlRuleAggregateFilter(50, GITHUB, "/proxy", fetchStore, registry); + GitRequestDetails details = makeInfoDetails("owner", "repo", "/owner/repo"); + FakeResponse resp = new FakeResponse(); + + aggregate.doHttpFilter(mockInfoRefsRequest(details, "git-upload-pack"), resp.mock); + + verify(resp.mock).sendError(403); + ArgumentCaptor captor = ArgumentCaptor.forClass(FetchRecord.class); + verify(fetchStore).record(captor.capture()); + assertEquals(FetchRecord.Result.BLOCKED, captor.getValue().getResult()); + } + + @Test + void infoRefs_fetchBlocked_denyRule_recordsFetch() throws Exception { + FetchStore fetchStore = mock(FetchStore.class); + var registry = new InMemoryUrlRuleRegistry(); + registry.save(AccessRule.builder() + .ruleOrder(100) + .access(AccessRule.Access.DENY) + .operations(AccessRule.Operations.BOTH) + .slug("/owner/repo") + .build()); + var aggregate = new UrlRuleAggregateFilter(50, GITHUB, "/proxy", fetchStore, registry); + GitRequestDetails details = makeInfoDetails("owner", "repo", "/owner/repo"); + FakeResponse resp = new FakeResponse(); + + aggregate.doHttpFilter(mockInfoRefsRequest(details, "git-upload-pack"), resp.mock); + + verify(resp.mock).sendError(403); + ArgumentCaptor captor = ArgumentCaptor.forClass(FetchRecord.class); + verify(fetchStore).record(captor.capture()); + assertEquals(FetchRecord.Result.BLOCKED, captor.getValue().getResult()); + } + + @Test + void infoRefs_pushBlocked_doesNotRecordFetch() throws Exception { + FetchStore fetchStore = mock(FetchStore.class); + var registry = new InMemoryUrlRuleRegistry(); + var aggregate = new UrlRuleAggregateFilter(50, GITHUB, "/proxy", fetchStore, registry); + GitRequestDetails details = makeInfoDetails("owner", "repo", "/owner/repo"); + FakeResponse resp = new FakeResponse(); + + aggregate.doHttpFilter(mockInfoRefsRequest(details, "git-receive-pack"), resp.mock); + + verify(resp.mock).sendError(403); + verify(fetchStore, never()).record(any()); + } + + @Test + void infoRefs_fetchAllowed_doesNotRecordFetch() throws Exception { + FetchStore fetchStore = mock(FetchStore.class); + var registry = new InMemoryUrlRuleRegistry(); + registry.save(AccessRule.builder() + .ruleOrder(100) + .access(AccessRule.Access.ALLOW) + .operations(AccessRule.Operations.BOTH) + .slug("/owner/repo") + .build()); + var aggregate = new UrlRuleAggregateFilter(50, GITHUB, "/proxy", fetchStore, registry); + GitRequestDetails details = makeInfoDetails("owner", "repo", "/owner/repo"); + FakeResponse resp = new FakeResponse(); + + aggregate.doHttpFilter(mockInfoRefsRequest(details, "git-upload-pack"), resp.mock); + + verify(resp.mock, never()).sendError(anyInt()); + verify(fetchStore, never()).record(any()); + } + @Test void infoRefs_customBlockedStatus_returns404() throws Exception { var provider = GenericProxyProvider.builder() diff --git a/git-proxy-java-server/src/main/java/org/finos/gitproxy/jetty/GitProxyServletRegistrar.java b/git-proxy-java-server/src/main/java/org/finos/gitproxy/jetty/GitProxyServletRegistrar.java index 2f85e91..6968bcf 100644 --- a/git-proxy-java-server/src/main/java/org/finos/gitproxy/jetty/GitProxyServletRegistrar.java +++ b/git-proxy-java-server/src/main/java/org/finos/gitproxy/jetty/GitProxyServletRegistrar.java @@ -91,7 +91,8 @@ public static void registerProviders( gitProxyCtx.heartbeatIntervalSeconds(), gitProxyCtx.failFast(), gitProxyCtx.upstreamConnectTimeoutSeconds(), - gitProxyCtx.urlRuleRegistry()); + gitProxyCtx.urlRuleRegistry(), + gitProxyCtx.fetchStore()); registerProxyServlet( context, provider, @@ -131,7 +132,8 @@ public static void registerGitServlet( int heartbeatIntervalSeconds, boolean failFast, int connectTimeoutSeconds, - UrlRuleRegistry urlRuleRegistry) { + UrlRuleRegistry urlRuleRegistry, + FetchStore fetchStore) { var resolver = new StoreAndForwardRepositoryResolver(cache, provider); var factory = new StoreAndForwardReceivePackFactory( @@ -166,6 +168,15 @@ public static void registerGitServlet( new FilterHolder(new SmartHttpErrorFilter()), pushMapping, EnumSet.of(DispatcherType.REQUEST)); context.addFilter( new FilterHolder(new BasicAuthChallengeFilter()), pushMapping, EnumSet.of(DispatcherType.REQUEST)); + context.addFilter( + new FilterHolder(new ParseGitRequestFilter(provider, PUSH_PATH_PREFIX)), + pushMapping, + EnumSet.of(DispatcherType.REQUEST)); + context.addFilter( + new FilterHolder( + new UrlRuleAggregateFilter(100, provider, PUSH_PATH_PREFIX, fetchStore, urlRuleRegistry)), + pushMapping, + EnumSet.of(DispatcherType.REQUEST)); log.info("Registered GitServlet for {} at {}", provider.getName(), pushMapping); }