fix(hooks): bound hook workspace manifest and HOOK.md reads by cxbAsDev · Pull Request #101472 · openclaw/openclaw

cxbAsDev · 2026-07-07T07:59:02Z

What Problem This Solves

Workspace hook discovery read package.json and HOOK.md through validated file descriptors without bounding the allocated content. A malicious or runaway hook descriptor could therefore exhaust memory during discovery.

Why This Change Was Made

Both files are small hook metadata. Discovery now reads each already-pinned descriptor through the shared bounded reader with the maintainer-approved 1 MiB limit. This keeps the existing path, symlink, hardlink, and descriptor-identity checks while also bounding files that grow after open.

User Impact

Oversized hook metadata is warned about and skipped. Discovery continues for other hooks, and an oversized package manifest still falls back to a plain HOOK.md in the same directory.

Maintainer Changes

Rebased onto current main.
Reused one metadata limit and the canonical descriptor-pinned reader.
Consolidated the regression coverage while retaining both oversized surfaces, continued discovery, exact-limit acceptance, and plain-hook fallback.

Evidence

node scripts/run-vitest.mjs src/hooks/workspace.test.ts — 10 tests passed.
oxlint src/hooks/workspace.ts src/hooks/workspace.test.ts — passed.
git diff --check origin/main...HEAD — passed.
Full-branch autoreview — clean, no accepted/actionable findings.

Co-authored-by: 陈宪彪0668000387 [email protected]

clawsweeper · 2026-07-07T08:15:49Z

Codex review: needs maintainer review before merge. Reviewed July 17, 2026, 6:11 AM ET / 10:11 UTC.

Summary
Caps workspace hook package.json and HOOK.md descriptor reads at 1 MiB, warns and skips oversized metadata, and adds boundary, continuation, exact-limit, and fallback coverage.

PR surface: Source +13, Tests +94. Total +107 across 2 files.

Reproducibility: yes. A workspace containing a package.json or HOOK.md larger than 1 MiB reaches an unbounded descriptor read on current main; the PR discussion also demonstrates the real discovery path skipping those files after the fix.

Review metrics: 1 noteworthy metric.

Bounded metadata reads: 2 reads capped at 1 MiB. Both workspace package.json and HOOK.md now share one canonical pinned-descriptor limit.

Stored data model
Persistent data-model change detected: serialized state: src/hooks/workspace.test.ts. Confirm migration or upgrade compatibility proof before merge.

Merge readiness
Overall: 🦞 diamond lobster
Proof: 🦞 diamond lobster
Patch quality: 🦞 diamond lobster
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Risk before merge

[P1] Existing workspace hooks with metadata larger than 1 MiB were previously accepted but will now be skipped with a warning, so an upgrade can disable unusually large custom hooks until their metadata is reduced.

Maintainer options:

Land the bounded contract (recommended)
Accept that oversized existing metadata is skipped with a warning; this matches the maintainer-refined head and the adjacent hook-install hardening.
Revise the compatibility boundary
If metadata above 1 MiB must remain supported, choose a larger cap or explicit upgrade guidance before merging.

Next step before merge

No automated repair remains; maintainers can complete normal exact-head CI and merge review.

Security
Cleared: The patch reduces memory-exhaustion exposure using an existing bounded-read helper and introduces no new dependency, secret, permission, download, or code-execution surface.

Review details

Best possible solution:

Land the canonical 1 MiB pinned-descriptor limit with warning-and-skip semantics once exact-head checks complete, preserving continued discovery and the plain HOOK.md fallback.

Do we have a high-confidence way to reproduce the issue?

Yes. A workspace containing a package.json or HOOK.md larger than 1 MiB reaches an unbounded descriptor read on current main; the PR discussion also demonstrates the real discovery path skipping those files after the fix.

Is this the best way to solve the issue?

Yes. Reusing the canonical descriptor-pinned bounded reader at the existing workspace discovery boundary is narrower and safer than adding another reader or changing hook resolution policy.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 4880231ff710.

Label changes

Label justifications:

P2: This is a bounded reliability and memory-exhaustion hardening fix for workspace hook discovery with limited blast radius.
merge-risk: 🚨 compatibility: The new hard limit can cause previously accepted oversized workspace hook metadata to be skipped after upgrade.
rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The discussion provides after-fix live output from the real workspace discovery function using normal and oversized temporary metadata, supplemented by focused boundary and fallback tests; posted evidence should remain free of private paths or secrets.
proof: sufficient: Contributor real behavior proof is sufficient. The discussion provides after-fix live output from the real workspace discovery function using normal and oversized temporary metadata, supplemented by focused boundary and fallback tests; posted evidence should remain free of private paths or secrets.

Evidence reviewed

PR surface:

Source +13, Tests +94. Total +107 across 2 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	15	2	+13
Tests	1	95	1	+94
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	2	110	3	+107

What I checked:

Current-main behavior: Current main still reads the validated workspace hook descriptor with an unbounded fs.readFileSync, so the central memory-exhaustion problem remains and this PR is still necessary. (src/hooks/workspace.ts:303, 4880231ff710)
Bounded implementation: The PR routes both workspace package.json and HOOK.md reads through readFileDescriptorBoundedSync with one 1 MiB metadata limit while retaining the descriptor pinned by the root-file boundary helper. (src/hooks/workspace.ts:22, 7e271d92946a)
Failure behavior: Oversized descriptor reads are converted into an actionable warning and a skipped metadata file rather than aborting discovery; an oversized manifest can still fall back to plain-hook discovery. (src/hooks/workspace.ts:303, 7e271d92946a)
Regression coverage: The changed tests cover oversized manifests and HOOK.md files, continued discovery of valid hooks, exact-limit acceptance, warning content, and plain-hook fallback. (src/hooks/workspace.test.ts:72, 7e271d92946a)
Sibling contract: The merged install-validation hardening at fix(hooks): bound HOOK.md reads during hook install validation #101469 already applies a 1 MiB bounded-read contract to hook installation metadata, supporting a consistent hook metadata boundary across the two distinct entry points. (src/hooks/install.ts, 5d34ea774a79)
Review continuity: Earlier findings requiring the canonical bounded descriptor reader were addressed; subsequent completed ClawSweeper cycles reported no findings, and the final head includes a maintainer-authored tightening commit. (src/hooks/workspace.ts:300, 7e271d92946a)

Likely related people:

steipete: Directly refined the PR head through the final refactor(hooks): tighten bounded metadata coverage commit and is the strongest available routing candidate for landing the selected compatibility boundary. (role: recent area contributor and branch refiner; confidence: high; commits: 7e271d92946a; files: src/hooks/workspace.ts, src/hooks/workspace.test.ts)
cxbAsDev: Implemented the merged hook-install metadata bound and the current workspace-discovery hardening series, providing recent history across both hook metadata entry points. (role: recent adjacent hardening contributor; confidence: high; commits: 5d34ea774a79, 1df73d475269, 24dda6637b93; files: src/hooks/install.ts, src/hooks/install.test.ts, src/hooks/workspace.ts)

What the crustacean ranks mean

🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works

ClawSweeper keeps one durable marker-backed review comment per issue or PR.
Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
Maintainers can also comment @clawsweeper review to request a fresh review only.
Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

Review history (29 earlier review cycles; latest 8 shown)

reviewed 2026-07-14T13:46:27.870Z sha 0db6b9d :: found issues before merge. :: [P1] Use the canonical bounded descriptor reader
reviewed 2026-07-15T04:21:43.867Z sha 62e07ea :: found issues before merge. :: [P1] Use the canonical bounded descriptor reader
reviewed 2026-07-16T02:38:31.644Z sha daa3b31 :: needs maintainer review before merge. :: none
reviewed 2026-07-16T03:37:58.324Z sha daa3b31 :: needs maintainer review before merge. :: none
reviewed 2026-07-17T02:56:10.410Z sha fa55a32 :: needs maintainer review before merge. :: none
reviewed 2026-07-17T03:54:25.371Z sha 8d02607 :: needs maintainer review before merge. :: none
reviewed 2026-07-17T04:36:12.432Z sha 8d02607 :: needs maintainer review before merge. :: none
reviewed 2026-07-17T09:54:01.462Z sha ebdd18a :: needs maintainer review before merge. :: none

cxbAsDev · 2026-07-07T09:45:49Z

@clawsweeper re-review\n\nI added real behavior proof to the PR body, including the local test command and output that exercises the oversized-file rejection path.

cxbAsDev · 2026-07-07T10:26:44Z

@clawsweeper re-review

Added real behavior proof that exercises loadHookEntriesFromDir in src/hooks/workspace.ts with normal and oversized inputs.

Ran a standalone Node script importing the real source function and loading a temp workspace containing:

a valid normal-hook with small HOOK.md + handler.js;
a big-pkg directory with a 1 MiB + 1 byte package.json;
a big-md directory with a 1 MiB + 1 byte HOOK.md.

Output showed only normal-hook was discovered and both oversized files were skipped:

Discovered hook entries: 1
  - normal-hook (source=openclaw-workspace)
PASS: only the normal hook was loaded; oversized files were skipped.

The bounded read happens before JSON parsing, frontmatter parsing, or handler resolution, so no downstream processing is reached for oversized metadata.

clawsweeper · 2026-07-07T10:26:47Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

cxbAsDev · 2026-07-07T10:56:19Z

@clawsweeper re-review

Addressed the [P2] finding: replaced the unbounded fs.readFileSync(opened.fd, ...) with a chunked bounded fd reader (readFdUtf8Bounded) that enforces maxBytes while bytes are read. Added regression coverage for the overflow case. Updated the real behavior proof to show both workspace discovery and the bounded fd reader behavior.

clawsweeper · 2026-07-07T10:56:21Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

cxbAsDev · 2026-07-17T02:07:14Z

Rebased onto current `upstream/main` (force-pushed as `fa55a3207bd`) and re-ran the hook workspace focused tests:

node scripts/run-vitest.mjs src/hooks/workspace.test.ts --run

Result: 12/12 passed.

The previous `checks-node-compact-small-1` failure was a flaky TUI PTY e2e timeout unrelated to this change; fresh CI is now running on the rebased head. Please re-review and update the contributor status label when CI finishes.

cxbAsDev · 2026-07-17T03:58:06Z

Rebased onto the latest `upstream/main` where the `check-test-types` declaration issue was fixed (force-pushed as `8d026076b4f`).

Local verification:

`node scripts/run-vitest.mjs src/hooks/workspace.test.ts --run` → 12/12 passed

ClawSweeper review noted that openRootFileSync validates stat size before returning the fd, but the subsequent fs.readFileSync of the fd was unbounded. A file that grows after validation could still OOM workspace discovery. Replace the full fd read with a chunked bounded reader that throws once maxBytes is exceeded, and add regression coverage for the overflow case.

ClawSweeper review required the maintainer-selected oversized-metadata diagnostic contract: warn and skip. readRootFileUtf8 now catches the RangeError from the canonical readFileDescriptorBoundedSync helper and emits one warning per oversized package.json/HOOK.md identifying the path and the byte limit, while discovery continues for other hooks. The redundant open-time maxBytes stat check is removed so the shared bounded reader is the single owner of the byte cap; overflow always surfaces as RangeError, keeping the warning accurate for both static oversized files and post-open growth. Add focused tests: warning content for both oversized surfaces, continued discovery alongside an oversized hook, exact-limit acceptance, and the plain-hook fallback when a package manifest is oversized.

steipete · 2026-07-17T10:23:25Z

Merged via squash.

Prepared head SHA: 7e271d92946a9961cd9f0da4757355971b47eb91
Landed commit: 4b2b5bcf89559b4c4e6ba24ba93ebf9d2c03d950

openclaw-barnacle Bot added the size: XS label Jul 7, 2026

cxbAsDev force-pushed the fix/bound-hook-workspace-file-reads branch 2 times, most recently from 9e067e4 to 391be6d Compare July 7, 2026 08:39

clawsweeper Bot added merge-risk: 🚨 availability 🚨 May cause crashes, hangs, restart loops, stalls, or process outages. and removed merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. labels Jul 7, 2026

cxbAsDev force-pushed the fix/bound-hook-workspace-file-reads branch from 391be6d to 661237e Compare July 7, 2026 09:27

cxbAsDev force-pushed the fix/bound-hook-workspace-file-reads branch from 661237e to 9f9bd82 Compare July 7, 2026 09:59

clawsweeper Bot mentioned this pull request Jul 7, 2026

fix(hooks): bound HOOK.md reads during hook install validation #101469

Merged

openclaw-barnacle Bot added size: S and removed size: XS labels Jul 7, 2026

cxbAsDev force-pushed the fix/bound-hook-workspace-file-reads branch from 4734252 to c596612 Compare July 7, 2026 11:17

clawsweeper Bot removed the rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. label Jul 17, 2026

cxbAsDev and others added 7 commits July 17, 2026 11:02

fix(hooks): bound hook workspace manifest and HOOK.md reads

1df73d4

test(hooks): cover bounded hook reads through public workspace API

50ba8b1

refactor(hooks): use canonical bounded fd reader for hook metadata

24dda66

chore: retrigger CI after rebase

b7b64d0

refactor(hooks): tighten bounded metadata coverage

7e271d9

xydt-juyaohui mentioned this pull request Jul 17, 2026

fix(auto-reply): bound AGENTS.md read in post-compaction context #109945

Merged

github-actions Bot mentioned this pull request Jul 17, 2026

📡 Upstream Digest — 2026-07-17 11:15 UTC curtismercier/openclaw-mods#1322

Open

xydt-juyaohui mentioned this pull request Jul 17, 2026

fix(sandbox): bound sandbox seed bootstrap file reads #110017

Merged

This was referenced Jul 18, 2026

fix(config): bound external catalog file reads with size limit #108200

Merged

fix(config): bound state-directory .env file reads with size limit #109487

Merged

cxbAsDev mentioned this pull request Jul 18, 2026

Bound trajectory pointer file read with size cap #111045

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(hooks): bound hook workspace manifest and HOOK.md reads#101472

fix(hooks): bound hook workspace manifest and HOOK.md reads#101472
steipete merged 7 commits into
openclaw:mainfrom
cxbAsDev:fix/bound-hook-workspace-file-reads

cxbAsDev commented Jul 7, 2026 •

edited by steipete

Loading

Uh oh!

clawsweeper Bot commented Jul 7, 2026 •

edited

Loading

Uh oh!

cxbAsDev commented Jul 7, 2026

Uh oh!

cxbAsDev commented Jul 7, 2026

Uh oh!

clawsweeper Bot commented Jul 7, 2026 •

edited

Loading

Uh oh!

cxbAsDev commented Jul 7, 2026

Uh oh!

clawsweeper Bot commented Jul 7, 2026 •

edited

Loading

Uh oh!

cxbAsDev commented Jul 17, 2026

Uh oh!

cxbAsDev commented Jul 17, 2026

Uh oh!

steipete commented Jul 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

cxbAsDev commented Jul 7, 2026 • edited by steipete Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What Problem This Solves

Why This Change Was Made

User Impact

Maintainer Changes

Evidence

Uh oh!

clawsweeper Bot commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cxbAsDev commented Jul 7, 2026

Uh oh!

cxbAsDev commented Jul 7, 2026

Uh oh!

clawsweeper Bot commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cxbAsDev commented Jul 7, 2026

Uh oh!

clawsweeper Bot commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cxbAsDev commented Jul 17, 2026

Uh oh!

cxbAsDev commented Jul 17, 2026

Uh oh!

steipete commented Jul 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

cxbAsDev commented Jul 7, 2026 •

edited by steipete

Loading

clawsweeper Bot commented Jul 7, 2026 •

edited

Loading

clawsweeper Bot commented Jul 7, 2026 •

edited

Loading

clawsweeper Bot commented Jul 7, 2026 •

edited

Loading