From 65e12d97a16fdbc1017df473ed8c30e60e4eb61f Mon Sep 17 00:00:00 2001 From: Afonso Jorge Ramos Date: Thu, 1 Oct 2026 01:48:09 +0200 Subject: [PATCH] feat(gitea): accept optional ports in instance hostnames Use forge-specific hostname validation for login, saved accounts, and API requests while retaining HTTPS and host-plus-port origin checks. Closes #3323 --- .../LoginWithPersonalAccessTokenForm.tsx | 3 +- .../LoginWithPersonalAccessToken.test.tsx | 116 +++++++++++++----- src/renderer/stores/useAccountsStore.test.ts | 39 +++++- src/renderer/stores/useAccountsStore.ts | 7 +- src/renderer/utils/auth/utils.ts | 4 +- .../utils/forges/bitbucket/adapter.ts | 2 + src/renderer/utils/forges/gitea/adapter.ts | 2 + src/renderer/utils/forges/gitea/auth.ts | 11 ++ .../utils/forges/gitea/client.test.ts | 75 +++++++++++ src/renderer/utils/forges/gitea/client.ts | 4 +- src/renderer/utils/forges/github/adapter.ts | 2 + src/renderer/utils/forges/gitlab/adapter.ts | 2 + src/renderer/utils/forges/types.ts | 1 + 13 files changed, 223 insertions(+), 45 deletions(-) create mode 100644 src/renderer/utils/forges/gitea/auth.ts diff --git a/src/renderer/components/login/LoginWithPersonalAccessTokenForm.tsx b/src/renderer/components/login/LoginWithPersonalAccessTokenForm.tsx index c313426e3..b7ab4a0ec 100644 --- a/src/renderer/components/login/LoginWithPersonalAccessTokenForm.tsx +++ b/src/renderer/components/login/LoginWithPersonalAccessTokenForm.tsx @@ -13,7 +13,6 @@ import { Header } from '../primitives/Header'; import type { Account, Forge, Hostname, Token } from '../../types'; -import { isValidHostname } from '../../utils/auth/utils'; import { rendererLogError, toError } from '../../utils/core/logger'; import { getAdapter } from '../../utils/forges/registry'; import { openExternalLink } from '../../utils/system/comms'; @@ -59,7 +58,7 @@ export const validateForm = (values: IFormData, forge: Forge = 'github'): IFormE if (!values.hostname) { errors.hostname = 'Hostname is required'; - } else if (!isValidHostname(values.hostname)) { + } else if (!adapter.validateHostname(values.hostname)) { errors.hostname = 'Hostname format is invalid'; } diff --git a/src/renderer/routes/gitea/LoginWithPersonalAccessToken.test.tsx b/src/renderer/routes/gitea/LoginWithPersonalAccessToken.test.tsx index 0305b748c..cfe325304 100644 --- a/src/renderer/routes/gitea/LoginWithPersonalAccessToken.test.tsx +++ b/src/renderer/routes/gitea/LoginWithPersonalAccessToken.test.tsx @@ -9,7 +9,7 @@ import { validateForm, } from '../../components/login/LoginWithPersonalAccessTokenForm'; -import type { Hostname, Token } from '../../types'; +import type { Forge, Hostname, Token } from '../../types'; import * as comms from '../../utils/system/comms'; import { GiteaLoginWithPersonalAccessTokenRoute } from './LoginWithPersonalAccessToken'; @@ -25,6 +25,48 @@ describe('renderer/routes/gitea/LoginWithPersonalAccessToken.tsx', () => { }); describe('form validation', () => { + it.each(['gitea.example.com', 'gitea.example.com:3000', 'forgejo.example.com:443'])( + 'accepts %s', + (hostname) => { + expect( + validateForm( + { + hostname: hostname as Hostname, + token: 'abcdef1234567890abcdef1234567890abcdef12' as Token, + }, + 'gitea', + ), + ).toEqual({}); + }, + ); + + it.each(['gitea.example.com:0', 'gitea.example.com:65536', 'gitea.example.com:https'])( + 'rejects invalid port in %s', + (hostname) => { + expect( + validateForm( + { + hostname: hostname as Hostname, + token: 'abcdef1234567890abcdef1234567890abcdef12' as Token, + }, + 'gitea', + ).hostname, + ).toBe('Hostname format is invalid'); + }, + ); + + it.each(['github', 'gitlab', 'bitbucket'])('keeps ports invalid for %s', (forge) => { + expect( + validateForm( + { + hostname: 'git.example.com:3000' as Hostname, + token: 'abcdef1234567890abcdef1234567890abcdef12' as Token, + }, + forge, + ).hostname, + ).toBe('Hostname format is invalid'); + }); + it('should validate the token matches the Gitea format', () => { const values: IFormData = { hostname: 'gitea.example.com' as Hostname, @@ -44,48 +86,54 @@ describe('renderer/routes/gitea/LoginWithPersonalAccessToken.tsx', () => { }); }); - it('should open the token settings for the configured hostname', async () => { - renderWithProviders(, { - loginWithPersonalAccessToken: loginWithPersonalAccessTokenMock, - }); - - await userEvent.type(screen.getByTestId('login-hostname'), 'gitea.example.com'); + it.each(['gitea.example.com', 'gitea.example.com:3000'])( + 'should open the token settings for %s', + async (hostname) => { + renderWithProviders(, { + loginWithPersonalAccessToken: loginWithPersonalAccessTokenMock, + }); - await userEvent.click(screen.getByTestId('login-create-token')); + await userEvent.type(screen.getByTestId('login-hostname'), hostname); - expect(openExternalLinkSpy).toHaveBeenCalledTimes(1); - expect(openExternalLinkSpy).toHaveBeenCalledWith( - 'https://gitea.example.com/user/settings/applications', - ); - }); + await userEvent.click(screen.getByTestId('login-create-token')); - it('should login using a token - success', async () => { - loginWithPersonalAccessTokenMock.mockResolvedValueOnce(null); + expect(openExternalLinkSpy).toHaveBeenCalledTimes(1); + expect(openExternalLinkSpy).toHaveBeenCalledWith( + `https://${hostname}/user/settings/applications`, + ); + }, + ); - renderWithProviders(, { - loginWithPersonalAccessToken: loginWithPersonalAccessTokenMock, - }); + it.each(['gitea.example.com', 'gitea.example.com:3000'])( + 'should login using a token on %s', + async (hostname) => { + loginWithPersonalAccessTokenMock.mockResolvedValueOnce(null); - await userEvent.type(screen.getByTestId('login-hostname'), 'gitea.example.com'); + renderWithProviders(, { + loginWithPersonalAccessToken: loginWithPersonalAccessTokenMock, + }); - await userEvent.type( - screen.getByTestId('login-token'), - 'abcdef1234567890abcdef1234567890abcdef12', - ); + await userEvent.type(screen.getByTestId('login-hostname'), hostname); - await userEvent.click(screen.getByTestId('login-submit')); + await userEvent.type( + screen.getByTestId('login-token'), + 'abcdef1234567890abcdef1234567890abcdef12', + ); - await waitFor(() => { - expect(loginWithPersonalAccessTokenMock).toHaveBeenCalledTimes(1); - expect(loginWithPersonalAccessTokenMock).toHaveBeenCalledWith({ - hostname: 'gitea.example.com', - token: 'abcdef1234567890abcdef1234567890abcdef12', - forge: 'gitea', + await userEvent.click(screen.getByTestId('login-submit')); + + await waitFor(() => { + expect(loginWithPersonalAccessTokenMock).toHaveBeenCalledTimes(1); + expect(loginWithPersonalAccessTokenMock).toHaveBeenCalledWith({ + hostname, + token: 'abcdef1234567890abcdef1234567890abcdef12', + forge: 'gitea', + }); + expect(navigateMock).toHaveBeenCalledTimes(1); + expect(navigateMock).toHaveBeenCalledWith('/'); }); - expect(navigateMock).toHaveBeenCalledTimes(1); - expect(navigateMock).toHaveBeenCalledWith('/'); - }); - }); + }, + ); it('should login using a token - failure', async () => { loginWithPersonalAccessTokenMock.mockRejectedValueOnce(new Error('invalid token')); diff --git a/src/renderer/stores/useAccountsStore.test.ts b/src/renderer/stores/useAccountsStore.test.ts index 6368f373b..a0dc474da 100644 --- a/src/renderer/stores/useAccountsStore.test.ts +++ b/src/renderer/stores/useAccountsStore.test.ts @@ -1,13 +1,14 @@ import { act, renderHook } from '@testing-library/react'; import { + mockGiteaAccount, mockGitHubCliAccount, mockGitHubCloudAccount, mockGitHubEnterpriseServerAccount, } from '../__mocks__/account-mocks'; import { mockRawUser } from '../utils/forges/github/__mocks__/response-mocks'; -import type { Account, Hostname, Link, Token } from '../types'; +import type { Account, Forge, Hostname, Link, Token } from '../types'; import type { GetAuthenticatedUserResponse } from '../utils/forges/github/types'; import { getRecommendedScopeNames } from '../utils/auth/scopes'; @@ -15,7 +16,7 @@ import * as logger from '../utils/core/logger'; import * as apiClient from '../utils/forges/github/client'; import { getAdapter } from '../utils/forges/registry'; import { DEFAULT_ACCOUNTS_STATE } from './defaults'; -import useAccountsStore from './useAccountsStore'; +import useAccountsStore, { sanitizeAccounts } from './useAccountsStore'; describe('renderer/stores/useAccountsStore.ts', () => { beforeEach(() => { @@ -28,6 +29,40 @@ describe('renderer/stores/useAccountsStore.ts', () => { expect(result.current).toMatchObject(DEFAULT_ACCOUNTS_STATE); }); + describe('sanitizeAccounts', () => { + it('preserves Gitea accounts with an optional port after rehydration', async () => { + const accounts = [ + mockGiteaAccount, + { ...mockGiteaAccount, hostname: 'gitea.example.com:3000' as Hostname }, + ]; + useAccountsStore.setState({ accounts }); + + await useAccountsStore.persist.rehydrate(); + + expect(useAccountsStore.getState().accounts).toEqual(accounts); + }); + + it.each(['gitea.example.com:0', 'gitea.example.com:65536', 'gitea.example.com:3000/path'])( + 'drops Gitea accounts with invalid hostname %s', + (hostname) => { + expect(sanitizeAccounts([{ ...mockGiteaAccount, hostname: hostname as Hostname }])).toEqual( + [], + ); + }, + ); + + it.each(['github', 'gitlab', 'bitbucket'])( + 'continues to reject ports for persisted %s accounts', + (forge) => { + expect( + sanitizeAccounts([ + { ...mockGiteaAccount, forge, hostname: 'git.example.com:3000' as Hostname }, + ]), + ).toEqual([]); + }, + ); + }); + describe('createAccount', () => { vi.spyOn(logger, 'rendererLogInfo').mockImplementation(vi.fn()); diff --git a/src/renderer/stores/useAccountsStore.ts b/src/renderer/stores/useAccountsStore.ts index 638eb3a7c..33448b962 100644 --- a/src/renderer/stores/useAccountsStore.ts +++ b/src/renderer/stores/useAccountsStore.ts @@ -7,7 +7,7 @@ import type { Account, Forge, Hostname, Token } from '../types'; import type { AuthMethod } from '../utils/auth/types'; import type { AccountsState, AccountsStore } from './types'; -import { getAccountUUID, isValidHostname, refreshAccount } from '../utils/auth/utils'; +import { getAccountUUID, refreshAccount } from '../utils/auth/utils'; import { rendererLogInfo, rendererLogWarn } from '../utils/core/logger'; import { getAccountAdapter, getAdapter, isKnownForge } from '../utils/forges/registry'; import { decryptValue, encryptValue } from '../utils/system/comms'; @@ -21,11 +21,12 @@ import { DEFAULT_ACCOUNTS_STATE } from './defaults'; */ export function sanitizeAccounts(accounts: Account[]): Account[] { return accounts.flatMap((a) => { - if (!a.hostname || !isValidHostname(a.hostname)) { + const forge = isKnownForge(a.forge) ? a.forge : 'github'; + if (!a.hostname || !getAdapter(forge).validateHostname(a.hostname)) { return []; } const sanitised: Account = { - forge: isKnownForge(a.forge) ? a.forge : 'github', + forge, method: a.method, platform: a.platform, version: a.version, diff --git a/src/renderer/utils/auth/utils.ts b/src/renderer/utils/auth/utils.ts index 651cb593a..259762a6e 100644 --- a/src/renderer/utils/auth/utils.ts +++ b/src/renderer/utils/auth/utils.ts @@ -1,4 +1,4 @@ -import type { Account, AccountUUID, Hostname, Link } from '../../types'; +import type { Account, AccountUUID, Link } from '../../types'; import { rendererLogError, rendererLogWarn, toError } from '../core/logger'; import { getAccountAdapter } from '../forges/registry'; @@ -52,7 +52,7 @@ export async function refreshAccount(account: Account): Promise { * @param hostname - The hostname string to validate. * @returns `true` if valid. */ -export function isValidHostname(hostname: Hostname) { +export function isValidHostname(hostname: string) { return /^([A-Z0-9]([A-Z0-9-]{0,61}[A-Z0-9])?\.)+[A-Z]{2,6}$/i.test(hostname); } diff --git a/src/renderer/utils/forges/bitbucket/adapter.ts b/src/renderer/utils/forges/bitbucket/adapter.ts index 56ca36c8a..e2f4598f2 100644 --- a/src/renderer/utils/forges/bitbucket/adapter.ts +++ b/src/renderer/utils/forges/bitbucket/adapter.ts @@ -13,6 +13,7 @@ import type { RefreshAccountData, } from '../types'; +import { isValidHostname } from '../../auth/utils'; import { decryptValue } from '../../system/comms'; import { fetchBitbucketAuthenticatedUser, @@ -98,6 +99,7 @@ export const bitbucketAdapter: ForgeAdapter = { defaultHostname: 'bitbucket.org' as Hostname, validateToken: (token) => token.length > 0, + validateHostname: isValidHostname, getPersonalAccessTokenSettingsUrl: (_hostname: Hostname) => ATLASSIAN_TOKEN_SETTINGS_URL, diff --git a/src/renderer/utils/forges/gitea/adapter.ts b/src/renderer/utils/forges/gitea/adapter.ts index bcd9bffb4..37608b18c 100644 --- a/src/renderer/utils/forges/gitea/adapter.ts +++ b/src/renderer/utils/forges/gitea/adapter.ts @@ -9,6 +9,7 @@ import type { } from '../types'; import { createNotificationHandler } from '../github/handlers'; +import { isValidGiteaHostname } from './auth'; import { fetchGiteaAuthenticatedUser, giteaGetJson, @@ -69,6 +70,7 @@ export const giteaAdapter: ForgeAdapter = { // Gitea PATs from /user/settings/applications are 40-char lowercase hex. validateToken: (token: Token) => /^[a-f0-9]{40}$/.test(token), + validateHostname: isValidGiteaHostname, getPersonalAccessTokenSettingsUrl: (hostname: Hostname) => `https://${hostname}/user/settings/applications` as Link, documentationUrl: GITEA_DOCS_URL, diff --git a/src/renderer/utils/forges/gitea/auth.ts b/src/renderer/utils/forges/gitea/auth.ts new file mode 100644 index 000000000..4aaeebf05 --- /dev/null +++ b/src/renderer/utils/forges/gitea/auth.ts @@ -0,0 +1,11 @@ +import { isValidHostname } from '../../auth/utils'; + +export function isValidGiteaHostname(hostname: string): boolean { + const match = /^([^:\s]+)(?::([0-9]{1,5}))?$/.exec(hostname); + if (!match || match[0] !== hostname || !isValidHostname(match[1])) { + return false; + } + + const port = match[2]; + return port === undefined || (Number(port) >= 1 && Number(port) <= 65535); +} diff --git a/src/renderer/utils/forges/gitea/client.test.ts b/src/renderer/utils/forges/gitea/client.test.ts index ed504586c..bbdba9704 100644 --- a/src/renderer/utils/forges/gitea/client.test.ts +++ b/src/renderer/utils/forges/gitea/client.test.ts @@ -33,6 +33,41 @@ describe('renderer/utils/forges/gitea/client.ts', () => { const url = getGiteaApiBaseUrl('gitea.example.com' as Hostname); expect(url.toString()).toBe('https://gitea.example.com/api/v1/'); }); + + it.each([ + ['gitea.example.com:3000', 'https://gitea.example.com:3000/api/v1/'], + ['gitea.example.com:443', 'https://gitea.example.com/api/v1/'], + ['GITEA.example.com:03000', 'https://gitea.example.com:3000/api/v1/'], + ['gitea.example.com:1', 'https://gitea.example.com:1/api/v1/'], + ['gitea.example.com:65535', 'https://gitea.example.com:65535/api/v1/'], + ])('builds the HTTPS API base for %s', (hostname, expected) => { + expect(getGiteaApiBaseUrl(hostname as Hostname).toString()).toBe(expected); + }); + + it.each([ + 'gitea.example.com:', + 'gitea.example.com:0', + 'gitea.example.com:65536', + 'gitea.example.com:-1', + 'gitea.example.com:3.5', + 'gitea.example.com:https', + 'gitea.example.com:3000:4000', + 'https://gitea.example.com:3000', + 'http://gitea.example.com:3000', + 'user@gitea.example.com:3000', + 'gitea.example.com:3000/path', + 'gitea.example.com:3000?query', + 'gitea.example.com:3000#fragment', + 'gitea.example.com:3000\n', + 'gitea.example.com\n', + ' gitea.example.com:3000', + 'gitea.example.com: 3000', + 'localhost:3000', + '127.0.0.1:3000', + '[::1]:3000', + ])('rejects invalid hostname %j', (hostname) => { + expect(() => getGiteaApiBaseUrl(hostname as Hostname)).toThrow(/invalid hostname/); + }); }); describe('listGiteaNotifications', () => { @@ -98,6 +133,19 @@ describe('renderer/utils/forges/gitea/client.ts', () => { }); describe('fetchGiteaAuthenticatedUser', () => { + it('sends the token to the configured HTTPS port', async () => { + fetchMock().mockResolvedValueOnce(jsonResponse({ id: 7, login: 'octocat' })); + + await fetchGiteaAuthenticatedUser({ + ...mockGiteaAccount, + hostname: 'gitea.example.com:3000' as Hostname, + }); + + expect(fetchMock()).toHaveBeenCalledWith('https://gitea.example.com:3000/api/v1/user', { + headers: { Accept: 'application/json', Authorization: 'token decrypted' }, + }); + }); + it('returns the user payload', async () => { fetchMock().mockResolvedValueOnce(jsonResponse({ id: 7, login: 'octocat' })); @@ -121,6 +169,33 @@ describe('renderer/utils/forges/gitea/client.ts', () => { }); describe('giteaGetJson', () => { + it.each([ + ['gitea.example.com:3000', 'https://gitea.example.com:3000/api/v1/x'], + ['gitea.example.com:443', 'https://gitea.example.com/api/v1/x'], + ])('follows URLs on the configured origin for %s', async (hostname, url) => { + fetchMock().mockResolvedValueOnce(jsonResponse({ id: 1 })); + + await expect( + giteaGetJson({ ...mockGiteaAccount, hostname: hostname as Hostname }, url), + ).resolves.toEqual({ id: 1 }); + expect(fetchMock()).toHaveBeenCalledWith(url, { + headers: { Accept: 'application/json', Authorization: 'token decrypted' }, + }); + }); + + it.each([ + ['gitea.example.com:3000', 'https://gitea.example.com:4000/api/v1/x'], + ['gitea.example.com:3000', 'https://gitea.example.com/api/v1/x'], + ['gitea.example.com', 'https://gitea.example.com:3000/api/v1/x'], + ['gitea.example.com:3000', 'http://gitea.example.com:3000/api/v1/x'], + ])('refuses to send the token from %s to %s', async (hostname, url) => { + await expect( + giteaGetJson({ ...mockGiteaAccount, hostname: hostname as Hostname }, url), + ).rejects.toThrow(/cross-origin Gitea URL/); + expect(fetchMock()).not.toHaveBeenCalled(); + expect(comms.decryptValue).not.toHaveBeenCalled(); + }); + it('GETs the supplied URL with auth headers and parses JSON', async () => { fetchMock().mockResolvedValueOnce(jsonResponse({ html_url: 'x' })); diff --git a/src/renderer/utils/forges/gitea/client.ts b/src/renderer/utils/forges/gitea/client.ts index 0c4d7a708..a004a23f9 100644 --- a/src/renderer/utils/forges/gitea/client.ts +++ b/src/renderer/utils/forges/gitea/client.ts @@ -3,13 +3,13 @@ import useSettingsStore from '../../../stores/useSettingsStore'; import type { Account, Hostname } from '../../../types'; import type { GiteaNotificationThread, GiteaUser } from './types'; -import { isValidHostname } from '../../auth/utils'; import { decryptValue } from '../../system/comms'; +import { isValidGiteaHostname } from './auth'; const PAGE_SIZE = 100; export function getGiteaApiBaseUrl(hostname: Hostname): URL { - if (!isValidHostname(hostname)) { + if (!isValidGiteaHostname(hostname)) { throw new Error('Refusing to build a Gitea API URL for invalid hostname.'); } return new URL(`https://${hostname}/api/v1/`); diff --git a/src/renderer/utils/forges/github/adapter.ts b/src/renderer/utils/forges/github/adapter.ts index dc7231c74..c1838dd29 100644 --- a/src/renderer/utils/forges/github/adapter.ts +++ b/src/renderer/utils/forges/github/adapter.ts @@ -12,6 +12,7 @@ import type { Account, Link, RawGitifyNotification } from '../../../types'; import type { AuthMethod } from '../../auth/types'; import type { ForgeAdapter, NotificationDisplayHelpers, RefreshAccountData } from '../types'; +import { isValidHostname } from '../../auth/utils'; import { extractHostVersion, getDeveloperSettingsURL, @@ -102,6 +103,7 @@ export const githubAdapter: ForgeAdapter = { defaultHostname: Constants.GITHUB_HOSTNAME, validateToken: isValidToken, + validateHostname: isValidHostname, getPersonalAccessTokenSettingsUrl: getNewTokenURL, documentationUrl: Constants.GITHUB_DOCS.PAT_URL as Link, getAuthMethodIcon: githubAuthMethodIcon, diff --git a/src/renderer/utils/forges/gitlab/adapter.ts b/src/renderer/utils/forges/gitlab/adapter.ts index 3c5675435..1f0a7744f 100644 --- a/src/renderer/utils/forges/gitlab/adapter.ts +++ b/src/renderer/utils/forges/gitlab/adapter.ts @@ -12,6 +12,7 @@ import type { RefreshAccountData, } from '../types'; +import { isValidHostname } from '../../auth/utils'; import { rendererLogWarn, toError } from '../../core/logger'; import { createNotificationHandler } from '../github/handlers'; import { @@ -140,6 +141,7 @@ export const gitlabAdapter: ForgeAdapter = { // reconfigure the prefix and the modern format embeds a routing suffix, so // any non-empty value is accepted rather than pinning a length or shape. validateToken: (token: Token) => token.trim().length > 0, + validateHostname: isValidHostname, getPersonalAccessTokenSettingsUrl: (hostname: Hostname) => `https://${hostname}/-/user_settings/personal_access_tokens` as Link, diff --git a/src/renderer/utils/forges/types.ts b/src/renderer/utils/forges/types.ts index 3aa2f0274..f4303aa90 100644 --- a/src/renderer/utils/forges/types.ts +++ b/src/renderer/utils/forges/types.ts @@ -146,6 +146,7 @@ export interface ForgeAdapter { /** Default hostname pre-filled in the PAT login form. */ defaultHostname?: Hostname; + validateHostname(hostname: Hostname): boolean; /** Whether the supplied token matches the forge's PAT format. */ validateToken(token: Token): boolean; /** URL to manage/create a personal access token on the forge. */