Skip to content

fix(codex): restrict computer-use installation#97955

Merged
eleqtrizit merged 1 commit into
openclaw:mainfrom
eleqtrizit:798
Jun 30, 2026
Merged

fix(codex): restrict computer-use installation#97955
eleqtrizit merged 1 commit into
openclaw:mainfrom
eleqtrizit:798

Conversation

@eleqtrizit

@eleqtrizit eleqtrizit commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

What Problem This Solves

Fixes an issue where an authorized non-owner chat sender could run /codex computer-use install and select Codex marketplace, plugin, and MCP server resources. Installation mutates trusted host state and can make installed MCP processes available without the owner or operator.admin authority required by sibling plugin-write commands.

Why This Change Was Made

The Codex command handler now applies the existing owner-or-operator.admin policy only to the mutating install action. Read-only status behavior, including existing overrides, remains available to authorized senders for compatibility.

User Impact

Owners and operator.admin Gateway clients retain the complete Computer Use setup workflow. Other authorized senders can still inspect status but can no longer install or reconfigure trusted Computer Use resources.

Evidence

  • A focused production-entry-point regression proves non-owner installation is denied before installCodexComputerUse is called.
  • An operator.admin regression proves privileged installation remains available.
  • A compatibility regression covers every status override class and proves the read-only path never invokes installation.
  • Direct inspection of OpenAI Codex rust-v0.139.0 confirmed that plugin/install installs and loads plugin MCP definitions, while plugin/read is the separate status lookup path.
  • Fresh structured Codex autoreview completed with no accepted or actionable findings.

Real Codex app-server behavior proof

Ran the production handleCodexCommand entry point against the bundled @openai/codex 0.139.0 app-server with an isolated OPENCLAW_STATE_DIR and a disposable local marketplace/plugin whose MCP server exposes one proof tool. No model credentials or personal Codex state were used.

$ corepack pnpm exec tsx /tmp/openclaw-798-real-app-server-proof.ts
REAL_CODEX_APP_SERVER_PROOF=PASS
CODEX_VERSION=0.139.0
NON_OWNER_OPERATOR_WRITE=DENIED_BEFORE_INSTALL
DENIED_PLUGIN_CACHE_ENTRIES=0
DENIED_MCP_LAUNCHES=0
OPERATOR_ADMIN=INSTALL_ALLOWED
ADMIN_PLUGIN_CACHE_ENTRIES=1
ADMIN_MCP_LAUNCHED=true
ADMIN_RESPONSE_READY=true

The same disposable setup was used for both calls. The denied operator.write caller returned before Codex plugin installation or MCP launch; the operator.admin caller installed the plugin through the real app-server, launched its MCP process, discovered its tool, and returned Computer Use: ready. The temporary state was removed after the run.

Summary

Restrict Codex Computer Use installation to trusted owner/admin callers without breaking the existing read-only command surface.

Changes

  • Share the Codex host-mutation authorization predicate with plugin-management writes.
  • Gate /codex computer-use install at the command-handler boundary.
  • Preserve help, menu, plain status, override-bearing status, aliases, flags, config, and protocol behavior.
  • Document the install authorization boundary.

Validation

  • node scripts/run-vitest.mjs extensions/codex/src/commands.test.ts extensions/codex/src/command-plugins-management.test.ts — 134 passed.
  • corepack pnpm build — passed.
  • corepack pnpm exec oxfmt --check extensions/codex/src/command-authorization.ts extensions/codex/src/command-handlers.ts extensions/codex/src/command-plugins-management.ts extensions/codex/src/commands.test.ts docs/plugins/codex-computer-use.md — passed.
  • corepack pnpm docs:check-mdx — passed for 685 files.
  • git diff --check upstream/main...HEAD — passed.

Notes

  • AI-assisted change; the implementation and dependency behavior were manually verified.
  • No config, schema, migration, gateway protocol, or Plugin SDK contract changes.
  • No changelog entry; release generation owns CHANGELOG.md.
  • Remote Crabbox/Testbox proof was unavailable in this environment because the Crabbox binary failed its startup sanity check and no Blacksmith client was installed; the real dependency process was exercised locally instead.

@openclaw-barnacle openclaw-barnacle Bot added docs Improvements or additions to documentation extensions: codex size: S maintainer Maintainer-authored PR labels Jun 29, 2026
@clawsweeper

clawsweeper Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed June 29, 2026, 8:08 PM ET / 00:08 UTC.

Summary
The PR adds a shared Codex host-mutation authorization helper, gates /codex computer-use install behind owner or operator.admin, updates sibling plugin management to use the helper, and documents/tests the boundary.

PR surface: Source +4, Tests +62, Docs +3. Total +69 across 5 files.

Reproducibility: yes. at source level: current main routes authorized /codex computer-use install to installCodexComputerUse without the owner/admin gate used by sibling plugin writes. I did not rerun the live app-server proof because this was a read-only review, but the PR body includes after-fix real app-server output.

Review metrics: 1 noteworthy metric.

  • Host mutation gates: 1 install action restricted; 1 existing write predicate shared. This is the compatibility-sensitive authorization boundary maintainers need to approve before merge.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
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.

Rank-up moves:

  • none.

Risk before merge

  • [P1] Merging intentionally changes existing behavior for authorized non-owner, non-admin senders from allowing /codex computer-use install to returning an authorization denial.
  • [P1] The protected maintainer label and linked GHSA/security context require explicit maintainer/security acceptance before merge.

Maintainer options:

  1. Accept the fail-closed hardening (recommended)
    Maintainers can merge after explicitly accepting that previous non-owner install access is being removed while owner and operator.admin installs remain supported.
  2. Pause for a lower-layer authorization design
    If maintainers want every Codex install caller gated below the chat command layer, pause this PR and design that app-server/plugin-runtime boundary separately.

Next step before merge

  • Manual review is the right lane because the remaining action is accepting a protected security/compatibility policy change, not repairing code.

Security
Cleared: The diff tightens an authorization boundary and does not add dependencies, workflows, package scripts, downloaded code, secrets handling, or new supply-chain surface.

Review details

Best possible solution:

Land a maintainer-reviewed version that gates only install-time host mutation, preserves status reads, and explicitly accepts the fail-closed compatibility change for non-owner authorized senders.

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

Yes, at source level: current main routes authorized /codex computer-use install to installCodexComputerUse without the owner/admin gate used by sibling plugin writes. I did not rerun the live app-server proof because this was a read-only review, but the PR body includes after-fix real app-server output.

Is this the best way to solve the issue?

Yes, this is the best inspected fix shape: the PR gates the mutating chat install action before Codex can add marketplaces or install plugins, while preserving status as a read path and matching the sibling /codex plugins write boundary. A deeper app-server-wide gate would be broader and needs separate maintainer design.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P0: The PR addresses a security authorization boundary where a non-owner authorized sender could mutate trusted Codex marketplace, plugin, and MCP host state.
  • merge-risk: 🚨 compatibility: The patch changes an existing authorized non-owner workflow for /codex computer-use install from allowed to denied.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit 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 comment provide terminal output from a real bundled Codex 0.139.0 app-server setup showing denied non-owner install and successful operator.admin install after the change.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body and follow-up comment provide terminal output from a real bundled Codex 0.139.0 app-server setup showing denied non-owner install and successful operator.admin install after the change.
Evidence reviewed

PR surface:

Source +4, Tests +62, Docs +3. Total +69 across 5 files.

View PR surface stats
Area Files Added Removed Net
Source 3 13 9 +4
Tests 1 62 0 +62
Docs 1 3 0 +3
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 5 78 9 +69

What I checked:

