Skip to content

pref(session): 会话目录化与 session_asset 发送链路落地#330

Merged
phantom5099 merged 5 commits into
1024XEngineer:mainfrom
phantom5099:main
Apr 17, 2026
Merged

pref(session): 会话目录化与 session_asset 发送链路落地#330
phantom5099 merged 5 commits into
1024XEngineer:mainfrom
phantom5099:main

Conversation

@phantom5099

Copy link
Copy Markdown
Collaborator

本次完成内容

本次改动围绕“会话级附件存储 + provider 发送”完成了主体闭环,重点是把会话数据和附件统一收敛到同一会话目录,并打通 session_asset 到模型请求阶段的读取与转换。

1. 会话存储结构升级为“会话目录模式”

  • 会话文件从平铺 *.json 调整为:
    • projects/<workspace-hash>/sessions/<session-id>/session.json
  • 同时在会话目录下预留并使用附件目录:
    • projects/<workspace-hash>/sessions/<session-id>/assets/

JSONStoreSave/Load/ListSummaries 已适配新结构,列表读取改为扫描会话子目录中的 session.json

2. 新增会话级附件存储能力

internal/session 新增 AssetStore 能力,并由 JSONStore 实现:

  • SaveAsset(ctx, sessionID, reader, mimeType)
  • Open(ctx, sessionID, assetID)
  • Stat(ctx, sessionID, assetID)

