From 0586f7d6128fd072ea9e6be25b628c97a9883f5b Mon Sep 17 00:00:00 2001 From: hatiyildiz Date: Sat, 9 May 2026 17:19:24 +0200 Subject: [PATCH] fix(catalyst-ui,api): null-map crash on /users + /login open-redirect MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit qa-loop iter-4 cluster `users-page-null-map-and-open-redirect` — TC-028/169/222 (P0) + TC-009 (P1 sec). Sub-A (P0 regression): /users and /provision/{id}/users SPA pages crashed with `TypeError: Cannot read properties of null (reading 'map')` rendering the error boundary. Root cause: the catalyst-api `unstructuredToUserAccess` left `Spec.Applications` as a nil slice when the source UserAccess CR omitted .spec.applications, which Go serializes as `null` over JSON — and the React UserAccessListPage called `applications.map(...)` directly. Fixes: - api: initialize Spec.Applications = []userAccessAppGrantBody{} in unstructuredToUserAccess so the wire shape is `[]` not `null` - ui: defensively normalize each item in listUserAccess (api client) so applications/keycloakGroups null-leaks never reach React - ui: tolerate nulls in grantsSummary, UserAccessListPage items rendering, and MembersList flattenForScope/grantForScope - test: BE check that an empty list serializes as `"items":[]` and that unstructuredToUserAccess emits `"applications":[]` - test: FE renders without crashing when applications is null AND when initialItems is null Sub-B (P1 security CWE-601): TC-009 anonymous /dashboard visit redirected to /login?next=//dashboard. The leading `//` is parsed by the browser as a protocol-relative URL — an attacker could craft `/login?next=//evil.com/path` and bounce victims off-origin after sign-in. Fixes: - new sanitizeNextParam in auth-gate: rejects empty / non-string, embedded NUL or whitespace, backslashes, explicit URL schemes, leading `//`, and any input not starting with a single `/` - rootBeforeLoad: sanitize the deep-link `next` BEFORE the redirect - loginRoute + loginVerifyRoute validateSearch: strip unsafe `next` so URL-supplied attack payloads never reach the components - VerifyPinPage: belt-and-suspenders sanitize at the consumer point (`window.location.replace(target)`) so a future caller bypassing validateSearch still can't smuggle an off-origin URL - test: 7-case sanitizeNextParam coverage (empty, safe paths, multi-slash, scheme-prefixed URLs, backslash variants, relative paths, control chars / whitespace) Files changed: - products/catalyst/bootstrap/api/internal/handler/user_access.go - products/catalyst/bootstrap/api/internal/handler/user_access_test.go - products/catalyst/bootstrap/ui/src/app/auth-gate.ts (+ test) - products/catalyst/bootstrap/ui/src/app/router.tsx - products/catalyst/bootstrap/ui/src/pages/admin/rbac/membersListHelpers.ts (+ test) - products/catalyst/bootstrap/ui/src/pages/admin/user-access/UserAccessListPage.tsx (+ test) - products/catalyst/bootstrap/ui/src/pages/admin/user-access/userAccess.api.ts - products/catalyst/bootstrap/ui/src/pages/auth/VerifyPinPage.tsx Tests: 54 UI tests pass (auth-gate + membersListHelpers + UserAccessListPage), all user_access handler Go tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../api/internal/handler/user_access.go | 8 +++ .../api/internal/handler/user_access_test.go | 45 +++++++++++++ .../bootstrap/ui/src/app/auth-gate.test.ts | 64 +++++++++++++++++++ .../bootstrap/ui/src/app/auth-gate.ts | 43 +++++++++++++ .../catalyst/bootstrap/ui/src/app/router.tsx | 22 +++++-- .../admin/rbac/membersListHelpers.test.ts | 25 ++++++++ .../pages/admin/rbac/membersListHelpers.ts | 13 ++-- .../user-access/UserAccessListPage.test.tsx | 55 +++++++++++++++- .../admin/user-access/UserAccessListPage.tsx | 9 ++- .../pages/admin/user-access/userAccess.api.ts | 41 +++++++++++- .../ui/src/pages/auth/VerifyPinPage.tsx | 9 ++- 11 files changed, 316 insertions(+), 18 deletions(-) diff --git a/products/catalyst/bootstrap/api/internal/handler/user_access.go b/products/catalyst/bootstrap/api/internal/handler/user_access.go index f910d15a..dbf55901 100644 --- a/products/catalyst/bootstrap/api/internal/handler/user_access.go +++ b/products/catalyst/bootstrap/api/internal/handler/user_access.go @@ -442,8 +442,16 @@ func userAccessToUnstructured(req userAccessRequest) *unstructured.Unstructured } func unstructuredToUserAccess(u *unstructured.Unstructured) userAccessItem { + // Initialize slice fields as empty (not nil) so JSON serialization + // emits `[]` rather than `null`. The UI renders `applications.map(...)` + // directly — null would crash the page with a TypeError. Caught on + // console.omantel.biz 2026-05-09 (qa-loop iter-4 cluster + // `users-page-null-map-and-open-redirect`). out := userAccessItem{ Name: u.GetName(), + Spec: userAccessSpecBody{ + Applications: []userAccessAppGrantBody{}, + }, } if ts := u.GetCreationTimestamp(); !ts.IsZero() { out.CreationTimestamp = ts.UTC().Format("2006-01-02T15:04:05Z") diff --git a/products/catalyst/bootstrap/api/internal/handler/user_access_test.go b/products/catalyst/bootstrap/api/internal/handler/user_access_test.go index 4c34d6f8..c6d97feb 100644 --- a/products/catalyst/bootstrap/api/internal/handler/user_access_test.go +++ b/products/catalyst/bootstrap/api/internal/handler/user_access_test.go @@ -140,6 +140,51 @@ func TestListUserAccess_Empty(t *testing.T) { if len(out.Items) != 0 { t.Fatalf("expected empty list; got %d items", len(out.Items)) } + // qa-loop iter-4 cluster `users-page-null-map-and-open-redirect`: + // the empty-list body MUST serialize the items field as `[]` (not + // `null`) so the React UI's `items.map(...)` does not throw + // `TypeError: Cannot read properties of null (reading 'map')`. + rawBody := rec.Body.String() + if !strings.Contains(rawBody, `"items":[]`) { + t.Fatalf("expected items field to serialize as []; body=%s", rawBody) + } + if strings.Contains(rawBody, `"items":null`) { + t.Fatalf("items field must never be null on the wire; body=%s", rawBody) + } +} + +// TestUnstructuredToUserAccess_NilApplicationsBecomesEmpty proves the +// Spec.Applications field initializes to an empty slice (not nil) so +// the JSON encoder emits `"applications":[]` for an UserAccess CR +// missing the field. Mirrors the FE-side normalizer in +// `userAccess.api.ts:normalizeItem`. qa-loop iter-4 cluster +// `users-page-null-map-and-open-redirect`. +func TestUnstructuredToUserAccess_NilApplicationsBecomesEmpty(t *testing.T) { + u := &unstructured.Unstructured{} + u.SetAPIVersion("access.openova.io/v1alpha1") + u.SetKind("UserAccess") + u.SetName("missing-apps") + // No spec at all — verify Applications is still [], not nil. + + out := unstructuredToUserAccess(u) + if out.Spec.Applications == nil { + t.Fatalf("Spec.Applications must be initialized as empty slice (not nil)") + } + if len(out.Spec.Applications) != 0 { + t.Fatalf("Spec.Applications should be empty; got %d", len(out.Spec.Applications)) + } + // JSON-roundtrip — must not produce a `null`. + body, err := json.Marshal(out) + if err != nil { + t.Fatalf("marshal: %v", err) + } + bodyStr := string(body) + if !strings.Contains(bodyStr, `"applications":[]`) { + t.Fatalf("expected applications:[]; got %s", bodyStr) + } + if strings.Contains(bodyStr, `"applications":null`) { + t.Fatalf("applications must never be null; got %s", bodyStr) + } } func TestListUserAccess_Populated(t *testing.T) { diff --git a/products/catalyst/bootstrap/ui/src/app/auth-gate.test.ts b/products/catalyst/bootstrap/ui/src/app/auth-gate.test.ts index a7aa5c6e..8c9f142b 100644 --- a/products/catalyst/bootstrap/ui/src/app/auth-gate.test.ts +++ b/products/catalyst/bootstrap/ui/src/app/auth-gate.test.ts @@ -4,6 +4,7 @@ import { isPublicPath, hasCatalystSession, probeWhoamiAndCacheMarker, + sanitizeNextParam, PUBLIC_PATH_PREFIXES, } from './auth-gate' @@ -181,6 +182,69 @@ describe('probeWhoamiAndCacheMarker', () => { }) }) +describe('sanitizeNextParam — open-redirect defense (CWE-601)', () => { + // qa-loop iter-4 cluster `users-page-null-map-and-open-redirect` — + // TC-009 surfaced /login?next=//dashboard with a leading `//` which + // would let an attacker craft /login?next=//evil.com to bounce the + // operator off-origin after sign-in. + + it('returns undefined for undefined / empty / non-string input', () => { + expect(sanitizeNextParam(undefined)).toBeUndefined() + expect(sanitizeNextParam('')).toBeUndefined() + expect(sanitizeNextParam(null)).toBeUndefined() + expect(sanitizeNextParam(123)).toBeUndefined() + expect(sanitizeNextParam({})).toBeUndefined() + }) + + it('passes safe single-leading-slash paths through unchanged', () => { + expect(sanitizeNextParam('/dashboard')).toBe('/dashboard') + expect(sanitizeNextParam('/provision/d-1/users')).toBe('/provision/d-1/users') + expect(sanitizeNextParam('/users/some-user@example.org?tab=grants')).toBe( + '/users/some-user@example.org?tab=grants', + ) + expect(sanitizeNextParam('/dashboard#anchor')).toBe('/dashboard#anchor') + }) + + it('collapses leading multi-slashes to a single slash (TC-009 attack vector)', () => { + // Protocol-relative URLs treated as host references by the browser + expect(sanitizeNextParam('//dashboard')).toBeUndefined() + expect(sanitizeNextParam('//evil.com/path')).toBeUndefined() + expect(sanitizeNextParam('///foo')).toBeUndefined() + expect(sanitizeNextParam('////a/b')).toBeUndefined() + }) + + it('rejects absolute URLs with explicit schemes', () => { + expect(sanitizeNextParam('http://evil.com/path')).toBeUndefined() + expect(sanitizeNextParam('https://evil.com/path')).toBeUndefined() + expect(sanitizeNextParam('javascript:alert(1)')).toBeUndefined() + expect(sanitizeNextParam('data:text/html,')).toBeUndefined() + expect(sanitizeNextParam('file:///etc/passwd')).toBeUndefined() + expect(sanitizeNextParam('vbscript:msgbox(1)')).toBeUndefined() + }) + + it('rejects backslash-prefixed paths (browser quirk)', () => { + // Some browsers normalize \ to / in URLs — catching `\\evil.com` + // and `/\evil.com` as cousins of `//evil.com`. + expect(sanitizeNextParam('\\\\evil.com/path')).toBeUndefined() + expect(sanitizeNextParam('/\\evil.com')).toBeUndefined() + expect(sanitizeNextParam('\\evil.com')).toBeUndefined() + }) + + it('rejects strings that do not start with /', () => { + expect(sanitizeNextParam('dashboard')).toBeUndefined() + expect(sanitizeNextParam('./dashboard')).toBeUndefined() + expect(sanitizeNextParam('../etc/passwd')).toBeUndefined() + expect(sanitizeNextParam('mailto:attacker@evil.com')).toBeUndefined() + }) + + it('rejects strings containing whitespace or control characters', () => { + expect(sanitizeNextParam('/dashboard\n')).toBeUndefined() + expect(sanitizeNextParam('/dashboard ')).toBeUndefined() + expect(sanitizeNextParam(' /dashboard')).toBeUndefined() + expect(sanitizeNextParam('/dash\x00board')).toBeUndefined() + }) +}) + describe('integration — anti-regression for #1090 cluster A2 bypass routes', () => { // For each of the 7 routes that bypassed the layout-only gate, the // rootBeforeLoad gate should now redirect to /login. We verify the diff --git a/products/catalyst/bootstrap/ui/src/app/auth-gate.ts b/products/catalyst/bootstrap/ui/src/app/auth-gate.ts index d0ec1808..66f02c31 100644 --- a/products/catalyst/bootstrap/ui/src/app/auth-gate.ts +++ b/products/catalyst/bootstrap/ui/src/app/auth-gate.ts @@ -39,6 +39,49 @@ export function canonicalisePath(pathname: string): string { return p.toLowerCase() } +/** + * Sanitize a `next=` redirect target so it can NEVER point to an + * external host (open-redirect class CWE-601). + * + * Rules (strictly reject anything ambiguous — paths only): + * - If the input is empty / non-string → return undefined. + * - Reject embedded NUL / control / whitespace characters. + * - Reject explicit URL schemes (`http:`, `https:`, `javascript:`, + * `data:`, `file:`, `vbscript:`, `mailto:`, etc). + * - Reject backslashes anywhere (some browsers normalize `\` to + * `/`, which would smuggle `\\evil.com` past a leading-slash + * check). + * - Reject anything that doesn't start with a single forward + * slash. In particular `//evil.com/foo` is rejected — the + * browser parses leading `//` as a protocol-relative authority + * reference (host = evil.com). + * + * Returns the sanitized path, or `undefined` if the input is unsafe + * — callers should fall back to the default landing page (e.g. + * `/dashboard` or `/wizard`). + * + * qa-loop iter-4 cluster `users-page-null-map-and-open-redirect` + * (TC-009 / 2026-05-09 routing matrix). The contract: post-login + * navigation MUST land on the same origin under all circumstances. + */ +export function sanitizeNextParam(raw: unknown): string | undefined { + if (typeof raw !== 'string' || raw.length === 0) return undefined + // Reject embedded NULs / control chars / any whitespace. + if (/[\x00-\x1f\s]/.test(raw)) return undefined + // Reject backslashes anywhere — some browsers normalize `\` to `/` + // before parsing, so `/\evil.com` and `\\evil.com` end up host + // references at runtime. + if (raw.indexOf('\\') !== -1) return undefined + // Reject schemes (http:, https:, javascript:, data:, file:, mailto:, + // vbscript:, etc.). A scheme is `[||+|.|-]*:`. + if (/^[a-zA-Z][a-zA-Z0-9+.-]*:/.test(raw)) return undefined + // Require single-leading-slash absolute path. Reject leading `//` + // (protocol-relative authority) and anything that doesn't start + // with `/` (relative paths or bare strings). + if (!raw.startsWith('/') || raw.startsWith('//')) return undefined + return raw +} + /** * Synchronous best-effort check that the operator has a session marker * cached in JS-readable storage. diff --git a/products/catalyst/bootstrap/ui/src/app/router.tsx b/products/catalyst/bootstrap/ui/src/app/router.tsx index 3ea1afea..5699e3e0 100644 --- a/products/catalyst/bootstrap/ui/src/app/router.tsx +++ b/products/catalyst/bootstrap/ui/src/app/router.tsx @@ -105,6 +105,7 @@ import { hasCatalystSession, isPublicPath, probeWhoamiAndCacheMarker, + sanitizeNextParam, } from './auth-gate' /** @@ -161,10 +162,15 @@ async function rootBeforeLoad({ location }: { location: { pathname: string } }) const whoami = await probeWhoamiAndCacheMarker(API_BASE) if (whoami === true) return if (whoami === null) return // 5xx / network error — fail open - // 401 — genuinely unauthenticated. Redirect with deep-link. + // 401 — genuinely unauthenticated. Redirect with deep-link, but + // sanitize the `next` so we can never construct an open-redirect + // payload (CWE-601). qa-loop iter-4 cluster + // `users-page-null-map-and-open-redirect`. + const rawNext = pathname + window.location.search + const safeNext = sanitizeNextParam(rawNext) throw redirect({ to: '/login', - search: { next: pathname + window.location.search }, + search: safeNext ? { next: safeNext } : {}, replace: true, }) } @@ -209,7 +215,12 @@ const loginRoute = createRoute({ component: LoginPage, validateSearch: (raw: Record): { next?: string; error?: string } => { const out: { next?: string; error?: string } = {} - if (typeof raw.next === 'string' && raw.next.length > 0) out.next = raw.next + // Sanitize `next` to prevent open-redirect (CWE-601): an attacker + // could craft /login?next=//evil.com so post-login navigation + // sends the operator off-origin. qa-loop iter-4 cluster + // `users-page-null-map-and-open-redirect`. + const safeNext = sanitizeNextParam(raw.next) + if (safeNext) out.next = safeNext if (typeof raw.error === 'string' && raw.error.length > 0) out.error = raw.error return out }, @@ -237,7 +248,10 @@ const loginVerifyRoute = createRoute({ if (typeof raw.requestId === 'string' && raw.requestId.length > 0) { out.requestId = raw.requestId } - if (typeof raw.next === 'string' && raw.next.length > 0) out.next = raw.next + // Same open-redirect sanitization as /login (CWE-601). qa-loop + // iter-4 cluster `users-page-null-map-and-open-redirect`. + const safeNext = sanitizeNextParam(raw.next) + if (safeNext) out.next = safeNext return out }, }) diff --git a/products/catalyst/bootstrap/ui/src/pages/admin/rbac/membersListHelpers.test.ts b/products/catalyst/bootstrap/ui/src/pages/admin/rbac/membersListHelpers.test.ts index c3f87303..74d54c90 100644 --- a/products/catalyst/bootstrap/ui/src/pages/admin/rbac/membersListHelpers.test.ts +++ b/products/catalyst/bootstrap/ui/src/pages/admin/rbac/membersListHelpers.test.ts @@ -81,6 +81,31 @@ describe('grantForScope', () => { }) }) +describe('null-safety (qa-loop iter-4 cluster users-page-null-map-and-open-redirect)', () => { + // The Go zero-value `[]User` slice serializes as `null`, not `[]`. + // The matrix client mostly normalizes, but defense in depth here: + // flattenForScope and grantForScope must not crash on a null users + // array or a null per-user access map. + it('flattenForScope tolerates users: null', () => { + const broken = { + users: null as unknown as never, + applications: [], + tiers: [], + } as unknown as AccessMatrixResponse + expect(flattenForScope(broken, { kind: 'application', value: 'wordpress' })).toEqual([]) + }) + + it('grantForScope tolerates user.access: null', () => { + const user = { + id: 'x', + source: 'keycloak', + access: null as unknown as never, + } + expect(grantForScope(user as never, { kind: 'application', value: 'wordpress' })).toBeUndefined() + expect(grantForScope(user as never, { kind: 'organization', value: 'acme' })).toBeUndefined() + }) +}) + describe('defaultScopesForScope', () => { it('returns application scope for kind=application', () => { const out = defaultScopesForScope({ kind: 'application', value: 'wordpress' }) diff --git a/products/catalyst/bootstrap/ui/src/pages/admin/rbac/membersListHelpers.ts b/products/catalyst/bootstrap/ui/src/pages/admin/rbac/membersListHelpers.ts index f91ab43a..097be858 100644 --- a/products/catalyst/bootstrap/ui/src/pages/admin/rbac/membersListHelpers.ts +++ b/products/catalyst/bootstrap/ui/src/pages/admin/rbac/membersListHelpers.ts @@ -23,13 +23,17 @@ export interface MemberRowData { /** flattenForScope maps the access-matrix shape (user → application * → grant) to one row per user that has a grant on the requested - * scope. Wildcards (`*`) and exact-app matches both qualify. */ + * scope. Wildcards (`*`) and exact-app matches both qualify. + * + * Defends against `users: null` from a Go zero-value slice — qa-loop + * iter-4 cluster `users-page-null-map-and-open-redirect` (the + * /users-page sibling crash). */ export function flattenForScope( matrix: AccessMatrixResponse, scope: MembersScope, ): MemberRowData[] { const out: MemberRowData[] = [] - for (const u of matrix.users) { + for (const u of matrix.users ?? []) { const grant = grantForScope(u, scope) if (grant) out.push({ user: u, grant }) } @@ -41,15 +45,16 @@ export function grantForScope( user: AccessMatrixUser, scope: MembersScope, ): AccessMatrixGrant | undefined { + const access = user.access ?? {} if (scope.kind === 'application') { // Direct match on the application key, OR fallback to the // synthetic '*' (global) grant. - return user.access[scope.value] ?? user.access['*'] + return access[scope.value] ?? access['*'] } // For organization scope, the matrix already pre-filtered to the // org via the ?org= query — so any grant on the user is in // scope. Surface the highest-tier grant. - const tiers = Object.values(user.access) + const tiers = Object.values(access) return tiers[0] } diff --git a/products/catalyst/bootstrap/ui/src/pages/admin/user-access/UserAccessListPage.test.tsx b/products/catalyst/bootstrap/ui/src/pages/admin/user-access/UserAccessListPage.test.tsx index 9b82a730..ef8a28e5 100644 --- a/products/catalyst/bootstrap/ui/src/pages/admin/user-access/UserAccessListPage.test.tsx +++ b/products/catalyst/bootstrap/ui/src/pages/admin/user-access/UserAccessListPage.test.tsx @@ -9,6 +9,7 @@ import { describe, it, expect, afterEach } from 'vitest' import { render, screen, cleanup } from '@testing-library/react' +import { QueryClient, QueryClientProvider } from '@tanstack/react-query' import { RouterProvider, createRouter, @@ -20,12 +21,17 @@ import { import { UserAccessListPage } from './UserAccessListPage' import type { UserAccessItem } from './userAccess.api' -function renderList(initialItems: UserAccessItem[]) { +function renderList(initialItems: UserAccessItem[] | null | undefined) { const rootRoute = createRootRoute({ component: () => }) const listRoute = createRoute({ getParentRoute: () => rootRoute, path: '/provision/$deploymentId/users', - component: () => , + component: () => ( + + ), }) const newRoute = createRoute({ getParentRoute: () => rootRoute, @@ -47,7 +53,18 @@ function renderList(initialItems: UserAccessItem[]) { routeTree: tree, history: createMemoryHistory({ initialEntries: ['/provision/d-1/users'] }), }) - return render() + // PortalShell + child sidebar consume useQuery hooks, so the tree + // must be wrapped in a QueryClientProvider. Without it every test + // bombs with "No QueryClient set, use QueryClientProvider to set one". + // Fixed alongside qa-loop iter-4 cluster + // `users-page-null-map-and-open-redirect` so the new null-safety + // test rows can render at all. + const qc = new QueryClient({ defaultOptions: { queries: { retry: false } } }) + return render( + + + , + ) } afterEach(() => cleanup()) @@ -117,4 +134,36 @@ describe('UserAccessListPage', () => { expect(cta).toBeTruthy() expect(cta.getAttribute('href') || '').toContain('/provision/d-1/users/new') }) + + // qa-loop iter-4 cluster `users-page-null-map-and-open-redirect` — + // TC-028/169/222 surfaced the page crashing with + // `TypeError: Cannot read properties of null (reading 'map')` when + // the BE serialized Go zero-value `[]Item` slices as `null` over + // the wire. The page renders defensively now even if the API or a + // future refactor leaks nulls back into the props. + it('renders without crashing when item.spec.applications is null', async () => { + const items = [ + { + name: 'broken-grant', + spec: { + user: { keycloakSubject: 'eve' }, + sovereignRef: 'omantel', + // Simulate a wire null leaking past the api-client normalizer. + applications: null as unknown as never, + }, + }, + ] as unknown as UserAccessItem[] + renderList(items) + // The row renders — no React error boundary engaged. + expect(await screen.findByTestId('user-access-row-broken-grant')).toBeTruthy() + // The grants summary cell is empty, but the page is alive. + expect(screen.getByText('eve')).toBeTruthy() + }) + + it('renders without crashing when initialItems itself is null-ish', async () => { + // Belt-and-suspenders: even if a parent component passes null/ + // undefined the page should render the empty-state without crashing. + renderList(null as unknown as UserAccessItem[]) + expect(await screen.findByTestId('user-access-empty-state')).toBeTruthy() + }) }) diff --git a/products/catalyst/bootstrap/ui/src/pages/admin/user-access/UserAccessListPage.tsx b/products/catalyst/bootstrap/ui/src/pages/admin/user-access/UserAccessListPage.tsx index 491b4712..4c935f1d 100644 --- a/products/catalyst/bootstrap/ui/src/pages/admin/user-access/UserAccessListPage.tsx +++ b/products/catalyst/bootstrap/ui/src/pages/admin/user-access/UserAccessListPage.tsx @@ -61,7 +61,10 @@ export function UserAccessListPage({ listUserAccess(deploymentId) .then((rows) => { if (!cancelled) { - setItems(rows) + // Defense in depth — even if a future BE change returns + // `null` for the items array, never crash. qa-loop iter-4 + // cluster `users-page-null-map-and-open-redirect`. + setItems(rows ?? []) setError(null) } }) @@ -117,7 +120,7 @@ export function UserAccessListPage({ {loading ? (
Loading…
- ) : items.length === 0 ? ( + ) : (items ?? []).length === 0 ? (
- {items.map((item) => ( + {(items ?? []).map((item) => ( ({ + ...g, + namespaces: g.namespaces ?? undefined, + vClusters: g.vClusters ?? undefined, + })), + }, + } } export async function createUserAccess( @@ -126,7 +157,11 @@ export function userLabel(user: UserAccessUser): string { /** * Compact summary of an UserAccess Claim's grants for the list view: * "helmwatch (editor), catalyst (admin)" + * + * Defensively handles `null`/`undefined` because Go zero-value slices + * serialize as `null` over the wire. qa-loop iter-4 cluster + * `users-page-null-map-and-open-redirect`. */ -export function grantsSummary(applications: UserAccessAppGrant[]): string { - return applications.map((g) => `${g.app} (${g.role})`).join(', ') +export function grantsSummary(applications: UserAccessAppGrant[] | null | undefined): string { + return (applications ?? []).map((g) => `${g.app} (${g.role})`).join(', ') } diff --git a/products/catalyst/bootstrap/ui/src/pages/auth/VerifyPinPage.tsx b/products/catalyst/bootstrap/ui/src/pages/auth/VerifyPinPage.tsx index d6c98617..cbc40d9a 100644 --- a/products/catalyst/bootstrap/ui/src/pages/auth/VerifyPinPage.tsx +++ b/products/catalyst/bootstrap/ui/src/pages/auth/VerifyPinPage.tsx @@ -19,6 +19,7 @@ import { AuthShell } from '@/app/layouts/AuthLayout' import { Button } from '@/shared/ui/button' import { PinInput6 } from '@/components/PinInput6' import { API_BASE } from '@/shared/config/urls' +import { sanitizeNextParam } from '@/app/auth-gate' type State = 'idle' | 'verifying' | 'error' @@ -92,7 +93,13 @@ export function VerifyPinPage() { // Hard navigation so the next page boot reads the cookie via // /whoami fresh and the auth gate sees the marker. Mirrors the // pattern Fix #A (PR #1093) uses on the unauth path. - const target = next ?? '/wizard' + // + // Belt-and-suspenders open-redirect defense (CWE-601): even + // though /login + /login/verify validateSearch already + // sanitizes `next`, we sanitize again here in case a future + // caller bypasses the route's validateSearch. qa-loop iter-4 + // cluster `users-page-null-map-and-open-redirect`. + const target = sanitizeNextParam(next) ?? '/wizard' if (typeof window !== 'undefined') { window.location.replace(target) return