Parcourir la source

fix(e2e-test-utils): 修复代码审查发现的问题

- 修复默认 fixtures 路径为 'web/tests/fixtures'(符合 AC #1)
- 创建测试图片占位文件(sample-id-card.jpg, sample-disability-card.jpg)
- 加强路径验证防止路径遍历攻击
- 改进选择器无效错误处理,提供详细调试建议
- 更新 Story 3.1 状态为 done,添加代码审查记录

Generated with [Claude Code](https://claude.ai/code)
via [Happy](https://happy.engineering)

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Happy <yesreply@happy.engineering>
yourname il y a 1 semaine
Parent
commit
7841d35a12

+ 32 - 1
_bmad-output/implementation-artifacts/3-1-file-upload-tool.md

@@ -1,6 +1,6 @@
 # Story 3.1: 开发文件上传工具函数
 
-Status: review
+Status: done
 
 <!-- Note: Validation is optional. Run validate-create-story for quality check before dev-story. -->
 
@@ -351,3 +351,34 @@ Claude Opus 4 (claude-opus-4-5-20251101)
 - 添加 `@types/node` 依赖支持 Node.js API 类型
 - **架构改进**:修改 MinioUploader/FileSelector/PhotoUploadField 组件,使其支持 E2E 测试
 - 添加 `testId` prop 机制,支持页面中多个上传组件的测试定位
+
+**2026-01-10** - 代码审查修复
+- ✅ 修复默认 fixtures 路径为 `web/tests/fixtures`(符合 AC #1)
+- ✅ 创建测试图片占位文件(sample-id-card.jpg, sample-disability-card.jpg)
+- ✅ 加强路径验证防止路径遍历攻击
+- ✅ 改进选择器无效错误处理,提供更详细的调试建议
+
+---
+
+## Code Review Record
+
+**Reviewer:** Claude (code-review workflow)
+**Review Date:** 2026-01-10
+**Original Status:** review
+**Final Status:** done
+
+### Issues Found and Fixed
+
+| Severity | Issue | Status | Fix Details |
+|----------|-------|--------|-------------|
+| HIGH | 默认 fixtures 路径错误 | ✅ Fixed | 修改为 `web/tests/fixtures` |
+| HIGH | 测试 fixtures 目录为空 | ✅ Fixed | 创建 sample-id-card.jpg 和 sample-disability-card.jpg |
+| HIGH | 错误场景区分不清晰 | ✅ Fixed | 改进选择器错误处理,提供详细调试建议 |
+| MEDIUM | 路径遍历漏洞风险 | ✅ Fixed | 添加双重验证确保解析路径在 fixtures 目录内 |
+| LOW | 缺少单元测试 | ⏳ Deferred | 计划在 Story 3.2 实现 |
+
+### Files Modified During Review
+
+1. `packages/e2e-test-utils/src/file-upload.ts` - 修复默认路径、加强验证、改进错误处理
+2. `packages/e2e-test-utils/src/types.ts` - 更新 JSDoc 注释
+3. `web/tests/fixtures/images/` - 创建测试图片占位文件

+ 1 - 1
_bmad-output/implementation-artifacts/sprint-status.yaml

@@ -64,7 +64,7 @@ development_status:
   # 目标: 遵循"先验证再扩展"策略,优先开发文件上传工具,解决当前测试超时阻塞问题
   # 模式: 工具开发 → 真实 E2E 测试验证 → 问题修复 → 稳定性验证
   epic-3: in-progress
-  3-1-file-upload-tool: review           # 开发文件上传工具函数(含 UI 组件架构改进)
+  3-1-file-upload-tool: done             # 开发文件上传工具函数(含 UI 组件架构改进)
   3-2-upload-unit-tests: backlog         # 编写文件上传工具的单元测试
   3-3-upload-e2e-integration: backlog    # 在 web/tests/e2e 中验证文件上传工具
   3-4-collect-feedback-fix: backlog      # 收集反馈并修复问题

+ 35 - 8
packages/e2e-test-utils/src/file-upload.ts

@@ -21,7 +21,7 @@ import { DEFAULT_TIMEOUTS } from './constants';
  * @param selector - 文件输入框的选择器
  * @param fileName - 要上传的文件名(相对于 fixtures 目录)
  * @param options - 可选配置
- * @param options.fixturesDir - fixtures 目录路径,默认为 'tests/fixtures'
+ * @param options.fixturesDir - fixtures 目录路径,默认为 'web/tests/fixtures'
  * @param options.timeout - 超时时间(毫秒),默认 5000ms
  * @param options.waitForUpload - 是否等待上传完成,默认为 true
  * @throws {E2ETestError} 当文件不存在或选择器无效时
@@ -53,7 +53,7 @@ export async function uploadFileToField(
   // 1. 合并默认配置
   const config = {
     timeout: options?.timeout ?? DEFAULT_TIMEOUTS.static,
-    fixturesDir: options?.fixturesDir ?? 'tests/fixtures',
+    fixturesDir: options?.fixturesDir ?? 'web/tests/fixtures',
     waitForUpload: options?.waitForUpload ?? true
   };
 
@@ -89,13 +89,20 @@ export async function uploadFileToField(
 
     console.debug(`[uploadFileToField] 上传完成`);
   } catch (error) {
-    // 选择器无效或其他错误
+    // 选择器无效或其他错误 - 提供详细的上下文信息
+    const errorMessage = error instanceof Error ? error.message : '未知错误';
+
     throwError({
       operation: 'uploadFileToField',
       target: `选择器 "${selector}"`,
-      expected: `文件输入框存在于页面`,
-      actual: error instanceof Error ? error.message : '未知错误',
-      suggestion: '检查选择器是否正确,确认文件输入框已渲染到页面'
+      expected: '文件输入框存在于页面且可访问',
+      actual: `错误: ${errorMessage}`,
+      suggestion: [
+        '检查选择器是否正确(推荐使用 data-testid)',
+        '确认文件输入框已渲染到页面',
+        '确认元素可见且未被隐藏(display: none)',
+        '检查是否需要等待页面加载完成'
+      ].join('\n        ')
     });
   }
 }
@@ -116,8 +123,28 @@ export async function uploadFileToField(
 
 function resolveFixturePath(fileName: string, fixturesDir: string): string {
   const normalizedFileName = path.normalize(fileName);
+
+  // 拒绝绝对路径和向上遍历路径
   if (normalizedFileName.startsWith("..") || path.isAbsolute(normalizedFileName)) {
-    throwError({ operation: "uploadFileToField", target: fileName, suggestion: "use relative path" });
+    throwError({
+      operation: "uploadFileToField",
+      target: fileName,
+      suggestion: "文件名必须是相对于 fixtures 目录的路径,不能使用 '..' 或绝对路径"
+    });
+  }
+
+  // 解析完整路径
+  const resolvedPath = path.resolve(path.join(fixturesDir, normalizedFileName));
+  const resolvedFixturesDir = path.resolve(fixturesDir);
+
+  // 验证解析后的路径在 fixtures 目录内(防止路径遍历攻击)
+  if (!resolvedPath.startsWith(resolvedFixturesDir)) {
+    throwError({
+      operation: "uploadFileToField",
+      target: fileName,
+      suggestion: "文件名路径试图访问 fixtures 目录之外的文件"
+    });
   }
-  return path.resolve(path.join(fixturesDir, normalizedFileName));
+
+  return resolvedPath;
 }

+ 1 - 1
packages/e2e-test-utils/src/types.ts

@@ -84,7 +84,7 @@ export interface AsyncSelectOptions extends BaseOptions {
  * @description
  * 用于文件上传工具函数的配置选项。
  *
- * @property {string} fixturesDir - fixtures 目录路径,默认为 'tests/fixtures'
+ * @property {string} fixturesDir - fixtures 目录路径,默认为 'web/tests/fixtures'
  * @property {boolean} waitForUpload - 是否等待上传完成,默认为 true
  *
  * @example

BIN
web/tests/fixtures/images/sample-disability-card.jpg


BIN
web/tests/fixtures/images/sample-id-card.jpg