Greptile Summary

This PR adds a security guard that blocks x-openclaw-session-key header overrides targeting internal session namespaces (subagent:, acp:, cron:) in the OpenAI-compatible and OpenResponses HTTP endpoints, returning a 400 invalid_request_error instead of forwarding the key to the agent layer.\n\nKey changes:\n- http-utils.ts: Adds resolveReservedSessionKeyPrefix / normalizeReservedSessionKeyCandidate helpers that correctly handle both bare keys (subagent:worker) and agent:<id>: scoped keys (agent:main:subagent:worker) in a case-insensitive, whitespace-tolerant way; consistent with existing semantics in isSubagentSessionKey, isCronSessionKey, and isAcpSessionKey.\n- openai-http.ts / openresponses-http.ts: Both now wrap resolveGatewayRequestContext in a try/catch that serialises the thrown error as a 400 response, preventing the request from reaching the agent layer.\n- Tests added for each endpoint and the utility function; the only minor gap is that only the subagent: prefix is exercised — acp: and cron: have no dedicated test cases.\n- Note: tools-invoke-http.ts accepts a body-supplied sessionKey via a separate code path without the same reserved-prefix guard — this appears to be a pre-existing gap outside the stated scope of this PR and may warrant a follow-up.

Confidence Score: 5/5

Safe to merge; the guard logic is correct and consistent with existing session-key semantics, and both HTTP endpoints are protected.

The only open finding is a P2 test-coverage gap (missing acp: and cron: cases alongside the existing subagent: test), which does not affect correctness of the shipped guard. No P0 or P1 issues found.

src/gateway/http-utils.request-context.test.ts — missing acp: and cron: prefix test cases.

This is a comment left during a code review.
Path: src/gateway/http-utils.request-context.test.ts
Line: 46-57

Comment:
**Only `subagent:` prefix tested, `acp:` and `cron:` cases uncovered**

The new test exercises only the `subagent:` reserved prefix. `RESERVED_SESSION_KEY_PREFIXES` also includes `acp:` and `cron:`, but there are no cases that verify those are blocked. A future refactor that drops or misspells one of those entries would break silently.

Consider adding additional `it` blocks (or parametrizing the existing one) to cover at least `acp:` and `cron:`:

```
// acp: prefix
{ "x-openclaw-session-key": "agent:main:acp:session" } → throws "...internal session namespace acp:"
// cron: prefix
{ "x-openclaw-session-key": "agent:main:cron:daily" }  → throws "...internal session namespace cron:"
// also test bare (non-agent-prefixed) variants e.g. "cron:daily"
```

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "fix(gateway): block internal HTTP sessio..." | Re-trigger Greptile}

Thanks for the context here. I swept through the related work, and this is now duplicate or superseded.

Close: the same gateway session-override fix was merged through the replacement PR #93443, and current main has the guard, endpoint handling, tests, and docs that this source branch was trying to add.

Root-cause cluster
Relationship: superseded
Canonical: #93443
Summary: The source PR is superseded by the merged replacement PR that implemented the same central gateway HTTP session-override guard on current main.

Members:

• canonical: fix(gateway): block internal HTTP session overrides #93443 - Merged replacement PR explicitly repairs and finalizes this source work and is present on current main.
• partial_overlap: fix(gateway): block internal session history over HTTP #55928 - Related internal-session HTTP boundary work for session history, but it targeted a different endpoint and was closed unmerged.
• adjacent_distinct: fix(hooks): reject internal session namespaces #55767 - Adjacent reserved internal namespace hardening for hooks, but it is a separate ingress path and not the canonical fix for this PR.

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Canonical path: Keep the merged replacement implementation on current main and close this source PR with contributor credit preserved by the replacement PR.

So I’m closing this here because the remaining work is already tracked in the canonical issue.

Refreshed this PR onto latest upstream/main (99950c7f1272dff6e2c34c2be45dfc5f89e62a60) and force-pushed the updated branch.

New head: e58b3893aee1fcece10a02f8ff24bde298b45ec5

What changed:

• Rebuilt the original minimal fix on top of current main.
• Rejects explicit x-openclaw-session-key overrides targeting internal namespaces over the OpenAI-compatible HTTP surfaces.
• Covers subagent:, acp:, and cron: namespaces, including agent-scoped forms via parseAgentSessionKey.
• Keeps error handling narrow: only GatewaySessionKeyOverrideError is mapped to HTTP 400; unexpected errors still surface through the normal server error path.

Review:

• Fresh gpt-5.5 xhigh review found no important issues requiring changes before push.
• Prior review feedback was considered:
  • Added acp: and cron: coverage alongside subagent: in http-utils.request-context.test.ts.
  • Kept the dedicated GatewaySessionKeyOverrideError so only invalid session override input maps to 400; unrelated context/config failures still surface through the normal server error path.
  • The noted tools-invoke-http.ts body-supplied sessionKey path was intentionally not folded into this PR to keep scope limited to OpenAI-compatible/OpenResponses header overrides. That separate HTTP ingress can be tracked as follow-up if maintainers want a broader all-HTTP policy.

Verification:

• Passed: ./node_modules/.bin/vitest run --config test/vitest/vitest.gateway-core.config.ts src/gateway/http-utils.request-context.test.ts
• Result: 1 file passed, 14 tests passed.
• Blocked locally: src/gateway/openai-http.test.ts / src/gateway/openresponses-http.test.ts failed during import because this local node_modules is missing declared dependencies (typebox, global-agent). No endpoint assertions ran in those suites.

ClawSweeper PR egg

🎁 Pass real behavior proof to wake the egg and unlock a hatchable treat.

Clownfish 🐠 reef update

Thanks for the work on this. Clownfish could not push to this branch with the permissions available, so it opened a narrow replacement PR to keep the fix swimming forward without losing the contributor trail. not your fault, just GitHub branch-permission tides.

Replacement PR: #93443
Source PR: #55789
This source PR remains open; the replacement is just the writable fix lane.
The replacement PR keeps the contributor trail visible for review and changelog credit.

fish notes: model gpt-5.5, reasoning medium; reviewed against b18a288.

ClawSweeper applied the proposed close for this PR.

• Action: closed this PR.
• Close reason: duplicate or superseded.
• Evidence: durable ClawSweeper review.
• Coverage proof: PR B clearly implements and merged the same core behavior PR A was trying to land, with current-main-compatible handling and verification. Any leftover PR A details are incidental or reviewable on the merged replacement, so PR A does not need independent review. Covering PR: fix(gateway): block internal HTTP session overrides #93443.