Skip to content

fix(security): 路径包含性与对外错误面各收敛到一处(CodeQL 47 条真修) - #155

Merged
ReSerendipity merged 1 commit into
mainfrom
fix/codeql-path-injection-unified
Sep 23, 2026
Merged

ReSerendipity merged 1 commit into
mainfrom
fix/codeql-path-injection-unified

Conversation

@ReSerendipity

Copy link
Copy Markdown
Owner

背景

第二阶段的开工基线写的是「open 30(path-injection 12 + stack-trace 18)」,按 §0 命令实测 origin/main@2433c10 是 open 54(path-injection 36 + stack-trace 18)/ dismissed 48 / fixed 10。差额 24 条 = §7.2 判过但没执行的那批(19 条「已缓解」+ 5 条 settings.py 真修)。54 条 open 的 created_at/updated_at 全等于 2026-09-10T03:38:29Z,不是重扫新开;09-23 16:24 那次扫描确实生效(把 #107/#108 翻成 fixed)。逐条口径写在 docs/SECURITY_CODEQL_TRIAGE.md §8.0。

读的过程中挖到的真缺陷(不是纸面问题)

persona_manager 三处守卫写的是 realpath(p).startswith(realpath(PERSONA_DIR)),少了 + os.sep:PERSONA_DIR=.../personas 时 .../personas_evil/trap.wav 同样以 personas 开头,判定放行。而 load_persona_embedding 的入口守卫只有这一条(没有白名单),传 name="../personas_evil/trap" 就是实际发生的越界读 —— wav_exists 用的 os.path.join(PERSONA_DIR, name) 会真的解析到兄弟目录。

改了什么(两处统一,不在每处再写一遍 basename)

  1. app/integrated_app/path_guard.py(新增):is_bare_filename(单段裸名:无分隔符/控制字符/隐藏/./../长度上限)+ ensure_within_dir(真实路径 + os.sep 严格前缀)+ resolve_bare_in_dir。接入 generation.preprocess_and_save_temp、persona_manager 固化/嵌入加载/删除、persona_metadata 读写、resolve_persona_ref、voxcpm2/design、voxcpm2/script、routes/persona._resolve_generated_audio。training._validate_path 本来就带 os.sep,不动,作对照。
  2. app/integrated_app/error_surface.py(新增):把 routes/model.py 私有的 _safe_error_message 提为共用的 safe_error_message / redact_paths。接入 settings.py 7 处 str(exc)(CodeQL 标 5 处,同形的 HTTPException(detail=…) 639/702 一并改)、routes/persona.py:325、training.py:538 与 :413(后者不是异常但把绝对路径写进了响应)、persona_manager 两条用户可见消息、_safe_error_msg 六条分支、_error_html 渲染入口(一处管住 toast 头 / 模板 / 内联降级三个出口)。
    顺带修 model.py 里脱敏晚一行:_notify_load(last_msg, …) 把未脱敏原文写进进度状态,客户端轮询仍拿得到路径。

判据与逐条台账

docs/SECURITY_CODEQL_TRIAGE.md §8:54 行逐条表(告警号 / 规则 / 文件:行 / 落点函数 / 判定 / 依据组),合计 真修 47|误报 4|已缓解 2|记录不修 1。§8.5 同时更正了我上一版的一句错话:transformers 4.52.x 那 16 条公告的豁免与逐条判定早就在 security.yml + SECURITY_DEPENDABOT_TRIAGE.md §1/§1a/§2/§4,并由 tests/test_dependency_consistency.py D3/D4 钉住 —— 我之前写成「已知未结」是因为没翻 security.yml。本 PR 不新增也不放宽任何豁免。

测试

新增 tests/test_path_guard.py(含 test_old_prefix_check_is_the_bug:断言旧写法确实放行,新写法拒绝)、tests/test_error_surface_leaks.py(settings 三端点 + persona 固化/删除消息 + _error_html 渲染出口,断言异常响应里不含路径、文件名仍保留);扩写 test_persona_embedding_load.py(兄弟目录那条)与 test_error_message_redaction.py(改指向共用模块)。

本地实测:ruff check/format 全绿;pytest -m "not gpu and not cuda and not integration and not benchmark and not e2e" → 2114 passed / 12 skipped;mypy 棘轮 103 → 103 不变。

未覆盖 / 需要知道的代价

  • 行为收紧的三处:resolve_persona_ref、voxcpm2/design 对非裸名音色返回 400(以前会被 basename 静默改写后继续查);voxcpm2/script 对非法角色名按既有局部降级策略跳过并 warning;preprocess_and_save_temp 对非法临时文件名抛 ValidationError。正常音色名走 _PERSONA_NAME_RE 白名单落盘,不受影响;手工丢进 personas/ 的带分隔符或点开头文件名会开始报错,这是有意的。
  • CodeQL 不认自定义净化器,合并后这一族的数字不会自己掉:47 条要按 mitigated 逐条 dismiss,理由绑定本 PR 的合并 SHA + 具体守卫行 + 测试名(model.py 那 11 条在 fix(security): 错误消息脱敏补上四条领域异常分支 #96 合并后仍是 open,是同一件事的先例)。
  • 真机(起服务点界面)验收未做 —— 本轮只有 pytest 与静态门禁;涉及上面三处行为收紧的路径需要人跑一遍克隆/设计/剧本工坊。

Refs: #97 #99 #153 #154

CC @ReSerendipity

CodeQL 54 条 open 逐条读完落点后落地其中 47 条真修。

两处统一:
- path_guard:裸文件名 + 真实路径严格前缀(含 os.sep)。读到的旧守卫
  startswith(realpath(PERSONA_DIR)) 少了 os.sep,兄弟目录 personas_evil
  会以 personas 前缀通过判定 —— load_persona_embedding 入口只有这一条
  防线(无白名单),因此是可用的越界读,不是纸面问题。
- error_surface:把 routes/model.py 私有的 _safe_error_message 提为共用,
  settings.py 7 处 str(exc)、training 启动失败与脚本绝对路径、persona 固化
  与删除的两条用户可见消息、_safe_error_msg 六条分支、_error_html 渲染入口
  各接上。顺带修 model.py 里脱敏晚一行的问题:_notify_load 把未脱敏原文
  写进进度状态,客户端轮询仍拿得到路径。

CodeQL 不认自定义净化器,所以这一族的数字要靠绑定证据的逐条 dismiss 收口
(判据见 docs/SECURITY_CODEQL_TRIAGE.md §8)。

新增 tests/test_path_guard.py、tests/test_error_surface_leaks.py,扩写
test_persona_embedding_load.py 与 test_error_message_redaction.py。
本地:ruff 全绿;pytest 2114 passed / 12 skipped;mypy 棘轮 103 不变。

Signed-off-by: ReSerendipity <ReSerendipity@users.noreply.github.com>
@ReSerendipity
ReSerendipity enabled auto-merge (squash) September 23, 2026 18:06
Comment thread app/integrated_app/routes/model.py Dismissed
Comment thread app/integrated_app/routes/model.py Dismissed
Comment thread app/integrated_app/routes/model.py Dismissed
Comment thread app/integrated_app/routes/model.py Dismissed
Comment thread app/integrated_app/routes/model.py Dismissed
Comment thread app/integrated_app/routes/system/settings.py Dismissed
Comment thread app/integrated_app/routes/system/settings.py Dismissed
Comment thread app/integrated_app/routes/system/settings.py Dismissed
Comment thread app/integrated_app/routes/system/settings.py Dismissed
Comment thread app/integrated_app/routes/training.py Dismissed
@ReSerendipity
ReSerendipity merged commit d875c9c into main Sep 23, 2026
31 checks passed
ReSerendipity added a commit that referenced this pull request Sep 23, 2026
重扫确认(run 35901422189 / headSha=d875c9c / success):
- py/path-injection 36 → 0(35 判 fixed + #14 按 mitigated 交差);
- py/stack-trace-exposure 7 判 fixed,但同位置重生 16 条新号 #114–#129
  —— 其中 9 条就是 §7.1 那批已 dismiss 的 model.py 调用点,被我删函数的
  行号推移顶了出来。逐条读过落点后全部按 mitigated 绑定 d875c9c 交差。

结论写进 §8.6:dismiss 绑的是告警号而不是代码事实,所以动这两族文件的重扫
成本是"再读一遍、再逐条交差一遍"。另记一次统计口径的坑:
state=fixed 的 --paginate 在结果集变动时会重复返回记录,去重后是
total 128 = dismissed 76 + fixed 52 + open 0。

新增 tests/test_security_surface_http.py(10 条)经真实 ASGI 栈复验 #155
三处行为收紧。第一版漏了先 GET / 取 csrf_token,写请求全以 CSRF 403 结束、
5 条断言"全绿"却没打到业务分支 —— 加正向对照才暴露,这个反例一并写进注释。

本地:pytest 2124 passed / 12 skipped;ruff check/format 全绿。

Signed-off-by: ReSerendipity <ReSerendipity@users.noreply.github.com>
Co-authored-by: ReSerendipity <ReSerendipity@users.noreply.github.com>
ReSerendipity added a commit that referenced this pull request Sep 24, 2026
delete_branch_on_merge=false 是仓库级显式设置,删分支等于替维护者改掉这条策略,
所以 #155/#156 的两个分支留在 origin,以后默认保留、要删单独取授权。
resolve_persona_ref 与 voxcpm2 两条路由的音色解析仍空着:不为补测试自行
下载权重或加载引擎(3-4GB 与 GPU 都在红线上),这三格只能用户侧真机收口。
另记 A1-A16 的跟踪载体待定:pip-audit 无 expiration 机制、Dependabot 只覆盖
A1-A8,汇总 issue 草稿已交、批复前保持只有文档没有 issue。

Signed-off-by: ReSerendipity <ReSerendipity@users.noreply.github.com>
Co-authored-by: ReSerendipity <ReSerendipity@users.noreply.github.com>
@ReSerendipity
ReSerendipity deleted the fix/codeql-path-injection-unified branch September 24, 2026 11:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants