feat(report): modernize HTML report UI and add exports#1338
Open
Santoshverma77 wants to merge 5 commits intoOWASP:masterfrom
Open
feat(report): modernize HTML report UI and add exports#1338Santoshverma77 wants to merge 5 commits intoOWASP:masterfrom
Santoshverma77 wants to merge 5 commits intoOWASP:masterfrom
Conversation
Improve the generated HTML report with a professional layout and styling. Add client-side export actions (JSON/CSV/Text) plus print-to-PDF support while preserving existing data rendering. Made-with: Cursor
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Summary by CodeRabbit
WalkthroughReworks the web report HTML scaffold and table markup, replaces client JSON parsing with a feature-rich, defensive implementation (including exports), tweaks core table header labels, and relaxes the Python version constraint in pyproject.toml. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@nettacker/web/static/report/json_parse.js`:
- Around line 22-26: The current loop incorrectly skips rendering when the
clipboard button is missing; update the check so you only skip when the
container is absent (keep the lookup for button using "json_clipboard_"+i but do
not continue if button is null), render/parse into the container regardless, and
only attach clipboard functionality if button exists; apply the same change to
the second block that uses "json_event_"+i and "json_clipboard_"+i so missing
clipboard elements no longer prevent JSON from being parsed, rendered, included
in summaries, or exported.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
nettacker/core/graph.pynettacker/web/static/report/html_table.cssnettacker/web/static/report/json_parse.jsnettacker/web/static/report/table_end.htmlnettacker/web/static/report/table_items.htmlnettacker/web/static/report/table_title.html
Made-with: Cursor
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@nettacker/web/static/report/json_parse.js`:
- Around line 112-139: The toCSV function derives headers from only the first
row which can drop fields present in later rows; update the headers logic in
toCSV to collect the union of keys across all objects in data (e.g., iterate
over data and add Object.keys(row) to a Set), then convert that set to an array
(optionally sorted for deterministic output) and use that array in the existing
headers/rows/escapeCell flow so all columns from every event are exported.
- Around line 142-166: The toText function currently ignores entries that only
have the fallback ev.raw (created when JSON parsing failed), resulting in empty
lines; update toText (in function toText) to detect ev.raw and include it in the
parts array (e.g., push ev.raw when present or when no other fields were added)
so non-JSON events render their raw content in the text export while preserving
existing fields like ev.date, ev.target, ev.module_name, ev.port, and ev.event.
Made-with: Cursor
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
nettacker/web/static/report/json_parse.js (2)
141-165:⚠️ Potential issue | 🟡 MinorText export still omits fallback
rawcontent for non-JSON entries.When parsing fails (Line 34), events become
{ raw: value }, buttoTextnever includesev.raw, producing near-empty lines at Line 163.🔧 Proposed fix
if (ev.event) { parts.push("- " + ev.event); } + if (!parts.length && ev.raw) { + parts.push(ev.raw); + } lines.push((index + 1) + ". " + parts.join(" "));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nettacker/web/static/report/json_parse.js` around lines 141 - 165, toText currently ignores the parsed fallback payload stored as ev.raw for non-JSON events, producing near-empty lines; update the toText function to check for ev.raw and include it in the parts array (e.g., push ev.raw when present, perhaps after ev.event or if parts is empty) so the rendered line contains the raw content for events that failed JSON parsing; ensure you reference the toText function and the ev.raw property when making the change so empty entries are replaced with the raw payload.
111-139:⚠️ Potential issue | 🟠 MajorCSV export still drops columns that appear after the first event.
Line 115 derives headers from only
data[0], so fields present only in later rows are omitted from export.🔧 Proposed fix
function toCSV(data) { if (!data.length) { return ""; } - const headers = Object.keys(data[0]); + const headerSet = new Set(); + data.forEach(function (row) { + Object.keys(row || {}).forEach(function (key) { + headerSet.add(key); + }); + }); + const headers = Array.from(headerSet); const escapeCell = function (cell) { @@ data.forEach(function (row) { rows.push( headers .map(function (key) { - return escapeCell(row[key]); + return escapeCell(row ? row[key] : ""); }) .join(","), ); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nettacker/web/static/report/json_parse.js` around lines 111 - 139, The CSV export in toCSV() builds headers from only data[0], dropping keys that appear later; update toCSV to compute headers as the union of all object keys across every row (preserving first-seen order) instead of Object.keys(data[0]). Locate function toCSV (and escapeCell helper) and replace the headers assignment with logic that iterates over data, collects unique keys into an array in encounter order, then use that headers array for the header row and subsequent row mapping so all fields from any event are included in the CSV.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pyproject.toml`:
- Line 47: Update the Python version constraint in pyproject.toml from
">=3.10,<3.14" to ">=3.10,<3.13" so the package metadata matches the documented
supported range (Python 3.10–3.12); locate the python = "...” entry in
pyproject.toml and modify the version specifier accordingly.
---
Duplicate comments:
In `@nettacker/web/static/report/json_parse.js`:
- Around line 141-165: toText currently ignores the parsed fallback payload
stored as ev.raw for non-JSON events, producing near-empty lines; update the
toText function to check for ev.raw and include it in the parts array (e.g.,
push ev.raw when present, perhaps after ev.event or if parts is empty) so the
rendered line contains the raw content for events that failed JSON parsing;
ensure you reference the toText function and the ev.raw property when making the
change so empty entries are replaced with the raw payload.
- Around line 111-139: The CSV export in toCSV() builds headers from only
data[0], dropping keys that appear later; update toCSV to compute headers as the
union of all object keys across every row (preserving first-seen order) instead
of Object.keys(data[0]). Locate function toCSV (and escapeCell helper) and
replace the headers assignment with logic that iterates over data, collects
unique keys into an array in encounter order, then use that headers array for
the header row and subsequent row mapping so all fields from any event are
included in the CSV.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
nettacker/web/static/report/html_table.cssnettacker/web/static/report/json_parse.jspyproject.toml
Made-with: Cursor
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
nettacker/web/static/report/json_parse.js (1)
139-146:⚠️ Potential issue | 🟡 MinorAdd defensive null-check for
rowin mapping.The header collection loop defensively checks
if (row && typeof row === "object")(line 118), but the row mapping at line 143 doesn't. Ifdataever contains a null/undefined entry,row[key]throws.🛡️ Proposed fix
data.forEach(function (row) { rows.push( headers .map(function (key) { - return escapeCell(row[key]); + return escapeCell(row ? row[key] : undefined); }) .join(","), ); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nettacker/web/static/report/json_parse.js` around lines 139 - 146, The mapping over data rows can throw if an entry is null/undefined; update the data.forEach mapping (the block that calls headers.map and escapeCell) to first check that row is an object (e.g., row && typeof row === "object") and skip or substitute an empty object/empty cells when it isn't; ensure the same defensive check used earlier is applied before calling row[key] so headers.map never accesses properties on null/undefined.
🧹 Nitpick comments (2)
nettacker/web/static/report/json_parse.js (2)
18-18: Remove unusedrawEventsarray.
rawEventsis populated at line 28 but never read. If there's no planned use, remove it to avoid confusion.🧹 Proposed fix
- const rawEvents = []; const events = [];And remove from the loop:
const value = container.innerText; - rawEvents.push(value);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nettacker/web/static/report/json_parse.js` at line 18, Remove the unused array declaration and its population: delete the declaration "const rawEvents = [];" and remove any "rawEvents.push(...)" calls inside the loop (where events are parsed/populated) so no dead variable remains; ensure there are no other references to rawEvents after removal and keep the rest of the parsing logic (e.g., the parsed events variable used elsewhere) intact.
91-93: Consider logging caught errors for debugging.Silent catch blocks can obscure issues during development. Since summary is non-critical, log rather than throw.
🔧 Proposed fix
} catch (e) { - // Non-fatal, summary is optional + // Non-fatal, summary is optional + console.warn("Summary population failed", e); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nettacker/web/static/report/json_parse.js` around lines 91 - 93, The catch block that swallows errors for the summary parse (the catch (e) after attempting to parse "summary") should log the exception instead of remaining silent; update the catch handling in nettacker/web/static/report/json_parse.js (the catch (e) block that comments "Non-fatal, summary is optional") to call the project's logging mechanism or at minimum console.warn/console.debug with a clear message and the error object (include context like "Failed to parse summary" and the error) so developers can diagnose malformed summary data without throwing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@nettacker/web/static/report/json_parse.js`:
- Around line 139-146: The mapping over data rows can throw if an entry is
null/undefined; update the data.forEach mapping (the block that calls
headers.map and escapeCell) to first check that row is an object (e.g., row &&
typeof row === "object") and skip or substitute an empty object/empty cells when
it isn't; ensure the same defensive check used earlier is applied before calling
row[key] so headers.map never accesses properties on null/undefined.
---
Nitpick comments:
In `@nettacker/web/static/report/json_parse.js`:
- Line 18: Remove the unused array declaration and its population: delete the
declaration "const rawEvents = [];" and remove any "rawEvents.push(...)" calls
inside the loop (where events are parsed/populated) so no dead variable remains;
ensure there are no other references to rawEvents after removal and keep the
rest of the parsing logic (e.g., the parsed events variable used elsewhere)
intact.
- Around line 91-93: The catch block that swallows errors for the summary parse
(the catch (e) after attempting to parse "summary") should log the exception
instead of remaining silent; update the catch handling in
nettacker/web/static/report/json_parse.js (the catch (e) block that comments
"Non-fatal, summary is optional") to call the project's logging mechanism or at
minimum console.warn/console.debug with a clear message and the error object
(include context like "Failed to parse summary" and the error) so developers can
diagnose malformed summary data without throwing.
Made-with: Cursor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Improve the generated HTML report with a professional layout and styling. Add client-side export actions (JSON/CSV/Text) plus print-to-PDF support while preserving existing data rendering.
Proposed change
Your PR description goes here.
Type of change
Checklist
make pre-commit, it didn't generate any changesmake test, all tests passed locally