Skip to content

validate previous_response_id input type/length - #2961

Closed
aydnOktay wants to merge 1 commit into
NousResearch:mainfrom
aydnOktay:feat/api-server-previous-response-id-validation
Closed

validate previous_response_id input type/length#2961
aydnOktay wants to merge 1 commit into
NousResearch:mainfrom
aydnOktay:feat/api-server-previous-response-id-validation

Conversation

@aydnOktay

Copy link
Copy Markdown
Contributor

This PR hardens the OpenAI-compatible Responses API by validating previous_response_id input.
It rejects:
non-string previous_response_id values (400)
empty or excessively long IDs (400)

Additionally, it adds regression tests covering invalid type and too-long cases to prevent accidental future regressions.
Endpoints affected:
POST /v1/responses

@alt-glitch alt-glitch added type/security Security vulnerability or hardening P3 Low — cosmetic, nice to have comp/gateway Gateway runner, session dispatch, delivery labels May 2, 2026

@teknium1 teknium1 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.

Thanks for the input-hardening work. The /v1/responses premise remains valid on current main: previous_response_id is read without validation at gateway/platforms/api_server.py:3227 and supplied to the response store at :3286.

Problems

  • The same public field is parsed in POST /v1/runs at gateway/platforms/api_server.py:4199 and passed to ResponseStore.get() at :4223, but this PR validates only /v1/responses. The Runs API documents previous_response_id support at website/docs/user-guide/features/api-server.md:247, so the hardening is incomplete.

Suggested changes

  • Extract the validation into a shared helper and use it in both request parsers; add /v1/runs invalid-type and overlong-ID coverage.
  • The linked salvage PR #62758 already identifies these two body-parse sites and may be a useful current-main porting reference.

Automated hermes-sweeper review.

@@ -654,6 +655,28 @@ async def _handle_responses(self, request: "web.Request") -> "web.Response":
conversation = body.get("conversation")

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.

Please share this validation with the /v1/runs parser too. Current main reads the same field at gateway/platforms/api_server.py:4199 and passes it to ResponseStore.get() at :4223; the API docs advertise previous_response_id for runs.

@teknium1 teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 12, 2026
Bartok9 pushed a commit to Bartok9/hermes-agent that referenced this pull request Aug 1, 2026
…ge of NousResearch#2961 by @aydnOktay)

Rebuilt on latest main (Bartok9 hygiene 2026-08-01).
Original: NousResearch#62758
@aydnOktay

Copy link
Copy Markdown
Contributor Author

Closing to concentrate review on a single linked product bug: #89979.

@aydnOktay aydnOktay closed this Aug 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/security Security vulnerability or hardening

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants