diff --git a/.changeset/notifications-redos-fix.md b/.changeset/notifications-redos-fix.md new file mode 100644 index 0000000000..32dbaaf3f2 --- /dev/null +++ b/.changeset/notifications-redos-fix.md @@ -0,0 +1,12 @@ +--- +"@objectstack/runtime": patch +--- + +fix(runtime): replace the polynomial-redos trailing-slash regex in the notifications domain with split+filter (CodeQL high, surfaced by #3507) + +The legacy `path.replace(/\/+$/, '')` in the notifications handler had +carried a polynomial-backtracking regex over request-controlled input since +ADR-0030; the domain extraction (#3507) made the line "changed code" and +CodeQL flagged it. Same split+filter treatment the security domain already +uses for the identical pattern. Redundant slashes in the sub-path now +collapse (`//read//` → `read`), matching the security domain's semantics. diff --git a/packages/runtime/src/domain-handler-registry.test.ts b/packages/runtime/src/domain-handler-registry.test.ts index 28b98824c3..7626fb7750 100644 --- a/packages/runtime/src/domain-handler-registry.test.ts +++ b/packages/runtime/src/domain-handler-registry.test.ts @@ -187,6 +187,14 @@ describe('HttpDispatcher extracted domains (PR-2)', () => { expect(notification.listInbox).toHaveBeenCalledWith('u1', expect.objectContaining({ limit: 5 })); }); + it('/notifications tolerates redundant slashes in the sub-path (split+filter, CodeQL redos fix)', async () => { + const notification = { listInbox: vi.fn(), markRead: vi.fn().mockResolvedValue({ updated: 1 }), markAllRead: vi.fn() }; + const context: any = { executionContext: { userId: 'u1' } }; + const result = await makeDispatcher({ notification }).handleNotification('//read//', 'POST', { ids: ['n1'] }, {}, context); + expect(result.response?.status).toBe(200); + expect(notification.markRead).toHaveBeenCalledWith('u1', ['n1']); + }); + it('/security responds 503 when no security service is wired (legacy in-handler semantics)', async () => { const result = await makeDispatcher().dispatch('GET', '/security/suggested-bindings', undefined, {}, {} as any); expect(result.handled).toBe(true); diff --git a/packages/runtime/src/domains/notifications.ts b/packages/runtime/src/domains/notifications.ts index 90c7f6929c..90a36c45ca 100644 --- a/packages/runtime/src/domains/notifications.ts +++ b/packages/runtime/src/domains/notifications.ts @@ -49,7 +49,12 @@ export async function handleNotificationRequest( } const m = method.toUpperCase(); - const subPath = path.replace(/^\/+/, '').replace(/\/+$/, ''); + // split+filter drops leading/trailing/duplicate slashes without a regex + // over request-controlled input (CodeQL js/polynomial-redos) — same + // treatment the security domain got for the identical latent pattern. + // Surfaced when the extraction (#3507) made this line "changed code": + // the legacy `.replace(/\/+$/, '')` had carried the trap since ADR-0030. + const subPath = path.split('/').filter(Boolean).join('/'); // GET /notifications — list the user's inbox joined with read-state. if (subPath === '' && m === 'GET') {