diff --git a/docs/frontend-build.md b/docs/frontend-build.md index 48a940e5..fb0e6fcd 100644 --- a/docs/frontend-build.md +++ b/docs/frontend-build.md @@ -84,5 +84,28 @@ ruleset where `no-undef`, `no-dupe-keys` and `no-duplicate-case` are errors. Rea those kinds are present in `src/routes/AppRoutes.jsx` and `src/App.jsx` today and are not reported by any build. -Restoring the field is tracked in #575, together with fixing the errors it surfaces — enabling it -before those are fixed would simply break the build for everyone. +The field is now present, extending `react-app`. That enables the rules that matter for +correctness — `no-undef`, `no-dupe-keys`, `no-duplicate-case` — and enabling it immediately caught +two more real defects: `Network` and `Award` were used as icon components in +`EnterpriseSecurityCenter.jsx` without being imported. + +Six rule families are explicitly switched **off** in `eslintConfig.rules`: + +| Rule | Why it is off | +| --- | --- | +| `no-unused-vars` | ~40 files carry pre-existing unused imports | +| `react-hooks/exhaustive-deps` | ~8 `useEffect` calls have incomplete dependency arrays | +| `eqeqeq` | `HttpService.js` uses `==` on status codes | +| `jsx-a11y/anchor-is-valid`, `jsx-a11y/alt-text`, `no-script-url` | pre-existing accessibility warnings | + +They are switched off rather than left as warnings because `react-scripts` promotes *every* warning +to a build error when `CI=true`, so leaving them on would fail the build for reasons unrelated to +the change that introduced the config. Each is worth fixing, but as its own PR with its own diff — +turning them on together would bury a real regression in ~200 lines of noise. + +## Route consistency + +`npm run check-routes` validates `src/routes/routeRegistry.js` against the pages on disk: every +referenced component is imported, every imported module exists, no page key or slug is claimed +twice, and every `src/pages/auth/*Page.jsx` is reachable from some URL. It runs from `prebuild` +alongside `check-syntax`. diff --git a/package.json b/package.json index 62169d38..91250b3c 100644 --- a/package.json +++ b/package.json @@ -32,10 +32,11 @@ "test": "vitest run", "test:watch": "vitest", "check-syntax": "node scripts/check-syntax.js", - "prebuild": "npm run check-syntax", + "prebuild": "npm run check-syntax && npm run check-routes", "build": "react-scripts build", "predeploy": "npm run build", - "deploy": "gh-pages -d build" + "deploy": "gh-pages -d build", + "check-routes": "node scripts/check-routes.js" }, "browserslist": { "production": [ @@ -48,5 +49,18 @@ "last 1 firefox version", "last 1 safari version" ] + }, + "eslintConfig": { + "extends": [ + "react-app" + ], + "rules": { + "no-unused-vars": "off", + "react-hooks/exhaustive-deps": "off", + "eqeqeq": "off", + "jsx-a11y/anchor-is-valid": "off", + "jsx-a11y/alt-text": "off", + "no-script-url": "off" + } } } diff --git a/scripts/check-routes.js b/scripts/check-routes.js new file mode 100644 index 00000000..993af114 --- /dev/null +++ b/scripts/check-routes.js @@ -0,0 +1,187 @@ +#!/usr/bin/env node +/** + * Validates src/routes/routeRegistry.js against the pages that actually exist on disk. + * + * Why this exists + * --------------- + * Route information used to live in three hand-maintained places — the `routeMap` object in + * App.jsx, the `switch` in AppRoutes.jsx, and the import list above it — with nothing keeping them + * consistent. The drift that accumulated: + * + * - `KeyVaultSecurityPage` and `MicrosegmentationPage` were rendered but never imported, an + * uncaught ReferenceError that the ErrorBoundary turns into a blank screen; + * - `case "keyvault-security"` and `case "microsegmentation"` each appeared twice, so the second + * was dead code and `/microsegmentation` silently rendered the ZTNA page instead; + * - `routeMap` declared `help` twice and `microsegmentation` twice with *different* targets, and + * in an object literal the last one silently wins; + * - fourteen security consoles existed as page components with no route at all. + * + * Collapsing the three lists into one registry removes the drift by construction. This script + * closes the remaining gap: it fails the build when the registry itself is inconsistent, or when a + * page is added to src/pages/auth/ and never registered. + * + * It works by static analysis rather than by importing the registry, so it needs no React runtime, + * no JSDOM and no test framework, and it runs in well under a second from `prebuild`. + * + * Usage: node scripts/check-routes.js + */ + +'use strict'; + +const fs = require('fs'); +const path = require('path'); + +const projectRoot = path.resolve(__dirname, '..'); +const registryPath = path.join(projectRoot, 'src', 'routes', 'routeRegistry.js'); +const authPagesDir = path.join(projectRoot, 'src', 'pages', 'auth'); + +const failures = []; + +function fail(message) { + failures.push(message); +} + +function readRegistry() { + if (!fs.existsSync(registryPath)) { + console.error(`check-routes: ${path.relative(projectRoot, registryPath)} not found.`); + process.exit(2); + } + return fs.readFileSync(registryPath, 'utf8'); +} + +/** `import Foo from "../pages/auth/Foo";` -> { Foo: '../pages/auth/Foo' } */ +function parseImports(source) { + const imports = {}; + const pattern = /^import\s+([A-Za-z_$][\w$]*)\s+from\s+["']([^"']+)["'];/gm; + let match; + while ((match = pattern.exec(source)) !== null) { + imports[match[1]] = match[2]; + } + return imports; +} + +/** + * Extracts the ROUTES entries. Deliberately regex-based and shallow: the registry is a flat array + * of object literals with a fixed shape, and a real parser would add a dependency for no gain. + */ +function parseRoutes(source) { + const start = source.indexOf('export const ROUTES = ['); + if (start < 0) { + console.error('check-routes: could not find `export const ROUTES = [` in the registry.'); + process.exit(2); + } + const end = source.indexOf('\n];', start); + const body = source.slice(start, end < 0 ? source.length : end); + + const routes = []; + const entryPattern = /\{\s*page:\s*"([^"]+)"\s*,\s*slugs:\s*\[([^\]]*)\]\s*,\s*component:\s*([A-Za-z_$][\w$]*)([^}]*)\}/g; + let match; + while ((match = entryPattern.exec(body)) !== null) { + const slugs = match[2] + .split(',') + .map((slug) => slug.trim().replace(/^["']|["']$/g, '')) + .filter((slug) => slug.length > 0 || slug === ''); + routes.push({ + page: match[1], + slugs: match[2].trim() === '""' ? [''] : slugs, + component: match[3], + rest: match[4], + isParameterised: /\bparam:\s*"/.test(match[4]), + }); + } + return routes; +} + +function main() { + const source = readRegistry(); + const imports = parseImports(source); + const routes = parseRoutes(source); + + if (routes.length === 0) { + console.error('check-routes: parsed zero routes out of the registry. The parsing in this ' + + 'script has drifted from the file and every check below would be vacuous.'); + process.exit(2); + } + + // 1. Every referenced component must be imported. This is the defect that produced a blank + // screen on /keyvault and /microsegmentation. + for (const route of routes) { + if (!imports[route.component]) { + fail(`route "${route.page}" renders ${route.component}, which is never imported`); + } + } + + // 2. Every imported page module must exist on disk. + for (const [name, specifier] of Object.entries(imports)) { + if (!specifier.startsWith('../pages/')) { + continue; + } + const resolved = path.resolve(path.dirname(registryPath), specifier); + const exists = ['.jsx', '.js', '/index.jsx', '/index.js'].some((suffix) => + fs.existsSync(resolved + suffix) + ); + if (!exists) { + fail(`import ${name} points at ${specifier}, which does not exist`); + } + } + + // 3. No duplicate page keys. + const seenPages = new Map(); + for (const route of routes) { + if (seenPages.has(route.page)) { + fail(`page key "${route.page}" is declared more than once`); + } + seenPages.set(route.page, route); + } + + // 4. No duplicate slugs. A parameterised route legitimately shares its prefix with its list page + // (`blog` and `blog/:slug` are different routes), so the two kinds get their own map: a slug + // may appear once as static and once as parameterised, but not twice within either kind. + // Skipping parameterised routes outright, as an earlier version of this script did, let two + // dynamic routes claim the same prefix — `resolvePath` would then always pick whichever came + // first in ROUTES and the other would be permanently unreachable. + const seenSlugs = new Map(); + const seenParameterisedSlugs = new Map(); + for (const route of routes) { + const seen = route.isParameterised ? seenParameterisedSlugs : seenSlugs; + const kind = route.isParameterised ? 'parameterised ' : ''; + for (const slug of route.slugs) { + if (seen.has(slug)) { + fail( + `${kind}slug "${slug || '(root)'}" is claimed by both "${seen.get(slug)}" and ` + + `"${route.page}" - one of them is unreachable` + ); + } + seen.set(slug, route.page); + } + } + + // 5. Every security console under src/pages/auth/ must be reachable. This is what left fourteen + // pages with no URL at all. + const registered = new Set(routes.map((route) => route.component)); + const authPages = fs + .readdirSync(authPagesDir) + .filter((file) => file.endsWith('Page.jsx')) + .map((file) => file.replace('.jsx', '')); + + for (const page of authPages) { + if (!registered.has(page)) { + fail(`${page} exists in src/pages/auth/ but is not registered, so no URL reaches it`); + } + } + + if (failures.length > 0) { + console.error(`\ncheck-routes: ${failures.length} problem(s) found.\n`); + failures.forEach((message) => console.error(` - ${message}`)); + console.error(''); + process.exit(1); + } + + console.log( + `check-routes: ${routes.length} routes, ${seenSlugs.size} slugs, ` + + `${seenParameterisedSlugs.size} parameterised, ` + + `${authPages.length} security consoles - all consistent.` + ); +} + +main(); diff --git a/src/App.jsx b/src/App.jsx index e0fda4db..8efeebce 100644 --- a/src/App.jsx +++ b/src/App.jsx @@ -54,91 +54,28 @@ const getRouteStateFromPath = () => { }; } - const routeMap = { - blog: "blog", - register: "register", - login: "login", - "forgot-password": "forgot-password", - "verify-otp": "verify-otp", - "reset-password": "reset-password", - dashboard: "dashboard", - equipment: "equipment", - "add-equipment": "add-equipment", - "edit-equipment": "edit-equipment", - "schedule-maintenance": "schedule-maintenance", - "request-equipment": "request-equipment", - maintenance: "maintenance", - tasks: "tasks", - "update-task": "update-task", - updatetask: "update-task", - orders: "orders", - orderstatus: "orderstatus", - about: "about", - contact: "contact", - guidelines: "guidelines", - help: "help", - awards: "awards", - terms: "terms", - guides: "guides", - security: "security", - status: "status", - authority: "authority-security", - "authority-security": "authority-security", - mfa: "mfa-security", - "mfa-security": "mfa-security", - sso: "sso-security", - "sso-security": "sso-security", - rbac: "rbac-security", - "rbac-security": "rbac-security", - zerotrust: "zerotrust-security", - "zerotrust-security": "zerotrust-security", - compliance: "compliance-security", - "compliance-security": "compliance-security", - soar: "threat-detection", - "soar-security": "threat-detection", - "threat-detection": "threat-detection", - keyvault: "keyvault-security", - "key-vault": "keyvault-security", - "keyvault-security": "keyvault-security", - certificate: "certificate", - dlp: "dlp-privacy", - "dlp-privacy": "dlp-privacy", - "privacy-guard": "dlp-privacy", - passkeys: "passkeys", - passwordless: "passkeys", - webauthn: "passkeys", - ztna: "ztna", - microsegmentation: "ztna", - "network-access": "ztna", - siem: "siem-analytics", - "siem-analytics": "siem-analytics", - "siem-security": "siem-analytics", - scim: "scim-provisioning", - "scim-provisioning": "scim-provisioning", - "command-center": "security-commandcenter", - "security-commandcenter": "security-commandcenter", - help: "help", - "help-center": "help", - vulnerability: "vulnerability", - "patch-management": "vulnerability", - microsegmentation: "microsegmentation", - sdp: "microsegmentation", - "perimeter-security": "microsegmentation", - grc: "grc-compliance", - "grc-compliance": "grc-compliance", - "audit-ledger": "grc-compliance", - pam: "pam", - "privileged-access": "pam", - "jit-elevation": "pam", - }; +/** + * Strips the GitHub Pages base path and surrounding slashes from the current location, leaving the + * bare route path for the registry to resolve. + */ +const currentRoutePath = () => + window.location.pathname + .replace(new RegExp(`^${BASE_PATH}`, "i"), "") + .replace(/^\/+|\/+$/g, ""); - return { - page: routeMap[path.toLowerCase()] || "landing", - data: null, - }; -}; +/** + * Route resolution is delegated to routeRegistry.js. + * + * This file used to carry its own `routeMap` object, maintained by hand in parallel with the + * `switch` in AppRoutes.jsx. The two drifted, and the map itself contained duplicate keys: `help` + * appeared twice, and `microsegmentation` appeared twice with *different* targets ("ztna" and + * "microsegmentation"). In an object literal the last wins silently, so `/microsegmentation` + * resolved to a page whose switch case was itself unreachable. + */ +const getRouteStateFromPath = () => resolvePath(currentRoutePath()); function AppContent() { + const { user } = useAuth(); const initialRoute = getRouteStateFromPath(); const [currentPage, setCurrentPage] = useState(initialRoute.page); const [pageData, setPageData] = useState(initialRoute.data); @@ -156,20 +93,8 @@ function AppContent() { setCurrentPage(page); setPageData(data); - const basePath = window.location.pathname.includes("/MedTrack_Application") - ? "/MedTrack_Application" - : ""; - - const nextPath = - page === "blog-post" && data - ? `${basePath}/blog/${encodeURIComponent(data)}` - : page === "edit-equipment" && data - ? `${basePath}/edit-equipment/${encodeURIComponent(data)}` - : page === "apply" && data - ? `${basePath}/apply/${encodeURIComponent(data)}` - : `${basePath}/${page}`; - - window.history.pushState({}, "", nextPath); + const basePath = window.location.pathname.includes(BASE_PATH) ? BASE_PATH : ""; + window.history.pushState({}, "", `${basePath}${buildPath(page, data)}`); window.scrollTo({ top: 0, behavior: "smooth" }); }; @@ -184,16 +109,14 @@ function AppContent() { return () => window.removeEventListener("popstate", handlePopState); }, []); - const noLayoutPages = [ - "login", - "register", - "forgot-password", - "verify-otp", - "reset-password", - "apply", - "dashboard" - ]; - const isAuthPage = noLayoutPages.includes(currentPage); + // Whether a page shows the navbar and footer is a property of the route, declared once in the + // registry, rather than a hard-coded list here that had to be kept in step with it. + // + // Evaluated against the *effective* page rather than the requested one. AppRoutes substitutes the + // login screen for an unauthenticated hit on a protected route, so keying chrome off currentPage + // wrapped the full-bleed login layout in the navbar and footer for a signed-out visit to + // /equipment. Both sides now derive the substitution from resolveEffectivePage. + const showChrome = hasChrome(resolveEffectivePage(user, currentPage)); return ( @@ -201,7 +124,6 @@ function AppContent() { className="flex flex-col min-h-screen bg-surface text-primary transition-colors duration-200" style={{ fontFamily: "'Plus Jakarta Sans', sans-serif" }} > - {!isAuthPage && ( @@ -238,7 +160,7 @@ function AppContent() { -{!isAuthPage &&