Commit 3dee6cb
committed
Dedupe backend/core into eval_mcp/core; harden path handling against CodeQL py/path-injection
Finishes the dedupe started in the previous commit. 11 modules in
backend/core/ were byte-identical or only-import-line copies of
eval_mcp/core/ equivalents (user_storage, eval_results, s3_client,
pricing, bedrock_client, document_chunking, judge_config, logging_utils,
pdf_report, pipeline_stages, provider_pricing.json). Deleted the backend
copies, rewired ~18 imports in backend/api/*, backend/core/agent.py,
backend/core/chat.py, backend/core/judge_prompt_builder.py, and
backend/core/judge_tools.py to pull from eval_mcp.core.*. One-way
dependency (backend -> eval_mcp) is now consistent across the whole
tree; eval_mcp is the sole source of truth.
Path-injection hardening in the consolidated eval_mcp/core:
- New safe_user_path(user_id, *parts) helper in eval_mcp/core/user_storage.py:
resolves the target and verifies it stays under both USER_STORAGE_BASE
and {base}/{user_id}, rejects user_ids containing '/', '\\', or '..',
rejects path parts that try to escape via traversal. This is the pattern
CodeQL's py/path-injection rule recognizes as safe.
- New _ensure_under_base(path) helper for Path objects assembled outside
the helper (used by _load_json_file, _save_json_file, and the two
configs-dir reads in eval_results.py).
- _get_json_store_dir now validates store_type is a single path segment.
- eval_mcp/s3_sync.py: replicate_async and _is_syncable now use
Path.resolve() + is_relative_to() instead of relative_to+ValueError.
- backend/api/compare.py report-save path routed through safe_user_path.
- eval_mcp/viewer.py report-download path routed through safe_user_path.
Behavior-neutral for existing EKS storage: Cognito user IDs are UUIDs
with only hex+hyphen characters, all directory names in the layout are
hardcoded strings ('configs', 'logs', 'store', etc.), and S3 key
construction uses plain strings unaffected by filesystem hardening.
Test plan:
- Unit-tested safe_user_path: accepts valid user/parts; rejects '', '.',
'..', '../secret', 'alice/../bob', 'alice/bob', 'alice\\\\bob',
and part-based escape attempts like ['..', '..', 'etc', 'passwd'].
- Smoke-imported backend.api.main, backend.api.compare, eval_mcp.server
in repo venv and inside the built backend Docker image.
- Booted 'python -m eval_mcp' with EVAL_MCP_TRANSPORT=http in the
container; uvicorn listens on :8002 (what the helm sidecar does).1 parent 99e780c commit 3dee6cb
21 files changed
Lines changed: 95 additions & 3621 deletions
File tree
- backend
- api
- core
- eval_mcp
- core
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
10 | 10 | | |
11 | 11 | | |
12 | 12 | | |
13 | | - | |
14 | | - | |
| 13 | + | |
| 14 | + | |
15 | 15 | | |
16 | 16 | | |
17 | 17 | | |
| |||
32 | 32 | | |
33 | 33 | | |
34 | 34 | | |
35 | | - | |
| 35 | + | |
36 | 36 | | |
37 | 37 | | |
38 | 38 | | |
| |||
63 | 63 | | |
64 | 64 | | |
65 | 65 | | |
66 | | - | |
| 66 | + | |
67 | 67 | | |
68 | 68 | | |
69 | 69 | | |
| |||
189 | 189 | | |
190 | 190 | | |
191 | 191 | | |
192 | | - | |
193 | | - | |
| 192 | + | |
| 193 | + | |
194 | 194 | | |
195 | 195 | | |
196 | 196 | | |
| |||
288 | 288 | | |
289 | 289 | | |
290 | 290 | | |
291 | | - | |
292 | | - | |
| 291 | + | |
| 292 | + | |
293 | 293 | | |
294 | 294 | | |
295 | 295 | | |
| |||
320 | 320 | | |
321 | 321 | | |
322 | 322 | | |
323 | | - | |
324 | | - | |
| 323 | + | |
| 324 | + | |
325 | 325 | | |
326 | 326 | | |
327 | 327 | | |
| |||
335 | 335 | | |
336 | 336 | | |
337 | 337 | | |
338 | | - | |
339 | | - | |
| 338 | + | |
| 339 | + | |
| 340 | + | |
340 | 341 | | |
341 | 342 | | |
342 | 343 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
17 | 17 | | |
18 | 18 | | |
19 | 19 | | |
20 | | - | |
| 20 | + | |
21 | 21 | | |
22 | 22 | | |
23 | | - | |
| 23 | + | |
24 | 24 | | |
25 | 25 | | |
26 | 26 | | |
27 | 27 | | |
28 | 28 | | |
29 | | - | |
| 29 | + | |
30 | 30 | | |
31 | 31 | | |
32 | 32 | | |
| |||
455 | 455 | | |
456 | 456 | | |
457 | 457 | | |
458 | | - | |
| 458 | + | |
459 | 459 | | |
460 | 460 | | |
461 | 461 | | |
| |||
1261 | 1261 | | |
1262 | 1262 | | |
1263 | 1263 | | |
1264 | | - | |
| 1264 | + | |
1265 | 1265 | | |
1266 | 1266 | | |
1267 | 1267 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
6 | 6 | | |
7 | 7 | | |
8 | 8 | | |
9 | | - | |
| 9 | + | |
10 | 10 | | |
11 | 11 | | |
12 | 12 | | |
| |||
0 commit comments