Fix resource leaks in static asset loading

[xovishnukosuri]

Fixes #1320

Replaces bare open().read() calls with Path.read_text() in log_data.py to ensure proper file handle management and avoid potential resource leaks.

Changes:

• Added read_static_text() helper function using Path.read_text()
• Updated css_1, json_parse_js, table_end, table_items, table_title to use the helper
• Added proper UTF-8 encoding specification

[coderabbitai]

Summary by CodeRabbit

• Refactor
  • Centralized and standardized how static files and templates are read across the app: added a shared UTF-8 text reader and replaced scattered file-read calls with the new reader, ensuring consistent UTF-8 handling and text loading semantics for logos, payloads, templates, and configuration files.

Walkthrough

Replaces direct file reads with UTF-8-aware readers: adds read_static_text(Path) -> str in nettacker/lib/html_log/log_data.py and switches multiple modules to use Path.read_text(encoding="utf-8") (or the new helper) for static assets and payload/config reads.

Changes

Cohort / File(s)	Summary
HTML log helper + refactors nettacker/lib/html_log/log_data.py	Added read_static_text(path: Path) -> str and replaced unmanaged open().read() calls for CSS/JS/HTML fragments with the new helper (centralizes UTF-8 reads and resource handling).
Core app/logo read nettacker/core/app.py	Switched logo file read to use Path.read_text(encoding="utf-8") instead of manual open().read().
Fuzzer payload reads nettacker/core/fuzzer.py, nettacker/core/utils/common.py	Replaced open(...).read().split(...) with Path.read_text(encoding="utf-8") then split — explicit UTF-8 text reads.
Messages YAML loading nettacker/core/messages.py	Now imports Path and reads YAML via Path.read_text(encoding="utf-8") wrapped in StringIO for parsing.
Report/graph template reads nettacker/lib/compare_report/engine.py, nettacker/lib/graph/d3_tree_v1/engine.py	Replaced raw open().read() template loads with Path.read_text(encoding="utf-8"); subsequent string substitutions unchanged.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Out of Scope Changes check	⚠️ Warning	The PR extends beyond the linked issue scope by fixing resource leaks in additional modules (app.py, fuzzer.py, messages.py, common.py, compare_report/engine.py, graph/d3_tree_v1/engine.py) not mentioned in issue #1320.	Either focus PR #1344 on log_data.py only, or create a new issue for the additional modules and update the linked issues accordingly.
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Fix resource leaks in static asset loading' clearly summarizes the main change—replacing unsafe open().read() calls with Path.read_text() for proper resource management.
Description check	✅ Passed	The description accurately relates to the changeset, explaining the issue fix, the helper function addition, and the specific files and methods updated.
Linked Issues check	✅ Passed	The PR directly addresses issue #1320 by replacing unsafe open().read() calls with Path.read_text() in log_data.py, adding a helper function, and ensuring UTF-8 encoding for the five affected files.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

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/lib/html_log/log_data.py`:
- Around line 6-7: Add a concise docstring to the public function
read_static_text(path: Path) describing its purpose (reads and returns the text
contents of a file), the parameter (path: Path — file path to read), the return
value (str — file contents decoded as UTF-8), and any noteworthy behavior (uses
encoding="utf-8"). Place the docstring immediately below the def
read_static_text(...) signature following standard triple-quoted style.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 87bc9d04-cb9b-4eae-bb8a-10b43a434ac0

📥 Commits

Reviewing files that changed from the base of the PR and between ceb05e4 and bc20fe5.

📒 Files selected for processing (1)

• nettacker/lib/html_log/log_data.py

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Add a docstring to read_static_text.
This is a public module-level function and should include a short docstring for maintainability and API clarity.

As per coding guidelines, "Keep functions small, use type hints where practical, and add docstrings for public APIs".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@nettacker/lib/html_log/log_data.py` around lines 6 - 7, Add a concise
docstring to the public function read_static_text(path: Path) describing its
purpose (reads and returns the text contents of a file), the parameter (path:
Path — file path to read), the return value (str — file contents decoded as
UTF-8), and any noteworthy behavior (uses encoding="utf-8"). Place the docstring
immediately below the def read_static_text(...) signature following standard
triple-quoted style.

[coderabbitai]

🧹 Nitpick comments (1)

nettacker/core/messages.py (1)

24-25: Unnecessary StringIO wrapper; consider yaml.safe_load for defense-in-depth.

1. The StringIO wrapper is unnecessary since yaml.load() accepts strings directly:

   return yaml.load(yaml_text, Loader=yaml.FullLoader)

2. Static analysis (Ruff S506) flags yaml.FullLoader as potentially unsafe because it allows instantiation of arbitrary objects. While these are internal locale files, using yaml.safe_load() would be more secure:

   return yaml.safe_load(yaml_text)

   If safe_load is too restrictive for the YAML content, the current approach is acceptable given these are trusted internal files.

♻️ Suggested simplification

 def load_yaml(filename):
     yaml_text = Path(filename).read_text(encoding="utf-8")
-    return yaml.load(StringIO(yaml_text), Loader=yaml.FullLoader)
+    return yaml.safe_load(yaml_text)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@nettacker/core/messages.py` around lines 24 - 25, Replace the unnecessary
StringIO wrapper and the unsafe yaml.FullLoader usage: read the file into
yaml_text via Path(...).read_text and pass that string directly to
yaml.safe_load instead of yaml.load(StringIO(...), Loader=yaml.FullLoader);
update the call sites referencing yaml.load and StringIO in this module (e.g.,
the function that reads message YAML files) to use yaml.safe_load(yaml_text) to
remove StringIO and harden against unsafe object construction while preserving
existing behavior for trusted internal files.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@nettacker/core/messages.py`:
- Around line 24-25: Replace the unnecessary StringIO wrapper and the unsafe
yaml.FullLoader usage: read the file into yaml_text via Path(...).read_text and
pass that string directly to yaml.safe_load instead of yaml.load(StringIO(...),
Loader=yaml.FullLoader); update the call sites referencing yaml.load and
StringIO in this module (e.g., the function that reads message YAML files) to
use yaml.safe_load(yaml_text) to remove StringIO and harden against unsafe
object construction while preserving existing behavior for trusted internal
files.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: d116e376-e55c-4524-9454-c5ae63cec997

📥 Commits

Reviewing files that changed from the base of the PR and between bc20fe5 and 2780f67.

📒 Files selected for processing (6)

• nettacker/core/app.py
• nettacker/core/fuzzer.py
• nettacker/core/messages.py
• nettacker/core/utils/common.py
• nettacker/lib/compare_report/engine.py
• nettacker/lib/graph/d3_tree_v1/engine.py

[Tech-Psycho95]

Hi @xovishnukosuri,
This issue is already assigned to me and I’m actively working on it.
Please avoid opening PRs for assigned issues to prevent duplication.
Thanks for understanding.