From bd7287aa58ac14452ade5a4d455764efa0638f72 Mon Sep 17 00:00:00 2001 From: Do Siki Date: Tue, 18 Aug 2026 12:18:47 +0200 Subject: [PATCH] fix: address code review findings from 2026-08-17 - scope no-cache headers to non-static routes (restore immutable asset caching) - reset cached rejected MongoDB promise so retries can succeed - use last X-Forwarded-For entry in Content Editor rate limiter (anti-spoofing) - remove weak Mongo defaults from compose files (fail loudly on missing env) - move staging banner text to common.json content - read APP_PORT from env file in deploy.sh healthcheck - filter network noise from staging smoke console assertions Closes MITHOME-48, MITHOME-49, MITHOME-50, MITHOME-51, MITHOME-52, MITHOME-53, MITHOME-54 --- TODO.md | 33 ++++++++++++++++++++++--- content-editor.js | 8 ++++-- deploy.sh | 6 ++++- docker-compose.prod.yml | 8 +++--- docker-compose.staging.yml | 7 ++++-- proto/e2e/staging-smoke.spec.ts | 6 ++++- proto/next.config.ts | 12 ++++++++- proto/src/app/layout.tsx | 3 ++- proto/src/content/common.json | 3 +++ proto/src/content/schema.js | 1 + proto/src/content/types.ts | 3 +++ proto/src/lib/mongodb.test.ts | 18 ++++++++++++++ proto/src/lib/mongodb.ts | 15 +++++++++-- scripts/test-content-editor-security.js | 9 +++++++ 14 files changed, 115 insertions(+), 17 deletions(-) diff --git a/TODO.md b/TODO.md index 91e49c3..52b2ab1 100755 --- a/TODO.md +++ b/TODO.md @@ -14,7 +14,7 @@ Next.js 15 alapú weboldal a mozdIT Bt. számára, Docker Compose-szal deployolv | Plane | Feladat | Státusz | |-------|---------|---------| | MITHOME-1 | Repo & Next.js bootstrap — dev szerver | ✅ | -| MITHOME-2 | Kezd��lap (Hero + USP + Webmail CTA) | ✅ | +| MITHOME-2 | Kezdőlap (Hero + USP + Webmail CTA) | ✅ | | MITHOME-3 | Rólunk oldal | ✅ | | MITHOME-4 | Szolgáltatások oldal | ✅ | | MITHOME-5 | Kapcsolat űrlap + API route | ✅ | @@ -27,6 +27,16 @@ Next.js 15 alapú weboldal a mozdIT Bt. számára, Docker Compose-szal deployolv | MITHOME-12 | Docker stack — Next.js + MongoDB + Loki + Grafana | ✅ | | MITHOME-28 | Custom CMS (content-editor.js) UI fejlesztés, Basic Auth & Publikálás gomb | ✅ | | MITHOME-29 | Webmail link (mail.mozdit.hu) és Logo finomhangolás | ✅ | +| MITHOME-30 | Bug: Content Editor sérti a JSON struktúrát és adattípusokat | ✅ | +| MITHOME-41 | Content Editor: runtime content schema-validáció és atomikus mentés | ✅ | +| MITHOME-43 | Content Editor: hozzáférésvédelem, CSRF-védelem és auditnapló | ✅ | +| MITHOME-48 | Cache-Control fejléc a statikus assetekre is kihat (perf regresszió) | ✅ | +| MITHOME-49 | Elutasított MongoDB client promise cache-elődik | ✅ | +| MITHOME-50 | X-Forwarded-For spoofing a Content Editor rate limiterében | ✅ | +| MITHOME-51 | Gyenge és ellentmondásos Mongo defaultok a compose fájlokban | ✅ | +| MITHOME-52 | Staging banner szöveg kerüljön content JSON-ba | ✅ | +| MITHOME-53 | deploy.sh: APP_PORT env nem látszik a shell healthcheckjének | ✅ | +| MITHOME-54 | Staging smoke teszt: console-error ellenőrzés szűrése zajra | ✅ | --- @@ -36,6 +46,16 @@ Next.js 15 alapú weboldal a mozdIT Bt. számára, Docker Compose-szal deployolv |-------|---------|---------|-----------| | MITHOME-13 | CI pipeline — lint + unit tesztek + staging deploy trigger | ⏳ | Elsődleges prioritás | | MITHOME-14 | Playwright E2E smoke tesztek + Lighthouse CI | ⏳ | Elsődleges prioritás | +| MITHOME-32 | Bug: staging és production MongoDB URI alapértelmezése nem hitelesített | ⏳ | | +| MITHOME-33 | Bug: staging és production Loki host-ra hivatkozik Loki service nélkül | ⏳ | | +| MITHOME-34 | Bug: site config minden környezetben development rekordot olvas és ír | ⏳ | | +| MITHOME-35 | Bug: aktív Gitea CI nem futtat integration/E2E tesztet és nem indít staging deployt | ⏳ | | +| MITHOME-36 | Bug: elavult GitHub/Linear workflow továbbra is aktív szinkronizációt végez | ⏳ | | +| MITHOME-42 | Content Editor: ellenőrzött publikálás és natív production release integráció | ⏳ | | +| MITHOME-44 | Környezetenkénti Content Release réteg: promotion és rollback | ⏳ | | +| MITHOME-45 | Content Editor: felhasználónkénti hitelesítés és auditnapló | ⏳ | | +| MITHOME-46 | Központi IDM/SSO réteg több céges alkalmazáshoz | ⏳ | | +| MITHOME-47 | Belső vállalati Portal – mozdIT operatív munkaasztal tervezése | ⏳ | | --- @@ -44,6 +64,7 @@ Next.js 15 alapú weboldal a mozdIT Bt. számára, Docker Compose-szal deployolv | Plane | Feladat | Státusz | Megjegyzés | |-------|---------|---------|-----------| | MITHOME-17 | Winston logger + Loki integráció | 🔄 | Winston + LokiTransport már implementálva `src/lib/logger.ts`-ben | +| MITHOME-31 | Bug: kapcsolatfelvételi API sikeres választ ad elveszett üzenetre | 🔄 | | --- @@ -51,8 +72,8 @@ Next.js 15 alapú weboldal a mozdIT Bt. számára, Docker Compose-szal deployolv | Plane | Feladat | Státusz | |-------|---------|---------| -| MITHOME-15 | Prod app + domain konfiguráció + HTTPS | 📋 | -| MITHOME-16 | Site config migrálás MongoDB-ba | 📋 | +| MITHOME-15 | Production domain, Nginx reverse proxy és HTTPS | 📋 | +| MITHOME-16 | Site config migrálása környezetfüggetlen MongoDB tárolásra | 📋 | | MITHOME-18 | Adatvédelmi tájékoztató oldal (/adatvedelem) — GDPR | 📋 | | MITHOME-19 | Accessibility (A11y) — WCAG 2.1 AA megfelelőség | 📋 | | MITHOME-20 | Lighthouse score ≥ 90 minden kategóriában | 📋 | @@ -62,7 +83,11 @@ Next.js 15 alapú weboldal a mozdIT Bt. számára, Docker Compose-szal deployolv | MITHOME-24 | Performance optimalizálás | 📋 | | MITHOME-25 | Reszponzív design finomítása | 📋 | | MITHOME-26 | Analytics integráció (Plausible / GA4) | 📋 | -| MITHOME-27 | Production környezet beállítása (mozdit.hu — Iron szerver) | 📋 | +| MITHOME-27 | Production környezet beállítása — natív Node.js/systemd (Iron szerver) | 📋 | +| MITHOME-38 | Production: Next.js standalone release, systemd service és rollback | 📋 | +| MITHOME-39 | Production: közös natív MongoDB több alkalmazás biztonságos kiszolgálására | 📋 | +| MITHOME-40 | Production MongoDB: automatizált mentés, visszaállítási próba és monitoring | 📋 | +| MITHOME-55 | Contact API válaszformátum igazítása a projekt konvencióhoz | 📋 | --- diff --git a/content-editor.js b/content-editor.js index 63a9d95..1bd4d7d 100644 --- a/content-editor.js +++ b/content-editor.js @@ -462,10 +462,13 @@ function securityConfigIsValid() { function getClientAddress(req) { // The editor only listens on 127.0.0.1; the staging Nginx proxy supplies this header. - // This prevents all remote visitors sharing the proxy address in the rate limiter. + // WHY: take the LAST entry. Nginx ($proxy_add_x_forwarded_for) appends the real client + // IP to the list, so the first entry may be a spoofed value sent by the client — using + // it would let attackers bypass the rate limiter with a fresh "IP" per request. const forwarded = req.headers['x-forwarded-for']; if (typeof forwarded === 'string' && forwarded.trim()) { - return forwarded.split(',')[0].trim(); + const parts = forwarded.split(',').map(part => part.trim()).filter(Boolean); + if (parts.length > 0) return parts[parts.length - 1]; } return req.socket.remoteAddress || 'unknown'; } @@ -646,6 +649,7 @@ module.exports = { validateContent, hasValidCredentials, hasValidCsrfToken, + getClientAddress, securityConfigIsValid, csrfToken: CSRF_TOKEN, }; diff --git a/deploy.sh b/deploy.sh index 7cf1885..d64a2d8 100755 --- a/deploy.sh +++ b/deploy.sh @@ -37,6 +37,10 @@ if [ ! -f "$ENV_FILE" ]; then fi echo "🔑 Környezeti változók betöltése ($ENV_FILE)..." +# WHY: a --env-file kapcsoló csak a compose változó-helyettesítését táplálja; +# a shell nem látja belőle az APP_PORT-ot, ezért a healthcheckhez expliciten kiolvassuk. +APP_PORT_VALUE="$(grep -E '^APP_PORT=' "$ENV_FILE" | tail -n 1 | cut -d= -f2- | tr -d '[:space:]' | tr -d '"' | tr -d "'")" + # 3. Docker konténerek újraépítése és indítása echo "🐳 Build és indítás..." # A friss Dockerfile- vagy build-arg-változásoknak is új konténerben kell érvényesülniük. @@ -44,7 +48,7 @@ docker compose --env-file "$ENV_FILE" -f "$COMPOSE_FILE" up --build --force-recr # 4. Healthcheck echo "⏳ Healthcheck (max 60s)..." -HEALTH_URL="http://localhost:${APP_PORT:-$DEFAULT_PORT}/api/health" +HEALTH_URL="http://localhost:${APP_PORT_VALUE:-$DEFAULT_PORT}/api/health" for i in $(seq 1 30); do if curl -sf "$HEALTH_URL" > /dev/null 2>&1; then echo "✅ Healthcheck OK: $HEALTH_URL" diff --git a/docker-compose.prod.yml b/docker-compose.prod.yml index 4c846de..d8a1832 100644 --- a/docker-compose.prod.yml +++ b/docker-compose.prod.yml @@ -12,8 +12,9 @@ services: - "8080:3000" # Host port 8080 elkerüli a lokális npm dev (3000) összeakadást environment: - NODE_ENV=production - # MONGODB and LOKI URLs should be updated to point to production services - - MONGODB_URI=${MONGODB_URI:-mongodb://mongodb:27017/mozdit} + # No fallback for MONGODB_URI: with root auth enabled on the Mongo container an + # unauthenticated default URI would silently break the app — fail loudly instead. + - MONGODB_URI=${MONGODB_URI} - MONGODB_DB=${MONGODB_DB:-mozdit} - NEXT_PUBLIC_SITE_URL=${NEXT_PUBLIC_SITE_URL:-https://mozdit.hu} - NEXT_PUBLIC_COMPANY_NAME=${NEXT_PUBLIC_COMPANY_NAME:-mozdIT Bt.} @@ -38,7 +39,8 @@ services: - "27018:27017" # Host port 27018 elkerüli az összeakadást a lokális Mongo-val environment: - MONGO_INITDB_ROOT_USERNAME=${MONGO_ROOT_USER:-admin} - - MONGO_INITDB_ROOT_PASSWORD=${MONGO_ROOT_PASSWORD:-password123} + # No weak default password: a missing value must fail the container loudly. + - MONGO_INITDB_ROOT_PASSWORD=${MONGO_ROOT_PASSWORD} - MONGO_INITDB_DATABASE=mozdit volumes: - mongodb_data_prod:/data/db diff --git a/docker-compose.staging.yml b/docker-compose.staging.yml index baea5e9..77c9f52 100644 --- a/docker-compose.staging.yml +++ b/docker-compose.staging.yml @@ -13,7 +13,9 @@ services: - "127.0.0.1:8081:3000" # Belső port — csak nginx-en keresztül elérhető environment: - NODE_ENV=production - - MONGODB_URI=${MONGODB_URI:-mongodb://mongodb:27017/mozdit} + # No fallback for MONGODB_URI: with root auth enabled on the Mongo container an + # unauthenticated default URI would silently break the app — fail loudly instead. + - MONGODB_URI=${MONGODB_URI} - MONGODB_DB=${MONGODB_DB:-mozdit} - NEXT_PUBLIC_SITE_URL=${NEXT_PUBLIC_SITE_URL:-https://stage.mozdit.hu} - NEXT_PUBLIC_DEPLOY_ENV=staging @@ -39,7 +41,8 @@ services: - "127.0.0.1:27019:27017" # Belső port, nem ütközik a prod 27018-cal environment: - MONGO_INITDB_ROOT_USERNAME=${MONGO_ROOT_USER:-admin} - - MONGO_INITDB_ROOT_PASSWORD=${MONGO_ROOT_PASSWORD:-password123} + # No weak default password: a missing value must fail the container loudly. + - MONGO_INITDB_ROOT_PASSWORD=${MONGO_ROOT_PASSWORD} - MONGO_INITDB_DATABASE=mozdit volumes: - mongodb_data_staging:/data/db diff --git a/proto/e2e/staging-smoke.spec.ts b/proto/e2e/staging-smoke.spec.ts index 4a3268c..7e85d5d 100644 --- a/proto/e2e/staging-smoke.spec.ts +++ b/proto/e2e/staging-smoke.spec.ts @@ -10,7 +10,11 @@ test('SMOKE-01: health endpoint is available', async ({ request }) => { test('SMOKE-02: homepage renders its critical shell without console errors', async ({ page }) => { const consoleErrors: string[] = [] page.on('console', message => { - if (message.type() === 'error') consoleErrors.push(message.text()) + if (message.type() !== 'error') return + // Chromium logs failed resource loads (e.g. favicon 404) as console errors. + // Those are network noise here; real JS errors must still fail the test. + if (message.text().startsWith('Failed to load resource')) return + consoleErrors.push(message.text()) }) await page.goto('/') diff --git a/proto/next.config.ts b/proto/next.config.ts index 01adc8f..c97ef84 100755 --- a/proto/next.config.ts +++ b/proto/next.config.ts @@ -37,7 +37,8 @@ const nextConfig: NextConfig = { async headers() { return [ { - source: '/(.*)', + // Security headers apply to every route, including static assets. + source: '/:path*', headers: [ { key: 'X-Frame-Options', @@ -51,6 +52,15 @@ const nextConfig: NextConfig = { key: 'Referrer-Policy', value: 'origin-when-cross-origin', }, + ], + }, + { + // WHY: no-cache must not hit hashed build assets (_next/static) or + // optimized images (_next/image); they are content-addressed and rely on + // long-lived caching. Overriding them would re-download the bundle on + // every page load. + source: '/((?!_next/static|_next/image).*)', + headers: [ { key: 'Cache-Control', value: 'private, no-cache, must-revalidate, max-age=0', diff --git a/proto/src/app/layout.tsx b/proto/src/app/layout.tsx index c62e553..cf49901 100755 --- a/proto/src/app/layout.tsx +++ b/proto/src/app/layout.tsx @@ -6,6 +6,7 @@ import Header from "../components/Header"; import Footer from "../components/Footer"; import { ThemeProvider } from "../components/ThemeProvider"; import { siteConfig } from "../config/site"; +import { common } from "../content"; const geistSans = Geist({ variable: "--font-geist-sans", @@ -93,7 +94,7 @@ export default function RootLayout({ {isStaging && (
- ⚠ STAGING / TESZTKÖRNYEZET — A STAGING OLDALT LÁTOD + {common.staging.banner}
)}
diff --git a/proto/src/content/common.json b/proto/src/content/common.json index 7b51a63..b5ec535 100644 --- a/proto/src/content/common.json +++ b/proto/src/content/common.json @@ -13,5 +13,8 @@ "required": "Ez a mező kötelező", "invalidEmail": "Érvénytelen email cím formátum", "minLength": "Legalább {min} karakter szükséges" + }, + "staging": { + "banner": "⚠ STAGING / TESZTKÖRNYEZET — A STAGING OLDALT LÁTOD" } } diff --git a/proto/src/content/schema.js b/proto/src/content/schema.js index 470ebf2..4f19d36 100644 --- a/proto/src/content/schema.js +++ b/proto/src/content/schema.js @@ -14,6 +14,7 @@ const schemas = { buttons: object({ contact: string, learnMore: string, webmail: string, sendMessage: string }), labels: object({ required: string, features: string }), validation: object({ required: string, invalidEmail: string, minLength: string }), + staging: object({ banner: string }), }), home: object({ hero: object({ title: string, subtitle: string, description: string, trustBullets: array(string), cta: object({ primary: ctaLink, secondary: ctaLink }) }), diff --git a/proto/src/content/types.ts b/proto/src/content/types.ts index 720806e..c369972 100755 --- a/proto/src/content/types.ts +++ b/proto/src/content/types.ts @@ -190,6 +190,9 @@ export interface CommonContent { invalidEmail: string minLength: string } + staging: { + banner: string + } } export interface LegalPageContent { diff --git a/proto/src/lib/mongodb.test.ts b/proto/src/lib/mongodb.test.ts index 9ad135e..ce945c3 100755 --- a/proto/src/lib/mongodb.test.ts +++ b/proto/src/lib/mongodb.test.ts @@ -139,5 +139,23 @@ describe('MongoDB Connection (Unit Tests)', () => { // Should be the same client instance expect(client1).toBe(client2) }) + + it('should not cache a rejected connection promise and retry on next call', async () => { + // First attempt fails (e.g. transient DB outage at startup) + const failingClient = { + connect: jest.fn().mockRejectedValueOnce(new Error('transient failure')), + db: jest.fn(), + close: jest.fn() + } + MockedMongoClient.mockImplementation(() => failingClient) + + const { getClientPromise } = await import('./mongodb') + + await expect(getClientPromise()).rejects.toThrow('transient failure') + + // Second call must retry instead of replaying the cached rejection + failingClient.connect.mockResolvedValueOnce(failingClient as any) + await expect(getClientPromise()).resolves.toBe(failingClient) + }) }) }) diff --git a/proto/src/lib/mongodb.ts b/proto/src/lib/mongodb.ts index 8eb1c85..beba78b 100755 --- a/proto/src/lib/mongodb.ts +++ b/proto/src/lib/mongodb.ts @@ -24,13 +24,24 @@ export function getClientPromise(): Promise { // @ts-expect-error - Global variable for development hot reload if (!global._mongoClientPromise) { // @ts-expect-error - Global variable for development hot reload - global._mongoClientPromise = createClientPromise() + global._mongoClientPromise = createClientPromise().catch((error: Error) => { + // A rejected promise must not stay cached: a transient failure would + // otherwise break every subsequent request in this process. + // @ts-expect-error - Global variable for development hot reload + global._mongoClientPromise = undefined + throw error + }) } // @ts-expect-error - Global variable for development hot reload return global._mongoClientPromise } - clientPromise ??= createClientPromise() + clientPromise ??= createClientPromise().catch((error: Error) => { + // See the development branch above: drop the cached rejection so the next + // call can retry the connection. + clientPromise = undefined + throw error + }) return clientPromise } diff --git a/scripts/test-content-editor-security.js b/scripts/test-content-editor-security.js index fb2704d..3679c17 100644 --- a/scripts/test-content-editor-security.js +++ b/scripts/test-content-editor-security.js @@ -8,6 +8,7 @@ process.env.CMS_DEPLOY_ENV = 'staging'; const { hasValidCredentials, hasValidCsrfToken, + getClientAddress, securityConfigIsValid, csrfToken, } = require('../content-editor'); @@ -21,4 +22,12 @@ assert.equal(hasValidCsrfToken({ headers: { 'x-csrf-token': csrfToken } }), true assert.equal(hasValidCsrfToken({ headers: { 'x-csrf-token': 'invalid-token' } }), false); assert.equal(hasValidCsrfToken({ headers: {} }), false); +// X-Forwarded-For: the appended (last) entry is the proxy-observed client address; +// a leading spoofed entry must not become the rate-limit key. +assert.equal(getClientAddress({ headers: { 'x-forwarded-for': '1.2.3.4, 5.6.7.8' }, socket: {} }), '5.6.7.8'); +assert.equal(getClientAddress({ headers: { 'x-forwarded-for': 'spoofed, , 9.9.9.9' }, socket: {} }), '9.9.9.9'); +assert.equal(getClientAddress({ headers: { 'x-forwarded-for': '5.6.7.8' }, socket: {} }), '5.6.7.8'); +assert.equal(getClientAddress({ headers: {}, socket: { remoteAddress: '127.0.0.1' } }), '127.0.0.1'); +assert.equal(getClientAddress({ headers: { 'x-forwarded-for': ' ' }, socket: {} }), 'unknown'); + console.log('Content Editor security guard test: OK');