Skip to content

security: pipeline API 路径沙箱化 (Fixes #17) - #30

Open
FenjuFu wants to merge 2 commits into
OpenRare2026:devfrom
FenjuFu:security/sandbox-pipeline-paths
Open

FenjuFu wants to merge 2 commits into
OpenRare2026:devfrom
FenjuFu:security/sandbox-pipeline-paths

Conversation

@FenjuFu

@FenjuFu FenjuFu commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Fixes #17

先说复现

Issue 里判断「不存在 shell 注入,但可指定任意可执行文件 + 任意路径」是准的。实际验证下来,还有一条 issue 没点到的链路,危害更直接:

output_dir 由调用方指定且未经校验,而 /jobs/{job_id}/files 和 /jobs/{job_id}/files/{path} 是相对 status.json 里记录的 output_dir 提供文件的。safe_job_file() 确实做了 relative_to(root) 防穿越 —— 但那个 root 本身就是攻击者给的。于是:

POST /run  {"input_vcf": "...", "output_dir": "<任意目录>", "dry_run": true}
GET  /jobs/<id>/files              → 列出该目录
GET  /jobs/<id>/files/secret.txt   → 拿到文件内容

在修复前的代码上实测:三步分别返回 202 / 200(列出了目录内容)/ 200 + 文件明文。dry_run: true 即可,完全不需要真的跑 pipeline。

同一次验证还确认了:--java-bin /tmp/evil.sh 确实进入了最终命令行;任意深度目录会被 mkdir(parents=True) 创建出来。

改法

  1. 路径沙箱。读路径必须落在部署本来就配置好的根目录内(仓库根、jobs 目录、ref/beagle/GENOS/cCRE/ncRNA 数据目录、OPENRARE_DATA_ROOT、OPENRARE_PUBLIC_DATA_ROOT),额外根用 FULL_PIPELINE_API_ALLOWED_ROOTS 追加。写路径必须落在 FULL_PIPELINE_API_OUTPUT_ROOT(默认 jobs 目录)内。全程走 resolve(),所以 .. 和符号链接都逃不出去。

    之所以默认根取自 path_utils,是为了让配置正确的现有部署零改动继续跑。

  2. java_bin / beagle_jar 从请求模型和 multipart 表单里删掉,改为纯服务端配置(JAVA_BIN / FULL_PIPELINE_BEAGLE_JAR)—— 按 issue 的方案 2。注意:原先不传这两个字段时走的就是脚本默认值,所以对任何非恶意调用方行为不变。

  3. job_id 格式校验(uuid4 hex)。这是顺带发现的:status_path() 直接拼 JOBS_DIR / job_id,构造 job_id 可以走出 jobs 目录。

  4. 读取侧复检:output_root_for_job() 和日志接口再确认一次路径在根内 —— 这样本次改动之前已经落库的恶意 job 也不会继续可用(返回 403),不需要清库。

  5. JOBS_DIR 在 import 时 resolve() 一次。原先它直接取环境变量,相对路径或软链会让后续所有包含判断失准。

测试

新增 test/test_path_sandbox.py,22 项检查全绿,注册为 pixi run path-sandbox-test。不依赖 pytest(仓库目前没有 pytest 基建,见 #19),直接 python test/test_path_sandbox.py 即可跑。

覆盖:上面每一条逃逸路径 + 「合法请求必须仍然成功」的反向用例(默认 output_dir、显式指定根内 output_dir、根内 input_vcf)。

补充一个测试里踩到的点:tempfile.mkdtemp() 在 Windows 返回 8.3 短路径,与 resolve() 后的路径不相等 —— 这也正是我把 JOBS_DIR 改成启动时 resolve 的原因,否则真实部署里若 jobs 目录含软链,包含判断会静默失效。

说明

本 PR 从 dev 切出,不含 #29(Fixes #15)的改动,两个可独立审阅、独立合并。

#16(无认证 / IDOR)我认为是这三个安全 issue 里唯一真正的架构决策,我会在 issue 下写方案而不是直接提 PR —— 沙箱化只是把「任何人可读任意文件」收敛成「任何人可读数据目录内的文件」,在认证落地之前,暴露面依然存在。

The run endpoints took filesystem paths straight from the request body and
passed them to the pipeline command. subprocess is called with a list, so
there was no shell injection, but the paths themselves were the problem:

  - java_bin and beagle_jar name executables the pipeline then runs.
  - input_vcf, ref_dir, ccre_bed, ncrna_bed and genos_evee_db could point
    anywhere the service user can read.
  - output_dir was mkdir'd and written to without any check, and because
    /jobs/{job_id}/files{,/{path}} serve files relative to the output_dir
    recorded in status.json, a caller could set output_dir to any directory
    and then list and download it. safe_job_file() confined paths to that
    root, but the root itself was attacker-supplied. dry_run=true was enough
    — no pipeline execution required.

Confirmed against the pre-fix code: submitting output_dir=<dir outside the
repo> returned 202, /files listed that directory, and the download route
returned the file contents with 200.

Changes:

  - Add a path sandbox. Read paths must resolve inside one of the roots the
    deployment already configures through path_utils (repo root, jobs dir,
    ref dir, beagle/GENOS/cCRE/ncRNA data dirs, OPENRARE_DATA_ROOT,
    OPENRARE_PUBLIC_DATA_ROOT), extendable via
    FULL_PIPELINE_API_ALLOWED_ROOTS. Outputs must resolve inside
    FULL_PIPELINE_API_OUTPUT_ROOT (default: the jobs dir). resolve() is used
    throughout, so '..' and symlinks cannot escape a root.
  - Drop java_bin and beagle_jar from RunRequest and the multipart form.
    Both stay server-side configuration (JAVA_BIN, FULL_PIPELINE_BEAGLE_JAR).
    Requests that omitted them already got the script defaults, so behaviour
    is unchanged for every non-malicious caller.
  - Validate job_id against the uuid4 hex format it is always generated in,
    so a crafted id cannot walk out of the jobs directory via status.json.
  - Re-check output_dir and the log path on read, so a job recorded before
    this change cannot still be used to read outside the roots.
  - Resolve JOBS_DIR once at import: FULL_PIPELINE_API_JOBS_DIR may be
    relative or symlinked, and every containment check compares resolved
    paths.

Add test/test_path_sandbox.py (22 checks, no pytest dependency, registered
as `pixi run path-sandbox-test`) covering each escape above plus the
in-sandbox requests that must still succeed.

Fixes OpenRare2026#17

Signed-off-by: FenjuFu <fufenjupku@gmail.com>

@pokemonjs pokemonjs left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

代码层面未发现会阻止合并的高危问题,核心安全修复有效;建议补充文档后合并。
发现的问题:

  • [P2] API 文档仍声明 beagle_jar 和 java_bin 可由请求传入,但 PR 已从 RunRequest 和 multipart 接口中移除这两个字段。当前 Pydantic 默认会忽略这两个额外字段,调用方不会收到明确错误,容易造成“配置成功但实际未生效”的误解。应同步更新 [modules/pipeline/complete_pipeline/README.md],并说明改用 FULL_PIPELINE_BEAGLE_JAR / JAVA_BIN 服务端配置。

beagle_jar and java_bin are no longer API fields, and path fields are
confined to the allowed roots. Explain the new settings and that old
requests passing those fields are silently ignored.

Signed-off-by: FenjuFu <92919259+FenjuFu@users.noreply.github.com>
@FenjuFu

FenjuFu commented Sep 18, 2026

Copy link
Copy Markdown
Contributor Author

@pokemonjs 感谢 review。P2 已在 f15e075 处理:modules/pipeline/complete_pipeline/README.md 的 API 字段表删掉了 beagle_jar、java_bin,并新增「路径沙箱与服务端配置」一节,说明:

  • 哪些路径字段受允许目录限制,以及如何用 FULL_PIPELINE_API_ALLOWED_ROOTS 追加目录;
  • output_dir 必须位于 FULL_PIPELINE_API_OUTPUT_ROOT 内(默认是 job 目录),示例里的 /path/to/output_dir 需要相应调整;
  • Beagle jar 和 Java 改由服务端的 FULL_PIPELINE_BEAGLE_JAR / JAVA_BIN 配置。旧调用方继续传这两个字段时不会报错,但会被忽略,文档里也写明了这一点。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants