Bound workspace doctor file reads with size cap by cxbAsDev · Pull Request #110592 · openclaw/openclaw

cxbAsDev · 2026-07-18T09:22:31Z

What Problem This Solves

shouldSuggestMemorySystem and migrateLegacyRootMemoryFile in the doctor workspace path call fs.promises.readFile on AGENTS.md and MEMORY.md files with no size limit. A corrupted or enormous workspace file can OOM the doctor path.

Why This Change Was Made

PR #101472 established the bounded-read pattern. The doctor workspace path reads user-authored workspace files (AGENTS.md, canonical MEMORY.md, legacy MEMORY.md) — the same unbounded-read vulnerability class. This change replaces all three fs.promises.readFile calls with readRegularFile using a 10 MB cap, which covers even very large memory files.

Additionally, migrateLegacyRootMemoryFile originally archived the legacy file before attempting the bounded read, meaning an oversized file would leave the legacy file already archived with no recovery path. The read is now performed before archival, so oversized files cleanly abort with no side effects.

User Impact

No change for normal workspace files under 10 MB. Files exceeding the cap result in graceful fallback (treated as unreadable) rather than process crash. The legacy migration path is safe: if either the canonical or legacy MEMORY.md exceeds 10 MB, the function returns without archiving the legacy file, preserving both files in place.

Evidence

Code changes

src/commands/doctor-workspace.ts:44: shouldSuggestMemorySystem uses readRegularFile with 10 MB cap
src/commands/doctor-workspace.ts:220-228: migrateLegacyRootMemoryFile reads both canonical and legacy MEMORY.md before calling moveLegacyRootMemoryFileToArchive — oversized files abort cleanly with legacy file intact
Added WORKSPACE_DOCTOR_MAX_BYTES = 10 * 1024 * 1024 constant
Added readRegularFile import from ../infra/regular-file.js
All readRegularFile results use .buffer.toString("utf-8") for string conversion

Behavioral proof — read-before-archive migration path

Scenario A: Normal files (under 10 MB cap)

Both canonical and legacy files read successfully — migration proceeds.

[Scenario A] Normal files (under 10 MB cap)
  ✓ Canonical MEMORY.md (0.1 KB): read OK
  ✓ Legacy memory.json (0.0 KB): read OK
  ✓ Legacy file would be archived (file exists: true)

Scenario B: 15 MB canonical MEMORY.md (exceeds 10 MB cap)

Read happens BEFORE archive. Oversized file is rejected cleanly, legacy file remains intact.

[Scenario B] 15 MB canonical MEMORY.md (exceeds 10 MB cap)
  Canonical MEMORY.md: 15 MB
  Legacy memory.json exists: true
  ✓ Canonical read rejected: File exceeds 10485760 bytes: ...
  ✓ Legacy file INTACT after oversized rejection: 43 bytes

Assertions:
  ✅ Oversized MEMORY.md is rejected
  ✅ Legacy file preserved (read-before-archive prevents data loss)
  Passed: 2/2

The critical difference from the before state:

Before: legacy file archived first, then bounded read failed → legacy file lost
After: both files read first, oversized file rejected cleanly → legacy file preserved

Unit tests

src/commands/doctor-workspace.test.ts: 3 tests passed (under |commands| Vitest shard)
Full CI: All gates green

Proof commands

node scripts/run-vitest.mjs "src/commands/doctor-workspace.test.ts"
node scripts/proof-doctor-migration.mjs

PR fix(hooks): bound hook workspace manifest and HOOK.md reads #101472 (established readRegularFile bounded-read pattern in src/cron/store.ts)
PR fix(cli): clarify oversized config mutation file errors #110593 (bounded config-set-input batch-file reads)
PR Bound plugin bundle command file reads with size cap #110594 (bounded bundle-command file reads with oversized diagnostic)

clawsweeper · 2026-07-18T09:29:52Z

Codex review: needs maintainer review before merge. Reviewed July 18, 2026, 7:54 AM ET / 11:54 UTC.

Summary
The branch caps three workspace-doctor reads at 10 MiB with readRegularFile and reads canonical and legacy memory files before archiving the legacy file.

PR surface: Source +27. Total +27 across 1 file.

Reproducibility: no. high-confidence current-main reproduction was executed in this read-only review, but the pre-fix unbounded-read path and the proposed capped failure path are directly identifiable from source and the PR includes after-fix terminal scenarios.

Review metrics: 1 noteworthy metric.

Bounded doctor inputs: 3 reads capped at 10 MiB. All user-authored workspace files on this doctor path now share an explicit memory-allocation boundary, including both inputs used by legacy-memory migration.

Stored data model
Persistent data-model change detected: migration/backfill/repair: src/commands/doctor-workspace.ts, serialized state: src/commands/doctor-workspace.ts, unknown-data-model-change: src/commands/doctor-workspace.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] The 10 MiB boundary intentionally changes doctor behavior for existing oversized workspace files: memory suggestion or legacy-memory migration will stop rather than loading the file. The PR describes this as the intended fail-safe behavior, but it remains an upgrade-compatibility choice to accept before merge.
[P1] The PR is mergeable but behind main; refresh the branch and review the resulting merge head rather than treating base drift as a source-level defect.

Maintainer options:

Accept the documented 10 MiB boundary (recommended)
Refresh against current main and merge if maintainers accept that pre-existing oversized workspace files no longer participate in doctor suggestion or legacy-memory migration.

Next step before merge

[P2] No discrete automated repair remains; refresh the branch against current main and complete normal maintainer merge review.

Security
Cleared: The one-file TypeScript change adds no dependency, workflow, secret, permission, package-resolution, or code-execution surface; it narrows allocation exposure for user-authored files.

Review details

Best possible solution:

Rebase onto current main, retain the shared 10 MiB bounded-read contract, and merge only if maintainers accept that oversized workspace files are safely skipped with no legacy archive side effect.

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

No high-confidence current-main reproduction was executed in this read-only review, but the pre-fix unbounded-read path and the proposed capped failure path are directly identifiable from source and the PR includes after-fix terminal scenarios.

Is this the best way to solve the issue?

Yes. Reusing the established shared bounded-reader primitive and preflighting both memory inputs before archival is the narrowest maintainable fix for the stated allocation and recoverability problems.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 39ddf710f0f3.

Label changes

Label changes:

add rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
remove rating: 🐚 platinum hermit: Current PR rating is rating: 🦞 diamond lobster, so this older rating label is no longer current.

Label justifications:

P2: This is a focused reliability and compatibility hardening change for the workspace-doctor path, without evidence of an active release-blocking outage.
merge-risk: 🚨 compatibility: Existing workspace files larger than 10 MiB now follow the unreadable-file path, which can suppress suggestion or migration behavior 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 (terminal): The PR body and follow-up comments show after-fix terminal behavior for normal and oversized files, including the critical read-before-archive migration case; redact workspace paths and content if any further proof is posted.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body and follow-up comments show after-fix terminal behavior for normal and oversized files, including the critical read-before-archive migration case; redact workspace paths and content if any further proof is posted.

Evidence reviewed

PR surface:

Source +27. Total +27 across 1 file.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	32	5	+27
Tests	0	0	0	0
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	1	32	5	+27

What I checked:

Changed doctor path: The branch replaces the AGENTS.md, canonical MEMORY.md, and legacy memory-file reads with the shared bounded reader, and converts returned buffers to UTF-8 strings before preserving the existing matching and migration logic. (src/commands/doctor-workspace.ts:20, 83210a49cd7f)
Recoverable migration ordering: The branch preloads both canonical and legacy content before calling the archive helper, so a size-cap failure returns through the existing error result without first moving the legacy file. (src/commands/doctor-workspace.ts:208, 83210a49cd7f)
Prior review continuity: The earlier blockers to decode the bounded-reader buffer and to avoid archival before a cap failure were addressed in commits 52fb9a1d261dc714a05f009ea97e83295480b203 and 83210a49cd7fab88799f5d75744b4bd0058b8c5e; the latest completed review had no remaining findings. (src/commands/doctor-workspace.ts:44, 83210a49cd7f)
Established shared pattern: Merged PR fix(hooks): bound hook workspace manifest and HOOK.md reads #101472 previously established bounded workspace-file reads with the shared regular-file reader; this PR applies that pattern to a separate doctor workflow. (src/infra/regular-file.ts:1, 4b2b5bcf8955)
Real behavior evidence: The PR body and follow-up comments provide after-fix terminal evidence for a normal workspace file and an oversized file, plus a migration scenario where a 15 MiB canonical memory file is rejected and the legacy file remains in place. (src/commands/doctor-workspace.ts:208, 83210a49cd7f)

Likely related people:

cxbAsDev: Previously authored the merged bounded workspace-file read work referenced by this PR and carried the follow-up repairs that resolved this branch’s earlier review findings. (role: bounded-read pattern contributor; confidence: medium; commits: 4b2b5bcf8955, 52fb9a1d261d, 83210a49cd7f; files: src/commands/doctor-workspace.ts, src/infra/regular-file.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 (5 earlier review cycles)

reviewed 2026-07-18T09:26:18.246Z sha b31713f :: needs real behavior proof before merge. :: [P1] Decode the bounded-read result before matching memory references | [P1] Decode bounded memory reads before calling trimEnd
reviewed 2026-07-18T09:39:31.669Z sha 52fb9a1 :: needs real behavior proof before merge. :: [P1] Keep the legacy-memory migration recoverable on cap failures
reviewed 2026-07-18T11:33:37.395Z sha 83210a4 :: needs real behavior proof before merge. :: none
reviewed 2026-07-18T11:40:03.127Z sha 83210a4 :: needs real behavior proof before merge. :: none
reviewed 2026-07-18T11:45:38.680Z sha 83210a4 :: needs maintainer review before merge. :: none

cxbAsDev · 2026-07-18T09:55:57Z

Behavior Proof — Bounded Workspace Doctor File Read

Test Coverage

运行 doctor-workspace 测试套件：3/3 通过

 ✓ src/commands/doctor-workspace.test.ts (3 tests) 349ms
     ✓ ignores lowercase-only root memory for automatic repair
     ✓ migrates legacy root memory file
     ✓ suggests memory system when missing
 Test Files  1 passed (1)
      Tests  3 passed (3)

Live Bounded-Read Validation

直接验证 readRegularFile 的 10 MB 上限：

PR 110592 ✅ normal AGENTS.md: 25 bytes              ← 正常 AGENTS.md 读取正确
PR 110592 ✅ oversized workspace file rejected (catchable)  ← 超大文件被 catch 块处理

两种场景全部通过：

正常 AGENTS.md — 读取正确，RegExp 匹配正常工作，与旧行为一致
超大工作区文件（20 MB > 10 MB cap）— 可抛出的异常，被 shouldSuggestMemorySystem 的 catch {} 处理为"视为内存引导缺失"

CI Gate

openclaw/ci-gate ✅ PASS

…ryFile

cxbAsDev · 2026-07-18T11:34:57Z

@clawsweeper re-review

clawsweeper · 2026-07-18T11:35:00Z

🦞🧹
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.

Re-review progress:

State: Complete
Detail: The durable review comment was published and its verdict was queued for target-serialized routing.
Run: https://github.com/openclaw/clawsweeper/actions/runs/29642958873
Updated: 2026-07-18T11:43:31.881Z

cxbAsDev · 2026-07-18T11:42:18Z

PR body updated with behavioral proof and compatibility contract. @clawsweeper re-review

cxbAsDev · 2026-07-18T11:50:33Z

PR body updated with full migration-path proof (read-before-archive scenarios A and B, 15 MB oversized rejected with legacy preserved). @clawsweeper re-review

steipete · 2026-07-18T20:25:47Z

Thanks for pushing this bounded-read direction. This is now fixed on main by #101448 / 881b0a6.

The landed version covers the same AGENTS.md and root-memory reads, with separate 1 MiB / 8 MiB limits, explicit read-limit and archive diagnostics, atomic-only archival, a post-move reread for concurrent growth, and regression coverage for oversized files, symlinks, archive races, and concurrent replacement. That is a stronger current-main implementation than rebasing this conflicting 10 MiB catch-and-skip variant, so I’m closing this as superseded.

Bound workspace doctor file reads with size cap

b31713f

openclaw-barnacle Bot added commands Command implementations size: XS labels Jul 18, 2026

fix: use buffer.toString for readRegularFile results

52fb9a1

clawsweeper Bot added the merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. label Jul 18, 2026

fix: read both memory files before archiving in migrateLegacyRootMemo…

83210a4

…ryFile

This was referenced Jul 18, 2026

fix(cli): clarify oversized config mutation file errors #110593

Merged

Bound plugin bundle command file reads with size cap #110594

Merged

clawsweeper Bot added rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. labels Jul 18, 2026

cxbAsDev mentioned this pull request Jul 18, 2026

Bound diagnostic config file read with size cap #110591

Merged

steipete self-assigned this Jul 18, 2026

steipete closed this Jul 18, 2026

This was referenced Jul 19, 2026

feat: bound reads from open file descriptors openclaw/fs-safe#41

Merged

refactor: centralize bounded file reads in fs-safe #111104

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Bound workspace doctor file reads with size cap#110592

Bound workspace doctor file reads with size cap#110592
cxbAsDev wants to merge 3 commits into
openclaw:mainfrom
cxbAsDev:fix/bound-doctor-workspace-reads

cxbAsDev commented Jul 18, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Jul 18, 2026 •

edited

Loading

Uh oh!

cxbAsDev commented Jul 18, 2026

Uh oh!

cxbAsDev commented Jul 18, 2026

Uh oh!

clawsweeper Bot commented Jul 18, 2026 •

edited

Loading

Uh oh!

cxbAsDev commented Jul 18, 2026

Uh oh!

cxbAsDev commented Jul 18, 2026

Uh oh!

steipete commented Jul 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

cxbAsDev commented Jul 18, 2026 • edited 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

Evidence

Code changes

Behavioral proof — read-before-archive migration path

Unit tests

Proof commands

Related

Uh oh!

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

Uh oh!

Uh oh!

cxbAsDev commented Jul 18, 2026

Behavior Proof — Bounded Workspace Doctor File Read

Test Coverage

Live Bounded-Read Validation

CI Gate

Uh oh!

cxbAsDev commented Jul 18, 2026

Uh oh!

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

Uh oh!

Uh oh!

cxbAsDev commented Jul 18, 2026

Uh oh!

cxbAsDev commented Jul 18, 2026

Uh oh!

steipete commented Jul 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

cxbAsDev commented Jul 18, 2026 •

edited

Loading

clawsweeper Bot commented Jul 18, 2026 •

edited

Loading

clawsweeper Bot commented Jul 18, 2026 •

edited

Loading