Skip to content

Commit 9ba3020

Browse files
coopernetesclaude
coopernetes
and
claude
authored
refactor: use provider name as canonical DB foreign key (#190)
* refactor: use provider name as canonical DB foreign key Replace the type/host compound key (e.g. github/github.com) with the user-configured provider name (e.g. github, internal-github) as the canonical identifier stored in all provider FK columns. Provider names are unique by YAML map key constraint, stable across hostname changes, and human-readable — removing the need for the type/host compound key entirely. Changes: - GitProxyProvider.getProviderId() now returns getName() - ProviderRegistry.resolveProvider() simplified; type/host fallback removed - JettyConfigurationBuilder: removed seenProviderIds conflict map (redundant), updated error messages and Javadocs - ProfileController + UserController: removed provider.replace('@', '/') decode - api.ts: removed providerToPathKey function and its two call sites - scripts/migrate-provider-ids.sql: one-off UPDATE script for existing installs - Tests: updated all provider ID assertions, removed backwards-compat test, added two tests for same-type multi-provider (github + internal-github) closes #188 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: pre-install grype ourselves, drop grype-version from scan-action scan-action downloads grype's install.sh from main at runtime and passes the version directly — the script constructs releases/{version} URLs which 404 for non-latest versions. Pre-install our checksum-verified binary first so scan-action uses the PATH binary instead. * chore: bump grype to 0.112.0 * fix: pre-install grype in docker-publish scan job, bump to 0.112.0 Same broken pattern as container-scan.yml — scan-action was downloading grype's install.sh from main which constructs invalid release tag URLs. Pre-install our checksum-verified binary first, drop grype-version. --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 172ca2e commit 9ba3020

19 files changed

Lines changed: 187 additions & 160 deletions

File tree

‎.github/workflows/container-scan.yml‎

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -12,17 +12,26 @@ jobs:
1212
name: Container Scan
1313
runs-on: ubuntu-latest
1414
env:
15-
GRYPE_VERSION: "0.111.1"
15+
GRYPE_VERSION: "0.112.0"
1616
steps:
1717
- name: Checkout
1818
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # ratchet:actions/checkout@v6
1919

20+
- name: Install grype
21+
run: |
22+
cd /tmp
23+
curl -sSfL -o "grype_${GRYPE_VERSION}_linux_amd64.tar.gz" \
24+
"https://github.com/anchore/grype/releases/download/v${GRYPE_VERSION}/grype_${GRYPE_VERSION}_linux_amd64.tar.gz"
25+
curl -sSfL -o "grype_${GRYPE_VERSION}_checksums.txt" \
26+
"https://github.com/anchore/grype/releases/download/v${GRYPE_VERSION}/grype_${GRYPE_VERSION}_checksums.txt"
27+
grep "grype_${GRYPE_VERSION}_linux_amd64.tar.gz" "grype_${GRYPE_VERSION}_checksums.txt" | sha256sum --check
28+
tar -xzf "grype_${GRYPE_VERSION}_linux_amd64.tar.gz" -C /usr/local/bin grype
29+
2030
- name: Scan image
2131
uses: anchore/scan-action@e1165082ffb1fe366ebaf02d8526e7c4989ea9d2 # ratchet:anchore/scan-action@v7
2232
id: scan
2333
with:
2434
image: ghcr.io/coopernetes/git-proxy-java:latest
25-
grype-version: ${{ env.GRYPE_VERSION }}
2635
fail-build: true
2736
severity-cutoff: high
2837
only-fixed: true
@@ -33,17 +42,6 @@ jobs:
3342
# in the GitHub Security tab (high CVSS score ≠ high actual risk for this workload).
3443
# The build still fails on high/critical with a fix available via fail-build: true above.
3544

36-
- name: Install grype
37-
if: always()
38-
run: |
39-
cd /tmp
40-
curl -sSfL -o "grype_${GRYPE_VERSION}_linux_amd64.tar.gz" \
41-
"https://github.com/anchore/grype/releases/download/v${GRYPE_VERSION}/grype_${GRYPE_VERSION}_linux_amd64.tar.gz"
42-
curl -sSfL -o "grype_${GRYPE_VERSION}_checksums.txt" \
43-
"https://github.com/anchore/grype/releases/download/v${GRYPE_VERSION}/grype_${GRYPE_VERSION}_checksums.txt"
44-
grep "grype_${GRYPE_VERSION}_linux_amd64.tar.gz" "grype_${GRYPE_VERSION}_checksums.txt" | sha256sum --check
45-
tar -xzf "grype_${GRYPE_VERSION}_linux_amd64.tar.gz" -C /usr/local/bin grype
46-
4745
- name: Generate human-readable report
4846
if: always()
4947
run: |

‎.github/workflows/docker-publish.yml‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -82,17 +82,26 @@ jobs:
8282
contents: read
8383
if: github.event_name == 'push' || github.event_name == 'workflow_dispatch'
8484
env:
85-
GRYPE_VERSION: "0.111.1"
85+
GRYPE_VERSION: "0.112.0"
8686
steps:
8787
- name: Checkout
8888
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # ratchet:actions/checkout@v6
8989

90+
- name: Install grype
91+
run: |
92+
cd /tmp
93+
curl -sSfL -o "grype_${GRYPE_VERSION}_linux_amd64.tar.gz" \
94+
"https://github.com/anchore/grype/releases/download/v${GRYPE_VERSION}/grype_${GRYPE_VERSION}_linux_amd64.tar.gz"
95+
curl -sSfL -o "grype_${GRYPE_VERSION}_checksums.txt" \
96+
"https://github.com/anchore/grype/releases/download/v${GRYPE_VERSION}/grype_${GRYPE_VERSION}_checksums.txt"
97+
grep "grype_${GRYPE_VERSION}_linux_amd64.tar.gz" "grype_${GRYPE_VERSION}_checksums.txt" | sha256sum --check
98+
tar -xzf "grype_${GRYPE_VERSION}_linux_amd64.tar.gz" -C /usr/local/bin grype
99+
90100
- name: Scan image
91101
uses: anchore/scan-action@e1165082ffb1fe366ebaf02d8526e7c4989ea9d2 # ratchet:anchore/scan-action@v7
92102
id: scan
93103
with:
94104
image: ghcr.io/${{ github.repository }}@${{ needs.build-and-push.outputs.digest }}
95-
grype-version: ${{ env.GRYPE_VERSION }}
96105
fail-build: true
97106
severity-cutoff: high
98107
only-fixed: true
@@ -103,10 +112,6 @@ jobs:
103112
# in the GitHub Security tab (high CVSS score ≠ high actual risk for this workload).
104113
# The build still fails on high/critical with a fix available via fail-build: true above.
105114

106-
- name: Add grype to PATH
107-
if: always()
108-
run: echo "$(dirname $(find /opt/hostedtoolcache/grype -name grype -type f | head -1))" >> $GITHUB_PATH
109-
110115
- name: Generate scan report
111116
if: always()
112117
run: |

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

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,12 @@ public interface GitProxyProvider {
1414
String getType();
1515

1616
/**
17-
* Unique provider identity combining type and host (e.g. "github/github.com", "github/github.corp.example.com").
18-
* Used for SCM identity resolution, permission matching, token caching, and DB storage. Two providers of the same
19-
* type but different hosts (e.g. public GitHub and internal GHES) have distinct provider IDs — tokens, identities,
20-
* and permissions are never shared across hosts.
17+
* Canonical provider identity — equals the user-configured name (the YAML config map key, e.g. {@code "github"},
18+
* {@code "internal"}). Used for SCM identity resolution, permission matching, token caching, and DB storage.
19+
* Provider names are unique by YAML map key constraint and stable across hostname changes.
2120
*/
2221
default String getProviderId() {
23-
return getType() + "/" + getUri().getHost();
22+
return getName();
2423
}
2524

2625
URI getUri();

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

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,8 @@
44
import org.finos.gitproxy.db.UrlRuleRegistry;
55

66
/**
7-
* Registry of configured git proxy providers. Keyed by the friendly name used as the config map key (e.g.
8-
* {@code github}, {@code internal-gitlab}), independent of the internal {@code type/host} provider ID used for request
9-
* routing.
7+
* Registry of configured git proxy providers. Keyed by the user-configured name (the YAML config map key, e.g.
8+
* {@code github}, {@code internal-gitlab}), which is also the canonical provider ID stored in the database.
109
*
1110
* <p>Consistent with {@link UrlRuleRegistry} — a lookup/discovery mechanism, not a CRUD store.
1211
*/
@@ -23,21 +22,13 @@ public interface ProviderRegistry {
2322
List<GitProxyProvider> getProviders();
2423

2524
/**
26-
* Resolves a provider by either its friendly name (e.g. {@code "github"}) or its canonical {@code type/host} ID
27-
* (e.g. {@code "github/github.com"}). Friendly name is tried first.
25+
* Resolves a provider by its name (the YAML config map key, e.g. {@code "github"}). Null/blank input returns null
26+
* (meaning "applies to all providers").
2827
*
29-
* <p>This is the entry point for validating config references in {@code permissions:}, {@code rules:}, and
30-
* {@code scm-identities:} — both forms are accepted so operators can use whichever is more readable.
31-
*
32-
* @return the provider, or {@code null} if neither a friendly name nor an ID match
28+
* @return the provider, or {@code null} if not found
3329
*/
3430
default GitProxyProvider resolveProvider(String nameOrId) {
3531
if (nameOrId == null || nameOrId.isBlank()) return null;
36-
GitProxyProvider byName = getProvider(nameOrId);
37-
if (byName != null) return byName;
38-
return getProviders().stream()
39-
.filter(p -> p.getProviderId().equals(nameOrId))
40-
.findFirst()
41-
.orElse(null);
32+
return getProvider(nameOrId);
4233
}
4334
}

‎git-proxy-java-core/src/test/java/org/finos/gitproxy/db/PushRecordMapperTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ void fromRequestDetails_withProvider_setsUpstreamUrl() {
127127
when(provider.getName()).thenReturn("github");
128128
when(provider.getType()).thenReturn("github");
129129
when(provider.getUri()).thenReturn(URI.create("https://github.com/"));
130-
when(provider.getProviderId()).thenReturn("github/github.com");
130+
when(provider.getProviderId()).thenReturn("github");
131131

132132
GitRequestDetails details = new GitRequestDetails();
133133
details.setRepoRef(

‎git-proxy-java-core/src/test/java/org/finos/gitproxy/git/ApprovalPreReceiveHookTest.java‎

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -175,14 +175,13 @@ void selfApproved_alreadyApprovedAtHookStart_noPerm_rejected() throws Exception
175175
.id(recordId)
176176
.status(PushStatus.APPROVED)
177177
.resolvedUser("alice")
178-
.provider("github/github.com")
178+
.provider("github")
179179
.url("/owner/repo")
180180
.attestation(att)
181181
.build();
182182
when(pushStore.findById(recordId)).thenReturn(Optional.of(record));
183183
RepoPermissionService perms = mock(RepoPermissionService.class);
184-
when(perms.isBypassReviewAllowed("alice", "github/github.com", "/owner/repo"))
185-
.thenReturn(false);
184+
when(perms.isBypassReviewAllowed("alice", "github", "/owner/repo")).thenReturn(false);
186185

187186
RevCommit c1 = createCommit("init");
188187
RevCommit c2 = createCommit("second");
@@ -193,7 +192,7 @@ void selfApproved_alreadyApprovedAtHookStart_noPerm_rejected() throws Exception
193192
.onPreReceive(rp, List.of(cmd));
194193

195194
assertEquals(ReceiveCommand.Result.REJECTED_OTHER_REASON, cmd.getResult());
196-
verify(perms).isBypassReviewAllowed("alice", "github/github.com", "/owner/repo");
195+
verify(perms).isBypassReviewAllowed("alice", "github", "/owner/repo");
197196
}
198197

199198
@Test
@@ -210,14 +209,13 @@ void selfApproved_alreadyApprovedAtHookStart_withPerm_passes() throws Exception
210209
.id(recordId)
211210
.status(PushStatus.APPROVED)
212211
.resolvedUser("alice")
213-
.provider("github/github.com")
212+
.provider("github")
214213
.url("/owner/repo")
215214
.attestation(att)
216215
.build();
217216
when(pushStore.findById(recordId)).thenReturn(Optional.of(record));
218217
RepoPermissionService perms = mock(RepoPermissionService.class);
219-
when(perms.isBypassReviewAllowed("alice", "github/github.com", "/owner/repo"))
220-
.thenReturn(true);
218+
when(perms.isBypassReviewAllowed("alice", "github", "/owner/repo")).thenReturn(true);
221219

222220
RevCommit c1 = createCommit("init");
223221
RevCommit c2 = createCommit("second");
@@ -241,7 +239,7 @@ void selfApproved_viaWaitForApproval_noPerm_rejected() throws Exception {
241239
.id(recordId)
242240
.status(PushStatus.PENDING)
243241
.resolvedUser("alice")
244-
.provider("github/github.com")
242+
.provider("github")
245243
.url("/owner/repo")
246244
.build();
247245
Attestation att = Attestation.builder()
@@ -253,16 +251,15 @@ void selfApproved_viaWaitForApproval_noPerm_rejected() throws Exception {
253251
.id(recordId)
254252
.status(PushStatus.APPROVED)
255253
.resolvedUser("alice")
256-
.provider("github/github.com")
254+
.provider("github")
257255
.url("/owner/repo")
258256
.attestation(att)
259257
.build();
260258
when(pushStore.findById(recordId)).thenReturn(Optional.of(pending)).thenReturn(Optional.of(approved));
261259
when(approvalGateway.waitForApproval(eq(recordId), any(), any(Duration.class)))
262260
.thenReturn(ApprovalResult.APPROVED);
263261
RepoPermissionService perms = mock(RepoPermissionService.class);
264-
when(perms.isBypassReviewAllowed("alice", "github/github.com", "/owner/repo"))
265-
.thenReturn(false);
262+
when(perms.isBypassReviewAllowed("alice", "github", "/owner/repo")).thenReturn(false);
266263

267264
RevCommit c1 = createCommit("init");
268265
RevCommit c2 = createCommit("second");
@@ -273,7 +270,7 @@ void selfApproved_viaWaitForApproval_noPerm_rejected() throws Exception {
273270
.onPreReceive(rp, List.of(cmd));
274271

275272
assertEquals(ReceiveCommand.Result.REJECTED_OTHER_REASON, cmd.getResult());
276-
verify(perms).isBypassReviewAllowed("alice", "github/github.com", "/owner/repo");
273+
verify(perms).isBypassReviewAllowed("alice", "github", "/owner/repo");
277274
}
278275

279276
@Test
@@ -291,7 +288,7 @@ void differentApproverThanPusher_noReVerifyNeeded() throws Exception {
291288
.id(recordId)
292289
.status(PushStatus.APPROVED)
293290
.resolvedUser("alice")
294-
.provider("github/github.com")
291+
.provider("github")
295292
.url("/owner/repo")
296293
.attestation(att)
297294
.build();

‎git-proxy-java-core/src/test/java/org/finos/gitproxy/git/CheckUserPushPermissionHookTest.java‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -123,8 +123,7 @@ void resolverReturnsEmpty_addsNotRegisteredIssue() throws Exception {
123123
void userNotAuthorized_addsUnauthorizedIssue() throws Exception {
124124
GitProxyProvider github = new GitHubProvider("/push");
125125
when(resolver.resolve(eq(github), eq("corp-user"), any())).thenReturn(Optional.of(userEntry("alice")));
126-
when(permService.isAllowedToPush("alice", "github/github.com", "/owner/repo"))
127-
.thenReturn(false);
126+
when(permService.isAllowedToPush("alice", "github", "/owner/repo")).thenReturn(false);
128127

129128
RevCommit c1 = createCommit("init");
130129
RevCommit c2 = createCommit("second");
@@ -154,8 +153,7 @@ void resolvedAndAuthorized_recordsPass() throws Exception {
154153
GitProxyProvider github = new GitHubProvider("/push");
155154
when(resolver.resolve(eq(github), eq("corp-user"), eq("ghp_secret")))
156155
.thenReturn(Optional.of(userEntry("alice")));
157-
when(permService.isAllowedToPush("alice", "github/github.com", "/owner/repo"))
158-
.thenReturn(true);
156+
when(permService.isAllowedToPush("alice", "github", "/owner/repo")).thenReturn(true);
159157

160158
RevCommit c1 = createCommit("init");
161159
RevCommit c2 = createCommit("second");
@@ -201,8 +199,7 @@ void nullResolver_withPushUser_passesInOpenMode() throws Exception {
201199
void provider_isPassedToResolver() throws Exception {
202200
GitProxyProvider github = new GitHubProvider("/push");
203201
when(resolver.resolve(eq(github), eq("my-user"), any())).thenReturn(Optional.of(userEntry("my-user")));
204-
when(permService.isAllowedToPush("my-user", "github/github.com", "/owner/repo"))
205-
.thenReturn(true);
202+
when(permService.isAllowedToPush("my-user", "github", "/owner/repo")).thenReturn(true);
206203

207204
RevCommit c1 = createCommit("init");
208205
RevCommit c2 = createCommit("second");

‎git-proxy-java-core/src/test/java/org/finos/gitproxy/provider/ProviderTest.java‎

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -164,18 +164,10 @@ void registry_resolveProvider_byFriendlyName() {
164164
assertSame(github, registry.resolveProvider("github"));
165165
}
166166

167-
@Test
168-
void registry_resolveProvider_byTypeHostId() {
169-
var github = new GitHubProvider("/proxy");
170-
var registry = new InMemoryProviderRegistry(Map.of("github", github));
171-
// "github/github.com" is the canonical type/host ID
172-
assertSame(github, registry.resolveProvider("github/github.com"));
173-
}
174-
175167
@Test
176168
void registry_resolveProvider_unknown_returnsNull() {
177169
var registry = new InMemoryProviderRegistry(Map.of());
178-
assertNull(registry.resolveProvider("nonexistent/host.example.com"));
170+
assertNull(registry.resolveProvider("nonexistent"));
179171
}
180172

181173
@Test

‎git-proxy-java-core/src/test/java/org/finos/gitproxy/service/CachingTokenPushIdentityResolverTest.java‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ void setUp() {
3131
when(provider.getName()).thenReturn("github");
3232
when(provider.getType()).thenReturn("github");
3333
when(provider.getUri()).thenReturn(URI.create("https://github.com"));
34-
when(provider.getProviderId()).thenReturn("github/github.com");
34+
when(provider.getProviderId()).thenReturn("github");
3535
resolver = new CachingTokenPushIdentityResolver(delegate, cache, userStore);
3636
}
3737

@@ -51,7 +51,7 @@ private static UserEntry entry(String username) {
5151
@Test
5252
void cacheHit_returnsCachedUser_delegateNotCalled() {
5353
UserEntry alice = entry("alice");
54-
when(cache.lookup(eq("github/github.com"), anyString())).thenReturn(Optional.of("alice"));
54+
when(cache.lookup(eq("github"), anyString())).thenReturn(Optional.of("alice"));
5555
when(userStore.findByUsername("alice")).thenReturn(Optional.of(alice));
5656

5757
var result = resolver.resolve(provider, "me", "good-token");
@@ -67,22 +67,22 @@ void cacheHit_returnsCachedUser_delegateNotCalled() {
6767
@Test
6868
void cacheMiss_delegateResolves_storesAndReturns() {
6969
UserEntry alice = entry("alice");
70-
when(cache.lookup(eq("github/github.com"), anyString())).thenReturn(Optional.empty());
70+
when(cache.lookup(eq("github"), anyString())).thenReturn(Optional.empty());
7171
when(delegate.resolve(provider, "me", "good-token")).thenReturn(Optional.of(alice));
7272

7373
var result = resolver.resolve(provider, "me", "good-token");
7474

7575
assertTrue(result.isPresent());
7676
assertEquals("alice", result.get().getUsername());
77-
verify(cache).store(eq("github/github.com"), anyString(), eq("alice"));
77+
verify(cache).store(eq("github"), anyString(), eq("alice"));
7878
verifyNoMoreInteractions(userStore);
7979
}
8080

8181
// ---- cache miss, delegate returns empty → nothing stored ----
8282

8383
@Test
8484
void cacheMiss_delegateEmpty_nothingStored() {
85-
when(cache.lookup(eq("github/github.com"), anyString())).thenReturn(Optional.empty());
85+
when(cache.lookup(eq("github"), anyString())).thenReturn(Optional.empty());
8686
when(delegate.resolve(provider, "me", "bad-token")).thenReturn(Optional.empty());
8787

8888
var result = resolver.resolve(provider, "me", "bad-token");
@@ -95,30 +95,30 @@ void cacheMiss_delegateEmpty_nothingStored() {
9595

9696
@Test
9797
void sameToken_lookupCalledWithSameHash() {
98-
when(cache.lookup(eq("github/github.com"), anyString())).thenReturn(Optional.empty());
98+
when(cache.lookup(eq("github"), anyString())).thenReturn(Optional.empty());
9999
when(delegate.resolve(any(), any(), any())).thenReturn(Optional.empty());
100100

101101
resolver.resolve(provider, "me", "my-token");
102102
resolver.resolve(provider, "me", "my-token");
103103

104104
// Capture both hash arguments and assert they are equal
105105
var captor = org.mockito.ArgumentCaptor.forClass(String.class);
106-
verify(cache, times(2)).lookup(eq("github/github.com"), captor.capture());
106+
verify(cache, times(2)).lookup(eq("github"), captor.capture());
107107
assertEquals(captor.getAllValues().get(0), captor.getAllValues().get(1));
108108
}
109109

110110
// ---- different tokens produce different hashes ----
111111

112112
@Test
113113
void differentTokens_differentHashes() {
114-
when(cache.lookup(eq("github/github.com"), anyString())).thenReturn(Optional.empty());
114+
when(cache.lookup(eq("github"), anyString())).thenReturn(Optional.empty());
115115
when(delegate.resolve(any(), any(), any())).thenReturn(Optional.empty());
116116

117117
resolver.resolve(provider, "me", "token-a");
118118
resolver.resolve(provider, "me", "token-b");
119119

120120
var captor = org.mockito.ArgumentCaptor.forClass(String.class);
121-
verify(cache, times(2)).lookup(eq("github/github.com"), captor.capture());
121+
verify(cache, times(2)).lookup(eq("github"), captor.capture());
122122
assertNotEquals(captor.getAllValues().get(0), captor.getAllValues().get(1));
123123
}
124124

0 commit comments

Comments
 (0)