fix(hooks): rebind hook agent session keys to the target agent by vincentkoc · Pull Request #58225 · openclaw/openclaw

vincentkoc · 2026-03-31T07:58:01Z

Summary

Problem: hook-triggered isolated runs could keep the agent id embedded in an incoming agent: session key even when the run was explicitly routed to a different target agent.
Why it matters: dedicated hook agents could inherit the wrong session-scoped tool and plugin identity from that preserved prefix.
What changed: hook dispatch now rewrites mismatched agent: session keys to the actual target agent, keeps duplicate-target stripping intact, and adds focused unit/server tests for both cases.
What did NOT change: hook session-key prefix policy and request-session-key enablement rules are unchanged.

Root Cause

The hook dispatch normalizer handled duplicate target-agent prefixes, but when the incoming key was already agent-scoped for a different agent it preserved that foreign prefix instead of rebinding it to the real target agent.

Test Plan

Added/updated coverage in src/gateway/hooks.test.ts.
Added/updated coverage in src/gateway/server.hooks.test.ts.
Tried: pnpm test -- src/gateway/hooks.test.ts src/gateway/server.hooks.test.ts
Tried: direct Vitest run for the same gateway files.
Ran: pnpm check

Validation Notes

pnpm check currently fails in untouched code at src/tasks/flow-registry.ts:252 and src/tasks/flow-registry.ts:253 with string | null to string | undefined type errors.
The gateway Vitest lane in this worktree did not return a clean completion signal during repeated targeted reruns, even after dependency install and a direct Vitest invocation.

Human Verification

Verified the helper now rewrites agent:<other>:... to agent:<target>:....
Verified server hook dispatch forwards the rebound key into the isolated runner.

Compatibility

Backward compatible: Yes
Config/env changes: No
Migration needed: No

AI-assisted: yes

greptile-apps · 2026-03-31T08:00:29Z

Greptile Summary

This PR fixes a one-line bug in normalizeHookDispatchSessionKey (src/gateway/hooks.ts) where incoming session keys scoped to a different agent (e.g. agent:main:slack:channel:c123) were preserved as-is when dispatching to a dedicated hook agent, allowing the foreign agent identity to leak into the isolated run. The fix changes parsed.agentId to targetAgentId in the return value so the key is rebound to the actual target agent before dispatch.

Key changes:

src/gateway/hooks.ts: Single-line fix — replace parsed.agentId with targetAgentId so mismatched agent: prefixes are rebound rather than forwarded verbatim.
src/gateway/hooks.test.ts: Updates the existing unit test to assert the new (correct) rebind behavior.
src/gateway/server.hooks.test.ts: Adds a new integration test verifying the full dispatch path rewrites agent:main:… → agent:hooks:… before the isolated runner is invoked.
CHANGELOG.md: Adds an entry for the fix.

Confidence Score: 5/5

Safe to merge — the fix is minimal, correct, and backed by both unit and integration tests with no regressions.

The change is a single-line fix to a clear bug where parsed.agentId was mistakenly used instead of targetAgentId. The surrounding guard conditions and the existing same-agent strip path are unchanged. Both the unit test and the new integration test confirm the correct rebind behavior end-to-end. No P0 or P1 issues were found.

No files require special attention.

_{Reviews (1): Last reviewed commit: "fix(hooks): rebind hook agent session ke..." | Re-trigger Greptile}

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a778dfa83d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

aisle-research-bot · 2026-03-31T15:16:33Z

🔒 Aisle Security Analysis

We found 1 potential security issue(s) in this PR:

#	Severity	Title
1	🔵 Low	Hooks endpoint discloses allowed session key prefixes in error responses

1. 🔵 Hooks endpoint discloses allowed session key prefixes in error responses

Property	Value
Severity	Low
CWE	CWE-209
Location	`src/gateway/server-http.ts:620-626`

Description

The hooks HTTP handler returns an error string that includes the full configured allowlist of allowedSessionKeyPrefixes when a session key fails the prefix policy check.

Input: attacker-controlled sessionKey (via /hooks/agent payload or via mappings output) is normalized and checked against the configured prefixes.
Behavior: on failure, the API returns a 400 JSON response containing getHookSessionKeyPrefixError(allowedPrefixes), which embeds the full allowlist (e.g., internal namespace/prefix conventions).
Impact: anyone who can reach this code path with a valid hook token can probe and enumerate internal configuration details (session key namespace/policy), which may aid targeted attacks or reveal internal identifiers.

