fix(serve-status): validate goal_id to block path traversal in reward routes - #3138
Merged
Merged
Conversation
… routes append_human_reward built the run-index path from a caller-supplied goal_id with no validation, letting an unauthenticated serve-status request read arbitrary files or append to arbitrary JSONL files. Add a compact goal-id validator used by append_human_reward and by the reward and configure-goal request parsers, failing closed with a 400 before any filesystem access. Addresses GHSA-hfmf-6xvq-9jw6.
huangruiteng
commented
Aug 12, 2026
huangruiteng
left a comment
huangruiteng
left a comment
Owner
Author
There was a problem hiding this comment.
Approval conclusion (author-owned PR; GitHub blocks formal self-approval)
中文结论
精确评审头： 3138@64bda433f（codex/security-serve-status-goal-id）
goal_id 强制单路径段校验：append_human_reward 在 I/O 前拒绝穿越，serve-status reward/configure-goal 路由对非法 goal_id 返回 400。exact head 上 16 项新增测试（含 HTTP 集成）+ reward-memory/trajectory-hygiene 回归全部通过。CI pytest 变红为 main 基线问题，与本 PR 无关。
结论：APPROVE（author-owned，以评论记录）。
English Verdict
APPROVE — exact head 64bda433f. goal_id is validated as a single path segment before any I/O; reward and configure-goal routes fail closed with HTTP 400 on traversal attempts. 16 new tests and regression suites pass; CI pytest red is a confirmed baseline failure.
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
1 participant
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
append_human_rewardbuilt the run-index path directly from a caller-suppliedgoal_id(runtime_root/goals/{goal_id}/runs/index.jsonl) with no validation. An unauthenticatedserve-statusrequest could therefore read arbitrary files or append to arbitrary existing JSONL files.Change
validate_goal_id()inloopx/feedback.py: rejects empty ids, path separators,.., and shell metacharacters (single-path-segment token).append_human_rewardvalidates first, before any filesystem access.serve-statusreward and configure-goal request parsers validategoal_idas defense in depth; invalid ids fail closed with HTTP 400.Validation
tests/test_feedback_goal_id_validation.py: 16 tests (safe/rejected id matrix, append rejection before I/O, reward + configure-goal HTTP 400 integration) - all pass.tests/capabilities/test_reward_memory_ingestion.py+tests/test_trajectory_hygiene.pypass (no regression).py_compileclean.Addresses GHSA-hfmf-6xvq-9jw6 (details kept private pending coordinated disclosure).