Skip to content

Commit 143f30f

Browse files
akirkakirk
committed
Address review feedback
1 parent e3aca80 commit 143f30f

File tree

3 files changed

+63
-43
lines changed

3 files changed

+63
-43
lines changed

‎packages/playground/storage/src/lib/git-sparse-checkout.ts‎

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -44,17 +44,6 @@ export class GitAuthenticationError extends Error {
4444

4545
export type GitAdditionalHeaders = (url: string) => Record<string, string>;
4646

47-
function resolveGitHeaders(
48-
url: string,
49-
headers?: GitAdditionalHeaders
50-
): Record<string, string> {
51-
if (!headers || typeof headers !== 'function') {
52-
return {};
53-
}
54-
55-
return headers(url);
56-
}
57-
5847
/**
5948
* Downloads specific files from a git repository.
6049
* It uses the git protocol over HTTP to fetch the files. It only uses
@@ -98,7 +87,7 @@ export async function sparseCheckout(
9887
const treesPack = await fetchWithoutBlobs(
9988
repoUrl,
10089
commitHash,
101-
resolveGitHeaders(repoUrl, options?.additionalHeaders)
90+
options?.additionalHeaders?.(repoUrl) ?? {}
10291
);
10392
const objects = await resolveObjects(treesPack.idx, commitHash, filesPaths);
10493

@@ -108,7 +97,7 @@ export async function sparseCheckout(
10897
? await fetchObjects(
10998
repoUrl,
11099
blobOids,
111-
resolveGitHeaders(repoUrl, options?.additionalHeaders)
100+
options?.additionalHeaders?.(repoUrl) ?? {}
112101
)
113102
: null;
114103

@@ -219,7 +208,7 @@ export async function listGitFiles(
219208
const treesPack = await fetchWithoutBlobs(
220209
repoUrl,
221210
commitHash,
222-
resolveGitHeaders(repoUrl, additionalHeaders)
211+
additionalHeaders?.(repoUrl) ?? {}
223212
);
224213
const rootTree = await resolveAllObjects(treesPack.idx, commitHash);
225214
if (!rootTree?.object) {
@@ -306,7 +295,7 @@ export async function listGitRefs(
306295
'content-type': 'application/x-git-upload-pack-request',
307296
'Content-Length': `${packbuffer.length}`,
308297
'Git-Protocol': 'version=2',
309-
...resolveGitHeaders(repoUrl, additionalHeaders),
298+
...(additionalHeaders?.(repoUrl) ?? {}),
310299
},
311300
body: packbuffer as any,
312301
});

‎packages/playground/website/src/github/git-auth-helpers.spec.ts‎

Lines changed: 28 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,11 @@
1-
import { describe, it, expect, beforeEach } from 'vitest';
1+
import { describe, it, expect, beforeEach, vi } from 'vitest';
22
import { createGitHubAuthHeaders, isGitHubUrl } from './git-auth-helpers';
33
import { oAuthState } from './state';
44

5+
vi.mock('virtual:cors-proxy-url', () => ({
6+
corsProxyUrl: 'https://corsproxyurl/',
7+
}));
8+
59
describe('isGitHubUrl', () => {
610
describe('direct GitHub URLs', () => {
711
it('returns true for github.com URLs', () => {
@@ -40,34 +44,27 @@ describe('isGitHubUrl', () => {
4044
it('returns true for GitHub URLs through CORS proxy', () => {
4145
expect(
4246
isGitHubUrl(
43-
'https://playground.wordpress.net/cors-proxy.php?https://github.com/user/repo'
44-
)
45-
).toBe(true);
46-
expect(
47-
isGitHubUrl(
48-
'http://127.0.0.1:5263/cors-proxy.php?https://github.com/user/repo'
47+
'https://corsproxyurl/?https://github.com/user/repo'
4948
)
5049
).toBe(true);
5150
});
5251

5352
it('returns false for non-GitHub URLs through CORS proxy', () => {
5453
expect(
5554
isGitHubUrl(
56-
'https://playground.wordpress.net/cors-proxy.php?https://gitlab.com/user/repo'
55+
'https://corsproxyurl/?https://gitlab.com/user/repo'
5756
)
5857
).toBe(false);
5958
});
6059

6160
it('returns false for malicious URLs through CORS proxy', () => {
6261
expect(
6362
isGitHubUrl(
64-
'https://playground.wordpress.net/cors-proxy.php?https://evil.com/github.com/fake'
63+
'https://corsproxyurl/?https://evil.com/github.com/fake'
6564
)
6665
).toBe(false);
6766
expect(
68-
isGitHubUrl(
69-
'http://127.0.0.1:5263/cors-proxy.php?https://github.com.evil.com'
70-
)
67+
isGitHubUrl('https://corsproxyurl/?https://github.com.evil.com')
7168
).toBe(false);
7269
});
7370
});
@@ -116,7 +113,7 @@ describe('createGitHubAuthHeaders integration', () => {
116113
it('includes Authorization header for GitHub URLs through CORS proxy', () => {
117114
const getHeaders = createGitHubAuthHeaders();
118115
const headers = getHeaders(
119-
'https://playground.wordpress.net/cors-proxy.php?https://github.com/user/repo'
116+
'https://corsproxyurl/?https://github.com/user/repo'
120117
);
121118

122119
expect(headers).toHaveProperty('Authorization');
@@ -136,7 +133,7 @@ describe('createGitHubAuthHeaders integration', () => {
136133
it('does NOT include Authorization header for non-GitHub URLs through CORS proxy', () => {
137134
const getHeaders = createGitHubAuthHeaders();
138135
const headers = getHeaders(
139-
'https://playground.wordpress.net/cors-proxy.php?https://gitlab.com/user/repo'
136+
'https://corsproxyurl/?https://gitlab.com/user/repo'
140137
);
141138

142139
expect(headers).toEqual({});
@@ -164,5 +161,22 @@ describe('createGitHubAuthHeaders integration', () => {
164161
const decoded = atob(headers.Authorization.replace('Basic ', ''));
165162
expect(decoded).toBe('test-token:');
166163
});
164+
165+
it('handles tokens with non-ASCII characters (UTF-8)', () => {
166+
// This would fail with plain btoa(): "characters outside of the Latin1 range"
167+
oAuthState.value = {
168+
token: 'test-token-ąñ-emoji-🔑',
169+
isAuthorizing: false,
170+
};
171+
const getHeaders = createGitHubAuthHeaders();
172+
const headers = getHeaders('https://github.com/user/repo');
173+
174+
expect(headers).toHaveProperty('Authorization');
175+
expect(headers.Authorization).toMatch(/^Basic /);
176+
177+
// Verify the encoding is valid base64
178+
const base64Part = headers.Authorization.replace('Basic ', '');
179+
expect(() => atob(base64Part)).not.toThrow();
180+
});
167181
});
168182
});

‎packages/playground/website/src/github/git-auth-helpers.ts‎

Lines changed: 31 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,30 @@
11
import { oAuthState } from './state';
2-
3-
const KNOWN_CORS_PROXY_URLS = [
4-
'https://playground.wordpress.net/cors-proxy.php?',
5-
'https://wordpress-playground-cors-proxy.net/?',
6-
'http://127.0.0.1:5263/cors-proxy.php?',
7-
];
2+
import { corsProxyUrl } from 'virtual:cors-proxy-url';
83

94
export function isGitHubUrl(url: string): boolean {
10-
for (const corsProxyUrl of KNOWN_CORS_PROXY_URLS) {
11-
if (url.startsWith(corsProxyUrl)) {
12-
url = url.substring(corsProxyUrl.length);
13-
break;
14-
}
15-
}
16-
175
try {
186
const urlObj = new URL(url);
7+
const corsProxyOrigin = new URL(corsProxyUrl).origin;
8+
9+
if (urlObj.origin === corsProxyOrigin && urlObj.search) {
10+
const queryWithoutQuestion = urlObj.search.substring(1);
11+
// Check if the query string starts with http:// or https://
12+
if (queryWithoutQuestion.match(/^https?:\/\//)) {
13+
const decodedUrl = decodeURIComponent(queryWithoutQuestion);
14+
try {
15+
const targetUrlObj = new URL(decodedUrl);
16+
const hostname = targetUrlObj.hostname;
17+
return (
18+
hostname === 'github.com' ||
19+
hostname === 'api.github.com'
20+
);
21+
} catch {
22+
// If parsing the target URL fails, fall through to direct check
23+
}
24+
}
25+
}
26+
27+
// Direct URL check
1928
const hostname = urlObj.hostname;
2029
return hostname === 'github.com' || hostname === 'api.github.com';
2130
} catch {
@@ -33,8 +42,16 @@ export function createGitHubAuthHeaders(): (
3342
return {};
3443
}
3544

45+
const encoder = new TextEncoder();
46+
const data = encoder.encode(`${token}:`);
47+
const binary = [];
48+
for (let i = 0; i < data.length; i++) {
49+
binary.push(String.fromCharCode(data[i]));
50+
}
51+
const encodedToken = btoa(binary.join(''));
52+
3653
return {
37-
Authorization: `Basic ${btoa(`${token}:`)}`,
54+
Authorization: `Basic ${encodedToken}`,
3855
// Tell the CORS proxy to forward the Authorization header
3956
'X-Cors-Proxy-Allowed-Request-Headers': 'Authorization',
4057
};

0 commit comments

Comments
 (0)