Bound diagnostic config file read with size cap by cxbAsDev · Pull Request #110591 · openclaw/openclaw

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

What Problem This Solves

readConfigExport in the diagnostic support export path calls fs.readFileSync(options.configPath, "utf8") on the user's config file with no size limit. A corrupted or enormous config file can OOM the diagnostic export path, preventing support bundle generation.

Why This Change Was Made

PR #101472 established the bounded-read pattern. The diagnostic export path reads the user's config file to include sanitized config in support bundles. Config files are user-controlled content and should not be trusted to have reasonable size. Using readRegularFileSync with an 8 MB cap (config files are typically <100 KB) prevents OOM from corrupted config files while maintaining diagnostic value.

User Impact

No user-visible change under normal conditions. Config files under 8 MB export identically to before. An oversized config file results in a graceful config-shape-read-failure entry in the diagnostic report instead of crashing the export.

Evidence

Code changes

src/logging/diagnostic-support-export.ts:348-350: Replaced fs.readFileSync(options.configPath, "utf8") with readRegularFileSync({ filePath: options.configPath, maxBytes: DIAGNOSTIC_CONFIG_MAX_BYTES }).buffer.toString("utf8")
src/logging/diagnostic-support-export.ts:41: Added DIAGNOSTIC_CONFIG_MAX_BYTES = 8 * 1024 * 1024 constant
Added readRegularFileSync import from ../infra/regular-file.js
Existing error handling already catches and reports read failures gracefully

Behavioral proof — real diagnostic export (via `writeDiagnosticSupportExport`)

Both scenarios call the actual writeDiagnosticSupportExport function with real temp directories, real config files, and real zip output validated by decompression and content inspection.

Test 1: Normal config file (~100 bytes) — accepted, parseOk: true

Config file: {"gateway":{"mode":"local","port":18789},"logging":{"redactSensitive":"off"}}
Config file size: 78 bytes
Export generated: /tmp/.../support-normal.zip
config/shape.json parseOk: true
Config values present: gateway.mode=local, gateway.port=18789
Result: ✅ PASS

The config is read through readRegularFileSync with the 8 MB cap, parsed as JSON5, and included in the support bundle with parseOk: true and all config values correctly present.

Test 2: Oversized config file (8 MB + 1 byte) — rejected, parseOk: false, bundle still complete

Config file size: 8388609 bytes (exceeds 8 MB cap)
Export generated: /tmp/.../support-oversized.zip
config/shape.json parseOk: false
Error in shape: "File exceeds 8388608 bytes: ..."
Bundle still has diagnostics.json: true
Bundle still has manifest.json: true
Bundle has support footer: true
Result: ✅ PASS

The oversized file is rejected at the stat-check stage by readRegularFileSync with File exceeds 8388608 bytes. The bundle still contains diagnostics.json, manifest.json, and the standard support footer.

Unit tests

src/logging/diagnostic-support-export.test.ts:
- ✓ reads a normal-size config file and reports parseOk: true (752ms)
- ✓ handles an oversized config file exceeding the 8 MB read cap gracefully (30ms)
Total: 2/2 new tests passed, 13/13 total in file
Full CI: All gates green

Proof command

node scripts/run-vitest.mjs "src/logging/diagnostic-support-export.test.ts" -t "reads a normal-size config|handles an oversized config"

PR fix(hooks): bound hook workspace manifest and HOOK.md reads #101472 (established readRegularFileSync bounded-read pattern in src/cron/store.ts)
PR Bound workspace doctor file reads with size cap #110592 (bounded doctor-workspace reads with read-before-archive ordering)
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:30:01Z

Codex review: needs maintainer review before merge. Reviewed July 18, 2026, 5:43 PM ET / 21:43 UTC.

Summary
The branch replaces the diagnostic support export’s unbounded synchronous config-file read with an 8 MiB bounded regular-file read and adds oversized-config export coverage.

PR surface: Source +8, Tests +46. Total +54 across 2 files.

Reproducibility: yes. from source and supplied proof: the former path synchronously read the configured file without a byte limit, while the branch exercises the real export with an 8 MiB-plus-one-byte file and confirms a completed archive.

Review metrics: 1 noteworthy metric.

Diagnostic read limit: 1 unbounded config read changed to an 8 MiB bounded read. The sole runtime behavior change intentionally affects only support-export handling of oversized existing config files.

Stored data model
Persistent data-model change detected: serialized state: src/logging/diagnostic-support-export.test.ts. Confirm migration or upgrade compatibility proof before merge.

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

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

Rank-up moves:

none.

Risk before merge

[P1] The intentional 8 MiB ceiling means an existing oversized config will no longer be included in diagnostic shape output; it instead produces the already-tested graceful failure entry, so the maintainer should retain this diagnostic-only compatibility boundary.

Maintainer options:

Merge with the diagnostic-only cap (recommended)
Accept the deliberate change that oversized configs yield a shape-read failure while the rest of the support archive remains available.
Adjust the cap before merge
If support exports must retain config shape data above 8 MiB, select and document a different bounded limit before landing.

Next step before merge

[P2] No actionable repair remains; this needs normal assigned-maintainer merge review of the intentional diagnostic compatibility boundary.

Security
Cleared: The diff adds no dependency, workflow, permission, secret, package-resolution, or code-execution surface; it reduces memory-exhaustion exposure in a user-controlled diagnostic read.

Review details

Best possible solution:

Keep the 8 MiB limit scoped to diagnostic export, retain the graceful parseOk: false archive behavior for oversized files, and merge after the assigned reviewer completes the normal final check.

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

Yes from source and supplied proof: the former path synchronously read the configured file without a byte limit, while the branch exercises the real export with an 8 MiB-plus-one-byte file and confirms a completed archive.

Is this the best way to solve the issue?

Yes: reusing the existing bounded regular-file primitive at the diagnostic-only read boundary is narrower and more maintainable than adding a separate reader or a general config-file policy.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

P2: This is a bounded diagnostic robustness fix with limited user impact and a focused implementation.
merge-risk: 🚨 compatibility: Existing configs larger than 8 MiB now produce a diagnostic shape-read failure instead of being read into the export.
rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body provides copied live output from the actual diagnostic-export function using real temporary config files and ZIP inspection for both normal and oversized inputs.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body provides copied live output from the actual diagnostic-export function using real temporary config files and ZIP inspection for both normal and oversized inputs.

Evidence reviewed

PR surface:

Source +8, Tests +46. Total +54 across 2 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	9	1	+8
Tests	1	46	0	+46
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	2	55	1	+54

What I checked:

Proposed runtime behavior: The PR changes readConfigExport to call readRegularFileSync with SUPPORT_EXPORT_CONFIG_MAX_BYTES before parsing the returned buffer, so an oversized config reaches the existing export failure path rather than being synchronously read without a size bound. (src/logging/diagnostic-support-export.ts:347, 34a4f79ae842)
Regression coverage: The added test writes an 8 MiB-plus-one-byte config, runs the real diagnostic export, verifies parseOk: false, preserves the expected archive entries, and checks the support footer remains present. (src/logging/diagnostic-support-export.test.ts:916, 34a4f79ae842)
Maintainer-reviewed boundary: Commit 437f4434114f32263ffc9605506efdcf926d7442 clarifies that the 8 MiB ceiling is specific to support export rather than a general config-file limit; that commit is part of the PR branch and was authored by the assigned reviewer. (src/logging/diagnostic-support-export.ts:41, 437f4434114f)
Established helper pattern: The related merged bounded-read work uses the same regular-file reader pattern, supporting reuse of the existing repository primitive instead of introducing a parallel limiter. (src/infra/regular-file.ts:1, 4b2b5bcf8955)
Current review state: The PR is open, mergeable, assigned to steipete, and its latest branch merges the then-current main base; it is therefore not an implemented-on-main or stale-cleanup candidate. (34a4f79ae842)

Likely related people:

steipete: Authored the branch’s clarification commit for the support-export-specific limit and is assigned to the PR. (role: recent area contributor; confidence: high; commits: 437f4434114f, 34a4f79ae842; files: src/logging/diagnostic-support-export.ts)
cxbAsDev: Authored the related merged bounded hook-metadata read work and the initial implementation and tests here, connecting them to the same regular-file helper pattern. (role: introduced bounded-read pattern; confidence: medium; commits: 4b2b5bcf8955, 0e41344c3012, 998f4c8507ef; files: src/infra/regular-file.ts, src/logging/diagnostic-support-export.ts, src/logging/diagnostic-support-export.test.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 (7 earlier review cycles)

reviewed 2026-07-18T09:26:32.212Z sha 0e41344 :: needs real behavior proof before merge. :: none
reviewed 2026-07-18T09:41:13.746Z sha 97c8aca :: needs real behavior proof before merge. :: none
reviewed 2026-07-18T11:38:05.169Z sha 998f4c8 :: needs real behavior proof before merge. :: none
reviewed 2026-07-18T12:20:05.231Z sha 998f4c8 :: needs real behavior proof before merge. :: none
reviewed 2026-07-18T12:49:30.067Z sha 998f4c8 :: needs maintainer review before merge. :: none
reviewed 2026-07-18T13:32:05.685Z sha 998f4c8 :: needs maintainer review before merge. :: none
reviewed 2026-07-18T21:04:02.755Z sha 437f443 :: needs maintainer review before merge. :: none

… export

cxbAsDev · 2026-07-18T09:50:18Z

Behavior Proof

Code Changes

Replaced fs.readFileSync(options.configPath, "utf8") with bounded readRegularFileSync({ maxBytes: DIAGNOSTIC_CONFIG_MAX_BYTES }) using proper { buffer } destructuring and buffer.toString("utf8")
Fixed a type error where readRegularFileSync was passed directly to parseConfigJson5 (which expects a string)

Local Verification

pnpm tsgo:core — ✅ passes
pnpm test src/logging/diagnostic.test.ts — ✅ 83 tests pass

CI (latest commit)

All relevant CI checks pass:

check-prod-types ✅ SUCCESS
check-lint ✅ SUCCESS
check-test-types ✅ SUCCESS
check-additional-extension-package-boundary ✅ SUCCESS
openclaw/ci-gate ✅ SUCCESS

Risk Assessment

This change replaces an unbounded fs.readFileSync with a bounded readRegularFileSync (8 MB cap) in the diagnostic config export path. Config files are typically <100 KB. An oversized config file results in a graceful error entry in the diagnostic report instead of crashing the export. No user-visible change under normal conditions.

PR openclaw#110591 — Add focused regression coverage for the bounded diagnostic config read: - Test: normal config file (<8MB) produces parseOk: true in config/shape.json - Test: oversized config file (>8MB) produces parseOk: false with graceful error handling - Both tests verify the support bundle zip is still written with all expected contents

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

@clawsweeper re-review

clawsweeper · 2026-07-18T11:34:54Z

🦞🧹
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/29646330777
Updated: 2026-07-18T13:35:22.051Z

cxbAsDev · 2026-07-18T12:45:30Z

@clawsweeper re-review

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

steipete · 2026-07-18T21:48:48Z

Merged via squash.

Prepared head SHA: 34a4f79ae842db6381f03c5ac1df8dd399eafa04
Landed commit: 87de81a5fd4a2d4e3ef8bff9af22a3ed2ab8b4e8

* Bound diagnostic config read with size cap * fix: use buffer.toString for readRegularFileSync result in diagnostic export * test: verify normal and oversized diagnostic config export behavior PR openclaw#110591 — Add focused regression coverage for the bounded diagnostic config read: - Test: normal config file (<8MB) produces parseOk: true in config/shape.json - Test: oversized config file (>8MB) produces parseOk: false with graceful error handling - Both tests verify the support bundle zip is still written with all expected contents * refactor(logging): clarify support export config limit Co-authored-by: 陈宪彪0668000387 <[email protected]> --------- Co-authored-by: Peter Steinberger <[email protected]>

Bound diagnostic config read with size cap

0e41344

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

fix: use buffer.toString for readRegularFileSync result in diagnostic…

97c8aca

… export

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

steipete self-assigned this Jul 18, 2026

steipete and others added 2 commits July 18, 2026 22:00

refactor(logging): clarify support export config limit

437f443

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

Merge branch 'main' into fix/bound-diagnostic-config-read

34a4f79

steipete merged commit 87de81a into openclaw:main Jul 18, 2026
116 checks passed

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

📡 Upstream Digest — 2026-07-18 22:39 UTC curtismercier/openclaw-mods#1339

Open

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 diagnostic config file read with size cap#110591

Bound diagnostic config file read with size cap#110591
steipete merged 5 commits into
openclaw:mainfrom
cxbAsDev:fix/bound-diagnostic-config-read

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!

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 — real diagnostic export (via writeDiagnosticSupportExport)

Unit tests

Proof command

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

Code Changes

Local Verification

CI (latest commit)

Risk Assessment

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!

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

Behavioral proof — real diagnostic export (via `writeDiagnosticSupportExport`)

clawsweeper Bot commented Jul 18, 2026 •

edited

Loading

clawsweeper Bot commented Jul 18, 2026 •

edited

Loading