Skip to content

Commit 85fb8df

Browse files
coopernetesclaude
andcommitted
refactor(rules): replace UrlRuleFilter with AccessRule; UrlRuleEvaluator uses UrlRuleRegistry as single source
- Delete UrlRuleFilter (HTTP filter used as a data container — wrong abstraction) - UrlRuleEvaluator now takes UrlRuleRegistry directly; no configRules field - UrlRuleAggregateFilter and RepositoryUrlRuleHook take UrlRuleRegistry - StoreAndForwardReceivePackFactory drops List<UrlRuleFilter> parameter - JettyConfigurationBuilder: buildUrlRuleFilters(provider) → buildConfigRules() returning List<AccessRule>; one AccessRule per (provider, pattern) entry - GitProxyServletRegistrar seeds config rules via registry.seedFromConfig() once at startup before the provider loop - Rename RepoRegistry → UrlRuleRegistry throughout - Rule evaluation: firewall/iptables semantics — single ordered list, first match wins; fail-closed (no matching rule → NotAllowed) - Remove OpenMode concept; proxy is always fail-closed - Tests rewritten to use AccessRule + InMemoryUrlRuleRegistry Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
1 parent d0e27a7 commit 85fb8df

31 files changed

Lines changed: 616 additions & 1042 deletions

git-proxy-java-core/src/main/java/org/finos/gitproxy/db/CompositeRepoRegistry.java renamed to git-proxy-java-core/src/main/java/org/finos/gitproxy/db/CompositeUrlRuleRegistry.java

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,20 +7,20 @@
77
import org.finos.gitproxy.db.model.AccessRule;
88