Likely related people:

  • pash-openai: Authored and merged the original Codex Computer Use setup PR that added computer-use.ts, command handling, docs, and tests for this surface. (role: feature introducer; confidence: high; commits: 67ffa3df8b07; files: extensions/codex/src/app-server/computer-use.ts, extensions/codex/src/command-handlers.ts, docs/plugins/codex-computer-use.md)
  • steipete: Recent path history shows multiple Computer Use setup-state hardening and marketplace auto-registration changes in the affected implementation and docs. (role: recent area contributor; confidence: high; commits: f7815cdd8f36, ea2d95e23e87, 697d85aefeb4; files: extensions/codex/src/app-server/computer-use.ts, docs/plugins/codex-computer-use.md)
  • kevinslin: Authored the sibling /codex plugins management surface and owner-or-operator.admin write gate that this PR reuses for Computer Use install. (role: adjacent owner; confidence: high; commits: 9b97e1ef2fd2, e82d19fb06b5, c5d34c8376f8; files: extensions/codex/src/command-plugins-management.ts, extensions/codex/src/command-plugins-management.test.ts, extensions/codex/src/commands.test.ts)
  • bdjben: Recently updated Codex Computer Use setup behavior and docs around app-server selection, adjacent to the install path changed here. (role: recent adjacent contributor; confidence: medium; commits: b3ac552c821b, 7691a8a9a371; files: docs/plugins/codex-computer-use.md, extensions/codex/src/app-server/shared-client.ts, extensions/codex/src/app-server/managed-binary.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.

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P0 Emergency: data loss, security bypass, crash loop, or unusable core runtime. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. labels Jun 29, 2026
@eleqtrizit

eleqtrizit commented Jun 29, 2026

Copy link
Copy Markdown
Contributor Author

Fresh real behavior proof rerun against the current merged main source.

The production handleCodexCommand entry point was exercised against the bundled real @openai/codex 0.139.0 app-server. The run used an isolated OpenClaw state directory plus a disposable local marketplace, plugin, and stdio MCP server exposing one proof tool. Model credentials were cleared from the child environment, and the temporary state was removed afterward.

REAL_CODEX_APP_SERVER_PROOF=PASS
CODEX_VERSION=0.139.0
NON_OWNER_OPERATOR_WRITE=DENIED_BEFORE_INSTALL
DENIED_PLUGIN_CACHE_ENTRIES=0
DENIED_MCP_LAUNCHED=false
OPERATOR_ADMIN=INSTALL_ALLOWED
ADMIN_PLUGIN_CACHE_ENTRIES=2
ADMIN_MCP_LAUNCHED=true
ADMIN_RESPONSE_READY=true
NON_OWNER_STATUS_READ=READY

This is behavioral, not mock-only, proof:

  • The non-owner operator.write request returned at the command authorization boundary before the real app-server created plugin cache state or launched the MCP child process.
  • The operator.admin request crossed the same production command path, installed the disposable plugin through the real app-server, launched its stdio MCP process, discovered its tool, and returned Computer Use: ready.
  • A non-owner operator.write status request then read the installed state successfully, proving the compatibility-preserving read path remains available.

Focused regression validation also passed:

node scripts/run-vitest.mjs extensions/codex/src/commands.test.ts
Test Files  1 passed (1)
Tests       125 passed (125)

The dependency contract was checked directly at OpenAI Codex tag rust-v0.139.0: plugin/install loads plugin MCP definitions, and local stdio MCP servers are launched as child processes. Together, the source-level regressions and the real dependency process run prove both sides of the intended boundary.

@clawsweeper

clawsweeper Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper could not start a re-review for this item.

Reason: re-review requires an open issue or PR.

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jun 30, 2026
@eleqtrizit
eleqtrizit merged commit e87b0df into openclaw:main Jun 30, 2026
190 of 203 checks passed
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jun 30, 2026
@eleqtrizit
eleqtrizit deleted the 798 branch June 30, 2026 19:27
@eleqtrizit

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs Improvements or additions to documentation extensions: codex maintainer Maintainer-authored PR merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. P0 Emergency: data loss, security bypass, crash loop, or unusable core runtime. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: S status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant