From e927c45d7ab3832885274aedfb54a9a9e0503ec0 Mon Sep 17 00:00:00 2001 From: Tim Naish Date: Wed, 1 Jul 2026 16:53:53 +0100 Subject: [PATCH] fix: trim whitespace in user handling functions and update tests --- src/config.ts | 6 +++++- src/github.ts | 4 +++- src/google.ts | 2 +- tests/config.spec.ts | 4 ++++ tests/github.spec.ts | 44 ++++++++++++++++++++++++++++++++++++++++++++ tests/google.spec.ts | 10 ++++++++++ 6 files changed, 67 insertions(+), 3 deletions(-) diff --git a/src/config.ts b/src/config.ts index ae74fd008..32823cf4c 100644 --- a/src/config.ts +++ b/src/config.ts @@ -9,7 +9,11 @@ export const config = { return parseInt(process.env.EXIT_CODE_ON_MISMATCH ?? '0') || 0 }, get ignoredUsers(): string[] { - return process.env.IGNORED_USERS?.toLowerCase().split(',') ?? [] + return ( + process.env.IGNORED_USERS?.toLowerCase() + .split(',') + .map((user) => user.trim()) ?? [] + ) }, get githubPrivateKey(): string { return Buffer.from(process.env.GITHUB_PRIVATE_KEY, 'base64').toString('utf-8') diff --git a/src/github.ts b/src/github.ts index be1955ade..24d31e7ca 100644 --- a/src/github.ts +++ b/src/github.ts @@ -49,7 +49,7 @@ export async function getGithubUsersFromGithub(): Promise> { export function formatUserList(users): Set { return new Set( users - .map((user) => user.login?.toLowerCase()) + .map((user) => user.login?.trim().toLowerCase()) .flat() .filter(Boolean), ) @@ -83,6 +83,7 @@ export async function addUsersToGitHubOrg(users: Set): Promise { + user = user.trim() const octokit = mod.getAuthenticatedOctokit() if (config.ignoredUsers.includes(user.toLowerCase())) { console.log(`Ignoring add for ${user}`) @@ -126,6 +127,7 @@ export async function removeUsersFromGitHubOrg(users: Set): Promise { + user = user.trim() const octokit = mod.getAuthenticatedOctokit() if (config.ignoredUsers.includes(user.toLowerCase())) { console.log(`Ignoring remove for ${user}`) diff --git a/src/google.ts b/src/google.ts index cb36543b7..565d94990 100644 --- a/src/google.ts +++ b/src/google.ts @@ -45,7 +45,7 @@ export function formatUserList(users): Set { return new Set( users .filter((user) => !user.suspended && !user.archived) - .map((user) => user.customSchemas?.Accounts?.github?.map((account) => account.value?.toLowerCase())) + .map((user) => user.customSchemas?.Accounts?.github?.map((account) => account.value?.trim().toLowerCase())) .flat() .filter(Boolean), ) diff --git a/tests/config.spec.ts b/tests/config.spec.ts index 9dda40e15..567204083 100644 --- a/tests/config.spec.ts +++ b/tests/config.spec.ts @@ -64,6 +64,10 @@ describe('ignoredUsers', () => { process.env.IGNORED_USERS = 'user1,user2,user3,USER4' expect(mod.config.ignoredUsers).toMatchSnapshot() }) + it('getIgnoredUsers trims whitespace around entries', () => { + process.env.IGNORED_USERS = ' user1, user2 ,user3 ' + expect(mod.config.ignoredUsers).toEqual(['user1', 'user2', 'user3']) + }) }) describe('githubPrivateKey', () => { diff --git a/tests/github.spec.ts b/tests/github.spec.ts index 8575d2dc2..3ebd7cd50 100644 --- a/tests/github.spec.ts +++ b/tests/github.spec.ts @@ -182,4 +182,48 @@ describe('github integration', () => { const response = [{ login: 'chrisns' }, { login: 'chrisns' }, { login: 'foo' }, {}] return expect(mod.formatUserList(response)).toEqual(new Set(['chrisns', 'foo'])) }) + + it('formatUserList trims leading and trailing whitespace from logins', () => { + const response = [{ login: ' chrisns' }, { login: 'foo ' }, { login: ' bar ' }] + return expect(mod.formatUserList(response)).toEqual(new Set(['chrisns', 'foo', 'bar'])) + }) + + it('addUserToGitHubOrg trims whitespace before checking ignored users', () => { + jest.spyOn(config, 'ignoredUsers', 'get').mockReturnValue(['foo']) + return expect(mod.addUserToGitHubOrg(' foo ')).resolves.toBe(false) + }) + + it('addUserToGitHubOrg trims whitespace before looking up the username', async () => { + const fakeOctokit = { + orgs: { + createInvitation: jest.fn().mockResolvedValue(true), + }, + } + jest.spyOn(config, 'githubOrg', 'get').mockReturnValue('myorg') + const getUserIdSpy = jest.spyOn(mod, 'getUserIdFromUsername').mockResolvedValue(123) + // @ts-expect-error mock service isn't a complete implementation, so being lazy and just doing the bare minimum + jest.spyOn(mod, 'getAuthenticatedOctokit').mockReturnValue(fakeOctokit) + const result = await mod.addUserToGitHubOrg(' foo ') + expect(result).toBe(true) + expect(getUserIdSpy).toHaveBeenCalledWith('foo') + }) + + it('removeUserFromGitHubOrg trims whitespace before checking ignored users', () => { + jest.spyOn(config, 'ignoredUsers', 'get').mockReturnValue(['foo']) + return expect(mod.removeUserFromGitHubOrg(' foo ')).resolves.toBe(false) + }) + + it('removeUserFromGitHubOrg trims whitespace before calling the API', async () => { + const fakeOctokit = { + orgs: { + removeMembershipForUser: jest.fn().mockResolvedValue(true), + }, + } + jest.spyOn(config, 'githubOrg', 'get').mockReturnValue('myorg') + // @ts-expect-error mock service isn't a complete implementation, so being lazy and just doing the bare minimum + jest.spyOn(mod, 'getAuthenticatedOctokit').mockReturnValue(fakeOctokit) + const result = await mod.removeUserFromGitHubOrg(' foo ') + expect(result).toBe(true) + expect(fakeOctokit.orgs.removeMembershipForUser).toHaveBeenCalledWith(expect.objectContaining({ username: 'foo' })) + }) }) diff --git a/tests/google.spec.ts b/tests/google.spec.ts index dc2fe0aa9..6ed5feabb 100644 --- a/tests/google.spec.ts +++ b/tests/google.spec.ts @@ -82,4 +82,14 @@ describe('google integration', () => { const result = mod.formatUserList(users) expect(result).toEqual(new Set(['active'])) }) + + it('formatUserList trims leading and trailing whitespace from handles', () => { + const users = [ + { customSchemas: { Accounts: { github: [{ value: ' chrisns' }] } }, suspended: false, archived: false }, + { customSchemas: { Accounts: { github: [{ value: 'foo ' }] } }, suspended: false, archived: false }, + { customSchemas: { Accounts: { github: [{ value: ' bar ' }] } }, suspended: false, archived: false }, + ] + const result = mod.formatUserList(users) + expect(result).toEqual(new Set(['chrisns', 'foo', 'bar'])) + }) })