refactor: remove documentation site and implement new web dashboard w… - #52
taha2samy-3 wants to merge 2 commits into
Conversation
…ith updated SBOM metadata configuration
📝 WalkthroughWalkthroughThe project replaces the MkDocs reporting site with a Node.js dashboard. It adds report aggregation, static serving, browser rendering, charts, compliance views, test inspection, fallback handling, and updated registry ownership. The previous documentation and Python reporting components are removed. ChangesNode.js dashboard migration
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Reports
participant Builder as web/builder.js
participant Data as web/data.json
participant Server as server.js
participant Dashboard
Reports->>Builder: Parse reports and metadata
Builder->>Data: Write normalized dashboard data
Dashboard->>Server: Request dashboard assets
Server->>Dashboard: Return static files
Dashboard->>Data: Fetch dashboard data
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
🟠 Major comments (18)
web/verify.js-17-29 (1)
17-29: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winStrengthen the validation beyond
project_name.
project_namealways has a fallback value (process.env.PROJECT_NAME || 'Wolfi OpenSSL FIPS'inweb/builder.jsLine 375), so it is never falsy. This means the check at Line 21 can never fail, and this verification step always reports success, even when the build produced only default/empty fallback data (no Trivy scans, no Pytest reports, no metadata found). Validate that core sections were actually populated, for examplereports,security.compliance, orpackages.🛠️ Proposed fix
if (!data.project_name) { throw new Error('Missing "project_name" field'); } + + if (!Array.isArray(data.packages) || data.packages.length === 0) { + throw new Error('No packages discovered; build likely ran without versions.hcl data'); + } + if (!data.security || !data.reports) { + throw new Error('Missing "security" or "reports" section in build output'); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/verify.js` around lines 17 - 29, Replace the ineffective project_name-only check in the verification flow with validation that core build data is populated, requiring at least one meaningful section such as reports, security.compliance, or packages. Keep the existing success and error handling in the surrounding try/catch, and reject fallback-only data before logging success.web/assets/js/main.js-254-264 (1)
254-264: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winBoth clipboard handlers assume the Clipboard API exists and neither handles a rejection. The shared root cause is duplicated clipboard logic with no availability guard and no
.catch.navigator.clipboardis undefined on an insecure origin other thanlocalhost, and this dashboard is served over plain HTTP. Accessing the page by host name or LAN IP makes every copy button raise aTypeError.
web/assets/js/main.js#L254-L264: add anavigator.clipboard?.writeTextguard before the call and a.catchthat reports the failure on the button. Extract the result into a shared helper.web/assets/js/tests.js#L284-L298: replace the inlinenavigator.clipboard.writeText(...)call incopyBtn.onclickwith the shared helper.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/assets/js/main.js` around lines 254 - 264, Eliminate duplicated clipboard handling by extracting a shared helper from copyToClipboard that guards navigator.clipboard?.writeText and catches failures, reporting the error on the button. Update web/assets/js/main.js lines 254-264 to implement the guarded helper and update web/assets/js/tests.js lines 284-298 so copyBtn.onclick uses that shared helper instead of calling navigator.clipboard.writeText inline.web/assets/js/tables.js-498-505 (1)
498-505: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winThe same HTML escaper is defined three times and carries the same two defects in each copy. The shared root cause is the absence of a common utility module. Each copy uses
if (!str) return ''followed bystr.replace(...), so each one throws aTypeErroron a numeric value fromdata.jsonand silently converts0andfalseto an empty string.
web/assets/js/tables.js#L498-L505: change the guard to a null and undefined check, then callString(str)before the replacements. This copy has the most call sites, so fix it first and export it from a newweb/assets/js/utils.js.web/assets/js/fallbacks.js#L224-L231: deleteescapeFallbackHTMLand import the sharedescapeHTML.web/assets/js/tests.js#L316-L323: delete the localescapeHTMLand import the shared one.
web/assets/js/main.jshas no escaper at all and needs the same import for the sinks flagged on its lines 128-152 and 216-250.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/assets/js/tables.js` around lines 498 - 505, The duplicated HTML escapers have inconsistent falsy handling and fail on numeric inputs. In web/assets/js/tables.js:498-505, update escapeHTML to return only for null or undefined, convert non-null values with String(str), and export it from new web/assets/js/utils.js; in web/assets/js/fallbacks.js:224-231 and web/assets/js/tests.js:316-323, remove the local escapers and import the shared utility; also import escapeHTML in web/assets/js/main.js for the sinks at lines 128-152 and 216-250.web/assets/js/tables.js-39-40 (1)
39-40: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winOnly the first Trivy target is read. Findings from other targets are dropped.
Line 40 reads
raw_scan?.Results?.[0]?.Vulnerabilities. Trivy emits one entry inResultsper scanned target, for example the OS package database and each language dependency file. The table therefore shows the findings of the first target only.
renderBaseScorecardsinweb/assets/js/main.jsline 84 reads the precomputedvulnerabilities.total. If the builder counts all targets, the headline CVE count will not match the table contents. For a compliance dashboard, a silent undercount in the detail view is a reporting defect.🛠️ Proposed fix
const variantData = data.security?.compliance?.[currentVariant] || {}; - const vulnerabilities = variantData.raw_scan?.Results?.[0]?.Vulnerabilities || []; + const results = Array.isArray(variantData.raw_scan?.Results) ? variantData.raw_scan.Results : []; + const vulnerabilities = results.flatMap(r => Array.isArray(r?.Vulnerabilities) ? r.Vulnerabilities : []);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/assets/js/tables.js` around lines 39 - 40, Update the vulnerability extraction near variantData so it iterates over every entry in raw_scan.Results and combines all available Vulnerabilities arrays into one collection, rather than reading only Results[0]. Preserve the existing empty-array fallback and ensure the resulting collection remains compatible with the table rendering and the precomputed vulnerabilities.total count.web/assets/js/charts.js-224-231 (1)
224-231: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
getOpsthrows when any signature record lacksalgorithmoroperation.
Array.prototype.findruns the predicate on every element until it matches. Lines 227-228 call.toUpperCase()and.toLowerCase()without a guard.sanitizeDashboardDatainweb/assets/js/fallbacks.jsline 63 verifies only thatbench_signatures_rawis an array. It does not verify the shape of the elements. One record with a missing or non-string field raisesTypeError: Cannot read properties of undefined (reading 'toUpperCase').
bootstrapDashboardwraps all renderers in onetry, so this single record replaces the entire dashboard with the parse-failure card.🛡️ Proposed fix
const getOps = (algo, op) => { - const record = rawSigs.find(s => - s.algorithm.toUpperCase() === algo.toUpperCase() && - s.operation.toLowerCase() === op.toLowerCase() - ); - return record ? record.ops_per_sec : 0; + const record = rawSigs.find(s => + typeof s?.algorithm === 'string' && + typeof s?.operation === 'string' && + s.algorithm.toUpperCase() === algo.toUpperCase() && + s.operation.toLowerCase() === op.toLowerCase() + ); + return typeof record?.ops_per_sec === 'number' ? record.ops_per_sec : 0; };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/assets/js/charts.js` around lines 224 - 231, Update getOps so its rawSigs.find predicate validates that each record’s algorithm and operation fields are strings before calling toUpperCase or toLowerCase. Skip malformed records and preserve the existing ops_per_sec return for valid matches, with 0 when no valid record matches.web/assets/js/tests.js-226-230 (1)
226-230: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winThe badge selector stops matching after the first open. The badge then shows stale data.
Line 226 selects the badge with the class
badge-accent. Line 229 overwritesclassNamewith eitherbadge badge-success ...orbadge badge-error .... Neither string containsbadge-accent.On every subsequent call,
document.querySelector('#forensicLogDrawerOverlay.badge-accent')returnsnull. Theifon line 227 is false, so lines 228-229 never run again. The drawer then displays the variant and outcome of the first inspected test for every later test. In a forensic log viewer this misattributes a pass or a fail to the wrong test.Select the element by a stable id.
🛠️ Proposed fix
In
initLogDrawerOverlay, add an id to the badge:- <span class="badge badge-accent text-[10px] py-0 px-1.5 font-bold mb-1">Pytest Standard Logs</span> + <span id="drawerVariantBadge" class="badge badge-accent text-[10px] py-0 px-1.5 font-bold mb-1">Pytest Standard Logs</span>Then here:
- const variantBadge = document.querySelector('`#forensicLogDrawerOverlay` .badge-accent'); + const variantBadge = document.getElementById('drawerVariantBadge');🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/assets/js/tests.js` around lines 226 - 230, Update initLogDrawerOverlay to assign a stable id to the variant badge, then change the badge lookup in the displayed update logic to query that id instead of the mutable badge-accent class. Preserve the existing text and pass/fail class updates so repeated drawer openings refresh the correct badge.web/assets/js/fallbacks.js-128-169 (1)
128-169: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winThe chart fallback destroys the live
<canvas>elements and leaves stale Chart.js instances.
renderFallbackStateassignscontainer.innerHTML, andcontaineris the parent of the canvas.main.jscallsrenderAESChart,renderHashChart, andrenderAsymmetricChartbeforecheckAndRenderFeatureFallbacks, so Chart.js instances already exist and hold references to those canvases. Two consequences follow:
- The module-level
aesChart,hashChart, andasymmetricChartvariables inweb/assets/js/charts.jskeep pointing to detached canvases.updateChartsThemethen callschart.update()on a detached canvas on every theme toggle.document.getElementById('aesCurveChart')returnsnullafterwards. Any later re-render returns early and the chart never comes back.Hide the canvas and append the fallback into a sibling node instead of overwriting the parent.
🛠️ Proposed approach
- const aesCanvas = document.getElementById('aesCurveChart'); - if (aesCanvas) { - const container = aesCanvas.parentElement; - if (container) { - renderFallbackState( - container, + const aesCanvas = document.getElementById('aesCurveChart'); + if (aesCanvas) { + const container = aesCanvas.parentElement; + if (container) { + aesCanvas.classList.add('hidden'); + const slot = document.createElement('div'); + container.appendChild(slot); + renderFallbackState( + slot, 'Performance Benchmark Data Pending', 'Automated OpenSSL speed runs have not been executed on this host. Run the benchmark tool to populate cryptographic throughput curves.', 'chart' ); } }Apply the same pattern to
hashComparisonChartandasymmetricVelocityChart. Alternatively, export adestroyCharts()helper fromcharts.jsand call it before the fallback replaces the containers.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/assets/js/fallbacks.js` around lines 128 - 169, Update the benchmark fallback handling in checkAndRenderFeatureFallbacks so it preserves the live canvases and existing Chart.js instances: hide each canvas and append or render the fallback in a separate sibling node rather than replacing the canvas parent’s innerHTML. Apply this consistently to aesCurveChart, hashComparisonChart, and asymmetricVelocityChart, ensuring the canvas IDs remain available for later chart rendering and theme updates.web/assets/js/main.js-309-349 (1)
309-349: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winOne
tryblock wraps the fetch and every renderer. A single renderer failure blanks the dashboard and misreports the cause.Two distinct problems follow from the current structure:
initThemeEngine()on line 320 runs only after the fetch succeeds. If the fetch fails, or if any renderer throws, the theme engine never initializes. The error card then renders with nodata-themeattribute, and the theme toggle has no click listener. The user cannot switch themes on the error page.- If
renderAESChart,initVulnerabilityInspector, orinitTestsSuiteViewerthrows, thecatchreplaces the whole overview panel and displays "We couldn't retrieve or parse web/data.json file". That message is wrong. The data parsed correctly. A rendering defect is reported as a data-build failure, which sends the user to the wrong place.Initialize the theme first, then isolate each renderer.
🛠️ Proposed restructure
async function bootstrapDashboard() { + initThemeEngine(); + + const safeRender = (name, fn) => { + try { + fn(); + } catch (err) { + console.error(`Dashboard feature "${name}" failed to render:`, err); + } + }; + try { const response = await fetch('data.json'); if (!response.ok) { throw new Error(`HTTP fetch error! Status code: ${response.status}`); } const rawData = await response.json(); appState = sanitizeDashboardData(rawData); window.globalDashboardState = appState; - - initThemeEngine(); - renderBaseScorecards(); - renderSystemBaseSpec(); - renderOpenSSLPackageSpecs(); - - renderAESChart(appState); - renderHashChart(appState); - renderAsymmetricChart(appState); - - initVulnerabilityInspector(appState); - renderHardeningScorecard(appState); - renderKicsSecurityViewer(appState); - - initTestsSuiteViewer(appState); - - renderRegistryArtifacts(); - setupPullTypeSelectors(); - - checkAndRenderFeatureFallbacks(appState); - + updateChartsTheme(appState); + + safeRender('scorecards', renderBaseScorecards); + safeRender('system spec', renderSystemBaseSpec); + safeRender('package spec', renderOpenSSLPackageSpecs); + safeRender('AES chart', () => renderAESChart(appState)); + safeRender('hash chart', () => renderHashChart(appState)); + safeRender('asymmetric chart', () => renderAsymmetricChart(appState)); + safeRender('vulnerabilities', () => initVulnerabilityInspector(appState)); + safeRender('hardening', () => renderHardeningScorecard(appState)); + safeRender('KICS', () => renderKicsSecurityViewer(appState)); + safeRender('tests', () => initTestsSuiteViewer(appState)); + safeRender('registry', renderRegistryArtifacts); + safeRender('pull selectors', setupPullTypeSelectors); + safeRender('fallbacks', () => checkAndRenderFeatureFallbacks(appState)); } catch (err) { console.error('Failed to fetch or parse data.json payload:', err); renderErrorFallback(err); } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/assets/js/main.js` around lines 309 - 349, Restructure bootstrapDashboard so initThemeEngine runs before fetch and remains available on both success and failure paths. Keep fetch/JSON parsing errors handled separately from rendering, then isolate each renderer or rendering group so one failure does not replace the entire dashboard or get reported as a data retrieval/parsing error; preserve the existing renderErrorFallback behavior only for fetch or parse failures.web/assets/js/tests.js-149-201 (1)
149-201: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winThe forensic drawer is a modal without keyboard support.
The overlay has no
role="dialog", noaria-modal, and no Escape-key handler. Focus is not moved into the drawer when it opens, and it is not restored when it closes. The page behind the overlay stays in the tab order while it is visually obscured, so a keyboard user tabs into hidden content.A keyboard user cannot dismiss the drawer except by tabbing blindly to the close button.
♿ Proposed fix
const overlay = document.createElement('div'); overlay.id = 'forensicLogDrawerOverlay'; + overlay.setAttribute('role', 'dialog'); + overlay.setAttribute('aria-modal', 'true'); + overlay.setAttribute('aria-labelledby', 'drawerTestTitle'); overlay.className = 'fixed inset-0 bg-slate-950/60 backdrop-blur-sm z-50 flex justify-end opacity-0 pointer-events-none transition-opacity duration-300'; @@ overlay.addEventListener('click', (e) => { if (e.target === overlay) { closeForensicLogsDrawer(); } }); + + document.addEventListener('keydown', (e) => { + if (e.key === 'Escape' && overlay.classList.contains('opacity-100')) { + closeForensicLogsDrawer(); + } + }); }In
openForensicLogsDrawer, storedocument.activeElement, then move focus to the close button. IncloseForensicLogsDrawer, restore focus to the stored element.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/assets/js/tests.js` around lines 149 - 201, Update initLogDrawerOverlay and the related openForensicLogsDrawer/closeForensicLogsDrawer flow to make the drawer an accessible modal: add dialog semantics with role="dialog" and aria-modal="true", handle Escape to close it, move focus to the close button when opened, store the previously active element, restore that element on close, and prevent keyboard tabbing into obscured page content while the drawer is open.web/assets/js/tests.js-124-131 (1)
124-131: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winA suite with no tests shows "100% Pass Rate" in the success color.
Line 125 sets
passRateto100whentotalis0. Line 131 then renders "100% Pass Rate" usingtext-[var(--color-success)]. A test suite that never ran is presented as fully passing.Line 131 also applies the success color unconditionally. A suite at 40% pass rate is rendered in green.
🛠️ Proposed fix
const total = stats.total || stats.passed + stats.failed || 0; - const passRate = total > 0 ? Math.round((stats.passed / total) * 100) : 100; + const hasRun = total > 0; + const passRate = hasRun ? Math.round((stats.passed / total) * 100) : 0; + const rateLabel = hasRun ? `${passRate}% Pass Rate` : 'Not Executed'; + const rateColor = !hasRun + ? 'text-[var(--text-muted)]' + : passRate === 100 + ? 'text-[var(--color-success)]' + : 'text-[var(--color-warning)]'; @@ - <div class="text-lg font-bold text-[var(--color-success)] mt-1 font-mono">${passRate}% Pass Rate</div> + <div class="text-lg font-bold ${rateColor} mt-1 font-mono">${rateLabel}</div>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/assets/js/tests.js` around lines 124 - 131, Update the pass-rate calculation and status markup in the test statistics rendering so a suite with total 0 is represented as not run rather than 100% passed. Make the status text and color conditional on the computed pass rate, using the success styling only for fully passing suites and an appropriate non-success style for partial or empty results.web/assets/js/charts.js-237-238 (1)
237-238: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winHardcoded Ed25519 numbers are rendered as measured throughput.
When no Ed25519 record exists, lines 237-238 substitute
21500and7400. The chart presents these as measured operations per second. No marker tells the viewer that the values are synthetic.The logarithmic y-axis on line 287 makes the result worse. Chart.js cannot plot
0on a log scale, so the missing RSA and ECDSA bars disappear. The fabricated Ed25519 bars are then the only visible data, and the chart looks like a completed benchmark run.Return
0for missing records and let the pending-state fallback handle the empty case.🛠️ Proposed fix
- const edSign = getOps('Ed25519', 'Sign') || 21500; - const edVerify = getOps('Ed25519', 'Verify') || 7400; + const edSign = getOps('Ed25519', 'Sign'); + const edVerify = getOps('Ed25519', 'Verify');🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/assets/js/charts.js` around lines 237 - 238, Update the Ed25519 fallback assignments near getOps('Ed25519', 'Sign') and getOps('Ed25519', 'Verify') to return 0 when records are missing instead of hardcoded throughput values, allowing the existing pending-state handling to represent an incomplete benchmark.web/assets/js/tests.js-242-247 (1)
242-247: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
l.levelnameis written toinnerHTMLwithout escaping.Line 245 escapes
l.msgbut interpolatesl.levelnamedirectly inside[${l.levelname}]. Both values come from the same pytest report record indata.json. The escaping is inconsistent for two fields of the same object.Line 244 also compares
l.levelnameagainst'ERROR'and'WARNING'only. Any other level, includingCRITICAL, is styled as success green.🔒 Proposed fix
logsHtml += emittedLogs.map(l => { - const levelClass = l.levelname === 'ERROR' || l.levelname === 'WARNING' ? 'text-amber-500 font-bold' : 'text-emerald-500'; - return `<div class="mb-2"><span class="text-[var(--text-muted)] font-bold">[${l.levelname}]</span> <span class="${levelClass}">${escapeHTML(l.msg)}</span></div>`; + const level = String(l?.levelname ?? 'INFO'); + const levelClass = ['ERROR', 'CRITICAL', 'FATAL'].includes(level) + ? 'text-rose-500 font-bold' + : level === 'WARNING' + ? 'text-amber-500 font-bold' + : 'text-emerald-500'; + return `<div class="mb-2"><span class="text-[var(--text-muted)] font-bold">[${escapeHTML(level)}]</span> <span class="${levelClass}">${escapeHTML(l?.msg)}</span></div>`; }).join('');🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/assets/js/tests.js` around lines 242 - 247, Update the emittedLogs rendering map to escape l.levelname before interpolating it into innerHTML, and classify all non-success levels—including CRITICAL—as error/warning styling instead of defaulting them to green. Preserve the existing escaped l.msg rendering and markup structure.Source: Linters/SAST tools
web/assets/js/main.js-351-364 (1)
351-364: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winThe error card writes an unescaped error string into
innerHTML.Line 361 interpolates
err.stack || err.message. Whenresponse.json()rejects, the engine embeds a fragment of the offending document in the message text. A malformed or crafteddata.jsontherefore places attacker-influenced characters into the DOM as markup.Escape the value. Better, use
textContenton the<pre>element, because no markup is intended there.🔒 Proposed fix
function renderErrorFallback(err) { const container = document.getElementById('tab-panel-overview'); if (container) { container.innerHTML = ` <div class="p-8 bg-red-500/10 border border-red-500/30 rounded-xl max-w-2xl mx-auto text-center space-y-3"> <svg class="w-12 h-12 text-red-500 mx-auto" fill="none" stroke="currentColor" viewBox="0 0 24 24"> <path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M12 9v2m0 4h.01m-6.938 4h13.856c1.54 0 2.502-1.667 1.732-3L13.732 4c-.77-1.333-2.694-1.333-3.464 0L3.34 16c-.77 1.333.192 3 1.732 3z"/> </svg> <h3 class="text-base font-bold text-[var(--text-primary)]">System Aggregator Parse Failure</h3> <p class="text-xs text-[var(--text-secondary)]">We couldn't retrieve or parse web/data.json file. Make sure the Node data build process completed successfully.</p> - <pre class="bg-black/40 text-left p-3 rounded-lg text-[10px] text-red-400 font-mono overflow-x-auto">${err.stack || err.message}</pre> + <pre class="bg-black/40 text-left p-3 rounded-lg text-[10px] text-red-400 font-mono overflow-x-auto" id="error-detail"></pre> </div> `; + const detail = container.querySelector('`#error-detail`'); + if (detail) detail.textContent = err.stack || err.message || String(err); } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/assets/js/main.js` around lines 351 - 364, Update renderErrorFallback so the error detail from err.stack or err.message is not interpolated into innerHTML. Render the static error card markup separately, then locate its pre element and assign the error text via textContent, preserving the existing fallback value without allowing it to be interpreted as HTML.Source: Linters/SAST tools
web/assets/js/tables.js-329-365 (1)
329-365: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winNumeric report fields are interpolated into
innerHTMLwithout escaping.Lines 337, 341, 345, and 361-365 interpolate
kics.files_scanned,kics.lines_scanned,totalCount, and theseverity_countersentries directly. The code assumes these are numbers, but JSON places no such constraint andsanitizeDashboardDatadoes not coerce them. A string value in any of these fields is injected as markup.Line 281 has the same pattern with
${find.line}.Coerce to a number at the read site.
🛠️ Proposed fix
+ const num = (v) => (Number.isFinite(Number(v)) ? Number(v) : 0); const sev = kics.severity_counters || { CRITICAL: 0, HIGH: 0, MEDIUM: 0, LOW: 0, INFO: 0 }; - const totalCount = kics.total_counter !== undefined ? kics.total_counter : 0; + const totalCount = num(kics.total_counter); @@ - <div class="text-xl font-bold text-[var(--text-primary)] mt-1 font-mono">${kics.files_scanned || 0}</div> + <div class="text-xl font-bold text-[var(--text-primary)] mt-1 font-mono">${num(kics.files_scanned)}</div> @@ - <div class="text-xl font-bold text-[var(--text-primary)] mt-1 font-mono">${kics.lines_scanned || 0}</div> + <div class="text-xl font-bold text-[var(--text-primary)] mt-1 font-mono">${num(kics.lines_scanned)}</div> @@ - <span class="badge badge-error py-1 px-3 font-mono text-xs">CRITICAL: <strong>${sev.CRITICAL || 0}</strong></span> - <span class="badge badge-error py-1 px-3 font-mono text-xs">HIGH: <strong>${sev.HIGH || 0}</strong></span> - <span class="badge badge-warning py-1 px-3 font-mono text-xs">MEDIUM: <strong>${sev.MEDIUM || 0}</strong></span> - <span class="badge badge-indigo py-1 px-3 font-mono text-xs">LOW: <strong>${sev.LOW || 0}</strong></span> - <span class="badge badge-accent py-1 px-3 font-mono text-xs">INFO: <strong>${sev.INFO || 0}</strong></span> + <span class="badge badge-error py-1 px-3 font-mono text-xs">CRITICAL: <strong>${num(sev.CRITICAL)}</strong></span> + <span class="badge badge-error py-1 px-3 font-mono text-xs">HIGH: <strong>${num(sev.HIGH)}</strong></span> + <span class="badge badge-warning py-1 px-3 font-mono text-xs">MEDIUM: <strong>${num(sev.MEDIUM)}</strong></span> + <span class="badge badge-indigo py-1 px-3 font-mono text-xs">LOW: <strong>${num(sev.LOW)}</strong></span> + <span class="badge badge-accent py-1 px-3 font-mono text-xs">INFO: <strong>${num(sev.INFO)}</strong></span>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/assets/js/tables.js` around lines 329 - 365, Coerce all interpolated numeric report values to numbers at their read sites before constructing the HTML: `kics.files_scanned`, `kics.lines_scanned`, `totalCount`, each `sev` severity entry, and `find.line` in the surrounding rendering logic. Update the related comparisons and interpolations to use these numeric values while preserving the existing defaults and display behavior.Source: Linters/SAST tools
web/assets/js/charts.js-37-39 (1)
37-39: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winGuard Chart.js against unavailable global usage
Chartis not imported;web/index.htmlrelies on the unversioned CDN script beforeassets/js/main.js. If that script fails or its module fails,new Chart(...)raisesReferenceError: Chart is not definedand the sharedbootstrapDashboard()tryreplaces the whole dashboard with parse failure.Use a pinned source for the Chart.js dependency, or add an availability guard before creating chart instances so rendering does not make the dashboard unavailable.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/assets/js/charts.js` around lines 37 - 39, Update the chart creation flow around aesChart and new Chart in bootstrapDashboard to avoid referencing an unavailable Chart global. Prefer a pinned Chart.js dependency in the page, or guard chart initialization by checking Chart availability and skip chart rendering while allowing the shared dashboard bootstrap to continue.web/assets/js/tests.js-50-60 (1)
50-60: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
renderSuitethrows when a test record lacksnodeid, andtoFixedthrows on a non-numeric duration.Line 52 calls
test.nodeid.split('::')with no guard.sanitizeDashboardDatainweb/assets/js/fallbacks.jsguarantees only thatreports[variant].testsis an array. It does not validate the elements. A record withoutnodeidraisesTypeError: Cannot read properties of undefined (reading 'split').Line 84 calls
(test.setup?.duration || 0).toFixed(3). Ifdurationis a string such as"0.42", the||guard passes it through and.toFixedraisesTypeError.
bootstrapDashboardinweb/assets/js/main.jswraps every renderer in onetry, so one malformed record replaces the whole dashboard with the parse-failure card.Line 53 also uses
.replace('test_', ''), which removes the first occurrence anywhere in the string rather than the prefix. Anchor the pattern.🛡️ Proposed fix
tests.forEach((test, idx) => { const isPassed = test.outcome === 'passed'; - const cleanTitle = test.nodeid.split('::').pop() - .replace('test_', '') + const nodeId = typeof test.nodeid === 'string' ? test.nodeid : 'unknown::unknown'; + const cleanTitle = nodeId.split('::').pop() + .replace(/^test_/, '') .replace(/_/g, ' ') .replace(/\b\w/g, c => c.toUpperCase()); @@ - const duration = test.call?.duration ? test.call.duration.toFixed(3) : '0.000'; + const toSeconds = (v) => (Number.isFinite(Number(v)) ? Number(v).toFixed(3) : '0.000'); + const duration = toSeconds(test.call?.duration);Apply
toSeconds(test.setup?.duration)on line 84 and useescapeHTML(nodeId)on line 77.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/assets/js/tests.js` around lines 50 - 60, Update renderSuite to safely handle malformed test records: default a missing test.nodeid before splitting, anchor the test_ removal to the prefix, and escape the resulting node ID with escapeHTML. Normalize setup and call durations through toSeconds before formatting with toFixed, preserving the existing zero fallback for invalid or absent values.web/assets/js/tables.js-64-69 (1)
64-69: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winThis code and
fallbacks.jstoggle different elements for the same empty state. The table can stay hidden permanently.Lines 66 and 69 toggle
tableBody.parentElement. The parent of a<tbody>is the<table>element.checkAndRenderFeatureFallbacksinweb/assets/js/fallbacks.jsline 100 instead hidestableBody.closest('.overflow-x-auto'), which is the outer wrapper.The failure sequence is:
- No scan data exists.
fallbacks.jsaddshiddento the.overflow-x-autowrapper.- The user selects a different variant that does have data.
render()removeshiddenfrom the<table>only. The wrapper keepshidden, so nothing appears.Use the same element in both modules. Line 30 also reads
emptyStatewithout a null guard, while line 35 guardstableBody. Add the matching guard.🛠️ Proposed fix
const tableBody = document.getElementById('vulnerability-table-rows'); const emptyState = document.getElementById('vulnerability-empty-state'); @@ - if (!tableBody) return; + if (!tableBody || !emptyState) return; + const tableWrapper = tableBody.closest('.overflow-x-auto') || tableBody.parentElement; @@ if (filtered.length === 0) { emptyState.classList.remove('hidden'); - tableBody.parentElement.classList.add('hidden'); + tableWrapper.classList.add('hidden'); } else { emptyState.classList.add('hidden'); - tableBody.parentElement.classList.remove('hidden'); + tableWrapper.classList.remove('hidden');🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/assets/js/tables.js` around lines 64 - 69, Update the empty-state visibility logic in render to toggle the table’s closest .overflow-x-auto wrapper, matching checkAndRenderFeatureFallbacks, instead of tableBody.parentElement. Also guard emptyState before accessing classList, consistent with the existing tableBody guard, while preserving the current empty/non-empty visibility behavior.web/assets/js/tables.js-384-443 (1)
384-443: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winThree different values represent the KICS finding count and they can disagree.
- Line 330 uses
kics.total_counterfor the "Total Issues" metric card.- Line 372 gates the entire findings section on
totalCount === 0.- Line 443 displays
findings.lengthas "Total Findings".- Lines 385-436 build the table rows from
kics.queries, not fromfindings.If
total_counteris0butqueriesis non-empty, the dashboard shows "Zero Security Misconfigurations Found" and hides real findings. Ifqueriesis absent whiletotal_counteris greater than zero, the table header renders with an empty body.Derive one count from one source, and gate the empty state on the same source that supplies the rows.
🛠️ Proposed change
- if (totalCount === 0) { + const queries = Array.isArray(kics.queries) ? kics.queries : []; + const rowCount = queries.reduce((n, q) => n + (Array.isArray(q.files) ? q.files.length : 0), 0); + if (rowCount === 0) { @@ - <span class="badge badge-accent py-0.5 px-2.5 font-mono text-[10px]">Total Findings: <strong>${findings.length}</strong></span> + <span class="badge badge-accent py-0.5 px-2.5 font-mono text-[10px]">Total Findings: <strong>${rowCount}</strong></span>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/assets/js/tables.js` around lines 384 - 443, Unify the findings count and empty-state logic around the normalized collection used to build table rows from kics.queries and their files. Update totalCount, the “Total Issues” metric, and the “Total Findings” badge to use that same collection length instead of kics.total_counter or findings.length, and gate the empty state on this count.
🟡 Minor comments (6)
web/builder.js-417-424 (1)
417-424: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReplace hardcoded
hardware_contextvalues with real measurements.
releaseis a literal placeholder string'Kernel Release', andcpu_cores/ram_gbare fixed constants instead of measured values. This dashboard reports these fields as the "Runner Infrastructure Environment" for a compliance audit, so a fabricated value undermines the accuracy of that report. Node'sosmodule provides these values natively.🛠️ Proposed fix
import fs from 'fs'; import path from 'path'; +import os from 'os'; import { fileURLToPath } from 'url';hardware_context: { system: process.platform === 'win32' ? 'Windows' : process.platform === 'darwin' ? 'macOS' : 'Linux', - release: 'Kernel Release', + release: os.release(), architecture: process.arch, - cpu_cores: 4, - ram_gb: 16, + cpu_cores: os.cpus().length, + ram_gb: Math.round(os.totalmem() / (1024 ** 3)), runner: process.env.CI ? 'GitHub Actions CI' : 'Local Runner' },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/builder.js` around lines 417 - 424, Update the hardware_context object to report actual runner measurements: use Node’s os module for the kernel release, CPU core count, and total memory converted to gigabytes, replacing the placeholder release and fixed cpu_cores/ram_gb constants while preserving the existing system, architecture, and runner fields.web/data.json-1-10 (1)
1-10: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRemove the generated
web/data.jsonfrom the repository.
web/builder.jswritesweb/data.json,tools/Taskfile.doc.ymlcompiles reports into it, andweb/verify.jsreads it. Addweb/data.jsonto.gitignoreand remove the committed snapshot so the web payload is produced by the pipeline instead of being tracked in source control.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/data.json` around lines 1 - 10, Remove the committed generated snapshot web/data.json, and add web/data.json to .gitignore so the builder.js and Taskfile.doc.yml pipeline output remains untracked while verify.js can continue reading the generated file.web/assets/js/fallbacks.js-117-126 (1)
117-126: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winThis fallback competes with the empty state already rendered by
renderKicsSecurityViewer.
renderKicsSecurityViewerinweb/assets/js/tables.js(lines 258-265) already writes "Zero IaC Infrastructure Security Violations Emitted by KICS." intokicsStaticAnalysisAccordionwhenfindings.length === 0. This block overwrites that message with "KICS Static Audit Pending". The two messages state different things. A clean scan is not a pending scan.Choose one owner for the empty state of
kicsStaticAnalysisAccordion. Use the presence of scan metadata, for exampledata.security.kics.total_counter !== undefined, to distinguish "clean" from "not run".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/assets/js/fallbacks.js` around lines 117 - 126, Remove the competing empty-state rendering from the KICS fallback block in fallbacks.js and make one owner responsible for kicsStaticAnalysisAccordion. Update the logic around renderKicsSecurityViewer and the KICS fallback to use data.security.kics.total_counter !== undefined (or the established scan-metadata equivalent) to distinguish a completed clean scan from a scan that has not run, preserving the clean-scan message and showing “KICS Static Audit Pending” only when metadata is absent.web/assets/js/fallbacks.js-47-70 (1)
47-70: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winPlaceholder values can present invented data as measured facts.
The fallback fills
openssl_version: '3.1.2',cpu_cores: 4,ram_gb: 16, andarchitecture: 'x86_64'.renderSystemBaseSpecinweb/assets/js/main.jsrenders these values without any "unknown" marker. A viewer cannot distinguish a real measurement from a default. This is a compliance-reporting dashboard, so invented hardware and version data is misleading.Use neutral sentinels such as
'N/A'(as already done forprovenance.digest), and let the render layer show a pending state.🛠️ Proposed change
- safeData.benchmarks.metadata = safeData.benchmarks.metadata || { fips: { openssl_version: '3.1.2' } }; + safeData.benchmarks.metadata = safeData.benchmarks.metadata || { fips: { openssl_version: 'N/A' } }; @@ safeData.hardware_context = safeData.hardware_context || { - system: 'Linux', - architecture: 'x86_64', - cpu_cores: 4, - ram_gb: 16, - runner: 'Local Runner' + system: 'N/A', + architecture: 'N/A', + cpu_cores: null, + ram_gb: null, + runner: 'N/A' };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/assets/js/fallbacks.js` around lines 47 - 70, Replace the fabricated fallback values in the benchmark metadata and hardware_context defaults—openssl_version, architecture, cpu_cores, and ram_gb—with neutral unknown sentinels such as the existing 'N/A' convention. Keep genuine runtime measurements unchanged and ensure renderSystemBaseSpec receives these sentinels so the UI can display its pending/unknown state.web/assets/js/main.js-16-26 (1)
16-26: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winThe initial theme is persisted even when the user never chose it.
setThemewrites tolocalStorageon line 48. The call on line 26 therefore stores the system-derived value on the first visit. After that,savedThemeis always set, so the dashboard stops followingprefers-color-scheme.Persist the theme only when the user clicks the toggle.
🛠️ Proposed change
- setTheme(currentTheme); + setTheme(currentTheme, false); if (themeToggle) { themeToggle.addEventListener('click', () => { const targetTheme = document.documentElement.getAttribute('data-theme') === 'dark' ? 'light' : 'dark'; - setTheme(targetTheme); + setTheme(targetTheme, true); }); }- function setTheme(theme) { + function setTheme(theme, persist = true) { document.documentElement.setAttribute('data-theme', theme); - localStorage.setItem('theme', theme); + if (persist) localStorage.setItem('theme', theme);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/assets/js/main.js` around lines 16 - 26, Update the initial theme setup around currentTheme and setTheme so it applies the saved, system-derived, or default theme without persisting it on first load. Ensure localStorage is written only by the user-triggered theme toggle handler, while preserving saved-theme precedence and system preference detection.web/assets/js/charts.js-130-142 (1)
130-142: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winThe index fallback can plot a different payload size under the "16KB" label.
Line 137 falls back from
arr[5]toarr[arr.length - 1]. If a metrics array holds fewer than six samples, the code plots the last available bucket. The chart labels on line 153 still read "SHA-256 (16KB)", "SHA-512 (16KB)", and "SHA3-256 (16KB)". The rendered value then does not match the stated payload size.The
||chain also treats a measured0as a missing sample. Use an explicit length check and a nullish check.🛠️ Proposed fix
const extractMB = (algo) => { - const arr = metrics[algo]?.fips || [0,0,0,0,0,0]; - const uArr = metrics[algo]?.ubuntu || [0,0,0,0,0,0]; - const dArr = metrics[algo]?.debian || [0,0,0,0,0,0]; - const aArr = metrics[algo]?.alpine || [0,0,0,0,0,0]; - return { - fips: Number(((arr[5] || arr[arr.length - 1] || 0) / 1024).toFixed(2)), - ubuntu: Number(((uArr[5] || uArr[uArr.length - 1] || 0) / 1024).toFixed(2)), - debian: Number(((dArr[5] || dArr[dArr.length - 1] || 0) / 1024).toFixed(2)), - alpine: Number(((aArr[5] || aArr[aArr.length - 1] || 0) / 1024).toFixed(2)) - }; + const at16KB = (arr) => { + if (!Array.isArray(arr) || arr.length < 6) return 0; + const v = arr[5]; + return typeof v === 'number' ? Number((v / 1024).toFixed(2)) : 0; + }; + return { + fips: at16KB(metrics[algo]?.fips), + ubuntu: at16KB(metrics[algo]?.ubuntu), + debian: at16KB(metrics[algo]?.debian), + alpine: at16KB(metrics[algo]?.alpine) + }; };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/assets/js/charts.js` around lines 130 - 142, Update extractMB so each algorithm reads only sample index 5 for the 16KB chart value, returning zero when that index is unavailable; do not fall back to the last array element. Use explicit length validation and nullish handling so a measured zero remains zero across fips, ubuntu, debian, and alpine.
🧹 Nitpick comments (5)
web/index.html (1)
7-10: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAvoid the Tailwind Play CDN in production; add integrity checks for CDN scripts.
Tailwind's Play CDN is documented as "designed for development purposes only, and is not intended for production." It also compiles utility CSS at runtime in the browser, adding load latency and risking flash-of-unstyled-content on a live dashboard. Additionally, neither this script nor the Chart.js CDN script has an
integrity/crossoriginattribute, so a compromised CDN could inject arbitrary code into a security compliance dashboard with no verification.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/index.html` around lines 7 - 10, Replace the Tailwind Play CDN script in the document with a production-built Tailwind stylesheet, and add verified integrity and crossorigin attributes to the Chart.js CDN script. Ensure the integrity hashes match the exact pinned CDN assets and preserve the dashboard’s existing styling and chart loading behavior.web/assets/js/fallbacks.js (1)
224-231: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
escapeFallbackHTMLduplicates the same escaper defined in two other modules.The identical function exists as
escapeHTMLinweb/assets/js/tables.js(lines 498-505) andweb/assets/js/tests.js(lines 316-323). Three copies will drift. Move one implementation into a shared module, for exampleweb/assets/js/utils.js, and import it in all three files.The guard
if (!str) return ''also converts0andfalseinto an empty string. ReturnString(str)for non-nullish input instead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/assets/js/fallbacks.js` around lines 224 - 231, Consolidate the duplicated HTML escaping logic from escapeFallbackHTML and the escapeHTML functions in tables.js and tests.js into a shared utility, then import and reuse that utility in all three modules. Preserve empty output only for nullish input; coerce other values such as 0 and false with String(str) before escaping.web/assets/js/tables.js (1)
8-26: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
renderCodeOrEmptyStateis duplicated intests.jsand the two copies have already diverged.
web/assets/js/tests.jslines 6-24 define a function with the same name and nearly the same body. Two differences exist:
- This copy treats the literal string
'N/A'as empty (line 9). Thetests.jscopy does not.- The default
customClassesvalue differs.The duplication is already producing inconsistent behavior for the same input. Move one implementation into a shared module and pass the differing default class string as an argument.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/assets/js/tables.js` around lines 8 - 26, Move renderCodeOrEmptyState into a shared JavaScript module and remove the duplicated implementation from both tables.js and tests.js. Preserve the shared empty-state behavior, including treating trimmed "N/A" as empty, and pass each caller’s differing default customClasses value explicitly so rendering remains consistent without duplicated logic.web/assets/js/charts.js (1)
304-330: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
updateChartsThemeignores itsdataparameter and accesses the options tree without guards.Line 304 declares
data, but the body never reads it.main.jsline 64 passesappState. Remove the parameter, or use it.Lines 308-312 also assume
chart.options.plugins.legend.labelsandchart.options.plugins.tooltipalways exist, while lines 314 and 319 do guard the scales. Make the access consistent.Related: if the chart fallback in
web/assets/js/fallbacks.jsreplaced a canvas parent, these chart objects still exist andchart.update()on line 324 runs against a detached canvas. Fixing the fallback container handling resolves that path.♻️ Proposed refactor
-export function updateChartsTheme(data) { +export function updateChartsTheme() { const colors = getThemeColors(); const updateOpts = (chart) => { - if (!chart) return; - chart.options.plugins.legend.labels.color = colors.textColor; - chart.options.plugins.tooltip.backgroundColor = colors.tooltipBg; - chart.options.plugins.tooltip.titleColor = colors.tooltipColor; - chart.options.plugins.tooltip.bodyColor = colors.tooltipColor; - chart.options.plugins.tooltip.borderColor = colors.tooltipBorder; + if (!chart?.options) return; + const plugins = chart.options.plugins; + if (plugins?.legend?.labels) plugins.legend.labels.color = colors.textColor; + if (plugins?.tooltip) { + plugins.tooltip.backgroundColor = colors.tooltipBg; + plugins.tooltip.titleColor = colors.tooltipColor; + plugins.tooltip.bodyColor = colors.tooltipColor; + plugins.tooltip.borderColor = colors.tooltipBorder; + }Update the call site in
web/assets/js/main.jsline 64 toupdateChartsTheme();.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/assets/js/charts.js` around lines 304 - 330, UpdateChartsTheme should no longer accept the unused data parameter, and update its caller in main.js to invoke it without arguments. In updateChartsTheme, guard the plugins, legend, labels, and tooltip objects before assigning theme colors, matching the existing scale guards; preserve chart.update() only for valid, attached chart instances and ensure fallback container replacement does not leave these charts targeting detached canvases.web/assets/js/tests.js (1)
86-86: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winThe inline
onclickinterpolatesactiveTestSuiteVariantinto a JavaScript string literal.Line 108 assigns
activeTestSuiteVariantfrom adata-suiteattribute. The value is authored inweb/index.htmltoday, so this is not currently exploitable. The pattern is still fragile: adata-suitevalue containing an apostrophe breaks the handler, and any future change that sources the variant fromdata.jsonturns this into an injection point.
web/assets/js/main.jsline 229 has the same pattern with data-derived values, where it is exploitable. Fix both with the same approach: bind withaddEventListenerand read the value fromdataset.🛠️ Proposed change
- <button class="inspect-btn ..." onclick="openForensicLogsDrawer('${activeTestSuiteVariant}', ${idx})"> + <button class="inspect-btn ..." data-suite="${escapeHTML(activeTestSuiteVariant)}" data-index="${idx}">Then after
container.appendChild(card):+ card.querySelector('.inspect-btn')?.addEventListener('click', (e) => { + const el = e.currentTarget; + window.openForensicLogsDrawer(el.dataset.suite, Number(el.dataset.index)); + });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/assets/js/tests.js` at line 86, Replace the inline onclick on the inspect button generated in tests.js with an addEventListener binding after container.appendChild(card), storing the required index and suite variant in dataset properties and reading them from the clicked element when calling openForensicLogsDrawer. Apply the same dataset-based event-binding approach to the corresponding data-derived handler in main.js, removing its inline interpolation and preserving the existing action.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 9570a497-fdd4-4a93-a2aa-d2b41bd7a172
⛔ Files ignored due to path filters (1)
Pipfile.lockis excluded by!**/*.lock
📒 Files selected for processing (49)
.gitignorePipfilebenchmark/parser.pybenchmark/run_benchmark.shdocker-bake.hcldocs/OPERATIONS.mddocs/architecture/index.mddocs/architecture/supply-chain.mddocs/index.mddocs/performance/benchmarks.mddocs/validation/compliance/development/docker-cis.mddocs/validation/compliance/development/k8s-nsa.mddocs/validation/compliance/development/k8s-pss.mddocs/validation/compliance/development/vulnerability.mddocs/validation/compliance/distroless/docker-cis.mddocs/validation/compliance/distroless/k8s-nsa.mddocs/validation/compliance/distroless/k8s-pss.mddocs/validation/compliance/distroless/vulnerability.mddocs/validation/compliance/standard/docker-cis.mddocs/validation/compliance/standard/k8s-nsa.mddocs/validation/compliance/standard/k8s-pss.mddocs/validation/compliance/standard/vulnerability.mddocs/validation/functional-tests/distroless-image.mddocs/validation/functional-tests/standard-image.mddocs/validation/index.mddocs/validation/static-analysis/kics-report.mdinfra/allure2/Dockerfileinfra/allure3/Dockerfilemain.pymetadata.jsonmkdocs.ymlpackage.jsonscripts/update_readme.pyscripts/upload_sbom.pyscripts/wolfi-pkg-updater.pyserver.jstaskfile.ymltasks/Taskfile.doc.ymlweb/assets/js/charts.jsweb/assets/js/fallbacks.jsweb/assets/js/main.jsweb/assets/js/tables.jsweb/assets/js/tests.jsweb/builder.jsweb/data.jsonweb/index.htmlweb/package.jsonweb/styles.cssweb/verify.js
💤 Files with no reviewable changes (25)
- infra/allure2/Dockerfile
- docs/validation/compliance/distroless/k8s-pss.md
- docs/validation/compliance/development/k8s-nsa.md
- docs/performance/benchmarks.md
- docs/validation/functional-tests/distroless-image.md
- docs/validation/compliance/distroless/k8s-nsa.md
- docs/validation/compliance/standard/k8s-pss.md
- docs/validation/compliance/standard/vulnerability.md
- docs/validation/static-analysis/kics-report.md
- docs/architecture/supply-chain.md
- docs/validation/compliance/distroless/vulnerability.md
- docs/validation/compliance/standard/k8s-nsa.md
- docs/validation/compliance/standard/docker-cis.md
- docs/validation/compliance/development/docker-cis.md
- docs/validation/compliance/development/k8s-pss.md
- mkdocs.yml
- docs/validation/functional-tests/standard-image.md
- docs/OPERATIONS.md
- docs/architecture/index.md
- docs/validation/index.md
- main.py
- infra/allure3/Dockerfile
- docs/validation/compliance/distroless/docker-cis.md
- docs/index.md
- docs/validation/compliance/development/vulnerability.md
| const filePath = path.join(WEB_DIR, pathname); | ||
|
|
||
| // Security check: Prevent directory traversal | ||
| if (!filePath.startsWith(WEB_DIR)) { | ||
| res.statusCode = 403; | ||
| res.setHeader('Content-Type', 'text/plain'); | ||
| res.end('Forbidden'); | ||
| return; | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
Fix the directory-traversal check: startsWith alone allows sibling-directory escape.
filePath.startsWith(WEB_DIR) is a bare string-prefix check. If a sibling entry next to web/ shares the same prefix (for example web-backup, web.bak, or any name starting with web), a request such as /../web-backup/secret.txt normalizes via path.join to a path outside WEB_DIR that still satisfies startsWith(WEB_DIR), because the string "…/web-backup" starts with "…/web". This bypasses the traversal guard. Require a path separator (or trailing sentinel) after WEB_DIR in the comparison.
🛠️ Proposed fix
const filePath = path.join(WEB_DIR, pathname);
// Security check: Prevent directory traversal
- if (!filePath.startsWith(WEB_DIR)) {
+ if (filePath !== WEB_DIR && !filePath.startsWith(WEB_DIR + path.sep)) {
res.statusCode = 403;
res.setHeader('Content-Type', 'text/plain');
res.end('Forbidden');
return;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const filePath = path.join(WEB_DIR, pathname); | |
| // Security check: Prevent directory traversal | |
| if (!filePath.startsWith(WEB_DIR)) { | |
| res.statusCode = 403; | |
| res.setHeader('Content-Type', 'text/plain'); | |
| res.end('Forbidden'); | |
| return; | |
| } | |
| const filePath = path.join(WEB_DIR, pathname); | |
| // Security check: Prevent directory traversal | |
| if (filePath !== WEB_DIR && !filePath.startsWith(WEB_DIR + path.sep)) { | |
| res.statusCode = 403; | |
| res.setHeader('Content-Type', 'text/plain'); | |
| res.end('Forbidden'); | |
| return; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@server.js` around lines 33 - 41, Update the directory-traversal validation
around filePath so it accepts only WEB_DIR itself or paths whose next character
is the platform path separator, rather than relying on a bare WEB_DIR.startsWith
prefix check. Preserve the existing 403 response for paths outside that
boundary.
| serve: | ||
| desc: "Run MkDocs server locally for live preview" | ||
| desc: "Run local Node.js preview server for the web dashboard" | ||
| cmds: | ||
| - pipenv run mkdocs serve --dev-addr 0.0.0.0:8000 | ||
|
|
||
|
|
||
| build-MkDocs-site: | ||
| desc: "Build the final FIPS Boundary Security Dashboard" | ||
| summary: | | ||
| This task generates the static site (MkDocs). | ||
| It assumes all JSON reports (Trivy, KICS, Benchmarks) | ||
| are already gathered in the 'reports/' folder. | ||
| cmds: | ||
| - echo "Building MkDocs Security Dashboard..." | ||
| - pipenv run mkdocs build --clean | ||
| - echo "Done! The site is ready in the 'site/' directory." | ||
| - cd web && node cli.js --serve |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd -i cli.js
fd -i server.jsRepository: taha2samy/openssl_fips
Length of output: 170
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== tracked files matching cli/server/task =="
git ls-files | rg -n '(^|/)(cli\.js|server\.js|Taskfile\..*yml)$|web/' || true
echo
echo "== Taskfile.doc.yml =="
if [ -f tasks/Taskfile.doc.yml ]; then
cat -n tasks/Taskfile.doc.yml
else
echo "missing tasks/Taskfile.doc.yml"
fi
echo
echo "== server.js first 220 lines =="
if [ -f server.js ]; then
sed -n '1,220p' server.js | cat -n
else
echo "missing server.js"
fi
echo
echo "== web/cli.js presence check =="
if [ -f web/cli.js ]; then
sed -n '1,160p' web/cli.js | cat -n
else
echo "web/cli.js not present"
fi
echo
echo "== package.json scripts =="
if [ -f package.json ]; then
node -e "const p=require('./package.json'); console.log(JSON.stringify({scripts:p.scripts, dependencies:p.dependencies, devDependencies:p.devDependencies}, null, 2))"
fiRepository: taha2samy/openssl_fips
Length of output: 3603
Fix the serve task to launch the tracked server.
tasks/Taskfile.doc.yml:9-12 runs cd web && node cli.js --serve, but web/cli.js is not present and the repository package script is start: node server.js. server.js services web/ from the repository root, so this task should launch server.js without changing directories.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tasks/Taskfile.doc.yml` around lines 9 - 12, Update the serve task in
Taskfile.doc.yml to invoke the tracked server.js from the repository root,
matching the package start script; remove the web directory change and preserve
the task’s existing description.
| container.innerHTML = specs.map(item => ` | ||
| <div class="flex items-center justify-between py-2 text-xs border-b border-[var(--border-color)] last:border-b-0"> | ||
| <span class="text-[var(--text-secondary)] font-medium">${item.label}</span> | ||
| <span class="font-mono text-[var(--text-primary)] select-all">${item.val}</span> | ||
| </div> | ||
| `).join(''); | ||
| } | ||
|
|
||
| // Side-by-side OpenSSL core packages | ||
| function renderOpenSSLPackageSpecs() { | ||
| const container = document.getElementById('sidebar-package-spec'); | ||
| if (!container) return; | ||
|
|
||
| const packages = appState.packages || []; | ||
| if (packages.length === 0) { | ||
| container.innerHTML = `<span class="italic text-[var(--text-muted)] text-xs">No core packages identified.</span>`; | ||
| return; | ||
| } | ||
|
|
||
| container.innerHTML = packages.map(pkg => ` | ||
| <div class="flex items-center justify-between py-2 text-xs border-b border-[var(--border-color)] last:border-b-0"> | ||
| <span class="text-[var(--text-primary)] font-semibold">${pkg.name}</span> | ||
| <span class="font-mono text-[var(--text-secondary)] bg-[var(--bg-tertiary)] border border-[var(--border-color)] px-1.5 py-0.5 rounded">${pkg.version}</span> | ||
| </div> | ||
| `).join(''); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
Unescaped report data reaches innerHTML in both sidebar renderers.
Lines 130-131 interpolate item.val, which carries hardware.kernel from data.json. Lines 149-150 interpolate pkg.name and pkg.version from data.json. Neither value is escaped, and both originate from generated scan and host telemetry rather than from literals in this file.
Every other renderer in this change escapes dynamic values. For example web/assets/js/tables.js line 95 uses escapeHTML. Apply the same treatment here.
🔒 Proposed fix
container.innerHTML = specs.map(item => `
<div class="flex items-center justify-between py-2 text-xs border-b border-[var(--border-color)] last:border-b-0">
- <span class="text-[var(--text-secondary)] font-medium">${item.label}</span>
- <span class="font-mono text-[var(--text-primary)] select-all">${item.val}</span>
+ <span class="text-[var(--text-secondary)] font-medium">${escapeHTML(item.label)}</span>
+ <span class="font-mono text-[var(--text-primary)] select-all">${escapeHTML(item.val)}</span>
</div>
`).join(''); container.innerHTML = packages.map(pkg => `
<div class="flex items-center justify-between py-2 text-xs border-b border-[var(--border-color)] last:border-b-0">
- <span class="text-[var(--text-primary)] font-semibold">${pkg.name}</span>
- <span class="font-mono text-[var(--text-secondary)] bg-[var(--bg-tertiary)] border border-[var(--border-color)] px-1.5 py-0.5 rounded">${pkg.version}</span>
+ <span class="text-[var(--text-primary)] font-semibold">${escapeHTML(pkg.name)}</span>
+ <span class="font-mono text-[var(--text-secondary)] bg-[var(--bg-tertiary)] border border-[var(--border-color)] px-1.5 py-0.5 rounded">${escapeHTML(pkg.version)}</span>
</div>
`).join('');main.js has no escapeHTML yet. Import it from the shared utility module proposed for web/assets/js/fallbacks.js, web/assets/js/tables.js, and web/assets/js/tests.js.
🧰 Tools
🪛 ast-grep (0.45.0)
[warning] 142-142: Avoid assigning untrusted data to innerHTML/outerHTML or document.write
Context: container.innerHTML = <span class="italic text-[var(--text-muted)] text-xs">No core packages identified.</span>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting').
(inner-outer-html)
[warning] 146-151: Avoid assigning untrusted data to innerHTML/outerHTML or document.write
Context: container.innerHTML = packages.map(pkg => <div class="flex items-center justify-between py-2 text-xs border-b border-[var(--border-color)] last:border-b-0"> <span class="text-[var(--text-primary)] font-semibold">${pkg.name}</span> <span class="font-mono text-[var(--text-secondary)] bg-[var(--bg-tertiary)] border border-[var(--border-color)] px-1.5 py-0.5 rounded">${pkg.version}</span> </div>).join('')
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting').
(inner-outer-html)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/assets/js/main.js` around lines 128 - 152, Escape the dynamic report
values interpolated by renderSpecs and renderOpenSSLPackageSpecs before
assigning their templates to innerHTML. Import and reuse the shared escapeHTML
utility for item.val, pkg.name, and pkg.version, preserving the existing
rendering and empty-state behavior.
Source: Linters/SAST tools
| const card = document.createElement('div'); | ||
| card.className = 'border border-[var(--border-color)] p-5 rounded-xl bg-[var(--bg-secondary)] space-y-4'; | ||
| card.innerHTML = ` | ||
| <div class="flex items-center justify-between flex-wrap gap-2"> | ||
| <h4 class="text-xs font-semibold text-[var(--text-primary)] flex items-center gap-2"> | ||
| ${item.name} | ||
| <span class="badge ${item.badge} text-[9px] py-0 px-1.5 font-bold">${item.badgeText}</span> | ||
| </h4> | ||
| <span class="text-[10px] text-[var(--text-muted)] font-mono">${displayLabel}</span> | ||
| </div> | ||
|
|
||
| <div class="flex items-center justify-between gap-3 bg-[var(--bg-primary)] p-2.5 rounded-lg border border-[var(--border-color)] font-mono text-[10.5px]"> | ||
| <span class="text-[var(--text-secondary)] select-all truncate">${copyText}</span> | ||
| <button class="copy-btn py-1 px-2 text-[9.5px]" onclick="copyToClipboard('${copyText}', this)">Copy</button> | ||
| </div> | ||
|
|
||
| <div class="flex flex-wrap gap-3 pt-1"> | ||
| <button class="flex items-center gap-1.5 py-1 px-2.5 rounded bg-[var(--bg-primary)] border border-[var(--border-color)] text-[10.5px] text-[var(--text-secondary)] font-semibold font-mono hover:bg-[var(--bg-tertiary)] transition-colors cursor-pointer" onclick="copyToClipboard('${item.digest}', this)"> | ||
| <svg class="w-3.5 h-3.5" fill="none" stroke="currentColor" viewBox="0 0 24 24"><path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M8 5H6a2 2 0 00-2 2v12a2 2 0 002 2h10a2 2 0 002-2v-1M8 5a2 2 0 002 2h2a2 2 0 002-2M8 5a2 2 0 012-2h2a2 2 0 012 2m0 0h2a2 2 0 012 2v3m2 4H10m0 0l3-3m-3 3l3 3"/></svg> | ||
| Digest: ${item.digest.substring(0, 12)}... | ||
| </button> | ||
|
|
||
| <a href="${item.provenanceUrl}" target="_blank" class="flex items-center gap-1.5 py-1 px-2.5 rounded bg-indigo-600/10 border border-indigo-500/30 text-[10.5px] text-indigo-400 font-semibold hover:bg-indigo-600/20 transition-colors"> | ||
| <svg class="w-3.5 h-3.5" fill="none" stroke="currentColor" viewBox="0 0 24 24"><path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M9 12l2 2 4-4m5.618-4.016A11.955 11.955 0 0112 2.944a11.955 11.955 0 01-8.618 3.04A12.02 12.02 0 003 9c0 5.591 3.824 10.29 9 11.622 5.176-1.332 9-6.03 9-11.622 0-1.042-.133-2.052-.382-3.016z"/></svg> | ||
| SLSA L3 Provenance | ||
| </a> | ||
|
|
||
| <a href="${item.sbomUrl}" target="_blank" class="flex items-center gap-1.5 py-1 px-2.5 rounded bg-emerald-600/10 border border-emerald-500/30 text-[10.5px] text-emerald-400 font-semibold hover:bg-emerald-600/20 transition-colors"> | ||
| <svg class="w-3.5 h-3.5" fill="none" stroke="currentColor" viewBox="0 0 24 24"><path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M4 16v1a3 3 0 003 3h10a3 3 0 003-3v-1m-4-4l-4 4m0 0l-4-4m4 4V4"/></svg> | ||
| CycloneDX SBOM | ||
| </a> | ||
| </div> | ||
| `; | ||
| container.appendChild(card); | ||
| }); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
Inline onclick handlers interpolate report data into a JavaScript string. This allows script injection.
Line 229 builds onclick="copyToClipboard('${copyText}', this)". copyText contains registry, owner, repoName, coreVersion, and item.digest, all read from data.json on lines 160-163 and 171-193. A single apostrophe in any of those values terminates the JavaScript string literal. A crafted value such as x'),alert(document.cookie),(0,' executes in the page origin. Line 233 has the same defect with item.digest.
Escaping is not enough here because the value crosses an HTML attribute boundary and then a JavaScript string boundary. Bind the handler with addEventListener and pass the value through dataset.
Two related points in the same block:
- Lines 238 and 243 place
item.provenanceUrlanditem.sbomUrldirectly intohref. Ajavascript:URL fromdata.jsonexecutes on click. Validate the scheme and accept onlyhttp:andhttps:. - Both anchors use
target="_blank"withoutrel="noopener noreferrer". Add the attribute.
🔒 Proposed fix
+ const safeUrl = (u) => {
+ try {
+ const parsed = new URL(u, window.location.href);
+ return ['http:', 'https:'].includes(parsed.protocol) ? parsed.href : '#';
+ } catch {
+ return '#';
+ }
+ };
+
const card = document.createElement('div');
card.className = 'border border-[var(--border-color)] p-5 rounded-xl bg-[var(--bg-secondary)] space-y-4';
card.innerHTML = `
@@
- <span class="text-[var(--text-secondary)] select-all truncate">${copyText}</span>
- <button class="copy-btn py-1 px-2 text-[9.5px]" onclick="copyToClipboard('${copyText}', this)">Copy</button>
+ <span class="text-[var(--text-secondary)] select-all truncate">${escapeHTML(copyText)}</span>
+ <button class="copy-btn py-1 px-2 text-[9.5px]" data-copy="${escapeHTML(copyText)}">Copy</button>
@@
- <button class="... cursor-pointer" onclick="copyToClipboard('${item.digest}', this)">
+ <button class="... cursor-pointer" data-copy="${escapeHTML(item.digest)}">
@@
- <a href="${item.provenanceUrl}" target="_blank" class="...">
+ <a href="${escapeHTML(safeUrl(item.provenanceUrl))}" target="_blank" rel="noopener noreferrer" class="...">
@@
- <a href="${item.sbomUrl}" target="_blank" class="...">
+ <a href="${escapeHTML(safeUrl(item.sbomUrl))}" target="_blank" rel="noopener noreferrer" class="...">
`;
+ card.querySelectorAll('[data-copy]').forEach(btn => {
+ btn.addEventListener('click', () => window.copyToClipboard(btn.dataset.copy, btn));
+ });
container.appendChild(card);🧰 Tools
🪛 ast-grep (0.45.0)
[warning] 217-247: Avoid assigning untrusted data to innerHTML/outerHTML or document.write
Context: card.innerHTML = `
${item.name}
${item.badgeText}
${displayLabel}
<div class="flex items-center justify-between gap-3 bg-[var(--bg-primary)] p-2.5 rounded-lg border border-[var(--border-color)] font-mono text-[10.5px]">
<span class="text-[var(--text-secondary)] select-all truncate">${copyText}</span>
<button class="copy-btn py-1 px-2 text-[9.5px]" onclick="copyToClipboard('${copyText}', this)">Copy</button>
</div>
<div class="flex flex-wrap gap-3 pt-1">
<button class="flex items-center gap-1.5 py-1 px-2.5 rounded bg-[var(--bg-primary)] border border-[var(--border-color)] text-[10.5px] text-[var(--text-secondary)] font-semibold font-mono hover:bg-[var(--bg-tertiary)] transition-colors cursor-pointer" onclick="copyToClipboard('${item.digest}', this)">
<svg class="w-3.5 h-3.5" fill="none" stroke="currentColor" viewBox="0 0 24 24"><path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M8 5H6a2 2 0 00-2 2v12a2 2 0 002 2h10a2 2 0 002-2v-1M8 5a2 2 0 002 2h2a2 2 0 002-2M8 5a2 2 0 012-2h2a2 2 0 012 2m0 0h2a2 2 0 012 2v3m2 4H10m0 0l3-3m-3 3l3 3"/></svg>
Digest: ${item.digest.substring(0, 12)}...
</button>
<a href="${item.provenanceUrl}" target="_blank" class="flex items-center gap-1.5 py-1 px-2.5 rounded bg-indigo-600/10 border border-indigo-500/30 text-[10.5px] text-indigo-400 font-semibold hover:bg-indigo-600/20 transition-colors">
<svg class="w-3.5 h-3.5" fill="none" stroke="currentColor" viewBox="0 0 24 24"><path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M9 12l2 2 4-4m5.618-4.016A11.955 11.955 0 0112 2.944a11.955 11.955 0 01-8.618 3.04A12.02 12.02 0 003 9c0 5.591 3.824 10.29 9 11.622 5.176-1.332 9-6.03 9-11.622 0-1.042-.133-2.052-.382-3.016z"/></svg>
SLSA L3 Provenance
</a>
<a href="${item.sbomUrl}" target="_blank" class="flex items-center gap-1.5 py-1 px-2.5 rounded bg-emerald-600/10 border border-emerald-500/30 text-[10.5px] text-emerald-400 font-semibold hover:bg-emerald-600/20 transition-colors">
<svg class="w-3.5 h-3.5" fill="none" stroke="currentColor" viewBox="0 0 24 24"><path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M4 16v1a3 3 0 003 3h10a3 3 0 003-3v-1m-4-4l-4 4m0 0l-4-4m4 4V4"/></svg>
CycloneDX SBOM
</a>
</div>
`
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting').
(inner-outer-html)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/assets/js/main.js` around lines 216 - 250, In the card-rendering loop,
remove the inline onclick handlers for the copy controls and bind them with
addEventListener using dataset values for copyText and item.digest, avoiding
interpolation into JavaScript or HTML attributes. Validate item.provenanceUrl
and item.sbomUrl so only http: and https: URLs are assigned to href, and add
rel="noopener noreferrer" to both target="_blank" anchors.
Source: Linters/SAST tools
| function parseSummaryControls(obj, defaultSuccess = 0, defaultFail = 0) { | ||
| if (!obj) return { SuccessCount: defaultSuccess, FailCount: defaultFail }; | ||
| if (typeof obj.SuccessCount === 'number' && typeof obj.FailCount === 'number') { | ||
| return obj; | ||
| } | ||
| const controls = obj.SummaryControls || []; | ||
| let FailCount = 0; | ||
| let SuccessCount = 0; | ||
| controls.forEach(ctrl => { | ||
| if (ctrl.TotalFail > 0) { | ||
| FailCount++; | ||
| } else { | ||
| SuccessCount++; | ||
| } | ||
| }); | ||
| if (controls.length === 0) { | ||
| return { SuccessCount: obj.SuccessCount !== undefined ? obj.SuccessCount : defaultSuccess, FailCount: obj.FailCount !== undefined ? obj.FailCount : defaultFail }; | ||
| } | ||
| return { SuccessCount, FailCount }; | ||
| } | ||
|
|
||
| const cis = parseSummaryControls(variantData.docker_cis, 12, 0); | ||
| const nsa = parseSummaryControls(variantData.k8s_nsa, 19, 0); | ||
| const pss = parseSummaryControls(variantData.k8s_pss_restricted, 16, 0); | ||
|
|
||
| const rules = [ | ||
| { name: 'Docker CIS Compliance Standard', passed: cis.SuccessCount, failed: cis.FailCount, desc: 'Verifies root-less isolation, strict signal handling, and secure mounts' }, | ||
| { name: 'NSA / CISA Kubernetes Hardening Standards', passed: nsa.SuccessCount, failed: nsa.FailCount, desc: 'Requires read-only file systems, non-privilege escalation constraints, and default-deny policies' }, | ||
| { name: 'Kubernetes Restricted Pod Security Standard (PSS)', passed: pss.SuccessCount, failed: pss.FailCount, desc: 'Requires drop-all capabilities, non-root user enforcement, and cryptographic validation boundary mapping' } | ||
| ]; | ||
|
|
||
| rules.forEach((rule, idx) => { | ||
| const total = rule.passed + rule.failed; | ||
| const rate = total > 0 ? Math.round((rule.passed / total) * 100) : 100; | ||
| const isCompliant = rule.failed === 0; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | 🏗️ Heavy lift
Missing telemetry is rendered as a positive measured result across four modules. One design decision drives all four sites: when data is absent, the code substitutes a plausible-looking value instead of an explicit "not measured" state. On a FIPS and CIS compliance dashboard, this converts an absent scan into an apparent pass.
web/assets/js/tables.js#L186-L220: stop defaultingparseSummaryControlsto12/0,19/0, and16/0, and stop derivingrate = 100andisCompliant = truefrom a0/0control count. Carry ahasDataflag and render a "Not Scanned" badge.web/assets/js/fallbacks.js#L47-L70: replace the inventedopenssl_version: '3.1.2',cpu_cores: 4,ram_gb: 16, andarchitecture: 'x86_64'defaults with'N/A'ornullsentinels.web/assets/js/charts.js#L237-L238: remove the|| 21500and|| 7400Ed25519 fallbacks so a missing benchmark reads as absent rather than as a measurement.web/assets/js/tests.js#L124-L131: stop settingpassRateto100whentotalis0, and stop applying the success color unconditionally.
Adopt one convention for the whole dashboard: absent data renders a neutral pending state, and only measured data renders a pass or fail verdict.
📍 Affects 4 files
web/assets/js/tables.js#L186-L220(this comment)web/assets/js/fallbacks.js#L47-L70web/assets/js/charts.js#L237-L238web/assets/js/tests.js#L124-L131
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/assets/js/tables.js` around lines 186 - 220, Adopt a neutral “Not
Scanned” state for absent dashboard data across all sites: in
web/assets/js/tables.js lines 186-220, remove the 12/0, 19/0, and 16/0 defaults
in parseSummaryControls, track hasData, and avoid treating 0/0 as a 100%
compliant result; render a “Not Scanned” badge instead. In
web/assets/js/fallbacks.js lines 47-70, replace invented openssl_version,
cpu_cores, ram_gb, and architecture defaults with 'N/A' or null. In
web/assets/js/charts.js lines 237-238, remove the 21500 and 7400 Ed25519
fallbacks. In web/assets/js/tests.js lines 124-131, leave passRate unset or
pending when total is zero and apply success styling only to measured results.
| <div>${escapeHTML(queryName)}</div> | ||
| ${queryUrl !== '#' ? `<a href="${escapeHTML(queryUrl)}" target="_blank" class="text-[10px] text-indigo-400 hover:underline flex items-center gap-1">Docs <svg class="w-3 h-3" fill="none" stroke="currentColor" viewBox="0 0 24 24"><path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M10 6H6a2 2 0 00-2 2v10a2 2 0 002 2h10a2 2 0 002-2v-4M14 4h6m0 0v6m0-6L10 14"/></svg></a>` : ''} |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
escapeHTML does not block a javascript: URL in href.
Line 413 escapes queryUrl, which prevents attribute breakout. It does not validate the URL scheme. The string javascript:alert(document.cookie) contains no character that escapeHTML rewrites, so it reaches the href unchanged and executes when the "Docs" link is clicked. query.query_url comes from the KICS report through data.json.
Validate the scheme before rendering.
🔒 Proposed fix
+ const safeHref = (u) => {
+ try {
+ const parsed = new URL(u, window.location.href);
+ return ['http:', 'https:'].includes(parsed.protocol) ? parsed.href : '';
+ } catch {
+ return '';
+ }
+ };
+ const docsHref = safeHref(queryUrl);
@@
- ${queryUrl !== '#' ? `<a href="${escapeHTML(queryUrl)}" target="_blank" class="...">Docs <svg ...></svg></a>` : ''}
+ ${docsHref ? `<a href="${escapeHTML(docsHref)}" target="_blank" rel="noopener noreferrer" class="...">Docs <svg ...></svg></a>` : ''}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/assets/js/tables.js` around lines 412 - 413, Validate queryUrl’s scheme
before rendering the Docs link in the template, allowing only safe HTTP(S) URLs
and treating invalid or unsafe schemes such as javascript: as unavailable. Keep
escapeHTML for the rendered href and preserve the existing queryUrl !== '#'
conditional behavior for valid URLs.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tasks/Taskfile.doc.yml`:
- Line 25: Update the serve task command to run the tracked dashboard entry
point from the repository root by invoking server.js directly, replacing the
web-directory cli.js invocation.
- Line 12: Update the description for the parse_benchmark_data task to
accurately describe the outputs produced by benchmark/parser.py and
benchmark/parser_signatures.py, naming the structured benchmark metrics and
signature CSV rather than claiming both outputs are CSV files.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 73d24052-c6ae-42f6-aaf2-5c5c8d37afef
📒 Files selected for processing (1)
tasks/Taskfile.doc.yml
|
|
||
| parse_benchmark_data: | ||
| desc: Parse benchmark results to be csv files results.csv and signatures.csv. | ||
| desc: "Parse benchmark results to be csv files results.csv and signatures.csv." |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the parse_benchmark_data description.
Line [12] says that the task creates results.csv and signatures.csv. benchmark/parser.py produces structured benchmark metrics, while benchmark/parser_signatures.py writes the signature CSV. Update the description to name the actual outputs.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tasks/Taskfile.doc.yml` at line 12, Update the description for the
parse_benchmark_data task to accurately describe the outputs produced by
benchmark/parser.py and benchmark/parser_signatures.py, naming the structured
benchmark metrics and signature CSV rather than claiming both outputs are CSV
files.
| serve: | ||
| desc: "Run local Node.js preview server for the web dashboard" | ||
| cmds: | ||
| - cd web && node cli.js --serve No newline at end of file |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major
Fix the serve task entry point.
Line [25] runs cd web && node cli.js --serve. The tracked dashboard server is server.js at the repository root, so this task does not launch the dashboard entry point. Run node server.js from the repository root.
Proposed fix
- - cd web && node cli.js --serve
+ - node server.js🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tasks/Taskfile.doc.yml` at line 25, Update the serve task command to run the
tracked dashboard entry point from the repository root by invoking server.js
directly, replacing the web-directory cli.js invocation.
1 participant
…ith updated SBOM metadata configuration
Summary by CodeRabbit