fix(security): bound plugin manifest reads in audit deep scan by cxbAsDev · Pull Request #101773 · openclaw/openclaw

cxbAsDev · 2026-07-07T15:53:44Z

What Problem This Solves

readPluginManifestExtensions in src/security/audit-extra.async.ts read each installed plugin's package.json with an unbounded fs.readFile. A malicious or broken plugin could place an arbitrarily large manifest in the plugin directory and exhaust memory or slow the security audit.

Why This Change Was Made

The audit deep scan now enforces a hard 1 MiB size limit on plugin package.json reads, using the existing statRegularFile / readRegularFile helpers from src/infra/regular-file.js, which also reject symlinks and non-file targets. Oversized manifests throw a clear error that surfaces as a plugins.code_safety.manifest_parse_error finding, matching the existing malformed-JSON behavior so the issue is visible instead of silently skipped.

User Impact

Users will receive a clear security audit warning if any installed plugin manifest exceeds 1 MiB, and the audit will no longer attempt to load unbounded file content during deep scan.

Evidence

Added a regression test in src/security/audit-extra.async.test.ts that creates a package.json of 1 MiB + 1 bytes and asserts a manifest_parse_error finding is emitted for the oversized plugin.
Local focused run: node scripts/run-vitest.mjs src/security/audit-extra.async.test.ts --run passes (7/7 tests).
oxlint clean on changed files.

Real behavior proof

$ node scripts/run-vitest.mjs src/security/audit-extra.async.test.ts --run
[test] starting test/vitest/vitest.unit-fast.config.ts
 RUN  v4.1.9
 ✓ |unit-fast| src/security/audit-extra.async.test.ts (7 tests) 126ms
 Test Files  1 passed (1)
      Tests  7 passed (7)
[test] passed 1 Vitest shard in 71.62s

clawsweeper · 2026-07-07T16:02:30Z

Codex review: needs maintainer review before merge. Reviewed July 15, 2026, 11:40 PM ET / July 16, 2026, 03:40 UTC.

Summary
The PR caps deep-audit reads of installed plugin package.json files at 1 MiB through safe regular-file helpers and adds oversized-manifest regression coverage.

PR surface: Source +16, Tests +19. Total +35 across 2 files.

Reproducibility: yes. Current main has an unbounded fs.readFile path, and the contributor demonstrated the patched CLI against a real 1 MiB + 1 byte installed manifest with the expected bounded warning.

Review metrics: 1 noteworthy metric.

New audit input limit: 1 MiB hard cap added. The exact threshold becomes a user-visible security-audit policy and should be consciously approved before merge.

Stored data model
Persistent data-model change detected: serialized state: src/security/audit-extra.async.test.ts, serialized state: src/security/audit-extra.async.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:

Obtain explicit security-owner approval for the 1 MiB and scan-fallback contract, then refresh the behind branch and checks.

Risk before merge

[P1] The 1 MiB limit is a new hard audit policy without a demonstrated shared manifest-size contract; a valid larger manifest would now be reported as malformed.
[P2] When a manifest is oversized or otherwise rejected, the audit scans the plugin directory but may miss declared entrypoints under normally skipped hidden or node_modules paths; maintainers must explicitly accept that fallback.

Maintainer options:

Confirm and land the policy (recommended)
Have the security-boundary owner approve the 1 MiB contract and warning fallback, then refresh the branch and required checks.
Centralize the limit first
Move the size policy to a shared plugin-manifest contract and cover audit plus relevant sibling consumers before merge.
Pause pending coverage design
Hold the PR if warning-only fallback is not acceptable for hidden or skipped declared entrypoints.

Next step before merge

[P2] A security-boundary owner must approve the permanent threshold and fallback semantics; there is no concrete code defect for an automated repair worker.

Maintainer decision needed

Question: Should deep security audits adopt 1 MiB as the canonical installed-plugin manifest limit and accept warning-only fallback when declared extension entrypoints cannot be read?
Rationale: The implementation is mechanically sound, but neither the threshold nor reduced declared-entry coverage for rejected manifests can be established as permanent security policy from tests or CI alone.
Likely owner: steipete — The strongest history signal is repeated ownership of the fs-safe and TOCTOU security boundary used by this patch.
Options:
- Approve the bounded audit policy (recommended): Keep the 1 MiB cap and warning-plus-directory-scan fallback as the intentional deep-audit security contract.
- Align a shared manifest contract: Define or reuse one plugin-manifest size limit across audit and other manifest consumers before landing this patch.
- Pause for stronger fallback coverage: Delay landing until rejected manifests receive a scan strategy that cannot miss normally skipped declared entrypoints.

Security
Cleared: The focused diff reduces unbounded-memory exposure, uses the repository's hardened file-read boundary, and introduces no concrete new security or supply-chain defect.

Review details

Best possible solution:

