Skip to content

Commit a6ef270

Browse files
fix: strengthen exception-leak static guard; lift safe_join to module import in export_api
1 parent 2eebda6 commit a6ef270

5 files changed

Lines changed: 37 additions & 21 deletions

File tree

CONTRIBUTING.md

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ Thanks for considering a patch. This repo is a small Flask app plus a hash-route
99
- **Python 3.12** (matches CI)
1010
- **Node 20+** (only if you change `static/js/` or run frontend unit tests)
1111

12-
CI runs **`pytest`**, **integration tests**, and **Vitest** on **ubuntu-latest** and **windows-latest** (Python 3.12, Node 20). Type-check (`mypy`) and production install smoke run on Ubuntu only.
12+
CI runs **`ruff check`**, **`ruff format --check`**, **`pip-audit`**, **`pytest`**, **integration tests**, and **Vitest** on **ubuntu-latest** and **windows-latest** (Python 3.12, Node 20). Type-check (`mypy`) and production install smoke run on Ubuntu only.
1313

1414
### Bootstrap (Windows PowerShell)
1515

@@ -58,6 +58,9 @@ When changing JSON response shapes, update the API reference stability column an
5858
### Python
5959

6060
```bash
61+
ruff check . # lint (E, F, W, I) — same gate as CI
62+
ruff format --check . # formatting gate; run `ruff format .` to fix
63+
pip-audit -r requirements.txt # production dependency audit (CI gate)
6164
pytest -q # full suite + coverage (see pyproject.toml)
6265
pytest tests/test_api_integration.py -v
6366
pytest tests/test_search.py -v
@@ -85,7 +88,8 @@ npm run test:coverage # optional
8588
| **Exception leakage** | `5xx` bodies are generic messages only. Log full tracebacks with `current_app.logger.exception(...)`. Never put `str(e)` or class names in HTTP JSON (issue #25). |
8689
| **Path safety** | Use `safe_join()` from `utils/session_path.py` for any path built from URL segments. |
8790
| **Imports** | stdlib → third-party → local, blank line between groups. |
88-
| **Line length** | ~100 characters; no enforced formatter yet. |
91+
| **Lint / format** | `ruff check .` and `ruff format --check .` (CI gates). Config in `pyproject.toml`; run `ruff format .` to apply formatting locally. |
92+
| **Line length** | 100 characters (`line-length` in `pyproject.toml`). |
8993

9094
## Tests required for common changes
9195

@@ -105,9 +109,10 @@ npm run test:coverage # optional
105109
- Branch names: `feat/<topic>`, `fix/<topic>`, `test/<topic>`, `chore/<topic>`, `docs/<topic>`.
106110
- One logical change per PR when possible.
107111
- PR checklist:
112+
- [ ] `ruff check .` and `ruff format --check .` green locally
108113
- [ ] `pytest -q` green locally
109114
- [ ] `npm test` green if JS changed
110-
- [ ] CI jobs green (`pytest`, `integration-tests`, `js-tests` on Ubuntu + Windows; `mypy`, `prod-install-smoke` on Ubuntu)
115+
- [ ] CI jobs green (`lint-and-audit`, `pytest`, `integration-tests`, `js-tests` on Ubuntu + Windows; `mypy`, `prod-install-smoke` on Ubuntu)
111116
- [ ] PR description includes a **Test plan** section
112117
- [ ] API changes update [`docs/api-reference.md`](docs/api-reference.md) if behavior or errors change
113118

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ npm ci && npm test # only if you changed static/js/
157157

158158
## Continuous integration
159159

160-
Every push and pull request runs **`ruff`**, **`pip-audit`**, **`pytest`**, **mypy**, **API integration tests**, and **vitest** on **Ubuntu and Windows** (Python 3.12, Node 20) via [`.github/workflows/ci.yml`](.github/workflows/ci.yml). A separate job verifies that `pip install -r requirements.txt` (production-only) is sufficient to import and boot the app.
160+
Every push and pull request runs **`ruff check`**, **`ruff format --check`**, **`pip-audit`**, **`pytest`**, **mypy**, **API integration tests**, and **vitest** on **Ubuntu and Windows** (Python 3.12, Node 20) via [`.github/workflows/ci.yml`](.github/workflows/ci.yml). A separate job verifies that `pip install -r requirements.txt` (production-only) is sufficient to import and boot the app.
161161

162162
## Exported Markdown Format
163163

SECURITY.md

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,11 @@ This project is pre-release. Security fixes are applied to the **latest `master`
1313

1414
**Please do not open public GitHub issues for security vulnerabilities.**
1515

16-
Report vulnerabilities privately via [GitHub Security Advisories](https://github.com/cppalliance/claude-code-chat-browser/security/advisories/new). Private vulnerability reporting must be enabled on the repository (Settings → Security → Private vulnerability reporting). If you cannot use that form, contact the repository maintainers through an existing private channel.
16+
**Primary path (always works):** Contact a [repository maintainer](https://github.com/cppalliance/claude-code-chat-browser/graphs/contributors) through a private channel you already use with the project (for example a direct message or private email). Include steps to reproduce, affected version/commit, and impact.
17+
18+
**GitHub Security Advisories (when enabled):** Once **Private vulnerability reporting** is turned on for this repository (Settings → Security → Private vulnerability reporting), external researchers may use the [private advisory form](https://github.com/cppalliance/claude-code-chat-browser/security/advisories/new). If that link returns 404 or the form is unavailable, use the primary path above — the advisory URL only works when the setting is enabled.
19+
20+
**Repository admins:** Enable private vulnerability reporting before merge if you want the advisory form to be the default reporter path; until then, maintainers should treat the primary path as authoritative.
1721

1822
## Response Timeline
1923

api/export_api.py

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
from utils.json_exporter import session_to_json
2323
from utils.jsonl_parser import parse_session
2424
from utils.md_exporter import session_to_markdown
25-
from utils.session_path import get_claude_projects_dir, list_projects
25+
from utils.session_path import get_claude_projects_dir, list_projects, safe_join
2626
from utils.session_stats import compute_stats
2727
from utils.slugify import slugify
2828

@@ -155,8 +155,6 @@ def _on_export_error(sid: str, exc: Exception) -> None:
155155

156156
@export_bp.route("/api/export/session/<path:project_name>/<session_id>")
157157
def export_session(project_name: str, session_id: str) -> FlaskReturn:
158-
from utils.session_path import safe_join
159-
160158
base = current_app.config.get("CLAUDE_PROJECTS_DIR") or get_claude_projects_dir()
161159
try:
162160
filepath = safe_join(base, project_name, f"{session_id}.jsonl")

tests/test_error_propagation.py

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ class names from a defensive blocklist.
2121
from __future__ import annotations
2222

2323
import json
24+
import re
2425
import sys
2526
from pathlib import Path
2627

@@ -210,21 +211,29 @@ def _boom(*args, **kwargs):
210211

211212

212213
class TestNoExceptionInterpolationInSource:
213-
"""Static guard: any future PR that re-introduces the
214-
`f"...{type(e).__name__}: {e}..."` pattern in api/ fails this test."""
214+
"""Static guard: any future PR that re-introduces exception interpolation
215+
in api/ response bodies fails this test.
216+
217+
Patterns caught:
218+
- type(e).__name__ — explicit class-name expose
219+
- {e} with any common — f-string that embeds the exception value directly
220+
trailing character (closing quote, comma, paren, space, closing brace)
221+
- {str(e)} / {repr(e)} — wrapped but still leaks message content
222+
"""
223+
224+
_LEAK_RE = re.compile(
225+
r"type\(e\)\.__name__" # explicit class name
226+
r"|\{e[\"',)\s}]" # {e} followed by: quote, comma, paren, space, closing brace
227+
r"|\{str\(e\)" # {str(e)} — still leaks message
228+
r"|\{repr\(e\)", # {repr(e)} — still leaks message
229+
)
215230

216231
def test_api_files_dont_interpolate_exception_in_jsonify(self):
217232
api_dir = REPO_ROOT / "api"
218233
for py_file in api_dir.glob("*.py"):
219234
src = py_file.read_text(encoding="utf-8")
220-
# Look for the specific footgun: jsonify(...) with f-string that
221-
# contains both `type(e)` or `{e}` AND the word "error".
222-
offending_patterns = [
223-
"type(e).__name__", # the class-name expose
224-
'{e}"', # bare {e} ending an f-string
225-
"{e},", # bare {e} in a dict-value f-string
226-
]
227-
for pat in offending_patterns:
228-
assert pat not in src, (
229-
f"{py_file.name} contains forbidden pattern {pat!r} — see issue #25"
230-
)
235+
m = self._LEAK_RE.search(src)
236+
assert m is None, (
237+
f"{py_file.name} contains forbidden exception-interpolation pattern "
238+
f"{m.group(0)!r} at position {m.start()} — see issue #25"
239+
)

0 commit comments

Comments
 (0)