实现细节:

  • 附件内容存储为 assets/<asset-id>.bin
  • 附件元数据存储为 assets/<asset-id>.json
  • 写入过程使用临时文件 + 原子替换
  • sessionID/assetID 增加格式校验,阻断路径穿越
  • MIME 约束为图片类型(image/*

3. runtime 注入会话附件读取器

runtime.Service 新增可注入的 SessionAssetStore,并在每轮 provider 请求快照中注入 SessionAssetReader(按当前 sessionID 绑定)。

这样 provider 在编码消息时可以按会话读取 session_asset,而不需要接触本地绝对路径。

4. provider 支持 session_asset 图片发送

chatcompletions 适配器完成以下变更:

  • ToOpenAIMessage 支持读取 session_asset
  • 请求阶段将附件二进制临时转换为 data URL 发送(仅请求内转换,不回写会话消息)
  • 删除“session_asset 不支持”阻断逻辑
  • 对缺失 reader、缺失 MIME、空附件等情况返回显式错误

5. 测试与文档同步

  • 更新 session 相关测试路径到新会话目录结构
  • 新增会话附件存储测试(保存/打开/元数据/异常分支)
  • 新增 runtime 附件读取器注入测试
  • 更新 provider 测试,覆盖 session_asset 编码发送路径
  • 更新 issue 文档中的目录结构描述为当前真实实现

测试结果

已通过:

  • go test ./internal/session
  • go test ./internal/runtime
  • go test ./internal/provider/openaicompat/chatcompletions
  • go test ./internal/provider/openaicompat
  • go test ./internal/app

@codecov

codecov Bot commented Apr 16, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 80.54711% with 64 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
internal/session/store.go 66.11% 36 Missing and 25 partials ⚠️
internal/session/asset_store.go 95.00% 1 Missing and 1 partial ⚠️
...l/provider/openaicompat/chatcompletions/request.go 98.75% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

fennoai[bot]

This comment was marked as outdated.

fennoai[bot]

This comment was marked as outdated.

fennoai[bot]

This comment was marked as outdated.

fennoai[bot]

This comment was marked as outdated.

fennoai[bot]

This comment was marked as outdated.

@phantom5099

Copy link
Copy Markdown
Collaborator Author

/code 修复comment的问题,并提高覆盖测试率

@fennoai

fennoai Bot commented Apr 16, 2026

Copy link
Copy Markdown
Contributor

已按触发指令完成“修复 comment 问题 + 提高覆盖测试率”,并已推送到当前 PR 分支 phantom5099/main(commit: 39a3c33)。

本次修复

  1. 修复 loadCustomProviders 读取副作用
  • internal/config/provider_loader.go
  • 读取阶段不再 MkdirAll;当 providers 目录不存在时返回空列表,避免只读目录启动失败。
  1. 修复 session_asset MIME 约束一致性
  • internal/session/asset_store.go
  • decodeStoredAssetMeta 新增 image/* 校验,防止脏/篡改 meta 绕过约束。
  1. 修复 Open -> Stat 嵌套读锁风险并统一大小上限常量
  • internal/session/store.go
  • Open 改为在已持锁路径走 statUnlocked,避免重复 RLock
  • 写入大小上限改为复用 providertypes.MaxSessionAssetBytes,消除双常量漂移。
  1. 修复 provider 发送前 MIME 校验缺失
  • internal/provider/openaicompat/chatcompletions/request.go
  • 在生成 data URL 前强制校验 image/*,并规范化为小写 MIME。

测试补充

  • internal/config/loader_test.go
    • 同步读取行为变更(missing providers dir 返回空且不创建目录)。
  • internal/session/asset_store_test.go
    • 新增 decodeStoredAssetMeta 非图片 MIME 拒绝分支。
    • 同步附件大小上限常量引用。
  • internal/provider/openaicompat/chatcompletions/provider_test.go
    • 新增 session_asset 的“非图片 MIME / 缺失 MIME”错误分支覆盖。

验证结果

  • go test ./internal/session
  • go test ./internal/provider/openaicompat/chatcompletions
  • go test ./internal/config
  • go test ./...
  • go test -cover ./internal/session ./internal/provider/openaicompat/chatcompletions ./internal/config

View job run

Generated with [codeagent](https://github.com/qbox/codeagent)
Co-authored-by: phantom5099 <245659304+phantom5099@users.noreply.github.com>
@phantom5099

Copy link
Copy Markdown
Collaborator Author

/review

@fennoai fennoai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

审查完成:主链路实现整体清晰,测试覆盖也比较完整;以下仅有 1 条建议关注的非阻断风险。

}
defer func() { _ = reader.Close() }()

data, err := io.ReadAll(io.LimitReader(reader, maxSessionAssetReadBytes+1))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

当前只对单个 session_asset 做了 20 MiB 限制,但没有限制单次请求内所有附件的累计大小。当一条消息包含多个 session_asset(每个都低于上限)时,会在构建请求阶段累计读入并 base64 膨胀,容易导致内存和请求体体积突增,触发 provider 413/超时或本地内存峰值问题。建议增加总量上限(例如在 BuildRequest/转换流程中累计字节并提前失败)。

@fennoai fennoai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

发现 1 个需要修复的回归风险,集中在会话存储路径迁移兼容性。

Comment thread internal/session/store.go
// filePath 生成会话 ID 对应的 JSON 文件路径。
func (s *JSONStore) filePath(id string) string {
return filepath.Join(s.baseDir, id+".json")
return filepath.Join(s.sessionDir(id), sessionFileName)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

filePath<sessionID>.json 切换到 <sessionID>/session.json,但本 PR 没有提供旧格式读取兼容或迁移逻辑;同时 ListSummaries 仅遍历目录项。结果是升级后历史会话会在列表中“消失”,并且 Load 无法读取旧会话文件,属于用户可见的数据回归。建议至少增加向后兼容读取(优先新路径,回退旧路径)或在启动阶段执行一次可回滚的迁移。

@fennoai fennoai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

复审完成。当前实现整体方向正确;我保留 1 个仍需确认的兼容性问题。

Comment thread internal/session/store.go
summaries := make([]Summary, 0, len(entries))
for _, entry := range entries {
if entry.IsDir() || filepath.Ext(entry.Name()) != ".json" {
if !entry.IsDir() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

会话存储切到目录结构后,这里只扫描目录项,且 Load 也只读 <session-id>/session.json。如果用户本地仍有旧版平铺 *.json 会话文件,升级后会直接“不可见/不可加载”。建议至少补一个兼容迁移路径(读取旧文件并迁移)或在启动阶段给出明确迁移提示。

@fennoai fennoai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

完成审查,发现 1 个需要处理的回归风险。

Comment thread internal/session/store.go

@fennoai fennoai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

发现 1 个需要确认的回归风险:会话存储从 <id>.json 切换到 <id>/session.json 后,Load/ListSummaries 不再读取旧布局。若这是有意的破坏性变更,建议在文档/发布说明中明确;否则建议补回退读取或迁移逻辑。

Comment thread internal/session/store.go
@phantom5099

Copy link
Copy Markdown
Collaborator Author

/code 提高测试覆盖率

@fennoai

fennoai Bot commented Apr 16, 2026

Copy link
Copy Markdown
Contributor

完成情况

已按“提高测试覆盖率”完成补测并推送到当前 PR head 分支 phantom5099/main

变更摘要

  • 新增/补强 internal/session/store_test.go
  • 新增/补强 internal/session/asset_store_test.go
  • 新增/补强 internal/provider/openaicompat/chatcompletions/provider_test.go

主要补齐分支:

  • JSONStore.Save/Load/ListSummaries 的 schema/id 校验与目录化异常路径
  • decodeStoredSession/decodeStoredSummary 的 schema 校验分支
  • decodeStoredAssetMeta/encodeStoredAssetMeta 的输入校验分支
  • BuildRequest/ToOpenAIMessage/resolveSessionAssetDataURLsession_asset 错误路径(缺 reader、空 asset id、open/read/cancel/empty asset 等)

验证结果

  • go test ./internal/session ./internal/provider/openaicompat/chatcompletions
  • go test -cover ./internal/session ./internal/runtime ./internal/provider/openaicompat/chatcompletions ./internal/config
    • internal/session: 88.8% -> 91.1%
    • internal/provider/openaicompat/chatcompletions: 90.4% -> 93.2%
  • go test ./...

提交信息

  • commit: 4f7ed64

View job run

Generated with [codeagent](https://github.com/qbox/codeagent)
Co-authored-by: phantom5099 <245659304+phantom5099@users.noreply.github.com>
@fennoai fennoai Bot mentioned this pull request Apr 16, 2026
@pionxe

pionxe commented Apr 17, 2026

Copy link
Copy Markdown
Collaborator

@xgopilot 仔细审查给定的 PR 代码变更。该 PR 将会话存储结构由平铺文件改为了目录结构,引入了二进制附件的本地存储,并在大模型请求时进行 Base64 转换。你的核心任务是寻找内存溢出风险(OOM)、向后兼容性断裂、跨平台文件锁冲突、路径穿越漏洞以及文件句柄泄漏。请忽略基础语法格式,直击架构隐患。

Review Checkpoints (重点审查清单)

请严格按照以下清单对代码进行交叉对比和逐行审查:

  1. 大对象内存与 OOM 风险 (Memory Management):

    • Base64 膨胀:chatcompletions 适配器将图片二进制转换为 Data URL 时,审查代码是否会将整个图片读入内存。如果是,是否设置了单张图片和总载荷的硬性大小上限(如 5MB 拦截)?
    • 缓冲区复用: 检查大对象的处理是否尽量使用了 io.Copybytes.Buffer 的池化复用(sync.Pool),以减轻 GC 压力,防止端侧进程被 OS 杀掉。
  2. 本地数据向后兼容性 (Backward Compatibility):

    • 旧版数据孤岛: JSONStore 的存储路径从平铺的 *.json 切换到了 <session-id>/session.json。检查 ListSummariesLoad 函数,是否包含对旧版平铺文件的扫描兼容,或者是否在初始化时提供了自动迁移逻辑?如果都没有,旧用户的历史会话将直接丢失!
  3. 跨平台文件系统陷阱 (Cross-Platform I/O):

    • Windows 下的重命名冲突: 写入机制使用了“临时文件 + 原子替换 (os.Rename)”。请检查在执行 os.Rename 之前,用于写入的临时文件句柄 *os.File 是否已经显式调用了 .Close()。在 Windows 环境下,重命名一个尚未关闭的文件会直接抛出 Access is denied 错误。
    • 临时文件安全性: 临时文件是否由 os.CreateTemp 生成,以防并发写入同名文件导致数据损坏?
  4. 安全与边界防御 (Security & Path Traversal):

    • 强格式校验: PR 提到阻断了 sessionID/assetID 的路径穿越。检查校验逻辑,是否使用了严格的白名单正则(例如 ^[a-zA-Z0-9_-]+$)?绝对不能仅仅依赖 strings.Replace(id, "../", "", -1) 这种容易被绕过的黑名单。
    • 路径逸出复核: 拼接完整路径后,是否使用了 filepath.IsLocal (Go 1.20+) 或绝对路径前缀匹配来二次确认最终路径被死死限制在 assets/ 目录下?
  5. 资源泄漏 (Resource Leaks):

    • 文件描述符 (FD) 泄漏: provider 层通过 SessionAssetReader 取到 io.Reader(通常是本地文件的 Wrapper)后,无论是发生序列化错误、网络中断还是正常发送完毕,是否都有稳妥的 defer reader.Close() 来保证底层文件句柄被释放?

Output Format

请以 Markdown 格式输出你的审查报告。要求直奔主题,结构如下:

  1. 🛑 致命缺陷 (Blockers): 指出可能导致用户内存耗尽、旧会话丢失、Windows 平台写入崩溃或文件描述符耗尽的具体代码行,并给出修复代码。
  2. ⚠️ 隐患与优化 (Suggestions): 指出路径防御可以更严谨的地方,或日志/错误包装等不够清晰的边缘场景。
  3. ✅ 架构合规项 (Passed): 简要列出 PR 中接口提取、目录收敛等做得好的地方。

如果你在上述清单中未发现问题,请在报告中明确回复“针对 [第 X 项],代码已通过严格审查,未发现逻辑漏洞”。

@fennoai

fennoai Bot commented Apr 17, 2026

Copy link
Copy Markdown
Contributor

🛑 致命缺陷 (Blockers)

  1. 向后兼容断裂:旧平铺会话不可见/不可加载
  • 位置:internal/session/store.go:154, internal/session/store.go:186, internal/session/store.go:196, internal/session/store.go:220
  • 问题:Load 只读 <id>/session.jsonListSummaries 只扫描目录并读取 */session.json,旧版 sessions/<id>.json 没有兼容扫描或迁移逻辑。升级后旧用户历史会话将直接“消失”。
  • 修复建议(示例):
func (s *JSONStore) legacyFilePath(id string) string {
	return filepath.Join(s.baseDir, id+".json")
}

func (s *JSONStore) Load(ctx context.Context, id string) (Session, error) {
	data, err := os.ReadFile(s.filePath(id))
	if err != nil && errors.Is(err, os.ErrNotExist) {
		data, err = os.ReadFile(s.legacyFilePath(id))
	}
	if err != nil {
		return Session{}, err
	}
	// decode...
}

func (s *JSONStore) ListSummaries(ctx context.Context) ([]Summary, error) {
	// 现有目录扫描保留
	// 额外扫描 baseDir 下 *.json(排除 assets 元数据)并合并去重(目录优先)
}
  1. 请求阶段内存峰值过高,存在 OOM 风险
  • 位置:internal/provider/openaicompat/chatcompletions/request.go:176, internal/provider/openaicompat/chatcompletions/request.go:202, internal/provider/openaicompat/chatcompletions/request.go:44, internal/provider/openaicompat/chatcompletions/provider.go:46
  • 问题:单图允许 20 MiB,并在 resolveSessionAssetDataURLReadAll 后再 base64,随后 json.Marshal(payload) 再复制一份。多图场景没有“总载荷硬上限”,峰值内存会叠加,容易触发端侧进程 OOM。
  • 修复建议(示例):
const (
	maxSessionAssetBytes      int64 = 5 * 1024 * 1024   // 单图硬上限
	maxSessionAssetsTotalBytes int64 = 12 * 1024 * 1024 // 单请求总上限(原始字节)
)

func BuildRequest(ctx context.Context, cfg provider.RuntimeConfig, req providertypes.GenerateRequest) (Request, error) {
	var total int64
	for _, m := range req.Messages {
		msg, used, err := ToOpenAIMessageWithBudget(ctx, m, req.SessionAssetReader, maxSessionAssetsTotalBytes-total)
		if err != nil { return Request{}, err }
		total += used
		if total > maxSessionAssetsTotalBytes {
			return Request{}, fmt.Errorf("session assets total exceeds %d bytes", maxSessionAssetsTotalBytes)
		}
		payload.Messages = append(payload.Messages, msg)
	}
	return payload, nil
}

⚠️ 隐患与优化 (Suggestions)

  1. 临时文件命名固定为 *.tmp,并发/多进程下存在冲突窗口
  • 位置:internal/session/store.go:127, internal/session/store.go:269, internal/session/store.go:306
  • 说明:当前不是 os.CreateTemp,若同一会话被多进程同时写入,可能互相覆盖或在 Windows 下放大 rename 失败概率。
  • 建议:改为 os.CreateTemp(dir, assetID+"-*.tmp"),写完 Sync+Close 后再 Rename
  1. 路径防御已做白名单校验,但缺少“最终路径二次约束”
  • 位置:internal/session/store.go:27, internal/session/store.go:530
  • 说明:当前正则校验较严格,已显著降低穿越风险;但可加一道 filepath.Rel(base, target) / filepath.IsLocal 复核作为 defense-in-depth。

✅ 架构合规项 (Passed)

  • 针对 [第 3 项:Windows rename 前句柄关闭],代码已通过严格审查,未发现逻辑漏洞:SaveAsset 在重命名前已显式 Closeinternal/session/store.go:275-277 -> :295)。
  • 针对 [第 4 项:ID 白名单校验],代码已通过严格审查,未发现逻辑漏洞:validateStorageID 使用白名单正则并在 Save/Load/SaveAsset/Open/Stat 统一调用。
  • 针对 [第 5 项:FD 泄漏],代码已通过严格审查,未发现逻辑漏洞:provider 侧在打开附件后使用 defer reader.Close()internal/provider/openaicompat/chatcompletions/request.go:174)。

