Repository navigation
Fix/security hardening - #3877
8079-debug wants to merge 3 commits into
Conversation
LauraGPT
left a comment
There was a problem hiding this comment.
Changes requested for the upload cleanup path in runtime/python/websocket/funASR-Service.py.
speech_recognition_Timestamp_cam_identify_speakers() saves the upload before parsing/validating the form. The new malformed-JSON except returns 400 before reaching the inference cleanup, and the empty/invalid speaker database 400 path also returns with the saved file still present. With the new UUID filenames, rejected requests leave separate files behind instead of cleaning up the upload.
Independently reproduced against exact head bb2e520acadd6484f509c6ec5b6045de0a597b55, file blob 9a4a00df893ccaa306e868d22ba5556ba3ab3ea2, using real Flask/Werkzeug multipart requests and a private filesystem:
identify_speakers=not-json,speaker_db={}: 400, one uploaded file remains.identify_speakers=false,speaker_db={: 400, one uploaded file remains.identify_speakers=true,speaker_db={}: 400, one uploaded file remains.- Valid
false/{}with an empty inference result: 200 and the upload is removed (passing control).
Each fixture was only 22 bytes. Cleanup assertions give 3 failed, 1 passed; no model inference is reached in the rejected cases. Environment: Python 3.12.3, Flask 3.1.2, Werkzeug 3.1.9, pytest 8.1.1. ASR/speaker model construction and inference were replaced at their boundary; Flask, multipart parsing and file operations were real. This is a scoped HTTP/filesystem result, not GPU/ASR validation or acceptance of all 17 changed files. The initial missing-Flask setup errors were retained separately and are not counted as this reproduction.
Please validate all form fields before persisting the upload, or place persistence, validation and inference under a cleanup finally that covers every return/exception. Add permanent route-level regressions for the rejected cases above plus a successful control. The pre-existing empty-database early return should be covered by the same cleanup rather than left outside it. Other changes in this PR still need their own relevant tests before merge.
Summary
Replaces unsafe dynamic-code parsing (
eval,yaml.Loader) with safe alternatives and hardens the runtime HTTP/WebSocket/Flask services against malformed client input and resource exhaustion. All changes are behavior-preserving for valid inputs.eval()on dataset/config/manifest strings →ast.literal_eval; data2vec conv-layer expressions use a restricted AST parser (literals and+/*only)yaml.load(Loader=yaml.Loader)→yaml.safe_loadin 6 runtime config/vocab loaders/tmppaths, collision-safe upload names, empty-result and form-JSON guards--max_upload_mb, default 100, returns 413), temp-file cleanup infinally, extension sanitization, empty-result guard, inference offloaded to a thread poolchunk_intervalclamped to ≥1 (division by zero),chunk_sizetype validation without session teardown,wav_namepath-traversal sanitization including Windows separatorsmkdtempinstead of a GC-reclaimedTemporaryDirectory().name, URL-path basename for saved filenamesType of change
Validation
python -m compileall funasr examples tests(exit 0)Runtime/deployment command tested
pytest tests/test_data2vec_compute_var.py tests/test_websocket_file_finalization.py tests/test_release_runtime_downloads.py tests/test_runtime_sdk_download_tool.py→ 56 passed20 additional behavior checks: malicious expressions and YAML tags are rejected; valid model configs, bytes literals and fairseq-style conv expressions still parse correctly
tests/test_training_docs_contract.pyhas 3 failures verified identical on a clean baseline worktree (pre-existing, unrelated)User impact
Production deployers of the runtime services: no debug-console exposure, no disk/memory exhaustion via uploads, no session crashes from malformed config messages. Toolkit users: parsing of dataset/manifest/config files can no longer execute arbitrary code.
Notes for reviewers
kaldi_data.pyandtorch.loadcheckpoint loading are intentionally left unchanged.