fix: fail closed for unknown rate limit operations

[KUANPEIEN]

Summary

• fail closed when RateLimiter receives an unknown operation instead of silently allowing the request
• return a structured 500 rate_limit_misconfigured response from enforcement paths so misconfigured wrappers are visible
• reuse the canonical is_valid_memory_id helper in legacy SafeDeletion so deletion validation matches the rest of the codebase

Bounty context

Addresses the rate limiter fail-open and memory ID validation inconsistency reported in #1438.
Related to the Memanto Bug & Exploit Challenge #770.

I intentionally did not narrow SourceType = str in this PR because the public API currently accepts custom source identifiers for agents/tools. That should be handled as a separate compatibility decision if maintainers want a stricter source enum.

Validation

• python -m pytest tests\test_rate_limit_validation.py -q
• python -m pytest tests\test_unit.py::TestMemoryWriteServiceDelete -q
• python -m ruff check memanto\app\utils\rate_limiting.py memanto\app\legacy\safe_deletion.py tests\test_rate_limit_validation.py
• git diff --check (passes; Git only warned about LF->CRLF on this Windows checkout)

Summary by CodeRabbit

• Bug Fixes

  • Memory ID validation now consistently uses the application’s standard format checks.
  • Rate limiting now fails closed when an operation isn’t configured, rather than proceeding.
  • Rate-limit misconfiguration is returned as a structured server error (HTTP 500) with error: rate_limit_misconfigured.

• Tests

  • Added coverage for rate-limit misconfiguration handling (including logging details).
  • Added coverage to confirm known operations are still allowed and memory ID validation stays consistent.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 58b38681-b4e1-40d4-89bf-d4258472c923

📥 Commits

Reviewing files that changed from the base of the PR and between 8dbd886 and 2153145.

📒 Files selected for processing (2)

• memanto/app/utils/rate_limiting.py
• tests/test_rate_limit_validation.py

🚧 Files skipped from review as they are similar to previous changes (2)

• memanto/app/utils/rate_limiting.py
• tests/test_rate_limit_validation.py

---

📝 Walkthrough

Walkthrough

The changes centralize SafeDeletion memory ID validation and make unknown rate-limit operations fail closed, translating configuration errors into HTTP 500 responses. New tests cover both behaviors and preserve known-operation behavior.

Changes

Validation and rate-limit hardening

Layer / File(s)	Summary
Canonical memory ID validation memanto/app/legacy/safe_deletion.py, tests/test_rate_limit_validation.py	SafeDeletion now delegates memory ID checks to is_valid_memory_id, with parametrized equivalence tests.
Rate-limit misconfiguration handling memanto/app/utils/rate_limiting.py, tests/test_rate_limit_validation.py	Unknown operations raise ValueError and are converted by enforcement into HTTP 500 errors; known operations still allow the first request.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main behavior change: failing closed for unknown rate-limit operations.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.}

[coderabbitai]

🧹 Nitpick comments (1)

memanto/app/utils/rate_limiting.py (1)

77-86: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Log misconfiguration errors for operational visibility.

The 429 rate-limit path logs via MemantoLogger.log_request, but the new 500 misconfiguration path does not log anything. A rate_limit_misconfigured event is a serious operational issue that should be visible in logs for alerting and debugging.

♻️ Proposed refactor: add logging to the 500 path

         try:
             allowed, retry_after = self.check_rate_limit(operation, agent_id)
         except ValueError as exc:
+            from memanto.app.utils.logging import MemantoLogger
+
+            MemantoLogger.log_request(
+                request_id="rate_limit",
+                route=f"/{operation}",
+                method="POST",
+                status_code=500,
+                latency_ms=0,
+                agent_id=agent_id,
+                errors=["rate_limit_misconfigured"],
+            )
             raise HTTPException(
                 status_code=500,
                 detail={
                     "error": "rate_limit_misconfigured",
                     "message": str(exc),
                 },
+                from exc,
             )

🤖 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 `@memanto/app/utils/rate_limiting.py` around lines 77 - 86, Update the
ValueError handler around check_rate_limit in the rate-limiting flow to log the
rate_limit_misconfigured event through MemantoLogger.log_request before raising
HTTPException. Include the relevant operation, agent_id, and exception details
while preserving the existing 500 response payload.

🤖 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.

Nitpick comments:
In `@memanto/app/utils/rate_limiting.py`:
- Around line 77-86: Update the ValueError handler around check_rate_limit in
the rate-limiting flow to log the rate_limit_misconfigured event through
MemantoLogger.log_request before raising HTTPException. Include the relevant
operation, agent_id, and exception details while preserving the existing 500
response payload.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 1f5a16b1-0eb9-4741-82aa-bd1eda093329

📥 Commits

Reviewing files that changed from the base of the PR and between 2044fdf and 8dbd886.

📒 Files selected for processing (3)

• memanto/app/legacy/safe_deletion.py
• memanto/app/utils/rate_limiting.py
• tests/test_rate_limit_validation.py