Project Codex hook notifications into agent events#70969
Conversation
🔒 Aisle Security AnalysisWe found 1 potential security issue(s) in this PR:
1. 🟡 Unbounded hook output forwarded into agent events can cause memory/CPU/network DoS
Description
This is risky because hook output can be influenced by:
As a result, a malicious (or buggy) hook can cause:
Vulnerable code: const entries = readHookOutputEntries(run.entries);
...
...(entries.length > 0 ? { entries } : {}),And RecommendationApply defensive caps/truncation before emitting hook output into agent events. Suggested approach:
Example: const MAX_ENTRIES = 50;
const MAX_ENTRY_TEXT = 4096;
const MAX_STATUS_MESSAGE = 2048;
function truncate(s: string, max: number): string {
return s.length > max ? s.slice(0, max) + "…" : s;
}
const rawEntries = readHookOutputEntries(run.entries);
const entries = rawEntries.slice(0, MAX_ENTRIES).map(e => ({
...("kind" in e ? { kind: e.kind } : {}),
text: truncate(e.text, MAX_ENTRY_TEXT),
}));
const statusMessage = readNullableString(run, "statusMessage");
const safeStatusMessage =
typeof statusMessage === "string" ? truncate(statusMessage, MAX_STATUS_MESSAGE) : statusMessage;
this.emitAgentEvent({
stream: "codex_app_server.hook",
data: {
...,
statusMessage: safeStatusMessage,
entries,
hookOutputTruncated: rawEntries.length > MAX_ENTRIES,
},
});This reduces the risk of IDE/extension instability and oversized telemetry payloads when opening untrusted projects. Analyzed PR: #70969 at commit Last updated on: 2026-04-24T06:39:27Z |
Greptile SummaryThis PR projects Codex native The implementation is clean and consistent with the existing projector patterns. The only minor inconsistency is that Confidence Score: 5/5Safe to merge — all findings are P2 style suggestions with no correctness impact. The change is well-scoped, turn-scoping is correctly inherited from the existing gate, the new handler and helpers are logically sound, and test coverage is solid. The single comment is a minor style inconsistency that does not affect runtime behavior. No files require special attention. Prompt To Fix All With AIThis is a comment left during a code review.
Path: extensions/codex/src/app-server/event-projector.ts
Line: 661-669
Comment:
**`readNumberLike` is more permissive than needed**
`durationMs` in the Codex hook protocol is always a `number` when present, yet `readNumberLike` adds a string-coercion branch that silently converts `"42"` → `42`. The existing `readNumber` helper already handles the numeric case correctly and is used everywhere else in this file. If string coercion is genuinely required (e.g., because the Codex server can serialize this field as a string), a comment explaining that would help; otherwise `readNumber` keeps the codebase consistent and avoids masking unexpected type mismatches.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "project codex hook notifications" | Re-trigger Greptile |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 756c44dde8
ℹ️ 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".
* 'main' of https://github.com/openclaw/openclaw: (521 commits) perf: slim reset model selection imports perf: slim directive parse test imports ci(release): clarify npm telegram approval label test: relax live tail readiness checks ci(release): use release approval for npm telegram e2e Project Codex hook notifications into agent events (openclaw#70969) fix: match codex verbose tool logs to pi (openclaw#70966) feat(diagnostics): attach trace context to otel logs (openclaw#70961) docs(faq): tighten FAQ stub pointers and Related cards docs(help): rewrite help index to match new tab structure, drop redundant troubleshooting H1 refactor(release): distill npm telegram docker runner ci(release): gate npm telegram e2e by release team ci(release): harden npm telegram beta e2e test(release): address npm telegram e2e review test(release): avoid docker argv secret values ci(release): add manual npm telegram beta e2e test(release): support convex npm telegram credentials docs: clarify private ws node setup fix: filter gateway node list locally feat(plugins): move Bonjour discovery into bundled plugin ...
* project codex hook notifications * keep codex hook duration strict * include thread scoped codex hook notifications
* project codex hook notifications * keep codex hook duration strict * include thread scoped codex hook notifications
* project codex hook notifications * keep codex hook duration strict * include thread scoped codex hook notifications
* project codex hook notifications * keep codex hook duration strict * include thread scoped codex hook notifications
* project codex hook notifications * keep codex hook duration strict * include thread scoped codex hook notifications
* project codex hook notifications * keep codex hook duration strict * include thread scoped codex hook notifications
* project codex hook notifications * keep codex hook duration strict * include thread scoped codex hook notifications
This is the next small piece of Codex app-server notification projection.
Most of the adapter-level parity work is already present on main now: Codex mode emits
llm_input,llm_output, andagent_end, mirrors transcript writes throughbefore_message_write, projects item lifecycle events, carries token usage, and fires compaction hooks as notification-level observations.One app-server notification family was still being ignored: Codex native hook lifecycle notifications. Codex now emits
hook/startedandhook/completedevents for its own native hooks. Those are not OpenClaw plugin hooks and should not be treated as if they are, but they are useful runtime evidence for trajectories and debugging.This change projects those notifications into
codex_app_server.hookagent events with normalized metadata such as the hook run id, event name, handler type, source, status, duration, and output entries. That lets OpenClaw observe the native Codex hook lifecycle without pretending it owns or can rewrite that lifecycle.I also added the focused projector coverage for started and completed hook notifications, and updated the Codex harness docs to state the boundary clearly: these notifications become debug/trajectory agent events, not OpenClaw plugin hook invocations.