Skip to content

Commit 2a00c7c

Browse files
committed
fix(proxy): remove dead email fallback and add fetch timeout in token identity resolver
GitHub's GET /user only returns email if the user has explicitly made it public — effectively never. Remove the if (identity.email) branch and the email field from ScmUserInfo to avoid the misleading implication that an email fallback exists. Add AbortSignal.timeout(5000) to the GitHub API fetch to prevent the push chain from hanging if the API is slow or unreachable.
1 parent 640ad1c commit 2a00c7c

3 files changed

Lines changed: 12 additions & 29 deletions

File tree

src/proxy/processors/push-action/resolveUserFromToken.ts

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -88,9 +88,7 @@ async function exec(req: Request, action: Action): Promise<Action> {
8888
return action;
8989
}
9090

91-
step.log(
92-
`${provider.name}: resolved SCM identity from token: ${identity.login} (${identity.email ?? 'no public email'})`,
93-
);
91+
step.log(`${provider.name}: resolved SCM identity from token: ${identity.login}`);
9492

9593
const user = await findUserByGitAccount(identity.login);
9694
if (user) {
@@ -107,9 +105,6 @@ async function exec(req: Request, action: Action): Promise<Action> {
107105
`Users can associate their account via PUT /api/v1/user/:username/git-account`,
108106
);
109107
action.user = identity.login;
110-
if (identity.email) {
111-
action.userEmail = identity.email;
112-
}
113108
}
114109
} catch (error: unknown) {
115110
const msg = getErrorMessage(error);

src/proxy/processors/push-action/tokenIdentity.ts

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ import crypto from 'crypto';
1818

1919
export type ScmUserInfo = {
2020
login: string;
21-
email?: string;
2221
};
2322

2423
type CacheEntry = { username: string; provider: string; cachedAt: number };
@@ -68,8 +67,6 @@ export interface TokenIdentityProvider {
6867

6968
type GitHubUserResponse = {
7069
login: string;
71-
id: number;
72-
email: string | null;
7370
};
7471

7572
export class GitHubTokenIdentityProvider implements TokenIdentityProvider {
@@ -86,6 +83,7 @@ export class GitHubTokenIdentityProvider implements TokenIdentityProvider {
8683
Authorization: `token ${token}`,
8784
Accept: 'application/vnd.github+json',
8885
},
86+
signal: AbortSignal.timeout(5000),
8987
});
9088

9189
if (!response.ok) {
@@ -98,7 +96,6 @@ export class GitHubTokenIdentityProvider implements TokenIdentityProvider {
9896
const user: GitHubUserResponse = await response.json();
9997
return {
10098
login: user.login,
101-
email: user.email ?? undefined,
10299
};
103100
} catch (e) {
104101
console.warn(`Failed to fetch GitHub identity: ${e}`);

test/processors/resolveUserFromToken.test.ts

Lines changed: 10 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -69,34 +69,24 @@ describe('GitHubTokenIdentityProvider', () => {
6969
});
7070

7171
describe('fetchScmIdentity', () => {
72-
it('should return login and email on success', async () => {
72+
it('should return login on success', async () => {
7373
fetchSpy.mockResolvedValueOnce({
7474
ok: true,
75-
json: async () => ({ login: 'octocat', id: 1, email: 'octocat@github.com' }),
75+
json: async () => ({ login: 'octocat' }),
7676
} as Response);
7777

7878
const result = await provider.fetchScmIdentity('ghp_token123');
7979

80-
expect(result).toEqual({ login: 'octocat', email: 'octocat@github.com' });
80+
expect(result).toEqual({ login: 'octocat' });
8181
expect(fetchSpy).toHaveBeenCalledWith('https://api.github.com/user', {
8282
headers: {
8383
Authorization: 'token ghp_token123',
8484
Accept: 'application/vnd.github+json',
8585
},
86+
signal: expect.any(AbortSignal),
8687
});
8788
});
8889

89-
it('should return login with undefined email when GitHub returns null email', async () => {
90-
fetchSpy.mockResolvedValueOnce({
91-
ok: true,
92-
json: async () => ({ login: 'private-user', id: 2, email: null }),
93-
} as Response);
94-
95-
const result = await provider.fetchScmIdentity('ghp_token456');
96-
97-
expect(result).toEqual({ login: 'private-user', email: undefined });
98-
});
99-
10090
it('should return null on non-OK response', async () => {
10191
fetchSpy.mockResolvedValueOnce({
10292
ok: false,
@@ -214,7 +204,7 @@ describe('resolveUserFromToken', () => {
214204
it('should resolve GitHub identity from token and fall back to SCM identity when no DB user', async () => {
215205
fetchSpy.mockResolvedValueOnce({
216206
ok: true,
217-
json: async () => ({ login: 'octocat', id: 1, email: 'octocat@github.com' }),
207+
json: async () => ({ login: 'octocat' }),
218208
} as Response);
219209

220210
const req = makeRequest();
@@ -223,12 +213,13 @@ describe('resolveUserFromToken', () => {
223213
const result = await exec(req, action);
224214

225215
expect(result.user).toBe('octocat');
226-
expect(result.userEmail).toBe('octocat@github.com');
216+
expect(result.userEmail).toBeUndefined();
227217
expect(fetchSpy).toHaveBeenCalledWith('https://api.github.com/user', {
228218
headers: {
229219
Authorization: 'token ghp_testtoken123',
230220
Accept: 'application/vnd.github+json',
231221
},
222+
signal: expect.any(AbortSignal),
232223
});
233224
});
234225

@@ -247,7 +238,7 @@ describe('resolveUserFromToken', () => {
247238

248239
fetchSpy.mockResolvedValueOnce({
249240
ok: true,
250-
json: async () => ({ login: 'octocat', id: 1, email: 'octocat@github.com' }),
241+
json: async () => ({ login: 'octocat' }),
251242
} as Response);
252243

253244
const req = makeRequest();
@@ -259,10 +250,10 @@ describe('resolveUserFromToken', () => {
259250
expect(result.userEmail).toBe('thomas.cooper@example.com');
260251
});
261252

262-
it('should handle GitHub identity with null email gracefully', async () => {
253+
it('should leave userEmail from parsePush untouched when no gitAccount match', async () => {
263254
fetchSpy.mockResolvedValueOnce({
264255
ok: true,
265-
json: async () => ({ login: 'octocat', id: 1, email: null }),
256+
json: async () => ({ login: 'octocat' }),
266257
} as Response);
267258

268259
const req = makeRequest();

0 commit comments

Comments
 (0)