Skip to content

Update README.md#6

Closed
miyoungc wants to merge 2 commits into
mainfrom
miyoungc-patch-1
Closed

Update README.md#6
miyoungc wants to merge 2 commits into
mainfrom
miyoungc-patch-1

Conversation

@miyoungc
Copy link
Copy Markdown
Contributor

No description provided.

@miyoungc miyoungc closed this Mar 16, 2026
jessesanford pushed a commit to jessesanford/NemoClaw that referenced this pull request Mar 24, 2026
Add onboard as an alias for setup
ericksoa pushed a commit that referenced this pull request May 1, 2026
…ons land cleanly (#2681) (#2851)

## Summary

Replaces the EACCES-swallow approach proposed in #2693 with proper Unix
group permissions. Control-UI toggles in the OpenClaw dashboard (Enable
Dreaming, account toggles, etc.) now **persist in default mode** instead
of throwing `GatewayRequestError: EACCES` or becoming silent no-ops.

## Background

OpenClaw 2026.4.24 (landed via #2484) introduced `mutateConfigFile` as
the new control-UI write path. Patch 4 in the Dockerfile only wraps the
legacy `replaceConfigFile` (plugin install path), so every config-toggle
click in the sandbox dashboard now EACCES'd.

#2693 proposed adding "Patch 4b" — a parallel try/catch that swallows
the EACCES. That makes toggles non-functional in the sandbox: the user
clicks "Enable Dreaming," gets no error, but nothing actually persists.
UX improves over the crash; underlying limitation stays.

This PR implements the alternative design Aaron sketched for #2681:
rather than wrapping each new write path in EACCES handlers, fix the
actual permissions so the writes succeed.

## Closes / Supersedes

- Closes #2681
- Supersedes #2693 — thanks @Sanjays2402 for raising the issue and the
initial swallow attempt that surfaced the deeper design question

## Implementation (the 6-item spec)

| # | Item | File |
|---|------|------|
| 1 | Keep `gateway` as a separate UID from `sandbox`; add it to the
`sandbox` group | `Dockerfile.base` |
| 2 | Stale-base fallback so older `sandbox-base:latest` tags get the
group membership at derived-image build time | `Dockerfile` |
| 3 | `/sandbox/.openclaw` group-writable + setgid on dirs;
`.config-hash` file mode 664 | `Dockerfile.base`, `Dockerfile` |
| 4 | `normalize_mutable_config_perms()` at startup, gated on Shields
state | `scripts/nemoclaw-start.sh` |
| 5 | `shields down` restores 660/2770 (group-writable + setgid) for
OpenClaw; Hermes left at historical 640/750 (no separate gateway UID,
contract doesn't apply) | `src/lib/shields.ts` |
| 6 | Tests assert the new invariant: writes succeed in default mode, no
new EACCES swallow | `test/repro-2681-group-writable.test.ts` |

## Why setgid

`chmod g+s` on directories means new files inherit `group=sandbox`
regardless of which UID created them. So `gateway` writes a file → file
is `group=sandbox` → the `sandbox` user (also in the group) can still
read it. Without setgid, gateway's writes would land with
`group=gateway` and the agent might lose read access on rotation.

## Patch 4 retention

The existing `Patch 4` (replaceConfigFile EACCES swallow) is
**intentionally retained** as a defensive fallback for:

- Older base images during the rollout window
- Host filesystems that don't honor setgid (rare, but possible on some
Windows/WSL2 configurations)
- Other write paths in OpenClaw that might surface in future versions

No new EACCES swallow patch is added — the `Patch 4b` approach from
#2693 is explicitly rejected per spec item #6.

## Verification

- [x] `npm run build:cli` compiles the changed `shields.ts`
- [x] 11/11 new tests pass in `test/repro-2681-group-writable.test.ts` —
assert structural invariants of the group-writable contract
- [x] 443/443 plugin tests pass
- [x] Pre-existing CLI tests that fail on this branch ALSO fail on
pristine main (`@oclif/core` module-not-found from in-flight migration;
not caused by this PR)
- [ ] **Brev E2E required** — touches Dockerfile + Dockerfile.base +
shields lifecycle. Adaptive matrix: M×DANGER → full Brev sweep before
merge

## Test plan

- [x] Unit: 11 structural assertions in
`repro-2681-group-writable.test.ts`
- [ ] CI: `build-sandbox-images` (validates the group-membership +
setgid Dockerfile changes)
- [ ] CI: `test-e2e-sandbox` (validates shields lifecycle + onboard
flow)
- [ ] CI: `test-e2e-gateway-isolation` (validates the
gateway-as-different-UID still runs cleanly)
- [ ] Manual repro: onboard, click "Enable Dreaming" in dashboard,
verify mutation persists across `nemoclaw status`

## Type of Change

- [x] Code change (feature, bug fix, or refactor)

## AI Disclosure

- [x] AI-assisted — tool: Claude Code
jyaunches added a commit to jyaunches/NemoClaw that referenced this pull request May 11, 2026
…unchable

Run NVIDIA#6 failed at refreshAndWaitForSsh with 'SSH not ready after 40
attempts' but produced NO diagnostic output between env-dump and
failure. Two fixes:

1. vitest.config.ts: override silent:isCi for the e2e-branch-validation
   project. The whole suite is one describe block so there's no test
   chatter to suppress, and diagnostic output from createBrevInstance
   / waitForSsh / waitForLaunchableReady is essential for Brev timing
   debugging. Local runs already show this; CI should too.

2. waitForSsh: default attempts by mode. Bare-VM startup-script path
   keeps 40 attempts (~5 min). Published launchable gets 96 attempts
   (~12 min) because the pre-baked image boot is fast (~2 min) but
   Brev platform SSH-proxy registration for launchable-provisioned
   instances appears to lag several more minutes before brev refresh
   picks them up. Empirical from run NVIDIA#6: instance created but SSH
   never came up in the 5-min window.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant