Skip to content

Commit cf470bf

Browse files
committed
feat(git-token-service): add opaque SCM session capabilities
1 parent a097a66 commit cf470bf

25 files changed

Lines changed: 3071 additions & 238 deletions

apps/web/src/app/api/integrations/gitlab/callback/route.test.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import { getGitLabOAuthCredentials } from '@/lib/integrations/platforms/gitlab/o
88
jest.mock('@/lib/user/server');
99
jest.mock('@/lib/drizzle', () => ({ db: {} }));
1010
jest.mock('@/lib/integrations/gitlab-service', () => ({
11-
normalizeInstanceUrl: jest.fn(),
11+
instanceUrlChanged: jest.fn(),
1212
}));
1313
jest.mock('@/routers/organizations/utils', () => ({
1414
ensureOrganizationAccess: jest.fn(),
@@ -164,11 +164,11 @@ describe('GET /api/integrations/gitlab/callback', () => {
164164
expect(mockedExchangeGitLabOAuthCode).not.toHaveBeenCalled();
165165
});
166166

167-
test('rejects signed state with an unsafe instance URL before exchanging an OAuth code', async () => {
167+
test('rejects signed state with an http instance URL before exchanging an OAuth code', async () => {
168168
const state = createGitLabOAuthState(
169169
{
170170
owner: { type: 'user', id: USER_ID },
171-
instanceUrl: 'http://127.0.0.1:8080',
171+
instanceUrl: 'http://gitlab.example.com',
172172
customCredentialsRef: 'cached-credentials-ref',
173173
},
174174
USER_ID
@@ -180,7 +180,7 @@ describe('GET /api/integrations/gitlab/callback', () => {
180180
)
181181
);
182182

183-
expectRedirectLocation(response, '/integrations/gitlab?error=connection_failed');
183+
expectRedirectLocation(response, '/integrations?error=invalid_state');
184184
expect(mockedGetGitLabOAuthCredentials).not.toHaveBeenCalled();
185185
expect(mockedExchangeGitLabOAuthCode).not.toHaveBeenCalled();
186186
});

apps/web/src/app/api/integrations/gitlab/connect/route.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -181,10 +181,10 @@ describe('GET /api/integrations/gitlab/connect', () => {
181181
expect(responseBody.url).toBe('https://gitlab.com/oauth/authorize?state=signed');
182182
});
183183

184-
test('does not initialize self-hosted OAuth for unsafe instance URLs', async () => {
184+
test('does not initialize self-hosted OAuth for http instance URLs', async () => {
185185
const response = await callGitLabConnectPost(
186186
makeJsonRequest('/api/integrations/gitlab/connect', {
187-
instanceUrl: 'http://127.0.0.1:8080',
187+
instanceUrl: 'http://gitlab.example.com',
188188
clientId: 'client-id',
189189
clientSecret: 'client-secret',
190190
})

apps/web/src/lib/integrations/gitlab-service.test.ts

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { normalizeInstanceUrl } from './gitlab-service';
1+
import { instanceUrlChanged, normalizeInstanceUrl } from './gitlab-service';
22

33
describe('normalizeInstanceUrl', () => {
44
it('treats undefined as gitlab.com', () => {
@@ -24,8 +24,10 @@ describe('normalizeInstanceUrl', () => {
2424
expect(normalizeInstanceUrl('https://gitlab.com')).toBe('https://gitlab.com');
2525
});
2626

27-
it('preserves self-hosted URLs', () => {
28-
expect(normalizeInstanceUrl('http://selfhosted.test:3123')).toBe('http://selfhosted.test:3123');
27+
it('preserves https self-hosted URLs', () => {
28+
expect(normalizeInstanceUrl('https://selfhosted.test:3123')).toBe(
29+
'https://selfhosted.test:3123'
30+
);
2931
});
3032

3133
it('preserves self-hosted base paths', () => {
@@ -34,6 +36,10 @@ describe('normalizeInstanceUrl', () => {
3436
);
3537
});
3638

39+
it('rejects http self-hosted URLs', () => {
40+
expect(() => normalizeInstanceUrl('http://selfhosted.test:3123')).toThrow('must use https');
41+
});
42+
3743
it('rejects unsafe self-hosted URLs', () => {
3844
expect(() => normalizeInstanceUrl('http://127.0.0.1:8080')).toThrow('host is not allowed');
3945
});
@@ -44,12 +50,24 @@ describe('normalizeInstanceUrl', () => {
4450

4551
// different instances
4652
expect(normalizeInstanceUrl('https://gitlab.com')).not.toBe(
47-
normalizeInstanceUrl('http://selfhosted.test:3123')
53+
normalizeInstanceUrl('https://selfhosted.test:3123')
4854
);
4955

5056
// same self-hosted instance with trailing slash difference
51-
expect(normalizeInstanceUrl('http://selfhosted.test:3123/')).toBe(
52-
normalizeInstanceUrl('http://selfhosted.test:3123')
57+
expect(normalizeInstanceUrl('https://selfhosted.test:3123/')).toBe(
58+
normalizeInstanceUrl('https://selfhosted.test:3123')
59+
);
60+
});
61+
62+
it('treats a legacy http URL as changed when reconnecting with https', () => {
63+
expect(instanceUrlChanged('http://selfhosted.test:3123', 'https://selfhosted.test:3123')).toBe(
64+
true
5365
);
5466
});
67+
68+
it('still rejects a new http URL when checking for an instance change', () => {
69+
expect(() =>
70+
instanceUrlChanged('https://selfhosted.test:3123', 'http://selfhosted.test:3123')
71+
).toThrow('must use https');
72+
});
5573
});

apps/web/src/lib/integrations/gitlab-service.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,13 @@ export function normalizeInstanceUrl(url?: string): string {
5353
* Returns true if the GitLab instance URL has changed between
5454
* the existing integration and the new connection.
5555
*/
56-
function instanceUrlChanged(existingUrl?: string, newUrl?: string): boolean {
57-
return normalizeInstanceUrl(existingUrl) !== normalizeInstanceUrl(newUrl);
56+
export function instanceUrlChanged(existingUrl: string | undefined, newUrl: string): boolean {
57+
const normalizedNewUrl = normalizeInstanceUrl(newUrl);
58+
try {
59+
return normalizeInstanceUrl(existingUrl) !== normalizedNewUrl;
60+
} catch {
61+
return true;
62+
}
5863
}
5964

6065
/**

apps/web/src/lib/integrations/oauth/platforms/gitlab-callback.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import {
1313
fetchGitLabProjects,
1414
calculateTokenExpiry,
1515
} from '@/lib/integrations/platforms/gitlab/adapter';
16-
import { normalizeInstanceUrl } from '@/lib/integrations/gitlab-service';
16+
import { instanceUrlChanged } from '@/lib/integrations/gitlab-service';
1717
import {
1818
isDefaultGitLabInstanceUrl,
1919
normalizeGitLabInstanceUrl,
@@ -187,8 +187,10 @@ export async function handleGitLabOAuthCallback(request: NextRequest) {
187187
// Detect if the GitLab instance URL changed (e.g. gitlab.com -> self-hosted)
188188
const isInstanceChange =
189189
existing !== undefined &&
190-
normalizeInstanceUrl(existingMetadata?.gitlab_instance_url as string | undefined) !==
191-
normalizeInstanceUrl(normalizedInstanceUrl);
190+
instanceUrlChanged(
191+
existingMetadata?.gitlab_instance_url as string | undefined,
192+
normalizedInstanceUrl
193+
);
192194

193195
const webhookSecret = isInstanceChange
194196
? generateWebhookSecret()

apps/web/src/lib/integrations/platforms/gitlab/adapter.test.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -350,6 +350,15 @@ describe('validateGitLabInstance', () => {
350350
expect(mockFetch).not.toHaveBeenCalled();
351351
});
352352

353+
it('should return invalid for http instances before fetching', async () => {
354+
const result = await validateGitLabInstance('http://gitlab.example.com');
355+
356+
expect(result.valid).toBe(false);
357+
expect(result.error).toContain('must use https');
358+
expect(mockLookup).not.toHaveBeenCalled();
359+
expect(mockFetch).not.toHaveBeenCalled();
360+
});
361+
353362
it('should return invalid for unsafe hosts before fetching', async () => {
354363
const result = await validateGitLabInstance('http://127.0.0.1:8080');
355364

@@ -871,6 +880,16 @@ describe('validatePersonalAccessToken', () => {
871880
mockFetch.mockReset();
872881
});
873882

883+
it('rejects http instance URLs before fetching', async () => {
884+
const result = await validatePersonalAccessToken('pat-token', 'http://gitlab.example.com');
885+
886+
expect(result).toEqual({
887+
valid: false,
888+
error: 'Invalid URL protocol. GitLab instance URLs must use https.',
889+
});
890+
expect(mockFetch).not.toHaveBeenCalled();
891+
});
892+
874893
it('rejects unsafe instance URLs before fetching', async () => {
875894
const result = await validatePersonalAccessToken('pat-token', 'http://169.254.169.254');
876895

apps/web/src/lib/integrations/platforms/gitlab/instance-url.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ describe('GitLab instance URL safety', () => {
3939

4040
it.each([
4141
['ftp://gitlab.example.com', 'Invalid URL protocol'],
42+
['http://gitlab.example.com', 'must use https'],
4243
[urlWithCredentials.toString(), 'must not include credentials'],
4344
['https://gitlab.example.com?next=/api', 'must not include query strings'],
4445
['https://gitlab.example.com#fragment', 'must not include query strings'],

apps/web/src/lib/integrations/platforms/gitlab/instance-url.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,10 @@ export function normalizeGitLabInstanceUrl(instanceUrl?: string): string {
4242
throw new GitLabInstanceUrlError('GitLab instance URL host is not allowed.');
4343
}
4444

45+
if (url.protocol !== 'https:') {
46+
throw new GitLabInstanceUrlError('Invalid URL protocol. GitLab instance URLs must use https.');
47+
}
48+
4549
const path = normalizeBasePath(url.pathname);
4650
return `${url.protocol}//${url.host.toLowerCase()}${path}`;
4751
}

apps/web/src/lib/integrations/platforms/gitlab/oauth-state.test.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,18 @@ describe('gitlab oauth state', () => {
3838
});
3939
});
4040

41+
test('rejects signed state for an http instance URL', () => {
42+
const state = createGitLabOAuthState(
43+
{
44+
owner: { type: 'user', id: 'user_123' },
45+
instanceUrl: 'http://gitlab.example.com',
46+
},
47+
'user_123'
48+
);
49+
50+
expect(verifyGitLabOAuthState(state)).toBeNull();
51+
});
52+
4153
test('round-trips a validated return path', () => {
4254
const state = createGitLabOAuthState(
4355
{

apps/web/src/lib/integrations/platforms/gitlab/oauth-state.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,9 @@ const GITLAB_OAUTH_STATE_PREFIX = 'gitlab:';
88

99
export const DEFAULT_GITLAB_OAUTH_INSTANCE_URL = 'https://gitlab.com';
1010

11-
function isHttpInstanceUrl(value: string): boolean {
11+
function isHttpsInstanceUrl(value: string): boolean {
1212
try {
13-
const protocol = new URL(value).protocol;
14-
return protocol === 'http:' || protocol === 'https:';
13+
return new URL(value).protocol === 'https:';
1514
} catch {
1615
return false;
1716
}
@@ -22,7 +21,7 @@ const GitLabOAuthStatePayloadSchema = z.object({
2221
z.object({ type: z.literal('user'), id: z.string().min(1) }),
2322
z.object({ type: z.literal('org'), id: z.string().min(1) }),
2423
]),
25-
instanceUrl: z.string().url().refine(isHttpInstanceUrl).optional(),
24+
instanceUrl: z.string().url().refine(isHttpsInstanceUrl).optional(),
2625
customCredentialsRef: z.string().min(1).optional(),
2726
returnTo: z
2827
.string()

0 commit comments

Comments
 (0)