Repository navigation
fix(download): reject oversized runtime metadata before replacement - #312
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
clawsweeper
Bot
added
P2
|
Codex review: needs maintainer review before merge. Reviewed October 6, 2026, 11:55 PM ET / October 7, 2026, 03:55 UTC (Revision 3). ClawSweeper reviewWhat this changesThe branch limits decoded runtime metadata to 64 MiB, adds boundary and installed-runtime preservation coverage, documents the cutoff, and records recent fixes in the changelog. Merge readiness✅ Ready for maintainer review The fix remains necessary: neither current main nor v0.2.48 bounds decoded JSON. No actionable correctness or security defect was found, and the current head resolves the previous breaking-change declaration blocker. Likely related people: SebTardif, vincentkoc, and shakkernerd are relevant routing candidates based on prior download and runtime work. Priority: P2 Review scores
Verification
How this fits togetherOCM reads release metadata from the official npm registry or custom manifests to select OpenClaw runtimes. Metadata validation precedes artifact downloads and publication of replacement runtime files. flowchart TD
A[Registry or custom manifest] --> B[HTTP reader and gzip decoding]
B --> C{Decoded JSON within 64 MiB?}
C -->|Yes| D[Parse and select release]
D --> E[Download and publish runtime]
C -->|No| F[Return size error]
F --> G[Keep installed runtime unchanged]
Before mergeNone. Agent review detailsSecurityNone. Review metrics
Technical reviewBest possible solution: Keep the decoded-byte bound before parsing and replacement, with the documented cutoff and recognized breaking-change declaration preserved at landing. Do we have a high-confidence way to reproduce the issue? Yes, source establishes that current main parses decoded JSON without a byte bound, and the PR supplies focused before/after regressions for oversized gzip metadata and runtime updates. This read-only review did not execute them. Is this the best way to solve the issue? Yes, reusing the existing capped-copy helper at the shared decoded-reader boundary is a narrow solution that covers official and custom metadata before replacement. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against 78f414cda7ba. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
History |
Bound decoded JSON before parsing and preserve installed runtime state when an oversized update manifest is rejected. Add exact-limit and installed-state regressions, document the custom-source compatibility boundary, and record previously landed round-eight fixes in Unreleased. Complete the approach proposed in #117. Co-authored-by: Sebastien Tardif <sebtardif@ncf.ca>
Record the compatibility boundary in commit metadata so the release reconciler requires an explicit version decision. The tested application tree is unchanged. BREAKING CHANGE: Runtime release JSON over 64 MiB after decompression is rejected. Reduce larger custom manifests before retrying installation or updates. Existing installed runtime files and metadata are preserved when the source is rejected.
ba3f42d to
97d10f7
Compare
clawsweeper
Bot
added
rating: 🦞 diamond lobster
|
Landed as 2e21cf5. The landed Git tree matches tested head The final commit contains the machine-recognized Both rejection regressions failed before the fix: gzip-expanded JSON was accepted, and an oversized valid update manifest replaced the installed runtime. Afterward, exact-limit acceptance and oversized-source rejection passed; the update regression verified unchanged installed bytes and metadata and no replacement artifact request. The ordered isolated Linux gate passed with Rust 1.99.0 and Node.js 24.21.0: diff/format checks, 33 release-policy Python tests, all-target compilation, 1,317 Rust tests, isolated Cargo installation/version smoke ( All 14 hosted checks passed at the final head, including Linux/macOS, Windows, Rust 1.88, npm, and CodeQL: https://github.com/openclaw/ocm/actions/runs/37568000261. |
|
Post-merge main verification passed on the first attempt at 2e21cf5: https://github.com/openclaw/ocm/actions/runs/37569240684. All nine CI matrix jobs and all four CodeQL analyses are green, including the Intel macOS signed/notarized npm fixture. Automatic release reconciliation was skipped. |
1 participant
What Problem This Solves
Runtime metadata could consume unbounded memory while loading a custom manifest or gzip-compressed registry response. This completes the decoded-byte bound proposed in #117 and proves that rejecting an oversized update source preserves an existing installation.
User Impact
Runtime release JSON is now limited to 64 MiB after decompression. Sources at the limit still work. Operators with larger custom manifests must reduce their metadata before retrying; an oversized source is rejected before downloading replacement artifacts or changing installed runtime files and metadata.
The final squash commit declares this cutoff as a
BREAKING CHANGEunder the repository release policy, so a future automatic patch release must stop for an explicit version decision. No package version is changed here.Why This Change Was Made
The shared JSON reader applies the existing capped-copy mechanism before parsing, so plain and gzip-expanded responses follow the same limit. The usage guide describes the compatibility boundary. This retains @SebTardif's original approach and adds exact-boundary and installed-runtime regression coverage.
The changelog also records the already-landed fixes in #307–#310, with contributor credit, so their user-visible behavior is available for the next release.
Evidence
Both new rejection regressions failed against unchanged production code: oversized gzip JSON was accepted, and the CLI accepted an oversized valid update manifest and replaced the installed runtime. After the fix, all download tests passed, including acceptance at exactly 64 MiB. The installed-runtime regression passed with byte-identical metadata and original executable content, no replacement file, and no request for the replacement artifact.
The ordered isolated AWS Linux baseline passed with Rust 1.99.0 and Node.js 24.21.0: formatting, 33 automatic-release policy tests, all-target compilation, all 1,317 Rust tests, isolated
cargo install --locked --path ., and the installedocm --version(0.2.48). Rust 1.88 all-target compilation also passed.The installed binary fetched the live official catalog successfully (266 releases; stable
2026.9.8at the time of the check). Independent Codex review completed with no actionable P0–P2 findings. Hosted platform checks remain required before merge.The remote proof covers the current application, Rust tests, and dependency inputs. The subsequent base change in #313 only adds npm notarization failure diagnostics; refreshed hosted CI is required for this head. The branch history and prepared final squash body both contain the recognized compatibility declaration.