fix(security): resolve all 40 open CodeQL alerts + check in 2 untracked files - #54
Merged
Conversation
… script from design exploration
…-injection) - pipeline._safe_resolve_path: simplify guard to a single startswith check so static analysis can prove containment (behavior unchanged for all callers, which always pass non-empty filenames) - llm._resolve_image_for_vision: resolve candidates via realpath and only accept files inside the managed output/ tree - media_refs: replace Path.resolve() checks with realpath + prefix guard in _is_under and resolve_local_media_path - oss_utils.upload_file: restrict uploads to output/ and the system temp dir; callers fall back gracefully (data URI / local path) when refused - api: sanitize uploaded file extensions through an allow-list map before embedding them in server-side paths (4 upload endpoints)
…nd-line-injection) All six flagged subprocess.run(ffmpeg) calls already used list arguments (no shell=True); the remaining taint entered through extract_last_frame's legacy absolute-path branch which bypassed _safe_resolve_path. That branch now realpath-resolves and requires containment in the managed output/ tree. Adds tests/test_ffmpeg_path_safety.py pinning both directions: - _safe_resolve_path accepts simple and multi-level relative paths, rejects ../ traversal, absolute escapes, and mixed traversal - extract_last_frame rejects /etc/passwd style and ../ style video_url - happy-path regression with a real ffmpeg run (lavfi-generated clip) proving frame extraction still works end-to-end
…QL py/stack-trace-exposure) - pipeline.refine_batch_generator: SSE frame_refine_error event now sends a generic message with the exception class name; full details remain in the server log - system_check: ffmpeg health-check failure message no longer embeds str(e); details are logged server-side instead
…rts)
- getAssetUrl: validate http/blob inputs with new URL() and an explicit
protocol allow-list (http/https/blob) so javascript:-style values can
never reach img/video src; local paths are encodeURI'd (covers 3
js/xss-through-dom alerts through one shared barrier)
- playground size labels: replace first-match-only .replace('*'/'x')
chains with a single global regex /[*x]/g (3 js/incomplete-sanitization
alerts; output unchanged for well-formed WxH size strings)
- backend-ci.yml: add top-level 'permissions: contents: read' so the GITHUB_TOKEN gets only what the job needs (actions/missing-workflow-permissions) - dev-setup.js: install Python deps via execFileSync with an argument list instead of interpolating the pip path into a shell string (js/shell-command-injection-from-environment)
|
|
EthanDuan05
pushed a commit
to EthanDuan05/lumenx
that referenced
this pull request
Aug 14, 2026
…URLs Round 2 for the 9 alerts CodeQL kept open after PR alibaba#54: - py/command-line-injection x6: build ffmpeg output filenames from store-backed frame.id/script.id instead of request parameters, and coerce offset_ms to int before it enters the adelay filter string. Runtime values are identical (ids matched by lookup), so behavior is unchanged; the request-parameter taint chain is simply broken. - js/xss-through-dom x3: strip HTML metacharacters from pass-through http/blob URLs in getAssetUrl (no-op for well-formed URLs).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Resolves all 40 open CodeQL alerts on main, grouped into atomic commits per rule for independent revert if needed. Also checks in 2 previously untracked files from design exploration.
Commits (one batch per rule)
Fix strategy (no behavior change for legitimate inputs)
_safe_resolve_pathhelper (realpath + single provablestartswithguard); multi-level legit paths likeassets/characters/x.pngstill pass (pinned by tests).oss_utils.upload_fileis fenced tooutput/+ system temp dir — callers degrade gracefully to data-URI/local (pinned by existingtest_wan26_reference_local_without_oss_uses_data_uri). Upload endpoints sanitize file extensions through a dict allow-list returning literals.shell=True); the remaining taint entered viaextract_last_frame's legacy absolute-path branch, now contained tooutput/. Newtests/test_ffmpeg_path_safety.pyincludes a real ffmpeg end-to-end run (legit path extracts a frame;/etc/passwdand../attempts rejected).type(exc).__name__), full details stay in server logs.getAssetUrl(new URLprotocol allow-list http/https/blob +encodeURI) covers all 3 sinks..replacechains → global regex/[*x]/g.contents: read.execFileSyncwith arg list in dev-setup.js.Verification
pytest: 209 passed (incl. 8 new regression tests, real-ffmpeg happy path verified locally)