From d5c70a5bd6e717103869841518df5af5b1542029 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 30 Sep 2026 10:11:59 +0000 Subject: [PATCH] =?UTF-8?q?Gesamtpr=C3=BCfung:=20Zeitlimit=20f=C3=BCr=20Gi?= =?UTF-8?q?tea,=20Aufr=C3=A4umen=20ohne=20Weiterleitung?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 5.5 --- server/config.js | 3 ++ server/services/giteaClient.js | 22 +++++++-- server/services/previewDeploy.js | 3 ++ server/services/repoMove.js | 84 ++++++++++++++++++++++++++++++-- test/giteaTimeout.test.js | 49 +++++++++++++++++++ test/helpers/fakeGitea.js | 47 ++++++++++++++++-- test/repoMove.test.js | 36 ++++++++++++++ 7 files changed, 233 insertions(+), 11 deletions(-) create mode 100644 test/giteaTimeout.test.js diff --git a/server/config.js b/server/config.js index 2dd2c8b..7f117ee 100644 --- a/server/config.js +++ b/server/config.js @@ -249,6 +249,9 @@ export const config = { .filter(Boolean), // Beim Zuweisen eines Projekts werden Pushes der letzten so-vielen Tage nachgetragen backfillDays: Number(process.env.GITEA_BACKFILL_DAYS) || 30, + // Zeitlimit fuer jeden Gitea-API-Aufruf (giteaFetch, fetchPreviewConfig); verhindert ein + // haengendes Portal, wenn Gitea nicht antwortet (Controller-Ruling final-fix Punkt 1). + timeoutMs: Number(process.env.GITEA_TIMEOUT_MS) || 30000, }, cors: { diff --git a/server/services/giteaClient.js b/server/services/giteaClient.js index 239b73d..003ac5f 100644 --- a/server/services/giteaClient.js +++ b/server/services/giteaClient.js @@ -1,5 +1,8 @@ import { config } from '../config.js' +/** Zeitlimit fuer jeden Gitea-API-Aufruf, in ms (ueberschreibbar via `GITEA_TIMEOUT_MS`, s. config.js). */ +const GITEA_TIMEOUT_MS = config.gitea.timeoutMs + /** * Baut die Authorization-/Content-Type-Header fuer einen Gitea-API-Aufruf. * Wirft, wenn GITEA_API_TOKEN nicht gesetzt ist. @@ -18,12 +21,23 @@ function giteaHeaders() { * Ruft `${config.gitea.baseUrl}/api/v1` auf und liefert die geparste JSON-Antwort * (oder null ohne Body). Eine Nicht-2xx-Antwort wirft eine Error mit .status; ein 409 * "same name already exists" wird in die deutsche Meldung uebersetzt (Subdomain-/Repo-Name). + * Bricht nach `GITEA_TIMEOUT_MS` ab (Standard 30 s); der Abbruch wirft eine Error OHNE .status, + * damit repoMove/projectAssignment ihn wie "Gitea nicht erreichbar" einordnen. */ export async function giteaFetch(path, options = {}) { - const response = await fetch(`${config.gitea.baseUrl}/api/v1${path}`, { - ...options, - headers: { ...giteaHeaders(), ...options.headers }, - }) + let response + try { + response = await fetch(`${config.gitea.baseUrl}/api/v1${path}`, { + ...options, + headers: { ...giteaHeaders(), ...options.headers }, + signal: options.signal || AbortSignal.timeout(GITEA_TIMEOUT_MS), + }) + } catch (err) { + if (err?.name === 'TimeoutError' || err?.name === 'AbortError') { + throw new Error(`Gitea antwortet nicht (Zeitlimit ${Math.round(GITEA_TIMEOUT_MS / 1000)} s)`) + } + throw err + } const text = await response.text() let data = null diff --git a/server/services/previewDeploy.js b/server/services/previewDeploy.js index 75310b3..46b2b88 100644 --- a/server/services/previewDeploy.js +++ b/server/services/previewDeploy.js @@ -63,6 +63,9 @@ export async function fetchPreviewConfig(repoFullName, branch) { const url = `${config.gitea.baseUrl}/api/v1/repos/${owner}/${repo}/contents/.webklar-preview.json?ref=${encodeURIComponent(branch)}` const response = await fetch(url, { headers: { Authorization: `token ${config.gitea.apiToken}` }, + // Gleiches Zeitlimit wie giteaFetch (server/services/giteaClient.js) - sonst haengt ein + // Preview-Deploy unbegrenzt, wenn Gitea nicht antwortet. + signal: AbortSignal.timeout(config.gitea.timeoutMs), }) if (!response.ok) return null diff --git a/server/services/repoMove.js b/server/services/repoMove.js index f70b242..ce5191e 100644 --- a/server/services/repoMove.js +++ b/server/services/repoMove.js @@ -142,6 +142,74 @@ async function deletePortalHooks(repoFullName) { return warnings } +/** + * Wie `deletePortalHooks`, aber fuer Schritt 1 (isOrgRepo(from)-Zweig): LIST und DELETE laufen mit + * `redirect: 'manual'`, damit ein von Hand AUS der Organisation herausbewegtes Repo (der Name + * `repoFullName` sieht per Owner noch wie WEBklar aus, Gitea antwortet aber mit einer + * Weiterleitung) nicht angefasst wird - eine ganz normale (Folge-)Anfrage wuerde sonst den + * Portal-Webhook eines inzwischen fremden Repos loeschen (final-fix Punkt 2). Eine 3xx- oder + * 404-Antwort der LIST gilt als "hier nicht (mehr) zu finden" (`{ redirected: true }`); die + * Aufloesung des tatsaechlichen aktuellen Namens ist Sache des Aufrufers. + */ +async function deletePortalHooksStrict(repoFullName) { + const portalUrl = portalWebhookUrl() + let hooks + try { + hooks = await giteaFetch(`/repos/${repoFullName}/hooks`, { redirect: 'manual' }) + } catch (err) { + if (err.status === 404 || (err.status >= 300 && err.status < 400)) return { redirected: true } + console.error(`[repoMove] Webhooks von ${repoFullName} konnten nicht gelesen werden:`, err.message) + return { redirected: false, warnings: [] } + } + + const warnings = [] + for (const hook of hooks || []) { + if (!hookUrl(hook).startsWith(portalUrl)) continue + try { + await giteaFetch(`/repos/${repoFullName}/hooks/${hook.id}`, { method: 'DELETE', redirect: 'manual' }) + } catch (err) { + console.error(`[repoMove] Webhook ${hook.id} bei ${repoFullName} konnte nicht geloescht werden:`, err.message) + warnings.push(WARN.hookNotRemoved(err.message)) + } + } + return { redirected: false, warnings } +} + +/** + * Schritt 1 selbst, wenn `deletePortalHooksStrict` eine Weiterleitung meldet: den kanonischen + * Namen normal (mit Redirect-Folge) aufloesen. Liegt er AUSSERHALB von WEBklar, wurde das Repo von + * Hand herausverschoben - nur das Projektdokument wird nachgezogen, OHNE es automatisch + * zurueckzuholen (final-fix Punkt 2). Liegt er weiterhin in WEBklar (z. B. nur umbenannt), wird + * dort ganz normal aufgeraeumt. + */ +async function resolveRedirectedOrgRepo(project, from) { + let canonical + try { + canonical = (await giteaFetch(`/repos/${from}`)).full_name + } catch (err) { + return classifyGiteaError(err) + } + + if (!isOrgRepo(canonical)) { + try { + await updateProjectDocument(project, canonical) + } catch (err) { + return { moved: null, warnings: [WARN.docNotUpdated(err.message)] } + } + return { moved: null, warnings: [] } + } + + const warnings = await deletePortalHooks(canonical) + if (!sameRepoFullName(canonical, from)) { + try { + await updateProjectDocument(project, canonical) + } catch (err) { + warnings.push(WARN.docNotUpdated(err.message)) + } + } + return { moved: null, warnings } +} + /** * Schritte 7-9, idempotent: Cache fuer alten und neuen Namen verwerfen, alte Portal-Webhooks am * neuen Ort loeschen, Preview-Arbeitsverzeichnis des alten Namens aufraeumen. Laeuft unabhaengig @@ -207,6 +275,11 @@ async function moveViaTransfer(project, from) { } const confirmedTo = transferResult.full_name + // Ein-Zeiler-Absicherung (final-fix Punkt 3): Gitea kann 2xx melden, ohne das Repo tatsaechlich + // verschoben zu haben (z. B. Teil-Fehlschlag). Erst wenn der bestaetigte Name wirklich in + // WEBklar liegt, gilt der Transfer als geglueckt - sonst kein Dokument-Update, kein Aufraeumen. + if (!isOrgRepo(confirmedTo)) return { failed: 'notMoved' } + let docErr = null try { await updateProjectDocument(project, confirmedTo) @@ -217,6 +290,9 @@ async function moveViaTransfer(project, from) { }) if (outcome.failed === 'gitea') return classifyGiteaError(outcome.err) + if (outcome.failed === 'notMoved') { + return { moved: null, warnings: ['Umzug nach WEBklar nicht möglich (Übertragung nicht abgeschlossen).'] } + } const confirmedTo = outcome.to // Das Aufraeumen (Cache, alter Webhook, Preview-Verzeichnis) haengt nicht am Dokument-Update - @@ -238,10 +314,12 @@ export async function moveRepoToOrg(project) { const from = project.repoFullName // Schritt 1: schon in der Organisation - nur noch nach einem liegen gebliebenen Einzel-Webhook - // schauen (R11), damit "Pushes nachtragen" auch das noch nachholen kann. + // schauen (R11), damit "Pushes nachtragen" auch das noch nachholen kann. Ohne Redirect-Folge + // (final-fix Punkt 2), damit ein von Hand herausverschobenes Repo nicht angefasst wird. if (isOrgRepo(from)) { - const warnings = await deletePortalHooks(from) - return { moved: null, warnings } + const result = await deletePortalHooksStrict(from) + if (result.redirected) return resolveRedirectedOrgRepo(project, from) + return { moved: null, warnings: result.warnings } } // Schritt 2: aktuellen Stand holen (fetch folgt Giteas 301-Weiterleitung von Hand verschobener diff --git a/test/giteaTimeout.test.js b/test/giteaTimeout.test.js new file mode 100644 index 0000000..14835fd --- /dev/null +++ b/test/giteaTimeout.test.js @@ -0,0 +1,49 @@ +import { test, before, after } from 'node:test' +import assert from 'node:assert/strict' +import { startFakeGitea } from './helpers/fakeGitea.js' + +let gitea +let giteaFetch +let moveRepoToOrg + +// Eigene Datei, weil GITEA_TIMEOUT_MS hier absichtlich sehr klein gesetzt wird (config.js liest +// die Umgebung beim Import - andere Testdateien duerfen davon nicht betroffen sein, jede +// Testdatei laeuft in einem eigenen Prozess). +before(async () => { + gitea = await startFakeGitea({ repos: [{ full_name: 'knso/langsam' }] }) + + Object.assign(process.env, { + GITEA_BASE_URL: gitea.baseUrl, + GITEA_API_TOKEN: 'test-token', + GITEA_TIMEOUT_MS: '200', + }) + ;({ giteaFetch } = await import('../server/services/giteaClient.js')) + ;({ moveRepoToOrg } = await import('../server/services/repoMove.js')) +}) + +after(async () => { + await gitea.close() +}) + +test('giteaFetch bricht bei Zeitueberschreitung ohne .status ab', async () => { + gitea.setResponseDelay(2000) + const start = Date.now() + await assert.rejects( + giteaFetch('/repos/knso/langsam'), + (err) => { + assert.equal(err.status, undefined) + assert.match(err.message, /Zeitlimit/) + return true + } + ) + assert.ok(Date.now() - start < 2000, 'giteaFetch haette nach ~200ms abbrechen muessen') +}) + +test('moveRepoToOrg meldet "Gitea ist gerade nicht erreichbar" statt zu haengen', async () => { + gitea.setResponseDelay(2000) + const start = Date.now() + const r = await moveRepoToOrg({ $id: 'x', repoFullName: 'knso/langsam' }) + assert.ok(Date.now() - start < 2000, 'moveRepoToOrg haette nicht haengen bleiben duerfen') + assert.equal(r.moved, null) + assert.match(r.warnings[0], /^Gitea ist gerade nicht erreichbar \(/) +}) diff --git a/test/helpers/fakeGitea.js b/test/helpers/fakeGitea.js index d360638..98d4afc 100644 --- a/test/helpers/fakeGitea.js +++ b/test/helpers/fakeGitea.js @@ -42,20 +42,27 @@ import { createHash } from 'node:crypto' * hooksOf: (fullName: string) => object[], * setDown: (down: boolean) => void, * setTransferDelay: (ms: number) => void, + * setResponseDelay: (ms: number) => void, + * setTransferNoop: () => void, * failNext: (spec: {method: string, pathIncludes: string, status: number}) => void, * close: () => Promise, * }>} `calls` waechst live, in der Reihenfolge, in der die Anfragen eintreffen - VOR einer - * etwaigen `setTransferDelay`-Verzoegerung der Antwort. `repo()`/`hooksOf()` lesen den - * aktuellen Stand direkt (ohne Weiterleitung ueber einen umgezogenen alten Namen). + * etwaigen `setTransferDelay`/`setResponseDelay`-Verzoegerung der Antwort. `repo()`/`hooksOf()` + * lesen den aktuellen Stand direkt (ohne Weiterleitung ueber einen umgezogenen alten Namen). * `failNext({method, pathIncludes, status})` laesst GENAU die naechste Anfrage, deren Methode * passt und deren Pfad `pathIncludes` enthaelt, mit diesem Status und einer generischen * JSON-Fehlermeldung scheitern (danach wieder normales Verhalten) - fuer Tests, die einen * einzelnen Aufruf gezielt fehlschlagen lassen wollen, ohne den ganzen Server abzuschalten. + * `setResponseDelay(ms)` verzoegert JEDE Antwort um `ms` (z.B. fuer einen Timeout-Test). + * `setTransferNoop()` laesst genau den naechsten Transfer-Aufruf mit 201 und UNVERAENDERTEM + * `full_name` antworten (Gitea meldet Erfolg, hat aber nichts verschoben). */ export async function startFakeGitea(options = {}) { const calls = [] let isDown = false let transferDelayMs = 0 + let responseDelayMs = 0 + let transferNoopNext = false const pendingFailures = [] let nextRepoId = 1 @@ -235,9 +242,17 @@ export async function startFakeGitea(options = {}) { const found = findRepoOrRedirect(match[1], match[2]) if (!found) return send(res, 404, { message: 'Not Found' }) if (found.redirectTo) return sendRepoRedirect(res, found.redirectTo, '/transfer') - const key = found.key const record = found.record + // Simuliert eine Gitea-Antwort, die Erfolg meldet, aber tatsaechlich nichts verschoben hat + // (Bug-Szenario fuer Controller-Ruling R10 Ein-Zeiler-Absicherung): 201 mit UNVERAENDERTEM + // full_name, kein Owner-/Key-Wechsel, keine Weiterleitung. + if (transferNoopNext) { + transferNoopNext = false + return send(res, 201, serializeRepo(record)) + } + + const key = found.key const newOwner = body?.new_owner if (!newOwner) return send(res, 422, { message: 'new_owner erforderlich' }) @@ -435,6 +450,8 @@ export async function startFakeGitea(options = {}) { body: parsedBody, }) + if (responseDelayMs > 0) await sleep(responseDelayMs) + const failIdx = pendingFailures.findIndex( (f) => f.method === req.method && url.pathname.includes(f.pathIncludes) ) @@ -510,6 +527,16 @@ export async function startFakeGitea(options = {}) { transferDelayMs = Number(ms) || 0 } + /** Verzoegert JEDE Antwort um `ms` (z.B. um einen `GITEA_TIMEOUT_MS`-Abbruch zu provozieren). */ + function setResponseDelay(ms) { + responseDelayMs = Number(ms) || 0 + } + + /** Laesst den naechsten Transfer-Aufruf 201 mit UNVERAENDERTEM full_name melden (kein Umzug). */ + function setTransferNoop() { + transferNoopNext = true + } + function failNext({ method, pathIncludes, status }) { pendingFailures.push({ method, pathIncludes, status }) } @@ -519,5 +546,17 @@ export async function startFakeGitea(options = {}) { return new Promise((resolve) => server.close(() => resolve())) } - return { baseUrl, calls, repo, addRepo, hooksOf, setDown, setTransferDelay, failNext, close } + return { + baseUrl, + calls, + repo, + addRepo, + hooksOf, + setDown, + setTransferDelay, + setResponseDelay, + setTransferNoop, + failNext, + close, + } } diff --git a/test/repoMove.test.js b/test/repoMove.test.js index 9115d35..b64dd67 100644 --- a/test/repoMove.test.js +++ b/test/repoMove.test.js @@ -44,6 +44,7 @@ before(async () => { { full_name: 'JUSN/doc-fail-test' }, { full_name: 'JUSN/verboten' }, { full_name: 'JUSN/Case-Test' }, + { full_name: 'knso/rausgezogen' }, ], // C1: Hooks kommen (wie im echten Gitea) OHNE Top-Level-`url` an - `url` hier ist nur die // Seed-Abkuerzung, gespeichert/ausgeliefert wird das Ziel ausschliesslich unter `config.url`. @@ -53,10 +54,14 @@ before(async () => { 'WEBklar/von-hand': [{ id: 210, url: PORTAL_HOOK_URL }], 'WEBklar/schon-da': [{ id: 310, url: PORTAL_HOOK_URL }], 'JUSN/doc-fail-test': [{ id: 410, url: PORTAL_HOOK_URL }], + 'knso/rausgezogen': [{ id: 510, url: PORTAL_HOOK_URL }], }, // knso/von-hand wurde von Hand bereits nach WEBklar/von-hand verschoben (kein Transfer-Aufruf noetig). + // WEBklar/rausgezogen wurde von Hand AUS der Organisation heraus nach knso/rausgezogen verschoben + // (final-fix Punkt 2: Schritt-1-Aufraeumen darf dieser Weiterleitung nicht folgen). redirects: { 'knso/von-hand': 'WEBklar/von-hand', + 'WEBklar/rausgezogen': 'knso/rausgezogen', }, }) @@ -72,6 +77,7 @@ before(async () => { 'JUSN/doc-fail-test', 'JUSN/verboten', 'jusn/case-test', // Appwrite kennt eine andere Schreibweise als Gitea (dort: JUSN/Case-Test) + 'WEBklar/rausgezogen', // Appwrite kennt noch den WEBklar-Namen, Gitea hat es herausverschoben ] const websiteProjects = repoNames.map((repoFullName) => { const $id = randomUUID() @@ -200,3 +206,33 @@ test('Gitea nicht erreichbar', async () => { assert.equal(r.moved, null) assert.match(r.warnings[0], /^Gitea ist gerade nicht erreichbar \(.+\)\. Bitte später ‚Pushes nachtragen‘ klicken\.$/) }) +test('Schritt 1 folgt keiner Weiterleitung: von Hand aus WEBklar herausverschobenes Repo wird nicht angefasst', async () => { + // final-fix Punkt 2: WEBklar/rausgezogen (isOrgRepo(from) === true) leitet inzwischen auf + // knso/rausgezogen um - das Aufraeumen darf dem NICHT folgen (sonst wuerde der Portal-Webhook + // eines fremden Repos geloescht). Stattdessen wird der kanonische Name aufgeloest und, weil er + // ausserhalb von WEBklar liegt, nur das Projektdokument nachgezogen. + const r = await moveRepoToOrg(doc('WEBklar/rausgezogen')) + assert.deepEqual(r, { moved: null, warnings: [] }) + assert.equal( + gitea.calls.some((c) => c.method === 'DELETE' && /rausgezogen/i.test(c.path)), + false + ) + assert.deepEqual(gitea.hooksOf('knso/rausgezogen'), [ + { id: 510, type: 'gitea', active: true, events: ['push'], config: { content_type: 'json', url: PORTAL_HOOK_URL } }, + ]) + const d = aw.get('websiteProjects', doc('WEBklar/rausgezogen').$id) + assert.equal(d.repoFullName, 'knso/rausgezogen') +}) +test('R10-Ein-Zeiler: Transfer meldet Erfolg, aber unveraenderten full_name -> kein Aufraeumen', async () => { + // final-fix Punkt 3: der Transfer selbst antwortet 201, aber full_name ist danach nicht in + // WEBklar (simuliert einen Gitea-Bug/Teil-Fehlschlag). Erst NACH dieser Pruefung darf + // aufgeraeumt werden - hier eben nicht. + gitea.setTransferNoop() + const r = await moveRepoToOrg(doc('ANDJ/unberuehrt')) + assert.equal(r.moved, null) + assert.deepEqual(r.warnings, ['Umzug nach WEBklar nicht möglich (Übertragung nicht abgeschlossen).']) + assert.equal( + gitea.calls.some((c) => c.method === 'DELETE' && /unberuehrt/i.test(c.path)), + false + ) +})