Skip to content

fix(research): 拒绝报告 ID 路径穿越输入 - #111

Open
Bluuok wants to merge 1 commit into
helsome:mainfrom
Bluuok:fix/research-report-id-boundary
Open

Bluuok wants to merge 1 commit into
helsome:mainfrom
Bluuok:fix/research-report-id-boundary

Conversation

@Bluuok

@Bluuok Bluuok commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

改动说明

What

  • ResearchReportRepository 的报告文件路径边界校验 report ID,只允许 ASCII 字母、数字、_-
  • 让非法 ID 在读报告、保存报告和由索引加载报告时 fail closed,并验证不会读取或覆盖报告目录之外的 JSON 文件。
  • 补充路径形式、NUL、非法索引、写队列恢复等测试。

Why

报告 ID 会参与拼接 research/reports/<id>.json。如果外部输入包含路径分隔符、盘符或其他路径语法,未经约束就可能改变目标路径;这是持久化边界应在 repository 层拒绝的输入。

关联 Issue

未发现对应的直接 Issue。本 PR 是独立的研究报告持久化输入边界修复。

测试报告(正式审核前必填)

环境

  • Bun:1.4.0
  • OS:Microsoft Windows 11 家庭版 中文版

实际执行命令与结果

bun test --isolate packages/shared/src/research/repository.test.ts packages/shared/src/research/repository.report-id.test.ts
→ 8 pass / 0 fail,22 expect() calls

bun test packages/shared --isolate
→ 914 pass / 0 fail,86 files,3582 expect() calls

bun run typecheck
→ core / i18n / shared / ui / electron 全部 exit 0

git diff --check
→ exit 0,无输出

已知失败 / Baseline(如有)

  • 无已知失败;以上全量共享包测试在本分支通过。

  • 仓库 CI 固定使用 Bun 1.4.2;本地实际使用 Bun 1.4.0,Bun 1.4.2 的 CI 结果待 GitHub Actions 复核。

  • 已提供实际测试命令与 pass/fail 结果

  • 已说明测试环境

  • 如果存在已知 baseline / 环境失败,已提供 main 对照或说明

  • 核心改动已有对应 focused test / smoke / integration 验证

UI 截图(仅可见 UI 变化时必填)

  • 本 PR 无可见 UI 变化(无需截图)

Scope / 后续

  • 允许集合与当前生成的 report-<runId> ID 约定一致;本 PR 不迁移历史文件、不改变报告 schema,也不改变外部 API 的错误码语义之外的行为。
  • 报告 repository 是最终路径边界;未来新增 report ID 来源仍需遵守同一约束。

@helsome helsome left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Draft 阶段已看过 diff 和测试材料:报告 ID 在读/写及索引消费时统一校验,目录穿越/非法 ID/合法历史 ID/失败后队列恢复均有实际文件系统 focused 覆盖,这是值得独立合入的安全修复。当前没有发现需要额外架构改动的 blocker;无需真实模型或外部数据源,此处安全边界是本地路径和持久化,不是 Provider。保持无可见 UI 变化声明即可。尊重 Draft,等作者标 Ready 后再做最终 approve/merge。

@Bluuok
Bluuok marked this pull request as ready for review September 17, 2026 05:38
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