Bound access log query and response sizes - #511
Conversation
|
PR Title: Bound access log query and response sizes Commit: 本次改动为访问日志的 admin HTTP 读取端点增加资源限额防护：1) accesslog 包新增 MaxFilterEntries(10,000) 与 MaxResponseBytes(64MiB) 常量，并新增 validateFilterLimits 校验上限，ReadFile/FilterLines 均会调用；2) admin 的 handleAccessLogs 改用 boundedLogBuffer 限制响应体积，超限时返回 413，同时 parseAccessLogQuery 对 limit/tail 查询参数增加上限校验；3) 新增两个单元测试。整体方向正确，覆盖了查询参数上限与响应体积上限两条路径。发现两个低严重度问题：ReadFile 将原有负 Limit 校验替换为 validateFilterLimits 后，当目标文件不存在时负 limit 会被静默接受（行为回归）；以及缺少响应超限 413 路径的 HTTP 层端到端测试。 |
![monkeyscan[bot]](https://avatars.githubusercontent.com/u/231811742?s=60&v=4){kind=small}
| if !strings.Contains(errAccessLogResponseTooLarge.Error(), "size limit") { | ||
| t.Fatal("oversize error does not describe the configured limit") | ||
| } | ||
| } |
There was a problem hiding this comment.
缺少响应超限（413）路径的端到端测试
本次改动的核心安全目标是限制 access log HTTP 响应体积，但新增测试只覆盖了 parseAccessLogQuery 的上限拒绝和 boundedLogBuffer 原语本身，缺少关键集成路径的验证：handleAccessLogs 在日志输出超过 MaxResponseBytes 时应返回 413、错误消息正确、且不会向客户端写出部分响应体。该路径涉及 ReadFile/FilterLines → boundedLogBuffer → errors.Is 错误传播与状态码映射，是本次改动最有回归风险的部分，目前没有任何测试覆盖，未来若对错误做包装或调整状态码映射，现有测试无法捕获回归。
Problem code:
Changed code at internal/admin/log_limits_test.go:22-34
Recommendation:
增加一个 handler 级测试：构造一个响应体积超过 MaxResponseBytes 的 access log（或将限额参数化以便注入较小值），调用 handleAccessLogs，断言状态码为 413、Content-Type 未被设置、且响应体不包含部分日志内容。
|
PR Title: Bound access log query and response sizes Commit: 本次变更在 internal/accesslog/accesslog.go 的 validateFilterLimits 中新增了对负 Limit 与负 Tail 的校验（分别返回 "limit must be non-negative" / "tail must be non-negative"）。 背景：此前负值校验只存在于 FilterLines 与 FollowFile 的内联代码中，而 ReadFile 在打开文件前只调用 validateFilterLimits（不含负值检查），导致目标日志文件不存在时负 limit 会被静默接受并返回成功——即已确认的历史问题 6a2cfd1d-8b5b-4567-9cda-c15abcac1695。本次改动将负值检查集中到 validateFilterLimits 后，ReadFile 在文件缺失时也能正确拒绝负 limit/tail，同时避免了 FilterLines 中 make([]byte, 0, 负容量) 的潜在 panic 路径。现有测试（TestReadFileEmptyMissingAndInvalidLimit、TestFilterLinesByFieldsAndLimits 等）已覆盖缺文件+负 limit、FilterLines 负 limit/tail 等场景，因此历史问题已随之修复，不再重复上报。 总体评估：改动方向正确、范围很小，属于对既有回归的缺陷修复，行为与错误信息与既有内联检查保持一致，未发现正确性或安全问题。遗留一个轻微的可维护性问题：FilterLines 开头仍保留与 validateFilterLimits 重复的内联负值检查，在本次改动后已不可达（死代码），且负值校验逻辑现在分散在 validateFilterLimits、FilterLines（死代码）与 FollowFile（内联、未调用 helper）三处，建议统一收敛。 |
|
PR Title: Bound access log query and response sizes Commit: 本次改动修改 internal/accesslog/accesslog.go，将分散的 limit/tail 校验收敛到 validateFilterLimits：1) FilterLines 删除紧随 validateFilterLimits 调用之后、永不可达的内联负值分支（死代码）；2) FollowFile 用 validateFilterLimits(filter) 替换原有的两段内联负值校验。 核对结论：validateFilterLimits 已完整包含对负 Limit/Tail 的校验及 MaxFilterEntries 上限校验，本次 diff 未改动其函数体。FilterLines 删除的分支确为不可达死代码；FollowFile 改为调用 validateFilterLimits 后，对负值输入保持原有"打开文件前快速失败"语义，错误消息一致；对超上限输入则由原先 waitOpen 阻塞后报错变为提前失败，属改进而非回归。FollowFile 内部仍调用 FilterLines，错误语义无漂移。 历史发现 cb3b2d46（三处重复校验 + FilterLines 死代码）已被本次改动实质解决：死代码已删除、FollowFile 已收敛到共享 helper，不应重复报告。 综合评估：这是一次行为等价或更优的校验收敛重构，未引入正确性/安全/可靠性回归，无可操作的新 finding。 |
|
Follow-up fixes after CI/code review:
Verification: |
1 participant
问题
访问日志接口只校验
limit和tail非负，没有最大值；非 follow 查询还会先将结果完整缓存在内存中。影响
恶意请求可以提交极大的
limit/tail，触发超大切片分配或让日志查询长期占用内存。超长日志行和大量结果还可能形成大响应，导致拒绝服务。修复内容
limit和tail最大限制为 10000 条。FilterLines、ReadFile、FollowFile共用边界校验，避免绕过 HTTP 层。413 Request Entity Too Large。验证
go test ./internal/accesslog ./internal/admin -run 'Test(ParseAccessLogQueryRejectsUnboundedEntryCounts|BoundedLogBufferRejectsOversizedResponse|AccessLog|ParseAccessLogQueryBoundaries)'通过。