View job run

@phantom5099

Copy link
Copy Markdown
Collaborator Author

🛑 致命缺陷 (Blockers)

  1. 向后兼容断裂:旧平铺会话不可见/不可加载
  • 位置:internal/session/store.go:154, internal/session/store.go:186, internal/session/store.go:196, internal/session/store.go:220
  • 问题:Load 只读 <id>/session.jsonListSummaries 只扫描目录并读取 */session.json,旧版 sessions/<id>.json 没有兼容扫描或迁移逻辑。升级后旧用户历史会话将直接“消失”。
  • 修复建议(示例):
func (s *JSONStore) legacyFilePath(id string) string {
	return filepath.Join(s.baseDir, id+".json")
}

func (s *JSONStore) Load(ctx context.Context, id string) (Session, error) {
	data, err := os.ReadFile(s.filePath(id))
	if err != nil && errors.Is(err, os.ErrNotExist) {
		data, err = os.ReadFile(s.legacyFilePath(id))
	}
	if err != nil {
		return Session{}, err
	}
	// decode...
}

func (s *JSONStore) ListSummaries(ctx context.Context) ([]Summary, error) {
	// 现有目录扫描保留
	// 额外扫描 baseDir 下 *.json(排除 assets 元数据)并合并去重(目录优先)
}
  1. 请求阶段内存峰值过高,存在 OOM 风险
  • 位置:internal/provider/openaicompat/chatcompletions/request.go:176, internal/provider/openaicompat/chatcompletions/request.go:202, internal/provider/openaicompat/chatcompletions/request.go:44, internal/provider/openaicompat/chatcompletions/provider.go:46
  • 问题:单图允许 20 MiB,并在 resolveSessionAssetDataURLReadAll 后再 base64,随后 json.Marshal(payload) 再复制一份。多图场景没有“总载荷硬上限”,峰值内存会叠加,容易触发端侧进程 OOM。
  • 修复建议(示例):
const (
	maxSessionAssetBytes      int64 = 5 * 1024 * 1024   // 单图硬上限
	maxSessionAssetsTotalBytes int64 = 12 * 1024 * 1024 // 单请求总上限(原始字节)
)

func BuildRequest(ctx context.Context, cfg provider.RuntimeConfig, req providertypes.GenerateRequest) (Request, error) {
	var total int64
	for _, m := range req.Messages {
		msg, used, err := ToOpenAIMessageWithBudget(ctx, m, req.SessionAssetReader, maxSessionAssetsTotalBytes-total)
		if err != nil { return Request{}, err }
		total += used
		if total > maxSessionAssetsTotalBytes {
			return Request{}, fmt.Errorf("session assets total exceeds %d bytes", maxSessionAssetsTotalBytes)
		}
		payload.Messages = append(payload.Messages, msg)
	}
	return payload, nil
}

⚠️ 隐患与优化 (Suggestions)

  1. 临时文件命名固定为 *.tmp,并发/多进程下存在冲突窗口
  • 位置:internal/session/store.go:127, internal/session/store.go:269, internal/session/store.go:306
  • 说明:当前不是 os.CreateTemp,若同一会话被多进程同时写入,可能互相覆盖或在 Windows 下放大 rename 失败概率。
  • 建议:改为 os.CreateTemp(dir, assetID+"-*.tmp"),写完 Sync+Close 后再 Rename
  1. 路径防御已做白名单校验,但缺少“最终路径二次约束”
  • 位置:internal/session/store.go:27, internal/session/store.go:530
  • 说明:当前正则校验较严格,已显著降低穿越风险;但可加一道 filepath.Rel(base, target) / filepath.IsLocal 复核作为 defense-in-depth。

✅ 架构合规项 (Passed)

  • 针对 [第 3 项:Windows rename 前句柄关闭],代码已通过严格审查,未发现逻辑漏洞:SaveAsset 在重命名前已显式 Closeinternal/session/store.go:275-277 -> :295)。
  • 针对 [第 4 项:ID 白名单校验],代码已通过严格审查,未发现逻辑漏洞:validateStorageID 使用白名单正则并在 Save/Load/SaveAsset/Open/Stat 统一调用。
  • 针对 [第 5 项:FD 泄漏],代码已通过严格审查,未发现逻辑漏洞:provider 侧在打开附件后使用 defer reader.Close()internal/provider/openaicompat/chatcompletions/request.go:174)。

View job run

@xgopilot 不要兼容读旧会话,然后把其他问题给我整理成一份清单输出

@fennoai

fennoai Bot commented Apr 17, 2026

Copy link
Copy Markdown
Contributor

其他问题清单(按严重级别)

已按你的要求排除“兼容读取旧平铺会话”项,仅保留其余问题。

  1. Blocker: 请求阶段存在明显内存放大,缺少总预算控制
  • 位置:internal/provider/openaicompat/chatcompletions/request.go:176, internal/provider/openaicompat/chatcompletions/request.go:202, internal/provider/openaicompat/chatcompletions/provider.go:46, internal/provider/types/asset_limits.go:5
  • 现状:单个 session_asset 允许到 20 MiB,并在请求构建阶段执行 ReadAll + base64,随后 json.Marshal(payload) 还会再产生一份大对象;多图时没有“总字节预算”上限。
  • 风险:多图输入会叠加放大峰值内存,导致进程 OOM 或频繁 GC 抖动。
  • 建议:
    • 增加“单请求总附件预算”(原始字节计)并在 BuildRequest 累加校验。
    • 下调单图硬上限(例如 5~10 MiB)或按 provider 能力配置化。
    • 超限时返回显式错误,避免进入 json.Marshal 阶段才触发内存压力。
  1. High: 临时文件命名固定 *.tmp,并发/多进程写入存在冲突窗口
  • 位置:internal/session/store.go:127, internal/session/store.go:269, internal/session/store.go:306
  • 现状:Save/SaveAsset/SaveAsset(meta) 都使用固定 target + ".tmp"
  • 风险:同会话并发写或多进程同时写入时,临时文件可能互相覆盖;在 Windows 上也会放大替换失败概率。
  • 建议:改为 os.CreateTemp(dir, pattern) 生成唯一临时文件,Sync + Close 后再原子 Rename
  1. Medium: 仅靠 ID 白名单,缺少落盘路径“二次约束”防护
  • 位置:internal/session/store.go:219-240, internal/session/store.go:530
  • 现状:当前主要依赖 validateStorageID 正则限制字符集合。
  • 风险:现有防护已较强,但仍缺少 defense-in-depth;后续若放宽 ID 规则或引入新入口,路径边界风险会回归。
  • 建议:对最终目标路径增加 filepath.Rel(base, target) + filepath.IsLocal(或等价前缀校验)复核,确保始终落在会话基目录内。

结论

本轮可优先修复顺序:内存总预算 > 临时文件唯一化 > 路径二次约束。未按你的要求引入旧会话兼容读取逻辑。

View job run

@phantom5099

Copy link
Copy Markdown
Collaborator Author

其他问题清单(按严重级别)

已按你的要求排除“兼容读取旧平铺会话”项,仅保留其余问题。

  1. Blocker: 请求阶段存在明显内存放大,缺少总预算控制
  • 位置:internal/provider/openaicompat/chatcompletions/request.go:176, internal/provider/openaicompat/chatcompletions/request.go:202, internal/provider/openaicompat/chatcompletions/provider.go:46, internal/provider/types/asset_limits.go:5

  • 现状:单个 session_asset 允许到 20 MiB,并在请求构建阶段执行 ReadAll + base64,随后 json.Marshal(payload) 还会再产生一份大对象;多图时没有“总字节预算”上限。

  • 风险:多图输入会叠加放大峰值内存,导致进程 OOM 或频繁 GC 抖动。

  • 建议:

    • 增加“单请求总附件预算”(原始字节计)并在 BuildRequest 累加校验。
    • 下调单图硬上限(例如 5~10 MiB)或按 provider 能力配置化。
    • 超限时返回显式错误,避免进入 json.Marshal 阶段才触发内存压力。
  1. High: 临时文件命名固定 *.tmp,并发/多进程写入存在冲突窗口
  • 位置:internal/session/store.go:127, internal/session/store.go:269, internal/session/store.go:306
  • 现状:Save/SaveAsset/SaveAsset(meta) 都使用固定 target + ".tmp"
  • 风险:同会话并发写或多进程同时写入时,临时文件可能互相覆盖;在 Windows 上也会放大替换失败概率。
  • 建议:改为 os.CreateTemp(dir, pattern) 生成唯一临时文件,Sync + Close 后再原子 Rename
  1. Medium: 仅靠 ID 白名单,缺少落盘路径“二次约束”防护
  • 位置:internal/session/store.go:219-240, internal/session/store.go:530
  • 现状:当前主要依赖 validateStorageID 正则限制字符集合。
  • 风险:现有防护已较强,但仍缺少 defense-in-depth;后续若放宽 ID 规则或引入新入口,路径边界风险会回归。
  • 建议:对最终目标路径增加 filepath.Rel(base, target) + filepath.IsLocal(或等价前缀校验)复核,确保始终落在会话基目录内。

结论

本轮可优先修复顺序:内存总预算 > 临时文件唯一化 > 路径二次约束。未按你的要求引入旧会话兼容读取逻辑。

View job run

@xgopilot 修复这条comment中提到的问题

@fennoai

fennoai Bot commented Apr 17, 2026

Copy link
Copy Markdown
Contributor

修复完成

已按你在该评论里要求的 3 项问题完成修复,并已推送到当前 PR 分支 phantom5099/main

提交:48043eb

本次修改

  1. 内存总预算控制(Blocker)
  • internal/provider/types/asset_limits.go
    • 新增 MaxSessionAssetsTotalBytes(单请求 session_asset 原始总字节上限)。
  • internal/provider/openaicompat/chatcompletions/request.go
    • BuildRequest 增加会话附件总预算累加与前置拦截。
    • 新增带预算的消息转换路径,按消息内 session_asset 实际读取字节累计。
    • resolveSessionAssetDataURL 新增 remainingBudget 控制,超预算时在构建请求阶段直接失败,避免继续放大到后续 json.Marshal
  1. 临时文件唯一化(High)
  • internal/session/store.go
    • Save / SaveAsset / 资产 meta 写入统一改为 os.CreateTemp 生成唯一临时文件。
    • 保留 Sync + Close + Rename 原子替换流程,避免固定 *.tmp 在并发/多进程下冲突。
    • 新增通用原子写辅助函数,统一错误处理与清理逻辑。
  1. 路径二次约束(Medium)
  • internal/session/store.go
    • 新增 ensurePathWithinBase,基于 filepath.Rel + filepath.IsLocal 做目标路径二次复核。
    • Save/Load/ListSummaries/SaveAsset/Open/statUnlocked 的关键读写路径执行 base 内约束,形成 defense-in-depth。

