From ae798034c5d89414b48800cae26e5cd77655157e Mon Sep 17 00:00:00 2001 From: Florian Zumpe Date: Sun, 18 Oct 2026 17:48:46 +0200 Subject: [PATCH] fixed sonarqube findings --- docs/README.md | 13 +++++ docs/agents/AGENTS.md | 19 ++++++ public/css/editor.css | 2 +- public/index.html | 2 +- public/js/editor-ui.js | 36 ++++++------ public/js/group-points.js | 2 +- public/js/groups.js | 44 +++++++------- src/services/collaboration.js | 3 +- src/services/point-locks.js | 10 +++- src/services/route-write-access.js | 2 +- test/deployment-workflow.test.js | 94 ++++++++++++++++++++++++++++++ 11 files changed, 181 insertions(+), 46 deletions(-) create mode 100644 test/deployment-workflow.test.js diff --git a/docs/README.md b/docs/README.md index 4956b5d..1055e2b 100644 --- a/docs/README.md +++ b/docs/README.md @@ -226,3 +226,16 @@ Wegwichtel verwendet keine Werbe- oder Tracking-Cookies und bindet keine Analyse Der Initialisierungsbildschirm bleibt nach erfolgreichem Laden sichtbar. Erst eine bewusste Pointer- oder Tastaturaktion öffnet die eigentliche Anwendung. Das Informationsfenster und die Datenschutzeinstellungen können bereits auf dem Startbildschirm verwendet werden, ohne die Anwendung weiterzuschalten. Gemeinsame Entwürfe und Wichtelcodes: [Gruppenbearbeitung.md](Gruppenbearbeitung.md). + +## Deployment-Freigabe + +Automatische Deployments benötigen ein erfolgreiches SonarQube Quality Gate. +Der Analysejob erzeugt danach eine Freigabe für den geprüften Commit, den +Workflow-Lauf und dessen aktuellen Versuch. Ohne passende Freigabe darf das +Deployment nicht fortfahren. Das SSH-Skript übernimmt ausschließlich diesen +Commit und prüft ihn vor dem Neustart des App-Dienstes. So kann ein inzwischen +weiter fortgeschrittener Branch nicht unbemerkt als ungeprüfter Stand starten. + +Der ausdrücklich manuell ausgewählte Workflow-Aufruf `job=deploy` bleibt als +bestehender Override verfügbar. Er benötigt eine bewusste Auswahl und ersetzt +keine neue Analyse. Ein gewöhnlicher Push kann diesen Override nicht verwenden. diff --git a/docs/agents/AGENTS.md b/docs/agents/AGENTS.md index a8dfd7c..e958456 100644 --- a/docs/agents/AGENTS.md +++ b/docs/agents/AGENTS.md @@ -178,3 +178,22 @@ Rechte fremder Urheber und bestehende Lizenzen bleiben davon unberührt. Vor Änderungen an Gruppenrechten und Code-Eingabe zusätzlich `docs/agents/GRUPPENBEARBEITUNG.md` lesen und bei Übergaben ausdrücklich nennen. + +## Deployment-Freigabe nach dem Quality Gate + +Am 18. Oktober 2026 ist Lauf 1828 für Commit 0e4cdfb trotz fehlgeschlagenem +Quality Gate bis zum erfolgreichen Deployment gelaufen. Die vorhandene +jobweite `if`-Bedingung allein hat das Deployment nicht verhindert. + +- Automatische Deployments benötigen ein erst nach erfolgreichem Gate erzeugtes + Freigabeartefakt für genau Commit, Workflow-Lauf und Laufversuch. +- Im Deployment-Schritt muss die Freigabe geprüft werden; im SSH-Skript müssen + eine fehlende Freigabe und ein abweichender Commit vor Änderungen abgewiesen + werden. Nur den freigegebenen Commit übernehmen und vor dem Neustart prüfen. +- Der bestehende ausdrückliche manuelle Aufruf `job=deploy` bleibt ein bewusster + Override. Ein Push darf diese Ausnahme nicht aktivieren. +- Verhaltenstests in `test/deployment-workflow.test.js` ausführen. Sie führen die + tatsächlichen Shell-Blöcke mit isolierten Dateien und simulierten externen + Befehlen aus; keine echten SSH-Verbindungen oder Neustarts im Test. +- Die neue Freigabekette muss nach dem nächsten Check-in im echten Workflow + bestätigt werden. Lokale Shell-Tests ersetzen keine erfolgreiche Pipeline. diff --git a/public/css/editor.css b/public/css/editor.css index 8463ba9..3ef207f 100644 --- a/public/css/editor.css +++ b/public/css/editor.css @@ -615,7 +615,7 @@ .group-code img { width: 100%; height: 64px; object-fit: contain; } .group-actions { max-width: min(18rem, 52vw); flex-wrap: wrap; justify-content: flex-end; } .group-actions .editor-secondary-button { font-size: .85rem; } -.group-status { min-height: 1.5rem; color: #9b2c2c; } +.group-status { display: block; min-height: 1.5rem; color: #9b2c2c; } @media (prefers-reduced-motion: reduce) { .group-wheel-viewport { scroll-behavior: auto; } } .point-compare-dialog { width: min(56rem, calc(100vw - 2rem)); } diff --git a/public/index.html b/public/index.html index 0d2f52a..2e4e8f2 100644 --- a/public/index.html +++ b/public/index.html @@ -607,7 +607,7 @@

