Conversation
* fix: 修复 P0/P1 安全漏洞与稳定性问题 - 认证: 修复默认空密码绕过，为日志接口添加认证与限流 - XSS: 废除课程操作内联 onclick，改用 data-id + 事件委托 - 数据一致性: 深拷贝默认配置、引入写锁、JSON损坏返回默认值、修复导入 announcements 丢失 - 安全加固: crypto.randomInt 生成密码、统一 IP 提取、rateLimitStore TTL 清理、限制请求体 1MB - 前端 Bug: 修复 localStorage 解析崩溃、URL.revokeObjectURL 过早释放、Array.prototype.at() 兼容性、起止周次校验 - 部署: 移除 jsdom 依赖，添加 SIGINT 优雅关闭 * refactor: 根据 Copilot 代码审查意见修复安全问题与数据一致性 - strictRateLimit key 改为仅基于 IP，防止通过更换 :file 绕过限流 - loadSchedule 中对 defaultPeriods 进行深拷贝，避免共享引用污染 - settings/import 路由先校验一致性再修改 schedule，防止 mutating cache 后才 throw - 日志接口认证仅允许 header 传参，防止密码写入请求日志 URL - 请求日志中间件对 URL 敏感参数做脱敏处理 - 启动时若自动生成密码，输出密码到 stdout 便于管理员获取 - index.html 添加 escapeAttr() 专门转义引号，修复 data-id 属性注入风险 * refactor: 根据 chatgpt-codex-connector 审查意见补充修复 - 导入 announcements 时过滤无效条目（防止 null/非对象导致 500） - 新增 TRUST_PROXY 环境变量，默认不信任 X-Forwarded-For，仅在显式开启时使用 * refactor: 根据第二轮 Copilot + GPT 审查意见修复 - data-type 属性改用 escapeAttr 转义引号 - requireLogAuth 对重复 x-password header 做数组降级为字符串 - import catch 区分 400/500，将校验错误正确返回客户端 - settings/import 保存前强制验证 periodSettings.length === totalPeriods - 启动 banner 中直接显示自动生成的密码（便于 docker logs 查看） - 导入 announcements 时对缺失/非法 id 重新生成 * refactor: 根据第三轮 Copilot 审查意见修复工程化问题 - 新增 HttpError 类替代 throw 普通对象，保留堆栈信息 - init() 中保存 createDefaultSchedule() 深拷贝，避免污染 defaultSchedule 基线 - 导入路由添加 isValidCourses 结构校验（验证 weekday 键及数组类型） - 启动 banner 默认不显示明文密码，新增 PRINT_EDIT_PASSWORD 显式开关 - docker-compose.yml 改回 避免 compose 变量未设置警告 * refactor: 根据第四轮 Copilot 审查意见实现原子更新与字段级长度限制 - settings/import/courses/announcements 路由改用深拷贝构造新对象， 在校验通过后才 saveSchedule，避免部分修改污染 scheduleCache - 新增 isValidCourse / isValidCourses / isValidAnnouncement 函数， 对课程名(100)、地点(100)、教师(50)、节次(20)、公告标题(100)、内容(2000) 等字段做长度上限校验 * refactor: 根据第五轮 Copilot 审查意见修复原子更新与限流清理 - PUT /api/schedule/courses 改用 isValidCourses 做完整结构与字段长度校验 - DELETE /api/announcements/:id 改用深拷贝 newSchedule 实现原子更新 - 限流 TTL 清理改为基于 entry.expiresAt（startTime + windowMs），避免写死15分钟导致短期窗口 key 残留 * fix: guard periodSettings entry type and validate imported announcements fully - isValidPeriodSettings: guard against null/non-object entries to avoid TypeError -> 500 on input like [null] (Copilot review) - Import: validate announcements with isValidAnnouncement and whitelist fields instead of spreading untrusted input (Copilot review) * fix: strict field validation for settings, import, and announcements - /api/schedule/settings: periodSettings validation failure now returns 400 instead of silently ignoring (Copilot review) - /api/import: periodSettings validation failure now returns 400 instead of silently ignoring (Copilot review) - /api/announcements: sanitize announcement before save with whitelisted fields and constrained id length; prevents arbitrary field injection (Copilot review) * fix: return sanitized announcement in response and preserve enabled on import - /api/announcements: response now returns sanitizedAnnouncement instead of raw input, so client gets the actual persisted id (Copilot review) - /api/import: preserve enabled field in announcement whitelist to maintain export->import idempotency (Copilot review) * fix: strict optional-field validation, normalize announcement id, update TODO - isValidCourse: use !== undefined/null instead of truthiness for optional fields so 0/false are rejected (Copilot review) - /api/announcements: normalize id once (trim) and use normalizedId for both lookup and sanitizedAnnouncement to avoid duplicates (Copilot review) - TODO.md: mark Array.prototype.at() compatibility as completed (Copilot review) * docs: record Copilot low-priority suggestions in TODO.md --------- Co-authored-by: root <root@localhost.localdomain>
* fix(test): 修复高优先级问题 - Docker安全、前端模块化、测试基础设施 高优先级修复（Issue #9）： 1. Docker安全：Dockerfile 添加 USER node 非root运行，数据目录权限调整 2. Dockerfile 内置 HEALTHCHECK，新增 .dockerignore 3. 前端模块化：将 index.html 内嵌 ~1000 行 JS 提取到 js/schedule.js 4. 测试基础设施：Jest + supertest 后端 API 测试（覆盖读写、认证、导入导出） 5. GitHub Actions CI：新增 ci.yml，在 push/PR 到 test/main 时自动跑测试 6. server.js 支持测试导出（module.exports），测试环境跳过限流 7. 修复 export 中文文件名 Content-Disposition 编码（RFC 5987） 8. TODO.md 更新：标记高优先级完成，记录中低优先级待办 * fix: 添加 src/server/public 符号链接，修复本地开发静态文件路径 原始项目本地开发时 make dev / npm start 找不到静态文件， 因为 server.js 默认 publicPath 为 src/server/public，但文件在 src/public/。 添加符号链接使本地开发和 Docker 行为一致。 * fix: 处理 Copilot PR Review 6 条评论 1. Dockerfile: chmod 755 → 750，缩小目录权限面 2. src/server/package.json: 移除 src/server 目录下的 jest/test 脚本 （测试文件在仓库根目录，由根目录 package.json 统一管理） 3. server.js: setInterval TTL 清理在测试环境禁用，解决 Jest open handles 4. server.js: Content-Disposition 同时提供 ASCII filename 回退 + RFC 5987 filename* 5. schedule.js: data-id 属性由 escapeHtml 改为 escapeAttr，防止属性注入 6. schedule.js: editAnnouncement 改用 annListCache 而非 schedule.announcements， 保证 renderAnnList 拉取最新数据后编辑操作数据一致 * fix: 处理 Copilot 第二轮 PR Review 5 条评论 1. schedule.js: courseModal 遮罩层关闭特判调用 closeModal()，修复函数名映射错误 2. schedule.js: 修正 autoSave 注释为'成功时提示已保存，失败时提示错误' 3. package.json: 移除 --forceExit，测试已可干净退出 4. server.js: 新增 /healthz 轻量健康检查端点（不写日志、不限流） Dockerfile + docker-compose.yml HEALTHCHECK 同步改为 /healthz 5. api.test.js: 增强 export 测试断言，验证 Content-Disposition 同时包含 filename*=UTF-8'' 和 ASCII fallback filename * fix: 处理 Copilot 第三轮 PR Review 3 条评论 1. server.js: RFC 5987 编码对 encodeURIComponent 结果额外转义 *!'() 等字符 2. Dockerfile: 移除 /app 的 chown，仅数据目录 /data 对 node 用户可写，/app 保持 root 只读 3. package.json: 测试命令开启 --coverage，与 CI workflow 的 coverage 上传步骤对齐 * fix: initPeriodSettings input value 由 escapeHtml 改为 escapeAttr Copilot review: escapeHtml 不转义引号，在 HTML 属性上下文中不安全。 改用 escapeAttr 以防止属性值注入。 * fix: 处理 Copilot 第四轮 PR Review 4 条评论 1. server.js: rateLimitCleanupInterval 调用 .unref()，避免 require 常驻； SIGTERM/SIGINT 中 clearInterval 优雅清理 2. api.test.js: 显式设置 process.env.NODE_ENV = 'test'，确保测试环境稳定 3. package.json: 新增 pretest 自动安装 src/server 依赖，解决根目录单独 npm test 失败 4. ci.yml: cache-dependency-path 同时包含 src/server/package-lock.json 和根目录 package-lock.json * fix: 处理 Copilot 第五轮 PR Review 4 条评论 1. TODO.md: 删除末尾重复提示语 2. package.json: 移除 pretest，避免 CI/本地重复安装 3. docker-compose.yml: 添加 user: 1000:1000 与容器 node 用户对齐 deploy/install.sh: 同步 chown 1000:1000 data logs，确保绑定挂载后可写 4. schedule.js: add-btn 移除内联 onclick，改为 data-action='add-course' + 事件委托 * fix: 处理 Copilot 第六轮 PR Review 4 条评论 1. docker-compose.yml: 添加权限注释说明，提醒首次启动需确保 data/logs 对 UID 1000 可写 deploy/install-prebuilt.sh: 同步添加 chown 1000:1000 data logs 2. api.test.js: 新增 GET /healthz 测试，断言返回 200 和 {status: 'ok'} 3. schedule.js: saveChanges() 选择器改为 [data-action='save']，移除 nth-child(2) 依赖 index.html: 保存按钮添加 data-action='save'，移除内联 onclick toolbar 添加事件委托处理保存按钮点击 4. 移除 src/server/public 符号链接，改为 Makefile dev 目标设置 PUBLIC_PATH=../../src/public 避免仓库中出现'两份'前端脚本的误解，明确单一来源 --------- Co-authored-by: root <root@localhost.localdomain>
修复 Issue #9 最后总结的两个问题： 1. withSaveLock 捕获异常后至少记录错误日志，避免 save 失败时完全静默 2. PUT /api/schedule/settings 不传 periodSettings 但修改 totalPeriods 时， 自动截断 periodSettings 到新的 totalPeriods 长度，避免长度不匹配
* feat(schedule): 单双周课程显示「本周不上」印章 - 单周/双周课程在不属于当前周时仍渲染，但添加红色方形印章标记 - 添加 skip-week-stamp CSS 样式：右上角绝对定位、52×52px 红色半透明方印 - 跳过周的课程降低透明度并添加灰度滤镜，避免与正常课程混淆 - 当前课程高亮仅对真正活跃的课程生效 * fix: 根据 Copilot 审查意见修复「本周不上」印章逻辑与布局 - isSkipWeek 增加 inWeekRange 判断，避免将未开始/已结束的单双周课程误标为跳周 - 将 skip-week-stamp 从 .course-info 内移出到 .course-item 级别，避免被 grayscale filter 去色 - 给 .course-item 添加 position: relative，保持印章绝对定位正确 * fix: 统一 endWeek 默认值并避免编辑模式印章遮挡按钮 - isActiveInWeek 硬编码 endWeek 默认值 20 → totalWeeks，与 inWeekRange 保持一致 - 编辑模式下 skip-week-stamp 右移 92px，避免覆盖 .course-actions 编辑/删除按钮 * fix: formatWeekRange 硬编码 16 → totalWeeks 与 isActiveInWeek 保持一致，避免学期总周数非 16 时周次范围显示错误 * fix: scope skip-week to current week * fix: keep generated period times valid * fix: isolate settings period draft * fix: avoid expanding imported period ranges * fix: look ahead after skipped next course * fix: align next-course dates with viewed week --------- Co-authored-by: root <root@localhost.localdomain> Co-authored-by: yang12535 <yang12535@example.com>
* fix: support webview week option rendering * test: guard webview array compatibility --------- Co-authored-by: yang12535 <yang12535@example.com>
# Conflicts: # Dockerfile # docker-entrypoint.sh
Owner
Author
|
已完成 RainYun 生产实例保存失败修复与实际验证：
|
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Pull request overview
Prepares the project for a RainYun RCA release by hardening server behavior (health checks, password handling, validation, and safer persistence), updating deployment artifacts to a pinned image/version, and adding/expanding automated tests plus end-user deployment documentation.
Changes:
- Add data-independent
GET /healthzthat verifies persistent storage writability (write + atomic rename), plus related init hard-fail behavior. - Implement explicit
EDIT_PASSWORDtri-state handling (unset/auto-generate vs fixed value vs explicit empty string), tighten request validation, and serialize writes to reduce concurrent-save data loss. - Extract front-end JS to
src/public/js/schedule.js, add compatibility-focused tests, and update deployment/docs/CI to match the RainYun RCA packaging model.
Reviewed changes
Copilot reviewed 21 out of 25 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| TODO.md | Update the security/bugfix checklist and add “known issues / follow-ups” tracking. |
| tests/frontend-compat.test.js | Add a static check ensuring the front-end avoids Option and .at() for compatibility. |
| tests/api.test.js | Add API tests for schedule endpoints, import/export behavior, /healthz, and password modes. |
| src/server/server.js | Implement /healthz, password resolution, validation hardening, write serialization, safer logging, export header fix, and test-friendly exports. |
| src/server/package.json | Bump server package version and remove unused dependency (jsdom). |
| src/public/js/schedule.js | New extracted front-end logic with XSS hardening, compatibility tweaks, and UI behavior fixes. |
| src/public/index.html | Switch to external JS file, adjust UI markup/styles, and remove large inline script. |
| README.md | Add RainYun RCA deployment entry, clarify password semantics, and update versioned deployment guidance. |
| package.json | Add root Jest/Supertest dev setup for running tests from repo root. |
| Makefile | Adjust make dev to set PUBLIC_PATH for local runs. |
| jest.config.js | Add Jest configuration and coverage collection settings. |
| docs/rainyun-rca.md | Add end-user RainYun RCA deployment guide and operational notes. |
| Dockerfile | Add su-exec, entrypoint, and container HEALTHCHECK against /healthz. |
| docker-entrypoint.sh | Initialize/chown mounted /data as root, then drop privileges to node. |
| docker-compose.yml | Pin image tag, simplify persistence to /data only, and use /healthz healthcheck. |
| deploy/rainyun-template.reference.json | Add human-readable RainYun RCA template reference for manual UI entry. |
| deploy/install.sh | Align install flow with /data-only persistence and adjust directory ownership. |
| deploy/install-prebuilt.sh | Align prebuilt install flow with /data-only persistence and adjust directory ownership. |
| CHANGELOG.md | Document v1.1.0/v1.1.1 changes relevant to RainYun RCA and persistence behavior. |
| .gitignore | Ignore coverage output. |
| .github/workflows/ci.yml | Add CI workflow to run tests on Node 20/22 and upload coverage artifact. |
| .env.example | Update example env defaults and clarify password-generation behavior. |
| .dockerignore | Reduce Docker build context by excluding tests/docs/deploy artifacts and runtime data. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
yang12535
changed the title
Comment on lines
+55
to
+64
| function isValidCourse(course) { | ||
| if (!course || typeof course !== 'object') return false; | ||
| if (typeof course.name !== 'string' || !course.name.trim() || course.name.length > 100) return false; | ||
| if (course.location !== undefined && course.location !== null && (typeof course.location !== 'string' || course.location.length > 100)) return false; | ||
| if (course.teacher !== undefined && course.teacher !== null && (typeof course.teacher !== 'string' || course.teacher.length > 50)) return false; | ||
| if (typeof course.period !== 'string' || !course.period.trim() || course.period.length > 20) return false; | ||
| if (course.type !== undefined && course.type !== null && typeof course.type !== 'string') return false; | ||
| if (course.skipWeek !== undefined && course.skipWeek !== null && (!Number.isInteger(course.skipWeek) || course.skipWeek < 1 || course.skipWeek > 30)) return false; | ||
| return true; | ||
| } |
Comment on lines
+117
to
+124
| function isValidAnnouncement(a) { | ||
| if (!a || typeof a !== 'object') return false; | ||
| if (typeof a.title !== 'string' || !a.title.trim() || a.title.length > 100) return false; | ||
| if (typeof a.content !== 'string' || !a.content.trim() || a.content.length > 2000) return false; | ||
| if (a.startDate && !isValidDateString(a.startDate)) return false; | ||
| if (a.endDate && !isValidDateString(a.endDate)) return false; | ||
| return true; | ||
| } |
| } | ||
| if (name !== undefined) newSchedule.name = String(name).slice(0, 100); | ||
| if (description !== undefined) newSchedule.description = String(description).slice(0, 200); | ||
| if (semesterStart && isValidDateString(semesterStart)) newSchedule.semesterStart = semesterStart; |
Comment on lines
284
to
292
| @@ -124,23 +291,68 @@ app.use((req, res, next) => { | |||
| next(); | |||
| }); | |||
Comment on lines
+201
to
+210
| p = p.replace(/[第节]/g, '').trim(); | ||
| if (p.includes('-')) { | ||
| const parts = p.split('-').map(Number); | ||
| if (parts.length === 2 && !isNaN(parts[0]) && !isNaN(parts[1]) && parts[0] <= parts[1]) { | ||
| return Array.from({length:parts[1]-parts[0]+1},(_,i)=>parts[0]+i); | ||
| } | ||
| } | ||
| if (p.includes(',')) return p.split(',').map(Number).filter(n => !isNaN(n) && n > 0); | ||
| const n = +p; | ||
| return !isNaN(n) && n > 0 ? [n] : []; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
v1.1.2preview fixes for/datapersistence, fallback timestamps,periodSettingsvalidation, and academic-week date alignment./healthz, static files, and API responses.GET /api/announcements) with the edit password viax-password; the public active-announcement endpoint remains public.semesterStartupdates.Why
The deployed preview image on the Volcengine VPS is healthy, but review found two remaining security gaps: static responses missed the configured security headers, and the management announcement list was readable without auth. Follow-up review also found that weak validation could allow inconsistent course week metadata or normalized invalid dates to be stored. The RainYun platform-side listing is not complete yet, so the docs avoid presenting it as live.
Validation
npm test(45 tests passed),git diff --check,node --check src/server/server.js, RainYun reference JSON parses.10.0.0.212:npm test(32 tests passed before latest validation additions), Docker image build, container smoke test for/healthz,/, and announcement auth behavior.ghcr.nju.edu.cn/yang12535/schedule-web:1008ec5): confirmed current container is running healthy with restart count 0; inspected logs and reproduced the pre-fix header/auth gaps.CI Testchecks pass on Node 20 and Node 22.Notes
RainYun RCA documentation and template files are pre-launch references only. The platform listing/template ID and public store links still need confirmation after the RainYun side is completed.