From bc3c15c1e1fc3370e91ace795d1c08b01a0ecce2 Mon Sep 17 00:00:00 2001 From: Talisson Costa Date: Thu, 6 Aug 2026 15:07:42 -0300 Subject: [PATCH 1/2] fix(organisations): don't fetch an organisation with a non-numeric id organisation-store seeds its id to the string 'account', and the dispatcher falls back to it, so any refresh before an organisation had loaded sent 'account' as the id. That fanned out into a 400 on projects/ and a 500 on organisations/{id}/users/, which surfaced locally as an unhandled rejection because _data rejects with the raw Response. getOrganisation now returns early on a non-numeric id. isNumericId narrows before converting, since Number() also accepts [], ['5'] and true, and requires a positive integer because these are serial keys. Onboarding, where this showed up, is left alone. Its refresh has never worked: the project switcher reads RTK, and createProject already invalidates the Project tag, so nothing depended on the failing call. Co-Authored-By: Claude Opus 5 (1M context) --- frontend/common/stores/organisation-store.js | 16 +++++---- .../utils/__tests__/isNumericId.test.ts | 34 +++++++++++++++++++ frontend/common/utils/isNumericId.ts | 7 ++++ 3 files changed, 51 insertions(+), 6 deletions(-) create mode 100644 frontend/common/utils/__tests__/isNumericId.test.ts create mode 100644 frontend/common/utils/isNumericId.ts diff --git a/frontend/common/stores/organisation-store.js b/frontend/common/stores/organisation-store.js index ff50e0ba316a..3ad6718ae364 100644 --- a/frontend/common/stores/organisation-store.js +++ b/frontend/common/stores/organisation-store.js @@ -7,8 +7,8 @@ import { getSubscriptionMetadata } from 'common/services/useSubscriptionMetadata import Dispatcher from 'common/dispatcher/dispatcher' import BaseStore from './base/_store' import data from 'common/data/base/_data' +import { isNumericId } from 'common/utils/isNumericId' import filter from 'lodash/filter' -import find from 'lodash/find' import findIndex from 'lodash/findIndex' import keyBy from 'lodash/keyBy' @@ -75,10 +75,7 @@ const controller = { const idInt = parseInt(id) store.saving() if (store.model) { - store.model.projects = filter( - store.model.projects, - (p) => p.id !== idInt, - ) + store.model.projects = filter(store.model.projects, (p) => p.id !== idInt) store.model.keyedProjects = keyBy(store.model.projects, 'id') } API.trackEvent(Constants.events.REMOVE_PROJECT) @@ -118,6 +115,11 @@ const controller = { }) }, getOrganisation: (id, force) => { + // store.id seeds to the string 'account' and the dispatcher falls back to + // it, so a non-numeric id reaches here. The API can only reject it. + if (!isNumericId(id)) { + return + } if (`${id}` !== `${store.id}` || force) { store.id = id store.loading() @@ -211,7 +213,9 @@ const controller = { projects.sort((a, b) => { const textA = a.name.toLowerCase() const textB = b.name.toLowerCase() - return textA < textB ? -1 : textA > textB ? 1 : 0 + // Not localeCompare: it orders accents differently. + if (textA < textB) return -1 + return textA > textB ? 1 : 0 }) store.model.projects = projects store.model.keyedProjects = keyBy(store.model.projects, 'id') diff --git a/frontend/common/utils/__tests__/isNumericId.test.ts b/frontend/common/utils/__tests__/isNumericId.test.ts new file mode 100644 index 000000000000..1913466c6696 --- /dev/null +++ b/frontend/common/utils/__tests__/isNumericId.test.ts @@ -0,0 +1,34 @@ +import { isNumericId } from 'common/utils/isNumericId' + +describe('isNumericId', () => { + it.each([1, 42, '7'])('accepts %p', (id) => { + expect(isNumericId(id)).toBe(true) + }) + + // 'account' prompted this: it reached projects/?organisation=account, which + // the API rejects as not an integer. + it.each(['account', 'undefined', 'NaN', '1.5', 'getting-started'])( + 'refuses %p', + (id) => { + expect(isNumericId(id)).toBe(false) + }, + ) + + it.each([undefined, null, '', 0, NaN])('refuses %p', (id) => { + expect(isNumericId(id)).toBe(false) + }) + + // Number() turns each of these into an integer, so truthiness alone lets + // them through. + it.each([true, [], ['5'], {}])('refuses the coercible %p', (id) => { + expect(isNumericId(id)).toBe(false) + }) + + // 0 and '0' disagreed under the truthiness check. Neither is a primary key. + it.each(['0', 0, '-1', -1])( + 'refuses %p, which is not a primary key', + (id) => { + expect(isNumericId(id)).toBe(false) + }, + ) +}) diff --git a/frontend/common/utils/isNumericId.ts b/frontend/common/utils/isNumericId.ts new file mode 100644 index 000000000000..b139d787cdba --- /dev/null +++ b/frontend/common/utils/isNumericId.ts @@ -0,0 +1,7 @@ +// Ids reach API calls unvalidated, since route params and store values arrive +// as strings. Number() also coerces non-ids ([] and true become numbers), so the +// input is narrowed first, and an id must be positive: these are serial keys. +export const isNumericId = (id: unknown): boolean => + (typeof id === 'number' || typeof id === 'string') && + Number.isInteger(Number(id)) && + Number(id) > 0 From 2e13a420665e3ac5778d1badefae577e821fe17f Mon Sep 17 00:00:00 2001 From: Talisson Costa Date: Thu, 6 Aug 2026 15:45:18 -0300 Subject: [PATCH 2/2] test(organisations): pin isNumericId accepting leading-zero ids '007' is accepted on purpose, since DRF coerces it to the key 7, but nothing held that in place. Raised by CodeRabbit on #8222. Co-Authored-By: Claude Opus 5 (1M context) --- frontend/common/utils/__tests__/isNumericId.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/frontend/common/utils/__tests__/isNumericId.test.ts b/frontend/common/utils/__tests__/isNumericId.test.ts index 1913466c6696..2b295e5139c5 100644 --- a/frontend/common/utils/__tests__/isNumericId.test.ts +++ b/frontend/common/utils/__tests__/isNumericId.test.ts @@ -1,7 +1,9 @@ import { isNumericId } from 'common/utils/isNumericId' describe('isNumericId', () => { - it.each([1, 42, '7'])('accepts %p', (id) => { + // '007' is deliberate: DRF coerces a leading-zero string to the key 7, so the + // request works and rejecting it would be stricter than the API. + it.each([1, 42, '7', '007'])('accepts %p', (id) => { expect(isNumericId(id)).toBe(true) })