Vulnerable response construction:

sendJson(res, 400, { ok: false, error: getHookSessionKeyPrefixError(allowedPrefixes) });

And the error string includes the allowlist:

`sessionKey must start with one of: ${prefixes.join(", ")}`

Recommendation

Avoid echoing the full configured allowlist to external callers.

Return a generic error message (and optionally a stable error code) to the client.
Log the detailed allowlist server-side for debugging.

Example:

if (allowedPrefixes && !isSessionKeyAllowedByPrefix(normalizedDispatchSessionKey, allowedPrefixes)) {
  logHooks.warn({ allowedPrefixes }, "hook sessionKey rejected by prefix policy");
  sendJson(res, 400, { ok: false, error: "sessionKey is not allowed by policy" });
  return true;
}

If you need client guidance, consider returning only the count of allowed prefixes or a link to documentation rather than the literal prefixes.

Analyzed PR: #58225 at commit 6781e0d

_{Last updated on: 2026-03-31T15:18:51Z}

…law#58225) * fix(hooks): rebind hook agent session keys * fix(hooks): preserve scoped hook session keys * fix(hooks): validate normalized dispatch keys

vincentkoc self-assigned this Mar 31, 2026

openclaw-barnacle Bot added gateway Gateway runtime size: XS maintainer Maintainer-authored PR labels Mar 31, 2026

chatgpt-codex-connector Bot reviewed Mar 31, 2026

View reviewed changes

Comment thread src/gateway/hooks.ts Outdated

fix(hooks): rebind hook agent session keys

04bbf27

vincentkoc force-pushed the triage-9xq6-hook-session-key branch from a778dfa to 04bbf27 Compare March 31, 2026 08:59

fix(hooks): preserve scoped hook session keys

1e57f29

openclaw-barnacle Bot added size: S and removed size: XS labels Mar 31, 2026

vincentkoc added 3 commits March 31, 2026 22:35

Merge branch 'main' into triage-9xq6-hook-session-key

c0a7de8

fix(hooks): validate normalized dispatch keys

7c1d2dc

Merge branch 'main' into triage-9xq6-hook-session-key

6781e0d

vincentkoc merged commit 1ca12ec into main Mar 31, 2026
9 checks passed

vincentkoc deleted the triage-9xq6-hook-session-key branch March 31, 2026 15:16

github-actions Bot mentioned this pull request Mar 31, 2026

📡 Upstream Digest — 2026-03-31 16:50 UTC curtismercier/openclaw-mods#423

Open

clawsweeper Bot mentioned this pull request May 2, 2026

feat: add before_block_reply plugin hook for block streaming interception #58445

Closed

clawsweeper Bot mentioned this pull request May 10, 2026

[Feature]: Configurable auto-cleanup and archival policy for webhook/hook sessions #10253

Closed

clawsweeper Bot mentioned this pull request May 21, 2026

[Bug]: hooks.mappings[].agentId and sessionKey silently ignored for action="wake" #64556

Open

clawsweeper Bot mentioned this pull request Jun 23, 2026

fix: route wake hooks to explicit sessions #70268

Closed

This was referenced Jul 2, 2026

feat(hooks): add waitForResult mode to POST /hooks/agent endpoint #67433

Open

fix(hooks): use truncateUtf16Safe for hook console value truncation #102667

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(hooks): rebind hook agent session keys to the target agent#58225

fix(hooks): rebind hook agent session keys to the target agent#58225
vincentkoc merged 5 commits into
mainfrom
triage-9xq6-hook-session-key

vincentkoc commented Mar 31, 2026

Uh oh!

greptile-apps Bot commented Mar 31, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

Uh oh!

aisle-research-bot Bot commented Mar 31, 2026 •

edited

Loading

Description

Recommendation

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

vincentkoc commented Mar 31, 2026

Summary

Root Cause

Test Plan

Validation Notes

Human Verification

Compatibility

Uh oh!

greptile-apps Bot commented Mar 31, 2026

Greptile Summary

Confidence Score: 5/5

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

aisle-research-bot Bot commented Mar 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔒 Aisle Security Analysis

Description

Recommendation

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

aisle-research-bot Bot commented Mar 31, 2026 •

edited

Loading