Push-Zeitmail: Review-Fixes (Rate-Limit begrenzt, kein NaN-Datum, Störung ≠ gelöscht)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

server/routes/pushTime.js

@@ -13,21 +13,43 @@ import { applyPushTime, formatMinutes, needsTimeEntry, parseMinutes, MAX_MINUTES
 
 const router = Router()
 
-// Einfaches Rate-Limit je IP (30 Anfragen / 15 Minuten)
+// Einfaches Rate-Limit je IP (30 Anfragen / 15 Minuten). Der Speicher bleibt begrenzt: abgelaufene
+// Eintraege fliegen hoechstens einmal pro Minute raus, und bei mehr als RATE_MAX_KEYS IPs wird der
+// aelteste Teil verworfen (Map behaelt die Einfuege-Reihenfolge).
 const RATE_LIMIT = 30
 const RATE_WINDOW_MS = 15 * 60 * 1000
+const RATE_MAX_KEYS = 10000
 const hits = new Map()
+let lastPrune = 0
 
 export function resetPushTimeRateLimitForTests() {
   hits.clear()
+  lastPrune = 0
 }
 
-function rateLimited(ip) {
+export function pushTimeRateLimitSizeForTests() {
-  const now = Date.now()
+  return hits.size
+}
+
+export function isRateLimited(ip, now = Date.now()) {
+  if (now - lastPrune >= 60 * 1000) {
+    lastPrune = now
     for (const [key, entry] of hits) if (entry.resetAt <= now) hits.delete(key)
-  const entry = hits.get(ip) || { count: 0, resetAt: now + RATE_WINDOW_MS }
+  }
-  entry.count += 1
+  let entry = hits.get(ip)
+  if (!entry || entry.resetAt <= now) {
+    hits.delete(ip)
+    if (hits.size >= RATE_MAX_KEYS) {
+      let drop = hits.size - RATE_MAX_KEYS + 1
+      for (const key of hits.keys()) {
+        if (drop-- <= 0) break
+        hits.delete(key)
+      }
+    }
+    entry = { count: 0, resetAt: now + RATE_WINDOW_MS }
     hits.set(ip, entry)
+  }
+  entry.count += 1
   return entry.count > RATE_LIMIT
 }
 
@@ -86,6 +108,12 @@ function errorPage(res, status, title, text) {
   page(res, status, title, `<h1>${e(title)}</h1><p class="mut">${e(text)}</p><a class="btn" href="${e(womsHome())}">Zum Ticketsystem →</a>`)
 }
 
+/** Stoerung (z. B. Appwrite nicht erreichbar): eigene HTML-Seite statt JSON des globalen Handlers */
+function failurePage(res, err) {
+  console.error('[pushTime]', err?.message || err)
+  errorPage(res, 503, 'Gerade nicht erreichbar', 'Bitte versuch es gleich noch einmal. Die Zeit wurde nicht gespeichert.')
+}
+
 /** Repo und Commit-Titel aus den Arbeitsblatt-Details (Format von gitWorksheet.js) */
 function pushInfo(details) {
   const lines = String(details || '').split('\n')
@@ -143,7 +171,7 @@ function alreadyPage(res, ws, ticket) {
 
 /** Token pruefen und Arbeitsblatt laden; schickt bei Fehlern selbst die Seite und liefert null. */
 async function resolve(req, res) {
-  if (rateLimited(req.ip || 'unbekannt')) {
+  if (isRateLimited(req.ip || 'unbekannt')) {
     errorPage(res, 429, 'Zu viele Anfragen', 'Bitte versuch es in ein paar Minuten noch einmal.')
     return null
   }
@@ -159,7 +187,8 @@ async function resolve(req, res) {
   let ws
   try {
     ws = await getDocument(config.collections.worksheets, check.worksheetId)
-  } catch {
+  } catch (err) {
+    if (err?.status !== 404) throw err
     errorPage(res, 404, 'Nicht mehr vorhanden', 'Dieses Arbeitsblatt gibt es nicht mehr.')
     return null
   }
@@ -170,18 +199,18 @@ async function resolve(req, res) {
   return { ws, ticket: await loadTicket(ws) }
 }
 
-router.get('/:token', async (req, res, next) => {
+router.get('/:token', async (req, res) => {
   try {
     const found = await resolve(req, res)
     if (!found) return
     if (!needsTimeEntry(found.ws)) return alreadyPage(res, found.ws, found.ticket)
     formPage(res, 200, { ...found, token: req.params.token, minutes: parseMinutes(req.query.min) ?? '' })
   } catch (err) {
-    next(err)
+    failurePage(res, err)
   }
 })
 
-router.post('/:token', async (req, res, next) => {
+router.post('/:token', async (req, res) => {
   try {
     const found = await resolve(req, res)
     if (!found) return
@@ -199,6 +228,9 @@ router.post('/:token', async (req, res, next) => {
 
     const result = await applyPushTime(found.ws.$id, minutes)
     if (result.status === 'already') return alreadyPage(res, result.worksheet, found.ticket)
+    if (result.status === 'invalid') {
+      return errorPage(res, 422, 'Nicht möglich', 'Bei diesem Push fehlt die Uhrzeit. Trag die Zeit bitte direkt im Ticketsystem ein.')
+    }
     if (result.status !== 'saved') {
       return errorPage(res, 404, 'Nicht mehr vorhanden', 'Dieses Arbeitsblatt gibt es nicht mehr.')
     }
@@ -210,7 +242,7 @@ router.post('/:token', async (req, res, next) => {
       `<h1>Danke!</h1><p class="ok">Eingetragen: ${e(formatMinutes(minutes))} auf #${e(woid)}</p>${openTicket(woid)}`
     )
   } catch (err) {
-    next(err)
+    failurePage(res, err)
   }
 })
 

server/services/pushTimeEntry.js

@@ -12,12 +12,20 @@ export const MAX_MINUTES = 720
 
 const pad = (n) => String(n).padStart(2, '0')
 
-/** Wand-Uhrzeit rueckwaerts rechnen ('tt.mm.jjjj', 'hhmm'); reine Kalenderrechnung, ohne Zeitzone. */
+/**
+ * Wand-Uhrzeit rueckwaerts rechnen ('tt.mm.jjjj', 'hhmm'); reine Kalenderrechnung, ohne Zeitzone
+ * (wie "Zeit nachtragen" in WOMS). Ungueltiges Datum/Uhrzeit -> null.
+ */
 export function startFromEnd(endDate, endTime, minutes) {
-  const [day, month, year] = String(endDate).split('.').map(Number)
+  const date = String(endDate || '').match(/^(\d{2})\.(\d{2})\.(\d{4})$/)
-  const hours = Number(String(endTime).slice(0, 2))
+  const time = String(endTime || '').match(/^(\d{2})(\d{2})$/)
-  const mins = Number(String(endTime).slice(2, 4))
+  if (!date || !time) return null
-  const start = new Date(Date.UTC(year, month - 1, day, hours, mins) - minutes * 60 * 1000)
+  const [day, month, year] = [Number(date[1]), Number(date[2]), Number(date[3])]
+  const [hours, mins] = [Number(time[1]), Number(time[2])]
+  const end = new Date(Date.UTC(year, month - 1, day, hours, mins))
+  // 31.02. o. ae. rollt in Date.UTC weiter - dann ist das Datum ungueltig
+  if (end.getUTCDate() !== day || end.getUTCMonth() !== month - 1 || hours > 23 || mins > 59) return null
+  const start = new Date(end.getTime() - minutes * 60 * 1000)
   return {
     startDate: `${pad(start.getUTCDate())}.${pad(start.getUTCMonth() + 1)}.${start.getUTCFullYear()}`,
     startTime: `${pad(start.getUTCHours())}${pad(start.getUTCMinutes())}`,
@@ -50,20 +58,25 @@ export function formatMinutes(min) {
 }
 
 /**
- * @returns {Promise<{ status: 'saved' | 'already' | 'missing' | 'not_git', worksheet?: object }>}
+ * @returns {Promise<{ status: 'saved' | 'already' | 'missing' | 'not_git' | 'invalid', worksheet?: object }>}
+ *   invalid: Push-Datum/-Uhrzeit im Arbeitsblatt unbrauchbar (nichts gespeichert).
+ *   Andere Appwrite-Fehler als 404 werden weitergeworfen.
  */
 export async function applyPushTime(worksheetId, minutes) {
   return withLocks([`ws-time:${worksheetId}`], async () => {
     let ws
     try {
       ws = await getDocument(config.collections.worksheets, worksheetId)
-    } catch {
+    } catch (err) {
-      return { status: 'missing' }
+      if (err?.status === 404) return { status: 'missing' }
+      throw err
     }
     if (ws.serviceType !== 'GIT') return { status: 'not_git', worksheet: ws }
     if (!needsTimeEntry(ws)) return { status: 'already', worksheet: ws }
 
-    const { startDate, startTime } = startFromEnd(ws.endDate, ws.endTime, minutes)
+    const start = startFromEnd(ws.endDate, ws.endTime, minutes)
+    if (!start) return { status: 'invalid', worksheet: ws }
+    const { startDate, startTime } = start
     const worksheet = await updateDocument(config.collections.worksheets, worksheetId, {
       startDate,
       startTime,

test/pushTimeEntry.test.js

@@ -17,6 +17,7 @@ before(async () => {
         openGit('ws1'),
         openGit('ws-race'),
         { $id: 'remote1', serviceType: 'Remote', startTime: '', totalTime: 0 },
+        { ...openGit('broken'), endDate: '', endTime: 'xx' },
       ],
     },
   })
@@ -72,3 +73,17 @@ test('gleichzeitig: nur einer speichert', async () => {
   const results = await Promise.all([applyPushTime('ws-race', 30), applyPushTime('ws-race', 90)])
   assert.deepEqual(results.map((r) => r.status).sort(), ['already', 'saved'])
 })
+
+test('kaputtes Push-Datum: nichts speichern (Review-Fix)', async () => {
+  assert.equal(startFromEnd('', '', 30), null)
+  assert.equal(startFromEnd('30.09.2026', '16:46', 30), null)
+  assert.equal(startFromEnd('31.02.2026', '1646', 30), null)
+  assert.equal((await applyPushTime('broken', 30)).status, 'invalid')
+  const ws = fake.get('worksheets', 'broken')
+  assert.deepEqual([ws.startTime, ws.totalTime, ws.isComment], ['', 0, true])
+})
+
+test('Appwrite-Störung ist nicht „missing“ (Review-Fix)', async () => {
+  fake.failNext({ method: 'GET', pathIncludes: '/documents/ws-race', status: 500 })
+  await assert.rejects(applyPushTime('ws-race', 30))
+})

test/pushTimeRoute.test.js

@@ -8,6 +8,8 @@ let server
 let baseUrl
 let createPushTimeToken
 let resetPushTimeRateLimitForTests
+let isRateLimited
+let rateLimitSize
 
 const gitSheet = (id, extra = {}) => ({
   $id: id, wsid: '100300', woid: '100118', workorderId: 't1', serviceType: 'GIT',
@@ -38,6 +40,8 @@ before(async () => {
   })
   const routes = await import('../server/routes/pushTime.js')
   resetPushTimeRateLimitForTests = routes.resetPushTimeRateLimitForTests
+  isRateLimited = routes.isRateLimited
+  rateLimitSize = routes.pushTimeRateLimitSizeForTests
   ;({ createPushTimeToken } = await import('../server/services/pushTimeToken.js'))
 
   const app = express()
@@ -159,3 +163,22 @@ test('Rate-Limit: 31. Anfrage → 429', async () => {
   assert.equal(r.status, 429)
   assert.match(r.html, /Zu viele Anfragen/)
 })
+
+test('Rate-Limit-Speicher bleibt begrenzt (Review-Fix)', () => {
+  const now = Date.now()
+  for (let i = 0; i < 12000; i += 1) isRateLimited(`10.0.${Math.floor(i / 250)}.${i % 250}`, now)
+  assert.ok(rateLimitSize() <= 10000, `Größe ${rateLimitSize()}`)
+  // nach Ablauf des Fensters wird aufgeräumt
+  isRateLimited('10.9.9.9', now + 16 * 60 * 1000)
+  assert.equal(rateLimitSize(), 1)
+})
+
+test('Appwrite-Störung: neutrale HTML-Seite statt „Nicht mehr vorhanden“ (Review-Fix)', async () => {
+  const t = token('open1')
+  fake.failNext({ method: 'GET', pathIncludes: '/documents/open1', status: 500 })
+  const r = await get(`/zeit/${t}`)
+  assert.equal(r.status, 503)
+  assert.match(r.headers.get('content-type'), /text\/html/)
+  assert.match(r.html, /Bitte versuch es gleich noch einmal/)
+  assert.doesNotMatch(r.html, /Nicht mehr vorhanden|fakeAppwrite/)
+})