Skip to content

test(storage): 修复 TestValidateRootPath 在含符号链接的系统上误报 - #131

Open
suantea wants to merge 1 commit into
PIKACHUIM:mainfrom
suantea:fix/storage-test-symlink
Open

suantea wants to merge 1 commit into
PIKACHUIM:mainfrom
suantea:fix/storage-test-symlink

Conversation

@suantea

@suantea suantea commented Sep 30, 2026

Copy link
Copy Markdown
Collaborator

问题

service/storage 的 TestValidateRootPath 在 macOS 上失败:

应返回规范化绝对路径,got=/private/var/folders/.../001 want=/var/folders/.../001

根因

这不是被测函数的 bug,而是测试断言写错了。

ValidateRootPath 会调 filepath.EvalSymlinks 解析软链:

// 解析符号链接,避免用软链绕过目录校验
if resolved, err := filepath.EvalSymlinks(abs); err == nil {
    abs = filepath.Clean(resolved)
}

这是刻意设计——防止用户用软链把共享根目录指到 smb.conf 等敏感文件上绕过校验。

而 macOS 的 t.TempDir() 返回 /var/folders/...,它本身是指向 /private/var/folders/... 的符号链接。测试却拿未解析软链的 dir 做期望值:

if got != filepath.Clean(dir) {   // ← 期望值没解析软链

为什么一直没被发现

该失败只在路径中存在符号链接的系统上出现。GitHub Actions 跑 ubuntu-latest,/tmp 不是软链,EvalSymlinks 返回原值——断言恰好成立,CI 一直是绿的。

也就是说,CI 的绿灯掩盖了这个测试在真实 macOS 开发环境下的失败。

修复

  • 期望值改为同样先 EvalSymlinks。Linux 上 /tmp 非软链,EvalSymlinks 返回原值,行为与之前完全一致,不影响现有 CI
  • 新增 TestValidateRootPathResolvesSymlink,覆盖「传入指向别处的软链时,必须返回解析后的真实路径」——这正是该函数防绕过的核心语义,此前无任何测试覆盖

验证

go test ./service/storage/ -run TestValidateRootPath -v
=== RUN   TestValidateRootPath
--- PASS: TestValidateRootPath
=== RUN   TestValidateRootPathResolvesSymlink
--- PASS: TestValidateRootPathResolvesSymlink

配合 #130 的编译修复,go test ./... 全量通过。

建议合并顺序

本 PR 与 #130(caddy 编译错误修复)无依赖关系,可独立评审。但 #130 合入前 go test ./... 无法在本地跑通——验证本 PR 时需两个都在本地。

ValidateRootPath 会调 filepath.EvalSymlinks 解析软链——这是刻意的,
用于防止用软链绕过共享根目录校验。macOS 的 t.TempDir() 返回
/var/folders/...,它是指向 /private/var/folders/... 的符号链接,
而测试拿未解析软链的 dir 做期望值,导致断言失败:

    got=/private/var/folders/... want=/var/folders/...

该失败只在含软链的系统上出现(Linux CI 无此问题),
此前一直被 CI 的绿灯掩盖。

- 期望值改为同样先 EvalSymlinks;Linux 上 /tmp 非软链,行为不变
- 新增 TestValidateRootPathResolvesSymlink:覆盖「传入软链时返回解析后
  的真实路径」——这正是该函数防绕过的语义,此前无测试覆盖

验证:go test ./... 全量通过。

This branch has not been deployed

No deployments
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.

1 participant