Skip to content

Commit 7dafe0b

Browse files
HOYALIMprekshivyascvapurvvkumariasenthilr-nv
authored
fix(snapshot): preserve sealed OpenClaw config (#9231)
<!-- markdownlint-disable MD041 --> ## Summary Preserve `openclaw.json` when rebuilding an older OpenClaw sandbox whose sealed mutable-config layout is valid but unreadable through the sandbox-user SSH transport. The ordinary SSH path remains first; only its permission-class failure can invoke a bounded, descriptor-safe privileged read of the declared config file before the existing local secret sanitizer runs. ## Related Issue Refs #9215 (addresses ask 1: preserve the sealed config during rebuild backup) ## Changes - Add an internal state-file capture fallback that runs only after an ordinary SSH read exits with status 1. - Authorize the fallback only for `/sandbox/.openclaw/openclaw.json` with the copy strategy. - Capture through the registered sandbox's privileged container lease while rejecting symbolic links, hard links, non-regular files, files over 16 MiB, and files or directories that change during the read. - Keep existing backup sanitization and manifest publication unchanged. - Exercise the production capture script against the accepted regular-file boundary and every rejected file-type, link, size, and replacement boundary. - Cover the full SSH-denied fallback-to-sanitized-backup path. ## Type of Change - [x] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [ ] Docs updated for user-facing behavior changes - [x] Docs not applicable — justification: This repairs an internal rebuild snapshot transport and does not change commands, flags, or the supported user workflow. - [x] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [x] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: The fallback is path- and strategy-scoped, runs under the existing privileged execution lease, performs a bounded descriptor read with no-follow, single-link, file-type, size, and stable-read checks, and still passes captured bytes through the existing snapshot sanitizer. - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Documentation Writer Review - [x] Documentation writer subagent reviewed the completed changes - Result: `no-docs-needed` - Evidence: The current branch revision restores the existing rebuild backup contract through an internal transport fallback. It does not change a documented command, option, configuration, default, or workflow. - Agent: Codex Desktop ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: - Station profile/scenario: - Result: - Supporting evidence: ## Verification - [x] PR description includes a `Signed-off-by:` line and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run validate:pr` passed after refreshing `origin/main` when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — current CI completed the production-script fixture, authority, and full snapshot integration coverage without a reported assertion failure. A separate snapshot test reached its existing 5-second timeout after more than six minutes of shard execution; one bounded failed-job retry is in progress. - [x] Applicable broad gate passed — the normal CI run passed static checks, build/typecheck, installer integration, package checks, and 11 of 12 CLI shards; the remaining bounded retry must pass before merge. - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) - [ ] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) --- Signed-off-by: Ho Lim <subhoya@gmail.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Preserved the declared OpenClaw configuration after an ordinary SSH read fails for a permission-class reason. - Added a bounded privileged descriptor read for the declared regular file, with path, file-type, link-count, size, and stable-read controls. - Kept the existing configuration sanitizer before backup publication. - **Tests** - Added coverage for the accepted regular-file boundary and rejected link, file-type, size, and concurrent-replacement boundaries. - Added integration coverage for the SSH-denied fallback and sanitized backup result. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Ho Lim <subhoya@gmail.com> Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> Signed-off-by: Apurv Kumaria <akumaria@nvidia.com> Co-authored-by: Prekshi Vyas <prekshiv@nvidia.com> Co-authored-by: Carlos Villela <cvillela@nvidia.com> Co-authored-by: Apurv Kumaria <akumaria@nvidia.com> Co-authored-by: Senthil Ravichandran <senthilr@nvidia.com>
1 parent 076d254 commit 7dafe0b

6 files changed

Lines changed: 681 additions & 14 deletions

File tree

src/lib/actions/sandbox/rebuild-flow-lifecycle.test.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,10 @@ describe("rebuildSandbox flow: lifecycle", () => {
8484
).resolves.toBeUndefined();
8585

8686
expect(harness.backupSandboxStateSpy).toHaveBeenCalledOnce();
87-
expect(harness.backupSandboxStateSpy).toHaveBeenCalledWith("alpha");
87+
expect(harness.backupSandboxStateSpy).toHaveBeenCalledWith(
88+
"alpha",
89+
expect.objectContaining({ captureStateFile: expect.any(Function) }),
90+
);
8891
expect(harness.prepareMcpBridgesForRebuildSpy).toHaveBeenCalledWith("alpha");
8992
expect(harness.prepareMcpBridgesForRebuildSpy.mock.invocationCallOrder[0]).toBeLessThan(
9093
harness.warnUnpreservedUserManagedFilesSpy.mock.invocationCallOrder[0],
Lines changed: 179 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,179 @@
1+
// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
import fs from "node:fs";
5+
import os from "node:os";
6+
import path from "node:path";
7+
import { spawnSync } from "node:child_process";
8+
9+
import { afterEach, describe, expect, it } from "vitest";
10+
11+
import { OPENCLAW_CONFIG_CAPTURE_SCRIPT } from "./backup-authority";
12+
13+
const CONFIG_NAME = "openclaw.json";
14+
const MAX_CONFIG_BYTES = 16 * 1024 * 1024;
15+
const PROTOCOL_PREFIX = "nemoclaw-openclaw-config-capture:";
16+
const fixtureRoots: string[] = [];
17+
18+
interface CaptureResult {
19+
readonly status: number | null;
20+
readonly stdout: Buffer;
21+
readonly stderr: string;
22+
}
23+
24+
function fixtureDirectory(): string {
25+
const root = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-openclaw-capture-"));
26+
fixtureRoots.push(root);
27+
const directory = path.join(root, ".openclaw");
28+
fs.mkdirSync(directory);
29+
return directory;
30+
}
31+
32+
function runCapture(directory: string, script = OPENCLAW_CONFIG_CAPTURE_SCRIPT): CaptureResult {
33+
const result = spawnSync("/usr/bin/python3", ["-I", "-S", "-c", script, directory, CONFIG_NAME], {
34+
encoding: null,
35+
timeout: 30_000,
36+
maxBuffer: MAX_CONFIG_BYTES + 1024 * 1024,
37+
});
38+
return {
39+
status: result.status,
40+
stdout: Buffer.isBuffer(result.stdout) ? result.stdout : Buffer.alloc(0),
41+
stderr: Buffer.isBuffer(result.stderr) ? result.stderr.toString("utf8") : "",
42+
};
43+
}
44+
45+
function mutationHarness(mutation: string): string {
46+
return `import os, sys
47+
capture_script = ${JSON.stringify(OPENCLAW_CONFIG_CAPTURE_SCRIPT)}
48+
directory = sys.argv[1]
49+
name = sys.argv[2]
50+
real_read = os.read
51+
mutated = False
52+
def mutate_after_first_read(fd, size):
53+
global mutated
54+
data = real_read(fd, size)
55+
if not mutated:
56+
mutated = True
57+
${mutation}
58+
return data
59+
os.read = mutate_after_first_read
60+
exec(capture_script)
61+
`;
62+
}
63+
64+
afterEach(() => {
65+
for (const root of fixtureRoots.splice(0)) {
66+
fs.rmSync(root, { recursive: true, force: true });
67+
}
68+
});
69+
70+
describe("OpenClaw privileged config capture script", () => {
71+
it("returns bytes only for a stable regular file", () => {
72+
const directory = fixtureDirectory();
73+
const expected = Buffer.from('{"models":{"default":"nvidia/test"}}\n');
74+
fs.writeFileSync(path.join(directory, CONFIG_NAME), expected);
75+
76+
const result = runCapture(directory);
77+
78+
expect(result).toEqual({ status: 0, stdout: expected, stderr: "" });
79+
});
80+
81+
it("returns all bytes for a stable file at the 16 MiB limit", () => {
82+
const directory = fixtureDirectory();
83+
const expected = Buffer.alloc(MAX_CONFIG_BYTES, 0xa5);
84+
fs.writeFileSync(path.join(directory, CONFIG_NAME), expected);
85+
86+
const result = runCapture(directory);
87+
88+
expect(result.status).toBe(0);
89+
expect(result.stderr).toBe("");
90+
expect(result.stdout).toHaveLength(expected.length);
91+
expect(result.stdout.equals(expected)).toBe(true);
92+
});
93+
94+
it.each([
95+
{
96+
kind: "symbolic link",
97+
setup(directory: string) {
98+
const target = path.join(path.dirname(directory), "target.json");
99+
fs.writeFileSync(target, "target");
100+
fs.symlinkSync(target, path.join(directory, CONFIG_NAME));
101+
},
102+
},
103+
{
104+
kind: "hard link",
105+
setup(directory: string) {
106+
const target = path.join(path.dirname(directory), "target.json");
107+
fs.writeFileSync(target, "target");
108+
fs.linkSync(target, path.join(directory, CONFIG_NAME));
109+
},
110+
},
111+
{
112+
kind: "FIFO",
113+
setup(directory: string) {
114+
const result = spawnSync("mkfifo", [path.join(directory, CONFIG_NAME)]);
115+
expect(result.status).toBe(0);
116+
},
117+
},
118+
{
119+
kind: "directory",
120+
setup(directory: string) {
121+
fs.mkdirSync(path.join(directory, CONFIG_NAME));
122+
},
123+
},
124+
{
125+
kind: "oversized file",
126+
setup(directory: string) {
127+
const descriptor = fs.openSync(path.join(directory, CONFIG_NAME), "w");
128+
try {
129+
fs.ftruncateSync(descriptor, MAX_CONFIG_BYTES + 1);
130+
} finally {
131+
fs.closeSync(descriptor);
132+
}
133+
},
134+
},
135+
])("rejects a $kind without returning captured bytes", ({ setup }) => {
136+
const directory = fixtureDirectory();
137+
setup(directory);
138+
139+
const result = runCapture(directory);
140+
141+
expect(result.status).not.toBe(0);
142+
expect(result.stdout).toEqual(Buffer.alloc(0));
143+
expect(result.stderr).toContain(PROTOCOL_PREFIX);
144+
});
145+
146+
it("rejects a file replaced during the read without returning captured bytes", () => {
147+
const directory = fixtureDirectory();
148+
fs.writeFileSync(path.join(directory, CONFIG_NAME), "original");
149+
const script = mutationHarness(
150+
` original = os.path.join(directory, name)\n` +
151+
` os.rename(original, original + ".old")\n` +
152+
` with open(original, "wb") as replacement:\n` +
153+
` replacement.write(b"replacement")`,
154+
);
155+
156+
const result = runCapture(directory, script);
157+
158+
expect(result.status).toBe(13);
159+
expect(result.stdout).toEqual(Buffer.alloc(0));
160+
expect(result.stderr).toBe(`${PROTOCOL_PREFIX}file-changed-during-read\n`);
161+
});
162+
163+
it("rejects a directory replaced during the read without returning captured bytes", () => {
164+
const directory = fixtureDirectory();
165+
fs.writeFileSync(path.join(directory, CONFIG_NAME), "original");
166+
const script = mutationHarness(
167+
` os.rename(directory, directory + ".old")\n` +
168+
` os.mkdir(directory)\n` +
169+
` with open(os.path.join(directory, name), "wb") as replacement:\n` +
170+
` replacement.write(b"replacement")`,
171+
);
172+
173+
const result = runCapture(directory, script);
174+
175+
expect(result.status).toBe(13);
176+
expect(result.stdout).toEqual(Buffer.alloc(0));
177+
expect(result.stderr).toBe(`${PROTOCOL_PREFIX}directory-changed-during-read\n`);
178+
});
179+
});

0 commit comments

Comments
 (0)