🛡️ Sentinel: [CRITICAL] Fix TOCTOU and Unbounded Memory Consumption in JSON Parsers

[ivangegovdve-sudo]

🚨 Severity: CRITICAL
💡 Vulnerability: Unbounded memory consumption and TOCTOU DoS due to unrestricted file read operations when loading JSON storage. An attacker could bypass the path.stat().st_size check by supplying a device file like /dev/zero (which falsely reports size 0), causing the application to consume memory until crashing.
🎯 Impact: Potential Denial of Service (DoS) and application crash if a special file or excessively large maliciously modified file is loaded.
🔧 Fix:

1. Added path.is_file() checks across all JSON file adapters to reject device files before reading.
2. Replaced path.read_text() with a secure bounded read f.read(10 * 1024 * 1024 + 1) which explicitly caps the memory allocated and raises a ValueError if the file exceeds the 10MB limit.
   ✅ Verification: Verified via a new test suite (tests/test_security_dos.py) that successfully checks the resilience of the bounded read and verifies that non-regular files are rejected. Passed uv run pytest, formatting, and typing checks.

---

PR created automatically by Jules for task 1530102924170072442 started by @ivangegovdve-sudo

[google-labs-jules]

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.

---

For security, I will only act on instructions from the user who triggered this task.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: df4908877c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Enforce 10MB cap using byte length

Reading with open(..., encoding="utf-8") and f.read(10 * 1024 * 1024 + 1) limits characters, then checks len(content) (also characters), so files larger than 10MB in bytes can bypass the limit when they contain multibyte UTF-8 characters (for example, ~15MB file decoding to <10M chars). This regresses the previous byte-based guard and weakens the DoS protection goal; the same pattern appears in multiple adapters in this commit. Use a binary bounded read (rb) and enforce the limit on raw byte count before decoding.

Useful? React with 👍 / 👎.

[gemini-code-assist]

Code Review

This pull request addresses a security vulnerability by implementing bounded file reads and replacing existence checks with file-type checks in several adapters. It also updates the security documentation and adds a new test suite. Review feedback suggests centralizing the 10MB file size limit into a constant to eliminate magic numbers and improve maintainability across both production and test code. Additionally, it is recommended to extend the test suite to cover all modified adapters for comprehensive verification.

[gemini-code-assist]

The new test suite is a great addition for verifying this critical security fix. However, it currently only includes tests for JsonFilePracticeRepository and CheckpointStore. The same vulnerability was fixed in JsonFileProgressRepository and JsonFileProgressSnapshotStore. To ensure comprehensive test coverage, please consider adding similar tests for these other adapters as well.

[gemini-code-assist]

To improve maintainability and avoid magic numbers, it's good practice to define the file size limit in a variable. This makes it easier to find and change the limit in the future and allows for a dynamic error message. Ideally, this would be a module-level constant.

Suggested change

	with open(path, encoding="utf-8") as f:
	content = f.read(10 * 1024 * 1024 + 1)
	if len(content) > 10 * 1024 * 1024:
	raise ValueError(f"Checkpoint file {path} exceeds 10MB size limit")
	parsed = json.loads(path.read_text(encoding="utf-8"))
	parsed = json.loads(content)
	limit_bytes = 10 * 1024 * 1024
	with open(path, encoding="utf-8") as f:
	content = f.read(limit_bytes + 1)
	if len(content) > limit_bytes:
	raise ValueError(f"Checkpoint file {path} exceeds {limit_bytes // (1024 * 1024)}MB size limit")
	parsed = json.loads(content)

[gemini-code-assist]

To improve maintainability and avoid magic numbers, it's good practice to define the file size limit in a variable. This makes it easier to find and change the limit in the future and allows for a dynamic error message. Ideally, this would be a module-level constant.

Suggested change

	with open(self._file_path, encoding="utf-8") as f:
	content = f.read(10 * 1024 * 1024 + 1)
	if len(content) > 10 * 1024 * 1024:
	raise ValueError(
	"Practice repository file "
	f"{self._file_path} exceeds 10MB size limit"
	)
	if not content.strip():
	return {"items": [], "attempts": []}
	parsed = json.loads(content)
	limit_bytes = 10 * 1024 * 1024
	with open(self._file_path, encoding="utf-8") as f:
	content = f.read(limit_bytes + 1)
	if len(content) > limit_bytes:
	raise ValueError(
	"Practice repository file "
	f"{self._file_path} exceeds {limit_bytes // (1024 * 1024)}MB size limit"
	)
	if not content.strip():
	return {"items": [], "attempts": []}
	parsed = json.loads(content)

[gemini-code-assist]

To improve maintainability and avoid magic numbers, it's good practice to define the file size limit in a variable. This makes it easier to find and change the limit in the future and allows for a dynamic error message. Ideally, this would be a module-level constant.

Suggested change

	with open(self._file_path, encoding="utf-8") as f:
	content = f.read(10 * 1024 * 1024 + 1)
	if len(content) > 10 * 1024 * 1024:
	raise ValueError(
	f"Progress snapshot file {self._file_path} exceeds 10MB size limit"
	)
	parsed = json.loads(content)
	limit_bytes = 10 * 1024 * 1024
	with open(self._file_path, encoding="utf-8") as f:
	content = f.read(limit_bytes + 1)
	if len(content) > limit_bytes:
	raise ValueError(
	f"Progress snapshot file {self._file_path} exceeds {limit_bytes // (1024 * 1024)}MB size limit"
	)
	parsed = json.loads(content)

[gemini-code-assist]

This test uses a magic number for the size limit, which is also present in the production code. To improve maintainability, consider defining this as a constant (e.g., MAX_FILE_SIZE_BYTES = 10 * 1024 * 1024) and using it in both the test and production code. This ensures that if the limit changes, the test is updated accordingly and remains effective.