Add tests and address review feedback

Author: akirk

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

@@ -3,7 +3,9 @@ import {
 	sparseCheckout,
 	listGitFiles,
 	resolveCommitHash,
+	type GitAdditionalHeaders,
 } from './git-sparse-checkout';
+import { vi } from 'vitest';
 
 describe('listRefs', () => {
 	it('should return the latest commit hash for a given ref', async () => {
@@ -190,3 +192,135 @@ describe('listGitFiles', () => {
 		);
 	});
 });
+
+describe('gitAdditionalHeaders callback', () => {
+	const repoUrl = 'https://github.com/WordPress/wordpress-playground.git';
+
+	it('should invoke callback with the actual URL being fetched', async () => {
+		const headerCallback = vi.fn<GitAdditionalHeaders>(() => ({}));
+
+		await listGitRefs(repoUrl, 'refs/heads/trunk', headerCallback);
+
+		expect(headerCallback).toHaveBeenCalledWith(repoUrl);
+	});
+
+	it('should successfully fetch when callback returns empty object', async () => {
+		const headerCallback: GitAdditionalHeaders = () => ({});
+
+		const refs = await listGitRefs(
+			repoUrl,
+			'refs/heads/trunk',
+			headerCallback
+		);
+
+		expect(refs).toHaveProperty('refs/heads/trunk');
+		expect(refs['refs/heads/trunk']).toMatch(/^[a-f0-9]{40}$/);
+	});
+
+	it('should pass callback through the full call chain', async () => {
+		const headerCallback = vi.fn<GitAdditionalHeaders>(() => ({}));
+
+		await resolveCommitHash(
+			repoUrl,
+			{ value: 'trunk', type: 'branch' },
+			headerCallback
+		);
+
+		expect(headerCallback).toHaveBeenCalledWith(repoUrl);
+	});
+});
+
+describe('authentication error handling', () => {
+	let originalFetch: typeof global.fetch;
+
+	beforeEach(() => {
+		originalFetch = global.fetch;
+	});
+
+	afterEach(() => {
+		global.fetch = originalFetch;
+	});
+
+	it('should throw GitAuthenticationError for 401 responses', async () => {
+		global.fetch = vi.fn().mockResolvedValue({
+			ok: false,
+			status: 401,
+			statusText: 'Unauthorized',
+		});
+
+		const headerCallback: GitAdditionalHeaders = () => ({
+			Authorization: 'Bearer token',
+		});
+
+		await expect(
+			listGitRefs(
+				'https://github.com/user/private-repo',
+				'refs/heads/main',
+				headerCallback
+			)
+		).rejects.toThrow(
+			'Authentication required to access private repository'
+		);
+	});
+
+	it('should throw GitAuthenticationError for 403 responses', async () => {
+		global.fetch = vi.fn().mockResolvedValue({
+			ok: false,
+			status: 403,
+			statusText: 'Forbidden',
+		});
+
+		const headerCallback: GitAdditionalHeaders = () => ({
+			Authorization: 'Bearer token',
+		});
+
+		await expect(
+			listGitRefs(
+				'https://github.com/user/private-repo',
+				'refs/heads/main',
+				headerCallback
+			)
+		).rejects.toThrow(
+			'Authentication required to access private repository'
+		);
+	});
+
+	it('should throw generic error for 404 even with auth token (ambiguous: repo not found OR no access)', async () => {
+		global.fetch = vi.fn().mockResolvedValue({
+			ok: false,
+			status: 404,
+			statusText: 'Not Found',
+		});
+
+		const headerCallback: GitAdditionalHeaders = () => ({
+			Authorization: 'Bearer token',
+		});
+
+		await expect(
+			listGitRefs(
+				'https://github.com/user/repo-or-no-access',
+				'refs/heads/main',
+				headerCallback
+			)
+		).rejects.toThrow(
+			'Failed to fetch git refs from https://github.com/user/repo-or-no-access: 404 Not Found'
+		);
+	});
+
+	it('should throw generic error for 404 without auth token', async () => {
+		global.fetch = vi.fn().mockResolvedValue({
+			ok: false,
+			status: 404,
+			statusText: 'Not Found',
+		});
+
+		await expect(
+			listGitRefs(
+				'https://github.com/user/nonexistent-repo',
+				'refs/heads/main'
+			)
+		).rejects.toThrow(
+			'Failed to fetch git refs from https://github.com/user/nonexistent-repo: 404 Not Found'
+		);
+	});
+});

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

@@ -44,17 +44,6 @@ export class GitAuthenticationError extends Error {
 
 export type GitAdditionalHeaders = (url: string) => Record<string, string>;
 
-function resolveGitHeaders(
-	url: string,
-	headers?: GitAdditionalHeaders
-): Record<string, string> {
-	if (!headers || typeof headers !== 'function') {
-		return {};
-	}
-
-	return headers(url);
-}
-
 /**
  * Downloads specific files from a git repository.
  * It uses the git protocol over HTTP to fetch the files. It only uses
@@ -98,7 +87,7 @@ export async function sparseCheckout(
 	const treesPack = await fetchWithoutBlobs(
 		repoUrl,
 		commitHash,
-		resolveGitHeaders(repoUrl, options?.additionalHeaders)
+		options?.additionalHeaders?.(repoUrl) ?? {}
 	);
 	const objects = await resolveObjects(treesPack.idx, commitHash, filesPaths);
 
@@ -108,7 +97,7 @@ export async function sparseCheckout(
 			? await fetchObjects(
 					repoUrl,
 					blobOids,
-					resolveGitHeaders(repoUrl, options?.additionalHeaders)
+					options?.additionalHeaders?.(repoUrl) ?? {}
 			  )
 			: null;
 
@@ -219,7 +208,7 @@ export async function listGitFiles(
 	const treesPack = await fetchWithoutBlobs(
 		repoUrl,
 		commitHash,
-		resolveGitHeaders(repoUrl, additionalHeaders)
+		additionalHeaders?.(repoUrl) ?? {}
 	);
 	const rootTree = await resolveAllObjects(treesPack.idx, commitHash);
 	if (!rootTree?.object) {
@@ -306,7 +295,7 @@ export async function listGitRefs(
 			'content-type': 'application/x-git-upload-pack-request',
 			'Content-Length': `${packbuffer.length}`,
 			'Git-Protocol': 'version=2',
-			...resolveGitHeaders(repoUrl, additionalHeaders),
+			...(additionalHeaders?.(repoUrl) ?? {}),
 		},
 		body: packbuffer as any,
 	});

packages/playground/website/src/github/acquire-oauth-token-if-needed.tsx

@@ -34,7 +34,8 @@ export async function acquireOAuthTokenIfNeeded() {
 
 		// Reload the page to retry the blueprint with the new token
 		// This is necessary because the blueprint failed before we had the token
-		window.location.href = url.toString();
+		// Use replace() instead of href assignment to avoid Chrome Error 5
+		window.location.replace(url.toString());
 	} finally {
 		oAuthState.value = {
 			...oAuthState.value,

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

@@ -0,0 +1,140 @@
+import { describe, it, expect, beforeEach, vi } from 'vitest';
+import { createGitHubAuthHeaders } from './git-auth-helpers';
+import { oAuthState } from './state';
+
+vi.mock('virtual:cors-proxy-url', () => ({
+	corsProxyUrl: 'https://corsproxyurl/',
+}));
+
+describe('createGitHubAuthHeaders', () => {
+	beforeEach(() => {
+		oAuthState.value = { token: '', isAuthorizing: false };
+	});
+
+	describe('with GitHub token present', () => {
+		beforeEach(() => {
+			oAuthState.value = {
+				token: 'gho_TestToken123',
+				isAuthorizing: false,
+			};
+		});
+
+		it('includes Authorization header for github.com URLs', () => {
+			const getHeaders = createGitHubAuthHeaders();
+			const headers = getHeaders('https://github.com/user/repo');
+
+			expect(headers).toHaveProperty('Authorization');
+			expect(headers.Authorization).toMatch(/^Basic /);
+			expect(headers).toHaveProperty(
+				'X-Cors-Proxy-Allowed-Request-Headers',
+				'Authorization'
+			);
+		});
+
+		it('includes Authorization header for api.github.com URLs', () => {
+			const getHeaders = createGitHubAuthHeaders();
+			const headers = getHeaders('https://api.github.com/repos');
+
+			expect(headers).toHaveProperty('Authorization');
+		});
+
+		it('includes Authorization header for GitHub URLs through CORS proxy', () => {
+			const getHeaders = createGitHubAuthHeaders();
+			const headers = getHeaders(
+				'https://corsproxyurl/?https://github.com/user/repo'
+			);
+
+			expect(headers).toHaveProperty('Authorization');
+			expect(headers).toHaveProperty(
+				'X-Cors-Proxy-Allowed-Request-Headers'
+			);
+		});
+
+		it('does NOT include Authorization header for non-GitHub URLs', () => {
+			const getHeaders = createGitHubAuthHeaders();
+
+			expect(getHeaders('https://gitlab.com/user/repo')).toEqual({});
+			expect(getHeaders('https://bitbucket.org/user/repo')).toEqual({});
+		});
+
+		it('does NOT include Authorization header for malicious URLs (security)', () => {
+			const getHeaders = createGitHubAuthHeaders();
+
+			// github.com in path
+			expect(getHeaders('https://evil.com/github.com/fake')).toEqual({});
+
+			// github.com in query parameter
+			expect(getHeaders('https://evil.com?redirect=github.com')).toEqual(
+				{}
+			);
+
+			// look-alike domains
+			expect(getHeaders('https://github.com.evil.com')).toEqual({});
+			expect(getHeaders('https://mygithub.com')).toEqual({});
+			expect(getHeaders('https://fakegithub.com')).toEqual({});
+		});
+
+		it('does NOT include Authorization header for non-GitHub URLs through CORS proxy', () => {
+			const getHeaders = createGitHubAuthHeaders();
+			const headers = getHeaders(
+				'https://corsproxyurl/?https://gitlab.com/user/repo'
+			);
+
+			expect(headers).toEqual({});
+		});
+
+		it('does NOT include Authorization header for malicious URLs through CORS proxy (security)', () => {
+			const getHeaders = createGitHubAuthHeaders();
+
+			expect(
+				getHeaders(
+					'https://corsproxyurl/?https://evil.com/github.com/fake'
+				)
+			).toEqual({});
+
+			expect(
+				getHeaders('https://corsproxyurl/?https://github.com.evil.com')
+			).toEqual({});
+		});
+	});
+
+	describe('without GitHub token', () => {
+		beforeEach(() => {
+			oAuthState.value = { token: '', isAuthorizing: false };
+		});
+
+		it('returns empty headers even for GitHub URLs', () => {
+			const getHeaders = createGitHubAuthHeaders();
+
+			expect(getHeaders('https://github.com/user/repo')).toEqual({});
+		});
+	});
+
+	describe('token encoding', () => {
+		it('encodes token correctly as Basic auth', () => {
+			oAuthState.value = { token: 'test-token', isAuthorizing: false };
+			const getHeaders = createGitHubAuthHeaders();
+			const headers = getHeaders('https://github.com/user/repo');
+
+			const decoded = atob(headers.Authorization.replace('Basic ', ''));
+			expect(decoded).toBe('test-token:');
+		});
+
+		it('handles tokens with non-ASCII characters (UTF-8)', () => {
+			// This would fail with plain btoa(): "characters outside of the Latin1 range"
+			oAuthState.value = {
+				token: 'test-token-ąñ-emoji-🔑',
+				isAuthorizing: false,
+			};
+			const getHeaders = createGitHubAuthHeaders();
+			const headers = getHeaders('https://github.com/user/repo');
+
+			expect(headers).toHaveProperty('Authorization');
+			expect(headers.Authorization).toMatch(/^Basic /);
+
+			// Verify the encoding is valid base64
+			const base64Part = headers.Authorization.replace('Basic ', '');
+			expect(() => atob(base64Part)).not.toThrow();
+		});
+	});
+});