测试更新

  • internal/provider/openaicompat/chatcompletions/provider_test.go
    • 新增单请求多附件总预算超限分支测试。
    • 同步更新 resolveSessionAssetDataURL 签名相关测试,并补充总预算错误分支。
  • internal/session/store_test.go
    • 同步临时文件机制调整后的失败路径测试。
    • 新增 ensurePathWithinBase 逃逸路径拒绝测试。

验证结果

  • go test ./internal/provider/openaicompat/chatcompletions ./internal/session
  • go test ./internal/runtime ./internal/provider/openaicompat ./internal/app
  • go test ./...

未做项:未引入“旧平铺会话兼容读取/迁移”逻辑(按你当前触发评论要求,本轮仅修复这 3 项)。

View job run

…path guard

Generated with [codeagent](https://github.com/qbox/codeagent)
Co-authored-by: phantom5099 <245659304+phantom5099@users.noreply.github.com>
@phantom5099

Copy link
Copy Markdown
Collaborator Author

/code 将测试覆盖率提高100%

@fennoai

fennoai Bot commented Apr 17, 2026

Copy link
Copy Markdown
Contributor

已按指令补充测试并推送到当前 PR 分支 phantom5099/main(commit: 5803c17)。

结果

  • 已补测文件:
    • internal/session/store_test.go
    • internal/session/asset_store_test.go
    • internal/runtime/session_assets_test.go
    • internal/provider/openaicompat/chatcompletions/provider_test.go
  • 新增覆盖点:
    • session:非法 session id、todo_version 回填、summary 缺字段、原子写入/替换失败分支、无效 base path 异常分支
    • session_assetOpen/Stat 的非法 session id 与缺失文件分支
    • runtime session asset reader:无上下文时回退到 context.Background() 分支
    • chatcompletionsremainingBudget<=0 与负预算路径

验证

  • go test ./internal/session ./internal/runtime ./internal/provider/openaicompat/chatcompletions
  • go test ./...
  • go test -cover ./internal/session ./internal/runtime ./internal/provider/openaicompat/chatcompletions
    • internal/session: 89.5% -> 90.4%
    • internal/runtime: 95.5%
    • internal/provider/openaicompat/chatcompletions: 92.9% -> 93.6%

未达成项说明

当前仓库整体覆盖率及相关包覆盖率仍未达到 100%。剩余低覆盖主要集中在 internal/session/store.go 的部分 I/O 异常分支(如 write/sync/close/chmod 系统调用失败路径)和 toOpenAIMessageWithBudget 中受 ValidateParts 前置校验保护的不可达分支。

View job run

Generated with [codeagent](https://github.com/qbox/codeagent)
Co-authored-by: phantom5099 <245659304+phantom5099@users.noreply.github.com>
@phantom5099 phantom5099 merged commit ee095ef into 1024XEngineer:main Apr 17, 2026
2 checks passed
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.

3 participants