Follow-up items surfaced during the code review of the timeline UX rework branch (#242). These were intentionally not fixed in that PR — the access-control/atomicity fixes are addressed separately — and are tracked here.
1. IPv6 rate-limit bypass on login (security)
server/src/routes/auth.ts uses a custom keyGenerator that returns the raw IP:
const getLoginLimiterKey = (req: Request) => req.ip ?? req.socket.remoteAddress ?? 'unknown'
Supplying a custom keyGenerator to express-rate-limit v7 bypasses its built-in IPv6 normalisation (ipKeyGenerator, which buckets by subnet). On IPv6 each distinct address gets its own bucket, so the 5-attempt login limit can be evaded by rotating addresses within a /64.
Why deferred: the locally installed express-rate-limit@7.5.1 tree does not export ipKeyGenerator (verified — only rateLimit, MemoryStore, default are exported), so the fix requires a dependency upgrade.
Fix:
2. Duplicate auto-generated named-resource names under concurrency (reliability)
server/src/routes/namedResources.ts POST reads namedResource.count to auto-generate a numbered name outside the transaction that creates the resource. Two concurrent POSTs can read the same count and generate duplicate auto-names.
Fix:
Surfaced by code review; access-control (IDOR) and count-desync findings from the same review are fixed in a separate PR.
Follow-up items surfaced during the code review of the timeline UX rework branch (#242). These were intentionally not fixed in that PR — the access-control/atomicity fixes are addressed separately — and are tracked here.
1. IPv6 rate-limit bypass on login (security)
server/src/routes/auth.tsuses a customkeyGeneratorthat returns the raw IP:Supplying a custom
keyGeneratortoexpress-rate-limitv7 bypasses its built-in IPv6 normalisation (ipKeyGenerator, which buckets by subnet). On IPv6 each distinct address gets its own bucket, so the 5-attempt login limit can be evaded by rotating addresses within a /64.Why deferred: the locally installed
express-rate-limit@7.5.1tree does not exportipKeyGenerator(verified — onlyrateLimit,MemoryStore,defaultare exported), so the fix requires a dependency upgrade.Fix:
express-rate-limitto a build that exportsipKeyGeneratorgetLoginLimiterKeyand theresetKeypath:return ipKeyGenerator(req.ip ?? req.socket.remoteAddress ?? 'unknown')2. Duplicate auto-generated named-resource names under concurrency (reliability)
server/src/routes/namedResources.tsPOST readsnamedResource.countto auto-generate a numbered name outside the transaction that creates the resource. Two concurrent POSTs can read the same count and generate duplicate auto-names.Fix:
Surfaced by code review; access-control (IDOR) and count-desync findings from the same review are fixed in a separate PR.