Best possible solution:

Handle stdout/stderr stream errors through the same settle-and-reject path as child process errors so runInstallPolicy fails closed, with regression coverage that expects security_scan_failed rather than allow.

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

Yes at source level: current main has stdout/stderr data listeners without error listeners, and Node throws unhandled error events without a listener. I did not run a full OpenClaw install-policy scenario in this read-only review.

Is this the best way to solve the issue?

No. Adding listeners is the right direction, but no-op listeners are not the best fix for a security policy boundary; the stream errors should reject so the existing fail-closed path handles them.

Full review comments:

• [P1] Fail closed instead of swallowing stream errors — src/security/install-policy.ts:603-605
  These no-op handlers prevent the crash, but they also let runPolicyCommand keep resolving successfully if stdout or stderr emits an error and the buffered stdout still parses as allow. security.installPolicy is the operator allow/block boundary and its docs define policy execution failures as fail-closed, so this handler should settle/reject and let runInstallPolicy return security_scan_failed instead of allowing the install.
  Confidence: 0.9

Overall correctness: patch is incorrect
Overall confidence: 0.9

AGENTS.md: found and applied where relevant.

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

Label changes:

• add P2: This is a focused security-boundary bugfix PR for an operator install-policy edge case, with limited blast radius but merge-blocking correctness concerns.
• add merge-risk: 🚨 security-boundary: Merging the no-op handlers could allow installs to proceed after policy stdout/stderr stream read errors instead of failing closed.
• add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🧂 unranked krab.
• add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body cites a mocked spawn regression test and oxlint only; add redacted terminal/log/live-output proof of the real install-policy path after the fix and update the PR body to trigger re-review.

Label justifications:

• P2: This is a focused security-boundary bugfix PR for an operator install-policy edge case, with limited blast radius but merge-blocking correctness concerns.
• merge-risk: 🚨 security-boundary: Merging the no-op handlers could allow installs to proceed after policy stdout/stderr stream read errors instead of failing closed.
• rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🧂 unranked krab.
• status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body cites a mocked spawn regression test and oxlint only; add redacted terminal/log/live-output proof of the real install-policy path after the fix and update the PR body to trigger re-review.

PR surface:

Source +2, Tests +84. Total +86 across 2 files.

Security concerns:

• [high] Install policy stream errors are ignored — src/security/install-policy.ts:603
  The proposed no-op handlers can convert a policy output read failure into a successful allow decision, which weakens a security boundary that should fail closed on policy execution failure.
  Confidence: 0.86

What I checked:

• PR diff changes stream-error handling: The PR adds no-op error listeners on child.stdout and child.stderr and its new test expects runInstallPolicy to resolve {} after emitting both stream errors. (src/security/install-policy.ts:603, 7d937f09e4d1)
• Current main behavior lacks stdout/stderr error handling: Current origin/main still attaches only data listeners for stdout/stderr in runPolicyCommand, so the central crash source is not already implemented on main. (src/security/install-policy.ts:603, 3f380635f59b)
• Install policy contract is fail-closed: The docs say the policy command must write a JSON decision on stdout and that non-zero exit, timeout, malformed JSON, missing fields, or unsupported protocol versions fail closed. Public docs: docs/tools/skills-config.md. (docs/tools/skills-config.md:187, d7e2096dc78e)
• Caller treats unblocked results as install-allowed: runOperatorInstallPolicy returns no block when runInstallPolicy returns an allowed result, so swallowing a stdout stream error can allow the install path to continue. (src/plugins/install-security-scan.runtime.ts:877, d7e2096dc78e)
• Node error-event behavior checked: A focused Node command confirmed an EventEmitter error event without a listener throws as an unhandled error, matching the PR's crash premise.
• Feature provenance: git blame shows runPolicyCommand and the install-policy execution path were introduced in commit 2bc50d0656bd3c5905f0d972ff429c8388c43b9e, from merged PR feat: verify AI access during macOS onboarding before the first chat #100288. (src/security/install-policy.ts:524, 2bc50d0656bd)

Likely related people:

• steipete: git blame and PR metadata show the install-policy runner, its docs, and the scan caller came from 2bc50d0656bd3c5905f0d972ff429c8388c43b9e / feat: verify AI access during macOS onboarding before the first chat #100288. (role: feature-history owner; confidence: high; commits: 2bc50d0656bd, c757675f348d; files: src/security/install-policy.ts, src/security/install-policy.test.ts, src/plugins/install-security-scan.runtime.ts)
• openclaw/openclaw-secops: .github/CODEOWNERS assigns src/security/ to this security review team, and this PR changes that security-sensitive path. (role: CODEOWNERS review owner; confidence: high; files: .github/CODEOWNERS, src/security/install-policy.ts)

• 🦀 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.

• 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.