Repository navigation
fix: isolate OpenClaw fixture writes from production state - #195
Conversation
Scrub inherited OpenClaw paths and service identity in the shared child-environment owner. Require marked self-check scopes or current-run OCM scopes before fixture writers touch config or dotenv files, rejecting production service targets and filesystem escapes.\n\nAdd red-on-base isolation and guard regressions, preserve cross-user transport, and document the scope boundary.
|
🦞👀 Pull request received. I will update this pull request when review starts. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4a8cc73e65
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const configured = canonical(value); | ||
| const protectedPath = match[1] === "HOME" ? path.join(configured, ".openclaw") | ||
| : match[1] === "CONFIG_PATH" ? path.dirname(configured) : configured; | ||
| if (within(protectedPath, target)) refuse(target, `configured by service ${entry}`); |
There was a problem hiding this comment.
When a disposable lab service declares OPENCLAW_HOME, OPENCLAW_STATE_DIR, or OPENCLAW_CONFIG_PATH, this check rejects its config even when the scope marker belongs to the current run. This breaks scenarios that intentionally configure an already-installed gateway: scenarios/exec-tool-safety.json installs and starts the service before run-exec-tool-safety.mjs calls assertOwnedOpenClawWrite, and agent-network-offline likewise configures its provider after starting the gateway. A synthetic current-run OCM scope passes validation before adding its user-service definition and fails immediately afterward. Distinguish services belonging to the verified disposable scope from production services instead of treating every service-configured path as production.
Useful? React with 👍 / 👎.
Problem
Running
node bin/kova.mjs self-checkfrom an OpenClaw Gateway agent's exec environment could overwrite the live host's OpenClaw config and.env. The command runners inheritedOPENCLAW_CONFIG_PATH,OPENCLAW_STATE_DIR, and service identity; these took precedence over self-check's temporaryOPENCLAW_HOME. The mock-auth writer accepted those paths without an ownership check.Incident timeline, 2026-10-31 UTC, on a production OpenClaw host running a systemd Gateway:
.envwas replaced with the two-line mock fixture. The original file was lost.openclaw.jsonwas rewritten with mock token auth, a loopback model-provider URL, and mock model/runtime settings. Gateway config validation prevented activation, but the CLI broke immediately.Fix
A single child-environment owner now removes inherited OpenClaw variables, legacy location variables, OCM active-env identity, and systemd/socket-activation identity before applying explicit Kova scopes. Both command runners, resource lookups, and OCM artifact launches use it.
Config and
.envfixture writers now share a fail-closed guard. Self-check creates an ownership marker in its temporary workspace. The runner marks OCM environments after successful provisioning or cloning, with a current-run owner; copied clone markers are rebound to the clone. Writers reject unmarked scopes, another run's OCM scope, service identity, default user state, service-configured paths, and symlink/hardlink escapes. Both config and.envdestinations are checked before the first write. There is no override flag. Cross-user OCM transport carries the same guard with its fixed helper sources.README documents the boundary. The changelog entry is under the checkout's current
Unreleased/Fixedsection: this branch is already 0.2.2 and its 0.1.4 entry is a dated historical release.Evidence
Red on base
54280b1c5218d47f44b9515cc4aabfafbc5eb07e: three incident regressions fail in a clean Git archive. The child runner retains inherited production environment identity, and the helper overwrites the temporary production fixture both with service markers and without a Kova ownership marker.Green:
node --test tests/openclaw-state-isolation.mjspasses all six tests. Under simulated production inheritance, the marked self-check scope receives mock config/auth while both production sentinel files remain byte-identical. Guard tests cover independently resolved config/.envpaths, service identity and service definitions/drop-ins, symlink/hardlink escapes, OCM run ownership, and cloning.Full repository proof:
npm run check:fullpasses on Node.js 26.12.1/macOS, with the CI-pinned OCM 0.2.48 binary in a disposableHOMEandOCM_HOME. This includes all 282 self-checks, isolation/transport and accounting tests, snapshots, web-payload validation, and synthetic release-contract checks.Independent review: isolated Codex autoreview is clean through P2. Initial local runs exposed the missing OCM prerequisite, two synthetic wrappers that relied on inherited locations/identity, and an over-broad dispatcher scrub; these were corrected before the final green run.
Exact-head CI: run 41160233651 passed for
4a8cc73e657dbfb5a5c1eacbc1a264c8dc94f985: Linux Node 22/24/26, macOS Node 24, and Website. All PR CodeQL checks also passed. No CI retry or follow-up push was needed.All execution used temporary synthetic fixtures. No real OpenClaw installation or live host was exercised. This PR is ready for coordinator admin merge; it must not be merged by this lane.