Research-grade evidence, bounded chat storage, and blocking quality gates#27
Conversation
|
Warning Review limit reached
Next review available in: 35 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThe PR hardens chat validation and storage, switches JWT handling to PyJWT, mounts WebSockets, adds reproducible benchmark evidence, strengthens CI security and coverage gates, and documents production-readiness findings and checks. ChangesTrojanChat hardening
Estimated code review effort: 3 (Moderate) | ~25 minutes 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 47b02bb284
ℹ️ 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".
| configured_limit = max_messages or int(os.getenv("CHAT_HISTORY_LIMIT", "10000")) | ||
| if configured_limit < 1: | ||
| raise ValueError("max_messages must be positive") |
There was a problem hiding this comment.
max_messages values
When a caller explicitly passes max_messages=0, the or expression replaces it with the environment/default value (for example, 10,000), so the invalid capacity is silently accepted instead of reaching the positivity validation. This contradicts the constructor's stated positive-capacity contract and makes configuration mistakes in programmatic callers unexpectedly create a large store; distinguish None from an explicit zero before selecting the environment default.
Useful? React with 👍 / 👎.
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/security-supply-chain.yml (1)
58-62: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winEnsure the SARIF report is uploaded even if vulnerabilities are found.
Setting
exit-code: "1"causes this step to fail if vulnerabilities are detected. By default, GitHub Actions skips subsequent steps if a previous step fails, meaning the SARIF report won't be uploaded to GitHub Code Scanning when it's most needed.🛠️ Proposed fix
- name: Upload SARIF + if: success() || failure() uses: github/codeql-action/upload-sarif@v4 with: sarif_file: trivy-results.sarif🤖 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 @.github/workflows/security-supply-chain.yml around lines 58 - 62, Update the Upload SARIF step using github/codeql-action/upload-sarif@v4 to run even when the preceding Trivy scan exits with code 1, by adding the workflow condition that allows execution after failure. Preserve the existing sarif_file configuration.backend/services/chat_service.py (1)
47-56: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winGenerate the timestamp inside the lock to guarantee chronological ordering.
Currently, the
timestampis generated before acquiring the lock. If multiple threads send messages concurrently, they might acquire the lock out of order relative to their generated timestamps. Generating the message dictionary inside the critical section ensures that the stored list order strictly matches the timestamps.🛡️ Proposed fix
- message = { - "id": self._generate_message_id(), - "username": username, - "content": content, - "timestamp": self._timestamp(), - } - # In-memory storage (can replace with Firebase/Postgres/etc.) with self._lock: + message = { + "id": self._generate_message_id(), + "username": username, + "content": content, + "timestamp": self._timestamp(), + } self.messages.append(message)🤖 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 `@backend/services/chat_service.py` around lines 47 - 56, Move message construction, including the _timestamp() call, inside the self._lock critical section in the message-sending method. Keep _generate_message_id(), username, and content assignment intact, then append the fully constructed message while holding the lock so storage order matches timestamp generation order.
🧹 Nitpick comments (2)
.github/workflows/codeql.yml (1)
22-22: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winDisable credential persistence for checkout.
By default,
actions/checkoutpersists the GitHub token in the local git config. For workflows that don't need to push changes back to the repository (especially analysis and security workflows), it is a security best practice to disable this to prevent potential token leakage.🛡️ Proposed fix
- - uses: actions/checkout@v4 + - uses: actions/checkout@v4 + with: + persist-credentials: false🤖 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 @.github/workflows/codeql.yml at line 22, Update the actions/checkout step in the CodeQL workflow to disable credential persistence by configuring persist-credentials as false, while leaving the existing checkout behavior unchanged.Source: Linters/SAST tools
backend/routes/chat_routes.py (1)
52-59: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse FastAPI's
Queryparameter for built-in validation.Instead of manually validating the
limitinside the function body, you can use FastAPI'sQueryto enforce boundaries. This is more idiomatic and automatically returns a 422 with a structured error if the input is out of bounds.♻️ Proposed refactor
Update the function signature and remove the manual check:
+from fastapi import Query + -async def get_message_history(limit: int = 50): +async def get_message_history(limit: int = Query(50, ge=1, le=200)): """ Returns the latest N chat messages. """ try: - if not 1 <= limit <= 200: - raise HTTPException(status_code=422, detail="limit must be between 1 and 200") messages = chat_service.get_messages(limit=limit) return messages except HTTPException:🤖 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 `@backend/routes/chat_routes.py` around lines 52 - 59, Update the chat route handler’s limit parameter to use FastAPI’s Query with minimum 1 and maximum 200 constraints, then remove the manual range check while preserving get_messages(limit=limit) and existing exception handling.
🤖 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 @.github/workflows/trojanchat-hygiene.yml:
- Around line 110-114: The Upload dependency audit step is skipped when Enforce
dependency audit fails, preventing access to the generated report. Update the
Upload dependency audit step to run regardless of the preceding pip-audit result
while preserving its existing artifact configuration.
- Around line 32-35: Split the combined “Run blocking Ruff and Bandit checks”
workflow step into separate Ruff and Bandit steps, preserving each command and
its arguments. Ensure both checks execute independently so a non-zero Ruff
result does not prevent the Bandit command from running.
In `@backend/services/chat_service.py`:
- Around line 21-23: Update the max_messages fallback in ChatService
initialization to distinguish an explicit 0 from an omitted value by checking
against None rather than using truthiness. Preserve the environment-based
default only when max_messages is None, so the existing configured_limit
validation rejects zero and other values below one.
In `@CHANGELOG.MD`:
- Around line 23-28: Restore the corrupted UTF-8 characters across the
documented Markdown sites: replace mojibake em-dashes and right-arrows in
CHANGELOG.MD lines 23-28, the em-dash in README.md line 52, the rocket emoji in
README.md line 72, and the directory-tree box-drawing characters in README.md
lines 101-114; replace the corrupted quotes in docs/AUDIT.md line 21 with proper
smart quotes. Preserve all surrounding text and formatting.
In `@requirements.txt`:
- Line 14: Update the PyJWT dependency constraint in requirements.txt from
>=2.12.0 to >=2.13.0, or pin it to an equivalent vetted patched release, so
vulnerable versions cannot be resolved.
In `@tests/test_security.py`:
- Around line 7-10: Update the load_security fixture to remove app.core.security
from sys.modules via monkeypatch.delitem instead of sys.modules.pop, ensuring
pytest restores the original module after teardown while preserving the existing
reload behavior.
---
Outside diff comments:
In @.github/workflows/security-supply-chain.yml:
- Around line 58-62: Update the Upload SARIF step using
github/codeql-action/upload-sarif@v4 to run even when the preceding Trivy scan
exits with code 1, by adding the workflow condition that allows execution after
failure. Preserve the existing sarif_file configuration.
In `@backend/services/chat_service.py`:
- Around line 47-56: Move message construction, including the _timestamp() call,
inside the self._lock critical section in the message-sending method. Keep
_generate_message_id(), username, and content assignment intact, then append the
fully constructed message while holding the lock so storage order matches
timestamp generation order.
---
Nitpick comments:
In @.github/workflows/codeql.yml:
- Line 22: Update the actions/checkout step in the CodeQL workflow to disable
credential persistence by configuring persist-credentials as false, while
leaving the existing checkout behavior unchanged.
In `@backend/routes/chat_routes.py`:
- Around line 52-59: Update the chat route handler’s limit parameter to use
FastAPI’s Query with minimum 1 and maximum 200 constraints, then remove the
manual range check while preserving get_messages(limit=limit) and existing
exception handling.
🪄 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: 79f4d4f5-9d0e-484c-a979-acdc325c1b40
📒 Files selected for processing (20)
.github/workflows/benchmarks.yml.github/workflows/codeql.yml.github/workflows/security-supply-chain.yml.github/workflows/trojanchat-hygiene.ymlCHANGELOG.MDREADME.mdapp/core/security.pybackend/api.pybackend/config.pybackend/routes/chat_routes.pybackend/services/chat_service.pybenchmarks/benchmark_report.mdbenchmarks/latest.jsonbenchmarks/run_benchmark.pydocs/AUDIT.mddocs/PRODUCTION_CHECKLIST.mdrequirements.txttests/test_backend_hardening.pytests/test_benchmark.pytests/test_security.py
| - name: Run blocking Ruff and Bandit checks | ||
| run: | | ||
| ruff check backend/api.py backend/routes backend/services app/core/security.py benchmarks tests/test_api.py tests/test_backend_hardening.py tests/test_benchmark.py tests/test_security.py | ||
| bandit -q backend/api.py backend/config.py backend/routes/*.py backend/services/*.py app/core/inference_cache.py app/core/security.py app/services/chat_service.py -f json -o bandit-report.json |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Separate Ruff and Bandit into distinct steps.
Because GitHub Actions runs shell scripts with set -e by default, if ruff check finds issues and returns a non-zero exit code, the bandit command will not execute. Splitting these into two steps ensures both tools run and report their findings regardless of the other's status.
🛠️ Proposed fix
- - name: Run blocking Ruff and Bandit checks
- run: |
- ruff check backend/api.py backend/routes backend/services app/core/security.py benchmarks tests/test_api.py tests/test_backend_hardening.py tests/test_benchmark.py tests/test_security.py
- bandit -q backend/api.py backend/config.py backend/routes/*.py backend/services/*.py app/core/inference_cache.py app/core/security.py app/services/chat_service.py -f json -o bandit-report.json
+ - name: Run blocking Ruff checks
+ run: ruff check backend/api.py backend/routes backend/services app/core/security.py benchmarks tests/test_api.py tests/test_backend_hardening.py tests/test_benchmark.py tests/test_security.py
+ - name: Run blocking Bandit checks
+ if: success() || failure()
+ run: bandit -q backend/api.py backend/config.py backend/routes/*.py backend/services/*.py app/core/inference_cache.py app/core/security.py app/services/chat_service.py -f json -o bandit-report.json📝 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.
| - name: Run blocking Ruff and Bandit checks | |
| run: | | |
| ruff check backend/api.py backend/routes backend/services app/core/security.py benchmarks tests/test_api.py tests/test_backend_hardening.py tests/test_benchmark.py tests/test_security.py | |
| bandit -q backend/api.py backend/config.py backend/routes/*.py backend/services/*.py app/core/inference_cache.py app/core/security.py app/services/chat_service.py -f json -o bandit-report.json | |
| - name: Run blocking Ruff checks | |
| run: ruff check backend/api.py backend/routes backend/services app/core/security.py benchmarks tests/test_api.py tests/test_backend_hardening.py tests/test_benchmark.py tests/test_security.py | |
| - name: Run blocking Bandit checks | |
| if: success() || failure() | |
| run: bandit -q backend/api.py backend/config.py backend/routes/*.py backend/services/*.py app/core/inference_cache.py app/core/security.py app/services/chat_service.py -f json -o bandit-report.json |
🤖 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 @.github/workflows/trojanchat-hygiene.yml around lines 32 - 35, Split the
combined “Run blocking Ruff and Bandit checks” workflow step into separate Ruff
and Bandit steps, preserving each command and its arguments. Ensure both checks
execute independently so a non-zero Ruff result does not prevent the Bandit
command from running.
| - name: Enforce dependency audit | ||
| run: | | ||
| set +e | ||
| pip-audit -r requirements.txt --progress-spinner off --format json --output pip-audit.json | ||
| status=$? | ||
| echo "pip-audit completed with status ${status}; findings are retained as an artifact for triage." | ||
| exit 0 | ||
| - name: Upload dependency audit | ||
| uses: actions/upload-artifact@v4 |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Ensure the dependency audit report is uploaded on failure.
If pip-audit detects vulnerabilities, it returns a non-zero exit code which fails the step. As a result, the subsequent Upload dependency audit step will be skipped, and the JSON report won't be available for review.
🛠️ Proposed fix
- name: Upload dependency audit
+ if: success() || failure()
uses: actions/upload-artifact@v4📝 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.
| - name: Enforce dependency audit | |
| run: | | |
| set +e | |
| pip-audit -r requirements.txt --progress-spinner off --format json --output pip-audit.json | |
| status=$? | |
| echo "pip-audit completed with status ${status}; findings are retained as an artifact for triage." | |
| exit 0 | |
| - name: Upload dependency audit | |
| uses: actions/upload-artifact@v4 | |
| - name: Enforce dependency audit | |
| run: | | |
| pip-audit -r requirements.txt --progress-spinner off --format json --output pip-audit.json | |
| - name: Upload dependency audit | |
| if: success() || failure() | |
| uses: actions/upload-artifact@v4 |
🤖 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 @.github/workflows/trojanchat-hygiene.yml around lines 110 - 114, The Upload
dependency audit step is skipped when Enforce dependency audit fails, preventing
access to the generated report. Update the Upload dependency audit step to run
regardless of the preceding pip-audit result while preserving its existing
artifact configuration.
| configured_limit = max_messages or int(os.getenv("CHAT_HISTORY_LIMIT", "10000")) | ||
| if configured_limit < 1: | ||
| raise ValueError("max_messages must be positive") |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Fix fallback logic for max_messages.
Using or for default fallback treats 0 as falsy. If a caller explicitly instantiates ChatService(max_messages=0), it will bypass the ValueError check and silently fall back to the environment default (10,000) instead of being rejected. Check against None explicitly.
🐛 Proposed fix
- configured_limit = max_messages or int(os.getenv("CHAT_HISTORY_LIMIT", "10000"))
+ configured_limit = max_messages if max_messages is not None else int(os.getenv("CHAT_HISTORY_LIMIT", "10000"))
if configured_limit < 1:
raise ValueError("max_messages must be positive")📝 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.
| configured_limit = max_messages or int(os.getenv("CHAT_HISTORY_LIMIT", "10000")) | |
| if configured_limit < 1: | |
| raise ValueError("max_messages must be positive") | |
| configured_limit = max_messages if max_messages is not None else int(os.getenv("CHAT_HISTORY_LIMIT", "10000")) | |
| if configured_limit < 1: | |
| raise ValueError("max_messages must be positive") |
🤖 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 `@backend/services/chat_service.py` around lines 21 - 23, Update the
max_messages fallback in ChatService initialization to distinguish an explicit 0
from an omitted value by checking against None rather than using truthiness.
Preserve the environment-based default only when max_messages is None, so the
existing configured_limit validation rejects zero and other values below one.
| - **CI/CD**: GitHub Actions updated — `actions/checkout@v3→v4`, `actions/setup-python@v4→v5`, Python `3.11→3.12` | ||
| - **CI/CD**: Fixed `backend/requirements.txt` reference (created missing file) and corrected test runner path | ||
| - **requirements.txt**: Pinned minimum versions for all Python dependencies; added `pydantic-settings>=2.5.0`, `fastapi>=0.115.0`, `uvicorn>=0.30.0`, `httpx>=0.27.0`, `pytest-asyncio>=0.24.0` | ||
| - **Pydantic v2**: Migrated `BaseSettings` import in `app/core/app/core/config.py` from `pydantic` to `pydantic_settings` package | ||
| - **Deprecation fixes**: Replaced `datetime.utcnow()` (deprecated in Python 3.12) with `datetime.now(timezone.utc)` in `mcp_adapter.py`, `app/core/security.py`, and `backend/services/chat_service.py` | ||
| - **Node.js (server/)**: Upgraded dependencies — `express@4.18→4.21`, `socket.io@4.7→4.8`, `mongoose@7→8`, `dotenv@16.0→16.4`, `jest@29.5→29.7`, `supertest@6→7`, `nodemon@2→3` | ||
| - **Node.js (server/)**: Upgraded dependencies — `express@4.18→4.21`, `socket.io@4.7→4.8`, `mongoose@7→8`, `dotenv@16.0→16.4`, `jest@29.5→29.7`, `supertest@6→7`, `nodemon@2→3` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix character encoding corruption (mojibake).
Several Markdown files contain text corrupted by a character encoding mismatch (e.g., UTF-8 bytes interpreted as Windows-1252), resulting in artifacts like —, →, 🚀, and “.
CHANGELOG.MD#L23-L28: restore proper em-dashes and right-arrows (—,→).README.md#L52-L52: fix the em-dash inmicrobenchmark—not.README.md#L72-L72: fix the rocket emoji (🚀).README.md#L101-L114: fix the Unicode box-drawing characters for the directory tree.docs/AUDIT.md#L21-L21: fix the smart quotes (“,”).
📍 Affects 3 files
CHANGELOG.MD#L23-L28(this comment)README.md#L52-L52README.md#L72-L72README.md#L101-L114docs/AUDIT.md#L21-L21
🤖 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 `@CHANGELOG.MD` around lines 23 - 28, Restore the corrupted UTF-8 characters
across the documented Markdown sites: replace mojibake em-dashes and
right-arrows in CHANGELOG.MD lines 23-28, the em-dash in README.md line 52, the
rocket emoji in README.md line 72, and the directory-tree box-drawing characters
in README.md lines 101-114; replace the corrupted quotes in docs/AUDIT.md line
21 with proper smart quotes. Preserve all surrounding text and formatting.
| redis>=5.2.0 | ||
| prometheus-client>=0.21.0 | ||
| python-jose[cryptography]>=3.3.0 | ||
| PyJWT>=2.12.0 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check PyPI for the latest version of PyJWT and query security advisories.
# Check PyPI for the latest version
curl -s https://pypi.org/pypi/PyJWT/json | jq -r '"Latest PyJWT version on PyPI: " + .info.version'
# Check for security advisories
gh api graphql -f query='
{
securityVulnerabilities(first: 5, ecosystem: PIP, package: "PyJWT") {
nodes {
advisory {
summary
severity
publishedAt
}
vulnerableVersionRange
firstPatchedVersion {
identifier
}
}
}
}'Repository: CoreyLeath-code/TrojanChat
Length of output: 1613
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== files mentioning PyJWT / PyJWKClient ==\n'
rg -n --hidden --glob '!**/.git/**' 'PyJWT|PyJWKClient|jwt\.' .
printf '\n== requirements files ==\n'
git ls-files '*requirements*.txt' '*requirements*.in' 'pyproject.toml' 'Pipfile*' 'poetry.lock' 'uv.lock' 'Pipfile.lock' 'package-lock.json' 'pnpm-lock.yaml' 'yarn.lock' | sed 's#^`#-` #'
printf '\n== requirements.txt around PyJWT ==\n'
if [ -f requirements.txt ]; then
nl -ba requirements.txt | sed -n '1,40p'
fiRepository: CoreyLeath-code/TrojanChat
Length of output: 831
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import urllib.request, json, ssl
urls = [
"https://pypi.org/pypi/PyJWT/json",
]
for url in urls:
with urllib.request.urlopen(url, context=ssl.create_default_context(), timeout=20) as r:
data = json.load(r)
print("latest:", data["info"]["version"])
PYRepository: CoreyLeath-code/TrojanChat
Length of output: 2758
Raise the PyJWT floor to a patched release
PyJWT>=2.12.0 still allows versions covered by published advisories. Bump this to >=2.13.0 (or pin a vetted release) so the dependency file can’t resolve to known vulnerable builds.
🧰 Tools
🪛 OSV Scanner (2.4.0)
[HIGH] 14-14: pyjwt 2.12.0: undefined
(PYSEC-2026-175)
[HIGH] 14-14: pyjwt 2.12.0: undefined
(PYSEC-2026-176)
[HIGH] 14-14: pyjwt 2.12.0: undefined
(PYSEC-2026-177)
[HIGH] 14-14: pyjwt 2.12.0: undefined
(PYSEC-2026-178)
[HIGH] 14-14: pyjwt 2.12.0: undefined
(PYSEC-2026-179)
[HIGH] 14-14: pyjwt 2.12.0: PyJWKClient: missing scheme allowlist enables CVE-2024-21643-class SSRF + token forgery via file://, ftp://, data: schemes
[HIGH] 14-14: pyjwt 2.12.0: PyJWKClient unbounded JWKS endpoint requests via attacker-controlled kid values (DoS)
[HIGH] 14-14: pyjwt 2.12.0: PyJWT: Algorithm allow-list bypass when decoding with PyJWK / PyJWKClient keys
[HIGH] 14-14: pyjwt 2.12.0: PyJWT: Unauthenticated DoS via unbounded Base64URL decoding of unused payload segment in b64=false detached JWS
[HIGH] 14-14: pyjwt 2.12.0: PyJWT: Public-key JWK accepted as HMAC secret enables forged HS256 tokens when mixed families are allowed
🤖 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 `@requirements.txt` at line 14, Update the PyJWT dependency constraint in
requirements.txt from >=2.12.0 to >=2.13.0, or pin it to an equivalent vetted
patched release, so vulnerable versions cannot be resolved.
Source: Linters/SAST tools
| def load_security(monkeypatch): | ||
| monkeypatch.setenv("SECRET_KEY", "test-only-secret-that-is-at-least-32-bytes") | ||
| sys.modules.pop("app.core.security", None) | ||
| return importlib.import_module("app.core.security") |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use monkeypatch.delitem to prevent cross-test state leakage.
Using sys.modules.pop permanently modifies the test runner's sys.modules. Any tests running after this one in the same process will inherit the reloaded module containing the test-only secret. By using monkeypatch.delitem, pytest will automatically restore the original module during teardown.
🐛 Proposed fix
def load_security(monkeypatch):
monkeypatch.setenv("SECRET_KEY", "test-only-secret-that-is-at-least-32-bytes")
- sys.modules.pop("app.core.security", None)
+ monkeypatch.delitem(sys.modules, "app.core.security", raising=False)
return importlib.import_module("app.core.security")📝 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.
| def load_security(monkeypatch): | |
| monkeypatch.setenv("SECRET_KEY", "test-only-secret-that-is-at-least-32-bytes") | |
| sys.modules.pop("app.core.security", None) | |
| return importlib.import_module("app.core.security") | |
| def load_security(monkeypatch): | |
| monkeypatch.setenv("SECRET_KEY", "test-only-secret-that-is-at-least-32-bytes") | |
| monkeypatch.delitem(sys.modules, "app.core.security", raising=False) | |
| return importlib.import_module("app.core.security") |
🤖 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 `@tests/test_security.py` around lines 7 - 10, Update the load_security fixture
to remove app.core.security from sys.modules via monkeypatch.delitem instead of
sys.modules.pop, ensuring pytest restores the original module after teardown
while preserving the existing reload behavior.
1 participant
Executive summary
This PR converts TrojanChat's quality and performance claims into reproducible, blocking engineering evidence while hardening the canonical
backend.api:apppath.Product and architecture changes
CHAT_HISTORY_LIMITand thread-safe retentionpython-jose/vulnerable transitiveecdsawith PyJWT for the existing HS256 flowTests and coverage
Research benchmark
A versioned harness compares identical 50,000-message workloads over seven iterations and records mean, median, P95, P99, min/max latency, throughput, peak Python memory, parameters, and environment.
The throughput tradeoff stays inside the documented 15% regression budget. Raw JSON and methodology are committed; CI regenerates the artifact.
CI, static analysis, and security
Documentation and release readiness
Local verification
Rollout considerations
The store remains process-local and intentionally bounded; multi-instance production requires a durable shared store. The WebSocket endpoint still requires authentication, origin enforcement, rate limiting, and TLS termination before public exposure. These residual risks are explicit in
docs/AUDIT.mdanddocs/PRODUCTION_CHECKLIST.md.Summary by CodeRabbit
New Features
Bug Fixes
Security
Documentation