99
/**
10-
* A {@link RepoRegistry} that merges read-only config-seeded rules (in-memory) with operator-managed rules (DB-backed).
11-
* CONFIG rules are never written to the database, so there are no stale duplicates across restarts.
10+
* A {@link UrlRuleRegistry} that merges read-only config-seeded rules (in-memory) with operator-managed rules
11+
* (DB-backed). CONFIG rules are never written to the database, so there are no stale duplicates across restarts.
1212
*
1313
* <ul>
1414
* <li>Reads ({@link #findAll}, {@link #findEnabledForProvider}) return merged results sorted by {@code rule_order}.
1515
* <li>Writes ({@link #save}, {@link #update}, {@link #delete}) delegate only to the DB registry.
1616
* </ul>
1717
*/
18-
public class CompositeRepoRegistry implements RepoRegistry {
18+
public class CompositeUrlRuleRegistry implements UrlRuleRegistry {
1919

20-
private final RepoRegistry configRegistry;
21-
private final RepoRegistry dbRegistry;
20+
private final UrlRuleRegistry configRegistry;
21+
private final UrlRuleRegistry dbRegistry;
2222

23-
public CompositeRepoRegistry(RepoRegistry configRegistry, RepoRegistry dbRegistry) {
23+
public CompositeUrlRuleRegistry(UrlRuleRegistry configRegistry, UrlRuleRegistry dbRegistry) {
2424
this.configRegistry = configRegistry;
2525
this.dbRegistry = dbRegistry;
2626
}
@@ -47,9 +47,7 @@ public void delete(String id) {
4747

4848
@Override
4949
public Optional<AccessRule> findById(String id) {
50-
Optional<AccessRule> fromDb = dbRegistry.findById(id);
51-
if (fromDb.isPresent()) return fromDb;
52-
return configRegistry.findById(id);
50+
return dbRegistry.findById(id).or(() -> configRegistry.findById(id));
5351
}
5452

5553
@Override
@@ -64,8 +62,8 @@ public List<AccessRule> findEnabledForProvider(String provider) {
6462

6563
/**
6664
* Re-seeds the in-memory config registry with a new set of CONFIG rules. DB-sourced rules are not affected. This
67-
* override is necessary because the default {@link RepoRegistry#seedFromConfig} would incorrectly attempt to delete
68-
* config rules via the DB registry.
65+
* override is necessary because the default {@link UrlRuleRegistry#seedFromConfig} would incorrectly attempt to
66+
* delete config rules via the DB registry.
6967
*/
7068
@Override
7169
public void seedFromConfig(List<AccessRule> rules) {

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
import com.mongodb.client.MongoClients;
55
import org.finos.gitproxy.db.mongo.MongoFetchStore;
66
import org.finos.gitproxy.db.mongo.MongoPushStore;
7-
import org.finos.gitproxy.db.mongo.MongoRepoRegistry;
7+
import org.finos.gitproxy.db.mongo.MongoUrlRuleRegistry;
88
import org.finos.gitproxy.permission.MongoRepoPermissionStore;
99
import org.finos.gitproxy.permission.RepoPermissionStore;
1010
import org.finos.gitproxy.user.MongoUserStore;
@@ -39,9 +39,9 @@ public FetchStore fetchStore() {
3939
return store;
4040
}
4141

42-
/** Create and initialize a {@link RepoRegistry} backed by this factory's client. */
43-
public RepoRegistry repoRegistry() {
44-
MongoRepoRegistry store = new MongoRepoRegistry(client, databaseName);
42+
/** Create and initialize a {@link UrlRuleRegistry} backed by this factory's client. */
43+
public UrlRuleRegistry repoRegistry() {
44+
MongoUrlRuleRegistry store = new MongoUrlRuleRegistry(client, databaseName);
4545
store.initialize();
4646
return store;
4747
}

git-proxy-java-core/src/main/java/org/finos/gitproxy/db/RepoRegistry.java renamed to git-proxy-java-core/src/main/java/org/finos/gitproxy/db/UrlRuleRegistry.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
* <p>Rules seeded from YAML configuration have {@code source = CONFIG}; rules created via the REST API have
1212
* {@code source = DB}. Both are stored together and evaluated identically by the filter chain.
1313
*/
14-
public interface RepoRegistry {
14+
public interface UrlRuleRegistry {
1515

1616
/** Persist a new rule. */
1717
void save(AccessRule rule);

git-proxy-java-core/src/main/java/org/finos/gitproxy/db/jdbc/JdbcRepoRegistry.java renamed to git-proxy-java-core/src/main/java/org/finos/gitproxy/db/jdbc/JdbcUrlRuleRegistry.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,22 +4,22 @@
44
import java.util.Map;
55
import java.util.Optional;
66
import javax.sql.DataSource;
7-
import org.finos.gitproxy.db.RepoRegistry;
7+
import org.finos.gitproxy.db.UrlRuleRegistry;
88
import org.finos.gitproxy.db.jdbc.mapper.AccessRuleRowMapper;
99
import org.finos.gitproxy.db.model.AccessRule;
1010
import org.springframework.jdbc.core.namedparam.MapSqlParameterSource;
1111
import org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate;
1212
import org.springframework.jdbc.datasource.DataSourceTransactionManager;
1313
import org.springframework.transaction.support.TransactionTemplate;
1414

15-
/** JDBC-backed {@link RepoRegistry}. Works with H2 and PostgreSQL. */
16-
public class JdbcRepoRegistry implements RepoRegistry {
15+
/** JDBC-backed {@link UrlRuleRegistry}. Works with H2 and PostgreSQL. */
16+
public class JdbcUrlRuleRegistry implements UrlRuleRegistry {
1717

1818
private final DataSource dataSource;
1919
private final NamedParameterJdbcTemplate jdbc;
2020
private final TransactionTemplate tx;
2121

22-
public JdbcRepoRegistry(DataSource dataSource) {
22+
public JdbcUrlRuleRegistry(DataSource dataSource) {
2323
this.dataSource = dataSource;
2424
this.jdbc = new NamedParameterJdbcTemplate(dataSource);
2525
this.tx = new TransactionTemplate(new DataSourceTransactionManager(dataSource));

git-proxy-java-core/src/main/java/org/finos/gitproxy/db/memory/InMemoryRepoRegistry.java

Lines changed: 0 additions & 54 deletions
This file was deleted.

git-proxy-java-core/src/main/java/org/finos/gitproxy/db/mongo/MongoRepoRegistry.java renamed to git-proxy-java-core/src/main/java/org/finos/gitproxy/db/mongo/MongoUrlRuleRegistry.java

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,20 +10,20 @@
1010
import java.util.List;
1111
import java.util.Optional;
1212
import org.bson.Document;
13-
import org.finos.gitproxy.db.RepoRegistry;
13+
import org.finos.gitproxy.db.UrlRuleRegistry;
1414
import org.finos.gitproxy.db.model.AccessRule;
1515
import org.slf4j.Logger;
1616
import org.slf4j.LoggerFactory;
1717

18-
/** MongoDB implementation of {@link RepoRegistry}. */
19-
public class MongoRepoRegistry implements RepoRegistry {
18+
/** MongoDB implementation of {@link UrlRuleRegistry}. */
19+
public class MongoUrlRuleRegistry implements UrlRuleRegistry {
2020

21-
private static final Logger log = LoggerFactory.getLogger(MongoRepoRegistry.class);
21+
private static final Logger log = LoggerFactory.getLogger(MongoUrlRuleRegistry.class);
2222
private static final String COLLECTION_NAME = "access_rules";
2323

2424
private final MongoDatabase database;
2525

26-
public MongoRepoRegistry(MongoClient mongoClient, String databaseName) {
26+
public MongoUrlRuleRegistry(MongoClient mongoClient, String databaseName) {
2727
this.database = mongoClient.getDatabase(databaseName);
2828
}
2929

@@ -53,7 +53,7 @@ public void delete(String id) {
5353
@Override
5454
public Optional<AccessRule> findById(String id) {
5555
Document doc = getCollection().find(Filters.eq("_id", id)).first();
56-
return Optional.ofNullable(doc).map(MongoRepoRegistry::fromDocument);
56+
return Optional.ofNullable(doc).map(MongoUrlRuleRegistry::fromDocument);
5757
}
5858

5959
@Override

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

Lines changed: 4 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -5,16 +5,14 @@
55
import static org.finos.gitproxy.git.GitClientUtils.sym;
66

77
import java.util.Collection;
8-
import java.util.List;
98
import lombok.extern.slf4j.Slf4j;
109
import org.eclipse.jgit.transport.ReceiveCommand;
1110
import org.eclipse.jgit.transport.ReceivePack;
12-
import org.finos.gitproxy.db.RepoRegistry;
11+
import org.finos.gitproxy.db.UrlRuleRegistry;
1312
import org.finos.gitproxy.db.model.PushStep;
1413
import org.finos.gitproxy.db.model.StepStatus;
1514
import org.finos.gitproxy.provider.GitProxyProvider;
1615
import org.finos.gitproxy.servlet.filter.UrlRuleEvaluator;
17-
import org.finos.gitproxy.servlet.filter.UrlRuleFilter;
1816

1917
/**
2018
* Pre-receive hook that enforces URL allow/deny rules in store-and-forward mode. Rule evaluation is delegated entirely
@@ -31,37 +29,20 @@ public class RepositoryUrlRuleHook implements GitProxyHook {
3129
private final ValidationContext validationContext;
3230
private final PushContext pushContext;
3331

34-
/** Open-mode constructor — no rules configured; always passes. Used in tests and simple setups. */
35-
public RepositoryUrlRuleHook(PushContext pushContext) {
36-
this(List.of(), null, null, null, pushContext);
37-
}
38-
3932
public RepositoryUrlRuleHook(
40-
List<UrlRuleFilter> urlRuleFilters,
41-
RepoRegistry repoRegistry,
33+
UrlRuleRegistry urlRuleRegistry,
4234
GitProxyProvider provider,
4335
ValidationContext validationContext,
4436
PushContext pushContext) {
45-
this.evaluator = new UrlRuleEvaluator(urlRuleFilters, repoRegistry, provider);
37+
this.evaluator = new UrlRuleEvaluator(urlRuleRegistry, provider);
4638
this.validationContext = validationContext;
4739
this.pushContext = pushContext;
4840
}
4941

5042
@Override
5143
public void onPreReceive(ReceivePack rp, Collection<ReceiveCommand> commands) {
52-
// Open mode: evaluator has no config rules and no registry
53-
// (detected inside evaluator — returns OpenMode)
54-
5544
String repoSlug = rp.getRepository().getConfig().getString("gitproxy", null, "repoSlug");
5645
if (repoSlug == null || repoSlug.isBlank()) {
57-
// No repoSlug means we can't evaluate rules — fail closed
58-
// Exception: if the evaluator is in pure open mode (no rules at all), allow it
59-
UrlRuleEvaluator.Result probe = evaluator.evaluate(null, null, null, HttpOperation.PUSH);
60-
if (probe instanceof UrlRuleEvaluator.Result.OpenMode) {
61-
log.debug("No repoSlug and no rules configured — allowing push (open mode)");
62-
recordPass();
63-
return;
64-
}
6546
log.warn("No repoSlug in repo config — cannot evaluate URL rules, blocking push (fail-closed)");
6647
blockPush(rp, commands, "Repository path unavailable");
6748
return;
@@ -84,12 +65,8 @@ public void onPreReceive(ReceivePack rp, Collection<ReceiveCommand> commands) {
8465
log.debug("Push allowed by rule: {}", a.ruleId());
8566
recordPass();
8667
}
87-
case UrlRuleEvaluator.Result.OpenMode m -> {
88-
log.debug("Push allowed — open mode (no allow rules configured)");
89-
recordPass();
90-
}
9168
case UrlRuleEvaluator.Result.NotAllowed n -> {
92-
log.debug("Push blocked — no allow rule matched");
69+
log.debug("Push blocked — no rule matched");
9370
blockPush(rp, commands, "Repository is not in the allow list");
9471
}
9572
}

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

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,11 @@
2121
import org.finos.gitproxy.config.GpgConfig;
2222
import org.finos.gitproxy.config.SecretScanConfig;
2323
import org.finos.gitproxy.db.PushStore;
24-
import org.finos.gitproxy.db.RepoRegistry;
24+
import org.finos.gitproxy.db.UrlRuleRegistry;
2525
import org.finos.gitproxy.permission.RepoPermissionService;
2626
import org.finos.gitproxy.provider.BitbucketProvider;
2727
import org.finos.gitproxy.provider.GitProxyProvider;
2828
import org.finos.gitproxy.service.PushIdentityResolver;
29-
import org.finos.gitproxy.servlet.filter.UrlRuleFilter;
3029

3130
/**
3231
* Factory that creates {@link ReceivePack} instances for store-and-forward push handling. Extracts credentials from the
@@ -50,8 +49,7 @@ public class StoreAndForwardReceivePackFactory implements ReceivePackFactory<Htt
5049
private final ApprovalGateway approvalGateway;
5150
private final String serviceUrl;
5251
private final Duration heartbeatInterval;
53-
private final List<UrlRuleFilter> urlRuleFilters;
54-
private final RepoRegistry repoRegistry;
52+
private final UrlRuleRegistry urlRuleRegistry;
5553

5654
/** Stop the validation hook chain after the first failure (see {@link ServerConfig#isFailFast()}). */
5755
private boolean failFast = false;
@@ -87,7 +85,6 @@ public StoreAndForwardReceivePackFactory(
8785
approvalGateway,
8886
null,
8987
DEFAULT_HEARTBEAT_INTERVAL,
90-
List.of(),
9188
null);
9289
}
9390

@@ -113,11 +110,9 @@ public StoreAndForwardReceivePackFactory(
113110
approvalGateway,
114111
serviceUrl,
115112
heartbeatInterval,
116-
List.of(),
117113
null);
118114
}
119115

120-
/** Live-reload constructor with URL rule enforcement. */
121116
public StoreAndForwardReceivePackFactory(
122117
GitProxyProvider provider,
123118
Supplier<CommitConfig> commitConfigSupplier,
@@ -130,8 +125,7 @@ public StoreAndForwardReceivePackFactory(
130125
ApprovalGateway approvalGateway,
131126
String serviceUrl,
132127
Duration heartbeatInterval,
133-
List<UrlRuleFilter> urlRuleFilters,
134-
RepoRegistry repoRegistry) {
128+
UrlRuleRegistry urlRuleRegistry) {
135129
this.provider = provider;
136130
this.commitConfigSupplier = commitConfigSupplier;
137131
this.diffScanConfigSupplier =
@@ -145,8 +139,7 @@ public StoreAndForwardReceivePackFactory(
145139
this.approvalGateway = approvalGateway;
146140
this.serviceUrl = serviceUrl;
147141
this.heartbeatInterval = heartbeatInterval != null ? heartbeatInterval : DEFAULT_HEARTBEAT_INTERVAL;
148-
this.urlRuleFilters = urlRuleFilters != null ? urlRuleFilters : List.of();
149-
this.repoRegistry = repoRegistry;
142+
this.urlRuleRegistry = urlRuleRegistry;
150143
}
151144

152145
@Override
@@ -241,7 +234,7 @@ public ReceivePack create(HttpServletRequest req, Repository db)
241234

242235
// Build and sort the orderable validation hook list
243236
List<GitProxyHook> validationHooks = new ArrayList<>(List.of(
244-
new RepositoryUrlRuleHook(urlRuleFilters, repoRegistry, provider, validationContext, pushContext),
237+
new RepositoryUrlRuleHook(urlRuleRegistry, provider, validationContext, pushContext),
245238
permissionHook,
246239
identityVerificationHook,
247240
new CheckEmptyBranchHook(pushContext),

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,14 @@
11
package org.finos.gitproxy.provider;
22

33
import java.util.List;
4+
import org.finos.gitproxy.db.UrlRuleRegistry;
45

56
/**
67
* Registry of configured git proxy providers. Keyed by the friendly name used as the config map key (e.g.
78
* {@code github}, {@code internal-gitlab}), independent of the internal {@code type/host} provider ID used for request
89
* routing.
910
*
10-
* <p>Consistent with {@link org.finos.gitproxy.db.RepoRegistry} — a lookup/discovery mechanism, not a CRUD store.
11+
* <p>Consistent with {@link UrlRuleRegistry} — a lookup/discovery mechanism, not a CRUD store.
1112
*/
1213
public interface ProviderRegistry {
1314

0 commit comments

Comments
 (0)