Land a bounded manifest read after the security owner confirms a canonical size policy and that warning plus directory fallback is the intended scan-coverage behavior, sharing the limit with other plugin-manifest consumers if they enforce the same contract.

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

Yes. Current main has an unbounded fs.readFile path, and the contributor demonstrated the patched CLI against a real 1 MiB + 1 byte installed manifest with the expected bounded warning.

Is this the best way to solve the issue?

Yes mechanically: the shared regular-file reader is the narrowest safe way to bound the read and preserve audit completion. Final acceptance still depends on owner confirmation of the threshold and scan-coverage contract.

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): After-fix terminal proof shows a real deep CLI audit reporting the oversized installed manifest with the exact bounded-read error while completing successfully.
remove status: ⏳ waiting on author: Current PR status label is status: 👀 ready for maintainer look.

Label justifications:

P2: This is focused defense-in-depth for a bounded deep-audit denial-of-service vector without evidence of active exploitation or core-runtime unavailability.
merge-risk: 🚨 security-boundary: Merging establishes a new hard size limit and changes how untrusted plugin manifests are classified and scanned.
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): After-fix terminal proof shows a real deep CLI audit reporting the oversized installed manifest with the exact bounded-read error while completing successfully.
proof: sufficient: Contributor real behavior proof is sufficient. After-fix terminal proof shows a real deep CLI audit reporting the oversized installed manifest with the exact bounded-read error while completing successfully.

Evidence reviewed

PR surface:

Source +16, Tests +19. Total +35 across 2 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	18	2	+16
Tests	1	19	0	+19
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	2	37	2	+35

What I checked:

Current-main behavior: At current main, readPluginManifestExtensions still reads each installed plugin manifest with unbounded fs.readFile; the central hardening remains necessary. (src/security/audit-extra.async.ts:99, 957cc81175a3)
Changed security boundary: The PR checks a 1 MiB maximum and uses readRegularFile with the same byte cap, replacing the unbounded read while rejecting unsafe non-regular targets. (src/security/audit-extra.async.ts:99, e127679198fa)
Failure containment: The caller converts manifest read or parse failures into plugins.code_safety.manifest_parse_error and continues scanning the plugin directory, so an oversized manifest remains visible instead of aborting the audit. (src/security/audit-extra.async.ts:1030, 957cc81175a3)
Regression coverage: The added test creates a 1 MiB + 1 byte manifest and verifies a warning finding containing the plugin name and too large detail. (src/security/audit-extra.async.test.ts:249, e127679198fa)
Real CLI proof: The contributor's July 7, 2026 transcript runs a deep JSON audit against a 1,048,577-byte installed manifest and shows the expected manifest_parse_error, exact size-limit detail, and exit code 0. (e127679198fa)
Safe-file ownership history: Repository history for the shared filesystem boundary repeatedly credits steipete with staged-write, symlink-race, TOCTOU-read, and related security hardening. (src/infra/fs-safe.ts:597, a0d5462)

Likely related people:

steipete: Recent history repeatedly connects this contributor to the fs-safe boundary, symlink-race, TOCTOU-read, and staged-file hardening used by this patch. (role: safe-file security boundary contributor; confidence: high; commits: a0d5462, 74ecdec, c823a85; files: src/infra/fs-safe.ts, src/security/audit-extra.async.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 (22 earlier review cycles; latest 8 shown)

reviewed 2026-07-12T22:48:41.726Z sha 7d63736 :: needs maintainer review before merge. :: none
reviewed 2026-07-12T23:33:57.947Z sha 7d63736 :: needs maintainer review before merge. :: none
reviewed 2026-07-13T05:41:00.282Z sha 1625fcb :: needs maintainer review before merge. :: none
reviewed 2026-07-14T13:44:13.849Z sha 98d2bfa :: needs maintainer review before merge. :: none
reviewed 2026-07-14T14:06:40.946Z sha 98d2bfa :: needs maintainer review before merge. :: none
reviewed 2026-07-15T04:21:34.140Z sha 35bec6b :: needs maintainer review before merge. :: none
reviewed 2026-07-15T05:32:54.152Z sha 7fdc9ce :: needs maintainer review before merge. :: none
reviewed 2026-07-15T06:52:58.855Z sha 7fdc9ce :: needs maintainer review before merge. :: none

cxbAsDev · 2026-07-07T16:23:53Z

@clawsweeper re-review — added real CLI proof with an oversized plugin manifest.

Created a 1 MiB + 1 byte (1048577 byte) ~/.openclaw/extensions/oversized-plugin/package.json in a temporary HOME and ran a deep audit:

$ ls -l /tmp/openclaw-plugin-proof/.openclaw/extensions/oversized-plugin/package.json
-rw-r--r-- 1 user user 1048577 Jul  8 00:17 /tmp/openclaw-plugin-proof/.openclaw/extensions/oversized-plugin/package.json

$ HOME=/tmp/openclaw-plugin-proof pnpm openclaw security audit --deep --json
$ node scripts/run-node.mjs security audit --deep --json
{
  "ts": 1783441389923,
  "summary": {
    "critical": 1,
    "warn": 7,
    "info": 1
  },
  "findings": [
    ...
    {
      "checkId": "plugins.code_safety.manifest_parse_error",
      "severity": "warn",
      "title": "Plugin \"oversized-plugin\" has a malformed package.json",
      "detail": "Could not parse plugin manifest: Error: Plugin manifest at /tmp/openclaw-plugin-proof/.openclaw/extensions/oversized-plugin/package.json is too large (1048577 bytes, max 1048576).\nThe extension entrypoint list is unavailable. Deep scan will cover the plugin directory but may miss entries declared via `openclaw.extensions`.",
      "remediation": "Inspect the plugin package.json for syntax errors. If the plugin is untrusted, remove it from your OpenClaw extensions state directory."
    },
    ...
  ],
  "secretDiagnostics": []
}
EXIT:0

The deep audit surfaces plugins.code_safety.manifest_parse_error with the expected "too large" detail for oversized-plugin, confirming the bounded manifest read catches an oversized package.json instead of buffering it into memory.

clawsweeper · 2026-07-07T16:23:56Z

🦞🧹
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-15T02:36:15Z

@clawsweeper re-review — branch is now rebased onto the latest upstream/main and the bounded plugin-manifest read is ready for maintainer review.

This PR addresses the unbounded deep-audit read of installed-plugin package.json files by capping them at 1 MiB and adding oversized-manifest regression coverage. No further code changes are planned from my side; the remaining blocker is maintainer review/land.

Could a maintainer please take a look? Thanks!

clawsweeper · 2026-07-15T02:36:18Z

🦞🧹
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: Review in progress
Detail: The exact-review queue leased this run; Codex is reviewing the item.
Run: https://github.com/openclaw/clawsweeper/actions/runs/29385601924
Updated: 2026-07-15T03:04:43.983Z

steipete · 2026-07-18T08:55:49Z

Merged via squash.

Prepared head SHA: e127679198faeeb47cee01ad41c3f1f33b3c7a02
Landed commit: aa0c92186b67c55d6f43ec6611ff689b66b65fd6

cxbAsDev requested a review from a team as a code owner July 7, 2026 15:53

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

cxbAsDev force-pushed the fix/bound-audit-plugin-manifest-read branch from 1260d8d to 699db8e Compare July 7, 2026 16:32

cxbAsDev force-pushed the fix/bound-audit-plugin-manifest-read branch 5 times, most recently from ca804bc to 12136cc Compare July 11, 2026 16:37

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 11, 2026

cxbAsDev force-pushed the fix/bound-audit-plugin-manifest-read branch 5 times, most recently from 2bf27c9 to 47d1257 Compare July 12, 2026 01:26

clawsweeper Bot added the merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. label Jul 12, 2026

cxbAsDev force-pushed the fix/bound-audit-plugin-manifest-read branch 3 times, most recently from 2643221 to 877d810 Compare July 12, 2026 04:37

cxbAsDev force-pushed the fix/bound-audit-plugin-manifest-read branch from e2294c1 to cf5d427 Compare July 12, 2026 10:38

cxbAsDev force-pushed the fix/bound-audit-plugin-manifest-read branch 9 times, most recently from 5330407 to be5bbe0 Compare July 15, 2026 02:16

cxbAsDev force-pushed the fix/bound-audit-plugin-manifest-read branch 2 times, most recently from 63e3fd3 to a9b3c64 Compare July 15, 2026 03:07

cxbAsDev force-pushed the fix/bound-audit-plugin-manifest-read branch from a9b3c64 to 7b55336 Compare July 15, 2026 03:31

fix(security): bound plugin manifest reads in audit deep scan

e127679

steipete mentioned this pull request Jul 18, 2026

fix(security): restore fs import dropped by bounded manifest-read change #110585

Merged

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

📡 Upstream Digest — 2026-07-18 09:16 UTC curtismercier/openclaw-mods#1332

Open

steipete mentioned this pull request Jul 18, 2026

fix(security): bound skill-file audit read (restore removed fs dependency) #110589

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(security): bound plugin manifest reads in audit deep scan#101773

fix(security): bound plugin manifest reads in audit deep scan#101773
steipete merged 1 commit into
openclaw:mainfrom
cxbAsDev:fix/bound-audit-plugin-manifest-read

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 15, 2026

Uh oh!

clawsweeper Bot commented Jul 15, 2026 •

edited

Loading

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 7, 2026

What Problem This Solves

Why This Change Was Made

User Impact

Evidence

Real behavior proof

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 15, 2026

Uh oh!

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

Uh oh!

Uh oh!

steipete commented Jul 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

clawsweeper Bot commented Jul 7, 2026 •

edited

Loading

clawsweeper Bot commented Jul 7, 2026 •

edited

Loading

clawsweeper Bot commented Jul 15, 2026 •

edited

Loading