Bridge Codex native hooks into OpenClaw#71008
Conversation
🔒 Aisle Security AnalysisWe found 4 potential security issue(s) in this PR:
1. 🟠 Memory exhaustion risk: native hook relay stores unbounded rawPayload in global invocation buffer
DescriptionThe native hook relay records the entire
Vulnerable code: function recordNativeHookRelayInvocation(invocation: NativeHookRelayInvocation): void {
invocations.push(invocation);
if (invocations.length > MAX_NATIVE_HOOK_RELAY_INVOCATIONS) {
invocations.splice(0, invocations.length - MAX_NATIVE_HOOK_RELAY_INVOCATIONS);
}
}RecommendationAvoid retaining full, unbounded payloads in memory. Recommended mitigations (pick one or combine):
Example: size-bound storage with redaction/truncation: const MAX_INVOCATION_PAYLOAD_BYTES = 256 * 1024; // 256KB
function safePayloadForRecord(raw: JsonValue): JsonValue {
const text = JSON.stringify(raw);
if (Buffer.byteLength(text, "utf8") <= MAX_INVOCATION_PAYLOAD_BYTES) return raw;
return {
__truncated: true,
__bytes: Buffer.byteLength(text, "utf8"),
preview: text.slice(0, 4096),
} as unknown as JsonValue;
}
function recordNativeHookRelayInvocation(invocation: NativeHookRelayInvocation): void {
invocations.push({ ...invocation, rawPayload: safePayloadForRecord(invocation.rawPayload) });
// existing ring buffer logic...
}Also consider not storing tool inputs/responses by default (or redacting known-sensitive keys) unless an explicit debug flag is enabled. 2. 🟡 Native hook relay uses relayId as bearer token and exposes it via command-line arguments
DescriptionThe native hook relay authentication/authorization relies on possession of a Impact if
Vulnerable code (relayId embedded in command; invocation authorized by relayId lookup): // command includes --relay-id <relayId>
return shellQuoteArgs([
...argv,
"hooks",
"relay",
"--provider",
params.provider,
"--relay-id",
params.relayId,
"--event",
params.event,
]);
// invocation authorizes by relayId only
const relayId = readNonEmptyString(params.relayId, "relayId");
const registration = relays.get(relayId);RecommendationAvoid treating Recommended mitigations (use one or combine):
Example sketch (HMAC-bound token): // when creating command/config
const nonce = randomUUID();
const mac = hmacSha256(sessionKeySecret, `${relayId}:${runId}:${event}:${nonce}`);
// pass nonce+mac (not relayId alone) to CLI
// on invoke
verifyHmac(sessionKeySecret, `${relayId}:${runId}:${event}:${nonce}`, mac);
assertAndConsumeNonce(relayId, nonce);3. 🟡 Codex native tool hook relay ignores adjusted params from before_tool_call hooks (sanitization bypass)
DescriptionThe Codex native hook relay forwards Codex
This creates a gap where existing or future security plugins that rely on argument rewriting (e.g., stripping dangerous shell flags, forcing safe working directories, removing network destinations) may silently become ineffective for Codex-native tool executions, potentially allowing unsafe tool invocations that would otherwise have been mitigated. Vulnerable code: const outcome = await runBeforeToolCallHook({ toolName, params: toolInput, ... });
if (outcome.blocked) {
return params.adapter.renderPreToolUseBlockResponse(outcome.reason);
}
// ... intentionally ignore adjusted params
return params.adapter.renderNoopResponse(params.invocation.event);RecommendationAvoid silently ignoring adjusted params for Codex-native tools. Options (pick one):
if (!outcome.blocked && outcome.params && !deepEqual(outcome.params, toolInput)) {
return params.adapter.renderPreToolUseBlockResponse(
"Tool blocked: parameter mutation requested but not supported for Codex-native tools"
);
}
Also update plugin-facing docs/contracts to explicitly state that parameter rewrites are not enforced for Codex-native tool calls and should use blocking/approval instead. 4. 🟡 Approval spoofing via unvalidated `pluginId` in `plugin.approval.request` (native hook relay uses synthetic ID)
DescriptionThe native hook relay permission flow creates approvals via
This enables approval identity spoofing / UI confusion: any component able to call Vulnerable code (synthetic ID used for security-sensitive approval UI): pluginId: `openclaw-native-hook-relay-${request.provider}`,RecommendationBind approval identity to an authenticated/authorized principal rather than caller-controlled strings. Options:
Example (conceptual): // native hook relay should not claim to be a plugin
await callGatewayTool("plugin.approval.request", opts, {
pluginId: null,
requesterType: "native_hook_relay",
requesterId: `native-hook-relay:${request.provider}`,
title,
description,
...
});And on the gateway handler: if (params.pluginId != null) {
// verify params.pluginId belongs to the authenticated plugin principal
assertCallerPluginIdMatches(params.pluginId, context);
}Analyzed PR: #71008 at commit Last updated on: 2026-04-24T15:36:50Z |
Greptile SummaryThis PR adds a provider-neutral native harness hook relay foundation: an in-process relay registry, a hidden
Confidence Score: 4/5Safe to merge after addressing the unbounded invocations array; all other infrastructure is well-structured and tested. Two P1 comments both point at the same root cause: the module-level src/agents/harness/native-hook-relay.ts — the Prompt To Fix All With AIThis is a comment left during a code review.
Path: src/agents/harness/native-hook-relay.ts
Line: 98
Comment:
**Unbounded in-memory invocations array — memory leak**
`invocations` accumulates every hook call for the lifetime of the process and is never pruned. `unregisterNativeHookRelay` only removes the relay from `relays`; the corresponding entries stay in `invocations` forever. Each entry retains the full `rawPayload` (arbitrary JSON that can include tool outputs). A long-running OpenClaw process handling many agent sessions will grow unboundedly here.
Since the PR description explicitly notes invocations are kept for "later mapping" in follow-up PRs, even a simple cap (e.g. a ring-buffer of the last N entries, or draining entries when a relay is unregistered) would prevent the leak from landing on main.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: src/agents/harness/native-hook-relay.ts
Line: 144-146
Comment:
**`unregister` does not clean up associated invocations**
`unregisterNativeHookRelay` removes the relay registration from `relays` but leaves all invocations it produced sitting in the `invocations` array. After a Codex run completes and the relay is unregistered, the run's tool-use payloads continue to occupy memory indefinitely. This is the companion to the unbounded-array concern above — fixing either one addresses both.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "Add native harness hook relay foundation" | Re-trigger Greptile |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7d2eaeb921
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
43a9a24 to
b98bba6
Compare
7b47724 to
d49b63a
Compare
OpenClaw’s Codex path now has the native hook bridge we were building toward, not just the relay foundation. The important split is still the same: OpenClaw dynamic tools are already executed by OpenClaw, while Codex-native tools like shell and apply patch run inside Codex. This PR gives those Codex-native tool events a controlled way back into OpenClaw’s plugin and approval surfaces.
The core relay stays provider-neutral. A native harness hook command can call
openclaw hooks relay, identify the active relay/session/run, and send a normalizedpre_tool_use,post_tool_use, orpermission_requestevent back through the authenticated gateway. Codex is the first adapter, but the relay shape is deliberately not Codex-only, so a future Claude Code adapter can reuse the same registry, gateway method, CLI command, and failure behavior.For Codex, the app-server adapter now injects per-thread hook config on both
thread/startandthread/resume. It enables onlyPreToolUse,PostToolUse, andPermissionRequest, uses Codex config overrides directly, and does not write user or global hook files. The relay is registered for the turn, cleaned up on success, failure, or abort, and scoped by provider, relay id, session id, session key, run id, allowed events, and expiry.The behavior now maps into the v1 contract:
PreToolUsecalls OpenClawbefore_tool_callfor Codex-native tools. Plugins can block; returned argument rewrites are intentionally ignored because Codex hooks do not support safe argument mutation yet.PostToolUsecalls OpenClawafter_tool_callwith the native tool input and response, giving plugins an observation path for Codex-native results.PermissionRequestgoes through OpenClaw approval policy. An allow or deny decision is rendered back into Codex hook JSON; if OpenClaw cannot decide, the relay defers so Codex’s normal permission path can continue.The concrete user story this fixes is a security plugin like Knostic Shield. Before this PR,
openclaw-shieldcould register its L3before_tool_callblocker, but Codex-native Bash calls could slip past that plugin surface because the shell execution happened inside Codex. I cloned the realknostic/openclaw-shieldplugin, loaded its actualregister(api)implementation into OpenClaw’s hook runner, and sent the relay a Codex-shapedPreToolUsepayload forBashwithrm -rf /tmp/openclaw-shield-smoke-target. The plugin registeredbefore_tool_callat priority 200, the relay returned Codex-compatible JSON withpermissionDecision: "deny", and the reason contained Knostic’s destructive-command block message. The same smoke sent a harmlessprintfcommand and got the expected empty stdout/stderr with exit code 0, meaning Codex would proceed normally.There are still deliberate v1 boundaries. This does not mutate native tool arguments, does not rewrite Codex’s native thread records, does not make
tool_result_persistapply to Codex-native tool records, and does not expose Stop/final-answer gating. Those are separate product decisions, not accidental gaps in this bridge.