-

+

Die Station wurde geändert

diff --git a/public/js/editor-ui.js b/public/js/editor-ui.js index 31324f2..be73344 100644 --- a/public/js/editor-ui.js +++ b/public/js/editor-ui.js @@ -214,7 +214,7 @@ globalThis.Wegwichtel = globalThis.Wegwichtel || {}; } function showListView() { - void ns.GroupPoints?.release(); + ns.GroupPoints?.release(); releasePoiPreview(); ns.RouteRecorder?.endEditing(); setDialogTitle('Streckenverwaltung'); @@ -224,7 +224,7 @@ globalThis.Wegwichtel = globalThis.Wegwichtel || {}; } function showRouteView() { - void ns.GroupPoints?.release(); + ns.GroupPoints?.release(); releasePoiPreview(); setDialogTitle(routeDialogTitle()); setHidden(byId(ids.routeListView), true); @@ -478,8 +478,8 @@ globalThis.Wegwichtel = globalThis.Wegwichtel || {}; renderRouteList(); setStatus(''); // Local drafts are usable even while this request is waiting for a connection. - void refreshPublicRoutes(requestGeneration); - void refreshSubmittedRoutes(requestGeneration); + refreshPublicRoutes(requestGeneration); + refreshSubmittedRoutes(requestGeneration); } async function refreshSubmittedRoutes(requestGeneration) { @@ -536,7 +536,7 @@ globalThis.Wegwichtel = globalThis.Wegwichtel || {}; setDialogExpanded(true); showListView(); await guarded(loadRoutes); - void ns.Groups?.refresh?.(); + ns.Groups?.refresh?.(); } async function openNewLocalRoute() { @@ -546,7 +546,7 @@ globalThis.Wegwichtel = globalThis.Wegwichtel || {}; function closeEditor() { if (pointSaveInProgress) return; - void ns.GroupPoints?.release(); + ns.GroupPoints?.release(); releasePoiPreview(); ns.RouteRecorder?.endEditing(); const dialog = byId(ids.dialog); @@ -948,8 +948,8 @@ globalThis.Wegwichtel = globalThis.Wegwichtel || {}; async function saveRoute() { const values = routeFromForm(); if (!values.name) throw new Error(ns.I18n.t("editor.error.route-name")); - if (state.currentSource !== 'local') await saveServerRoute(values); - else await saveLocalRoute(values); + if (state.currentSource === 'local') await saveLocalRoute(values); + else await saveServerRoute(values); } function safeGpxFilename(name) { @@ -1157,7 +1157,7 @@ globalThis.Wegwichtel = globalThis.Wegwichtel || {}; } function newPoi() { - void ns.GroupPoints?.release(); + ns.GroupPoints?.release(); releasePoiPreview(); const position = currentPosition(); const lat = position?.lat ?? null; @@ -1195,9 +1195,9 @@ globalThis.Wegwichtel = globalThis.Wegwichtel || {}; if (draft) { state.pendingAudioFile = draft.media?.audio || null; state.pendingImages = (draft.media?.images || []).map(file => ({ file, key: `pending-image-${ns.LocalRoutes.createPoiId()}`, previewUrl: objectUrlFor(file) })); - const hiddenImages = state.groupDeletions.map(item => item.path); - state.currentPoi = { ...poi, images: (poi.images || []).filter(image => !hiddenImages.includes(`pictures/${image.id}`)), - ...(hiddenImages.includes('audio') ? { audioUrl: null } : {}) }; + const hiddenImages = new Set(state.groupDeletions.map(item => item.path)); + state.currentPoi = { ...poi, images: (poi.images || []).filter(image => !hiddenImages.has(`pictures/${image.id}`)), + ...(hiddenImages.has('audio') ? { audioUrl: null } : {}) }; renderPoiMediaPreview(); updatePoiMediaStatus(); setStatus(ns.I18n.t(state.groupDeletePoint ? 'point-locks.delete-restored' : 'point-locks.restored')); @@ -1325,7 +1325,7 @@ globalThis.Wegwichtel = globalThis.Wegwichtel || {}; await uploadOne(`${basePath}/audio`, 'audio', state.pendingAudioFile, method); if (state.currentSource === 'group') { state.pendingAudioFile = null; await rememberGroupPoint(poiFromForm(), currentPoiId); } } - for (const pending of [...state.pendingImages]) { + for (const pending of state.pendingImages) { await uploadOne(`${basePath}/pictures`, 'picture', pending.file); if (state.currentSource === 'group') { state.pendingImages = state.pendingImages.filter(item => item !== pending); @@ -1366,7 +1366,7 @@ globalThis.Wegwichtel = globalThis.Wegwichtel || {}; await ns.GroupPoints.remove(currentRouteId, 'new'); await ns.GroupPoints.acceptNew(currentRouteId, saved); } - for (const deletion of [...state.groupDeletions]) { + for (const deletion of state.groupDeletions) { await sendDelete(`/routes/${currentRouteId}/pois/${savedPoiId}/${deletion.path}`); state.groupDeletions = state.groupDeletions.filter(item => item !== deletion); await rememberGroupPoint(values, savedPoiId); @@ -1725,14 +1725,14 @@ globalThis.Wegwichtel = globalThis.Wegwichtel || {}; byId(ids.dialog)?.addEventListener(eventName, event => { if (pointSaveInProgress && eventName === 'cancel') { event.preventDefault(); return; } releasePoiPreview(); - void ns.GroupPoints?.release(); + ns.GroupPoints?.release(); ns.RouteRecorder?.endEditing(); }); } activate(byId(ids.button), () => guarded(openEditor)); activate(byId(ids.close), closeEditor); activate(byId(ids.newRoute), newRoute); - activate(byId(ids.routeBack), () => { showListView(); void guarded(loadRoutes); }); + activate(byId(ids.routeBack), () => { showListView(); guarded(loadRoutes); }); activate(byId(ids.routeUpload), () => guarded(uploadLocalRoute)); activate(byId(ids.rightsOpen), () => ns.LegalUi?.open('terms')); activate(byId(ids.addPoi), newPoi); @@ -1742,8 +1742,8 @@ globalThis.Wegwichtel = globalThis.Wegwichtel || {}; byId(ids.poiImages)?.addEventListener('change', () => guarded(handleImageSelection)); activate(byId(ids.poiMediaPreview), event => guarded(() => mediaPreviewAction(event))); activate(byId(ids.poiDelete), () => { - if (state.currentSource === 'group') { state.groupDeletePoint = true; void guarded(savePoi); } - else void guarded(deleteCurrentPoi); + if (state.currentSource === 'group') { state.groupDeletePoint = true; guarded(savePoi); } + else guarded(deleteCurrentPoi); }); activate(byId(ids.poiCancel), showRouteView); activate(byId(ids.routeList), event => guarded(() => routeListAction(event))); diff --git a/public/js/group-points.js b/public/js/group-points.js index b0b6a35..ac472bd 100644 --- a/public/js/group-points.js +++ b/public/js/group-points.js @@ -178,7 +178,7 @@ globalThis.Wegwichtel ??= {}; } } finally { touching = false; } } - globalThis.setInterval?.(() => { void keepActive(); }, 30_000); + globalThis.setInterval?.(() => { keepActive(); }, 30_000); function headersFor(path) { const match = /^\/routes\/(\d+)\/pois\/(\d+)(?:\/|$)/.exec(path); if (!match || !current || String(current.routeId) !== match[1] || String(current.poiId) !== match[2]) return {}; diff --git a/public/js/groups.js b/public/js/groups.js index 4e3610d..3aceb82 100644 --- a/public/js/groups.js +++ b/public/js/groups.js @@ -22,12 +22,12 @@ globalThis.Wegwichtel ??= {}; button.addEventListener('pointerup', event => { if (event.isPrimary === false || (event.button && event.button !== 0)) return; event.preventDefault(); - void action(); + action(); }); button.addEventListener('keydown', event => { if (!['Enter', ' '].includes(event.key)) return; event.preventDefault(); - void action(); + action(); }); } @@ -147,7 +147,7 @@ globalThis.Wegwichtel ??= {}; function openEntry() { dialogGeneration += 1; - selection = Array(codeLength).fill(0); + selection = new Array(codeLength).fill(0); element(ids.status).textContent = ''; element(ids.entry).hidden = false; element(ids.invitation).hidden = true; @@ -297,34 +297,36 @@ globalThis.Wegwichtel ??= {}; if (routeId && revision !== null && /^\d+$/.test(revision)) revisions.set(routeId, Number(revision)); } + async function joinSelectedGroup() { + const generation = dialogGeneration; + const motifs = ns.GroupSymbols.list(); + let payload; + try { + payload = await request('/join', 'POST', { + code: selection.map(index => motifs[index].unicode), creator_id: ns.LocalRoutes.creatorId() + }); + } catch (error) { + if (!element(ids.dialog).open || generation !== dialogGeneration) return; + throw error; + } + if (!element(ids.dialog).open || generation !== dialogGeneration) return; + await edit(payload.group); + await refresh(); + } + function init() { - globalThis.setInterval?.(() => { void refreshInvitation(); }, 30_000); + globalThis.setInterval?.(() => { refreshInvitation(); }, 30_000); activate(element('group-share-start'), () => finishShare(true)); activate(element('group-share-close'), () => finishShare(false)); element('group-share-dialog').addEventListener('close', () => { const resolve = shareDecision; shareDecision = null; resolve?.(false); }); - document.addEventListener('wegwichtel:point-drafts-changed', () => { void renderPending(); void ns.StorageUsage?.refresh(); }); + document.addEventListener('wegwichtel:point-drafts-changed', () => { renderPending(); ns.StorageUsage?.refresh(); }); element(ids.dialog).addEventListener('cancel', () => { dialogGeneration += 1; }); element(ids.dialog).addEventListener('close', () => { dialogGeneration += 1; }); for (const id of [ids.joinOpen, ids.editorJoin]) activate(element(id), openEntry); activate(element(ids.close), () => element(ids.dialog).close()); - activate(element(ids.join), () => guarded(async () => { - const generation = dialogGeneration; - const motifs = ns.GroupSymbols.list(); - let payload; - try { - payload = await request('/join', 'POST', { - code: selection.map(index => motifs[index].unicode), creator_id: ns.LocalRoutes.creatorId() - }); - } catch (error) { - if (!element(ids.dialog).open || generation !== dialogGeneration) return; - throw error; - } - if (!element(ids.dialog).open || generation !== dialogGeneration) return; - await edit(payload.group); - await refresh(); - })); + activate(element(ids.join), () => guarded(joinSelectedGroup)); activate(element(ids.edit), () => guarded(() => edit(currentGroup))); } diff --git a/src/services/collaboration.js b/src/services/collaboration.js index fc904bb..5ae405c 100644 --- a/src/services/collaboration.js +++ b/src/services/collaboration.js @@ -1,4 +1,5 @@ import crypto from 'node:crypto'; +import { isLoopbackAddress } from './route-write-access.js'; import { guardPoint, clearPointLocks, hasPointLocks } from './point-locks.js'; import fs from 'node:fs'; import path from 'node:path'; @@ -163,7 +164,7 @@ export function listGroups(db, req) { } function failureKey(req) { - const localProxy = ['127.0.0.1', '::1', '::ffff:127.0.0.1'].includes(req.socket?.remoteAddress); + const localProxy = isLoopbackAddress(req.socket?.remoteAddress); return hash(String(localProxy && req.headers['x-real-ip'] || req.socket?.remoteAddress || 'unknown')); } diff --git a/src/services/point-locks.js b/src/services/point-locks.js index 81a4894..8598aa5 100644 --- a/src/services/point-locks.js +++ b/src/services/point-locks.js @@ -25,7 +25,10 @@ function read(routeId, poiId) { } return lock; } - catch (error) { if (error.code === 'ENOENT') return null; throw error; } + catch (error) { + if (error.code === 'ENOENT') return null; + throw error; + } } function write(routeId, poiId, lock) { const target = filename(routeId, poiId); @@ -86,7 +89,10 @@ export function hasPointLocks(routeId) { const directory = path.join(routeDirectory(routeId), 'locks'); let entries; try { entries = fs.readdirSync(directory, { withFileTypes: true }); } - catch (error) { if (error.code === 'ENOENT') return false; throw error; } + catch (error) { + if (error.code === 'ENOENT') return false; + throw error; + } let locked = false; for (const entry of entries) { if (entry.isFile() && /^\d+\.json$/.test(entry.name) && read(routeId, Number(entry.name.slice(0, -5)))) locked = true; diff --git a/src/services/route-write-access.js b/src/services/route-write-access.js index 7906b21..f5bd9f8 100644 --- a/src/services/route-write-access.js +++ b/src/services/route-write-access.js @@ -13,7 +13,7 @@ function hasForwardingHeader(req, name) { return value != null && String(value).trim().length > 0; } -function isLoopbackAddress(address) { +export function isLoopbackAddress(address) { if (typeof address !== 'string' || address.length === 0) return false; if (address === '::1') return true; diff --git a/test/deployment-workflow.test.js b/test/deployment-workflow.test.js new file mode 100644 index 0000000..7c2ca8e --- /dev/null +++ b/test/deployment-workflow.test.js @@ -0,0 +1,94 @@ +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { spawnSync } from 'node:child_process'; +import test from 'node:test'; + +const workflow = fs.readFileSync(new URL('../.gitea/workflows/build-and-deploy.yaml', import.meta.url), 'utf8'); +const commit = 'a'.repeat(40); +function script(name, attribute = 'run') { + const lines = workflow.split('\n'); + const start = lines.findIndex(line => line.trim() === `- name: ${name}`); + assert.notEqual(start, -1); + const offset = lines.findIndex((line, index) => index > start && line.trim() === `${attribute}: |-`); + assert.notEqual(offset, -1); + const indent = lines[offset].search(/\S/) + 2; + const result = []; + for (const line of lines.slice(offset + 1)) { + if (line.trim() && line.search(/\S/) < indent) break; + result.push(line.slice(indent)); + } + return result.join('\n'); +} +function fixture(t) { + const directory = fs.mkdtempSync(path.join(os.tmpdir(), 'wegwichtel-deploy-')); + t.after(() => fs.rmSync(directory, { recursive: true, force: true })); + const output = path.join(directory, 'output'); + const env = { ...process.env, EXPECTED_COMMIT: commit, WORKFLOW_RUN: '1828', WORKFLOW_ATTEMPT: '1', + RUN_EVENT: 'push', SELECTED_JOB: '', GITHUB_OUTPUT: output }; + function run(source, extra = {}) { return spawnSync('sh', ['-s'], { input: source, cwd: directory, env: { ...env, ...extra }, encoding: 'utf8' }); } + function proof(value = `${commit}:1828:1`) { + fs.mkdirSync(path.join(directory, 'deployment-approval'), { recursive: true }); + fs.writeFileSync(path.join(directory, 'deployment-approval', 'quality-gate-passed.txt'), value); + } + return { directory, output, env, run, proof }; +} + +test('a deployment approval is written only after the Quality Gate step succeeded', t => { + const h = fixture(t), source = script('Write deployment approval'); + for (const outcome of ['', 'failure', 'skipped', 'cancelled']) { + assert.notEqual(h.run(source, { GATE_OUTCOME: outcome }).status, 0); + assert.equal(fs.existsSync(path.join(h.directory, 'quality-gate-passed.txt')), false); + } + assert.equal(h.run(source, { GATE_OUTCOME: 'success' }).status, 0); + assert.equal(fs.readFileSync(path.join(h.directory, 'quality-gate-passed.txt'), 'utf8'), `${commit}:1828:1\n`); +}); + +test('push deployments reject missing, mismatching and previous-attempt approval artifacts', t => { + const h = fixture(t), source = script('Verify deployment approval'); + assert.notEqual(h.run(source).status, 0); + for (const proof of ['wrong', `${'b'.repeat(40)}:1828:1`, `${commit}:1827:1`, `${commit}:1828:0`]) { + h.proof(proof); assert.notEqual(h.run(source).status, 0); + assert.equal(fs.existsSync(h.output), false); + } + h.proof(); assert.equal(h.run(source).status, 0); + assert.equal(fs.readFileSync(h.output, 'utf8'), `commit=${commit}\n`); +}); + +test('only an explicit manual deploy can use the existing manual override', t => { + const h = fixture(t), source = script('Verify deployment approval'); + assert.notEqual(h.run(source, { RUN_EVENT: 'push', SELECTED_JOB: 'deploy' }).status, 0); + assert.notEqual(h.run(source, { RUN_EVENT: 'workflow_dispatch', SELECTED_JOB: 'all' }).status, 0); + assert.equal(h.run(source, { RUN_EVENT: 'workflow_dispatch', SELECTED_JOB: 'deploy' }).status, 0); + assert.equal(fs.readFileSync(h.output, 'utf8'), `commit=${commit}\n`); +}); + +test('the remote script refuses missing approval before git access and restarts only the approved commit', t => { + const h = fixture(t), source = script('Deploy Wegwichtel', 'script'); + const log = path.join(h.directory, 'calls'); + const stubs = String.raw` +cd() { return 0; } +git() { + printf 'git %s\n' "$*" >> "$TASK_LOG" + if [ "$1" = merge ] && [ "$MERGE_FAILS" = yes ]; then return 1; fi + if [ "$1" = rev-parse ]; then printf '%s\n' "$ACTUAL_COMMIT"; fi +} +systemctl() { printf 'systemctl %s\n' "$*" >> "$TASK_LOG"; } +`; + const invoke = (approval, actual = commit, mergeFails = 'no') => h.run(stubs + source, { + DEPLOY_APPROVED_COMMIT: approval, ACTUAL_COMMIT: actual, TASK_LOG: log, MERGE_FAILS: mergeFails + }); + assert.notEqual(invoke('').status, 0); assert.equal(fs.existsSync(log), false); + assert.notEqual(invoke('b'.repeat(40)).status, 0); assert.equal(fs.existsSync(log), false); + assert.notEqual(invoke(commit, 'b'.repeat(40)).status, 0); + assert.doesNotMatch(fs.readFileSync(log, 'utf8'), /systemctl/); + fs.rmSync(log); + assert.notEqual(invoke(commit, commit, 'yes').status, 0); + assert.doesNotMatch(fs.readFileSync(log, 'utf8'), /systemctl/); + fs.rmSync(log); + assert.equal(invoke(commit).status, 0); + const calls = fs.readFileSync(log, 'utf8'); + assert.match(calls, new RegExp(`git merge --ff-only ${commit}`)); + assert.match(calls, /systemctl --user restart wegwichtel.service/); +});